Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -130,6 +130,7 @@ using TrackingKind = bugreporter::TrackingKind; TrackingKind TKind; + const StackFrameContext *OriginSFC; public: /// Creates a visitor for every VarDecl inside a Stmt and registers it with @@ -143,11 +144,19 @@ /// \param EnableNullFPSuppression Whether we should employ false positive /// suppression (inlined defensive checks, returned null). /// \param TKind May limit the amount of notes added to the bug report. + /// \param OriginSFC Only adds notes when the last store happened in a + /// different stackframe to this one. Disregarded if the tracking kind + /// is thorough. + /// This is useful, because for non-tracked regions, notes about + /// changes to its value in a nested stackframe could be pruned, and + /// this visitor can prevent that without polluting the bugpath too + /// much. FindLastStoreBRVisitor(KnownSVal V, const MemRegion *R, bool InEnableNullFPSuppression, - TrackingKind TKind) + TrackingKind TKind, + const StackFrameContext *OriginSFC = nullptr) : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression), - TKind(TKind) { + TKind(TKind), OriginSFC(OriginSFC) { assert(R); } Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -1438,6 +1438,10 @@ StoreSite, InitE, BR, TKind, EnableNullFPSuppression); } + if (TKind == TrackingKind::Condition && + !OriginSFC->isParentOf(StoreSite->getStackFrame())) + return nullptr; + // Okay, we've found the binding. Emit an appropriate message. SmallString<256> sbuf; llvm::raw_svector_ostream os(sbuf); @@ -1463,7 +1467,7 @@ if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) { if (auto KV = State->getSVal(OriginalR).getAs()) BR.addVisitor(llvm::make_unique( - *KV, OriginalR, EnableNullFPSuppression, TKind)); + *KV, OriginalR, EnableNullFPSuppression, TKind, OriginSFC)); } } } @@ -1892,6 +1896,7 @@ return false; ProgramStateRef LVState = LVNode->getState(); + const StackFrameContext *SFC = LVNode->getStackFrame(); // We only track expressions if we believe that they are important. Chances // are good that control dependencies to the tracking point are also improtant @@ -1928,7 +1933,7 @@ if (RR && !LVIsNull) if (auto KV = LVal.getAs()) report.addVisitor(llvm::make_unique( - *KV, RR, EnableNullFPSuppression, TKind)); + *KV, RR, EnableNullFPSuppression, TKind, SFC)); // In case of C++ references, we want to differentiate between a null // reference and reference to null pointer. @@ -1965,7 +1970,7 @@ if (auto KV = V.getAs()) report.addVisitor(llvm::make_unique( - *KV, R, EnableNullFPSuppression, TKind)); + *KV, R, EnableNullFPSuppression, TKind, SFC)); return true; } } @@ -2005,7 +2010,7 @@ if (CanDereference) if (auto KV = RVal.getAs()) report.addVisitor(llvm::make_unique( - *KV, L->getRegion(), EnableNullFPSuppression, TKind)); + *KV, L->getRegion(), EnableNullFPSuppression, TKind, SFC)); const MemRegion *RegionRVal = RVal.getAsRegion(); if (RegionRVal && isa(RegionRVal)) { Index: clang/test/Analysis/track-control-dependency-conditions.cpp =================================================================== --- clang/test/Analysis/track-control-dependency-conditions.cpp +++ clang/test/Analysis/track-control-dependency-conditions.cpp @@ -127,10 +127,9 @@ void test() { int *x = 0; // expected-note{{'x' initialized to a null pointer value}} - if (int flag = foo()) // tracking-note{{'flag' initialized here}} - // debug-note@-1{{Tracking condition 'flag'}} - // expected-note@-2{{Assuming 'flag' is not equal to 0}} - // expected-note@-3{{Taking true branch}} + if (int flag = foo()) // debug-note{{Tracking condition 'flag'}} + // expected-note@-1{{Assuming 'flag' is not equal to 0}} + // expected-note@-2{{Taking true branch}} *x = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} @@ -157,6 +156,28 @@ } // end of namespace variable_declaration_in_condition +namespace note_from_different_but_not_nested_stackframe { + +void nullptrDeref(int *ptr, bool True) { + if (True) // expected-note{{'True' is true}} + // expected-note@-1{{Taking true branch}} + // debug-note@-2{{Tracking condition 'True}} + *ptr = 5; + // expected-note@-1{{Dereference of null pointer (loaded from variable 'ptr')}} + // expected-warning@-2{{Dereference of null pointer (loaded from variable 'ptr')}} +} + +void f() { + int *ptr = nullptr; + // expected-note@-1{{'ptr' initialized to a null pointer value}} + bool True = true; + nullptrDeref(ptr, True); + // expected-note@-1{{Passing null pointer value via 1st parameter 'ptr'}} + // expected-note@-2{{Calling 'nullptrDeref'}} +} + +} // end of namespace note_from_different_but_not_nested_stackframe + namespace important_returning_pointer_loaded_from { bool coin(); @@ -194,8 +215,8 @@ int *getIntPtr(); int *conjurePointer() { - int *i = getIntPtr(); // tracking-note{{'i' initialized here}} - return i; // tracking-note{{Returning pointer (loaded from 'i')}} + int *i = getIntPtr(); + return i; } void f(int *ptr) { @@ -203,11 +224,9 @@ // expected-note@-1{{Taking false branch}} ; if (!conjurePointer()) - // tracking-note@-1{{Calling 'conjurePointer'}} - // tracking-note@-2{{Returning from 'conjurePointer'}} - // debug-note@-3{{Tracking condition '!conjurePointer()'}} - // expected-note@-4{{Assuming the condition is true}} - // expected-note@-5{{Taking true branch}} + // debug-note@-1{{Tracking condition '!conjurePointer()'}} + // expected-note@-2{{Assuming the condition is true}} + // expected-note@-3{{Taking true branch}} *ptr = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} } @@ -225,10 +244,9 @@ int *x = 0; // expected-note{{'x' initialized to a null pointer value}} if (cast(conjure())) - // tracking-note@-1{{Passing value via 1st parameter 'P'}} - // debug-note@-2{{Tracking condition 'cast(conjure())'}} - // expected-note@-3{{Assuming the condition is false}} - // expected-note@-4{{Taking false branch}} + // debug-note@-1{{Tracking condition 'cast(conjure())'}} + // expected-note@-2{{Assuming the condition is false}} + // expected-note@-3{{Taking false branch}} return; *x = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} @@ -284,7 +302,7 @@ int getInt(); void f() { - int flag = getInt(); // tracking-note{{'flag' initialized here}} + int flag = getInt(); int *x = 0; // expected-note{{'x' initialized to a null pointer value}} if (flag) // expected-note{{Assuming 'flag' is not equal to 0}} // expected-note@-1{{Taking true branch}} @@ -299,8 +317,8 @@ int getInt(); void f(int y) { - y = 1; // tracking-note{{The value 1 is assigned to 'y'}} - flag = y; // tracking-note{{The value 1 is assigned to 'flag'}} + y = 1; + flag = y; int *x = 0; // expected-note{{'x' initialized to a null pointer value}} if (flag) // expected-note{{'flag' is 1}} @@ -317,9 +335,8 @@ void foo() { int y; - y = 1; // tracking-note{{The value 1 is assigned to 'y'}} + y = 1; flag = y; // tracking-note{{The value 1 is assigned to 'flag'}} - } void f(int y) { @@ -350,7 +367,7 @@ foo(); // tracking-note{{Calling 'foo'}} // tracking-note@-1{{Returning from 'foo'}} - y = flag; // tracking-note{{Value assigned to 'y'}} + y = flag; if (y) // expected-note{{Assuming 'y' is not equal to 0}} // expected-note@-1{{Taking true branch}} @@ -399,7 +416,7 @@ void f(int flag) { int *x = 0; // expected-note{{'x' initialized to a null pointer value}} - flag = getInt(); // tracking-note{{Value assigned to 'flag'}} + flag = getInt(); assert(flag); // tracking-note{{Calling 'assert'}} // tracking-note@-1{{Returning from 'assert'}}