[cfe-commits] r138220 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/CFRefCount.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
Ted Kremenek
kremenek at apple.com
Sun Aug 21 17:48:26 PDT 2011
Was the performance hit observable? If so, what are you seeing? Why do you think there is a performance hit?
On Aug 21, 2011, at 12:41 PM, Jordy Rose <jediknil at belkadan.com> wrote:
> Author: jrose
> Date: Sun Aug 21 14:41:36 2011
> New Revision: 138220
>
> URL: http://llvm.org/viewvc/llvm-project?rev=138220&view=rev
> Log:
> [analyzer] Migrate return value handling from CFRefCount to ExprEngine. This seems to result in a minor performance hit, but I think that will go away again once we eliminate TransferFuncs from function calls entirely.
>
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
> cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=138220&r1=138219&r2=138220&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Sun Aug 21 14:41:36 2011
> @@ -408,10 +408,7 @@
>
> protected:
> void evalObjCMessage(ExplodedNodeSet &Dst, const ObjCMessage &msg,
> - ExplodedNode *Pred, const ProgramState *state) {
> - assert (Builder && "StmtNodeBuilder must be defined.");
> - getTF().evalObjCMessage(Dst, *this, *Builder, msg, Pred, state);
> - }
> + ExplodedNode *Pred, const ProgramState *state);
>
> const ProgramState *MarkBranch(const ProgramState *St, const Stmt *Terminator,
> bool branchTaken);
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp?rev=138220&r1=138219&r2=138220&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/CFRefCount.cpp Sun Aug 21 14:41:36 2011
> @@ -140,7 +140,6 @@
> public:
> enum Kind { NoRet, Alias, OwnedSymbol, OwnedAllocatedSymbol,
> NotOwnedSymbol, GCNotOwnedSymbol, ARCNotOwnedSymbol,
> - ReceiverAlias,
> OwnedWhenTrackedReceiver };
>
> enum ObjKind { CF, ObjC, AnyObj };
> @@ -175,9 +174,6 @@
> static RetEffect MakeAlias(unsigned Idx) {
> return RetEffect(Alias, Idx);
> }
> - static RetEffect MakeReceiverAlias() {
> - return RetEffect(ReceiverAlias);
> - }
> static RetEffect MakeOwned(ObjKind o, bool isAllocated = false) {
> return RetEffect(isAllocated ? OwnedAllocatedSymbol : OwnedSymbol, o);
> }
> @@ -1485,30 +1481,29 @@
> getPersistentSummary(RetEffect::MakeOwned(RetEffect::CF, true));
>
> // Create the "retain" selector.
> - RetEffect E = RetEffect::MakeReceiverAlias();
> - RetainSummary *Summ = getPersistentSummary(E, IncRefMsg);
> + RetEffect NoRet = RetEffect::MakeNoRet();
> + RetainSummary *Summ = getPersistentSummary(NoRet, IncRefMsg);
> addNSObjectMethSummary(GetNullarySelector("retain", Ctx), Summ);
>
> // Create the "release" selector.
> - Summ = getPersistentSummary(E, DecRefMsg);
> + Summ = getPersistentSummary(NoRet, DecRefMsg);
> addNSObjectMethSummary(GetNullarySelector("release", Ctx), Summ);
>
> // Create the "drain" selector.
> - Summ = getPersistentSummary(E, isGCEnabled() ? DoNothing : DecRef);
> + Summ = getPersistentSummary(NoRet, isGCEnabled() ? DoNothing : DecRef);
> addNSObjectMethSummary(GetNullarySelector("drain", Ctx), Summ);
>
> // Create the -dealloc summary.
> - Summ = getPersistentSummary(RetEffect::MakeNoRet(), Dealloc);
> + Summ = getPersistentSummary(NoRet, Dealloc);
> addNSObjectMethSummary(GetNullarySelector("dealloc", Ctx), Summ);
>
> // Create the "autorelease" selector.
> - Summ = getPersistentSummary(E, Autorelease);
> + Summ = getPersistentSummary(NoRet, Autorelease);
> addNSObjectMethSummary(GetNullarySelector("autorelease", Ctx), Summ);
>
> // Specially handle NSAutoreleasePool.
> addInstMethSummary("NSAutoreleasePool", "init",
> - getPersistentSummary(RetEffect::MakeReceiverAlias(),
> - NewAutoreleasePool));
> + getPersistentSummary(NoRet, NewAutoreleasePool));
>
> // For NSWindow, allocated objects are (initially) self-owned.
> // FIXME: For now we opt for false negatives with NSWindow, as these objects
> @@ -2738,31 +2733,15 @@
>
> switch (RE.getKind()) {
> default:
> - assert (false && "Unhandled RetEffect."); break;
> -
> - case RetEffect::NoRet: {
> - // Make up a symbol for the return value (not reference counted).
> - // FIXME: Most of this logic is not specific to the retain/release
> - // checker.
> -
> - // FIXME: We eventually should handle structs and other compound types
> - // that are returned by value.
> -
> - // Use the result type from callOrMsg as it automatically adjusts
> - // for methods/functions that return references.
> - QualType resultTy = callOrMsg.getResultType(Eng.getContext());
> - if (Loc::isLocType(resultTy) ||
> - (resultTy->isIntegerType() && resultTy->isScalarType())) {
> - unsigned Count = Builder.getCurrentBlockCount();
> - SValBuilder &svalBuilder = Eng.getSValBuilder();
> - SVal X = svalBuilder.getConjuredSymbolVal(NULL, Ex, resultTy, Count);
> - state = state->BindExpr(Ex, X, false);
> - }
> + llvm_unreachable("Unhandled RetEffect."); break;
>
> + case RetEffect::NoRet:
> + // No work necessary.
> break;
> - }
>
> case RetEffect::Alias: {
> + // FIXME: This should be moved to an eval::call check and limited to the
> + // specific functions that return aliases of their arguments.
> unsigned idx = RE.getIndex();
> assert (idx < callOrMsg.getNumArgs());
> SVal V = callOrMsg.getArgSValAsScalarOrLoc(idx);
> @@ -2770,28 +2749,18 @@
> break;
> }
>
> - case RetEffect::ReceiverAlias: {
> - assert(Receiver);
> - SVal V = Receiver.getSValAsScalarOrLoc(state);
> - state = state->BindExpr(Ex, V, false);
> - break;
> - }
> -
> case RetEffect::OwnedAllocatedSymbol:
> - case RetEffect::OwnedSymbol: {
> - unsigned Count = Builder.getCurrentBlockCount();
> - SValBuilder &svalBuilder = Eng.getSValBuilder();
> - SymbolRef Sym = svalBuilder.getConjuredSymbol(Ex, Count);
> -
> - // Use the result type from callOrMsg as it automatically adjusts
> - // for methods/functions that return references.
> - QualType resultTy = callOrMsg.getResultType(Eng.getContext());
> - state = state->set<RefBindings>(Sym, RefVal::makeOwned(RE.getObjKind(),
> - resultTy));
> - state = state->BindExpr(Ex, svalBuilder.makeLoc(Sym), false);
> + case RetEffect::OwnedSymbol:
> + if (SymbolRef Sym = state->getSVal(Ex).getAsSymbol()) {
> + // Use the result type from callOrMsg as it automatically adjusts
> + // for methods/functions that return references.
> + QualType ResultTy = callOrMsg.getResultType(Eng.getContext());
> + state = state->set<RefBindings>(Sym, RefVal::makeOwned(RE.getObjKind(),
> + ResultTy));
> + }
>
> // FIXME: Add a flag to the checker where allocations are assumed to
> - // *not fail.
> + // *not* fail. (The code below is out-of-date, though.)
> #if 0
> if (RE.getKind() == RetEffect::OwnedAllocatedSymbol) {
> bool isFeasible;
> @@ -2801,18 +2770,17 @@
> #endif
>
> break;
> - }
>
> case RetEffect::GCNotOwnedSymbol:
> case RetEffect::ARCNotOwnedSymbol:
> case RetEffect::NotOwnedSymbol: {
> - unsigned Count = Builder.getCurrentBlockCount();
> - SValBuilder &svalBuilder = Eng.getSValBuilder();
> - SymbolRef Sym = svalBuilder.getConjuredSymbol(Ex, Count);
> - QualType RetT = GetReturnType(Ex, svalBuilder.getContext());
> - state = state->set<RefBindings>(Sym, RefVal::makeNotOwned(RE.getObjKind(),
> - RetT));
> - state = state->BindExpr(Ex, svalBuilder.makeLoc(Sym), false);
> + if (SymbolRef Sym = state->getSVal(Ex).getAsSymbol()) {
> + // Use GetReturnType in order to give [NSFoo alloc] the type NSFoo *.
> + QualType ResultTy = GetReturnType(Ex, Eng.getContext());
> + state =
> + state->set<RefBindings>(Sym, RefVal::makeNotOwned(RE.getObjKind(),
> + ResultTy));
> + }
> break;
> }
> }
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=138220&r1=138219&r2=138220&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Sun Aug 21 14:41:36 2011
> @@ -83,18 +83,40 @@
> Eng.InlineCall(Dst, CE, Pred)) {
> return;
> }
> -
> +
> + // First handle the return value.
> StmtNodeBuilder &Builder = Eng.getBuilder();
> assert(&Builder && "StmtNodeBuilder must be defined.");
> -
> - // Dispatch to the plug-in transfer function.
> - unsigned oldSize = Dst.size();
> - SaveOr OldHasGen(Builder.hasGeneratedNode);
> -
> - // Dispatch to transfer function logic to handle the call itself.
> +
> + // Get the callee.
> const Expr *Callee = CE->getCallee()->IgnoreParens();
> const ProgramState *state = Pred->getState();
> SVal L = state->getSVal(Callee);
> +
> + // Figure out the result type. We do this dance to handle references.
> + QualType ResultTy;
> + if (const FunctionDecl *FD = L.getAsFunctionDecl())
> + ResultTy = FD->getResultType();
> + else
> + ResultTy = CE->getType();
> +
> + if (CE->isLValue())
> + ResultTy = Eng.getContext().getPointerType(ResultTy);
> +
> + // Conjure a symbol value to use as the result.
> + SValBuilder &SVB = Eng.getSValBuilder();
> + unsigned Count = Builder.getCurrentBlockCount();
> + SVal RetVal = SVB.getConjuredSymbolVal(0, CE, ResultTy, Count);
> +
> + // Generate a new ExplodedNode with the return value set.
> + state = state->BindExpr(CE, RetVal);
> + Pred = Builder.generateNode(CE, state, Pred);
> +
> + // Then handle everything else.
> + unsigned oldSize = Dst.size();
> + SaveOr OldHasGen(Builder.hasGeneratedNode);
> +
> + // Dispatch to transfer function logic to handle the rest of the call.
> Eng.getTF().evalCall(Dst, Eng, Builder, CE, L, Pred);
>
> // Handle the case where no nodes where generated. Auto-generate that
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp?rev=138220&r1=138219&r2=138220&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp Sun Aug 21 14:41:36 2011
> @@ -237,3 +237,44 @@
> // the created nodes in 'Dst'.
> getCheckerManager().runCheckersForPostObjCMessage(Dst, dstEval, msg, *this);
> }
> +
> +void ExprEngine::evalObjCMessage(ExplodedNodeSet &Dst, const ObjCMessage &msg,
> + ExplodedNode *Pred,
> + const ProgramState *state) {
> + assert (Builder && "StmtNodeBuilder must be defined.");
> +
> + // First handle the return value.
> + SVal ReturnValue = UnknownVal();
> +
> + // Some method families have known return values.
> + switch (msg.getMethodFamily()) {
> + default:
> + break;
> + case OMF_autorelease:
> + case OMF_retain:
> + case OMF_self: {
> + // These methods return their receivers.
> + // FIXME: Should OMF_init be included here?
> + const Expr *ReceiverE = msg.getInstanceReceiver();
> + if (ReceiverE)
> + ReturnValue = state->getSVal(ReceiverE);
> + break;
> + }
> + }
> +
> + // If we failed to figure out the return value, use a conjured value instead.
> + if (ReturnValue.isUnknown()) {
> + SValBuilder &SVB = getSValBuilder();
> + QualType ResultTy = msg.getResultType(getContext());
> + unsigned Count = Builder->getCurrentBlockCount();
> + const Expr *CurrentE = cast<Expr>(currentStmt);
> + ReturnValue = SVB.getConjuredSymbolVal(NULL, CurrentE, ResultTy, Count);
> + }
> +
> + // Bind the return value.
> + state = state->BindExpr(currentStmt, ReturnValue);
> +
> + // Now we can handle the other aspects of the message.
> + getTF().evalObjCMessage(Dst, *this, *Builder, msg, Pred, state);
> +}
> +
>
>
> _______________________________________________
> 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