[clang] a880256 - [analyzer] Consider array subscripts to be interesting lvalues.

[Artem Dergachev via cfe-commits]

Author: Valeriy Savchenko
Date: 2020-04-23T19:52:45+03:00
New Revision: a88025672f89374bfa584e2179a557f44d86da11

URL: https://github.com/llvm/llvm-project/commit/a88025672f89374bfa584e2179a557f44d86da11
DIFF: https://github.com/llvm/llvm-project/commit/a88025672f89374bfa584e2179a557f44d86da11.diff

LOG: [analyzer] Consider array subscripts to be interesting lvalues.

Static analyzer has a mechanism of clearing redundant nodes when
analysis hits a certain threshold with a number of nodes in exploded
graph (default is 1000). It is similar to GC and aims removing nodes
not useful for analysis. Unfortunately nodes corresponding to array
subscript expressions (that actively participate in data propagation)
get removed during the cleanup. This might prevent the analyzer from
generating useful notes about where it thinks the data came from.

This fix is pretty much consistent with the way analysis works
already. Lvalue "interestingness" stands for the analyzer's
possibility of tracking values through them.

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

Added: 
    clang/test/Analysis/CheckThatArraySubsciptNodeIsNotCollected.cpp

Modified: 
    clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
index c4838492271c..635495e9bf60 100644
--- a/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp
@@ -50,9 +50,8 @@ ExplodedGraph::~ExplodedGraph() = default;
 bool ExplodedGraph::isInterestingLValueExpr(const Expr *Ex) {
   if (!Ex->isLValue())
     return false;
-  return isa<DeclRefExpr>(Ex) ||
-         isa<MemberExpr>(Ex) ||
-         isa<ObjCIvarRefExpr>(Ex);
+  return isa<DeclRefExpr>(Ex) || isa<MemberExpr>(Ex) ||
+         isa<ObjCIvarRefExpr>(Ex) || isa<ArraySubscriptExpr>(Ex);
 }
 
 bool ExplodedGraph::shouldCollect(const ExplodedNode *node) {

diff  --git a/clang/test/Analysis/CheckThatArraySubsciptNodeIsNotCollected.cpp b/clang/test/Analysis/CheckThatArraySubsciptNodeIsNotCollected.cpp
new file mode 100644
index 000000000000..b1c0f339bd56
--- /dev/null
+++ b/clang/test/Analysis/CheckThatArraySubsciptNodeIsNotCollected.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=text -verify %s
+
+class A {
+public:
+  int method();
+};
+
+A *foo();
+void bar(A *);
+
+int index;
+
+// We want to check here that the notes about the origins of the null pointer
+// (array[index] = foo()) will get to the final report.
+//
+// The analyzer used to drop exploded nodes for array subscripts when it was
+// time to collect redundant nodes. This GC-like mechanism kicks in only when
+// the exploded graph is large enough (>1K nodes). For this reason, 'index'
+// is a global variable, and the sink point is inside of a loop.
+
+void test() {
+  A *array[42];
+  A *found;
+
+  for (index = 0; (array[index] = foo()); ++index) { // expected-note {{Loop condition is false. Execution continues on line 34}}
+    // expected-note at -1 {{Value assigned to 'index'}}
+    // expected-note at -2 {{Assigning value}}
+    // expected-note at -3 {{Assuming pointer value is null}}
+    if (array[0])
+      break;
+  }
+
+  do {
+    found = array[index]; // expected-note {{Null pointer value stored to 'found'}}
+
+    if (found->method()) // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
+      // expected-note at -1 {{Called C++ object pointer is null}}
+      bar(found);
+  } while (--index);
+}