Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -55,6 +55,20 @@ Inline_Minimal = 0x1 }; + /// Hints for figuring out of a call should be inlined during evalCall(). + struct EvalCallOptions { + /// This call is a constructor or a destructor for which we do not currently + /// compute the this-region correctly. + bool IsCtorOrDtorWithImproperlyModeledTargetRegion = false; + /// This call is a constructor or a destructor for a single element within + /// an array, a part of array construction or destruction. + bool IsArrayCtorOrDtor = false; + /// This call is a constructor or a destructor of a temporary value. + bool IsTemporaryCtorOrDtor = false; + + EvalCallOptions() {} + }; + private: AnalysisManager &AMgr; @@ -449,7 +463,8 @@ void VisitCXXDestructor(QualType ObjectType, const MemRegion *Dest, const Stmt *S, bool IsBaseDtor, - ExplodedNode *Pred, ExplodedNodeSet &Dst); + ExplodedNode *Pred, ExplodedNodeSet &Dst, + const EvalCallOptions &Options); void VisitCXXNewAllocatorCall(const CXXNewExpr *CNE, ExplodedNode *Pred, @@ -574,14 +589,6 @@ const LocationContext *LCtx, ProgramStateRef State); - struct EvalCallOptions { - bool IsConstructorWithImproperlyModeledTargetRegion = false; - bool IsArrayConstructorOrDestructor = false; - bool IsConstructorIntoTemporary = false; - - EvalCallOptions() {} - }; - /// Evaluate a call, running pre- and post-call checks and allowing checkers /// to be responsible for handling the evaluation of the call itself. void evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred, @@ -665,6 +672,17 @@ const Expr *InitWithAdjustments, const Expr *Result = nullptr); + /// Returns a region representing the first element of a (possibly + /// multi-dimensional) array, for the purposes of element construction or + /// destruction. + /// + /// On return, \p Ty will be set to the base type of the array. + /// + /// If the type is not an array type at all, the original value is returned. + /// Otherwise the "IsArray" flag is set. + static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue, + QualType &Ty, bool &IsArray); + /// For a DeclStmt or CXXInitCtorInitializer, walk backward in the current CFG /// block to find the constructor expression that directly constructed into /// the storage for this statement. Returns null if the constructor for this Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -802,8 +802,15 @@ varType = cast(Region)->getValueType(); } + // 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 + // invalidate the entire array). + EvalCallOptions CallOpts; + Region = makeZeroElementRegion(state, loc::MemRegionVal(Region), varType, + CallOpts.IsArrayCtorOrDtor).getAsRegion(); + VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(), /*IsBase=*/ false, - Pred, Dst); + Pred, Dst, CallOpts); } void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor, @@ -813,12 +820,12 @@ const LocationContext *LCtx = Pred->getLocationContext(); const CXXDeleteExpr *DE = Dtor.getDeleteExpr(); const Stmt *Arg = DE->getArgument(); + QualType DTy = DE->getDestroyedType(); SVal ArgVal = State->getSVal(Arg, LCtx); // If the argument to delete is known to be a null value, // don't run destructor. if (State->isNull(ArgVal).isConstrainedTrue()) { - QualType DTy = DE->getDestroyedType(); QualType BTy = getContext().getBaseElementType(DTy); const CXXRecordDecl *RD = BTy->getAsCXXRecordDecl(); const CXXDestructorDecl *Dtor = RD->getDestructor(); @@ -829,10 +836,19 @@ return; } - VisitCXXDestructor(DE->getDestroyedType(), - ArgVal.getAsRegion(), - DE, /*IsBase=*/ false, - Pred, Dst); + EvalCallOptions CallOpts; + const MemRegion *ArgR = ArgVal.getAsRegion(); + if (DE->isArrayForm()) { + // 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 + // invalidate the entire array). + CallOpts.IsArrayCtorOrDtor = true; + if (ArgR) + ArgR = getStoreManager().GetElementZeroRegion(cast(ArgR), DTy); + } + + VisitCXXDestructor(DE->getDestroyedType(), ArgR, DE, /*IsBase=*/false, + Pred, Dst, CallOpts); } void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D, @@ -851,12 +867,13 @@ Base->isVirtual()); VisitCXXDestructor(BaseTy, BaseVal.castAs().getRegion(), - CurDtor->getBody(), /*IsBase=*/ true, Pred, Dst); + CurDtor->getBody(), /*IsBase=*/ true, Pred, Dst, {}); } void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D, ExplodedNode *Pred, ExplodedNodeSet &Dst) { const FieldDecl *Member = D.getFieldDecl(); + QualType T = Member->getType(); ProgramStateRef State = Pred->getState(); const LocationContext *LCtx = Pred->getLocationContext(); @@ -866,9 +883,15 @@ SVal FieldVal = State->getLValue(Member, State->getSVal(ThisVal).castAs()); - VisitCXXDestructor(Member->getType(), - FieldVal.castAs().getRegion(), - CurDtor->getBody(), /*IsBase=*/false, Pred, Dst); + // 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 + // invalidate the entire array). + EvalCallOptions CallOpts; + FieldVal = makeZeroElementRegion(State, FieldVal, T, + CallOpts.IsArrayCtorOrDtor); + + VisitCXXDestructor(T, FieldVal.castAs().getRegion(), + CurDtor->getBody(), /*IsBase=*/false, Pred, Dst, CallOpts); } void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D, @@ -892,10 +915,14 @@ assert(CleanDtorState.size() <= 1); ExplodedNode *CleanPred = CleanDtorState.empty() ? Pred : *CleanDtorState.begin(); + // FIXME: Inlining of temporary destructors is not supported yet anyway, so // we just put a NULL region for now. This will need to be changed later. + EvalCallOptions CallOpts; + CallOpts.IsTemporaryCtorOrDtor = true; + CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; VisitCXXDestructor(varType, nullptr, D.getBindTemporaryExpr(), - /*IsBase=*/false, CleanPred, Dst); + /*IsBase=*/false, CleanPred, Dst, CallOpts); } void ExprEngine::processCleanupTemporaryBranch(const CXXBindTemporaryExpr *BTE, Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -84,16 +84,8 @@ } -/// Returns a region representing the first element of a (possibly -/// multi-dimensional) array, for the purposes of element construction or -/// destruction. -/// -/// On return, \p Ty will be set to the base type of the array. -/// -/// If the type is not an array type at all, the original value is returned. -/// Otherwise the "IsArray" flag is set. -static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue, - QualType &Ty, bool &IsArray) { +SVal ExprEngine::makeZeroElementRegion(ProgramStateRef State, SVal LValue, + QualType &Ty, bool &IsArray) { SValBuilder &SVB = State->getStateManager().getSValBuilder(); ASTContext &Ctx = SVB.getContext(); @@ -128,7 +120,7 @@ if (CNE->isArray()) { // TODO: In fact, we need to call the constructor for every // allocated element, not just the first one! - CallOpts.IsArrayConstructorOrDestructor = true; + CallOpts.IsArrayCtorOrDtor = true; return getStoreManager().GetElementZeroRegion( MR, CNE->getType()->getPointeeType()); } @@ -140,10 +132,10 @@ SVal LValue = State->getLValue(Var, LCtx); QualType Ty = Var->getType(); LValue = makeZeroElementRegion(State, LValue, Ty, - CallOpts.IsArrayConstructorOrDestructor); + CallOpts.IsArrayCtorOrDtor); return LValue.getAsRegion(); } else if (auto *RS = dyn_cast(TriggerStmt)) { - CallOpts.IsConstructorIntoTemporary = true; + CallOpts.IsTemporaryCtorOrDtor = true; return MRMgr.getCXXTempObjectRegion(CE, LCtx); } // TODO: Consider other directly initialized elements. @@ -166,7 +158,7 @@ QualType Ty = Field->getType(); FieldVal = makeZeroElementRegion(State, FieldVal, Ty, - CallOpts.IsArrayConstructorOrDestructor); + CallOpts.IsArrayCtorOrDtor); return FieldVal.getAsRegion(); } @@ -176,7 +168,7 @@ } // If we couldn't find an existing region to construct into, assume we're // constructing a temporary. Notify the caller of our failure. - CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true; + CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; return MRMgr.getCXXTempObjectRegion(CE, LCtx); } @@ -262,7 +254,7 @@ if (dyn_cast_or_null(LCtx->getParentMap().getParent(CE))) { MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); Target = MRMgr.getCXXTempObjectRegion(CE, LCtx); - CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true; + CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true; break; } // FALLTHROUGH @@ -332,7 +324,7 @@ if (CE->getConstructor()->isTrivial() && CE->getConstructor()->isCopyOrMoveConstructor() && - !CallOpts.IsArrayConstructorOrDestructor) { + !CallOpts.IsArrayCtorOrDtor) { // FIXME: Handle other kinds of trivial constructors as well. for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end(); I != E; ++I) @@ -387,23 +379,11 @@ const Stmt *S, bool IsBaseDtor, ExplodedNode *Pred, - ExplodedNodeSet &Dst) { + ExplodedNodeSet &Dst, + const EvalCallOptions &CallOpts) { const LocationContext *LCtx = Pred->getLocationContext(); ProgramStateRef State = Pred->getState(); - // 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 - // invalidate the entire array). - SVal DestVal = UnknownVal(); - if (Dest) - DestVal = loc::MemRegionVal(Dest); - - EvalCallOptions CallOpts; - DestVal = makeZeroElementRegion(State, DestVal, ObjectType, - CallOpts.IsArrayConstructorOrDestructor); - - Dest = DestVal.getAsRegion(); - const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl(); assert(RecordDecl && "Only CXXRecordDecls should have destructors"); const CXXDestructorDecl *DtorDecl = RecordDecl->getDestructor(); Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -650,7 +650,7 @@ // initializers for array fields in default move/copy constructors. // We still allow construction into ElementRegion targets when they don't // represent array elements. - if (CallOpts.IsArrayConstructorOrDestructor) + if (CallOpts.IsArrayCtorOrDtor) return CIP_DisallowedOnce; // Inlining constructors requires including initializers in the CFG. @@ -670,14 +670,14 @@ if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete) { // If we don't handle temporary destructors, we shouldn't inline // their constructors. - if (CallOpts.IsConstructorIntoTemporary && + if (CallOpts.IsTemporaryCtorOrDtor && !Opts.includeTemporaryDtorsInCFG()) return CIP_DisallowedOnce; - // If we did not construct the correct this-region, it would be pointless + // If we did not find the correct this-region, it would be pointless // to inline the constructor. Instead we will simply invalidate // the fake temporary target. - if (CallOpts.IsConstructorWithImproperlyModeledTargetRegion) + if (CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion) return CIP_DisallowedOnce; } @@ -693,9 +693,14 @@ (void)ADC; // FIXME: We don't handle constructors or destructors for arrays properly. - if (CallOpts.IsArrayConstructorOrDestructor) + if (CallOpts.IsArrayCtorOrDtor) return CIP_DisallowedOnce; + // If we did not find the correct this-region, it would be pointless + // to inline the destructor. Instead we will simply invalidate + // the fake temporary target. + if (CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion) + return CIP_DisallowedOnce; break; } case CE_CXXAllocator: @@ -844,14 +849,6 @@ AnalysisDeclContextManager &ADCMgr = AMgr.getAnalysisDeclContextManager(); AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D); - // Temporary object destructor processing is currently broken, so we never - // inline them. - // FIXME: Remove this once temp destructors are working. - if (isa(Call)) { - if ((*currBldrCtx->getBlock())[currStmtIdx].getAs()) - return false; - } - // The auto-synthesized bodies are essential to inline as they are // usually small and commonly used. Note: we should do this check early on to // ensure we always inline these calls. Index: test/Analysis/new.cpp =================================================================== --- test/Analysis/new.cpp +++ test/Analysis/new.cpp @@ -311,7 +311,8 @@ void testArrayDestr() { NoReturnDtor *p = new NoReturnDtor[2]; delete[] p; // Calls the base destructor which aborts, checked below - clang_analyzer_eval(true); // no-warning + //TODO: clang_analyzer_eval should not be called + clang_analyzer_eval(true); // expected-warning{{TRUE}} } // Invalidate Region even in case of default destructor