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

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Mon May 29 13:54:02 2017
New Revision: 304170

URL: http://llvm.org/viewvc/llvm-project?rev=304170&view=rev
Log:
[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.

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
    cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=304170&r1=304169&r2=304170&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h Mon May 29 13:54:02 2017
@@ -44,8 +44,6 @@ typedef std::unique_ptr<ConstraintManage
 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;

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h?rev=304170&r1=304169&r2=304170&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h Mon May 29 13:54:02 2017
@@ -39,7 +39,7 @@ template<> struct ProgramStateTrait<Tain
 /// 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; }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp?rev=304170&r1=304169&r2=304170&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ProgramState.cpp Mon May 29 13:54:02 2017
@@ -703,13 +703,12 @@ ProgramStateRef ProgramState::addPartial
   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 S
       // 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;
         }
       }