Szelethus updated this revision to Diff 210337.
Szelethus marked an inline comment as done.
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/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
@@ -71,6 +71,20 @@
// Utility functions.
//===----------------------------------------------------------------------===//
+/// Implementation function for trackExpressionValue().
+static bool trackExpressionValue(
+ const ExplodedNode *InputNode,
+ const Expr *E, BugReport &report,
+ bool EnableNullFPSuppression,
+ bugreporter::TrackingKind TKind);
+
+bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
+ const Expr *E, BugReport &report,
+ bool EnableNullFPSuppression) {
+ return ::trackExpressionValue(InputNode, E, report, EnableNullFPSuppression,
+ TrackingKind::ThoroughTracking);
+}
+
static const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
if (B->isAdditiveOp() && B->getType()->isPointerType()) {
if (B->getLHS()->getType()->isPointerType()) {
@@ -1162,9 +1176,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) {
@@ -1335,7 +1385,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();
}
@@ -1400,8 +1450,7 @@
if (!IsParam)
InitE = InitE->IgnoreParenCasts();
- bugreporter::trackExpressionValue(StoreSite, InitE, BR,
- EnableNullFPSuppression);
+ trackExpressionValue(StoreSite, InitE, BR, EnableNullFPSuppression, TKind);
}
// Okay, we've found the binding. Emit an appropriate message.
@@ -1429,7 +1478,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));
}
}
}
@@ -1730,8 +1779,9 @@
// isn't sufficient, because a new visitor is created for each tracked
// expression, hence the BugReport level set.
if (BR.addTrackedCondition(N)) {
- bugreporter::trackExpressionValue(
- N, Condition, BR, /*EnableNullFPSuppression=*/false);
+ trackExpressionValue(
+ N, Condition, BR, /*EnableNullFPSuppression=*/false,
+ bugreporter::TrackingKind::ConditionTracking);
return constructDebugPieceForTrackedCondition(Condition, N, BRC);
}
}
@@ -1843,9 +1893,14 @@
return N;
}
-bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
- const Expr *E, BugReport &report,
- bool EnableNullFPSuppression) {
+static bool trackExpressionValue(
+ const ExplodedNode *InputNode,
+ const Expr *E, BugReport &report,
+ bool EnableNullFPSuppression,
+ bugreporter::TrackingKind TKind /* = TrackingKind::ThoroughTracking*/) {
+
+ using namespace bugreporter;
+
if (!E || !InputNode)
return false;
@@ -1890,7 +1945,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.
@@ -1927,7 +1982,7 @@
if (auto KV = V.getAs<KnownSVal>())
report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
- *KV, R, EnableNullFPSuppression));
+ *KV, R, EnableNullFPSuppression, TKind));
return true;
}
}
@@ -1967,7 +2022,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)) {
@@ -2034,45 +2089,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::ThoroughTracking));
R->disablePathPruning();
// need location of block
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,36 @@
BugReport &BR);
};
+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);
+
+/// Specifies the type of tracking for an expression. For instance, we'd like to
+/// gather far more information about a variable found to be a cause of a null
+/// pointer dereference, while tracking a condition to that extent would pollute
+/// the bug report without much of an added value.
+enum class TrackingKind {
+ ThoroughTracking,
+ ConditionTracking
+};
+
+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 +124,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 +388,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
