[PATCH] D49568: [analyzer][WIP] Scan the program state map in the visitor only once in DanglingInternalBufferChecker

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs created this revision.
rnkovacs added reviewers: NoQ, xazax.hun, george.karpenkov.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, szepet, 
baloghadamsoftware, whisperity.

In order to avoid scanning the map at each node in the bug reporter visitor, 
the `MemRegion` representing the container object associated with the tracked 
pointer symbol is looked up once in the visitor constructor. However, at the 
point where the visitor is constructed, we no longer find the appropriate 
`MemRegion` entry in the map, as it has already been cleaned up.
In this patch, I removed some clean-ups to allow for the look-up, but I suspect 
this is not the right approach.


Repository:
  rC Clang

https://reviews.llvm.org/D49568

Files:
  lib/StaticAnalyzer/Checkers/AllocationState.h
  lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp


Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1994,9 +1994,10 @@
     R->addRange(Range);
     R->addVisitor(llvm::make_unique<MallocBugVisitor>(Sym));
 
-    const RefState *RS = C.getState()->get<RegionState>(Sym);
+    ProgramStateRef State = C.getState();
+    const RefState *RS = State->get<RegionState>(Sym);
     if (RS->getAllocationFamily() == AF_InternalBuffer)
-      R->addVisitor(allocation_state::getDanglingBufferBRVisitor(Sym));
+      R->addVisitor(allocation_state::getDanglingBufferBRVisitor(Sym, State));
 
     C.emitReport(std::move(R));
   }
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -54,9 +54,17 @@
 public:
   class DanglingBufferBRVisitor : public BugReporterVisitor {
     SymbolRef PtrToBuf;
+    const MemRegion *ObjRegion;
 
   public:
-    DanglingBufferBRVisitor(SymbolRef Sym) : PtrToBuf(Sym) {}
+    DanglingBufferBRVisitor(SymbolRef Sym, ProgramStateRef State)
+        : PtrToBuf(Sym), ObjRegion(nullptr) {
+      RawPtrMapTy Map = State->get<RawPtrMap>();
+      for (auto const Entry : Map) {
+        if (Entry.second.contains(Sym))
+          ObjRegion = Entry.first;
+      }
+    }
 
     static void *getTag() {
       static int Tag = 0;
@@ -72,15 +80,11 @@
                                                    BugReporterContext &BRC,
                                                    BugReport &BR) override;
 
-    // FIXME: Scan the map once in the visitor's constructor and do a direct
-    // lookup by region.
-    bool isSymbolTracked(ProgramStateRef State, SymbolRef Sym) {
-      RawPtrMapTy Map = State->get<RawPtrMap>();
-      for (const auto Entry : Map) {
-        if (Entry.second.contains(Sym))
-          return true;
-      }
-      return false;
+    bool isSymbolTracked(ProgramStateRef State) {
+      const PtrSet *SymbolSet = State->get<RawPtrMap>(ObjRegion);
+      if (!SymbolSet)
+        return false;
+      return SymbolSet->contains(PtrToBuf);
     }
   };
 
@@ -180,7 +184,6 @@
         // `RefState` in MallocChecker's `RegionState` program state map.
         State = allocation_state::markReleased(State, Symbol, Origin);
       }
-      State = State->remove<RawPtrMap>(ObjRegion);
       C.addTransition(State);
       return;
     }
@@ -193,11 +196,6 @@
   PtrSet::Factory &F = State->getStateManager().get_context<PtrSet>();
   RawPtrMapTy RPM = State->get<RawPtrMap>();
   for (const auto Entry : RPM) {
-    if (!SymReaper.isLiveRegion(Entry.first)) {
-      // 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) {
@@ -217,8 +215,7 @@
     const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC,
     BugReport &BR) {
 
-  if (!isSymbolTracked(N->getState(), PtrToBuf) ||
-      isSymbolTracked(PrevN->getState(), PtrToBuf))
+  if (!isSymbolTracked(N->getState()) || isSymbolTracked(PrevN->getState()))
     return nullptr;
 
   const Stmt *S = PathDiagnosticLocation::getStmt(N);
@@ -238,9 +235,10 @@
 namespace ento {
 namespace allocation_state {
 
-std::unique_ptr<BugReporterVisitor> getDanglingBufferBRVisitor(SymbolRef Sym) {
+std::unique_ptr<BugReporterVisitor>
+getDanglingBufferBRVisitor(SymbolRef Sym, ProgramStateRef State) {
   return llvm::make_unique<
-      DanglingInternalBufferChecker::DanglingBufferBRVisitor>(Sym);
+      DanglingInternalBufferChecker::DanglingBufferBRVisitor>(Sym, State);
 }
 
 } // end namespace allocation_state
Index: lib/StaticAnalyzer/Checkers/AllocationState.h
===================================================================
--- lib/StaticAnalyzer/Checkers/AllocationState.h
+++ lib/StaticAnalyzer/Checkers/AllocationState.h
@@ -24,7 +24,8 @@
 /// This function provides an additional visitor that augments the bug report
 /// with information relevant to memory errors caused by the misuse of
 /// AF_InternalBuffer symbols.
-std::unique_ptr<BugReporterVisitor> getDanglingBufferBRVisitor(SymbolRef Sym);
+std::unique_ptr<BugReporterVisitor>
+getDanglingBufferBRVisitor(SymbolRef Sym, ProgramStateRef State);
 
 } // end namespace allocation_state
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits