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,11 @@ const LocationContext *LCtx, ProgramStateRef State); + struct EvalCallFlags { + bool IsConstructorWithImproperlyModeledTargetRegion : 1; + bool IsArrayConstructorOrDestructor : 1; + }; + /// 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 +579,9 @@ /// \brief Default implementation of call evaluation. void defaultEvalCall(NodeBuilder &B, ExplodedNode *Pred, - const CallEvent &Call); + const CallEvent &Call, + EvalCallFlags Flags = {}); + private: void evalLoadCommon(ExplodedNodeSet &Dst, const Expr *NodeEx, /* Eventually will be a CFGStmt */ @@ -600,7 +607,7 @@ /// 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, EvalCallFlags Flags); bool inlineCall(const CallEvent &Call, const Decl *D, NodeBuilder &Bldr, ExplodedNode *Pred, ProgramStateRef State); @@ -650,11 +657,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 a flag is + /// set in \p Flags. const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE, - ExplodedNode *Pred); + ExplodedNode *Pred, + EvalCallFlags &Flags); /// 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 @@ -91,13 +91,14 @@ /// /// If the type is not an array type at all, the original value is returned. static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue, - QualType &Ty) { + QualType &Ty, bool &IsIndeedAnArray) { 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); + IsIndeedAnArray = true; } return LValue; @@ -106,7 +107,8 @@ const MemRegion * ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE, - ExplodedNode *Pred) { + ExplodedNode *Pred, + EvalCallFlags &Flags) { const LocationContext *LCtx = Pred->getLocationContext(); ProgramStateRef State = Pred->getState(); @@ -122,9 +124,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! + Flags.IsArrayConstructorOrDestructor = true; return getStoreManager().GetElementZeroRegion( MR, CNE->getType()->getPointeeType()); } @@ -136,7 +138,10 @@ if (Var->getInit() && Var->getInit()->IgnoreImplicit() == CE) { SVal LValue = State->getLValue(Var, LCtx); QualType Ty = Var->getType(); - LValue = makeZeroElementRegion(State, LValue, Ty); + bool IsArray = false; + LValue = makeZeroElementRegion(State, LValue, Ty, IsArray); + if (IsArray) + Flags.IsArrayConstructorOrDestructor = true; return LValue.getAsRegion(); } } @@ -162,7 +167,10 @@ } QualType Ty = Field->getType(); - FieldVal = makeZeroElementRegion(State, FieldVal, Ty); + bool IsArray = false; + FieldVal = makeZeroElementRegion(State, FieldVal, Ty, IsArray); + if (IsArray) + Flags.IsArrayConstructorOrDestructor = true; return FieldVal.getAsRegion(); } @@ -171,7 +179,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. + Flags.IsConstructorWithImproperlyModeledTargetRegion = true; MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); return MRMgr.getCXXTempObjectRegion(CE, LCtx); } @@ -265,9 +274,11 @@ // For now, we just run the first constructor (which should still invalidate // the entire array). + EvalCallFlags Flags{}; + switch (CE->getConstructionKind()) { case CXXConstructExpr::CK_Complete: { - Target = getRegionForConstructedObject(CE, Pred); + Target = getRegionForConstructedObject(CE, Pred, Flags); break; } case CXXConstructExpr::CK_VirtualBase: @@ -304,6 +315,7 @@ if (dyn_cast_or_null(LCtx->getParentMap().getParent(CE))) { MemRegionManager &MRMgr = getSValBuilder().getRegionManager(); Target = MRMgr.getCXXTempObjectRegion(CE, LCtx); + Flags.IsConstructorWithImproperlyModeledTargetRegion = true; break; } // FALLTHROUGH @@ -371,10 +383,9 @@ ExplodedNodeSet DstEvaluated; StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx); - bool IsArray = isa(Target); if (CE->getConstructor()->isTrivial() && CE->getConstructor()->isCopyOrMoveConstructor() && - !IsArray) { + !Flags.IsArrayConstructorOrDestructor) { // FIXME: Handle other kinds of trivial constructors as well. for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end(); I != E; ++I) @@ -383,7 +394,7 @@ } else { for (ExplodedNodeSet::iterator I = DstPreCall.begin(), E = DstPreCall.end(); I != E; ++I) - defaultEvalCall(Bldr, *I, *Call); + defaultEvalCall(Bldr, *I, *Call, Flags); } // If the CFG was contructed without elements for temporary destructors @@ -431,7 +442,14 @@ SVal DestVal = UnknownVal(); if (Dest) DestVal = loc::MemRegionVal(Dest); - DestVal = makeZeroElementRegion(State, DestVal, ObjectType); + + EvalCallFlags Flags{}; + + bool IsArray = false; + DestVal = makeZeroElementRegion(State, DestVal, ObjectType, IsArray); + if (IsArray) + Flags.IsArrayConstructorOrDestructor = true; + Dest = DestVal.getAsRegion(); const CXXRecordDecl *RecordDecl = ObjectType->getAsCXXRecordDecl(); @@ -454,7 +472,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, Flags); ExplodedNodeSet DstPostCall; getCheckerManager().runCheckersForPostCall(Dst, DstInvalidated, Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -624,7 +624,8 @@ static CallInlinePolicy mayInlineCallKind(const CallEvent &Call, const ExplodedNode *Pred, - AnalyzerOptions &Opts) { + AnalyzerOptions &Opts, + ExprEngine::EvalCallFlags Flags) { const LocationContext *CurLC = Pred->getLocationContext(); const StackFrameContext *CallerSFC = CurLC->getCurrentStackFrame(); switch (Call.getKind()) { @@ -658,18 +659,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 (Flags.IsArrayConstructorOrDestructor) + return CIP_DisallowedOnce; // Inlining constructors requires including initializers in the CFG. const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); @@ -688,7 +679,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 (Flags.IsConstructorWithImproperlyModeledTargetRegion) return CIP_DisallowedOnce; break; @@ -702,11 +693,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 (Flags.IsArrayConstructorOrDestructor) return CIP_DisallowedOnce; break; @@ -847,7 +835,8 @@ } bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D, - const ExplodedNode *Pred) { + const ExplodedNode *Pred, + EvalCallFlags Flags) { if (!D) return false; @@ -894,7 +883,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, Flags); if (CIP != CIP_Allowed) { if (CIP == CIP_DisallowedAlways) { assert(!MayInline.hasValue() || MayInline.getValue()); @@ -946,7 +935,8 @@ } void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred, - const CallEvent &CallTemplate) { + const CallEvent &CallTemplate, + EvalCallFlags Flags) { // Make sure we have the most recent state attached to the call. ProgramStateRef State = Pred->getState(); CallEventRef<> Call = CallTemplate.cloneWithState(State); @@ -969,7 +959,7 @@ } else { RuntimeDefinition RD = Call->getRuntimeDefinition(); const Decl *D = RD.getDecl(); - if (shouldInlineCall(*Call, D, Pred)) { + if (shouldInlineCall(*Call, D, Pred, Flags)) { 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