Skip to content

Commit

Permalink
[analyzer] Change SymbolReaper to store region roots implied by the E…
Browse files Browse the repository at this point in the history
…nvironment, allowing it be queried when

determining if symbols derived from regions are still live.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@137005 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
tkremenek committed Aug 6, 2011
1 parent 5a58c6d commit bea2753
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ class EnvironmentManager {
SVal V);

Environment removeDeadBindings(Environment Env,
SymbolReaper &SymReaper, const GRState *ST,
SmallVectorImpl<const MemRegion*>& RegionRoots);
SymbolReaper &SymReaper, const GRState *ST);
};

} // end GR namespace
Expand Down
3 changes: 1 addition & 2 deletions include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,7 @@ class StoreManager {
const MemRegion *castRegion(const MemRegion *region, QualType CastToTy);

virtual StoreRef removeDeadBindings(Store store, const StackFrameContext *LCtx,
SymbolReaper& SymReaper,
SmallVectorImpl<const MemRegion*>& RegionRoots) = 0;
SymbolReaper& SymReaper) = 0;

virtual StoreRef BindDecl(Store store, const VarRegion *VR, SVal initVal) = 0;

Expand Down
22 changes: 17 additions & 5 deletions include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -416,11 +416,15 @@ class SymbolManager {
};

class SymbolReaper {
typedef llvm::DenseSet<SymbolRef> SetTy;
typedef llvm::DenseSet<SymbolRef> SymbolSetTy;
typedef llvm::DenseSet<const MemRegion *> RegionSetTy;

SetTy TheLiving;
SetTy MetadataInUse;
SetTy TheDead;
SymbolSetTy TheLiving;
SymbolSetTy MetadataInUse;
SymbolSetTy TheDead;

RegionSetTy RegionRoots;

const LocationContext *LCtx;
const Stmt *Loc;
SymbolManager& SymMgr;
Expand All @@ -438,6 +442,7 @@ class SymbolReaper {
const Stmt *getCurrentStatement() const { return Loc; }

bool isLive(SymbolRef sym);
bool isLiveRegion(const MemRegion *region);
bool isLive(const Stmt *ExprVal) const;
bool isLive(const VarRegion *VR, bool includeStoreBindings = false) const;

Expand All @@ -458,13 +463,17 @@ class SymbolReaper {
// Returns true if the symbol is dead, false if live.
bool maybeDead(SymbolRef sym);

typedef SetTy::const_iterator dead_iterator;
typedef SymbolSetTy::const_iterator dead_iterator;
dead_iterator dead_begin() const { return TheDead.begin(); }
dead_iterator dead_end() const { return TheDead.end(); }

bool hasDeadSymbols() const {
return !TheDead.empty();
}

typedef RegionSetTy::const_iterator region_iterator;
region_iterator region_begin() const { return RegionRoots.begin(); }
region_iterator region_end() const { return RegionRoots.end(); }

/// isDead - Returns whether or not a symbol has been confirmed dead. This
/// should only be called once all marking of dead symbols has completed.
Expand All @@ -473,6 +482,8 @@ class SymbolReaper {
return TheDead.count(sym);
}

void markLive(const MemRegion *region);

/// Set to the value of the symbolic store after
/// StoreManager::removeDeadBindings has been called.
void setReapedStore(StoreRef st) { reapedStore = st; }
Expand All @@ -484,6 +495,7 @@ class SymbolVisitor {
// GRStateManager::scanReachableSymbols. The method returns \c true if
// symbols should continue be scanned and \c false otherwise.
virtual bool VisitSymbol(SymbolRef sym) = 0;
virtual bool VisitMemRegion(const MemRegion *region) { return true; };
virtual ~SymbolVisitor();
};

Expand Down
7 changes: 3 additions & 4 deletions lib/StaticAnalyzer/Core/Environment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,7 @@ static inline bool IsLocation(const Stmt *S) {
Environment
EnvironmentManager::removeDeadBindings(Environment Env,
SymbolReaper &SymReaper,
const GRState *ST,
SmallVectorImpl<const MemRegion*> &DRoots) {
const GRState *ST) {

// We construct a new Environment object entirely, as this is cheaper than
// individually removing all the subexpression bindings (which will greatly
Expand Down Expand Up @@ -175,8 +174,8 @@ EnvironmentManager::removeDeadBindings(Environment Env,

// If the block expr's value is a memory region, then mark that region.
if (isa<loc::MemRegionVal>(X)) {
const MemRegion* R = cast<loc::MemRegionVal>(X).getRegion();
DRoots.push_back(R);
const MemRegion *R = cast<loc::MemRegionVal>(X).getRegion();
SymReaper.markLive(R);
}

// Mark all symbols in the block expr's value live.
Expand Down
6 changes: 2 additions & 4 deletions lib/StaticAnalyzer/Core/GRState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,13 @@ GRStateManager::removeDeadBindings(const GRState* state,
// those around. This code more than likely can be made faster, and the
// frequency of which this method is called should be experimented with
// for optimum performance.
SmallVector<const MemRegion*, 10> RegionRoots;
GRState NewState = *state;

NewState.Env = EnvMgr.removeDeadBindings(NewState.Env, SymReaper,
state, RegionRoots);
NewState.Env = EnvMgr.removeDeadBindings(NewState.Env, SymReaper, state);

// Clean up the store.
StoreRef newStore = StoreMgr->removeDeadBindings(NewState.getStore(), LCtx,
SymReaper, RegionRoots);
SymReaper);
NewState.setStore(newStore);
SymReaper.setReapedStore(newStore);

Expand Down
15 changes: 7 additions & 8 deletions lib/StaticAnalyzer/Core/RegionStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -373,10 +373,10 @@ class RegionStoreManager : public StoreManager {
/// removeDeadBindings - Scans the RegionStore of 'state' for dead values.
/// It returns a new Store with these values removed.
StoreRef removeDeadBindings(Store store, const StackFrameContext *LCtx,
SymbolReaper& SymReaper,
SmallVectorImpl<const MemRegion*>& RegionRoots);
SymbolReaper& SymReaper);

StoreRef enterStackFrame(const GRState *state, const StackFrameContext *frame);
StoreRef enterStackFrame(const GRState *state,
const StackFrameContext *frame);

//===------------------------------------------------------------------===//
// Region "extents".
Expand Down Expand Up @@ -1774,17 +1774,16 @@ bool removeDeadBindingsWorker::UpdatePostponed() {

StoreRef RegionStoreManager::removeDeadBindings(Store store,
const StackFrameContext *LCtx,
SymbolReaper& SymReaper,
SmallVectorImpl<const MemRegion*>& RegionRoots)
{
SymbolReaper& SymReaper) {
RegionBindings B = GetRegionBindings(store);
removeDeadBindingsWorker W(*this, StateMgr, B, SymReaper, LCtx);
W.GenerateClusters();

// Enqueue the region roots onto the worklist.
for (SmallVectorImpl<const MemRegion*>::iterator I=RegionRoots.begin(),
E=RegionRoots.end(); I!=E; ++I)
for (SymbolReaper::region_iterator I = SymReaper.region_begin(),
E = SymReaper.region_end(); I != E; ++I) {
W.AddToWorkList(*I);
}

do W.RunWorkList(); while (W.UpdatePostponed());

Expand Down
17 changes: 12 additions & 5 deletions lib/StaticAnalyzer/Core/SymbolManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@ void SymbolReaper::markLive(SymbolRef sym) {
TheDead.erase(sym);
}

void SymbolReaper::markLive(const MemRegion *region) {
RegionRoots.insert(region);
}

void SymbolReaper::markInUse(SymbolRef sym) {
if (isa<SymbolMetadata>(sym))
MetadataInUse.insert(sym);
Expand All @@ -266,14 +270,17 @@ bool SymbolReaper::maybeDead(SymbolRef sym) {
return true;
}

static bool IsLiveRegion(SymbolReaper &Reaper, const MemRegion *MR) {
bool SymbolReaper::isLiveRegion(const MemRegion *MR) {
if (RegionRoots.count(MR))
return true;

MR = MR->getBaseRegion();

if (const SymbolicRegion *SR = dyn_cast<SymbolicRegion>(MR))
return Reaper.isLive(SR->getSymbol());
return isLive(SR->getSymbol());

if (const VarRegion *VR = dyn_cast<VarRegion>(MR))
return Reaper.isLive(VR, true);
return isLive(VR, true);

// FIXME: This is a gross over-approximation. What we really need is a way to
// tell if anything still refers to this region. Unlike SymbolicRegions,
Expand Down Expand Up @@ -304,7 +311,7 @@ bool SymbolReaper::isLive(SymbolRef sym) {
}

if (const SymbolExtent *extent = dyn_cast<SymbolExtent>(sym)) {
if (IsLiveRegion(*this, extent->getRegion())) {
if (isLiveRegion(extent->getRegion())) {
markLive(sym);
return true;
}
Expand All @@ -313,7 +320,7 @@ bool SymbolReaper::isLive(SymbolRef sym) {

if (const SymbolMetadata *metadata = dyn_cast<SymbolMetadata>(sym)) {
if (MetadataInUse.count(sym)) {
if (IsLiveRegion(*this, metadata->getRegion())) {
if (isLiveRegion(metadata->getRegion())) {
markLive(sym);
MetadataInUse.erase(sym);
return true;
Expand Down

0 comments on commit bea2753

Please sign in to comment.