[analyzer] Prevent crashes in FindLastStoreBRVisitor

This patch is a band-aid. A proper solution would be too change
trackNullOrUndefValue to only try to dereference the pointer when it is
relevant to the problem.

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

llvm-svn: 342920

3 files changed, 32 insertions, 4 deletions

diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index da019f83c21..6722a0ae52c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -83,6 +83,8 @@ public:
                     BugReport &BR);
 };
 
+/// Finds last store into the given region,
+/// which is different from a given symbolic value.
 class FindLastStoreBRVisitor final : public BugReporterVisitor {
   const MemRegion *R;
   SVal V;
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 461e64ebf9a..bb93d096930 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1294,8 +1294,7 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
         if (const auto *BDR =
               dyn_cast_or_null<BlockDataRegion>(V.getAsRegion())) {
           if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) {
-            if (Optional<KnownSVal> KV =
-                State->getSVal(OriginalR).getAs<KnownSVal>())
+            if (auto KV = State->getSVal(OriginalR).getAs<KnownSVal>())
               BR.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
                   *KV, OriginalR, EnableNullFPSuppression));
           }
@@ -1752,8 +1751,18 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N,
     else
       RVal = state->getSVal(L->getRegion());
 
-    if (auto KV = RVal.getAs<KnownSVal>())
-      report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
+    // FIXME: this is a hack for fixing a later crash when attempting to
+    // dereference a void* pointer.
+    // We should not try to dereference pointers at all when we don't care
+    // what is written inside the pointer.
+    bool ShouldFindLastStore = true;
+    if (const auto *SR = dyn_cast<SymbolicRegion>(L->getRegion()))
+      if (SR->getSymbol()->getType()->getPointeeType()->isVoidType())
+        ShouldFindLastStore = false;
+
+    if (ShouldFindLastStore)
+      if (auto KV = RVal.getAs<KnownSVal>())
+        report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
             *KV, L->getRegion(), EnableNullFPSuppression));
 
     const MemRegion *RegionRVal = RVal.getAsRegion();
diff --git a/clang/test/Analysis/diagnostics/find_last_store.c b/clang/test/Analysis/diagnostics/find_last_store.c
new file mode 100644
index 00000000000..9bf601ea30e
--- /dev/null
+++ b/clang/test/Analysis/diagnostics/find_last_store.c
@@ -0,0 +1,17 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=text -verify %s
+typedef struct { float b; } c;
+void *a();
+void *d() {
+  return a(); // expected-note{{Returning pointer}}
+}
+
+void no_find_last_store() {
+  c *e = d(); // expected-note{{Calling 'd'}}
+              // expected-note@-1{{Returning from 'd'}}
+              // expected-note@-2{{'e' initialized here}}
+
+  (void)(e || e->b); // expected-note{{Assuming 'e' is null}}
+      // expected-note@-1{{Left side of '||' is false}}
+      // expected-note@-2{{Access to field 'b' results in a dereference of a null pointer (loaded from variable 'e')}}
+      // expected-warning@-3{{Access to field 'b' results in a dereference of a null pointer (loaded from variable 'e')}}
+}