Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ 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(S); if (!E) return nullptr; - E = E->IgnoreParenCasts(); while (true) { - if (const BinaryOperator *B = dyn_cast(E)) { - assert(B->isAssignmentOp()); - E = B->getLHS()->IgnoreParenCasts(); - continue; - } - else if (const UnaryOperator *U = dyn_cast(E)) { - if (U->getOpcode() == UO_Deref) - return U->getSubExpr()->IgnoreParenCasts(); - } - else if (const MemberExpr *ME = dyn_cast(E)) { - if (ME->isImplicitAccess()) { - return ME; - } else if (ME->isArrow() || isDeclRefExprToReference(ME->getBase())) { - return ME->getBase()->IgnoreParenCasts(); + if (const CastExpr *CE = dyn_cast(E)) { + if (CE->getCastKind() == CK_LValueToRValue) { + // This cast represents the load we're looking for. + break; + } + E = CE->getSubExpr(); + } else if (isa(E)) { + // Probably more arithmetic can be pattern-matched here, + // but for now give up. + break; + } else if (const UnaryOperator *U = dyn_cast(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(E)) { - return IvarRef->getBase()->IgnoreParenCasts(); - } - else if (const ArraySubscriptExpr *AE = dyn_cast(E)) { - return getDerefExpr(AE->getBase()); - } - else if (isa(E)) { - return E; + // Pattern match for a few useful cases: a[0], p->f, *p etc. + else if (const MemberExpr *ME = dyn_cast(E)) { + E = ME->getBase(); + } else if (const ObjCIvarRefExpr *IvarRef = dyn_cast(E)) { + E = IvarRef->getBase(); + } else if (const ArraySubscriptExpr *AE = dyn_cast(E)) { + E = AE->getBase(); + } else if (const ParenExpr *PE = dyn_cast(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(E)) + if (CE->getCastKind() == CK_LValueToRValue) + E = CE->getSubExpr(); + + return E; } const Stmt *bugreporter::GetDenomExpr(const ExplodedNode *N) { Index: test/Analysis/null-deref-path-notes.c =================================================================== --- /dev/null +++ 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: test/Analysis/null-deref-path-notes.cpp =================================================================== --- /dev/null +++ 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'}} +} +} Index: test/Analysis/null-deref-path-notes.m =================================================================== --- test/Analysis/null-deref-path-notes.m +++ 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: diagnostics // CHECK-NEXT: @@ -801,4 +818,227 @@ // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: path +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line67 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line67 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line67 +// CHECK-NEXT: col10 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Null pointer value stored to 'p' +// CHECK-NEXT: message +// CHECK-NEXT: Null pointer value stored to 'p' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line67 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line67 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line68 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line68 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line68 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line68 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line68 +// CHECK-NEXT: col14 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Calling 'useArray' +// CHECK-NEXT: message +// CHECK-NEXT: Calling 'useArray' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line60 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Entered call from 'testWithArrayPtr' +// CHECK-NEXT: message +// CHECK-NEXT: Entered call from 'testWithArrayPtr' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line60 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line60 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col8 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col8 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col8 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Array access (via ivar 'p') results in a null pointer dereference +// CHECK-NEXT: message +// CHECK-NEXT: Array access (via ivar 'p') results in a null pointer dereference +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: descriptionArray access (via ivar 'p') results in a null pointer dereference +// CHECK-NEXT: categoryLogic error +// CHECK-NEXT: typeDereference of null pointer +// CHECK-NEXT: check_namecore.NullDereference +// CHECK-NEXT: +// CHECK-NEXT: issue_hash_content_of_line_in_contextfb0ad1e4e3090d9834d542eb54bc9d2e +// CHECK-NEXT: issue_context_kindObjective-C method +// CHECK-NEXT: issue_contextuseArray +// CHECK-NEXT: issue_hash_function_offset1 +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line61 +// CHECK-NEXT: col8 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: // CHECK-NEXT: