Szelethus updated this revision to Diff 211752.
Szelethus added a comment.
- Make sure that the tracking kind is forwarded for each invocation of
`trackExpressionValue` in `BugReporterVisitor.cpp`
- Expose the tracking kind to all users
- Add some more docs
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64270/new/
https://reviews.llvm.org/D64270
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -851,13 +851,13 @@
bool EnableNullFPSuppression;
bool ShouldInvalidate = true;
AnalyzerOptions& Options;
+ bugreporter::TrackingKind TKind;
public:
- ReturnVisitor(const StackFrameContext *Frame,
- bool Suppressed,
- AnalyzerOptions &Options)
+ ReturnVisitor(const StackFrameContext *Frame, bool Suppressed,
+ AnalyzerOptions &Options, bugreporter::TrackingKind TKind)
: CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
- Options(Options) {}
+ Options(Options), TKind(TKind) {}
static void *getTag() {
static int Tag = 0;
@@ -879,7 +879,8 @@
/// bug report, so it can print a note later.
static void addVisitorIfNecessary(const ExplodedNode *Node, const Stmt *S,
BugReport &BR,
- bool InEnableNullFPSuppression) {
+ bool InEnableNullFPSuppression,
+ bugreporter::TrackingKind TKind) {
if (!CallEvent::isCallStmt(S))
return;
@@ -952,7 +953,7 @@
BR.addVisitor(llvm::make_unique<ReturnVisitor>(CalleeContext,
EnableNullFPSuppression,
- Options));
+ Options, TKind));
}
std::shared_ptr<PathDiagnosticPiece>
@@ -1000,8 +1001,9 @@
RetE = RetE->IgnoreParenCasts();
- // If we're returning 0, we should track where that 0 came from.
- bugreporter::trackExpressionValue(N, RetE, BR, EnableNullFPSuppression);
+ // Let's track the return value.
+ bugreporter::trackExpressionValue(
+ N, RetE, BR, TKind, EnableNullFPSuppression);
// Build an appropriate message based on the return value.
SmallString<64> Msg;
@@ -1114,7 +1116,7 @@
if (!State->isNull(*ArgV).isConstrainedTrue())
continue;
- if (bugreporter::trackExpressionValue(N, ArgE, BR, EnableNullFPSuppression))
+ if (trackExpressionValue(N, ArgE, BR, TKind, EnableNullFPSuppression))
ShouldInvalidate = false;
// If we /can't/ track the null pointer, we should err on the side of
@@ -1158,9 +1160,45 @@
ID.AddPointer(&tag);
ID.AddPointer(R);
ID.Add(V);
+ ID.AddInteger(static_cast<int>(TKind));
ID.AddBoolean(EnableNullFPSuppression);
}
+void FindLastStoreBRVisitor::registerStatementVarDecls(
+ BugReport &BR, const Stmt *S, bool EnableNullFPSuppression,
+ TrackingKind TKind) {
+
+ const ExplodedNode *N = BR.getErrorNode();
+ std::deque<const Stmt *> WorkList;
+ WorkList.push_back(S);
+
+ while (!WorkList.empty()) {
+ const Stmt *Head = WorkList.front();
+ WorkList.pop_front();
+
+ ProgramStateManager &StateMgr = N->getState()->getStateManager();
+
+ if (const auto *DR = dyn_cast<DeclRefExpr>(Head)) {
+ if (const auto *VD = dyn_cast<VarDecl>(DR->getDecl())) {
+ const VarRegion *R =
+ StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext());
+
+ // What did we load?
+ SVal V = N->getSVal(S);
+
+ if (V.getAs<loc::ConcreteInt>() || V.getAs<nonloc::ConcreteInt>()) {
+ // Register a new visitor with the BugReport.
+ BR.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
+ V.castAs<KnownSVal>(), R, EnableNullFPSuppression, TKind));
+ }
+ }
+ }
+
+ for (const Stmt *SubStmt : Head->children())
+ WorkList.push_back(SubStmt);
+ }
+}
+
/// Returns true if \p N represents the DeclStmt declaring and initializing
/// \p VR.
static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
@@ -1331,7 +1369,7 @@
// should track the initializer expression.
if (Optional<PostInitializer> PIP = Pred->getLocationAs<PostInitializer>()) {
const MemRegion *FieldReg = (const MemRegion *)PIP->getLocationValue();
- if (FieldReg && FieldReg == R) {
+ if (FieldReg == R) {
StoreSite = Pred;
InitE = PIP->getInitializer()->getInit();
}
@@ -1396,8 +1434,8 @@
if (!IsParam)
InitE = InitE->IgnoreParenCasts();
- bugreporter::trackExpressionValue(StoreSite, InitE, BR,
- EnableNullFPSuppression);
+ bugreporter::trackExpressionValue(
+ StoreSite, InitE, BR, TKind, EnableNullFPSuppression);
}
// Okay, we've found the binding. Emit an appropriate message.
@@ -1425,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));
+ *KV, OriginalR, EnableNullFPSuppression, TKind));
}
}
}
@@ -1727,7 +1765,8 @@
// expression, hence the BugReport level set.
if (BR.addTrackedCondition(N)) {
bugreporter::trackExpressionValue(
- N, Condition, BR, /*EnableNullFPSuppression=*/false);
+ N, Condition, BR, bugreporter::TrackingKind::Condition,
+ /*EnableNullFPSuppression=*/false);
return constructDebugPieceForTrackedCondition(Condition, N, BRC);
}
}
@@ -1841,7 +1880,9 @@
bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
const Expr *E, BugReport &report,
+ bugreporter::TrackingKind TKind,
bool EnableNullFPSuppression) {
+
if (!E || !InputNode)
return false;
@@ -1865,12 +1906,13 @@
// At this point in the path, the receiver should be live since we are at the
// message send expr. If it is nil, start tracking it.
if (const Expr *Receiver = NilReceiverBRVisitor::getNilReceiver(Inner, LVNode))
- trackExpressionValue(LVNode, Receiver, report, EnableNullFPSuppression);
+ trackExpressionValue(
+ LVNode, Receiver, report, TKind, EnableNullFPSuppression);
// Track the index if this is an array subscript.
if (const auto *Arr = dyn_cast<ArraySubscriptExpr>(Inner))
trackExpressionValue(
- LVNode, Arr->getIdx(), report, /*EnableNullFPSuppression*/ false);
+ LVNode, Arr->getIdx(), report, TKind, /*EnableNullFPSuppression*/false);
// See if the expression we're interested refers to a variable.
// If so, we can track both its contents and constraints on its value.
@@ -1886,7 +1928,7 @@
if (RR && !LVIsNull)
if (auto KV = LVal.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
- *KV, RR, EnableNullFPSuppression));
+ *KV, RR, EnableNullFPSuppression, TKind));
// In case of C++ references, we want to differentiate between a null
// reference and reference to null pointer.
@@ -1923,7 +1965,7 @@
if (auto KV = V.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
- *KV, R, EnableNullFPSuppression));
+ *KV, R, EnableNullFPSuppression, TKind));
return true;
}
}
@@ -1933,7 +1975,7 @@
SVal V = LVState->getSValAsScalarOrLoc(Inner, LVNode->getLocationContext());
ReturnVisitor::addVisitorIfNecessary(
- LVNode, Inner, report, EnableNullFPSuppression);
+ LVNode, Inner, report, EnableNullFPSuppression, TKind);
// Is it a symbolic value?
if (auto L = V.getAs<loc::MemRegionVal>()) {
@@ -1963,7 +2005,7 @@
if (CanDereference)
if (auto KV = RVal.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
- *KV, L->getRegion(), EnableNullFPSuppression));
+ *KV, L->getRegion(), EnableNullFPSuppression, TKind));
const MemRegion *RegionRVal = RVal.getAsRegion();
if (RegionRVal && isa<SymbolicRegion>(RegionRVal)) {
@@ -2021,8 +2063,9 @@
// The receiver was nil, and hence the method was skipped.
// Register a BugReporterVisitor to issue a message telling us how
// the receiver was null.
- bugreporter::trackExpressionValue(N, Receiver, BR,
- /*EnableNullFPSuppression*/ false);
+ bugreporter::trackExpressionValue(
+ N, Receiver, BR, bugreporter::TrackingKind::Thorough,
+ /*EnableNullFPSuppression*/ false);
// Issue a message saying that the method was skipped.
PathDiagnosticLocation L(Receiver, BRC.getSourceManager(),
N->getLocationContext());
@@ -2030,45 +2073,6 @@
}
//===----------------------------------------------------------------------===//
-// Implementation of FindLastStoreBRVisitor.
-//===----------------------------------------------------------------------===//
-
-// Registers every VarDecl inside a Stmt with a last store visitor.
-void FindLastStoreBRVisitor::registerStatementVarDecls(BugReport &BR,
- const Stmt *S,
- bool EnableNullFPSuppression) {
- const ExplodedNode *N = BR.getErrorNode();
- std::deque<const Stmt *> WorkList;
- WorkList.push_back(S);
-
- while (!WorkList.empty()) {
- const Stmt *Head = WorkList.front();
- WorkList.pop_front();
-
- ProgramStateManager &StateMgr = N->getState()->getStateManager();
-
- if (const auto *DR = dyn_cast<DeclRefExpr>(Head)) {
- if (const auto *VD = dyn_cast<VarDecl>(DR->getDecl())) {
- const VarRegion *R =
- StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext());
-
- // What did we load?
- SVal V = N->getSVal(S);
-
- if (V.getAs<loc::ConcreteInt>() || V.getAs<nonloc::ConcreteInt>()) {
- // Register a new visitor with the BugReport.
- BR.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
- V.castAs<KnownSVal>(), R, EnableNullFPSuppression));
- }
- }
- }
-
- for (const Stmt *SubStmt : Head->children())
- WorkList.push_back(SubStmt);
- }
-}
-
-//===----------------------------------------------------------------------===//
// Visitor that tries to report interesting diagnostics from conditions.
//===----------------------------------------------------------------------===//
Index: clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
@@ -87,7 +87,8 @@
if (const Expr *Ex = FindBlockDeclRefExpr(BE->getBody(), VD))
R->addRange(Ex->getSourceRange());
R->addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
- *V, VR, /*EnableNullFPSuppression*/ false));
+ *V, VR, /*EnableNullFPSuppression*/ false,
+ bugreporter::TrackingKind::Thorough));
R->disablePathPruning();
// need location of block
C.emitReport(std::move(R));
Index: clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
@@ -146,8 +146,8 @@
initBugType();
auto R = llvm::make_unique<BugReport>(*BT, "Index is out of bounds", N);
R->addRange(IdxExpr->getSourceRange());
- bugreporter::trackExpressionValue(N, IdxExpr, *R,
- /*EnableNullFPSuppression=*/false);
+ bugreporter::trackExpressionValue(
+ N, IdxExpr, *R, bugreporter::TrackingKind::Thorough, false);
C.emitReport(std::move(R));
return;
}
Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -282,7 +282,8 @@
N);
R->addRange(RS->getSourceRange());
- bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false);
+ bugreporter::trackExpressionValue(N, RS->getRetValue(), *R,
+ bugreporter::TrackingKind::Thorough, false);
C.emitReport(std::move(R));
}
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
@@ -83,6 +83,40 @@
BugReport &BR);
};
+namespace bugreporter {
+
+/// Specifies the type of tracking for an expression.
+enum class TrackingKind {
+ /// Default tracking kind -- specifies that as much information should be
+ /// gathered about the tracked expression value as possible.
+ Thorough,
+ /// Specifies that a more moderate tracking should be used for the expression
+ /// value. This will essentially make sure that functions relevant to the it
+ /// aren't pruned, but otherwise relies on the user reading the code or
+ /// following the arrows.
+ Condition
+};
+
+/// Attempts to add visitors to track expression value back to its point of
+/// origin.
+///
+/// \param N A node "downstream" from the evaluation of the statement.
+/// \param E The expression value which we are tracking
+/// \param R The bug report to which visitors should be attached.
+/// \param EnableNullFPSuppression Whether we should employ false positive
+/// suppression (inlined defensive checks, returned null).
+///
+/// \return Whether or not the function was able to add visitors for this
+/// statement. Note that returning \c true does not actually imply
+/// that any visitors were added.
+bool trackExpressionValue(const ExplodedNode *N, const Expr *E, BugReport &R,
+ TrackingKind TKind = TrackingKind::Thorough,
+ bool EnableNullFPSuppression = true);
+
+const Expr *getDerefExpr(const Stmt *S);
+
+} // namespace bugreporter
+
/// Finds last store into the given region,
/// which is different from a given symbolic value.
class FindLastStoreBRVisitor final : public BugReporterVisitor {
@@ -94,15 +128,28 @@
/// bug, we are going to employ false positive suppression.
bool EnableNullFPSuppression;
+ using TrackingKind = bugreporter::TrackingKind;
+ TrackingKind TKind;
+
public:
/// Creates a visitor for every VarDecl inside a Stmt and registers it with
/// the BugReport.
static void registerStatementVarDecls(BugReport &BR, const Stmt *S,
- bool EnableNullFPSuppression);
-
+ bool EnableNullFPSuppression,
+ TrackingKind TKind);
+
+ /// \param V We're searching for the store where \c R received this value.
+ /// \param R The region we're tracking.
+ /// \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.
FindLastStoreBRVisitor(KnownSVal V, const MemRegion *R,
- bool InEnableNullFPSuppression)
- : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression) {}
+ bool InEnableNullFPSuppression,
+ TrackingKind TKind)
+ : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression),
+ TKind(TKind) {
+ assert(R);
+ }
void Profile(llvm::FoldingSetNodeID &ID) const override;
@@ -345,27 +392,6 @@
BugReport &R) override;
};
-namespace bugreporter {
-
-/// Attempts to add visitors to track expression value back to its point of
-/// origin.
-///
-/// \param N A node "downstream" from the evaluation of the statement.
-/// \param E The expression value which we are tracking
-/// \param R The bug report to which visitors should be attached.
-/// \param EnableNullFPSuppression Whether we should employ false positive
-/// suppression (inlined defensive checks, returned null).
-///
-/// \return Whether or not the function was able to add visitors for this
-/// statement. Note that returning \c true does not actually imply
-/// that any visitors were added.
-bool trackExpressionValue(const ExplodedNode *N, const Expr *E, BugReport &R,
- bool EnableNullFPSuppression = true);
-
-const Expr *getDerefExpr(const Stmt *S);
-
-} // namespace bugreporter
-
} // namespace ento
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
