[analyzer] Don't communicate evaluation failures through memregion hierarchy.

[NoQ]

Summary

• If we are unable to determine a good target region for a C++ constructor call, we come up with a temporary region and construct into that.
• If we suddenly notice that our constructor is part of an C++ array construction (operator new[] or an array-typed local variable), we come up with the element-zero region and construct into that, same for destructors.

Both are bad not only because we fail to model stuff correctly, but also because we use region structure to communicate the fact that we failed to ExprEngine::evalCall, which would later decide if we should inline the call or not by looking at the region structure. The former is particularly bad because this prevents us from supporting the actual construction into temporaries - even if we produce a correct temporary region, we won't inline the constructor because temporary region is an (incorrect) indication of failure.

In order to unblock the progress in this area, this patch adds an auxiliary flag for evalCall's clients to communicate potential problems with the call. Then evalCall would decide if it should still inline the call. Apparently, the current decision process is non-trivial in this case: for instance, we may still inline the constructor even if the target region is incorrect aka temporary, when the class's destructor is trivial. In any case, it's good to have all the when-to-inline heuristics in one place.

For now ExprEngine::evalCall itself doesn't have a flag - only ExprEngine::defaultEvalCall does. Because for now it's only relevant to constructor/destructor calls, but checker-side evalCall only works for simple call expressions - that's a separate issue. Ideally the flag should go there as well, and defaultEvalCall would only be called from ExprEngine::evalCall.

Functional change here is accidental - by communicating array destructor situation properly, we're able to fix an old FIXME.

[george.karpenkov]

include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

570 ↗

(On Diff #131184)

Those are not really flags. Perhaps options?

572 ↗

(On Diff #131184)

OK my C++ knowledge is weak here.
What happens if you don't initialize those at the callsite and then read? Wouldn't be safer to set them both to false in the declaration?

661 ↗

(On Diff #131184)

Which flag?

lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

92 ↗

(On Diff #131184)

Doxygen comment for out-parameter? E.g. it's non-obvious that it only sets the flag to true and does not touch it otherwise.

94 ↗

(On Diff #131184)

Perhaps IsArray will also do?

142 ↗

(On Diff #131184)

Or I suppose you could do makeZeroElementRegion(..., &Flags.IsArrayConstructorOrDestructor) to express the intent

171 ↗

(On Diff #131184)

Same as previous comment, I suppose you could write into IsArrayConstructorOrDestructor directly.

451 ↗

(On Diff #131184)

Again perhaps write into Flags.Is... directly?

[NoQ]

Address comments.

include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

572 ↗

(On Diff #131184)

Yeah, i guess i'd add a constructor. Unfortunately inline initialization for bitfields is not available until C++20. So i hope we'd be able to get rid of this construct before we switch to C++20 =)

661 ↗

(On Diff #131184)

The respective flag. To be exact, the IntentionallyLongTemporaryFlagNameThatNobodyWouldEverBotherToReadCorrectly one :)

Fxd thx :)

[dcoughlin]

This looks good to me. However, I do think you should take George's suggestion to have makeZeroElementRegion() have a boolean out parameter rather than a EvalCallOptions out parameter. This avoids unnecessarily coupling of makeZeroElementRegion() and EvalallOptions. You will need to make the boolean fields not bitfields (since we can't take a bitfield's address in C++). But EvalCallOptions is only on the stack (and we can pass it by const reference) so I don't think the increased size is something we should worry about.

[NoQ]

Switched to plain bools and in-class initializers.

[NoQ]

Functional change here is accidental - by communicating array destructor situation properly, we're able to fix an old FIXME.

Minor temporary insanity. This test was "fixed" because in mayInlineCall() for destructors i started to look for the flag that i never set for destructors :/

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

void evalStore(ExplodedNodeSet &Dst, const Expr *AssignE, const Expr *StoreE,
                  const ProgramPointTag *tag = nullptr);
 
   /// \brief Create a new state in which the call return value is binded to the
   /// call origin expression.
   ProgramStateRef bindReturnValue(const CallEvent &Call,
                                   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,
                 const CallEvent &Call);
 
   /// \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 */
                       const Expr *BoundEx,
                       ExplodedNode *Pred,
                       ProgramStateRef St,
                       SVal location,
                       const ProgramPointTag *tag,
                       QualType LoadTy);
 
   // FIXME: 'tag' should be removed, and a LocationContext should be used
   // instead.
   void evalLocation(ExplodedNodeSet &Dst,
                     const Stmt *NodeEx, /* This will eventually be a CFGStmt */
                     const Stmt *BoundEx,
                     ExplodedNode *Pred,
                     ProgramStateRef St, SVal location,
                     const ProgramPointTag *tag, bool isLoad);
 
   /// Count the stack depth and determine if the call is recursive.
   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);
 
   /// \brief Conservatively evaluate call by invalidating regions and binding
   /// a conjured return value.
   void conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
                             ExplodedNode *Pred, ProgramStateRef State);
private:
   /// CFGElement for the DeclStmt or CXXInitCtorInitializer or CXXNewExpr which
   /// is directly constructed by this constructor. Returns None if the current
   /// constructor expression did not directly construct into an existing
   /// region.
   Optional<CFGElement> findElementDirectlyInitializedByCurrentConstructor();
 
   /// 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
   /// cleared once the respective CXXNewExpr CFGStmt element is processed.
   static ProgramStateRef
   setCXXNewAllocatorValue(ProgramStateRef State, const CXXNewExpr *CNE,
                           const LocationContext *CallerLC, SVal V);
 

cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

for (ExplodedNodeSet::iterator I = Dst.begin(), E = Dst.end();
     else
       State = bindReturnValue(Call, LCtx, State);
     Bldr.generateNode(PS, State, *I);
   }
 }
 
 
 /// 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;
 }
 
 
 const MemRegion *
 ExprEngine::getRegionForConstructedObject(const CXXConstructExpr *CE,
-                                          ExplodedNode *Pred) {
+                                          ExplodedNode *Pred,
+                                          EvalCallOptions &CallOpts) {
   const LocationContext *LCtx = Pred->getLocationContext();
   ProgramStateRef State = Pred->getState();
 
   // See if we're constructing an existing region by looking at the next
   // element in the CFG.
 
   if (auto Elem = findElementDirectlyInitializedByCurrentConstructor()) {
     if (Optional<CFGStmt> StmtElem = Elem->getAs<CFGStmt>()) {
       if (const CXXNewExpr *CNE = dyn_cast<CXXNewExpr>(StmtElem->getStmt())) {
         if (AMgr.getAnalyzerOptions().mayInlineCXXAllocator()) {
           // TODO: Detect when the allocator returns a null pointer.
           // Constructor shall not be called in this case.
           if (const SubRegion *MR = dyn_cast_or_null<SubRegion>(
                   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());
             }
             return MR;
           }
         }
       } else if (auto *DS = dyn_cast<DeclStmt>(StmtElem->getStmt())) {
         if (const auto *Var = dyn_cast<VarDecl>(DS->getSingleDecl())) {
           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();
           }
         }
       } else {
         llvm_unreachable("Unexpected directly initialized element!");
       }
     } else if (Optional<CFGInitializer> InitElem = Elem->getAs<CFGInitializer>()) {
       const CXXCtorInitializer *Init = InitElem->getInitializer();
if (Optional<CFGStmt> StmtElem = Elem->getAs<CFGStmt>()) {
         Field = Init->getIndirectMember();
         FieldVal = State->getLValue(Init->getIndirectMember(), ThisVal);
       } else {
         Field = Init->getMember();
         FieldVal = State->getLValue(Init->getMember(), ThisVal);
       }
 
       QualType Ty = Field->getType();
-      FieldVal = makeZeroElementRegion(State, FieldVal, Ty);
+      FieldVal = makeZeroElementRegion(State, FieldVal, Ty,
+                                       CallOpts.IsArrayConstructorOrDestructor);
       return FieldVal.getAsRegion();
     }
 
     // FIXME: This will eventually need to handle new-expressions as well.
     // Don't forget to update the pre-constructor initialization code in
     // 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);
 }
 
 /// Returns true if the initializer for \Elem can be a direct
 /// constructor.
 static bool canHaveDirectConstructor(CFGElement Elem){
   // DeclStmts and CXXCtorInitializers for fields can be directly constructed.
void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
   ProgramStateRef State = Pred->getState();
 
   const MemRegion *Target = nullptr;
 
   // FIXME: Handle arrays, which run the same constructor for every element.
   // 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:
     // Make sure we are not calling virtual base class initializers twice.
     // Only the most-derived object should initialize virtual base classes.
     if (const Stmt *Outer = LCtx->getCurrentStackFrame()->getCallSite()) {
       const CXXConstructExpr *OuterCtor = dyn_cast<CXXConstructExpr>(Outer);
       if (OuterCtor) {
case CXXConstructExpr::CK_NonVirtualBase:
     // FIXME: For now this code essentially bails out. We need to find the
     // correct target region and set it.
     // FIXME: Instead of relying on the ParentMap, we should have the
     // trigger-statement (InitListExpr in this case) passed down from CFG or
     // otherwise always available during construction.
     if (dyn_cast_or_null<InitListExpr>(LCtx->getParentMap().getParent(CE))) {
       MemRegionManager &MRMgr = getSValBuilder().getRegionManager();
       Target = MRMgr.getCXXTempObjectRegion(CE, LCtx);
+      CallOpts.IsConstructorWithImproperlyModeledTargetRegion = true;
       break;
     }
     // FALLTHROUGH
   case CXXConstructExpr::CK_Delegating: {
     const CXXMethodDecl *CurCtor = cast<CXXMethodDecl>(LCtx->getDecl());
     Loc ThisPtr = getSValBuilder().getCXXThis(CurCtor,
                                               LCtx->getCurrentStackFrame());
     SVal ThisVal = State->getSVal(ThisPtr);
void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
 
   ExplodedNodeSet DstPreCall;
   getCheckerManager().runCheckersForPreCall(DstPreCall, PreInitialized,
                                             *Call, *this);
 
   ExplodedNodeSet DstEvaluated;
   StmtNodeBuilder Bldr(DstPreCall, DstEvaluated, *currBldrCtx);
 
-  bool IsArray = isa<ElementRegion>(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)
       performTrivialCopy(Bldr, *I, *Call);
 
   } 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
   // and the just-called constructor created a temporary object then
   // stop exploration if the temporary object has a noreturn constructor.
   // This can lose coverage because the destructor, if it were present
   // in the CFG, would be called at the end of the full expression or
   // later (for life-time extended temporaries) -- but avoids infeasible
void ExprEngine::VisitCXXDestructor(QualType ObjectType,
   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);
-  DestVal = makeZeroElementRegion(State, DestVal, ObjectType);
+
+  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();
 
   CallEventManager &CEMgr = getStateManager().getCallEventManager();
   CallEventRef<CXXDestructorCall> Call =
     CEMgr.getCXXDestructorCall(DtorDecl, S, Dest, IsBaseDtor, State, LCtx);
 
   PrettyStackTraceLoc CrashInfo(getContext().getSourceManager(),
                                 Call->getSourceRange().getBegin(),
                                 "Error evaluating destructor");
 
   ExplodedNodeSet DstPreCall;
   getCheckerManager().runCheckersForPreCall(DstPreCall, Pred,
                                             *Call, *this);
 
   ExplodedNodeSet DstInvalidated;
   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,
                                              *Call, *this);
 }
 
 void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr *CNE,
                                           ExplodedNode *Pred,

cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

void ExprEngine::conservativeEvalCall(const CallEvent &Call, NodeBuilder &Bldr,
                                       ProgramStateRef State) {
   State = Call.invalidateRegions(currBldrCtx->blockCount(), State);
   State = bindReturnValue(Call, Pred->getLocationContext(), State);
 
   // And make the result node.
   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()) {
   case CE_Function:
   case CE_Block:
     break;
   case CE_CXXMember:
   case CE_CXXMemberOperator:
if (ParentExpr && isa<CXXNewExpr>(ParentExpr) &&
         !Opts.mayInlineCXXAllocator())
       return CIP_DisallowedOnce;
 
     // FIXME: We don't handle constructors or destructors for arrays properly.
     // Even once we do, we still need to be careful about implicitly-generated
     // 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<ElementRegion>(Target)) {
-      if (ParentExpr)
-        if (const CXXNewExpr *NewExpr = dyn_cast<CXXNewExpr>(ParentExpr))
-          if (NewExpr->isArray())
-            return CIP_DisallowedOnce;
+    if (CallOpts.IsArrayConstructorOrDestructor)
 
-      if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(
-              cast<SubRegion>(Target)->getSuperRegion()))
-        if (TR->getValueType()->isArrayType())
-          return CIP_DisallowedOnce;
-    }
-
     // Inlining constructors requires including initializers in the CFG.
     const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext();
     assert(ADC->getCFGBuildOptions().AddInitializers && "No CFG initializers");
     (void)ADC;
 
     // If the destructor is trivial, it's always safe to inline the constructor.
     if (Ctor.getDecl()->getParent()->hasTrivialDestructor())
       break;
 
     // For other types, only inline constructors if destructor inlining is
     // also enabled.
     if (!Opts.mayInlineCXXMemberFunction(CIMK_Destructors))
       return CIP_DisallowedAlways;
 
     // 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<CXXTempObjectRegion>(Target))
+      if (CallOpts.IsConstructorWithImproperlyModeledTargetRegion)
         return CIP_DisallowedOnce;
 
     break;
   }
   case CE_CXXDestructor: {
     if (!Opts.mayInlineCXXMemberFunction(CIMK_Destructors))
       return CIP_DisallowedAlways;
 
     // Inlining destructors requires building the CFG correctly.
     const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext();
     assert(ADC->getCFGBuildOptions().AddImplicitDtors && "No CFG destructors");
     (void)ADC;
 
-    const CXXDestructorCall &Dtor = cast<CXXDestructorCall>(Call);
-
     // FIXME: We don't handle constructors or destructors for arrays properly.
-    const MemRegion *Target = Dtor.getCXXThisVal().getAsRegion();
-    if (Target && isa<ElementRegion>(Target))
+    if (CallOpts.IsArrayConstructorOrDestructor)
       return CIP_DisallowedOnce;
 
     break;
   }
   case CE_CXXAllocator:
     if (Opts.mayInlineCXXAllocator())
       break;
     // Do not inline allocators until we model deallocators.
static bool mayInlineDecl(AnalysisDeclContext *CalleeADC,
   // run.  If so, bail out.
   if (!CalleeADC->getAnalysis<RelaxedLiveVariables>())
     return false;
 
   return true;
 }
 
 bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D,
-                                  const ExplodedNode *Pred) {
+                                  const ExplodedNode *Pred,
+                                  const EvalCallOptions &CallOpts) {
   if (!D)
     return false;
 
   AnalysisManager &AMgr = getAnalysisManager();
   AnalyzerOptions &Opts = AMgr.options;
   AnalysisDeclContextManager &ADCMgr = AMgr.getAnalysisDeclContextManager();
   AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D);
 
if (mayInlineDecl(CalleeADC, Opts)) {
       return false;
     }
   }
 
   // Check if we should inline a call based on its kind.
   // 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());
       Engine.FunctionSummaries->markShouldNotInline(D);
     }
     return false;
   }
 
if (!MD)
     return false;
   if (!(MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()))
     return false;
 
   return MD->isTrivial();
 }
 
 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);
 
   // Special-case trivial assignment operators.
   if (isTrivialObjectAssignment(*Call)) {
     performTrivialCopy(Bldr, Pred, *Call);
     return;
   }
 
   // Try to inline the call.
   // The origin expression here is just used as a kind of checksum;
   // this should still be safe even for CallEvents that don't come from exprs.
   const Expr *E = Call->getOriginExpr();
 
   ProgramStateRef InlinedFailedState = getInlineFailedState(State, E);
   if (InlinedFailedState) {
     // If we already tried once and failed, make sure we don't retry later.
     State = InlinedFailedState;
   } 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;
 
         // Explore with and without inlining the call.
         if (Options.getIPAMode() == IPAK_DynamicDispatchBifurcate) {
           BifurcateCall(RD.getDispatchRegion(), *Call, D, Bldr, Pred);
           return;
         }

cfe/trunk/test/Analysis/new.cpp

void testArrayNull() {
   NoReturnDtor *fooArray = 0;
   delete[] fooArray; // should not call destructor, checked below
   clang_analyzer_eval(true); // expected-warning{{TRUE}}
 }
 
 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
 class InvalidateDestTest {
 public:
   int x;
   int *y;
   ~InvalidateDestTest();