This revision was automatically updated to reflect the committed changes.
Closed by commit rL314287: [analyzer] Fix and refactor
bugreporter::getDerefExpr() API. (authored by dergachev).
Changed prior to commit:
https://reviews.llvm.org/D37023?vs=116673&id=116781#toc
Repository:
rL LLVM
https://reviews.llvm.org/D37023
Files:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/null-deref-path-notes.c
cfe/trunk/test/Analysis/null-deref-path-notes.cpp
cfe/trunk/test/Analysis/null-deref-path-notes.m
Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -42,48 +42,68 @@
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) {
Index: cfe/trunk/test/Analysis/null-deref-path-notes.m
===================================================================
--- cfe/trunk/test/Analysis/null-deref-path-notes.m
+++ cfe/trunk/test/Analysis/null-deref-path-notes.m
@@ -50,6 +50,23 @@
*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 @@
// 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>
Index: cfe/trunk/test/Analysis/null-deref-path-notes.c
===================================================================
--- cfe/trunk/test/Analysis/null-deref-path-notes.c
+++ cfe/trunk/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}}
+}
Index: cfe/trunk/test/Analysis/null-deref-path-notes.cpp
===================================================================
--- cfe/trunk/test/Analysis/null-deref-path-notes.cpp
+++ cfe/trunk/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'}}
+}
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
