r341722 - [analyzer] Remove the "postponed" hack, deal with derived symbols using an extra map
George Karpenkov via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 7 15:07:57 PDT 2018
Author: george.karpenkov
Date: Fri Sep 7 15:07:57 2018
New Revision: 341722
URL: http://llvm.org/viewvc/llvm-project?rev=341722&view=rev
Log:
[analyzer] Remove the "postponed" hack, deal with derived symbols using 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
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h?rev=341722&r1=341721&r2=341722&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h Fri Sep 7 15:07:57 2018
@@ -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));
}
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h?rev=341722&r1=341721&r2=341722&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h Fri Sep 7 15:07:57 2018
@@ -130,6 +130,8 @@ public:
/// 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));
}
Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=341722&r1=341721&r2=341722&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Fri Sep 7 15:07:57 2018
@@ -2394,7 +2394,10 @@ RegionStoreManager::bindAggregate(Region
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 @@ public:
bool AddToWorkList(const MemRegion *R);
- bool UpdatePostponed();
void VisitBinding(SVal V);
+
+private:
+ void populateWorklistFromSymbol(SymbolRef s);
};
}
@@ -2436,10 +2441,11 @@ void RemoveDeadBindingsWorker::VisitAdde
}
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::VisitAdde
// 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::VisitBind
// 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::VisitBind
// 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;
-
- 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;
- }
+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);
+
+ 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::removeDeadB
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
More information about the cfe-commits
mailing list