Skip to content

Commit

Permalink
[analyzer] Replace StoreManager::CastRetrievedVal with SValBuilder::e…
Browse files Browse the repository at this point in the history 
…valCast

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
  • Loading branch information
@ASDenysPetrov
ASDenysPetrov committed Apr 13, 2021
1 parent 682d1df commit 7736b08
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 85 deletions.
6 changes: 0 additions & 6 deletions clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down
8 changes: 4 additions & 4 deletions clang/lib/StaticAnalyzer/Core/RegionStore.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1478,15 +1478,15 @@ 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
// value to the element type. Eventually we want to compose these values
// 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)) {
Expand All @@ -1496,7 +1496,7 @@ SVal RegionStoreManager::getBinding(RegionBindingsConstRef B, Loc L, QualType T)
// 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<VarRegion>(R)) {
Expand All @@ -1506,7 +1506,7 @@ SVal RegionStoreManager::getBinding(RegionBindingsConstRef B, Loc L, QualType T)
// 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);
Expand Down
132 changes: 102 additions & 30 deletions clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 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:
Expand Down Expand Up @@ -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) {
Expand All @@ -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,
Expand Down Expand Up @@ -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))
Expand All @@ -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.
Expand All @@ -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()) {
Expand All @@ -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;
Expand Down Expand Up @@ -807,17 +861,20 @@ 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);
return L;
}

// 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)) {
Expand All @@ -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.
Expand All @@ -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);
}
Expand All @@ -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.
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>())
Expand Down Expand Up @@ -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.
Expand Down
42 changes: 0 additions & 42 deletions clang/lib/StaticAnalyzer/Core/Store.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 7736b08

Please sign in to comment.