Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -567,6 +567,13 @@ const LocationContext *LCtx, ProgramStateRef State); + struct EvalCallOptions { + bool IsConstructorWithImproperlyModeledTargetRegion = false; + bool IsArrayConstructorOrDestructor = 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, @@ -574,7 +581,9 @@ /// \brief Default implementation of call evaluation. void defaultEvalCall(NodeBuilder &B, ExplodedNode *Pred, - const CallEvent &Call); + const CallEvent &Call, + const EvalCallOptions &CallOpts = {}); + private: void evalLoadCommon(ExplodedNodeSet &Dst, const Expr *NodeEx, /* Eventually will be a CFGStmt */ @@ -598,9 +607,23 @@ void examineStackFrames(const Decl *D, const LocationContext *LCtx, bool &IsRecursive, unsigned &StackDepth); + enum CallInlinePolicy { + CIP_Allowed, + CIP_DisallowedOnce, + CIP_DisallowedAlways + }; + + /// \brief See if a particular call should be inlined, by only looking + /// at the call event and the current state of analysis. + CallInlinePolicy mayInlineCallKind(const CallEvent &Call, + const ExplodedNode *Pred, + AnalyzerOptions &Opts, + const EvalCallOptions &CallOpts); + /// Checks our policies and decides weither the given call should be inlined. bool shouldInlineCall(const CallEvent &Call, const Decl *D, - const ExplodedNode *Pred); + const ExplodedNode *Pred, + const EvalCallOptions &CallOpts = {}); bool inlineCall(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, ExplodedNode *Pred, ProgramStateRef State); @@ -650,11 +673,11 @@ /// For a given constructor, look forward in the current CFG block to /// determine the region into which an object will be constructed by \p CE. - /// Returns either a field or local variable region if the object will be - /// directly constructed in an existing region or a temporary object region - /// if not. + /// When the lookahead fails, a temporary region is returned, and the + /// IsConstructorWithImproperlyModeledTargetRegion flag is set in \p CallOpts. const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE, - ExplodedNode *Pred); + ExplodedNode *Pred, + EvalCallOptions &CallOpts); /// Store the region returned by operator new() so that the constructor /// that follows it knew what location to initialize. The value should be Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -85,19 +85,22 @@ /// Returns a region representing the first element of a (possibly -/// multi-dimensional) array. +/// 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) { + QualType &Ty, bool &IsArray) { SValBuilder &SVB = State->getStateManager().getSValBuilder(); ASTContext &Ctx = SVB.getContext(); while (const ArrayType *AT = Ctx.getAsArrayType(Ty)) { Ty = AT->getElementType(); LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue); + IsArray = true; } return LValue; @@ -106,7 +109,8 @@ const MemRegion * ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE, - ExplodedNode *Pred) { + ExplodedNode *Pred, + EvalCallOptions &CallOpts) { const LocationContext *LCtx = Pred->getLocationContext(); ProgramStateRef State = Pred->getState(); @@ -122,9 +126,9 @@ if (const SubRegion *MR = dyn_cast_or_null( getCXXNewAllocatorValue(State, CNE, LCtx).getAsRegion())) { if (CNE->isArray()) { - // TODO: This code exists only to trigger the suppression for - // array constructors. In fact, we need to call the constructor - // for every allocated element, not just the first one! + // TODO: In fact, we need to call the constructor for every + // allocated element, not just the first one! + CallOpts.IsArrayConstructorOrDestructor = true; return getStoreManager().GetElementZeroRegion( MR, CNE->getType()->getPointeeType()); } @@ -136,7 +140,8 @@ if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) { SVal LValue = State->getLValue(Var, LCtx); QualType Ty = Var->getType(); - LValue = makeZeroElementRegion(State, LValue, Ty); + LValue = makeZeroElementRegion( + State, LValue, Ty, CallOpts.IsArrayConstructorOrDestructor); return LValue.getAsRegion(); } } @@ -162,7 +167,8 @@ } QualType Ty = Field->getType(); - FieldVal = makeZeroElementRegion(State, FieldVal, Ty); + FieldVal = makeZeroElementRegion(State, FieldVal, Ty, + CallOpts.IsArrayConstructorOrDestructor); return FieldVal.getAsRegion(); } @@ -171,7 +177,8 @@ // ExprEngine::VisitCXXConstructExpr. } // If we couldn't find an existing region to construct into, assume we're - // constructing a temporary. + // constructing a temporary. Notify the caller of our failure. + CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true; MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); return MRMgr.getCXXTempObjectRegion(CE, LCtx); } @@ -265,9 +272,11 @@ // For now, we just run the first constructor (which should still invalidate // the entire array). + EvalCallOptions CallOpts; + switch (CE->getConstructionKind()) { case CXXConstructExpr::CK_Complete: { - Target = getRegionForConstructedObject(CE, Pred); + Target = getRegionForConstructedObject(CE, Pred, CallOpts); break; } case CXXConstructExpr::CK_VirtualBase: @@ -304,6 +313,7 @@ if (dyn_cast_or_null(LCtx->getParentMap().getParent(CE))) { MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); Target = MRMgr.getCXXTempObjectRegion(CE, LCtx); + CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true; break; } // FALLTHROUGH @@ -371,10 +381,9 @@ ExplodedNodeSet DstEvaluated; StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx); - bool IsArray = isa(Target); if (CE->getConstructor()->isTrivial() && CE->getConstructor()->isCopyOrMoveConstructor() && - !IsArray) { + !CallOpts.IsArrayConstructorOrDestructor) { // FIXME: Handle other kinds of trivial constructors as well. for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end(); I != E; ++I) @@ -383,7 +392,7 @@ } else { for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end(); I != E; ++I) - defaultEvalCall(Bldr, *I, *Call); + defaultEvalCall(Bldr, *I, *Call, CallOpts); } // If the CFG was contructed without elements for temporary destructors @@ -431,7 +440,11 @@ SVal DestVal = UnknownVal(); if (Dest) DestVal = loc::MemRegionVal(Dest); - DestVal = makeZeroElementRegion(State, DestVal, ObjectType); + + EvalCallOptions CallOpts; + DestVal = makeZeroElementRegion(State, DestVal, ObjectType, + CallOpts.IsArrayConstructorOrDestructor); + Dest = DestVal.getAsRegion(); const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl(); @@ -454,7 +467,7 @@ StmtNodeBuilder Bldr(DstPreCall, DstInvalidated, *currBldrCtx); for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end(); I != E; ++I) - defaultEvalCall(Bldr, *I, *Call); + defaultEvalCall(Bldr, *I, *Call, CallOpts); ExplodedNodeSet DstPostCall; getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated, Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -616,15 +616,10 @@ Bldr.generateNode(Call.getProgramPoint(), State, Pred); } -enum CallInlinePolicy { - CIP_Allowed, - CIP_DisallowedOnce, - CIP_DisallowedAlways -}; - -static CallInlinePolicy mayInlineCallKind(const CallEvent &Call, - const ExplodedNode *Pred, - AnalyzerOptions &Opts) { +ExprEngine::CallInlinePolicy +ExprEngine::mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred, + AnalyzerOptions &Opts, + const ExprEngine::EvalCallOptions &CallOpts) { const LocationContext *CurLC = Pred->getLocationContext(); const StackFrameContext *CallerSFC = CurLC->getCurrentStackFrame(); switch (Call.getKind()) { @@ -658,18 +653,8 @@ // initializers for array fields in default move/copy constructors. // We still allow construction into ElementRegion targets when they don't // represent array elements. - const MemRegion *Target = Ctor.getCXXThisVal().getAsRegion(); - if (Target && isa(Target)) { - if (ParentExpr) - if (const CXXNewExpr *NewExpr = dyn_cast(ParentExpr)) - if (NewExpr->isArray()) - return CIP_DisallowedOnce; - - if (const TypedValueRegion *TR = dyn_cast( - cast(Target)->getSuperRegion())) - if (TR->getValueType()->isArrayType()) - return CIP_DisallowedOnce; - } + if (CallOpts.IsArrayConstructorOrDestructor) + return CIP_DisallowedOnce; // Inlining constructors requires including initializers in the CFG. const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); @@ -688,7 +673,7 @@ // FIXME: This is a hack. We don't handle temporary destructors // right now, so we shouldn't inline their constructors. if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete) - if (!Target || isa(Target)) + if (CallOpts.IsConstructorWithImproperlyModeledTargetRegion) return CIP_DisallowedOnce; break; @@ -702,11 +687,8 @@ assert(ADC->getCFGBuildOptions().AddImplicitDtors && "No CFG destructors"); (void)ADC; - const CXXDestructorCall &Dtor = cast(Call); - // FIXME: We don't handle constructors or destructors for arrays properly. - const MemRegion *Target = Dtor.getCXXThisVal().getAsRegion(); - if (Target && isa(Target)) + if (CallOpts.IsArrayConstructorOrDestructor) return CIP_DisallowedOnce; break; @@ -847,7 +829,8 @@ } bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D, - const ExplodedNode *Pred) { + const ExplodedNode *Pred, + const EvalCallOptions &CallOpts) { if (!D) return false; @@ -894,7 +877,7 @@ // FIXME: this checks both static and dynamic properties of the call, which // means we're redoing a bit of work that could be cached in the function // summary. - CallInlinePolicy CIP = mayInlineCallKind(Call, Pred, Opts); + CallInlinePolicy CIP = mayInlineCallKind(Call, Pred, Opts, CallOpts); if (CIP != CIP_Allowed) { if (CIP == CIP_DisallowedAlways) { assert(!MayInline.hasValue() || MayInline.getValue()); @@ -946,7 +929,8 @@ } void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, - const CallEvent &CallTemplate) { + const CallEvent &CallTemplate, + const EvalCallOptions &CallOpts) { // Make sure we have the most recent state attached to the call. ProgramStateRef State = Pred->getState(); CallEventRef<> Call = CallTemplate.cloneWithState(State); @@ -969,7 +953,7 @@ } else { RuntimeDefinition RD = Call->getRuntimeDefinition(); const Decl *D = RD.getDecl(); - if (shouldInlineCall(*Call, D, Pred)) { + if (shouldInlineCall(*Call, D, Pred, CallOpts)) { if (RD.mayHaveOtherDefinitions()) { AnalyzerOptions &Options = getAnalysisManager().options; Index: test/Analysis/new.cpp =================================================================== --- test/Analysis/new.cpp +++ test/Analysis/new.cpp @@ -311,8 +311,7 @@ void testArrayDestr() { NoReturnDtor *p = new NoReturnDtor[2]; delete[] p; // Calls the base destructor which aborts, checked below - //TODO: clang_analyzer_eval should not be called - clang_analyzer_eval(true); // expected-warning{{TRUE}} + clang_analyzer_eval(true); // no-warning } // Invalidate Region even in case of default destructor