diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h @@ -280,12 +280,6 @@ QualType pointeeTy, uint64_t index = 0); - /// CastRetrievedVal - Used by subclasses of StoreManager to implement - /// implicit casts that arise from loads from regions that are reinterpreted - /// as another region. - SVal CastRetrievedVal(SVal val, const TypedValueRegion *region, - QualType castTy); - private: SVal getLValueFieldOrIvar(const Decl *decl, SVal base); }; diff --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp --- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp +++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp @@ -1478,7 +1478,7 @@ return UnknownVal(); if (const FieldRegion* FR = dyn_cast(R)) - return CastRetrievedVal(getBindingForField(B, FR), FR, T); + return svalBuilder.evalCast(getBindingForField(B, FR), T, QualType{}); if (const ElementRegion* ER = dyn_cast(R)) { // FIXME: Here we actually perform an implicit conversion from the loaded @@ -1486,7 +1486,7 @@ // more intelligently. For example, an 'element' can encompass multiple // bound regions (e.g., several bound bytes), or could be a subset of // a larger value. - return CastRetrievedVal(getBindingForElement(B, ER), ER, T); + return svalBuilder.evalCast(getBindingForElement(B, ER), T, QualType{}); } if (const ObjCIvarRegion *IVR = dyn_cast(R)) { @@ -1496,7 +1496,7 @@ // reinterpretted, it is possible we stored a different value that could // fit within the ivar. Either we need to cast these when storing them // or reinterpret them lazily (as we do here). - return CastRetrievedVal(getBindingForObjCIvar(B, IVR), IVR, T); + return svalBuilder.evalCast(getBindingForObjCIvar(B, IVR), T, QualType{}); } if (const VarRegion *VR = dyn_cast(R)) { @@ -1506,7 +1506,7 @@ // variable is reinterpretted, it is possible we stored a different value // that could fit within the variable. Either we need to cast these when // storing them or reinterpret them lazily (as we do here). - return CastRetrievedVal(getBindingForVar(B, VR), VR, T); + return svalBuilder.evalCast(getBindingForVar(B, VR), T, QualType{}); } const SVal *V = B.lookup(R, BindingKey::Direct); diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp --- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -544,20 +544,39 @@ // `evalCastKind` and `evalCastSubKind` are helpers //===----------------------------------------------------------------------===// +/// Cast a given SVal to another SVal using given QualType's. +/// \param V -- SVal that should be casted. +/// \param CastTy -- QualType that V should be casted according to. +/// \param OriginalTy -- QualType which is associated to V. It provides +/// additional information about what type the cast performs from. +/// \returns the most appropriate casted SVal. +/// Note: Many cases don't use an exact OriginalTy. It can be extracted +/// from SVal or the cast can performs unconditionaly. Always pass OriginalTy! +/// It can be crucial in certain cases and generates different results. +/// FIXME: If `OriginalTy.isNull()` is true, then cast performs based on CastTy +/// only. This behavior is uncertain and should be improved. SVal SValBuilder::evalCast(SVal V, QualType CastTy, QualType OriginalTy) { - CastTy = Context.getCanonicalType(CastTy); - OriginalTy = Context.getCanonicalType(OriginalTy); - if (CastTy == OriginalTy) + if (CastTy.isNull()) return V; - // FIXME: Move this check to the most appropriate evalCastKind/evalCastSubKind - // function. - // For const casts, casts to void, just propagate the value. - if (!CastTy->isVariableArrayType() && !OriginalTy->isVariableArrayType()) - if (shouldBeModeledWithNoOp(Context, Context.getPointerType(CastTy), - Context.getPointerType(OriginalTy))) + CastTy = Context.getCanonicalType(CastTy); + + const bool IsUnknownOriginalType = OriginalTy.isNull(); + if (!IsUnknownOriginalType) { + OriginalTy = Context.getCanonicalType(OriginalTy); + + if (CastTy == OriginalTy) return V; + // FIXME: Move this check to the most appropriate + // evalCastKind/evalCastSubKind function. For const casts, casts to void, + // just propagate the value. + if (!CastTy->isVariableArrayType() && !OriginalTy->isVariableArrayType()) + if (shouldBeModeledWithNoOp(Context, Context.getPointerType(CastTy), + Context.getPointerType(OriginalTy))) + return V; + } + // Cast SVal according to kinds. switch (V.getBaseKind()) { case SVal::UndefinedValKind: @@ -591,9 +610,9 @@ return evalCastSubKind(V.castAs(), CastTy, OriginalTy); case loc::MemRegionValKind: return evalCastSubKind(V.castAs(), CastTy, OriginalTy); - default: - llvm_unreachable("Unknown SVal kind"); } + + llvm_unreachable("Unknown SVal kind"); } SVal SValBuilder::evalCastKind(NonLoc V, QualType CastTy, QualType OriginalTy) { @@ -613,9 +632,9 @@ case nonloc::PointerToMemberKind: return evalCastSubKind(V.castAs(), CastTy, OriginalTy); - default: - llvm_unreachable("Unknown SVal kind"); } + + llvm_unreachable("Unknown SVal kind"); } SVal SValBuilder::evalCastSubKind(loc::ConcreteInt V, QualType CastTy, @@ -652,10 +671,13 @@ return makeLocAsInteger(V, BitWidth); } - // Array to pointer. - if (isa(OriginalTy)) - if (CastTy->isPointerType() || CastTy->isReferenceType()) - return UnknownVal(); + const bool IsUnknownOriginalType = OriginalTy.isNull(); + if (!IsUnknownOriginalType) { + // Array to pointer. + if (isa(OriginalTy)) + if (CastTy->isPointerType() || CastTy->isReferenceType()) + return UnknownVal(); + } // Pointer to any pointer. if (Loc::isLocType(CastTy)) @@ -665,6 +687,11 @@ return UnknownVal(); } +static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) { + return ty1->getPointeeType().getCanonicalType().getTypePtr() == + ty2->getPointeeType().getCanonicalType().getTypePtr(); +} + SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, QualType CastTy, QualType OriginalTy) { // Pointer to bool. @@ -685,8 +712,12 @@ return makeTruthVal(true, CastTy); } + const bool IsUnknownOriginalType = OriginalTy.isNull(); // Try to cast to array - const auto *ArrayTy = dyn_cast(OriginalTy.getCanonicalType()); + const auto *ArrayTy = + IsUnknownOriginalType + ? nullptr + : dyn_cast(OriginalTy.getCanonicalType()); // Pointer to integer. if (CastTy->isIntegralOrEnumerationType()) { @@ -707,6 +738,29 @@ // Pointer to pointer. if (Loc::isLocType(CastTy)) { + + if (IsUnknownOriginalType) { + // When retrieving symbolic pointer and expecting a non-void pointer, + // wrap them into element regions of the expected type if necessary. + // It is necessary to make sure that the retrieved value makes sense, + // because there's no other cast in the AST that would tell us to cast + // it to the correct pointer type. We might need to do that for non-void + // pointers as well. + // FIXME: We really need a single good function to perform casts for us + // correctly every time we need it. + if (CastTy->isPointerType() && !CastTy->isVoidPointerType()) { + const MemRegion *R = V.getRegion(); + if (const auto *SR = dyn_cast(R)) { + QualType SRTy = SR->getSymbol()->getType(); + if (!hasSameUnqualifiedPointeeType(SRTy, CastTy)) { + R = StateMgr.getStoreManager().castRegion(SR, CastTy); + return loc::MemRegionVal(R); + } + } + } + return V; + } + if (OriginalTy->isIntegralOrEnumerationType() || OriginalTy->isBlockPointerType() || OriginalTy->isFunctionPointerType()) return V; @@ -807,7 +861,10 @@ // Pass to Loc function. return evalCastKind(L, CastTy, OriginalTy); - if (Loc::isLocType(CastTy) && OriginalTy->isIntegralOrEnumerationType()) { + const bool IsUnknownOriginalType = OriginalTy.isNull(); + // Pointer as integer to pointer. + if (!IsUnknownOriginalType && Loc::isLocType(CastTy) && + OriginalTy->isIntegralOrEnumerationType()) { if (const MemRegion *R = L.getAsRegion()) if ((R = StateMgr.getStoreManager().castRegion(R, CastTy))) return loc::MemRegionVal(R); @@ -815,9 +872,9 @@ } // Pointer as integer with region to integer/pointer. - if (const MemRegion *R = L.getAsRegion()) { + const MemRegion *R = L.getAsRegion(); + if (!IsUnknownOriginalType && R) { if (CastTy->isIntegralOrEnumerationType()) - // Pass to MemRegion function. return evalCastSubKind(loc::MemRegionVal(R), CastTy, OriginalTy); if (Loc::isLocType(CastTy)) { @@ -830,15 +887,28 @@ return loc::MemRegionVal(R); } } else { - if (Loc::isLocType(CastTy)) + if (Loc::isLocType(CastTy)) { + if (IsUnknownOriginalType) + return evalCastSubKind(loc::MemRegionVal(R), CastTy, OriginalTy); return L; + } - // FIXME: Correctly support promotions/truncations. - const unsigned CastSize = Context.getIntWidth(CastTy); - if (CastSize == V.getNumBits()) - return V; + SymbolRef SE = nullptr; + if (R) { + if (const SymbolicRegion *SR = + dyn_cast(R->StripCasts())) { + SE = SR->getSymbol(); + } + } - return makeLocAsInteger(L, CastSize); + if (!CastTy->isFloatingType() || !SE || SE->getType()->isFloatingType()) { + // FIXME: Correctly support promotions/truncations. + const unsigned CastSize = Context.getIntWidth(CastTy); + if (CastSize == V.getNumBits()) + return V; + + return makeLocAsInteger(L, CastSize); + } } // Pointer as integer to whatever else. @@ -849,13 +919,13 @@ QualType OriginalTy) { SymbolRef SE = V.getSymbol(); + const bool IsUnknownOriginalType = OriginalTy.isNull(); // Symbol to bool. - if (CastTy->isBooleanType()) { + if (!IsUnknownOriginalType && CastTy->isBooleanType()) { // Non-float to bool. if (Loc::isLocType(OriginalTy) || OriginalTy->isIntegralOrEnumerationType() || OriginalTy->isMemberPointerType()) { - SymbolRef SE = V.getSymbol(); BasicValueFactory &BVF = getBasicValueFactory(); return makeNonLoc(SE, BO_NE, BVF.getValue(0, SE->getType()), CastTy); } @@ -872,7 +942,9 @@ if (haveSameType(T, CastTy)) return V; if (!Loc::isLocType(CastTy)) - return makeNonLoc(SE, T, CastTy); + if (!IsUnknownOriginalType || !CastTy->isFloatingType() || + T->isFloatingType()) + return makeNonLoc(SE, T, CastTy); } // Symbol to pointer and whatever else. diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp --- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -65,12 +65,12 @@ // Transfer function for Casts. //===----------------------------------------------------------------------===// +// FIXME: This function should be eliminated and replaced with `evalCast` SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) { - assert(Val.getAs() || Val.getAs()); - return Val.getAs() ? evalCastFromLoc(Val.castAs(), CastTy) - : evalCastFromNonLoc(Val.castAs(), CastTy); + return evalCast(Val, CastTy, QualType{}); } +// FIXME: This function should be eliminated and replaced with `evalCast` SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) { bool isLocType = Loc::isLocType(castTy); if (val.getAs()) @@ -127,6 +127,7 @@ return makeIntVal(i); } +// FIXME: This function should be eliminated and replaced with `evalCast` SVal SimpleSValBuilder::evalCastFromLoc(Loc val, QualType castTy) { // Casts from pointers -> pointers, just return the lval. diff --git a/clang/lib/StaticAnalyzer/Core/Store.cpp b/clang/lib/StaticAnalyzer/Core/Store.cpp --- a/clang/lib/StaticAnalyzer/Core/Store.cpp +++ b/clang/lib/StaticAnalyzer/Core/Store.cpp @@ -394,48 +394,6 @@ return UnknownVal(); } -static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) { - return ty1->getPointeeType().getCanonicalType().getTypePtr() == - ty2->getPointeeType().getCanonicalType().getTypePtr(); -} - -/// CastRetrievedVal - Used by subclasses of StoreManager to implement -/// implicit casts that arise from loads from regions that are reinterpreted -/// as another region. -SVal StoreManager::CastRetrievedVal(SVal V, const TypedValueRegion *R, - QualType castTy) { - if (castTy.isNull() || V.isUnknownOrUndef()) - return V; - - // The dispatchCast() call below would convert the int into a float. - // What we want, however, is a bit-by-bit reinterpretation of the int - // as a float, which usually yields nothing garbage. For now skip casts - // from ints to floats. - // TODO: What other combinations of types are affected? - if (castTy->isFloatingType()) { - SymbolRef Sym = V.getAsSymbol(); - if (Sym && !Sym->getType()->isFloatingType()) - return UnknownVal(); - } - - // When retrieving symbolic pointer and expecting a non-void pointer, - // wrap them into element regions of the expected type if necessary. - // SValBuilder::dispatchCast() doesn't do that, but it is necessary to - // make sure that the retrieved value makes sense, because there's no other - // cast in the AST that would tell us to cast it to the correct pointer type. - // We might need to do that for non-void pointers as well. - // FIXME: We really need a single good function to perform casts for us - // correctly every time we need it. - if (castTy->isPointerType() && !castTy->isVoidPointerType()) - if (const auto *SR = dyn_cast_or_null(V.getAsRegion())) { - QualType sr = SR->getSymbol()->getType(); - if (!hasSameUnqualifiedPointeeType(sr, castTy)) - return loc::MemRegionVal(castRegion(SR, castTy)); - } - - return svalBuilder.dispatchCast(V, castTy); -} - SVal StoreManager::getLValueFieldOrIvar(const Decl *D, SVal Base) { if (Base.isUnknownOrUndef()) return Base;