r368817 - [analyzer] Note last writes to a condition only in a nested stackframe
Kristof Umann via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 14 02:39:38 PDT 2019
Author: szelethus
Date: Wed Aug 14 02:39:38 2019
New Revision: 368817
URL: http://llvm.org/viewvc/llvm-project?rev=368817&view=rev
Log:
[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
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h?rev=368817&r1=368816&r2=368817&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h Wed Aug 14 02:39:38 2019
@@ -132,6 +132,7 @@ class FindLastStoreBRVisitor final : pub
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);
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=368817&r1=368816&r2=368817&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Aug 14 02:39:38 2019
@@ -1440,6 +1440,10 @@ FindLastStoreBRVisitor::VisitNode(const
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
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(c
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(c
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(c
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(c
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)) {
Modified: cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp?rev=368817&r1=368816&r2=368817&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp (original)
+++ cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp Wed Aug 14 02:39:38 2019
@@ -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 at -1{{Tracking condition 'flag'}}
- // expected-note at -2{{Assuming 'flag' is not equal to 0}}
- // expected-note at -3{{Taking true branch}}
+ if (int flag = foo()) // debug-note{{Tracking condition 'flag'}}
+ // expected-note at -1{{Assuming 'flag' is not equal to 0}}
+ // expected-note at -2{{Taking true branch}}
*x = 5; // expected-warning{{Dereference of null pointer}}
// expected-note at -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 at -1{{Taking true branch}}
+ // debug-note at -2{{Tracking condition 'True}}
+ *ptr = 5;
+ // expected-note at -1{{Dereference of null pointer (loaded from variable 'ptr')}}
+ // expected-warning at -2{{Dereference of null pointer (loaded from variable 'ptr')}}
+}
+
+void f() {
+ int *ptr = nullptr;
+ // expected-note at -1{{'ptr' initialized to a null pointer value}}
+ bool True = true;
+ nullptrDeref(ptr, True);
+ // expected-note at -1{{Passing null pointer value via 1st parameter 'ptr'}}
+ // expected-note at -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 at -1{{Taking false branch}}
;
if (!conjurePointer())
- // tracking-note at -1{{Calling 'conjurePointer'}}
- // tracking-note at -2{{Returning from 'conjurePointer'}}
- // debug-note at -3{{Tracking condition '!conjurePointer()'}}
- // expected-note at -4{{Assuming the condition is true}}
- // expected-note at -5{{Taking true branch}}
+ // debug-note at -1{{Tracking condition '!conjurePointer()'}}
+ // expected-note at -2{{Assuming the condition is true}}
+ // expected-note at -3{{Taking true branch}}
*ptr = 5; // expected-warning{{Dereference of null pointer}}
// expected-note at -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 at -1{{Passing value via 1st parameter 'P'}}
- // debug-note at -2{{Tracking condition 'cast(conjure())'}}
- // expected-note at -3{{Assuming the condition is false}}
- // expected-note at -4{{Taking false branch}}
+ // debug-note at -1{{Tracking condition 'cast(conjure())'}}
+ // expected-note at -2{{Assuming the condition is false}}
+ // expected-note at -3{{Taking false branch}}
return;
*x = 5; // expected-warning{{Dereference of null pointer}}
// expected-note at -1{{Dereference of null pointer}}
@@ -314,7 +332,7 @@ namespace tracked_condition_is_only_init
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 at -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 at -1{{Returning from 'foo'}}
- y = flag; // tracking-note{{Value assigned to 'y'}}
+ foo(); // tracking-note{{Calling 'foo'}}
+ // tracking-note at -1{{Returning from 'foo'}}
+ y = flag;
if (y) // expected-note{{Assuming 'y' is not equal to 0}}
// expected-note at -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 at -1{{Returning from 'assert'}}
More information about the cfe-commits
mailing list