From: Ted Kremenek Date: Tue, 21 Jul 2009 21:03:30 +0000 (+0000) Subject: Fix PR 4594 by refactoring almost all casting logic from GRExprEngine::VisitCast X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=32c3fa4195762ba93f0b7114ab36c0941bc34432;p=clang Fix PR 4594 by refactoring almost all casting logic from GRExprEngine::VisitCast to SValuator::EvalCast. In the process, the StoreManagers now use this new cast machinery, and the hack in GRExprEngine::EvalBind to handle implicit casts involving OSAtomicCompareAndSwap and friends has been removed (and replaced with logic closer to the logic specific to those functions). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@76641 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Analysis/PathSensitive/GRExprEngine.h b/include/clang/Analysis/PathSensitive/GRExprEngine.h index 142361b18e..0b3307c315 100644 --- a/include/clang/Analysis/PathSensitive/GRExprEngine.h +++ b/include/clang/Analysis/PathSensitive/GRExprEngine.h @@ -228,6 +228,8 @@ public: /// getCFG - Returns the CFG associated with this analysis. CFG& getCFG() { return G.getCFG(); } + SValuator &getSValuator() { return SVator; } + GRTransferFuncs& getTF() { return *StateMgr.TF; } BugReporter& getBugReporter() { return BR; } @@ -532,11 +534,6 @@ protected: /// VisitCast - Transfer function logic for all casts (implicit and explicit). void VisitCast(Expr* CastE, Expr* Ex, NodeTy* Pred, NodeSet& Dst); - /// VisitCastPointerToInteger - Transfer function (called by VisitCast) that - /// handles pointer to integer casts and array to integer casts. - void VisitCastPointerToInteger(SVal V, const GRState* state, QualType PtrTy, - Expr* CastE, NodeTy* Pred, NodeSet& Dst); - /// VisitCompoundLiteralExpr - Transfer function logic for compound literals. void VisitCompoundLiteralExpr(CompoundLiteralExpr* CL, NodeTy* Pred, NodeSet& Dst, bool asLValue); @@ -600,11 +597,7 @@ protected: /// expressions of the form 'x != 0' and generate new nodes (stored in Dst) /// with those assumptions. void EvalEagerlyAssume(NodeSet& Dst, NodeSet& Src, Expr *Ex); - - SVal EvalCast(SVal X, QualType CastT) { - return SVator.EvalCast(X, CastT); - } - + SVal EvalMinus(SVal X) { return X.isValid() ? SVator.EvalMinus(cast(X)) : X; } diff --git a/include/clang/Analysis/PathSensitive/GRState.h b/include/clang/Analysis/PathSensitive/GRState.h index 0da8f5243e..1551f961cf 100644 --- a/include/clang/Analysis/PathSensitive/GRState.h +++ b/include/clang/Analysis/PathSensitive/GRState.h @@ -450,7 +450,7 @@ public: : EnvMgr(alloc), ISetFactory(alloc), GDMFactory(alloc), - ValueMgr(alloc, Ctx), + ValueMgr(alloc, Ctx, *this), Alloc(alloc), cfg(c), codedecl(cd), @@ -693,11 +693,11 @@ inline SVal GRState::getSValAsScalarOrLoc(const Stmt *S) const { } inline SVal GRState::getSVal(Loc LV, QualType T) const { - return Mgr->StoreMgr->Retrieve(this, LV, T); + return Mgr->StoreMgr->Retrieve(this, LV, T).getSVal(); } inline SVal GRState::getSVal(const MemRegion* R) const { - return Mgr->StoreMgr->Retrieve(this, loc::MemRegionVal(R)); + return Mgr->StoreMgr->Retrieve(this, loc::MemRegionVal(R)).getSVal(); } inline BasicValueFactory &GRState::getBasicVals() const { diff --git a/include/clang/Analysis/PathSensitive/SValuator.h b/include/clang/Analysis/PathSensitive/SValuator.h index 6bc63ac7c9..5a6e6945e7 100644 --- a/include/clang/Analysis/PathSensitive/SValuator.h +++ b/include/clang/Analysis/PathSensitive/SValuator.h @@ -24,24 +24,28 @@ class GRState; class ValueManager; class SValuator { + friend class ValueManager; protected: ValueManager &ValMgr; + + virtual SVal EvalCastNL(NonLoc val, QualType castTy) = 0; + virtual SVal EvalCastL(Loc val, QualType castTy) = 0; + public: SValuator(ValueManager &valMgr) : ValMgr(valMgr) {} virtual ~SValuator() {} - virtual SVal EvalCastNL(NonLoc val, QualType castTy) = 0; - - virtual SVal EvalCastL(Loc val, QualType castTy) = 0; + class CastResult : public std::pair { + public: + const GRState *getState() const { return first; } + SVal getSVal() const { return second; } + CastResult(const GRState *s, SVal v) + : std::pair(s, v) {} + }; - SVal EvalCast(SVal val, QualType castTy) { - if (val.isUnknownOrUndef()) - return val; - - return isa(val) ? EvalCastL(cast(val), castTy) - : EvalCastNL(cast(val), castTy); - } + CastResult EvalCast(SVal val, const GRState *state, + QualType castTy, QualType originalType); virtual SVal EvalMinus(NonLoc val) = 0; diff --git a/include/clang/Analysis/PathSensitive/Store.h b/include/clang/Analysis/PathSensitive/Store.h index da20439e12..37fc641c2e 100644 --- a/include/clang/Analysis/PathSensitive/Store.h +++ b/include/clang/Analysis/PathSensitive/Store.h @@ -61,8 +61,8 @@ public: /// expected type of the returned value. This is used if the value is /// lazily computed. /// \return The value bound to the location \c loc. - virtual SVal Retrieve(const GRState *state, Loc loc, - QualType T = QualType()) = 0; + virtual SValuator::CastResult Retrieve(const GRState *state, Loc loc, + QualType T = QualType()) = 0; /// Return a state with the specified value bound to the given location. /// \param[in] state The analysis state. diff --git a/include/clang/Analysis/PathSensitive/ValueManager.h b/include/clang/Analysis/PathSensitive/ValueManager.h index 329f5fe8a6..9a535b5415 100644 --- a/include/clang/Analysis/PathSensitive/ValueManager.h +++ b/include/clang/Analysis/PathSensitive/ValueManager.h @@ -26,6 +26,9 @@ namespace llvm { class BumpPtrAllocator; } namespace clang { + +class GRStateManager; + class ValueManager { ASTContext &Context; @@ -39,14 +42,17 @@ class ValueManager { MemRegionManager MemMgr; + GRStateManager &StateMgr; + const QualType ArrayIndexTy; const unsigned ArrayIndexWidth; public: - ValueManager(llvm::BumpPtrAllocator &alloc, ASTContext &context) + ValueManager(llvm::BumpPtrAllocator &alloc, ASTContext &context, + GRStateManager &stateMgr) : Context(context), BasicVals(Context, alloc), SymMgr(Context, BasicVals, alloc), - MemMgr(Context, alloc), + MemMgr(Context, alloc), StateMgr(stateMgr), ArrayIndexTy(Context.IntTy), ArrayIndexWidth(Context.getTypeSize(ArrayIndexTy)) { @@ -59,6 +65,8 @@ public: ASTContext &getContext() { return Context; } const ASTContext &getContext() const { return Context; } + GRStateManager &getStateManager() { return StateMgr; } + BasicValueFactory &getBasicValueFactory() { return BasicVals; } const BasicValueFactory &getBasicValueFactory() const { return BasicVals; } diff --git a/lib/Analysis/BasicStore.cpp b/lib/Analysis/BasicStore.cpp index 7aa63c1c63..f276975e9b 100644 --- a/lib/Analysis/BasicStore.cpp +++ b/lib/Analysis/BasicStore.cpp @@ -49,7 +49,8 @@ public: return new BasicStoreSubRegionMap(); } - SVal Retrieve(const GRState *state, Loc loc, QualType T = QualType()); + SValuator::CastResult Retrieve(const GRState *state, Loc loc, + QualType T = QualType()); const GRState *Bind(const GRState *state, Loc L, SVal V) { return state->makeWithStore(BindInternal(state->getStore(), L, V)); @@ -269,10 +270,11 @@ static bool isHigherOrderRawPtr(QualType T, ASTContext &C) { } } -SVal BasicStoreManager::Retrieve(const GRState *state, Loc loc, QualType T) { +SValuator::CastResult BasicStoreManager::Retrieve(const GRState *state, + Loc loc, QualType T) { if (isa(loc)) - return UnknownVal(); + return SValuator::CastResult(state, UnknownVal()); assert (!isa(loc)); @@ -288,7 +290,7 @@ SVal BasicStoreManager::Retrieve(const GRState *state, Loc loc, QualType T) { QualType T = ER->getLocationType(Ctx); if (!isHigherOrderRawPtr(T, Ctx)) - return UnknownVal(); + return SValuator::CastResult(state, UnknownVal()); // FIXME: Should check for element 0. // Otherwise, strip the element region. @@ -296,25 +298,25 @@ SVal BasicStoreManager::Retrieve(const GRState *state, Loc loc, QualType T) { } if (!(isa(R) || isa(R))) - return UnknownVal(); + return SValuator::CastResult(state, UnknownVal()); BindingsTy B = GetBindings(state->getStore()); BindingsTy::data_type* T = B.lookup(R); - return T ? *T : UnknownVal(); + return SValuator::CastResult(state, T ? *T : UnknownVal()); } case loc::ConcreteIntKind: // Some clients may call GetSVal with such an option simply because // they are doing a quick scan through their Locs (potentially to // invalidate their bindings). Just return Undefined. - return UndefinedVal(); + return SValuator::CastResult(state, UndefinedVal()); default: assert (false && "Invalid Loc."); break; } - return UnknownVal(); + return SValuator::CastResult(state, UnknownVal()); } Store BasicStoreManager::BindInternal(Store store, Loc loc, SVal V) { @@ -426,8 +428,8 @@ BasicStoreManager::RemoveDeadBindings(const GRState *state, Stmt* Loc, if (Marked.count(MR)) break; - Marked.insert(MR); - SVal X = Retrieve(state, loc::MemRegionVal(MR)); + Marked.insert(MR); + SVal X = Retrieve(state, loc::MemRegionVal(MR)).getSVal(); // FIXME: We need to handle symbols nested in region definitions. for (symbol_iterator SI=X.symbol_begin(),SE=X.symbol_end();SI!=SE;++SI) diff --git a/lib/Analysis/CMakeLists.txt b/lib/Analysis/CMakeLists.txt index 6c49e8f924..05bbd806ee 100644 --- a/lib/Analysis/CMakeLists.txt +++ b/lib/Analysis/CMakeLists.txt @@ -27,6 +27,7 @@ add_clang_library(clangAnalysis RangeConstraintManager.cpp RegionStore.cpp SVals.cpp + SValuator.cpp SimpleConstraintManager.cpp SimpleSValuator.cpp Store.cpp diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp index 006c3a7d9a..01cc033063 100644 --- a/lib/Analysis/GRExprEngine.cpp +++ b/lib/Analysis/GRExprEngine.cpp @@ -1036,7 +1036,7 @@ void GRExprEngine::VisitMemberExpr(MemberExpr* M, NodeTy* Pred, /// EvalBind - Handle the semantics of binding a value to a specific location. /// This method is used by EvalStore and (soon) VisitDeclStmt, and others. void GRExprEngine::EvalBind(NodeSet& Dst, Expr* Ex, NodeTy* Pred, - const GRState* state, SVal location, SVal Val) { + const GRState* state, SVal location, SVal Val) { const GRState* newState = 0; @@ -1049,24 +1049,7 @@ void GRExprEngine::EvalBind(NodeSet& Dst, Expr* Ex, NodeTy* Pred, else { // We are binding to a value other than 'unknown'. Perform the binding // using the StoreManager. - Loc L = cast(location); - - // Handle implicit casts not reflected in the AST. This can be due to - // custom checker logic such as what handles OSAtomicCompareAndSwap. - if (!Val.isUnknownOrUndef()) - if (const TypedRegion *R = - dyn_cast_or_null(L.getAsRegion())) { - assert(R->isBoundable()); - QualType ValTy = R->getValueType(getContext()); - if (Loc::IsLocType(ValTy)) { - if (!isa(Val)) - Val = SVator.EvalCastNL(cast(Val), ValTy); - } - else if (!isa(Val)) - Val = SVator.EvalCastL(cast(Val), ValTy); - } - - newState = state->bindLoc(L, Val); + newState = state->bindLoc(cast(location), Val); } // The next thing to do is check if the GRTransferFuncs object wants to @@ -1343,8 +1326,18 @@ static bool EvalOSAtomicCompareAndSwap(ExplodedNodeSet& Dst, if (stateEqual) { // Perform the store. ExplodedNodeSet TmpStore; + SVal val = stateEqual->getSVal(newValueExpr); + + // Handle implicit value casts. + if (const TypedRegion *R = + dyn_cast_or_null(location.getAsRegion())) { + llvm::tie(state, val) = + Engine.getSValuator().EvalCast(val, state, R->getValueType(C), + newValueExpr->getType()); + } + Engine.EvalStore(TmpStore, theValueExpr, N, stateEqual, location, - stateEqual->getSVal(newValueExpr), OSAtomicStoreTag); + val, OSAtomicStoreTag); // Now bind the result of the comparison. for (ExplodedNodeSet::iterator I2 = TmpStore.begin(), @@ -2034,22 +2027,6 @@ void GRExprEngine::VisitObjCMessageExprDispatchHelper(ObjCMessageExpr* ME, // Transfer functions: Miscellaneous statements. //===----------------------------------------------------------------------===// -void GRExprEngine::VisitCastPointerToInteger(SVal V, const GRState* state, - QualType PtrTy, - Expr* CastE, NodeTy* Pred, - NodeSet& Dst) { - if (!V.isUnknownOrUndef()) { - // FIXME: Determine if the number of bits of the target type is - // equal or exceeds the number of bits to store the pointer value. - // If not, flag an error. - MakeNode(Dst, CastE, Pred, state->bindExpr(CastE, EvalCast(cast(V), - CastE->getType()))); - } - else - MakeNode(Dst, CastE, Pred, state->bindExpr(CastE, V)); -} - - void GRExprEngine::VisitCast(Expr* CastE, Expr* Ex, NodeTy* Pred, NodeSet& Dst){ NodeSet S1; QualType T = CastE->getType(); @@ -2070,128 +2047,14 @@ void GRExprEngine::VisitCast(Expr* CastE, Expr* Ex, NodeTy* Pred, NodeSet& Dst){ return; } - - // FIXME: The rest of this should probably just go into EvalCall, and - // let the transfer function object be responsible for constructing - // nodes. - + for (NodeSet::iterator I1 = S1.begin(), E1 = S1.end(); I1 != E1; ++I1) { NodeTy* N = *I1; const GRState* state = GetState(N); SVal V = state->getSVal(Ex); - ASTContext& C = getContext(); - - // Unknown? - if (V.isUnknown()) { - Dst.Add(N); - continue; - } - - // Undefined? - if (V.isUndef()) - goto PassThrough; - - // For const casts, just propagate the value. - if (C.getCanonicalType(T).getUnqualifiedType() == - C.getCanonicalType(ExTy).getUnqualifiedType()) - goto PassThrough; - - // Check for casts from pointers to integers. - if (T->isIntegerType() && Loc::IsLocType(ExTy)) { - VisitCastPointerToInteger(V, state, ExTy, CastE, N, Dst); - continue; - } - - // Check for casts from integers to pointers. - if (Loc::IsLocType(T) && ExTy->isIntegerType()) { - if (nonloc::LocAsInteger *LV = dyn_cast(&V)) { - // Just unpackage the lval and return it. - V = LV->getLoc(); - MakeNode(Dst, CastE, N, state->bindExpr(CastE, V)); - continue; - } - - goto DispatchCast; - } - - // Just pass through function and block pointers. - if (ExTy->isBlockPointerType() || ExTy->isFunctionPointerType()) { - assert(Loc::IsLocType(T)); - goto PassThrough; - } - - // Check for casts from array type to another type. - if (ExTy->isArrayType()) { - // We will always decay to a pointer. - V = StateMgr.ArrayToPointer(cast(V)); - - // Are we casting from an array to a pointer? If so just pass on - // the decayed value. - if (T->isPointerType()) - goto PassThrough; - - // Are we casting from an array to an integer? If so, cast the decayed - // pointer value to an integer. - assert(T->isIntegerType()); - QualType ElemTy = cast(ExTy)->getElementType(); - QualType PointerTy = getContext().getPointerType(ElemTy); - VisitCastPointerToInteger(V, state, PointerTy, CastE, N, Dst); - continue; - } - - // Check for casts from a region to a specific type. - if (loc::MemRegionVal *RV = dyn_cast(&V)) { - // FIXME: For TypedViewRegions, we should handle the case where the - // underlying symbolic pointer is a function pointer or - // block pointer. - - // FIXME: We should handle the case where we strip off view layers to get - // to a desugared type. - - assert(Loc::IsLocType(T)); - // We get a symbolic function pointer for a dereference of a function - // pointer, but it is of function type. Example: - - // struct FPRec { - // void (*my_func)(int * x); - // }; - // - // int bar(int x); - // - // int f1_a(struct FPRec* foo) { - // int x; - // (*foo->my_func)(&x); - // return bar(x)+1; // no-warning - // } - - assert(Loc::IsLocType(ExTy) || ExTy->isFunctionType()); - - const MemRegion* R = RV->getRegion(); - StoreManager& StoreMgr = getStoreManager(); - - // Delegate to store manager to get the result of casting a region - // to a different type. - const StoreManager::CastResult& Res = StoreMgr.CastRegion(state, R, T); - - // Inspect the result. If the MemRegion* returned is NULL, this - // expression evaluates to UnknownVal. - R = Res.getRegion(); - if (R) { V = loc::MemRegionVal(R); } else { V = UnknownVal(); } - - // Generate the new node in the ExplodedGraph. - MakeNode(Dst, CastE, N, Res.getState()->bindExpr(CastE, V)); - continue; - } - // All other cases. - DispatchCast: { - MakeNode(Dst, CastE, N, state->bindExpr(CastE, - EvalCast(V, CastE->getType()))); - continue; - } - - PassThrough: { - MakeNode(Dst, CastE, N, state->bindExpr(CastE, V)); - } + const SValuator::CastResult &Res = SVator.EvalCast(V, state, T, ExTy); + state = Res.getState()->bindExpr(CastE, Res.getSVal()); + MakeNode(Dst, CastE, N, state); } } @@ -3034,17 +2897,19 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, // The RHS is not Unknown. // Get the computation type. - QualType CTy = cast(B)->getComputationResultType(); + QualType CTy = + cast(B)->getComputationResultType(); CTy = getContext().getCanonicalType(CTy); - QualType CLHSTy = cast(B)->getComputationLHSType(); - CLHSTy = getContext().getCanonicalType(CTy); + QualType CLHSTy = + cast(B)->getComputationLHSType(); + CLHSTy = getContext().getCanonicalType(CLHSTy); QualType LTy = getContext().getCanonicalType(LHS->getType()); QualType RTy = getContext().getCanonicalType(RHS->getType()); // Promote LHS. - V = EvalCast(V, CLHSTy); + llvm::tie(state, V) = SVator.EvalCast(V, state, CLHSTy, LTy); // Evaluate operands and promote to result type. if (RightV.isUndef()) { @@ -3055,8 +2920,10 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, } // Compute the result of the operation. - SVal Result = EvalCast(EvalBinOp(state, Op, V, RightV, CTy), - B->getType()); + SVal Result; + llvm::tie(state, Result) = SVator.EvalCast(EvalBinOp(state, Op, V, + RightV, CTy), + state, B->getType(), CTy); if (Result.isUndef()) { // The operands were not undefined, but the result is undefined. @@ -3085,12 +2952,12 @@ void GRExprEngine::VisitBinaryOperator(BinaryOperator* B, LHSVal = ValMgr.getConjuredSymbolVal(B->getRHS(), LTy, Count); // However, we need to convert the symbol to the computation type. - Result = (LTy == CTy) ? LHSVal : EvalCast(LHSVal,CTy); + llvm::tie(state, Result) = SVator.EvalCast(LHSVal, state, CTy, LTy); } else { // The left-hand side may bind to a different value then the // computation type. - LHSVal = (LTy == CTy) ? Result : EvalCast(Result,LTy); + llvm::tie(state, LHSVal) = SVator.EvalCast(Result, state, LTy, CTy); } EvalStore(Dst, B, LHS, *I3, state->bindExpr(B, Result), location, diff --git a/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp index 2ea2b88df2..dc54c8fa39 100644 --- a/lib/Analysis/RegionStore.cpp +++ b/lib/Analysis/RegionStore.cpp @@ -279,7 +279,8 @@ public: /// return undefined /// else /// return symbolic - SVal Retrieve(const GRState *state, Loc L, QualType T = QualType()); + SValuator::CastResult Retrieve(const GRState *state, Loc L, + QualType T = QualType()); SVal RetrieveElement(const GRState *state, const ElementRegion *R); @@ -291,7 +292,8 @@ public: SVal RetrieveLazySymbol(const GRState *state, const TypedRegion *R); - SVal CastRetrievedVal(SVal val, const TypedRegion *R, QualType castTy); + SValuator::CastResult CastRetrievedVal(SVal val, const GRState *state, + const TypedRegion *R, QualType castTy); /// Retrieve the values in a struct and return a CompoundVal, used when doing /// struct copy: @@ -780,7 +782,8 @@ static bool IsReinterpreted(QualType RTy, QualType UsedTy, ASTContext &Ctx) { return true; } -SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { +SValuator::CastResult +RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { assert(!isa(L) && "location unknown"); assert(!isa(L) && "location undefined"); @@ -788,7 +791,7 @@ SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { // FIXME: Is this even possible? Shouldn't this be treated as a null // dereference at a higher level? if (isa(L)) - return UndefinedVal(); + return SValuator::CastResult(state, UndefinedVal()); const MemRegion *MR = cast(L).getRegion(); @@ -799,7 +802,7 @@ SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { // read(p); // c = *p; if (isa(MR)) - return UnknownVal(); + return SValuator::CastResult(state, UnknownVal()); if (isa(MR)) { ASTContext &Ctx = getContext(); @@ -832,33 +835,33 @@ SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { } if (RTy->isStructureType()) - return RetrieveStruct(state, R); + return SValuator::CastResult(state, RetrieveStruct(state, R)); if (RTy->isArrayType()) - return RetrieveArray(state, R); + return SValuator::CastResult(state, RetrieveArray(state, R)); // FIXME: handle Vector types. if (RTy->isVectorType()) - return UnknownVal(); + return SValuator::CastResult(state, UnknownVal()); if (const FieldRegion* FR = dyn_cast(R)) - return CastRetrievedVal(RetrieveField(state, FR), FR, T); + return CastRetrievedVal(RetrieveField(state, FR), state, FR, T); if (const ElementRegion* ER = dyn_cast(R)) - return CastRetrievedVal(RetrieveElement(state, ER), ER, T); + return CastRetrievedVal(RetrieveElement(state, ER), state, ER, T); if (const ObjCIvarRegion *IVR = dyn_cast(R)) - return CastRetrievedVal(RetrieveObjCIvar(state, IVR), IVR, T); + return CastRetrievedVal(RetrieveObjCIvar(state, IVR), state, IVR, T); if (const VarRegion *VR = dyn_cast(R)) - return CastRetrievedVal(RetrieveVar(state, VR), VR, T); + return CastRetrievedVal(RetrieveVar(state, VR), state, VR, T); RegionBindingsTy B = GetRegionBindings(state->getStore()); RegionBindingsTy::data_type* V = B.lookup(R); // Check if the region has a binding. if (V) - return *V; + return SValuator::CastResult(state, *V); // The location does not have a bound value. This means that it has // the value it had upon its creation and/or entry to the analyzed @@ -870,7 +873,7 @@ SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { // upon creation. All heap allocated blocks are considered to // have undefined values as well unless they are explicitly bound // to specific values. - return UndefinedVal(); + return SValuator::CastResult(state, UndefinedVal()); } // If the region is already cast to another type, use that type to create the @@ -881,7 +884,8 @@ SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { } // All other values are symbolic. - return ValMgr.getRegionValueSymbolValOrUnknown(R, RTy); + return SValuator::CastResult(state, + ValMgr.getRegionValueSymbolValOrUnknown(R, RTy)); } SVal RegionStoreManager::RetrieveElement(const GRState* state, @@ -1079,7 +1083,7 @@ SVal RegionStoreManager::RetrieveStruct(const GRState *state, Field != FieldEnd; ++Field) { FieldRegion* FR = MRMgr.getFieldRegion(*Field, R); QualType FTy = (*Field)->getType(); - SVal FieldValue = Retrieve(state, loc::MemRegionVal(FR), FTy); + SVal FieldValue = Retrieve(state, loc::MemRegionVal(FR), FTy).getSVal(); StructVal = getBasicVals().consVals(FieldValue, StructVal); } @@ -1099,28 +1103,22 @@ SVal RegionStoreManager::RetrieveArray(const GRState *state, ElementRegion* ER = MRMgr.getElementRegion(CAT->getElementType(), Idx, R, getContext()); QualType ETy = ER->getElementType(); - SVal ElementVal = Retrieve(state, loc::MemRegionVal(ER), ETy); + SVal ElementVal = Retrieve(state, loc::MemRegionVal(ER), ETy).getSVal(); ArrayVal = getBasicVals().consVals(ElementVal, ArrayVal); } return ValMgr.makeCompoundVal(T, ArrayVal); } -SVal RegionStoreManager::CastRetrievedVal(SVal V, const TypedRegion *R, - QualType castTy) { - +SValuator::CastResult RegionStoreManager::CastRetrievedVal(SVal V, + const GRState *state, + const TypedRegion *R, + QualType castTy) { if (castTy.isNull()) - return V; + return SValuator::CastResult(state, V); ASTContext &Ctx = getContext(); - QualType valTy = R->getValueType(Ctx); - castTy = Ctx.getCanonicalType(castTy); - - - if (valTy == castTy) - return V; - - return ValMgr.getSValuator().EvalCast(V, castTy); + return ValMgr.getSValuator().EvalCast(V, state, castTy, R->getValueType(Ctx)); } //===----------------------------------------------------------------------===// diff --git a/lib/Analysis/SValuator.cpp b/lib/Analysis/SValuator.cpp new file mode 100644 index 0000000000..b06b94f321 --- /dev/null +++ b/lib/Analysis/SValuator.cpp @@ -0,0 +1,123 @@ +// SValuator.cpp - Basic class for all SValuator implementations --*- C++ -*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines SValuator, the base class for all (complete) SValuator +// implementations. +// +//===----------------------------------------------------------------------===// + +#include "clang/Analysis/PathSensitive/SValuator.h" +#include "clang/Analysis/PathSensitive/GRState.h" + +using namespace clang; + +SValuator::CastResult SValuator::EvalCast(SVal val, const GRState *state, + QualType castTy, QualType originalTy){ + + if (val.isUnknownOrUndef() || castTy == originalTy) + return CastResult(state, val); + + ASTContext &C = ValMgr.getContext(); + + // For const casts, just propagate the value. + if (C.getCanonicalType(castTy).getUnqualifiedType() == + C.getCanonicalType(originalTy).getUnqualifiedType()) + return CastResult(state, val); + + // Check for casts from pointers to integers. + if (castTy->isIntegerType() && Loc::IsLocType(originalTy)) + return CastResult(state, EvalCastL(cast(val), castTy)); + + // Check for casts from integers to pointers. + if (Loc::IsLocType(castTy) && originalTy->isIntegerType()) { + if (nonloc::LocAsInteger *LV = dyn_cast(&val)) { + // Just unpackage the lval and return it. + return CastResult(state, LV->getLoc()); + } + + goto DispatchCast; + } + + // Just pass through function and block pointers. + if (originalTy->isBlockPointerType() || originalTy->isFunctionPointerType()) { + assert(Loc::IsLocType(castTy)); + return CastResult(state, val); + } + + // Check for casts from array type to another type. + if (originalTy->isArrayType()) { + // We will always decay to a pointer. + val = ValMgr.getStateManager().ArrayToPointer(cast(val)); + + // Are we casting from an array to a pointer? If so just pass on + // the decayed value. + if (castTy->isPointerType()) + return CastResult(state, val); + + // Are we casting from an array to an integer? If so, cast the decayed + // pointer value to an integer. + assert(castTy->isIntegerType()); + + // FIXME: Keep these here for now in case we decide soon that we + // need the original decayed type. + // QualType elemTy = cast(originalTy)->getElementType(); + // QualType pointerTy = C.getPointerType(elemTy); + return CastResult(state, EvalCastL(cast(val), castTy)); + } + + // Check for casts from a region to a specific type. + if (const MemRegion *R = val.getAsRegion()) { + // FIXME: For TypedViewRegions, we should handle the case where the + // underlying symbolic pointer is a function pointer or + // block pointer. + + // FIXME: We should handle the case where we strip off view layers to get + // to a desugared type. + + assert(Loc::IsLocType(castTy)); + // We get a symbolic function pointer for a dereference of a function + // pointer, but it is of function type. Example: + + // struct FPRec { + // void (*my_func)(int * x); + // }; + // + // int bar(int x); + // + // int f1_a(struct FPRec* foo) { + // int x; + // (*foo->my_func)(&x); + // return bar(x)+1; // no-warning + // } + + assert(Loc::IsLocType(originalTy) || originalTy->isFunctionType() || + originalTy->isBlockPointerType()); + + StoreManager &storeMgr = ValMgr.getStateManager().getStoreManager(); + + // Delegate to store manager to get the result of casting a region + // to a different type. + const StoreManager::CastResult& Res = storeMgr.CastRegion(state, R, castTy); + + // Inspect the result. If the MemRegion* returned is NULL, this + // expression evaluates to UnknownVal. + R = Res.getRegion(); + + if (R) + return CastResult(Res.getState(), loc::MemRegionVal(R)); + + return CastResult(Res.getState(), UnknownVal()); + } + + // All other cases. +DispatchCast: + return CastResult(state, + isa(val) ? EvalCastL(cast(val), castTy) + : EvalCastNL(cast(val), castTy)); +} \ No newline at end of file diff --git a/lib/Analysis/SimpleSValuator.cpp b/lib/Analysis/SimpleSValuator.cpp index 7462fe65fe..9850b2e036 100644 --- a/lib/Analysis/SimpleSValuator.cpp +++ b/lib/Analysis/SimpleSValuator.cpp @@ -19,12 +19,14 @@ using namespace clang; namespace { class VISIBILITY_HIDDEN SimpleSValuator : public SValuator { +protected: + virtual SVal EvalCastNL(NonLoc val, QualType castTy); + virtual SVal EvalCastL(Loc val, QualType castTy); + public: SimpleSValuator(ValueManager &valMgr) : SValuator(valMgr) {} virtual ~SimpleSValuator() {} - virtual SVal EvalCastNL(NonLoc val, QualType castTy); - virtual SVal EvalCastL(Loc val, QualType castTy); virtual SVal EvalMinus(NonLoc val); virtual SVal EvalComplement(NonLoc val); virtual SVal EvalBinOpNN(BinaryOperator::Opcode op, NonLoc lhs, NonLoc rhs, diff --git a/lib/Analysis/ValueManager.cpp b/lib/Analysis/ValueManager.cpp index 658dfa1186..ee6b5cbeeb 100644 --- a/lib/Analysis/ValueManager.cpp +++ b/lib/Analysis/ValueManager.cpp @@ -56,6 +56,9 @@ NonLoc ValueManager::makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op, SVal ValueManager::convertToArrayIndex(SVal V) { + if (V.isUnknownOrUndef()) + return V; + // Common case: we have an appropriately sized integer. if (nonloc::ConcreteInt* CI = dyn_cast(&V)) { const llvm::APSInt& I = CI->getValue(); @@ -63,7 +66,7 @@ SVal ValueManager::convertToArrayIndex(SVal V) { return V; } - return SVator->EvalCast(V, ArrayIndexTy); + return SVator->EvalCastNL(cast(V), ArrayIndexTy); } SVal ValueManager::getRegionValueSymbolVal(const MemRegion* R, QualType T) { diff --git a/test/Analysis/misc-ps.m b/test/Analysis/misc-ps.m index 1eef2ba968..de39a0004b 100644 --- a/test/Analysis/misc-ps.m +++ b/test/Analysis/misc-ps.m @@ -437,7 +437,6 @@ void test_block_cast() { (void (^)(void *))test_block_cast_aux(); // expected-warning{{expression result unused}} } -// ** THIS TEST FAILS ** // Test comparison of 'id' instance variable to a null void* constant after // performing an OSAtomicCompareAndSwap32Barrier. // This previously was a crash in RegionStoreManager. @@ -454,5 +453,9 @@ void test_block_cast() { } @end - - +// PR 4594 - This was a crash when handling casts in SimpleSValuator. +void PR4594() { + char *buf[1]; + char **foo = buf; + *foo = "test"; +}