I'm curious what the motivation was for choosing to store the symbols as Base -> [list of dependents] instead of Dependent -> Base. I doubt many symbols are going to have multiple dependents, especially when they're being ORed together instead of ANDed. I realize that it makes it easy to clear the map when the base symbol dies, but since every dead symbol sweep hits every symbol anyway that doesn't seem to be a problem. (SymbolDerived works the other way around.)
Also, separate from implementation, would it be worth it to use this to /implement/ SymbolDerived? They're basically SymbolRegionValues with dependencies. On Aug 11, 2011, at 9:43, Anna Zaks wrote: > Author: zaks > Date: Thu Aug 11 11:43:28 2011 > New Revision: 137309 > > URL: http://llvm.org/viewvc/llvm-project?rev=137309&view=rev > Log: > Analyzer Core: Adding support for user-defined symbol dependencies. (For > example, the allocated resource symbol only needs to be freed if no error has > been returned by the allocator, so a checker might want to make the lifespan > of the error code symbol depend on the allocated resource symbol.) Note, by > default, the map that holds the dependencies will get destroyed along with > the SymbolManager at the end of function exploration. > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h > cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h?rev=137309&r1=137308&r2=137309&view=diff > ============================================================================== > --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h > (original) > +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h > Thu Aug 11 11:43:28 2011 > @@ -91,6 +91,7 @@ > }; > > typedef const SymbolData* SymbolRef; > +typedef llvm::SmallVector<SymbolRef, 1> SymbolRefSmallVectorTy; > > /// A symbol representing the value of a MemRegion. > class SymbolRegionValue : public SymbolData { > @@ -357,8 +358,12 @@ > > class SymbolManager { > typedef llvm::FoldingSet<SymExpr> DataSetTy; > + typedef llvm::DenseMap<SymbolRef, SymbolRefSmallVectorTy> SymbolDependTy; > > DataSetTy DataSet; > + /// Stores the extra dependencies between symbols: the data should be kept > + /// alive as long as the key is live. > + SymbolDependTy SymbolDependencies; > unsigned SymbolCounter; > llvm::BumpPtrAllocator& BPAlloc; > BasicValueFactory &BV; > @@ -413,6 +418,16 @@ > return SE->getType(Ctx); > } > > + /// \brief Add artificial symbol dependency. > + /// > + /// The dependent symbol should stay alive as long as the primary is alive. > + void addSymbolDependency(const SymbolRef Primary, const SymbolRef > Dependent); > + > + /// \brief Drop all user-added dependencies on the primary symbol. > + void removeSymbolDependencies(const SymbolRef Primary); > + > + const SymbolRefSmallVectorTy *getDependentSymbols(const SymbolRef Primary); > + > ASTContext &getContext() { return Ctx; } > BasicValueFactory &getBasicVals() { return BV; } > }; > @@ -495,6 +510,10 @@ > /// \brief Set to the value of the symbolic store after > /// StoreManager::removeDeadBindings has been called. > void setReapedStore(StoreRef st) { reapedStore = st; } > + > +private: > + /// Mark the symbols dependent on the input symbol as live. > + void markDependentsLive(SymbolRef sym); > }; > > class SymbolVisitor { > > Modified: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp?rev=137309&r1=137308&r2=137309&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp Thu Aug 11 11:43:28 > 2011 > @@ -248,9 +248,36 @@ > return false; > } > > +void SymbolManager::addSymbolDependency(const SymbolRef Primary, > + const SymbolRef Dependent) { > + SymbolDependencies[Primary].push_back(Dependent); > +} > + > +void SymbolManager::removeSymbolDependencies(const SymbolRef Primary) { > + SymbolDependencies.erase(Primary); > +} > + > +const SymbolRefSmallVectorTy *SymbolManager::getDependentSymbols( > + const SymbolRef > Primary) { > + SymbolDependTy::const_iterator I = SymbolDependencies.find(Primary); > + if (I == SymbolDependencies.end()) > + return 0; > + return &I->second; > +} > + > +void SymbolReaper::markDependentsLive(SymbolRef sym) { > + if (const SymbolRefSmallVectorTy *Deps = SymMgr.getDependentSymbols(sym)) { > + for (SymbolRefSmallVectorTy::const_iterator I = Deps->begin(), > + E = Deps->end(); I != E; > ++I) { > + markLive(*I); > + } > + } > +} > + > void SymbolReaper::markLive(SymbolRef sym) { > TheLiving.insert(sym); > TheDead.erase(sym); > + markDependentsLive(sym); > } > > void SymbolReaper::markLive(const MemRegion *region) { > @@ -299,8 +326,10 @@ > } > > bool SymbolReaper::isLive(SymbolRef sym) { > - if (TheLiving.count(sym)) > + if (TheLiving.count(sym)) { > + markDependentsLive(sym); > return true; > + } > > if (const SymbolDerived *derived = dyn_cast<SymbolDerived>(sym)) { > if (isLive(derived->getParentSymbol())) { > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
