diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index eb20b7d6cc98..eeb606ebfde5 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -530,7 +530,7 @@ public: void VisitCXXDestructor(QualType ObjectType, const MemRegion *Dest, const Stmt *S, bool IsBaseDtor, ExplodedNode *Pred, ExplodedNodeSet &Dst, - const EvalCallOptions &Options); + EvalCallOptions &Options); void VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, ExplodedNode *Pred, diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index 1fef5b3c1edd..c5a1f9b857e0 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -977,8 +977,8 @@ void ExprEngine::ProcessAutomaticObjDtor(const CFGAutomaticObjDtor Dtor, Region = makeZeroElementRegion(state, loc::MemRegionVal(Region), varType, CallOpts.IsArrayCtorOrDtor).getAsRegion(); - VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(), /*IsBase=*/ false, - Pred, Dst, CallOpts); + VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(), + /*IsBase=*/false, Pred, Dst, CallOpts); } void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor, @@ -1036,8 +1036,9 @@ void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D, SVal BaseVal = getStoreManager().evalDerivedToBase(ThisVal, BaseTy, Base->isVirtual()); - VisitCXXDestructor(BaseTy, BaseVal.castAs().getRegion(), - CurDtor->getBody(), /*IsBase=*/ true, Pred, Dst, {}); + EvalCallOptions CallOpts; + VisitCXXDestructor(BaseTy, BaseVal.getAsRegion(), CurDtor->getBody(), + /*IsBase=*/true, Pred, Dst, CallOpts); } void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D, @@ -1048,10 +1049,10 @@ void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D, const LocationContext *LCtx = Pred->getLocationContext(); const auto *CurDtor = cast(LCtx->getDecl()); - Loc ThisVal = getSValBuilder().getCXXThis(CurDtor, - LCtx->getStackFrame()); - SVal FieldVal = - State->getLValue(Member, State->getSVal(ThisVal).castAs()); + Loc ThisStorageLoc = + getSValBuilder().getCXXThis(CurDtor, LCtx->getStackFrame()); + Loc ThisLoc = State->getSVal(ThisStorageLoc).castAs(); + SVal FieldVal = State->getLValue(Member, ThisLoc); // FIXME: We need to run the same destructor on every element of the array. // This workaround will just run the first destructor (which will still @@ -1060,8 +1061,8 @@ void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D, FieldVal = makeZeroElementRegion(State, FieldVal, T, CallOpts.IsArrayCtorOrDtor); - VisitCXXDestructor(T, FieldVal.castAs().getRegion(), - CurDtor->getBody(), /*IsBase=*/false, Pred, Dst, CallOpts); + VisitCXXDestructor(T, FieldVal.getAsRegion(), CurDtor->getBody(), + /*IsBase=*/false, Pred, Dst, CallOpts); } void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D, @@ -1109,8 +1110,6 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D, EvalCallOptions CallOpts; CallOpts.IsTemporaryCtorOrDtor = true; if (!MR) { - CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; - // If we have no MR, we still need to unwrap the array to avoid destroying // the whole array at once. Regardless, we'd eventually need to model array // destructors properly, element-by-element. diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 10c8cb1a9ee1..91afde603ca9 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -604,7 +604,7 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType, bool IsBaseDtor, ExplodedNode *Pred, ExplodedNodeSet &Dst, - const EvalCallOptions &CallOpts) { + EvalCallOptions &CallOpts) { assert(S && "A destructor without a trigger!"); const LocationContext *LCtx = Pred->getLocationContext(); ProgramStateRef State = Pred->getState(); @@ -612,7 +612,6 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType, const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl(); assert(RecordDecl && "Only CXXRecordDecls should have destructors"); const CXXDestructorDecl *DtorDecl = RecordDecl->getDestructor(); - // FIXME: There should always be a Decl, otherwise the destructor call // shouldn't have been added to the CFG in the first place. if (!DtorDecl) { @@ -626,9 +625,27 @@ void ExprEngine::VisitCXXDestructor(QualType ObjectType, return; } + if (!Dest) { + // We're trying to destroy something that is not a region. This may happen + // for a variety of reasons (unknown target region, concrete integer instead + // of target region, etc.). The current code makes an attempt to recover. + // FIXME: We probably don't really need to recover when we're dealing + // with concrete integers specifically. + CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; + if (const Expr *E = dyn_cast_or_null(S)) { + Dest = MRMgr.getCXXTempObjectRegion(E, Pred->getLocationContext()); + } else { + static SimpleProgramPointTag T("ExprEngine", "SkipInvalidDestructor"); + NodeBuilder Bldr(Pred, Dst, *currBldrCtx); + Bldr.generateSink(Pred->getLocation().withTag(&T), + Pred->getState(), Pred); + return; + } + } + CallEventManager &CEMgr = getStateManager().getCallEventManager(); CallEventRef Call = - CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx); + CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx); PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(), Call->getSourceRange().getBegin(), diff --git a/clang/test/Analysis/dtor.cpp b/clang/test/Analysis/dtor.cpp index d843f03aada7..1c6251781545 100644 --- a/clang/test/Analysis/dtor.cpp +++ b/clang/test/Analysis/dtor.cpp @@ -540,3 +540,33 @@ void f() { clang_analyzer_eval(__alignof(NonTrivial) > 0); // expected-warning{{TRUE}} } } + +namespace dtor_over_loc_concrete_int { +struct A { + ~A() {} +}; + +struct B { + A a; + ~B() {} +}; + +struct C : A { + ~C() {} +}; + +void testB() { + B *b = (B *)-1; + b->~B(); // no-crash +} + +void testC() { + C *c = (C *)-1; + c->~C(); // no-crash +} + +void testAutoDtor() { + const A &a = *(A *)-1; + // no-crash +} +} // namespace dtor_over_loc_concrete_int