Szelethus updated this revision to Diff 208275.
Szelethus added a comment.
Rebase after D64271 <https://reviews.llvm.org/D64271> being abandoned.
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
@@ -129,10 +129,9 @@
if (int flag = foo()) // tracking-note{{Calling 'foo'}}
// tracking-note@-1{{Returning from 'foo'}}
- // tracking-note@-2{{'flag' initialized here}}
- // debug-note@-3{{Tracking condition 'flag'}}
- // expected-note@-4{{Assuming 'flag' is not equal to 0}}
- // expected-note@-5{{Taking true branch}}
+ // debug-note@-2{{Tracking condition 'flag'}}
+ // expected-note@-3{{Assuming 'flag' is not equal to 0}}
+ // expected-note@-4{{Taking true branch}}
*x = 5; // expected-warning{{Dereference of null pointer}}
// expected-note@-1{{Dereference of null pointer}}
@@ -211,7 +210,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}}
@@ -226,9 +225,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}}
// expected-note@-1{{Taking true branch}}
@@ -244,7 +242,7 @@
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'}}
}
@@ -277,7 +275,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}}
@@ -326,7 +324,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,7 @@
if (!StoreSite)
return nullptr;
+
Satisfied = true;
// If we have an expression that provided the value, try to track where it
@@ -1429,10 +1430,14 @@
if (!IsParam)
InitE = InitE->IgnoreParenCasts();
- bugreporter::trackExpressionValue(StoreSite, InitE, BR,
- EnableNullFPSuppression);
+ ::trackExpressionValue(StoreSite, InitE, BR,
+ EnableNullFPSuppression, TKind);
}
+ if (TKind == TrackingKind::ConditionTracking &&
+ StoreSite->getStackFrame() == OriginSFC)
+ return nullptr;
+
// Okay, we've found the binding. Emit an appropriate message.
SmallString<256> sbuf;
llvm::raw_svector_ostream os(sbuf);
@@ -1458,7 +1463,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));
}
}
}
@@ -1890,6 +1895,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
@@ -1925,7 +1931,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.
@@ -1962,7 +1968,7 @@
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 +2008,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
@@ -139,11 +140,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
