forked from OSchip/llvm-project
[analyzer] Note last writes to a condition only in a nested stackframe
Exactly what it says on the tin! The comments in the code detail this a little more too. Differential Revision: https://reviews.llvm.org/D64272 llvm-svn: 368817
This commit is contained in:
parent
0e5530abfc
commit
967583bc08
|
@ -132,6 +132,7 @@ class FindLastStoreBRVisitor final : public BugReporterVisitor {
|
|||
|
||||
using TrackingKind = bugreporter::TrackingKind;
|
||||
TrackingKind TKind;
|
||||
const StackFrameContext *OriginSFC;
|
||||
|
||||
public:
|
||||
/// Creates a visitor for every VarDecl inside a Stmt and registers it with
|
||||
|
@ -145,11 +146,18 @@ public:
|
|||
/// \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)
|
||||
bool InEnableNullFPSuppression, TrackingKind TKind,
|
||||
const StackFrameContext *OriginSFC = nullptr)
|
||||
: R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression),
|
||||
TKind(TKind) {
|
||||
TKind(TKind), OriginSFC(OriginSFC) {
|
||||
assert(R);
|
||||
}
|
||||
|
||||
|
|
|
@ -1440,6 +1440,10 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
|
|||
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);
|
||||
|
@ -1465,7 +1469,7 @@ FindLastStoreBRVisitor::VisitNode(const ExplodedNode *Succ,
|
|||
if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) {
|
||||
if (auto KV = State->getSVal(OriginalR).getAs<KnownSVal>())
|
||||
BR.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
|
||||
*KV, OriginalR, EnableNullFPSuppression, TKind));
|
||||
*KV, OriginalR, EnableNullFPSuppression, TKind, OriginSFC));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -1890,6 +1894,7 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
|
|||
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
|
||||
|
@ -1926,7 +1931,7 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
|
|||
if (RR && !LVIsNull)
|
||||
if (auto KV = LVal.getAs<KnownSVal>())
|
||||
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
|
||||
*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.
|
||||
|
@ -1963,7 +1968,7 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
|
|||
|
||||
if (auto KV = V.getAs<KnownSVal>())
|
||||
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
|
||||
*KV, R, EnableNullFPSuppression, TKind));
|
||||
*KV, R, EnableNullFPSuppression, TKind, SFC));
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
@ -2002,7 +2007,7 @@ bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
|
|||
if (CanDereference)
|
||||
if (auto KV = RVal.getAs<KnownSVal>())
|
||||
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
|
||||
*KV, L->getRegion(), EnableNullFPSuppression, TKind));
|
||||
*KV, L->getRegion(), EnableNullFPSuppression, TKind, SFC));
|
||||
|
||||
const MemRegion *RegionRVal = RVal.getAsRegion();
|
||||
if (RegionRVal && isa<SymbolicRegion>(RegionRVal)) {
|
||||
|
|
|
@ -127,10 +127,9 @@ int bar;
|
|||
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 @@ void test() {
|
|||
|
||||
} // 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 @@ bool coin();
|
|||
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 @@ void f(int *ptr) {
|
|||
// 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 @@ void f() {
|
|||
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}}
|
||||
|
@ -314,7 +332,7 @@ namespace tracked_condition_is_only_initialized {
|
|||
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}}
|
||||
|
@ -329,8 +347,8 @@ int flag;
|
|||
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}}
|
||||
|
@ -347,9 +365,8 @@ int getInt();
|
|||
|
||||
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) {
|
||||
|
@ -378,9 +395,9 @@ void f() {
|
|||
int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
|
||||
int y = 0;
|
||||
|
||||
foo(); // tracking-note{{Calling 'foo'}}
|
||||
// tracking-note@-1{{Returning from 'foo'}}
|
||||
y = flag; // tracking-note{{Value assigned to 'y'}}
|
||||
foo(); // tracking-note{{Calling 'foo'}}
|
||||
// tracking-note@-1{{Returning from 'foo'}}
|
||||
y = flag;
|
||||
|
||||
if (y) // expected-note{{Assuming 'y' is not equal to 0}}
|
||||
// expected-note@-1{{Taking true branch}}
|
||||
|
@ -429,7 +446,7 @@ int getInt();
|
|||
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'}}
|
||||
|
||||
|
|
Loading…
Reference in New Issue