From: Artem Dergachev <artem.dergachev@gmail.com>
Date: Mon, 29 May 2017 18:54:02 +0000 (+0000)
Subject: [analyzer] Fix immutable map factory lifetime for partial taint.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7007fd8ccfb31283792a5ee1c94c590bf0792e96;p=clang

[analyzer] Fix immutable map factory lifetime for partial taint.

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.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@304170 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
index 6f9df0b09f..e3a2164b11 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -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.
@@ -90,7 +88,6 @@ private:
   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.
@@ -468,6 +465,7 @@ private:
   std::unique_ptr<ConstraintManager>   ConstraintMgr;
 
   ProgramState::GenericDataMap::Factory     GDMFactory;
+  TaintedSubRegions::Factory TSRFactory;
 
   typedef llvm::DenseMap<void*,std::pair<void*,void (*)(void*)> > GDMContextsTy;
   GDMContextsTy GDMContexts;
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h b/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
index 1797fc1c49..7b76263f04 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
@@ -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; }
diff --git a/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp
index fc26de1a1f..3215c3ccd2 100644
--- a/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -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;
 }
@@ -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;
         }
       }