Author: dergachev Date: Tue Aug 20 14:41:17 2019 New Revision: 369450 URL: http://llvm.org/viewvc/llvm-project?rev=369450&view=rev Log: [analyzer] Fix a crash when destroying a non-region.
Add defensive check that prevents a crash when we try to evaluate a destructor whose this-value is a concrete integer that isn't a null. Differential Revision: https://reviews.llvm.org/D65349 Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp cfe/trunk/test/Analysis/dtor.cpp Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=369450&r1=369449&r2=369450&view=diff ============================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Tue Aug 20 14:41:17 2019 @@ -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, Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=369450&r1=369449&r2=369450&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Tue Aug 20 14:41:17 2019 @@ -977,8 +977,8 @@ void ExprEngine::ProcessAutomaticObjDtor 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 C SVal BaseVal = getStoreManager().evalDerivedToBase(ThisVal, BaseTy, Base->isVirtual()); - VisitCXXDestructor(BaseTy, BaseVal.castAs<loc::MemRegionVal>().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 const LocationContext *LCtx = Pred->getLocationContext(); const auto *CurDtor = cast<CXXDestructorDecl>(LCtx->getDecl()); - Loc ThisVal = getSValBuilder().getCXXThis(CurDtor, - LCtx->getStackFrame()); - SVal FieldVal = - State->getLValue(Member, State->getSVal(ThisVal).castAs<Loc>()); + Loc ThisStorageLoc = + getSValBuilder().getCXXThis(CurDtor, LCtx->getStackFrame()); + Loc ThisLoc = State->getSVal(ThisStorageLoc).castAs<Loc>(); + 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 FieldVal = makeZeroElementRegion(State, FieldVal, T, CallOpts.IsArrayCtorOrDtor); - VisitCXXDestructor(T, FieldVal.castAs<loc::MemRegionVal>().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(co 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. Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=369450&r1=369449&r2=369450&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Tue Aug 20 14:41:17 2019 @@ -604,7 +604,7 @@ void ExprEngine::VisitCXXDestructor(Qual 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(Qual 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(Qual 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<Expr>(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<CXXDestructorCall> Call = - CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx); + CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx); PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(), Call->getSourceRange().getBegin(), Modified: cfe/trunk/test/Analysis/dtor.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=369450&r1=369449&r2=369450&view=diff ============================================================================== --- cfe/trunk/test/Analysis/dtor.cpp (original) +++ cfe/trunk/test/Analysis/dtor.cpp Tue Aug 20 14:41:17 2019 @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
