This is much better! See minor comments below. Anna. On Jul 30, 2012, at 6:07 PM, Jordan Rose wrote:
> Author: jrose > Date: Mon Jul 30 20:07:55 2012 > New Revision: 161017 > > URL: http://llvm.org/viewvc/llvm-project?rev=161017&view=rev > Log: > [analyzer] Let CallEvent decide what goes in an inital stack frame. > > This removes explicit checks for 'this' and 'self' from > Store::enterStackFrame. It also removes getCXXThisRegion() as a virtual > method on all CallEvents; it's now only implemented in the parts of the > hierarchy where it is relevant. Finally, it removes the option to ask > for the ParmVarDecls attached to the definition of an inlined function, > saving a recomputation of the result of getRuntimeDefinition(). > > No visible functionality change! > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h > cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp > cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h?rev=161017&r1=161016&r2=161017&view=diff > ============================================================================== > --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h > (original) > +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h Mon > Jul 30 20:07:55 2012 > @@ -218,12 +218,6 @@ > /// \brief Returns the result type, adjusted for references. > QualType getResultType() const; > > - /// \brief Returns the value of the implicit 'this' object, or > UndefinedVal if > - /// this is not a C++ member function call. > - virtual SVal getCXXThisVal() const { > - return UndefinedVal(); > - } > - > /// \brief Returns true if any of the arguments appear to represent > callbacks. > bool hasNonZeroCallbackArg() const; > > @@ -247,6 +241,14 @@ > ProgramStateRef invalidateRegions(unsigned BlockCount, > ProgramStateRef Orig = 0) const; > > + typedef std::pair<Loc, SVal> FrameBindingTy; > + typedef SmallVectorImpl<FrameBindingTy> BindingsTy; > + > + /// Populates the given SmallVector with the bindings in the callee's stack > + /// frame at the start of this call. > + virtual void getInitialStackFrameContents(const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const = 0; > + > /// Returns a copy of this CallEvent, but using the given state. > template <typename T> > CallEventRef<T> cloneWithState(ProgramStateRef NewState) const; > @@ -284,9 +286,9 @@ > /// If the call has no accessible declaration (or definition, if > /// \p UseDefinitionParams is set), \c param_begin() will be equal to > /// \c param_end(). > - virtual param_iterator param_begin(bool UseDefinitionParams = false) const > =0; > + virtual param_iterator param_begin() const =0; > /// \sa param_begin() > - virtual param_iterator param_end(bool UseDefinitionParams = false) const = > 0; > + virtual param_iterator param_end() const = 0; > > typedef llvm::mapped_iterator<param_iterator, get_type_fun> > param_type_iterator; > @@ -344,8 +346,11 @@ > > virtual bool argumentsMayEscape() const; > > - virtual param_iterator param_begin(bool UseDefinitionParams = false) const; > - virtual param_iterator param_end(bool UseDefinitionParams = false) const; > + virtual void getInitialStackFrameContents(const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const; > + > + virtual param_iterator param_begin() const; > + virtual param_iterator param_end() const; > > static bool classof(const CallEvent *CA) { > return CA->getKind() >= CE_BEG_FUNCTION_CALLS && > @@ -415,8 +420,14 @@ > CXXInstanceCall(const CXXInstanceCall &Other) : SimpleCall(Other) {} > > public: > + /// \brief Returns the value of the implicit 'this' object. > + virtual SVal getCXXThisVal() const = 0; > + > virtual const Decl *getRuntimeDefinition() const; > > + virtual void getInitialStackFrameContents(const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const; > + > static bool classof(const CallEvent *CA) { > return CA->getKind() >= CE_BEG_CXX_INSTANCE_CALLS && > CA->getKind() <= CE_END_CXX_INSTANCE_CALLS; > @@ -529,8 +540,11 @@ > return getBlockDecl(); > } > > - virtual param_iterator param_begin(bool UseDefinitionParams = false) const; > - virtual param_iterator param_end(bool UseDefinitionParams = false) const; > + virtual void getInitialStackFrameContents(const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const; > + > + virtual param_iterator param_begin() const; > + virtual param_iterator param_end() const; > > virtual Kind getKind() const { return CE_Block; } > > @@ -579,8 +593,12 @@ > return getOriginExpr()->getArg(Index); > } > > + /// \brief Returns the value of the implicit 'this' object. > virtual SVal getCXXThisVal() const; > > + virtual void getInitialStackFrameContents(const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const; > + > virtual Kind getKind() const { return CE_CXXConstructor; } > > static bool classof(const CallEvent *CA) { > @@ -620,9 +638,14 @@ > virtual SourceRange getSourceRange() const { return Location; } > virtual unsigned getNumArgs() const { return 0; } > > + /// \brief Returns the value of the implicit 'this' object. > virtual SVal getCXXThisVal() const; > + > virtual const Decl *getRuntimeDefinition() const; > > + virtual void getInitialStackFrameContents(const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const; > + > virtual Kind getKind() const { return CE_CXXDestructor; } > > static bool classof(const CallEvent *CA) { > @@ -754,13 +777,13 @@ > llvm_unreachable("Unknown message kind"); > } > > - // TODO: We might want to only compute this once (or change the API for > - // getting the parameters). Currently, this gets called 3 times during > - // inlining. > virtual const Decl *getRuntimeDefinition() const; > > - virtual param_iterator param_begin(bool UseDefinitionParams = false) const; > - virtual param_iterator param_end(bool UseDefinitionParams = false) const; > + virtual void getInitialStackFrameContents(const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const; > + > + virtual param_iterator param_begin() const; > + virtual param_iterator param_end() const; > > virtual Kind getKind() const { return CE_ObjCMessage; } > > > Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=161017&r1=161016&r2=161017&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Mon Jul 30 20:07:55 2012 > @@ -232,25 +232,53 @@ > || isa<CXXConstructExpr>(S); > } > > +static void addParameterValuesToBindings(const StackFrameContext *CalleeCtx, > + CallEvent::BindingsTy &Bindings, > + SValBuilder &SVB, > + const CallEvent &Call, > + CallEvent::param_iterator I, > + CallEvent::param_iterator E) { > + MemRegionManager &MRMgr = SVB.getRegionManager(); > + > + unsigned Idx = 0; > + for (; I != E; ++I, ++Idx) { > + const ParmVarDecl *ParamDecl = *I; > + assert(ParamDecl && "Formal parameter has no decl?"); > + > + SVal ArgVal = Call.getArgSVal(Idx); > + if (!ArgVal.isUnknown()) { > + Loc ParamLoc = SVB.makeLoc(MRMgr.getVarRegion(ParamDecl, CalleeCtx)); > + Bindings.push_back(std::make_pair(ParamLoc, ArgVal)); > + } > + } > + > + // FIXME: Variadic arguments are not handled at all right now. > +} > + > > -CallEvent::param_iterator > -AnyFunctionCall::param_begin(bool UseDefinitionParams) const { > - const Decl *D = UseDefinitionParams ? getRuntimeDefinition() > - : getDecl(); > +CallEvent::param_iterator AnyFunctionCall::param_begin() const { > + const FunctionDecl *D = getDecl(); > if (!D) > return 0; > > - return cast<FunctionDecl>(D)->param_begin(); > + return D->param_begin(); > } > > -CallEvent::param_iterator > -AnyFunctionCall::param_end(bool UseDefinitionParams) const { > - const Decl *D = UseDefinitionParams ? getRuntimeDefinition() > - : getDecl(); > +CallEvent::param_iterator AnyFunctionCall::param_end() const { > + const FunctionDecl *D = getDecl(); > if (!D) > return 0; > > - return cast<FunctionDecl>(D)->param_end(); > + return D->param_end(); > +} > + > +void AnyFunctionCall::getInitialStackFrameContents( > + const StackFrameContext *CalleeCtx, > + BindingsTy &Bindings) const { > + const FunctionDecl *D = cast<FunctionDecl>(CalleeCtx->getDecl()); > + SValBuilder &SVB = getState()->getStateManager().getSValBuilder(); > + addParameterValuesToBindings(CalleeCtx, Bindings, SVB, *this, > + D->param_begin(), D->param_end()); > } > > QualType AnyFunctionCall::getDeclaredResultType() const { > @@ -371,6 +399,21 @@ > return 0; > } > > +void CXXInstanceCall::getInitialStackFrameContents( > + const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const { > + AnyFunctionCall::getInitialStackFrameContents(CalleeCtx, Bindings); > + > + SVal ThisVal = getCXXThisVal(); > + if (!ThisVal.isUnknown()) { > + SValBuilder &SVB = getState()->getStateManager().getSValBuilder(); > + const CXXMethodDecl *MD = cast<CXXMethodDecl>(CalleeCtx->getDecl()); > + Loc ThisLoc = SVB.getCXXThis(MD, CalleeCtx); > + Bindings.push_back(std::make_pair(ThisLoc, ThisVal)); > + } > +} > + > + > > SVal CXXMemberCall::getCXXThisVal() const { > const Expr *Base = getOriginExpr()->getImplicitObjectArgument(); > @@ -397,22 +440,14 @@ > return dyn_cast_or_null<BlockDataRegion>(DataReg); > } > > -CallEvent::param_iterator > -BlockCall::param_begin(bool UseDefinitionParams) const { > - // Blocks don't have distinct declarations and definitions. > - (void)UseDefinitionParams; > - > +CallEvent::param_iterator BlockCall::param_begin() const { > const BlockDecl *D = getBlockDecl(); > if (!D) > return 0; > return D->param_begin(); > } > > -CallEvent::param_iterator > -BlockCall::param_end(bool UseDefinitionParams) const { > - // Blocks don't have distinct declarations and definitions. > - (void)UseDefinitionParams; > - > +CallEvent::param_iterator BlockCall::param_end() const { > const BlockDecl *D = getBlockDecl(); > if (!D) > return 0; > @@ -425,6 +460,15 @@ > Regions.push_back(R); > } > > +void BlockCall::getInitialStackFrameContents(const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const { > + const BlockDecl *D = cast<BlockDecl>(CalleeCtx->getDecl()); > + SValBuilder &SVB = getState()->getStateManager().getSValBuilder(); > + addParameterValuesToBindings(CalleeCtx, Bindings, SVB, *this, > + D->param_begin(), D->param_end()); > +} > + > + Can't you reuse AnyFunctionCall's implementation of getInitialStackFrameContents(), param_begin(), and param_end()? > > QualType BlockCall::getDeclaredResultType() const { > const BlockDataRegion *BR = getBlockRegion(); > if (!BR) > @@ -445,6 +489,21 @@ > Regions.push_back(static_cast<const MemRegion *>(Data)); > } > > +void CXXConstructorCall::getInitialStackFrameContents( > + const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const { > + AnyFunctionCall::getInitialStackFrameContents(CalleeCtx, Bindings); > + > + SVal ThisVal = getCXXThisVal(); > + if (!ThisVal.isUnknown()) { > + SValBuilder &SVB = getState()->getStateManager().getSValBuilder(); > + const CXXMethodDecl *MD = cast<CXXMethodDecl>(CalleeCtx->getDecl()); > + Loc ThisLoc = SVB.getCXXThis(MD, CalleeCtx); > + Bindings.push_back(std::make_pair(ThisLoc, ThisVal)); > + } > +} > + > + > Is there a reason why CXXConstructor, CXXDestructor, CXXInstanceCall cannot share the implementation (I noticed that CXXConstructorCall is not subclassed from instance call.. )? > SVal CXXDestructorCall::getCXXThisVal() const { > if (Data) > @@ -474,25 +533,35 @@ > return 0; > } > > +void CXXDestructorCall::getInitialStackFrameContents( > + const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const { > + AnyFunctionCall::getInitialStackFrameContents(CalleeCtx, Bindings); > + > + SVal ThisVal = getCXXThisVal(); > + if (!ThisVal.isUnknown()) { > + SValBuilder &SVB = getState()->getStateManager().getSValBuilder(); > + const CXXMethodDecl *MD = cast<CXXMethodDecl>(CalleeCtx->getDecl()); > + Loc ThisLoc = SVB.getCXXThis(MD, CalleeCtx); > + Bindings.push_back(std::make_pair(ThisLoc, ThisVal)); > + } > +} > > -CallEvent::param_iterator > -ObjCMethodCall::param_begin(bool UseDefinitionParams) const { > - const Decl *D = UseDefinitionParams ? getRuntimeDefinition() > - : getDecl(); > + > +CallEvent::param_iterator ObjCMethodCall::param_begin() const { > + const ObjCMethodDecl *D = getDecl(); > if (!D) > return 0; > > - return cast<ObjCMethodDecl>(D)->param_begin(); > + return D->param_begin(); > } > > -CallEvent::param_iterator > -ObjCMethodCall::param_end(bool UseDefinitionParams) const { > - const Decl *D = UseDefinitionParams ? getRuntimeDefinition() > - : getDecl(); > +CallEvent::param_iterator ObjCMethodCall::param_end() const { > + const ObjCMethodDecl *D = getDecl(); > if (!D) > return 0; > > - return cast<ObjCMethodDecl>(D)->param_end(); > + return D->param_end(); > } > > void > @@ -626,6 +695,23 @@ > return 0; > } > > +void ObjCMethodCall::getInitialStackFrameContents( > + const StackFrameContext > *CalleeCtx, > + BindingsTy &Bindings) const { > + const ObjCMethodDecl *D = cast<ObjCMethodDecl>(CalleeCtx->getDecl()); > + SValBuilder &SVB = getState()->getStateManager().getSValBuilder(); > + addParameterValuesToBindings(CalleeCtx, Bindings, SVB, *this, > + D->param_begin(), D->param_end()); > + > + SVal SelfVal = getReceiverSVal(); > + if (!SelfVal.isUnknown()) { > + const VarDecl *SelfD = > CalleeCtx->getAnalysisDeclContext()->getSelfDecl(); > + MemRegionManager &MRMgr = SVB.getRegionManager(); > + Loc SelfLoc = SVB.makeLoc(MRMgr.getVarRegion(SelfD, CalleeCtx)); > + Bindings.push_back(std::make_pair(SelfLoc, SelfVal)); > + } > +} > + > > CallEventRef<SimpleCall> > CallEventManager::getSimpleCall(const CallExpr *CE, ProgramStateRef State, > > Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=161017&r1=161016&r2=161017&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Mon Jul 30 > 20:07:55 2012 > @@ -284,14 +284,14 @@ > const StackFrameContext *CallerSFC = CurLC->getCurrentStackFrame(); > const LocationContext *ParentOfCallee = 0; > > + // FIXME: Refactor this check into a hypothetical CallEvent::canInline. > switch (Call.getKind()) { > case CE_Function: > case CE_CXXMember: > case CE_CXXMemberOperator: > // These are always at least possible to inline. > break; > - case CE_CXXConstructor: > - case CE_CXXDestructor: { > + case CE_CXXConstructor: { > // Only inline constructors and destructors if we built the CFGs for them > // properly. > const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); > @@ -299,19 +299,37 @@ > !ADC->getCFGBuildOptions().AddInitializers) > return false; > > + const CXXConstructorCall &Ctor = cast<CXXConstructorCall>(Call); > + > // FIXME: We don't handle constructors or destructors for arrays properly. > - const MemRegion *Target = Call.getCXXThisVal().getAsRegion(); > + const MemRegion *Target = Ctor.getCXXThisVal().getAsRegion(); > if (Target && isa<ElementRegion>(Target)) > return false; > > // FIXME: This is a hack. We don't handle temporary destructors > // right now, so we shouldn't inline their constructors. > - if (const CXXConstructorCall *Ctor = > dyn_cast<CXXConstructorCall>(&Call)) { > - const CXXConstructExpr *CtorExpr = Ctor->getOriginExpr(); > - if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete) > - if (!Target || !isa<DeclRegion>(Target)) > - return false; > - } > + const CXXConstructExpr *CtorExpr = Ctor.getOriginExpr(); > + if (CtorExpr->getConstructionKind() == CXXConstructExpr::CK_Complete) > + if (!Target || !isa<DeclRegion>(Target)) > + return false; > + > + break; > + } > + case CE_CXXDestructor: { > + // Only inline constructors and destructors if we built the CFGs for them > + // properly. > + const AnalysisDeclContext *ADC = CallerSFC->getAnalysisDeclContext(); > + if (!ADC->getCFGBuildOptions().AddImplicitDtors || > + !ADC->getCFGBuildOptions().AddInitializers) > + return false; > + > + 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)) > + return false; > + > break; > } > case CE_CXXAllocator: > > Modified: cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp?rev=161017&r1=161016&r2=161017&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp Mon Jul 30 20:07:55 2012 > @@ -29,37 +29,15 @@ > const StackFrameContext *LCtx) { > StoreRef Store = StoreRef(OldStore, *this); > > - unsigned Idx = 0; > - for (CallEvent::param_iterator I = > Call.param_begin(/*UseDefinition=*/true), > - E = Call.param_end(/*UseDefinition=*/true); > - I != E; ++I, ++Idx) { > - const ParmVarDecl *Decl = *I; > - assert(Decl && "Formal parameter has no decl?"); > - > - SVal ArgVal = Call.getArgSVal(Idx); > - if (!ArgVal.isUnknown()) { > - Store = Bind(Store.getStore(), > - svalBuilder.makeLoc(MRMgr.getVarRegion(Decl, LCtx)), > - ArgVal); > - } > - } > + SmallVector<CallEvent::FrameBindingTy, 16> InitialBindings; > + Call.getInitialStackFrameContents(LCtx, InitialBindings); > > - // FIXME: We will eventually want to generalize this to handle other non- > - // parameter arguments besides 'this' (such as 'self' for ObjC methods). > - SVal ThisVal = Call.getCXXThisVal(); > - if (isa<DefinedSVal>(ThisVal)) { > - const CXXMethodDecl *MD = cast<CXXMethodDecl>(Call.getDecl()); > - loc::MemRegionVal ThisRegion = svalBuilder.getCXXThis(MD, LCtx); > - Store = Bind(Store.getStore(), ThisRegion, ThisVal); > + for (CallEvent::BindingsTy::iterator I = InitialBindings.begin(), > + E = InitialBindings.end(); > + I != E; ++I) { > + Store = Bind(Store.getStore(), I->first, I->second); > } > > - if (const ObjCMethodCall *MCall = dyn_cast<ObjCMethodCall>(&Call)) { > - SVal SelfVal = MCall->getReceiverSVal(); > - const VarDecl *SelfDecl = LCtx->getAnalysisDeclContext()->getSelfDecl(); > - Store = Bind(Store.getStore(), > - svalBuilder.makeLoc(MRMgr.getVarRegion(SelfDecl, LCtx)), > - SelfVal); > - } > return Store; > } > > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
