Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, dcoughlin, Charusso,
baloghadamsoftware.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D64271: [analyzer] Don't track the right
hand side of the last store for conditions.
Exactly what it says on the tin! The comments in the code detail this a little
more too.
Repository:
rG LLVM Github Monorepo
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}}
@@ -209,7 +208,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}}
@@ -225,7 +224,7 @@
void f(int y) {
y = 1;
- flag = y; // tracking-note{{The value 1 is assigned to 'flag'}}
+ flag = y;
int *x = 0; // expected-note{{'x' initialized to a null pointer value}}
if (flag) // expected-note{{'flag' is 1}}
// expected-note@-1{{Taking true branch}}
@@ -298,7 +297,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
@@ -1421,6 +1421,11 @@
if (!StoreSite)
return nullptr;
+
+ if (TKind != TrackingKind::ThoroughTracking &&
+ StoreSite->getStackFrame() == OriginSFC)
+ return nullptr;
+
Satisfied = true;
// If we have an expression that provided the value, try to track where it
@@ -1466,7 +1471,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));
}
}
}
@@ -1898,6 +1903,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
@@ -1933,7 +1939,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.
@@ -1970,7 +1976,7 @@
if (auto KV = V.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
- *KV, R, EnableNullFPSuppression, TKind));
+ *KV, R, EnableNullFPSuppression, TKind, SFC));
return true;
}
}
@@ -2008,7 +2014,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
@@ -126,6 +126,7 @@
using TrackingKind = bugreporter::TrackingKind;
TrackingKind TKind;
+ const StackFrameContext *OriginSFC;
public:
/// Creates a visitor for every VarDecl inside a Stmt and registers it with
@@ -140,11 +141,19 @@
/// suppression (inlined defensive checks, returned null).
/// \param TKind May limit the amount of notes added to the bug report. For
/// conditions, ignores the initialization point.
+ /// \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
