[PATCH] D42991: [analyzer] Use EvalCallOptions for destructors as well.
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL325209: [analyzer] Decide on inlining destructors via EvalCallOptions. (authored by dergachev, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D42991?vs=133299&id=134364#toc Repository: rL LLVM https://reviews.llvm.org/D42991 Files: 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/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp cfe/trunk/test/Analysis/new.cpp Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ cfe/trunk/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(); @@ -131,7 +123,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()); } @@ -143,7 +135,7 @@ 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)) { // TODO: We should construct into a CXXBindTemporaryExpr or a @@ -154,7 +146,7 @@ // function, we should be able to take the call-site CFG element, // and it should contain (but right now it wouldn't) some sort of // construction context that'd give us the right temporary expression. -CallOpts.IsConstructorIntoTemporary = true; +CallOpts.IsTemporaryCtorOrDtor = true; return MRMgr.getCXXTempObjectRegion(CE, LCtx); } // TODO: Consider other directly initialized elements. @@ -177,7 +169,7 @@ QualType Ty = Field->getType(); FieldVal = makeZeroElementRegion(State, FieldVal, Ty, - CallOpts.IsArrayConstructorOrDestructor); + CallOpts.IsArrayCtorOrDtor); return FieldVal.getAsRegion(); } @@ -187,7 +179,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); } @@ -273,7 +265,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 @@ -343,7 +335,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) @@ -400,23 +392,11 @@ const Stmt *S, bool IsBaseDtor, ExplodedNode *Pred, -ExplodedNodeSet &Dst) { +ExplodedNodeSet &Dst, +const EvalCallOptions &CallOpts) {
[PATCH] D42991: [analyzer] Use EvalCallOptions for destructors as well.
NoQ updated this revision to Diff 133299. NoQ added a comment. Remove the check in `shouldInlineCall()` as well. It's quite covered by the `IsConstructorWithImproperlyModeledTargetRegion` check. https://reviews.llvm.org/D42991 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/new.cpp 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 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: 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(
[PATCH] D42991: [analyzer] Use EvalCallOptions for destructors as well.
NoQ updated this revision to Diff 133106. NoQ added a comment. Remove a couple of accidental blank lines. https://reviews.llvm.org/D42991 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/ExprEngineCXX.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp test/Analysis/new.cpp 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 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: 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