[PATCH] D64271: [analyzer] Don't track the right hand side of the last store for conditions

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, rnkovacs, Charusso, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D64270: [analyzer][NFC] Prepare visitors for different tracking kinds.

Exactly what it says on the tin!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64271

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/track-control-dependency-conditions.cpp


Index: clang/test/Analysis/track-control-dependency-conditions.cpp
===================================================================
--- clang/test/Analysis/track-control-dependency-conditions.cpp
+++ clang/test/Analysis/track-control-dependency-conditions.cpp
@@ -119,7 +119,7 @@
 bool coin();
 
 bool foo() {
-  return coin(); // tracking-note{{Returning value}}
+  return coin();
 }
 
 int bar;
@@ -127,12 +127,10 @@
 void test() {
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
 
-  if (int flag = foo()) // tracking-note{{Calling 'foo'}}
-                        // tracking-note at -1{{Returning from 'foo'}}
-                        // tracking-note at -2{{'flag' initialized here}}
-                        // debug-note at -3{{Tracking condition 'flag'}}
-                        // expected-note at -4{{Assuming 'flag' is not equal to 0}}
-                        // expected-note at -5{{Taking true branch}}
+  if (int flag = foo()) // tracking-note{{'flag' initialized here}}
+                        // debug-note at -1{{Tracking condition 'flag'}}
+                        // expected-note at -2{{Assuming 'flag' is not equal to 0}}
+                        // expected-note at -3{{Taking true branch}}
 
     *x = 5; // expected-warning{{Dereference of null pointer}}
             // expected-note at -1{{Dereference of null pointer}}
@@ -226,9 +224,8 @@
 int getInt();
 
 void f(int y) {
-  y = 1; // tracking-note{{The value 1 is assigned to 'y'}}
+  y = 1;
   flag = y; // tracking-note{{The value 1 is assigned to 'flag'}}
-
   int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
   if (flag) // expected-note{{'flag' is 1}}
             // expected-note at -1{{Taking true branch}}
@@ -244,9 +241,8 @@
 
 void foo() {
   int y;
-  y = 1; // tracking-note{{The value 1 is assigned to 'y'}}
+  y = 1;
   flag = y; // tracking-note{{The value 1 is assigned to 'flag'}}
-
 }
 
 void f(int y) {
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1425,16 +1425,20 @@
 
   // If we have an expression that provided the value, try to track where it
   // came from.
-  if (InitE) {
-    if (V.isUndef() ||
-        V.getAs<loc::ConcreteInt>() || V.getAs<nonloc::ConcreteInt>()) {
-      if (!IsParam)
-        InitE = InitE->IgnoreParenCasts();
-      bugreporter::trackExpressionValue(StoreSite, InitE, BR,
-                                   EnableNullFPSuppression);
+  // For tracked conditions, this isn't really important, it would pollute the
+  // bug report far too much.
+  if (TKind != TrackingKind::ConditionTracking) {
+    if (InitE) {
+      if (V.isUndef() ||
+          V.getAs<loc::ConcreteInt>() || V.getAs<nonloc::ConcreteInt>()) {
+        if (!IsParam)
+          InitE = InitE->IgnoreParenCasts();
+        bugreporter::trackExpressionValue(StoreSite, InitE, BR,
+                                     EnableNullFPSuppression);
+      }
+      ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
+                                           BR, EnableNullFPSuppression);
     }
-    ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
-                                         BR, EnableNullFPSuppression);
   }
 
   // Okay, we've found the binding. Emit an appropriate message.
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -138,7 +138,8 @@
   /// \param R The region we're tracking.
   /// \param EnableNullFPSuppression Whether we should employ false positive
   ///         suppression (inlined defensive checks, returned null).
-  /// \param TKind May limit the amount of notes added to the bug report.
+  /// \param TKind May limit the amount of notes added to the bug report. For
+  ///        conditions, ignores the initialization point.
   FindLastStoreBRVisitor(KnownSVal V, const MemRegion *R,
                          bool InEnableNullFPSuppression,
                          TrackingKind TKind)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64271.208242.patch
Type: text/x-patch
Size: 4370 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190705/3298970f/attachment.bin>