[clang] 7736b08 - [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast
Denys Petrov via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 13 08:10:15 PDT 2021
Author: Denys Petrov
Date: 2021-04-13T18:10:06+03:00
New Revision: 7736b08c287274361f2cdf13512015708af4d335
URL: https://github.com/llvm/llvm-project/commit/7736b08c287274361f2cdf13512015708af4d335
DIFF: https://github.com/llvm/llvm-project/commit/7736b08c287274361f2cdf13512015708af4d335.diff
LOG: [analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::evalCast
Summary: Move logic from CastRetrievedVal to evalCast and replace CastRetrievedVal with evalCast. Also move guts from SimpleSValBuilder::dispatchCast inside evalCast.
evalCast intends to substitute dispatchCast, evalCastFromNonLoc and evalCastFromLoc in the future. OriginalTy provides additional information for casting, which is useful for some cases and useless for others. If `OriginalTy.isNull()` is true, then cast performs based on CastTy only. Now evalCast operates in two ways. It retains all previous behavior and take over dispatchCast behavior. dispatchCast, evalCastFromNonLoc and evalCastFromLoc is considered as buggy since it doesn't take into account OriginalTy of the SVal and should be improved.
>From this patch use evalCast instead of dispatchCast, evalCastFromNonLoc and evalCastFromLoc functions. dispatchCast redirects to evalCast.
This patch shall not change any behavior.
Differential Revision: https://reviews.llvm.org/D96090
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
clang/lib/StaticAnalyzer/Core/Store.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
index c3b590e4784e4..947913ae4eee9 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -280,12 +280,6 @@ class StoreManager {
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
index 5acc51674e9a6..4ffa1aacb41fa 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1478,7 +1478,7 @@ SVal RegionStoreManager::getBinding(RegionBindingsConstRef B, Loc L, QualType T)
return UnknownVal();
if (const FieldRegion* FR = dyn_cast<FieldRegion>(R))
- return CastRetrievedVal(getBindingForField(B, FR), FR, T);
+ return svalBuilder.evalCast(getBindingForField(B, FR), T, QualType{});
if (const ElementRegion* ER = dyn_cast<ElementRegion>(R)) {
// FIXME: Here we actually perform an implicit conversion from the loaded
@@ -1486,7 +1486,7 @@ SVal RegionStoreManager::getBinding(RegionBindingsConstRef B, Loc L, QualType T)
// 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<ObjCIvarRegion>(R)) {
@@ -1496,7 +1496,7 @@ SVal RegionStoreManager::getBinding(RegionBindingsConstRef B, Loc L, QualType T)
// reinterpretted, it is possible we stored a
diff erent 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<VarRegion>(R)) {
@@ -1506,7 +1506,7 @@ SVal RegionStoreManager::getBinding(RegionBindingsConstRef B, Loc L, QualType T)
// variable is reinterpretted, it is possible we stored a
diff erent 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
index 9942b7e1423cf..9a9edb25f2d77 100644
--- a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -544,20 +544,39 @@ SVal SValBuilder::evalIntegralCast(ProgramStateRef state, SVal val,
// `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
diff erent 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 @@ SVal SValBuilder::evalCastKind(Loc V, QualType CastTy, QualType OriginalTy) {
return evalCastSubKind(V.castAs<loc::GotoLabel>(), CastTy, OriginalTy);
case loc::MemRegionValKind:
return evalCastSubKind(V.castAs<loc::MemRegionVal>(), 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 @@ SVal SValBuilder::evalCastKind(NonLoc V, QualType CastTy, QualType OriginalTy) {
case nonloc::PointerToMemberKind:
return evalCastSubKind(V.castAs<nonloc::PointerToMember>(), 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 @@ SVal SValBuilder::evalCastSubKind(loc::GotoLabel V, QualType CastTy,
return makeLocAsInteger(V, BitWidth);
}
- // Array to pointer.
- if (isa<ArrayType>(OriginalTy))
- if (CastTy->isPointerType() || CastTy->isReferenceType())
- return UnknownVal();
+ const bool IsUnknownOriginalType = OriginalTy.isNull();
+ if (!IsUnknownOriginalType) {
+ // Array to pointer.
+ if (isa<ArrayType>(OriginalTy))
+ if (CastTy->isPointerType() || CastTy->isReferenceType())
+ return UnknownVal();
+ }
// Pointer to any pointer.
if (Loc::isLocType(CastTy))
@@ -665,6 +687,11 @@ SVal SValBuilder::evalCastSubKind(loc::GotoLabel V, QualType CastTy,
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 @@ SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, QualType CastTy,
return makeTruthVal(true, CastTy);
}
+ const bool IsUnknownOriginalType = OriginalTy.isNull();
// Try to cast to array
- const auto *ArrayTy = dyn_cast<ArrayType>(OriginalTy.getCanonicalType());
+ const auto *ArrayTy =
+ IsUnknownOriginalType
+ ? nullptr
+ : dyn_cast<ArrayType>(OriginalTy.getCanonicalType());
// Pointer to integer.
if (CastTy->isIntegralOrEnumerationType()) {
@@ -707,6 +738,29 @@ SVal SValBuilder::evalCastSubKind(loc::MemRegionVal V, QualType CastTy,
// 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<SymbolicRegion>(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 @@ SVal SValBuilder::evalCastSubKind(nonloc::LocAsInteger V, QualType CastTy,
// 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 @@ SVal SValBuilder::evalCastSubKind(nonloc::LocAsInteger V, QualType CastTy,
}
// 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 @@ SVal SValBuilder::evalCastSubKind(nonloc::LocAsInteger V, QualType CastTy,
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<SymbolicRegion>(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 @@ SVal SValBuilder::evalCastSubKind(nonloc::SymbolVal V, QualType CastTy,
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 @@ SVal SValBuilder::evalCastSubKind(nonloc::SymbolVal V, QualType CastTy,
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
index 872616fedb4eb..7fe2374c99148 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -65,12 +65,12 @@ SValBuilder *ento::createSimpleSValBuilder(llvm::BumpPtrAllocator &alloc,
// Transfer function for Casts.
//===----------------------------------------------------------------------===//
+// FIXME: This function should be eliminated and replaced with `evalCast`
SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) {
- assert(Val.getAs<Loc>() || Val.getAs<NonLoc>());
- return Val.getAs<Loc>() ? evalCastFromLoc(Val.castAs<Loc>(), CastTy)
- : evalCastFromNonLoc(Val.castAs<NonLoc>(), 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<nonloc::PointerToMember>())
@@ -127,6 +127,7 @@ SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
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
index ea617bbeeba1a..c563b44efc13e 100644
--- a/clang/lib/StaticAnalyzer/Core/Store.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -394,48 +394,6 @@ SVal StoreManager::attemptDownCast(SVal Base, QualType TargetType,
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<SymbolicRegion>(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;
More information about the cfe-commits
mailing list