RedDocMD updated this revision to Diff 366558.
RedDocMD added a comment.

Connecting to MallocChecker


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105821/new/

https://reviews.llvm.org/D105821

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -753,10 +753,13 @@
                                             *Call, *this);
 
   ExplodedNodeSet DstInvalidated;
-  StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
-  for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end();
-       I != E; ++I)
-    defaultEvalCall(Bldr, *I, *Call, CallOpts);
+  // StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx);
+  // for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E =
+  // DstPreCall.end();
+  //      I != E; ++I)
+  //   defaultEvalCall(Bldr, *I, *Call, CallOpts);
+  getCheckerManager().runCheckersForEvalCall(DstInvalidated, DstPreCall, *Call,
+                                             *this, CallOpts);
 
   getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated,
                                              *Call, *this);
Index: clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
+++ clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -664,14 +664,11 @@
     for (const auto &EvalCallChecker : EvalCallCheckers) {
       // TODO: Support the situation when the call doesn't correspond
       // to any Expr.
-      ProgramPoint L = ProgramPoint::getProgramPoint(
-          Call.getOriginExpr(), ProgramPoint::PostStmtKind,
-          Pred->getLocationContext(), EvalCallChecker.Checker);
       bool evaluated = false;
       { // CheckerContext generates transitions(populates checkDest) on
         // destruction, so introduce the scope to make sure it gets properly
         // populated.
-        CheckerContext C(B, Eng, Pred, L);
+        CheckerContext C(B, Eng, Pred, Call.getProgramPoint());
         evaluated = EvalCallChecker(Call, C);
       }
       assert(!(evaluated && anyEvaluated)
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -11,6 +11,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "AllocationState.h"
 #include "Move.h"
 #include "SmartPtr.h"
 
@@ -46,6 +47,8 @@
   bool isBoolConversionMethod(const CallEvent &Call) const;
 
 public:
+  SmartPtrModeling(CheckerManager &ChkMgr) : ChkMgr(ChkMgr) {}
+
   // Whether the checker should model for null dereferences of smart pointers.
   DefaultBool ModelSmartPtrDereference;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
@@ -92,6 +95,7 @@
   const CallDescription StdMakeUniqueCall{{"std", "make_unique"}};
   const CallDescription StdMakeUniqueForOverwriteCall{
       {"std", "make_unique_for_overwrite"}};
+  CheckerManager &ChkMgr;
 };
 } // end of anonymous namespace
 
@@ -139,7 +143,7 @@
 
   if (RD->getDeclName().isIdentifier()) {
     StringRef Name = RD->getName();
-    return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr";
+    return llvm::is_contained(STD_PTR_NAMES, Name);
   }
   return false;
 }
@@ -157,20 +161,6 @@
 } // namespace ento
 } // namespace clang
 
-// If a region is removed all of the subregions need to be removed too.
-static TrackedRegionMapTy
-removeTrackedSubregions(TrackedRegionMapTy RegionMap,
-                        TrackedRegionMapTy::Factory &RegionMapFactory,
-                        const MemRegion *Region) {
-  if (!Region)
-    return RegionMap;
-  for (const auto &E : RegionMap) {
-    if (E.first->isSubRegionOf(Region))
-      RegionMap = RegionMapFactory.remove(RegionMap, E.first);
-  }
-  return RegionMap;
-}
-
 static ProgramStateRef updateSwappedRegion(ProgramStateRef State,
                                            const MemRegion *Region,
                                            const SVal *RegionInnerPointerVal) {
@@ -275,6 +265,32 @@
          smartptr::isStdSmartPtr(Call.getArgExpr(1));
 }
 
+ProgramStateRef
+invalidateInnerPointer(const MemRegion *ThisRegion, ProgramStateRef State,
+                       const CallEvent &Call, CheckerContext &C) {
+  const auto *InnerPtrVal = State->get<TrackedRegionMap>(ThisRegion);
+  if (InnerPtrVal) {
+    const auto *Sym = InnerPtrVal->getAsSymbol();
+    if (Sym)
+      State = allocation_state::markReleased(State, Sym, Call.getOriginExpr());
+    State = State->invalidateRegions(*InnerPtrVal, nullptr, C.blockCount(),
+                                     C.getLocationContext(), true);
+
+    const QualType &Type = getInnerPointerType(Call, C);
+    const auto *RD = Type->getAsCXXRecordDecl();
+    if (!RD)
+      return State;
+    const auto *DD = RD->getDestructor();
+
+    const auto InnerDestrCall =
+        C.getStateManager().getCallEventManager().getCXXDestructorCall(
+            DD, nullptr, InnerPtrVal->getAsRegion(), RD->bases().empty(), State,
+            C.getLocationContext());
+    State = InnerDestrCall->invalidateRegions(C.blockCount(), State);
+  }
+  return State;
+}
+
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
                                 CheckerContext &C) const {
 
@@ -339,6 +355,7 @@
     // We don't leave a note here since it is guaranteed the
     // unique_ptr from this call is non-null (hence is safe to de-reference).
     C.addTransition(State);
+    // FIXME: Invalidate globals on object construction
     return true;
   }
 
@@ -372,6 +389,21 @@
     }
   }
 
+  if (const auto *DC = dyn_cast<CXXDestructorCall>(&Call)) {
+    const MemRegion *ThisRegion = DC->getCXXThisVal().getAsRegion();
+    if (!ThisRegion)
+      return false;
+    State = invalidateInnerPointer(ThisRegion, State, Call, C);
+    State = State->remove<TrackedRegionMap>(ThisRegion);
+    // This tag is required to prevent later crashes due to the non-addition
+    // of new States. Having a tag ensures that the call to addTransition
+    // actually adds a new state.
+    static SimpleProgramPointTag SPPT("SmartPtrModeling",
+                                      "on destructor modeling");
+    C.addTransition(State, &SPPT);
+    return true;
+  }
+
   if (!ModelSmartPtrDereference)
     return false;
 
@@ -402,10 +434,14 @@
           }));
     } else {
       const auto *TrackingExpr = Call.getArgExpr(0);
-      assert(TrackingExpr->getType()->isPointerType() &&
-             "Adding a non pointer value to TrackedRegionMap");
+      if (!TrackingExpr->getType()->isPointerType())
+        return false;
       auto ArgVal = Call.getArgSVal(0);
       State = State->set<TrackedRegionMap>(ThisRegion, ArgVal);
+      // Escape the pointer passed here
+      State = C.getStateManager().getOwningEngine().processPointerEscapedOnBind(
+          State, {std::make_pair(CC->getCXXThisVal(), ArgVal)},
+          C.getLocationContext(), PSK_DirectEscapeOnCall, &Call);
 
       C.addTransition(State, C.getNoteTag([ThisRegion, TrackingExpr,
                                            ArgVal](PathSensitiveBugReport &BR,
@@ -561,10 +597,8 @@
     Out << Sep << "Smart ptr regions :" << NL;
     for (auto I : RS) {
       I.first->dumpToStream(Out);
-      if (smartptr::isNullSmartPtr(State, I.first))
-        Out << ": Null";
-      else
-        Out << ": Non Null";
+      Out << ": ";
+      I.second.dumpToStream(Out);
       Out << NL;
     }
   }
@@ -575,12 +609,43 @@
     ArrayRef<const MemRegion *> ExplicitRegions,
     ArrayRef<const MemRegion *> Regions, const LocationContext *LCtx,
     const CallEvent *Call) const {
-  TrackedRegionMapTy RegionMap = State->get<TrackedRegionMap>();
-  TrackedRegionMapTy::Factory &RegionMapFactory =
-      State->get_context<TrackedRegionMap>();
-  for (const auto *Region : Regions)
-    RegionMap = removeTrackedSubregions(RegionMap, RegionMapFactory,
-                                        Region->getBaseRegion());
+
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+    InvalidatedSymbols &Symbols;
+
+  public:
+    explicit CollectReachableSymbolsCallback(InvalidatedSymbols &Symbols)
+        : Symbols(Symbols){}
+
+    const InvalidatedSymbols &getSymbols() const { return Symbols; }
+
+    bool VisitSymbol(SymbolRef Sym) override {
+      Symbols.insert(Sym);
+      return true;
+    }
+  };
+
+  InvalidatedSymbols Symbols;
+  CollectReachableSymbolsCallback CallBack(Symbols);
+  auto RegionMap = State->get<TrackedRegionMap>();
+  auto &RegionMapFactory = State->get_context<TrackedRegionMap>();
+
+  for (const auto *Region : Regions) {
+    for (const auto &E : RegionMap) {
+      if (E.first->isSubRegionOf(Region)) {
+        State->scanReachableSymbols(E.second, CallBack);
+        RegionMap = RegionMapFactory.remove(RegionMap, E.first);
+        State->scanReachableSymbols(loc::MemRegionVal(E.first), CallBack);
+      }
+    }
+  }
+
+  const auto &EscapeeSymbols = CallBack.getSymbols();
+  if (!EscapeeSymbols.empty()) {
+    PointerEscapeKind Kind = Call ? PSK_IndirectEscapeOnCall : PSK_EscapeOther;
+    State = ChkMgr.runCheckersForPointerEscape(State, EscapeeSymbols, Call,
+                                               Kind, nullptr);
+  }
   return State->set<TrackedRegionMap>(RegionMap);
 }
 
@@ -609,7 +674,13 @@
 
   assert(Call.getArgExpr(0)->getType()->isPointerType() &&
          "Adding a non pointer value to TrackedRegionMap");
-  State = State->set<TrackedRegionMap>(ThisRegion, Call.getArgSVal(0));
+  State = invalidateInnerPointer(ThisRegion, State, Call, C);
+  const auto ArgVal = Call.getArgSVal(0);
+  State = State->set<TrackedRegionMap>(ThisRegion, ArgVal);
+  // Escape the pointer passed here
+  State = C.getStateManager().getOwningEngine().processPointerEscapedOnBind(
+      State, {std::make_pair(IC->getCXXThisVal(), ArgVal)},
+      C.getLocationContext(), PSK_DirectEscapeOnCall, &Call);
   const auto *TrackingExpr = Call.getArgExpr(0);
   C.addTransition(
       State, C.getNoteTag([ThisRegion, TrackingExpr](PathSensitiveBugReport &BR,
@@ -903,7 +974,7 @@
 }
 
 void ento::registerSmartPtrModeling(CheckerManager &Mgr) {
-  auto *Checker = Mgr.registerChecker<SmartPtrModeling>();
+  auto *Checker = Mgr.registerChecker<SmartPtrModeling>(Mgr);
   Checker->ModelSmartPtrDereference =
       Mgr.getAnalyzerOptions().getCheckerBooleanOption(
           Checker, "ModelSmartPtrDereference");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to