diff --git a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp --- a/clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp +++ b/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(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(&Call)) { + const MemRegion *ThisRegion = DC->getCXXThisVal().getAsRegion(); + if (!ThisRegion) + return false; + State = invalidateInnerPointer(ThisRegion, State, Call, C); + State = State->remove(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(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 ExplicitRegions, ArrayRef Regions, const LocationContext *LCtx, const CallEvent *Call) const { - TrackedRegionMapTy RegionMap = State->get(); - TrackedRegionMapTy::Factory &RegionMapFactory = - State->get_context(); - 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(); + auto &RegionMapFactory = State->get_context(); + + 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(RegionMap); } @@ -609,7 +674,13 @@ assert(Call.getArgExpr(0)->getType()->isPointerType() && "Adding a non pointer value to TrackedRegionMap"); - State = State->set(ThisRegion, Call.getArgSVal(0)); + State = invalidateInnerPointer(ThisRegion, State, Call, C); + const auto ArgVal = Call.getArgSVal(0); + State = State->set(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(); + auto *Checker = Mgr.registerChecker(Mgr); Checker->ModelSmartPtrDereference = Mgr.getAnalyzerOptions().getCheckerBooleanOption( Checker, "ModelSmartPtrDereference"); diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp --- a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp +++ b/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) diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/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);