r368752 - [analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of interestingness propagation

Kristof Umann via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 13 14:48:17 PDT 2019


Author: szelethus
Date: Tue Aug 13 14:48:17 2019
New Revision: 368752

URL: http://llvm.org/viewvc/llvm-project?rev=368752&view=rev
Log:
[analyzer][NFC] Refactoring BugReporter.cpp P6.: Completely get rid of interestingness propagation

Apparently this does literally nothing.

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.

Differential Revision: https://reviews.llvm.org/D65487

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=368752&r1=368751&r2=368752&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Tue Aug 13 14:48:17 2019
@@ -99,8 +99,6 @@ using CallWithEntryStack = SmallVector<C
 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 =
@@ -126,7 +124,6 @@ public:
   /// TODO: PathDiagnostic has a stack doing the same thing, shouldn't we use
   /// that instead?
   CallWithEntryStack 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.
@@ -943,74 +940,6 @@ void PathDiagnosticBuilder::generateMini
   }
 }
 
-// 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.
 //===----------------------------------------------------------------------===//
@@ -1219,13 +1148,6 @@ void PathDiagnosticBuilder::generatePath
     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();
@@ -1244,13 +1166,6 @@ void PathDiagnosticBuilder::generatePath
     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.
@@ -1267,18 +1182,6 @@ void PathDiagnosticBuilder::generatePath
       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());




More information about the cfe-commits mailing list