From 364b9f95fa47b0ca7f1cc694195f7a9953652f81 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Mon, 27 Aug 2012 20:18:30 +0000 Subject: [PATCH] [analyzer] Look through casts when trying to track a null pointer dereference. Also, add comments to addTrackNullOrUndefValueVisitor. Thanks for the review, Anna! git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@162695 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/BugReporterVisitors.cpp | 52 +- test/Analysis/inlining/path-notes.c | 449 ++++++++++++++++-- 2 files changed, 436 insertions(+), 65 deletions(-) diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 956174457b..d17777714a 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -36,7 +36,10 @@ const Stmt *bugreporter::GetDerefExpr(const ExplodedNode *N) { if (!Loc) return 0; - const Stmt *S = Loc->getStmt(); + const Expr *S = dyn_cast(Loc->getStmt()); + if (!S) + return 0; + S = S->IgnoreParenCasts(); while (true) { if (const BinaryOperator *B = dyn_cast(S)) { @@ -316,6 +319,7 @@ public: const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) { + // Only print a message at the interesting return statement. if (ReturnNode != BRC.getNodeResolver().getOriginalNode(N)) return 0; @@ -351,8 +355,7 @@ void bugreporter::addTrackNullOrUndefValueVisitor(const ExplodedNode *N, ProgramStateManager &StateMgr = N->getState()->getStateManager(); - // Walk through nodes until we get one that matches the statement - // exactly. + // Walk through nodes until we get one that matches the statement exactly. while (N) { const ProgramPoint &pp = N->getLocation(); if (const PostStmt *ps = dyn_cast(&pp)) { @@ -370,20 +373,26 @@ void bugreporter::addTrackNullOrUndefValueVisitor(const ExplodedNode *N, ProgramStateRef state = N->getState(); - // Walk through lvalue-to-rvalue conversions. - const Expr *Ex = dyn_cast(S); - if (Ex) { + // See if the expression we're interested refers to a variable. + // If so, we can track both its contents and constraints on its value. + if (const Expr *Ex = dyn_cast(S)) { + // Strip off parens and casts. Note that this will never have issues with + // C++ user-defined implicit conversions, because those have a constructor + // or function call inside. Ex = Ex->IgnoreParenCasts(); if (const DeclRefExpr *DR = dyn_cast(Ex)) { + // FIXME: Right now we only track VarDecls because it's non-trivial to + // get a MemRegion for any other DeclRefExprs. if (const VarDecl *VD = dyn_cast(DR->getDecl())) { const VarRegion *R = StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext()); - // What did we load? + // Mark both the variable region and its contents as interesting. SVal V = state->getRawSVal(loc::MemRegionVal(R)); report->markInteresting(R); report->markInteresting(V); + // If the contents are symbolic, find out when they became null. if (V.getAsLocSymbol()) { BugReporterVisitor *ConstraintTracker = new TrackConstraintBRVisitor(cast(V), false); @@ -396,6 +405,8 @@ void bugreporter::addTrackNullOrUndefValueVisitor(const ExplodedNode *N, } } + // If the expression does NOT refer to a variable, we can still track + // constraints on its contents. SVal V = state->getSValAsScalarOrLoc(S, N->getLocationContext()); // Uncomment this to find cases where we aren't properly getting the @@ -404,29 +415,36 @@ void bugreporter::addTrackNullOrUndefValueVisitor(const ExplodedNode *N, // Is it a symbolic value? if (loc::MemRegionVal *L = dyn_cast(&V)) { - const SubRegion *R = cast(L->getRegion()); - while (R && !isa(R)) { - R = dyn_cast(R->getSuperRegion()); - } - - if (R) { - report->markInteresting(R); - report->addVisitor(new TrackConstraintBRVisitor(loc::MemRegionVal(R), + const MemRegion *Base = L->getRegion()->getBaseRegion(); + if (isa(Base)) { + report->markInteresting(Base); + report->addVisitor(new TrackConstraintBRVisitor(loc::MemRegionVal(Base), false)); } - } else { + } else if (isa(V)) { + // Otherwise, if it's a null constant, we want to know where it came from. + // In particular, if it came from an inlined function call, + // we need to make sure that function isn't pruned in our output. + // Walk backwards to just before the post-statement checks. ProgramPoint PP = N->getLocation(); while (N && isa(PP = N->getLocation())) N = N->getFirstPred(); + // If we found an inlined call before the post-statement checks, look + // for a return statement within the call. if (N && isa(PP)) { - // Find a ReturnStmt, if there is one. + // Walk backwards within the inlined function until we find a statement. + // This will look through multiple levels of inlining, but stop if the + // last thing that happened was an empty function being inlined. + // FIXME: This may not work in the presence of destructors. do { N = N->getFirstPred(); PP = N->getLocation(); } while (!isa(PP) && !isa(PP)); + // If the last statement we found is a 'return ', make sure we + // show this return...and recursively track the value being returned. if (const StmtPoint *SP = dyn_cast(&PP)) { if (const ReturnStmt *Ret = SP->getStmtAs()) { if (const Expr *RetE = Ret->getRetValue()) { diff --git a/test/Analysis/inlining/path-notes.c b/test/Analysis/inlining/path-notes.c index 66a2f270df..c1b03ff693 100644 --- a/test/Analysis/inlining/path-notes.c +++ b/test/Analysis/inlining/path-notes.c @@ -59,10 +59,10 @@ void testStoreCheck(int *a) { int *getZero() { int *p = 0; - // expected-note@-1 {{Variable 'p' initialized to a null pointer value}} + // expected-note@-1 + {{Variable 'p' initialized to a null pointer value}} // ^ This note checks that we add a second visitor for the return value. return p; - // expected-note@-1 {{Returning null pointer (loaded from 'p')}} + // expected-note@-1 + {{Returning null pointer (loaded from 'p')}} } void testReturnZero() { @@ -72,6 +72,13 @@ void testReturnZero() { // expected-note@-3 {{Dereference of null pointer}} } +int testReturnZero2() { + return *getZero(); // expected-warning{{Dereference of null pointer}} + // expected-note@-1 {{Calling 'getZero'}} + // expected-note@-2 {{Returning from 'getZero'}} + // expected-note@-3 {{Dereference of null pointer}} +} + void testInitZero() { // FIXME: Diagnostics: Need to step into the function whose result is assigned to an interesting region int *a = getZero(); @@ -1675,12 +1682,358 @@ void testStoreZero(int *a) { // CHECK: start // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line76 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col8 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: end +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col17 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: kindevent +// CHECK: location +// CHECK: +// CHECK: line76 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: ranges +// CHECK: +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col19 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: depth0 +// CHECK: extended_message +// CHECK: Calling 'getZero' +// CHECK: message +// CHECK: Calling 'getZero' +// CHECK: +// CHECK: +// CHECK: kindevent +// CHECK: location +// CHECK: +// CHECK: line60 +// CHECK: col1 +// CHECK: file0 +// CHECK: +// CHECK: depth1 +// CHECK: extended_message +// CHECK: Entered call from 'testReturnZero2' +// CHECK: message +// CHECK: Entered call from 'testReturnZero2' +// CHECK: +// CHECK: +// CHECK: kindcontrol +// CHECK: edges +// CHECK: +// CHECK: +// CHECK: start +// CHECK: +// CHECK: +// CHECK: line60 +// CHECK: col1 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line60 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: end +// CHECK: +// CHECK: +// CHECK: line61 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line61 +// CHECK: col5 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: kindevent +// CHECK: location +// CHECK: +// CHECK: line61 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: ranges +// CHECK: +// CHECK: +// CHECK: +// CHECK: line61 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line61 +// CHECK: col8 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: depth1 +// CHECK: extended_message +// CHECK: Variable 'p' initialized to a null pointer value +// CHECK: message +// CHECK: Variable 'p' initialized to a null pointer value +// CHECK: +// CHECK: +// CHECK: kindcontrol +// CHECK: edges +// CHECK: +// CHECK: +// CHECK: start +// CHECK: +// CHECK: +// CHECK: line61 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line61 +// CHECK: col5 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: end +// CHECK: +// CHECK: +// CHECK: line64 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line64 +// CHECK: col8 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: kindevent +// CHECK: location +// CHECK: +// CHECK: line64 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: ranges +// CHECK: +// CHECK: +// CHECK: +// CHECK: line64 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line64 +// CHECK: col10 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: depth1 +// CHECK: extended_message +// CHECK: Returning null pointer (loaded from 'p') +// CHECK: message +// CHECK: Returning null pointer (loaded from 'p') +// CHECK: +// CHECK: +// CHECK: kindevent +// CHECK: location +// CHECK: +// CHECK: line76 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: ranges +// CHECK: +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col19 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: depth1 +// CHECK: extended_message +// CHECK: Returning from 'getZero' +// CHECK: message +// CHECK: Returning from 'getZero' +// CHECK: +// CHECK: +// CHECK: kindcontrol +// CHECK: edges +// CHECK: +// CHECK: +// CHECK: start +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col8 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: end +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col17 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: kindcontrol +// CHECK: edges +// CHECK: +// CHECK: +// CHECK: start +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col17 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: end +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col10 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col10 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: kindevent +// CHECK: location +// CHECK: +// CHECK: line76 +// CHECK: col10 +// CHECK: file0 +// CHECK: +// CHECK: ranges +// CHECK: +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col10 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line76 +// CHECK: col19 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: depth0 +// CHECK: extended_message +// CHECK: Dereference of null pointer +// CHECK: message +// CHECK: Dereference of null pointer +// CHECK: +// CHECK: +// CHECK: descriptionDereference of null pointer +// CHECK: categoryLogic error +// CHECK: typeDereference of null pointer +// CHECK: issue_context_kindfunction +// CHECK: issue_contexttestReturnZero2 +// CHECK: issue_hash1 +// CHECK: location +// CHECK: +// CHECK: line76 +// CHECK: col10 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: path +// CHECK: +// CHECK: +// CHECK: kindcontrol +// CHECK: edges +// CHECK: +// CHECK: +// CHECK: start +// CHECK: +// CHECK: +// CHECK: line84 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col5 // CHECK: file0 // CHECK: @@ -1688,12 +2041,12 @@ void testStoreZero(int *a) { // CHECK: end // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col12 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col18 // CHECK: file0 // CHECK: @@ -1709,12 +2062,12 @@ void testStoreZero(int *a) { // CHECK: start // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col5 // CHECK: file0 // CHECK: @@ -1722,12 +2075,12 @@ void testStoreZero(int *a) { // CHECK: end // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col12 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col18 // CHECK: file0 // CHECK: @@ -1743,12 +2096,12 @@ void testStoreZero(int *a) { // CHECK: start // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col12 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col18 // CHECK: file0 // CHECK: @@ -1756,12 +2109,12 @@ void testStoreZero(int *a) { // CHECK: end // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col5 // CHECK: file0 // CHECK: @@ -1773,7 +2126,7 @@ void testStoreZero(int *a) { // CHECK: kindevent // CHECK: location // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col3 // CHECK: file0 // CHECK: @@ -1781,12 +2134,12 @@ void testStoreZero(int *a) { // CHECK: // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col8 // CHECK: file0 // CHECK: @@ -1806,12 +2159,12 @@ void testStoreZero(int *a) { // CHECK: start // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line77 +// CHECK: line84 // CHECK: col5 // CHECK: file0 // CHECK: @@ -1819,12 +2172,12 @@ void testStoreZero(int *a) { // CHECK: end // CHECK: // CHECK: -// CHECK: line79 +// CHECK: line86 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line79 +// CHECK: line86 // CHECK: col3 // CHECK: file0 // CHECK: @@ -1836,7 +2189,7 @@ void testStoreZero(int *a) { // CHECK: kindevent // CHECK: location // CHECK: -// CHECK: line79 +// CHECK: line86 // CHECK: col3 // CHECK: file0 // CHECK: @@ -1844,12 +2197,12 @@ void testStoreZero(int *a) { // CHECK: // CHECK: // CHECK: -// CHECK: line79 +// CHECK: line86 // CHECK: col4 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line79 +// CHECK: line86 // CHECK: col4 // CHECK: file0 // CHECK: @@ -1870,7 +2223,7 @@ void testStoreZero(int *a) { // CHECK: issue_hash4 // CHECK: location // CHECK: -// CHECK: line79 +// CHECK: line86 // CHECK: col3 // CHECK: file0 // CHECK: @@ -1886,12 +2239,12 @@ void testStoreZero(int *a) { // CHECK: start // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: @@ -1899,12 +2252,12 @@ void testStoreZero(int *a) { // CHECK: end // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col7 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col13 // CHECK: file0 // CHECK: @@ -1920,12 +2273,12 @@ void testStoreZero(int *a) { // CHECK: start // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: @@ -1933,12 +2286,12 @@ void testStoreZero(int *a) { // CHECK: end // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col7 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col13 // CHECK: file0 // CHECK: @@ -1954,12 +2307,12 @@ void testStoreZero(int *a) { // CHECK: start // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col7 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col13 // CHECK: file0 // CHECK: @@ -1967,12 +2320,12 @@ void testStoreZero(int *a) { // CHECK: end // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: @@ -1984,7 +2337,7 @@ void testStoreZero(int *a) { // CHECK: kindevent // CHECK: location // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: @@ -1992,12 +2345,12 @@ void testStoreZero(int *a) { // CHECK: // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col15 // CHECK: file0 // CHECK: @@ -2017,12 +2370,12 @@ void testStoreZero(int *a) { // CHECK: start // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line85 +// CHECK: line92 // CHECK: col3 // CHECK: file0 // CHECK: @@ -2030,12 +2383,12 @@ void testStoreZero(int *a) { // CHECK: end // CHECK: // CHECK: -// CHECK: line87 +// CHECK: line94 // CHECK: col3 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line87 +// CHECK: line94 // CHECK: col3 // CHECK: file0 // CHECK: @@ -2047,7 +2400,7 @@ void testStoreZero(int *a) { // CHECK: kindevent // CHECK: location // CHECK: -// CHECK: line87 +// CHECK: line94 // CHECK: col3 // CHECK: file0 // CHECK: @@ -2055,12 +2408,12 @@ void testStoreZero(int *a) { // CHECK: // CHECK: // CHECK: -// CHECK: line87 +// CHECK: line94 // CHECK: col4 // CHECK: file0 // CHECK: // CHECK: -// CHECK: line87 +// CHECK: line94 // CHECK: col4 // CHECK: file0 // CHECK: @@ -2081,7 +2434,7 @@ void testStoreZero(int *a) { // CHECK: issue_hash4 // CHECK: location // CHECK: -// CHECK: line87 +// CHECK: line94 // CHECK: col3 // CHECK: file0 // CHECK: -- 2.40.0