r314287 - [analyzer] Fix and refactor bugreporter::getDerefExpr() API.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 27 02:33:37 PDT 2017
Author: dergachev
Date: Wed Sep 27 02:33:37 2017
New Revision: 314287
URL: http://llvm.org/viewvc/llvm-project?rev=314287&view=rev
Log:
[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
Added:
cfe/trunk/test/Analysis/null-deref-path-notes.c
cfe/trunk/test/Analysis/null-deref-path-notes.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/null-deref-path-notes.m
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=314287&r1=314286&r2=314287&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Sep 27 02:33:37 2017
@@ -42,48 +42,68 @@ bool bugreporter::isDeclRefExprToReferen
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) {
Added: cfe/trunk/test/Analysis/null-deref-path-notes.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-path-notes.c?rev=314287&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/null-deref-path-notes.c (added)
+++ cfe/trunk/test/Analysis/null-deref-path-notes.c Wed Sep 27 02:33:37 2017
@@ -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 at -1{{Array access results in a null pointer dereference}}
+}
Added: cfe/trunk/test/Analysis/null-deref-path-notes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-path-notes.cpp?rev=314287&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/null-deref-path-notes.cpp (added)
+++ cfe/trunk/test/Analysis/null-deref-path-notes.cpp Wed Sep 27 02:33:37 2017
@@ -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 at -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'}}
+}
+}
Modified: cfe/trunk/test/Analysis/null-deref-path-notes.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-path-notes.m?rev=314287&r1=314286&r2=314287&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/null-deref-path-notes.m (original)
+++ cfe/trunk/test/Analysis/null-deref-path-notes.m Wed Sep 27 02:33:37 2017
@@ -50,6 +50,23 @@ void repeatedStores(int coin) {
*p = 1; // expected-warning{{Dereference of null pointer}} expected-note{{Dereference of null pointer}}
}
+ at interface WithArrayPtr
+- (void) useArray;
+ at end
+
+ at implementation WithArrayPtr {
+ at public int *p;
+}
+- (void)useArray {
+ p[1] = 2; // expected-warning{{Array access (via ivar 'p') results in a null pointer dereference}}
+ // expected-note at -1{{Array access (via ivar 'p') results in a null pointer dereference}}
+}
+ at 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 'p'</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Null pointer value stored to 'p'</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 'useArray'</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Calling 'useArray'</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 'testWithArrayPtr'</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Entered call from 'testWithArrayPtr'</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 'p') results in a null pointer dereference</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Array access (via ivar 'p') results in a null pointer dereference</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>description</key><string>Array access (via ivar 'p') 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>
More information about the cfe-commits
mailing list