[cfe-commits] r161017 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h lib/StaticAnalyzer/Core/CallEvent.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp lib/StaticAnalyzer/Core/Store.cpp
Anna Zaks
ganna at apple.com
Tue Jul 31 18:55:11 PDT 2012
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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list