r340475 - [analyzer] Track non-zero values in ReturnVisitor

George Karpenkov via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 22 16:17:25 PDT 2018


Author: george.karpenkov
Date: Wed Aug 22 16:17:25 2018
New Revision: 340475

URL: http://llvm.org/viewvc/llvm-project?rev=340475&view=rev
Log:
[analyzer] Track non-zero values in ReturnVisitor

Tracking those can help to provide much better diagnostics in many cases.

In general, most of the visitor machinery should be refactored to allow
tracking the origin of arbitrary values.

rdar://36039765

Differential Revision: https://reviews.llvm.org/D51131

Added:
    cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/test/Analysis/uninit-const.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=340475&r1=340474&r2=340475&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Aug 22 16:17:25 2018
@@ -887,15 +887,6 @@ public:
 
     RetE = RetE->IgnoreParenCasts();
 
-    // If we can't prove the return value is 0, just mark it interesting, and
-    // make sure to track it into any further inner functions.
-    if (!State->isNull(V).isConstrainedTrue()) {
-      BR.markInteresting(V);
-      ReturnVisitor::addVisitorIfNecessary(N, RetE, BR,
-                                           EnableNullFPSuppression);
-      return nullptr;
-    }
-
     // If we're returning 0, we should track where that 0 came from.
     bugreporter::trackNullOrUndefValue(N, RetE, BR, /*IsArg*/ false,
                                        EnableNullFPSuppression);
@@ -904,20 +895,33 @@ public:
     SmallString<64> Msg;
     llvm::raw_svector_ostream Out(Msg);
 
-    if (V.getAs<Loc>()) {
-      // If we have counter-suppression enabled, make sure we keep visiting
-      // future nodes. We want to emit a path note as well, in case
-      // the report is resurrected as valid later on.
-      if (EnableNullFPSuppression &&
-          Options.shouldAvoidSuppressingNullArgumentPaths())
-        Mode = MaybeUnsuppress;
+    if (State->isNull(V).isConstrainedTrue()) {
+      if (V.getAs<Loc>()) {
+
+        // If we have counter-suppression enabled, make sure we keep visiting
+        // future nodes. We want to emit a path note as well, in case
+        // the report is resurrected as valid later on.
+        if (EnableNullFPSuppression &&
+            Options.shouldAvoidSuppressingNullArgumentPaths())
+          Mode = MaybeUnsuppress;
+
+        if (RetE->getType()->isObjCObjectPointerType()) {
+          Out << "Returning nil";
+        } else {
+          Out << "Returning null pointer";
+        }
+      } else {
+        Out << "Returning zero";
+      }
 
-      if (RetE->getType()->isObjCObjectPointerType())
-        Out << "Returning nil";
-      else
-        Out << "Returning null pointer";
     } else {
-      Out << "Returning zero";
+      if (auto CI = V.getAs<nonloc::ConcreteInt>()) {
+        Out << "Returning the value " << CI->getValue();
+      } else if (V.getAs<Loc>()) {
+        Out << "Returning pointer";
+      } else {
+        Out << "Returning value";
+      }
     }
 
     if (LValue) {
@@ -1262,10 +1266,9 @@ FindLastStoreBRVisitor::VisitNode(const
         InitE = InitE->IgnoreParenCasts();
       bugreporter::trackNullOrUndefValue(StoreSite, InitE, BR, IsParam,
                                          EnableNullFPSuppression);
-    } else {
-      ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
-                                           BR, EnableNullFPSuppression);
     }
+    ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
+                                         BR, EnableNullFPSuppression);
   }
 
   // Okay, we've found the binding. Emit an appropriate message.

Added: cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp?rev=340475&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp (added)
+++ cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp Wed Aug 22 16:17:25 2018
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -x c++ -analyzer-checker=core -analyzer-output=text -verify %s
+
+typedef unsigned char uint8_t;
+
+#define UINT8_MAX 255
+#define TCP_MAXWIN 65535
+
+uint8_t get_uint8_max() {
+  uint8_t rcvscale = UINT8_MAX; // expected-note{{'rcvscale' initialized to 255}}
+  return rcvscale; // expected-note{{Returning the value 255 (loaded from 'rcvscale')}}
+}
+
+void shift_by_undefined_value() {
+  uint8_t shift_amount = get_uint8_max(); // expected-note{{'shift_amount' initialized to 255}}
+                                // expected-note at -1{{Calling 'get_uint8_max'}}
+                                // expected-note at -2{{Returning from 'get_uint8_max'}}
+  (void)(TCP_MAXWIN << shift_amount); // expected-warning{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
+                                      // expected-note at -1{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}}
+}

Modified: cfe/trunk/test/Analysis/uninit-const.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-const.cpp?rev=340475&r1=340474&r2=340475&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/uninit-const.cpp (original)
+++ cfe/trunk/test/Analysis/uninit-const.cpp Wed Aug 22 16:17:25 2018
@@ -49,15 +49,17 @@ void f7(void) {
 
 
 int& f6_1_sub(int &p) {
-  return p;
+  return p; // expected-note{{Returning without writing to 'p'}}
+            // expected-note at -1{{Returning pointer (reference to 't')}}
 }
 
 void f6_1(void) {
   int t; // expected-note{{'t' declared without an initial value}}
   int p = f6_1_sub(t); //expected-warning {{Assigned value is garbage or undefined}}
-                       //expected-note at -1 {{Calling 'f6_1_sub'}}
-                       //expected-note at -2 {{Returning from 'f6_1_sub'}}
-                       //expected-note at -3 {{Assigned value is garbage or undefined}}
+                       //expected-note at -1 {{Passing value via 1st parameter 'p'}}
+                       //expected-note at -2 {{Calling 'f6_1_sub'}}
+                       //expected-note at -3 {{Returning from 'f6_1_sub'}}
+                       //expected-note at -4 {{Assigned value is garbage or undefined}}
   int q = p;
   doStuff6(q);
 }




More information about the cfe-commits mailing list