[analyzer] Remove the "postponed" hack, deal with derived symbols usi…

…ng an extra map

The "derived" symbols indicate children fields of a larger symbol.
As parents do not have pointers to their children, the garbage
collection algorithm the analyzer currently uses adds such symbols into
a "postponed" category, and then keeps running through the worklist
until the fixed point is reached.

The current patch rectifies that by instead using a helper map which
stores pointers from parents to children, so that no fixed point
calculation is necessary.

The current patch yields ~5% improvement in running time on sqlite.

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

llvm-svn: 341722

clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h

@@ -769,6 +769,9 @@ class SymbolicRegion : public SubRegion {
     assert(s->getType()->isAnyPointerType() ||
            s->getType()->isReferenceType() ||
            s->getType()->isBlockPointerType());
+
+    // populateWorklistFromSymbol() relies on this assertion, and needs to be
+    // updated if more cases are introduced.
     assert(isa<UnknownSpaceRegion>(sreg) || isa<HeapSpaceRegion>(sreg));
   }
 

clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h

@@ -130,6 +130,8 @@ class StoreManager {
   ///  used to query and manipulate MemRegion objects.
   MemRegionManager& getRegionManager() { return MRMgr; }
 
+  SValBuilder& getSValBuilder() { return svalBuilder; }
+
   virtual Loc getLValueVar(const VarDecl *VD, const LocationContext *LC) {
     return svalBuilder.makeLoc(MRMgr.getVarRegion(VD, LC));
   }

clang/lib/StaticAnalyzer/Core/RegionStore.cpp

@@ -2394,7 +2394,10 @@ RegionStoreManager::bindAggregate(RegionBindingsConstRef B,
 namespace {
 class RemoveDeadBindingsWorker
     : public ClusterAnalysis<RemoveDeadBindingsWorker> {
-  SmallVector<const SymbolicRegion *, 12> Postponed;
+  using ChildrenListTy = SmallVector<const SymbolDerived *, 4>;
+  using MapParentsToDerivedTy = llvm::DenseMap<SymbolRef, ChildrenListTy>;
+
+  MapParentsToDerivedTy ParentsToDerived;
   SymbolReaper &SymReaper;
   const StackFrameContext *CurrentLCtx;
 
@@ -2415,8 +2418,10 @@ class RemoveDeadBindingsWorker
 
   bool AddToWorkList(const MemRegion *R);
 
-  bool UpdatePostponed();
   void VisitBinding(SVal V);
+
+private:
+  void populateWorklistFromSymbol(SymbolRef s);
 };
 }
 
@@ -2436,10 +2441,11 @@ void RemoveDeadBindingsWorker::VisitAddedToCluster(const MemRegion *baseR,
   }
 
   if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(baseR)) {
-    if (SymReaper.isLive(SR->getSymbol()))
+    if (SymReaper.isLive(SR->getSymbol())) {
       AddToWorkList(SR, &C);
-    else
-      Postponed.push_back(SR);
+    } else if (const auto *SD = dyn_cast<SymbolDerived>(SR->getSymbol())) {
+      ParentsToDerived[SD->getParentSymbol()].push_back(SD);
+    }
 
     return;
   }
@@ -2451,7 +2457,7 @@ void RemoveDeadBindingsWorker::VisitAddedToCluster(const MemRegion *baseR,
 
   // CXXThisRegion in the current or parent location context is live.
   if (const CXXThisRegion *TR = dyn_cast<CXXThisRegion>(baseR)) {
-    const StackArgumentsSpaceRegion *StackReg =
+    const auto *StackReg =
       cast<StackArgumentsSpaceRegion>(TR->getSuperRegion());
     const StackFrameContext *RegCtx = StackReg->getStackFrame();
     if (CurrentLCtx &&
@@ -2496,6 +2502,15 @@ void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
   // If V is a region, then add it to the worklist.
   if (const MemRegion *R = V.getAsRegion()) {
     AddToWorkList(R);
+
+    if (const auto *TVR = dyn_cast<TypedValueRegion>(R)) {
+      DefinedOrUnknownSVal RVS =
+          RM.getSValBuilder().getRegionValueSymbolVal(TVR);
+      if (const MemRegion *SR = RVS.getAsRegion()) {
+        AddToWorkList(SR);
+      }
+    }
+
     SymReaper.markLive(R);
 
     // All regions captured by a block are also live.
@@ -2509,27 +2524,30 @@ void RemoveDeadBindingsWorker::VisitBinding(SVal V) {
 
 
   // Update the set of live symbols.
-  for (SymExpr::symbol_iterator SI = V.symbol_begin(), SE = V.symbol_end();
-       SI!=SE; ++SI)
+  for (auto SI = V.symbol_begin(), SE = V.symbol_end(); SI != SE; ++SI) {
+    populateWorklistFromSymbol(*SI);
+
+    for (const auto *SD : ParentsToDerived[*SI])
+      populateWorklistFromSymbol(SD);
+
     SymReaper.markLive(*SI);
+  }
 }
 
-bool RemoveDeadBindingsWorker::UpdatePostponed() {
-  // See if any postponed SymbolicRegions are actually live now, after
-  // having done a scan.
-  bool changed = false;
+void RemoveDeadBindingsWorker::populateWorklistFromSymbol(SymbolRef S) {
+  if (const auto *SD = dyn_cast<SymbolData>(S)) {
+    if (Loc::isLocType(SD->getType()) && !SymReaper.isLive(SD)) {
+      const SymbolicRegion *SR = RM.getRegionManager().getSymbolicRegion(SD);
 
-  for (SmallVectorImpl<const SymbolicRegion*>::iterator
-        I = Postponed.begin(), E = Postponed.end() ; I != E ; ++I) {
-    if (const SymbolicRegion *SR = *I) {
-      if (SymReaper.isLive(SR->getSymbol())) {
-        changed |= AddToWorkList(SR);
-        *I = nullptr;
-      }
+      if (B.contains(SR))
+        AddToWorkList(SR);
+
+      const SymbolicRegion *SHR =
+          RM.getRegionManager().getSymbolicHeapRegion(SD);
+      if (B.contains(SHR))
+        AddToWorkList(SHR);
     }
   }
-
-  return changed;
 }
 
 StoreRef RegionStoreManager::removeDeadBindings(Store store,
@@ -2545,7 +2563,7 @@ StoreRef RegionStoreManager::removeDeadBindings(Store store,
     W.AddToWorkList(*I);
   }
 
-  do W.RunWorkList(); while (W.UpdatePostponed());
+  W.RunWorkList();
 
   // We have now scanned the store, marking reachable regions and symbols
   // as live.  We now remove all the regions that are dead from the store