Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, baloghadamsoftware,
Charusso, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy,
mikhail.ramalho, a.sidorin, szepet, whisperity.
Do you remember the teaser in D65378 <https://reviews.llvm.org/D65378>?
When you think about this, it makes sense. If something is really important,
we're tracking it anyways, and that system is sophisticated enough to mark
actually interesting statements as such. I wouldn't say that it's even
//likely// that subexpressions are also interesting (`array[10 - x + x]`), so I
guess even if this produced any effects, its probably undesirable.
---
I originally started working on this refactoring effort to dig from the point
where diagnostics construction starts, down to this part of the code. Turns out
it doesn't do anything. In fact, after the analysis ends, only visitors may
mark objects as interesting (which has any actual effect), which is AMAZING to
know, because `ConditionBRVisitor` only has interestiness to work with when it
constructs a diagnostics piece.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65487
Files:
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -98,8 +98,6 @@
using VisitorsDiagnosticsTy =
llvm::DenseMap<const ExplodedNode *, std::vector<PathDiagnosticPieceRef>>;
-using InterestingExprs = llvm::DenseSet<const Expr *>;
-
/// A map from PathDiagnosticPiece to the LocationContext of the inlined
/// function call it represents.
using LocationContextMap =
@@ -125,7 +123,6 @@
/// TODO: PathDiagnostic has a stack doing the same thing, shouldn't we use
/// that instead?
StackDiagVector CallStack;
- InterestingExprs IE;
/// The bug report we're constructing. For ease of use, this field is kept
/// public, though some "shortcut" getters are provided for commonly used
/// methods of PathDiagnostic.
@@ -942,74 +939,6 @@
}
}
-// Cone-of-influence: support the reverse propagation of "interesting" symbols
-// and values by tracing interesting calculations backwards through evaluated
-// expressions along a path. This is probably overly complicated, but the idea
-// is that if an expression computed an "interesting" value, the child
-// expressions are also likely to be "interesting" as well (which then
-// propagates to the values they in turn compute). This reverse propagation
-// is needed to track interesting correlations across function call boundaries,
-// where formal arguments bind to actual arguments, etc. This is also needed
-// because the constraint solver sometimes simplifies certain symbolic values
-// into constants when appropriate, and this complicates reasoning about
-// interesting values.
-
-static void reversePropagateIntererstingSymbols(BugReport &R,
- InterestingExprs &IE,
- const ProgramState *State,
- const Expr *Ex,
- const LocationContext *LCtx) {
- SVal V = State->getSVal(Ex, LCtx);
- if (!(R.isInteresting(V) || IE.count(Ex)))
- return;
-
- switch (Ex->getStmtClass()) {
- default:
- if (!isa<CastExpr>(Ex))
- break;
- LLVM_FALLTHROUGH;
- case Stmt::BinaryOperatorClass:
- case Stmt::UnaryOperatorClass: {
- for (const Stmt *SubStmt : Ex->children()) {
- if (const auto *child = dyn_cast_or_null<Expr>(SubStmt)) {
- IE.insert(child);
- SVal ChildV = State->getSVal(child, LCtx);
- R.markInteresting(ChildV);
- }
- }
- break;
- }
- }
-
- R.markInteresting(V);
-}
-
-static void reversePropagateInterestingSymbols(BugReport &R,
- InterestingExprs &IE,
- const ProgramState *State,
- const LocationContext *CalleeCtx)
-{
- // FIXME: Handle non-CallExpr-based CallEvents.
- const StackFrameContext *Callee = CalleeCtx->getStackFrame();
- const Stmt *CallSite = Callee->getCallSite();
- if (const auto *CE = dyn_cast_or_null<CallExpr>(CallSite)) {
- if (const auto *FD = dyn_cast<FunctionDecl>(CalleeCtx->getDecl())) {
- FunctionDecl::param_const_iterator PI = FD->param_begin(),
- PE = FD->param_end();
- CallExpr::const_arg_iterator AI = CE->arg_begin(), AE = CE->arg_end();
- for (; AI != AE && PI != PE; ++AI, ++PI) {
- if (const Expr *ArgE = *AI) {
- if (const ParmVarDecl *PD = *PI) {
- Loc LV = State->getLValue(PD, CalleeCtx);
- if (R.isInteresting(LV) || R.isInteresting(State->getRawSVal(LV)))
- IE.insert(ArgE);
- }
- }
- }
- }
- }
-}
-
//===----------------------------------------------------------------------===//
// Functions for determining if a loop was executed 0 times.
//===----------------------------------------------------------------------===//
@@ -1218,13 +1147,6 @@
C.updateLocCtxMap(&Call->path, CE->getCalleeContext());
if (C.shouldAddPathEdges()) {
- const Stmt *S = CE->getCalleeContext()->getCallSite();
- // Propagate the interesting symbols accordingly.
- if (const auto *Ex = dyn_cast_or_null<Expr>(S)) {
- reversePropagateIntererstingSymbols(
- *getBugReport(), C.IE, C.getCurrentNode()->getState().get(), Ex,
- C.getCurrLocationContext());
- }
// Add the edge to the return site.
addEdgeToPath(C.getActivePath(), PrevLoc, Call->callReturn);
PrevLoc.invalidate();
@@ -1243,13 +1165,6 @@
if (!C.shouldAddPathEdges())
return;
- // For expressions, make sure we propagate the
- // interesting symbols correctly.
- if (const Expr *Ex = PS->getStmtAs<Expr>())
- reversePropagateIntererstingSymbols(*getBugReport(), C.IE,
- C.getCurrentNode()->getState().get(),
- Ex, C.getCurrLocationContext());
-
// Add an edge. If this is an ObjCForCollectionStmt do
// not add an edge here as it appears in the CFG both
// as a terminator and as a terminator condition.
@@ -1266,18 +1181,6 @@
return;
}
- // Does this represent entering a call? If so, look at propagating
- // interesting symbols across call boundaries.
- if (const ExplodedNode *NextNode = C.getCurrentNode()->getFirstPred()) {
- const LocationContext *CallerCtx = NextNode->getLocationContext();
- const LocationContext *CalleeCtx = C.getCurrLocationContext();
- if (CallerCtx != CalleeCtx && C.shouldAddPathEdges()) {
- reversePropagateInterestingSymbols(*getBugReport(), C.IE,
- C.getCurrentNode()->getState().get(),
- CalleeCtx);
- }
- }
-
// Are we jumping to the head of a loop? Add a special diagnostic.
if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) {
PathDiagnosticLocation L(Loop, SM, C.getCurrLocationContext());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
