r177121 - [analyzer] Change the way in which IDC Visitor decides to kick in and make sure it attaches in the given edge case

Anna Zaks ganna at apple.com
Thu Mar 14 15:31:56 PDT 2013


Author: zaks
Date: Thu Mar 14 17:31:56 2013
New Revision: 177121

URL: http://llvm.org/viewvc/llvm-project?rev=177121&view=rev
Log:
[analyzer] Change the way in which IDC Visitor decides to kick in and make sure it attaches in the given edge case

In the test case below, the value V is not constrained to 0 in ErrorNode but it is in node N.
So we used to fail to register the Suppression visitor.

We also need to change the way we determine that the Visitor should kick in because the node N belongs to
the ExplodedGraph and might not be on the BugReporter path that the visitor sees. Instead of trying to match the node,
turn on the visitor when we see the last node in which the symbol is ‘0’.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h?rev=177121&r1=177120&r2=177121&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h Thu Mar 14 17:31:56 2013
@@ -290,12 +290,12 @@ class SuppressInlineDefensiveChecksVisit
   /// Track if we found the node where the constraint was first added.
   bool IsSatisfied;
 
-  /// \brief The node from which we should start tracking the value.
-  /// Note: Since the visitors can be registered on nodes previous to the last
+  /// Since the visitors can be registered on nodes previous to the last
   /// node in the BugReport, but the path traversal always starts with the last
   /// node, the visitor invariant (that we start with a node in which V is null)
-  /// might not hold when node visitation starts.
-  const ExplodedNode *StartN;
+  /// might not hold when node visitation starts. We are going to start tracking
+  /// from the last node in which the value is null.
+  bool IsTrackingTurnedOn;
 
 public:
   SuppressInlineDefensiveChecksVisitor(DefinedSVal Val, const ExplodedNode *N);
@@ -306,10 +306,6 @@ public:
   /// to make all PathDiagnosticPieces created by this visitor.
   static const char *getTag();
 
-  PathDiagnosticPiece *getEndPath(BugReporterContext &BRC,
-                                  const ExplodedNode *N,
-                                  BugReport &BR);
-
   PathDiagnosticPiece *VisitNode(const ExplodedNode *Succ,
                                  const ExplodedNode *Pred,
                                  BugReporterContext &BRC,

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=177121&r1=177120&r2=177121&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Thu Mar 14 17:31:56 2013
@@ -700,16 +700,14 @@ TrackConstraintBRVisitor::VisitNode(cons
 
 SuppressInlineDefensiveChecksVisitor::
 SuppressInlineDefensiveChecksVisitor(DefinedSVal Value, const ExplodedNode *N)
-  : V(Value), IsSatisfied(false), StartN(N) {
-
-  assert(N->getState()->isNull(V).isConstrainedTrue() &&
-         "The visitor only tracks the cases where V is constrained to 0");
+  : V(Value), IsSatisfied(false), IsTrackingTurnedOn(false) {
+    assert(N->getState()->isNull(V).isConstrainedTrue() &&
+           "The visitor only tracks the cases where V is constrained to 0");
 }
 
 void SuppressInlineDefensiveChecksVisitor::Profile(FoldingSetNodeID &ID) const {
   static int id = 0;
   ID.AddPointer(&id);
-  ID.AddPointer(StartN);
   ID.Add(V);
 }
 
@@ -718,33 +716,25 @@ const char *SuppressInlineDefensiveCheck
 }
 
 PathDiagnosticPiece *
-SuppressInlineDefensiveChecksVisitor::getEndPath(BugReporterContext &BRC,
-                                                 const ExplodedNode *N,
-                                                 BugReport &BR) {
-  if (StartN == BR.getErrorNode())
-    StartN = 0;
-  return 0;
-}
-
-PathDiagnosticPiece *
 SuppressInlineDefensiveChecksVisitor::VisitNode(const ExplodedNode *Succ,
                                                 const ExplodedNode *Pred,
                                                 BugReporterContext &BRC,
                                                 BugReport &BR) {
   if (IsSatisfied)
     return 0;
-
-  // Start tracking after we see node StartN.
-  if (StartN == Succ)
-    StartN = 0;
-  if (StartN)
-    return 0;
-
   AnalyzerOptions &Options =
-    BRC.getBugReporter().getEngine().getAnalysisManager().options;
+  BRC.getBugReporter().getEngine().getAnalysisManager().options;
   if (!Options.shouldSuppressInlinedDefensiveChecks())
     return 0;
 
+  // Start tracking after we see the first state in which the value is null.
+  if (!IsTrackingTurnedOn)
+    if (Succ->getState()->isNull(V).isConstrainedTrue())
+      IsTrackingTurnedOn = true;
+  if (!IsTrackingTurnedOn)
+    return 0;
+
+
   // Check if in the previous state it was feasible for this value
   // to *not* be null.
   if (Pred->getState()->assume(V, true)) {
@@ -899,10 +889,10 @@ bool bugreporter::trackNullOrUndefValue(
         report.addVisitor(ConstraintTracker);
 
         // Add visitor, which will suppress inline defensive checks.
-        if (ErrorNode->getState()->isNull(V).isConstrainedTrue()) {
+        if (N->getState()->isNull(V).isConstrainedTrue()) {
           BugReporterVisitor *IDCSuppressor =
             new SuppressInlineDefensiveChecksVisitor(V.castAs<DefinedSVal>(),
-                                                     ErrorNode);
+                                                     N);
           report.addVisitor(IDCSuppressor);
         }
       }

Modified: cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp?rev=177121&r1=177120&r2=177121&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp (original)
+++ cfe/trunk/test/Analysis/inlining/inline-defensive-checks.cpp Thu Mar 14 17:31:56 2013
@@ -1,6 +1,12 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core -verify %s
 // expected-no-diagnostics
 
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+                           unsigned int __line, __const char *__function)
+__attribute__ ((__noreturn__));
+#define assert(expr) \
+((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
 class ButterFly {
 private:
   ButterFly() { }
@@ -29,4 +35,21 @@ class X{
 		subtest1();
 		return subtest2();
 	}
-};
\ No newline at end of file
+};
+
+typedef const int *Ty;
+extern
+Ty notNullArg(Ty cf) __attribute__((nonnull));
+typedef const void *CFTypeRef;
+extern Ty getTyVal();
+inline void radar13224271_callee(Ty def, Ty& result ) {
+	result = def;
+  // Clearly indicates that result cannot be 0 if def is not NULL.
+	assert( (result != 0) || (def == 0) );
+}
+void radar13224271_caller()
+{
+	Ty value;
+	radar13224271_callee(getTyVal(), value );
+	notNullArg(value); // no-warning
+}
\ No newline at end of file





More information about the cfe-commits mailing list