Skip to content

Commit

Permalink
[analyzer] Fix immutable map factory lifetime for partial taint.
Browse files Browse the repository at this point in the history
This should fix the leaks found by asan buildbot in r304162.

Also don't store a reference to the factory with every map value,
which is the only difference between ImmutableMap and ImmutableMapRef.

llvm-svn: 304170
  • Loading branch information
haoNoQ committed May 29, 2017
1 parent 41e01b3 commit 4917f89
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ typedef std::unique_ptr<ConstraintManager>(*ConstraintManagerCreator)(
typedef std::unique_ptr<StoreManager>(*StoreManagerCreator)(
ProgramStateManager &);
typedef llvm::ImmutableMap<const SubRegion*, TaintTagType> TaintedSubRegions;
typedef llvm::ImmutableMapRef<const SubRegion*, TaintTagType>
TaintedSubRegionsRef;

//===----------------------------------------------------------------------===//
// ProgramStateTrait - Traits used by the Generic Data Map of a ProgramState.
Expand Down Expand Up @@ -90,7 +88,6 @@ class ProgramState : public llvm::FoldingSetNode {
Store store; // Maps a location to its current value.
GenericDataMap GDM; // Custom data stored by a client of this class.
unsigned refCount;
TaintedSubRegions::Factory TSRFactory;

/// makeWithStore - Return a ProgramState with the same values as the current
/// state with the exception of using the specified Store.
Expand Down Expand Up @@ -468,6 +465,7 @@ class ProgramStateManager {
std::unique_ptr<ConstraintManager> ConstraintMgr;

ProgramState::GenericDataMap::Factory GDMFactory;
TaintedSubRegions::Factory TSRFactory;

typedef llvm::DenseMap<void*,std::pair<void*,void (*)(void*)> > GDMContextsTy;
GDMContextsTy GDMContexts;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ template<> struct ProgramStateTrait<TaintMap>
/// underlying regions. This is used to efficiently check whether a symbol is
/// tainted when it represents a sub-region of a tainted symbol.
struct DerivedSymTaint {};
typedef llvm::ImmutableMap<SymbolRef, TaintedSubRegionsRef> DerivedSymTaintImpl;
typedef llvm::ImmutableMap<SymbolRef, TaintedSubRegions> DerivedSymTaintImpl;
template<> struct ProgramStateTrait<DerivedSymTaint>
: public ProgramStatePartialTrait<DerivedSymTaintImpl> {
static void *GDMIndex() { static int index; return &index; }
Expand Down
23 changes: 10 additions & 13 deletions clang/lib/StaticAnalyzer/Core/ProgramState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -703,13 +703,12 @@ ProgramStateRef ProgramState::addPartialTaint(SymbolRef ParentSym,
if (SubRegion == SubRegion->getBaseRegion())
return addTaint(ParentSym, Kind);

TaintedSubRegionsRef TaintedSubRegions(0, TSRFactory.getTreeFactory());
if (const TaintedSubRegionsRef *SavedTaintedRegions =
get<DerivedSymTaint>(ParentSym))
TaintedSubRegions = *SavedTaintedRegions;
const TaintedSubRegions *SavedRegs = get<DerivedSymTaint>(ParentSym);
TaintedSubRegions Regs =
SavedRegs ? *SavedRegs : stateMgr->TSRFactory.getEmptyMap();

TaintedSubRegions = TaintedSubRegions.add(SubRegion, Kind);
ProgramStateRef NewState = set<DerivedSymTaint>(ParentSym, TaintedSubRegions);
Regs = stateMgr->TSRFactory.add(Regs, SubRegion, Kind);
ProgramStateRef NewState = set<DerivedSymTaint>(ParentSym, Regs);
assert(NewState);
return NewState;
}
Expand Down Expand Up @@ -772,18 +771,16 @@ bool ProgramState::isTainted(SymbolRef Sym, TaintTagType Kind) const {
// If this is a SymbolDerived with the same parent symbol as another
// tainted SymbolDerived and a region that's a sub-region of that tainted
// symbol, it's also tainted.
if (const TaintedSubRegionsRef *SymRegions =
get<DerivedSymTaint>(SD->getParentSymbol())) {
if (const TaintedSubRegions *Regs =
get<DerivedSymTaint>(SD->getParentSymbol())) {
const TypedValueRegion *R = SD->getRegion();
for (TaintedSubRegionsRef::iterator I = SymRegions->begin(),
E = SymRegions->end();
I != E; ++I) {
for (auto I : *Regs) {
// FIXME: The logic to identify tainted regions could be more
// complete. For example, this would not currently identify
// overlapping fields in a union as tainted. To identify this we can
// check for overlapping/nested byte offsets.
if (Kind == I->second &&
(R == I->first || R->isSubRegionOf(I->first)))
if (Kind == I.second &&
(R == I.first || R->isSubRegionOf(I.first)))
return true;
}
}
Expand Down

0 comments on commit 4917f89

Please sign in to comment.