[analyzer] Track multiple raw pointer symbols in DanglingInternalBufferChecker.

Previously, the checker only tracked one raw pointer symbol for each
container object. But member functions returning a pointer to the
object's inner buffer may be called on the object several times. These
pointer symbols are now collected in a set inside the program state map
and thus all of them is checked for use-after-free problems.

Differential Revision: https://reviews.llvm.org/D49057

llvm-svn: 336835

1 files changed, 50 insertions, 18 deletions

diff --git a/clang/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
index fe575cc5fca..2df86fdd3a6 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -24,10 +24,23 @@
 using namespace clang;
 using namespace ento;
 
-// FIXME: member functions that return a pointer to the container's internal
-// buffer may be called on the object many times, so the object's memory
-// region should have a list of pointer symbols associated with it.
-REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, SymbolRef)
+using PtrSet = llvm::ImmutableSet<SymbolRef>;
+
+// Associate container objects with a set of raw pointer symbols.
+REGISTER_MAP_WITH_PROGRAMSTATE(RawPtrMap, const MemRegion *, PtrSet)
+
+// This is a trick to gain access to PtrSet's Factory.
+namespace clang {
+namespace ento {
+template<> struct ProgramStateTrait<PtrSet>
+  : public ProgramStatePartialTrait<PtrSet> {
+  static void *GDMIndex() {
+    static int Index = 0;
+    return &Index;
+  }
+};
+} // end namespace ento
+} // end namespace clang
 
 namespace {
 
@@ -61,7 +74,7 @@ public:
     bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) {
       RawPtrMapTy Map = State->get<RawPtrMap>();
       for (const auto Entry : Map) {
-        if (Entry.second == Sym)
+        if (Entry.second.contains(Sym))
           return true;
       }
       return false;
@@ -88,11 +101,11 @@ void DanglingInternalBufferChecker::checkPostCall(const CallEvent &Call,
     return;
 
   SVal Obj = ICall->getCXXThisVal();
-  const auto *TypedR = dyn_cast_or_null<TypedValueRegion>(Obj.getAsRegion());
-  if (!TypedR)
+  const auto *ObjRegion = dyn_cast_or_null<TypedValueRegion>(Obj.getAsRegion());
+  if (!ObjRegion)
     return;
 
-  auto *TypeDecl = TypedR->getValueType()->getAsCXXRecordDecl();
+  auto *TypeDecl = ObjRegion->getValueType()->getAsCXXRecordDecl();
   if (TypeDecl->getName() != "basic_string")
     return;
 
@@ -100,20 +113,30 @@ void DanglingInternalBufferChecker::checkPostCall(const CallEvent &Call,
 
   if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
     SVal RawPtr = Call.getReturnValue();
-    if (!RawPtr.isUnknown()) {
-      State = State->set<RawPtrMap>(TypedR, RawPtr.getAsSymbol());
+    if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
+      // Start tracking this raw pointer by adding it to the set of symbols
+      // associated with this container object in the program state map.
+      PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
+      const PtrSet *SetPtr = State->get<RawPtrMap>(ObjRegion);
+      PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet();
+      assert(C.wasInlined || !Set.contains(Sym));
+      Set = F.add(Set, Sym);
+      State = State->set<RawPtrMap>(ObjRegion, Set);
       C.addTransition(State);
     }
     return;
   }
 
   if (isa<CXXDestructorCall>(ICall)) {
-    if (State->contains<RawPtrMap>(TypedR)) {
-      const SymbolRef *StrBufferPtr = State->get<RawPtrMap>(TypedR);
-      // FIXME: What if Origin is null?
+    if (const PtrSet *PS = State->get<RawPtrMap>(ObjRegion)) {
+      // Mark all pointer symbols associated with the deleted object released.
       const Expr *Origin = Call.getOriginExpr();
-      State = allocation_state::markReleased(State, *StrBufferPtr, Origin);
-      State = State->remove<RawPtrMap>(TypedR);
+      for (const auto Symbol : *PS) {
+        // NOTE: `Origin` may be null, and will be stored so in the symbol's
+        // `RefState` in MallocChecker's `RegionState` program state map.
+        State = allocation_state::markReleased(State, Symbol, Origin);
+      }
+      State = State->remove<RawPtrMap>(ObjRegion);
       C.addTransition(State);
       return;
     }
@@ -123,15 +146,24 @@ void DanglingInternalBufferChecker::checkPostCall(const CallEvent &Call,
 void DanglingInternalBufferChecker::checkDeadSymbols(SymbolReaper &SymReaper,
                                                      CheckerContext &C) const {
   ProgramStateRef State = C.getState();
+  PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
   RawPtrMapTy RPM = State->get<RawPtrMap>();
   for (const auto Entry : RPM) {
-    if (!SymReaper.isLive(Entry.second))
-      State = State->remove<RawPtrMap>(Entry.first);
     if (!SymReaper.isLiveRegion(Entry.first)) {
-      // Due to incomplete destructor support, some dead regions might still
+      // Due to incomplete destructor support, some dead regions might
       // remain in the program state map. Clean them up.
       State = State->remove<RawPtrMap>(Entry.first);
     }
+    if (const PtrSet *OldSet = State->get<RawPtrMap>(Entry.first)) {
+      PtrSet CleanedUpSet = *OldSet;
+      for (const auto Symbol : Entry.second) {
+        if (!SymReaper.isLive(Symbol))
+          CleanedUpSet = F.remove(CleanedUpSet, Symbol);
+      }
+      State = CleanedUpSet.isEmpty()
+              ? State->remove<RawPtrMap>(Entry.first)
+              : State->set<RawPtrMap>(Entry.first, CleanedUpSet);
+    }
   }
   C.addTransition(State);
 }