r342920 - [analyzer] Prevent crashes in FindLastStoreBRVisitor

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Mon Sep 24 14:20:30 2018
New Revision: 342920

URL: http://llvm.org/viewvc/llvm-project?rev=342920&view=rev
Log:
[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

Added:
    cfe/trunk/test/Analysis/diagnostics/find_last_store.c
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h?rev=342920&r1=342919&r2=342920&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h Mon Sep 24 14:20:30 2018
@@ -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;

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=342920&r1=342919&r2=342920&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Mon Sep 24 14:20:30 2018
@@ -1294,8 +1294,7 @@ FindLastStoreBRVisitor::VisitNode(const
         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(
     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();

Added: cfe/trunk/test/Analysis/diagnostics/find_last_store.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/find_last_store.c?rev=342920&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/find_last_store.c (added)
+++ cfe/trunk/test/Analysis/diagnostics/find_last_store.c Mon Sep 24 14:20:30 2018
@@ -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 at -1{{Returning from 'd'}}
+              // expected-note at -2{{'e' initialized here}}
+
+  (void)(e || e->b); // expected-note{{Assuming 'e' is null}}
+      // expected-note at -1{{Left side of '||' is false}}
+      // expected-note at -2{{Access to field 'b' results in a dereference of a null pointer (loaded from variable 'e')}}
+      // expected-warning at -3{{Access to field 'b' results in a dereference of a null pointer (loaded from variable 'e')}}
+}