]> granicus.if.org Git - clang/commitdiff
[analyzer] When a symbol is null, we should track its constraints.
authorJordan Rose <jordan_rose@apple.com>
Fri, 3 Aug 2012 23:09:01 +0000 (23:09 +0000)
committerJordan Rose <jordan_rose@apple.com>
Fri, 3 Aug 2012 23:09:01 +0000 (23:09 +0000)
Because of this, we would previously emit NO path notes when a parameter
is constrained to null (because there are no stores). Now we show where we
made the assumption, which is much more useful.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161280 91177308-0d34-0410-b5e6-96231b3b80d8

18 files changed:
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
lib/StaticAnalyzer/Checkers/AttrNonNullChecker.cpp
lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
lib/StaticAnalyzer/Checkers/ObjCAtSyncChecker.cpp
lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp
lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Checkers/UndefinedArraySubscriptChecker.cpp
lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
lib/StaticAnalyzer/Checkers/VLASizeChecker.cpp
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
test/Analysis/inlining/path-notes.c
test/Analysis/method-call-path-notes.cpp

index 3e62a920b76edc73334d72899ea07b2babb1b54d..f53c15f117c97004ba7839394386b7b4f2dd1844 100644 (file)
@@ -226,9 +226,8 @@ public:
   
 namespace bugreporter {
 
-BugReporterVisitor *getTrackNullOrUndefValueVisitor(const ExplodedNode *N,
-                                                    const Stmt *S,
-                                                    BugReport *R);
+void addTrackNullOrUndefValueVisitor(const ExplodedNode *N, const Stmt *S,
+                                     BugReport *R);
 
 const Stmt *GetDerefExpr(const ExplodedNode *N);
 const Stmt *GetDenomExpr(const ExplodedNode *N);
index e337b3ebdff631969f7d3172e1eacd1154c1da3c..c582cfc4a81b8294b84b3d8e74f45afa0361cdc1 100644 (file)
@@ -105,8 +105,7 @@ void AttrNonNullChecker::checkPreCall(const CallEvent &Call,
         // Highlight the range of the argument that was null.
         R->addRange(Call.getArgSourceRange(idx));
         if (const Expr *ArgE = Call.getArgExpr(idx))
-          R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(errorNode,
-                                                                     ArgE, R));
+          bugreporter::addTrackNullOrUndefValueVisitor(errorNode, ArgE, R);
         // Emit the bug report.
         C.EmitReport(R);
       }
index 27bc6fb661cafc3a4b8967349b02ad252406f5f8..955e79ae4661c21e4eb06f1bc6be93059d6b78f5 100644 (file)
@@ -432,8 +432,7 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE,
 
     BugReport *report = new BugReport(*BT, description, N);
     report->addRange(Arg->getSourceRange());
-    report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Arg,
-                                                                    report));
+    bugreporter::addTrackNullOrUndefValueVisitor(N, Arg, report);
     C.EmitReport(report);
     return;
   }
index 69373749a23b35f4734282851f562b7b4a948f05..483082a37f443892d1730f5c021795454f9bb781 100644 (file)
@@ -252,8 +252,7 @@ ProgramStateRef CStringChecker::checkNonNull(CheckerContext &C,
     BugReport *report = new BugReport(*BT, os.str(), N);
 
     report->addRange(S->getSourceRange());
-    report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, S,
-                                                                    report));
+    bugreporter::addTrackNullOrUndefValueVisitor(N, S, report);
     C.EmitReport(report);
     return NULL;
   }
index 70b6241dea07b214bfddb98490c8552b30cf3180..e09d6885a98a7bab259cc8c4b3c9f7388e17d8dc 100644 (file)
@@ -75,7 +75,7 @@ void CallAndMessageChecker::emitBadCall(BugType *BT, CheckerContext &C,
   BugReport *R = new BugReport(*BT, BT->getName(), N);
   if (BadE) {
     R->addRange(BadE->getSourceRange());
-    R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, BadE, R));
+    bugreporter::addTrackNullOrUndefValueVisitor(N, BadE, R);
   }
   C.EmitReport(R);
 }
@@ -122,8 +122,7 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C,
       BugReport *R = new BugReport(*BT, Desc, N);
       R->addRange(argRange);
       if (argEx)
-        R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, argEx,
-                                                                   R));
+        bugreporter::addTrackNullOrUndefValueVisitor(N, argEx, R);
       C.EmitReport(R);
     }
     return true;
@@ -320,9 +319,7 @@ void CallAndMessageChecker::checkPreObjCMessage(const ObjCMethodCall &msg,
 
       // FIXME: getTrackNullOrUndefValueVisitor can't handle "super" yet.
       if (const Expr *ReceiverE = ME->getInstanceReceiver())
-        R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
-                                                                   ReceiverE,
-                                                                   R));
+        bugreporter::addTrackNullOrUndefValueVisitor(N, ReceiverE, R);
       C.EmitReport(R);
     }
     return;
@@ -364,9 +361,7 @@ void CallAndMessageChecker::emitNilReceiverBug(CheckerContext &C,
   report->addRange(ME->getReceiverRange());
   // FIXME: This won't track "self" in messages to super.
   if (const Expr *receiver = ME->getInstanceReceiver()) {
-    report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
-                                                                    receiver,
-                                                                    report));
+    bugreporter::addTrackNullOrUndefValueVisitor(N, receiver, report);
   }
   C.EmitReport(report);
 }
index a0022541f98ee366e0bb9687eb99f285b3b0a488..a94d7a773ec06d347dff10bdb4b35fb81c47ba4b 100644 (file)
@@ -166,10 +166,8 @@ void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S,
                   buf.empty() ? BT_null->getDescription() : buf.str(),
                   N);
 
-  report->addVisitor(
-    bugreporter::getTrackNullOrUndefValueVisitor(N,
-                                                 bugreporter::GetDerefExpr(N),
-                                                 report));
+  bugreporter::addTrackNullOrUndefValueVisitor(N, bugreporter::GetDerefExpr(N),
+                                               report);
 
   for (SmallVectorImpl<SourceRange>::iterator
        I = Ranges.begin(), E = Ranges.end(); I!=E; ++I)
@@ -193,8 +191,9 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
 
       BugReport *report =
         new BugReport(*BT_undef, BT_undef->getDescription(), N);
-      report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
-                                        bugreporter::GetDerefExpr(N), report));
+      bugreporter::addTrackNullOrUndefValueVisitor(N,
+                                                   bugreporter::GetDerefExpr(N),
+                                                   report);
       report->disablePathPruning();
       C.EmitReport(report);
     }
index 5094a0336232852125f2f11c1053a161eb466bb6..dcf6a8603ec7060d3e0fef2feb04c40980ad37ee 100644 (file)
@@ -42,8 +42,9 @@ void DivZeroChecker::reportBug(const char *Msg,
     BugReport *R =
       new BugReport(*BT, Msg, N);
 
-    R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
-                                 bugreporter::GetDenomExpr(N), R));
+    bugreporter::addTrackNullOrUndefValueVisitor(N,
+                                                 bugreporter::GetDenomExpr(N),
+                                                 R);
     C.EmitReport(R);
   }
 }
index 777e9ea219b3be3dac071829975fb7a41d7fef9c..4cc92ce9e95824c99ceccb1b2c0753c76140048f 100644 (file)
@@ -50,8 +50,7 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S,
                                   "for @synchronized"));
       BugReport *report =
         new BugReport(*BT_undef, BT_undef->getDescription(), N);
-      report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Ex,
-                                                                      report));
+      bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, report);
       C.EmitReport(report);
     }
     return;
@@ -74,8 +73,7 @@ void ObjCAtSyncChecker::checkPreStmt(const ObjCAtSynchronizedStmt *S,
                                    "(no synchronization will occur)"));
         BugReport *report =
           new BugReport(*BT_null, BT_null->getDescription(), N);
-        report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Ex,
-                                                                        report));
+        bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, report);
 
         C.EmitReport(report);
         return;
index 0851836f1fd73bcb9f064b9894bd7f1f2b0e5ba4..ca2a55d1e7bb47fed3a5be80796c7e9867bada5c 100644 (file)
@@ -55,8 +55,7 @@ void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS,
 
   report->disablePathPruning();
   report->addRange(RetE->getSourceRange());
-  report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, RetE,
-                                                                  report));
+  bugreporter::addTrackNullOrUndefValueVisitor(N, RetE, report);
 
   C.EmitReport(report);
 }
index 48b194107e9cae64a0fbec71416ab8cf8ff3d61c..70a33c76db0cd7d576baec67ad8b628ee324764c 100644 (file)
@@ -99,7 +99,7 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition,
 
       // Emit the bug report.
       BugReport *R = new BugReport(*BT, BT->getDescription(), N);
-      R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Ex, R));
+      bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, R);
       R->addRange(Ex->getSourceRange());
       R->disablePathPruning();
 
index a27fa1dcbec891b5329693ee00848fd986a51da9..e220499d73f4178efea9db6ab4158679d95ec041 100644 (file)
@@ -76,12 +76,10 @@ void UndefResultChecker::checkPostStmt(const BinaryOperator *B,
     BugReport *report = new BugReport(*BT, OS.str(), N);
     if (Ex) {
       report->addRange(Ex->getSourceRange());
-      report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Ex,
-                                                                      report));
+      bugreporter::addTrackNullOrUndefValueVisitor(N, Ex, report);
     }
     else
-      report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, B,
-                                                                      report));
+      bugreporter::addTrackNullOrUndefValueVisitor(N, B, report);
     
     report->disablePathPruning();
     C.EmitReport(report);
index 0297c4eb14e9b33dc269f5314cb2b7898b4fef79..6ae3c1875fbf4b68e788eab1c6798bd7c293c9c5 100644 (file)
@@ -42,9 +42,7 @@ UndefinedArraySubscriptChecker::checkPreStmt(const ArraySubscriptExpr *A,
       // Generate a report for this bug.
       BugReport *R = new BugReport(*BT, BT->getName(), N);
       R->addRange(A->getIdx()->getSourceRange());
-      R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N,
-                                                                 A->getIdx(),
-                                                                 R));
+      bugreporter::addTrackNullOrUndefValueVisitor(N, A->getIdx(), R);
       C.EmitReport(R);
     }
   }
index 7b1081f6bb3d8f3ea67836dd63b5842d595f636d..14a884e01b64adcf03620571846c336d27c3d2ab 100644 (file)
@@ -78,7 +78,7 @@ void UndefinedAssignmentChecker::checkBind(SVal location, SVal val,
   BugReport *R = new BugReport(*BT, str, N);
   if (ex) {
     R->addRange(ex->getSourceRange());
-    R->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, ex, R));
+    bugreporter::addTrackNullOrUndefValueVisitor(N, ex, R);
   }
   R->disablePathPruning();
   C.EmitReport(R);
index f173cde17d12f938f2afd1e06037a016ac5494cb..d35455c2191b06b5e87ea5662bdc1d70f4d65f5f 100644 (file)
@@ -224,8 +224,7 @@ bool UnixAPIChecker::ReportZeroByteAllocation(CheckerContext &C,
   BugReport *report = new BugReport(*BT_mallocZero, os.str(), N);
 
   report->addRange(arg->getSourceRange());
-  report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, arg,
-                                                                  report));
+  bugreporter::addTrackNullOrUndefValueVisitor(N, arg, report);
   C.EmitReport(report);
 
   return true;
index 38c9cc1f333c3b8f9a32c0c229c3a4a841a3c04f..fab4adf3e0e29923f95c63cbc9cf061796b9854c 100644 (file)
@@ -69,8 +69,7 @@ void VLASizeChecker::reportBug(VLASize_Kind Kind,
 
   BugReport *report = new BugReport(*BT, os.str(), N);
   report->addRange(SizeE->getSourceRange());
-  report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, SizeE,
-                                                                  report));
+  bugreporter::addTrackNullOrUndefValueVisitor(N, SizeE, report);
   C.EmitReport(report);
   return;
 }
index 3dedcb3edf0378c5981c9036b1917fc6a8d2998c..46aa9e2b9123b20595bbdec3ba76e85ee6197d74 100644 (file)
@@ -197,6 +197,9 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *N,
             os << "declared without an initial value";
         }
       }
+      else {
+        os << "initialized here";
+      }
     }
   }
 
@@ -223,7 +226,7 @@ PathDiagnosticPiece *FindLastStoreBRVisitor::VisitNode(const ExplodedNode *N,
                << " is assigned to ";
     }
     else
-      return NULL;
+      os << "Value assigned to ";
 
     if (const VarRegion *VR = dyn_cast<VarRegion>(R)) {
       os << '\'' << *VR->getDecl() << '\'';
@@ -293,12 +296,11 @@ TrackConstraintBRVisitor::VisitNode(const ExplodedNode *N,
   return NULL;
 }
 
-BugReporterVisitor *
-bugreporter::getTrackNullOrUndefValueVisitor(const ExplodedNode *N,
-                                             const Stmt *S,
-                                             BugReport *report) {
+void bugreporter::addTrackNullOrUndefValueVisitor(const ExplodedNode *N,
+                                                  const Stmt *S,
+                                                  BugReport *report) {
   if (!S || !N)
-    return 0;
+    return;
 
   ProgramStateManager &StateMgr = N->getState()->getStateManager();
 
@@ -314,7 +316,7 @@ bugreporter::getTrackNullOrUndefValueVisitor(const ExplodedNode *N,
   }
 
   if (!N)
-    return 0;
+    return;
   
   ProgramStateRef state = N->getState();
 
@@ -331,7 +333,15 @@ bugreporter::getTrackNullOrUndefValueVisitor(const ExplodedNode *N,
         SVal V = state->getRawSVal(loc::MemRegionVal(R));
         report->markInteresting(R);
         report->markInteresting(V);
-        return new FindLastStoreBRVisitor(V, R);
+
+        if (V.getAsLocSymbol()) {
+          BugReporterVisitor *ConstraintTracker
+            = new TrackConstraintBRVisitor(cast<loc::MemRegionVal>(V), false);
+          report->addVisitor(ConstraintTracker);
+        }
+
+        report->addVisitor(new FindLastStoreBRVisitor(V, R));
+        return;
       }
     }
   }
@@ -351,11 +361,10 @@ bugreporter::getTrackNullOrUndefValueVisitor(const ExplodedNode *N,
 
     if (R) {
       report->markInteresting(R);
-      return new TrackConstraintBRVisitor(loc::MemRegionVal(R), false);
+      report->addVisitor(new TrackConstraintBRVisitor(loc::MemRegionVal(R),
+                                                      false));
     }
   }
-
-  return 0;
 }
 
 BugReporterVisitor *
@@ -397,7 +406,7 @@ PathDiagnosticPiece *NilReceiverBRVisitor::VisitNode(const ExplodedNode *N,
   // The receiver was nil, and hence the method was skipped.
   // Register a BugReporterVisitor to issue a message telling us how
   // the receiver was null.
-  BR.addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, Receiver, &BR));
+  bugreporter::addTrackNullOrUndefValueVisitor(N, Receiver, &BR);
   // Issue a message saying that the method was skipped.
   PathDiagnosticLocation L(Receiver, BRC.getSourceManager(),
                                      N->getLocationContext());
index 1db3c5aab8d43c0a5d1a1bb92b493331a36e51e4..53bc4249b7fd6a1690dd6a24c1457c250bdf03f7 100644 (file)
@@ -11,4 +11,46 @@ void testZero(int *a) {
   // expected-note@-2 {{Returning from 'zero'}}
   *a = 1; // expected-warning{{Dereference of null pointer}}
   // expected-note@-1 {{Dereference of null pointer (loaded from variable 'a')}}
-}
\ No newline at end of file
+}
+
+
+void check(int *p) {
+  if (p) {
+    // expected-note@-1 + {{Assuming 'p' is null}}
+    // expected-note@-2 + {{Assuming pointer value is null}}
+    // expected-note@-3 + {{Taking false branch}}
+    return;
+  }
+  return;
+}
+
+void testCheck(int *a) {
+  check(a);
+  // expected-note@-1 {{Calling 'check'}}
+  // expected-note@-2 {{Returning from 'check'}}
+  *a = 1; // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1 {{Dereference of null pointer (loaded from variable 'a')}}
+}
+
+
+int *getPointer();
+
+void testInitCheck() {
+  int *a = getPointer();
+  // expected-note@-1 {{Variable 'a' initialized here}}
+  check(a);
+  // expected-note@-1 {{Calling 'check'}}
+  // expected-note@-2 {{Returning from 'check'}}
+  *a = 1; // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1 {{Dereference of null pointer (loaded from variable 'a')}}
+}
+
+void testStoreCheck(int *a) {
+  a = getPointer();
+  // expected-note@-1 {{Value assigned to 'a'}}
+  check(a);
+  // expected-note@-1 {{Calling 'check'}}
+  // expected-note@-2 {{Returning from 'check'}}
+  *a = 1; // expected-warning{{Dereference of null pointer}}
+  // expected-note@-1 {{Dereference of null pointer (loaded from variable 'a')}}
+}
index 1e17b838cc18cb547d3ff241f1207365ce67c203..6298ca043fb8bf9e3857ee97538c912031984352 100644 (file)
@@ -24,7 +24,7 @@ void test_ic_set_to_null() {
 }
 
 void test_ic_null(TestInstanceCall *p) {
-  if (!p) // expected-note {{Taking true branch}}
+  if (!p) // expected-note {{Assuming pointer value is null}} expected-note {{Taking true branch}}
     p->foo(); // expected-warning {{Called C++ object pointer is null}} expected-note{{Called C++ object pointer is null}}
 }