Szelethus updated this revision to Diff 211758.
Szelethus added a comment.
- Only in a **nested** stackframe. Not different stackframe.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64272/new/
https://reviews.llvm.org/D64272
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/test/Analysis/track-control-dependency-conditions.cpp
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'}}
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<KnownSVal>())
BR.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
- *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<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.
@@ -1965,7 +1970,7 @@
if (auto KV = V.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
- *KV, R, EnableNullFPSuppression, TKind));
+ *KV, R, EnableNullFPSuppression, TKind, SFC));
return true;
}
}
@@ -2005,7 +2010,7 @@
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)) {
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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
