[analyzer] Fix and refactor bugreporter::getDerefExpr() API.

This API is used by checkers (and other entities) in order to track where does
a value originate from, by jumping from an expression value of which is equal
to that value to the expression from which this value has "appeared". For
example, it may be an lvalue from which the rvalue was loaded, or a function
call from which the dereferenced pointer was returned.

The function now avoids incorrectly unwrapping implicit lvalue-to-rvalue casts,
which caused crashes and incorrect intermediate diagnostic pieces. It also no
longer relies on how the expression is written when guessing what it means.

Fixes pr34373 and pr34731.

rdar://problem/33594502

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@314287 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index d00182a871c1e7abf02f35c52dd441db1446dd73..716f23b2bc4d944eadeeec8bfb7b1fdf5fed8a16 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -42,48 +42,68 @@ bool bugreporter::isDeclRefExprToReference(const Expr *E) {
   return false;
 }
 
+/// Given that expression S represents a pointer that would be dereferenced,
+/// try to find a sub-expression from which the pointer came from.
+/// This is used for tracking down origins of a null or undefined value:
+/// "this is null because that is null because that is null" etc.
+/// We wipe away field and element offsets because they merely add offsets.
+/// We also wipe away all casts except lvalue-to-rvalue casts, because the
+/// latter represent an actual pointer dereference; however, we remove
+/// the final lvalue-to-rvalue cast before returning from this function
+/// because it demonstrates more clearly from where the pointer rvalue was
+/// loaded. Examples:
+///   x->y.z      ==>  x (lvalue)
+///   foo()->y.z  ==>  foo() (rvalue)
 const Expr *bugreporter::getDerefExpr(const Stmt *S) {
-  // Pattern match for a few useful cases:
-  //   a[0], p->f, *p
   const Expr *E = dyn_cast<Expr>(S);
   if (!E)
     return nullptr;
-  E = E->IgnoreParenCasts();
 
   while (true) {
-    if (const BinaryOperator *B = dyn_cast<BinaryOperator>(E)) {
-      assert(B->isAssignmentOp());
-      E = B->getLHS()->IgnoreParenCasts();
-      continue;
-    }
-    else if (const UnaryOperator *U = dyn_cast<UnaryOperator>(E)) {
-      if (U->getOpcode() == UO_Deref)
-        return U->getSubExpr()->IgnoreParenCasts();
-    }
-    else if (const MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
-      if (ME->isImplicitAccess()) {
-        return ME;
-      } else if (ME->isArrow() || isDeclRefExprToReference(ME->getBase())) {
-        return ME->getBase()->IgnoreParenCasts();
+    if (const CastExpr *CE = dyn_cast<CastExpr>(E)) {
+      if (CE->getCastKind() == CK_LValueToRValue) {
+        // This cast represents the load we're looking for.
+        break;
+      }
+      E = CE->getSubExpr();
+    } else if (isa<BinaryOperator>(E)) {
+      // Probably more arithmetic can be pattern-matched here,
+      // but for now give up.
+      break;
+    } else if (const UnaryOperator *U = dyn_cast<UnaryOperator>(E)) {
+      if (U->getOpcode() == UO_Deref) {
+        // Operators '*' and '&' don't actually mean anything.
+        // We look at casts instead.
+        E = U->getSubExpr();
       } else {
-        // If we have a member expr with a dot, the base must have been
-        // dereferenced.
-        return getDerefExpr(ME->getBase());
+        // Probably more arithmetic can be pattern-matched here,
+        // but for now give up.
+        break;
       }
     }
-    else if (const ObjCIvarRefExpr *IvarRef = dyn_cast<ObjCIvarRefExpr>(E)) {
-      return IvarRef->getBase()->IgnoreParenCasts();
-    }
-    else if (const ArraySubscriptExpr *AE = dyn_cast<ArraySubscriptExpr>(E)) {
-      return getDerefExpr(AE->getBase());
-    }
-    else if (isa<DeclRefExpr>(E)) {
-      return E;
+    // Pattern match for a few useful cases: a[0], p->f, *p etc.
+    else if (const MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
+      E = ME->getBase();
+    } else if (const ObjCIvarRefExpr *IvarRef = dyn_cast<ObjCIvarRefExpr>(E)) {
+      E = IvarRef->getBase();
+    } else if (const ArraySubscriptExpr *AE = dyn_cast<ArraySubscriptExpr>(E)) {
+      E = AE->getBase();
+    } else if (const ParenExpr *PE = dyn_cast<ParenExpr>(E)) {
+      E = PE->getSubExpr();
+    } else {
+      // Other arbitrary stuff.
+      break;
     }
-    break;
   }
 
-  return nullptr;
+  // Special case: remove the final lvalue-to-rvalue cast, but do not recurse
+  // deeper into the sub-expression. This way we return the lvalue from which
+  // our pointer rvalue was loaded.
+  if (const ImplicitCastExpr *CE = dyn_cast<ImplicitCastExpr>(E))
+    if (CE->getCastKind() == CK_LValueToRValue)
+      E = CE->getSubExpr();
+
+  return E;
 }
 
 const Stmt *bugreporter::GetDenomExpr(const ExplodedNode *N) {

diff --git a/test/Analysis/null-deref-path-notes.c b/test/Analysis/null-deref-path-notes.c
new file mode 100644 (file)
index 0000000..7070bc4
--- /dev/null
+++ b/test/Analysis/null-deref-path-notes.c
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -w -x c -analyzer-checker=core -analyzer-output=text -verify %s
+
+// Avoid the crash when finding the expression for tracking the origins
+// of the null pointer for path notes. Apparently, not much actual tracking
+// needs to be done in this example.
+void pr34373() {
+  int *a = 0;
+  (a + 0)[0]; // expected-warning{{Array access results in a null pointer dereference}}
+              // expected-note@-1{{Array access results in a null pointer dereference}}
+}

diff --git a/test/Analysis/null-deref-path-notes.cpp b/test/Analysis/null-deref-path-notes.cpp
new file mode 100644 (file)
index 0000000..617f5de
--- /dev/null
+++ b/test/Analysis/null-deref-path-notes.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -w -x c++ -analyzer-checker=core -analyzer-output=text -analyzer-eagerly-assume -verify %s
+
+namespace pr34731 {
+int b;
+class c {
+  class B {
+   public:
+    double ***d;
+    B();
+  };
+  void e(double **, int);
+  void f(B &, int &);
+};
+
+// Properly track the null pointer in the array field back to the default
+// constructor of 'h'.
+void c::f(B &g, int &i) {
+  e(g.d[9], i); // expected-warning{{Array access (via field 'd') results in a null pointer dereference}}
+                // expected-note@-1{{Array access (via field 'd') results in a null pointer dereference}}
+  B h, a; // expected-note{{Value assigned to 'h.d'}}
+  a.d == __null; // expected-note{{Assuming the condition is true}}
+  a.d != h.d; // expected-note{{Assuming pointer value is null}}
+  f(h, b); // expected-note{{Calling 'c::f'}}
+}
+}

diff --git a/test/Analysis/null-deref-path-notes.m b/test/Analysis/null-deref-path-notes.m
index 242b5daa9bfc136fa0d672d5559ae6143117e15f..39cf9c79f3116b9015244f4763aaab8bc55cf0df 100644 (file)
--- a/test/Analysis/null-deref-path-notes.m
+++ b/test/Analysis/null-deref-path-notes.m
@@ -50,6 +50,23 @@ void repeatedStores(int coin) {
   *p = 1; // expected-warning{{Dereference of null pointer}} expected-note{{Dereference of null pointer}}
 }
 
+@interface WithArrayPtr
+- (void) useArray;
+@end
+
+@implementation WithArrayPtr {
+@public int *p;
+}
+- (void)useArray {
+  p[1] = 2; // expected-warning{{Array access (via ivar 'p') results in a null pointer dereference}}
+            // expected-note@-1{{Array access (via ivar 'p') results in a null pointer dereference}}
+}
+@end
+
+void testWithArrayPtr(WithArrayPtr *w) {
+  w->p = 0; // expected-note{{Null pointer value stored to 'p'}}
+  [w useArray]; // expected-note{{Calling 'useArray'}}
+}
 
 // CHECK:  <key>diagnostics</key>
 // CHECK-NEXT:  <array>
@@ -801,4 +818,227 @@ void repeatedStores(int coin) {
 // CHECK-NEXT:    <key>file</key><integer>0</integer>
 // CHECK-NEXT:   </dict>
 // CHECK-NEXT:   </dict>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>path</key>
+// CHECK-NEXT:    <array>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>event</string>
+// CHECK-NEXT:      <key>location</key>
+// CHECK-NEXT:      <dict>
+// CHECK-NEXT:       <key>line</key><integer>67</integer>
+// CHECK-NEXT:       <key>col</key><integer>3</integer>
+// CHECK-NEXT:       <key>file</key><integer>0</integer>
+// CHECK-NEXT:      </dict>
+// CHECK-NEXT:      <key>ranges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:        <array>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>67</integer>
+// CHECK-NEXT:          <key>col</key><integer>3</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>67</integer>
+// CHECK-NEXT:          <key>col</key><integer>10</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:        </array>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:      <key>depth</key><integer>0</integer>
+// CHECK-NEXT:      <key>extended_message</key>
+// CHECK-NEXT:      <string>Null pointer value stored to &apos;p&apos;</string>
+// CHECK-NEXT:      <key>message</key>
+// CHECK-NEXT:      <string>Null pointer value stored to &apos;p&apos;</string>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>control</string>
+// CHECK-NEXT:      <key>edges</key>
+// CHECK-NEXT:       <array>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>start</key>
+// CHECK-NEXT:          <array>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>67</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>67</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:          </array>
+// CHECK-NEXT:         <key>end</key>
+// CHECK-NEXT:          <array>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>68</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>68</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:          </array>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:       </array>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>event</string>
+// CHECK-NEXT:      <key>location</key>
+// CHECK-NEXT:      <dict>
+// CHECK-NEXT:       <key>line</key><integer>68</integer>
+// CHECK-NEXT:       <key>col</key><integer>3</integer>
+// CHECK-NEXT:       <key>file</key><integer>0</integer>
+// CHECK-NEXT:      </dict>
+// CHECK-NEXT:      <key>ranges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:        <array>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>68</integer>
+// CHECK-NEXT:          <key>col</key><integer>3</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>68</integer>
+// CHECK-NEXT:          <key>col</key><integer>14</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:        </array>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:      <key>depth</key><integer>0</integer>
+// CHECK-NEXT:      <key>extended_message</key>
+// CHECK-NEXT:      <string>Calling &apos;useArray&apos;</string>
+// CHECK-NEXT:      <key>message</key>
+// CHECK-NEXT:      <string>Calling &apos;useArray&apos;</string>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>event</string>
+// CHECK-NEXT:      <key>location</key>
+// CHECK-NEXT:      <dict>
+// CHECK-NEXT:       <key>line</key><integer>60</integer>
+// CHECK-NEXT:       <key>col</key><integer>1</integer>
+// CHECK-NEXT:       <key>file</key><integer>0</integer>
+// CHECK-NEXT:      </dict>
+// CHECK-NEXT:      <key>depth</key><integer>1</integer>
+// CHECK-NEXT:      <key>extended_message</key>
+// CHECK-NEXT:      <string>Entered call from &apos;testWithArrayPtr&apos;</string>
+// CHECK-NEXT:      <key>message</key>
+// CHECK-NEXT:      <string>Entered call from &apos;testWithArrayPtr&apos;</string>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>control</string>
+// CHECK-NEXT:      <key>edges</key>
+// CHECK-NEXT:       <array>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>start</key>
+// CHECK-NEXT:          <array>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>60</integer>
+// CHECK-NEXT:            <key>col</key><integer>1</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>60</integer>
+// CHECK-NEXT:            <key>col</key><integer>1</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:          </array>
+// CHECK-NEXT:         <key>end</key>
+// CHECK-NEXT:          <array>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>61</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>61</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:          </array>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:       </array>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>control</string>
+// CHECK-NEXT:      <key>edges</key>
+// CHECK-NEXT:       <array>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>start</key>
+// CHECK-NEXT:          <array>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>61</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>61</integer>
+// CHECK-NEXT:            <key>col</key><integer>3</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:          </array>
+// CHECK-NEXT:         <key>end</key>
+// CHECK-NEXT:          <array>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>61</integer>
+// CHECK-NEXT:            <key>col</key><integer>8</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:           <dict>
+// CHECK-NEXT:            <key>line</key><integer>61</integer>
+// CHECK-NEXT:            <key>col</key><integer>8</integer>
+// CHECK-NEXT:            <key>file</key><integer>0</integer>
+// CHECK-NEXT:           </dict>
+// CHECK-NEXT:          </array>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:       </array>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>kind</key><string>event</string>
+// CHECK-NEXT:      <key>location</key>
+// CHECK-NEXT:      <dict>
+// CHECK-NEXT:       <key>line</key><integer>61</integer>
+// CHECK-NEXT:       <key>col</key><integer>8</integer>
+// CHECK-NEXT:       <key>file</key><integer>0</integer>
+// CHECK-NEXT:      </dict>
+// CHECK-NEXT:      <key>ranges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:        <array>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>61</integer>
+// CHECK-NEXT:          <key>col</key><integer>3</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:         <dict>
+// CHECK-NEXT:          <key>line</key><integer>61</integer>
+// CHECK-NEXT:          <key>col</key><integer>3</integer>
+// CHECK-NEXT:          <key>file</key><integer>0</integer>
+// CHECK-NEXT:         </dict>
+// CHECK-NEXT:        </array>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:      <key>depth</key><integer>1</integer>
+// CHECK-NEXT:      <key>extended_message</key>
+// CHECK-NEXT:      <string>Array access (via ivar &apos;p&apos;) results in a null pointer dereference</string>
+// CHECK-NEXT:      <key>message</key>
+// CHECK-NEXT:      <string>Array access (via ivar &apos;p&apos;) results in a null pointer dereference</string>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:    </array>
+// CHECK-NEXT:    <key>description</key><string>Array access (via ivar &apos;p&apos;) results in a null pointer dereference</string>
+// CHECK-NEXT:    <key>category</key><string>Logic error</string>
+// CHECK-NEXT:    <key>type</key><string>Dereference of null pointer</string>
+// CHECK-NEXT:    <key>check_name</key><string>core.NullDereference</string>
+// CHECK-NEXT:    <!-- This hash is experimental and going to change! -->
+// CHECK-NEXT:    <key>issue_hash_content_of_line_in_context</key><string>fb0ad1e4e3090d9834d542eb54bc9d2e</string>
+// CHECK-NEXT:   <key>issue_context_kind</key><string>Objective-C method</string>
+// CHECK-NEXT:   <key>issue_context</key><string>useArray</string>
+// CHECK-NEXT:   <key>issue_hash_function_offset</key><string>1</string>
+// CHECK-NEXT:   <key>location</key>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>line</key><integer>61</integer>
+// CHECK-NEXT:    <key>col</key><integer>8</integer>
+// CHECK-NEXT:    <key>file</key><integer>0</integer>
+// CHECK-NEXT:   </dict>
+// CHECK-NEXT:   </dict>
 // CHECK-NEXT:  </array>