ASDenysPetrov updated this revision to Diff 298388.
ASDenysPetrov added a comment.
Updated. Moved `castRegion()` to `SValBuilder`.
Actually it wasn't so hard as I thought :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89055/new/
https://reviews.llvm.org/D89055
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
clang/lib/StaticAnalyzer/Core/Store.cpp
clang/test/Analysis/casts.c
clang/test/Analysis/string.c
Index: clang/test/Analysis/string.c
===================================================================
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -363,6 +363,14 @@
strcpy(x, y); // no-warning
}
+// PR37503
+void *get_void_ptr();
+char ***type_punned_ptr;
+void strcpy_no_assertion(char c) {
+ *(unsigned char **)type_punned_ptr = (unsigned char *)(get_void_ptr());
+ strcpy(**type_punned_ptr, &c); // no-crash
+}
+
//===----------------------------------------------------------------------===
// stpcpy()
//===----------------------------------------------------------------------===
Index: clang/test/Analysis/casts.c
===================================================================
--- clang/test/Analysis/casts.c
+++ clang/test/Analysis/casts.c
@@ -245,3 +245,8 @@
return a * a;
}
+void no_crash_reinterpret_char_as_uchar(char ***a, int *b) {
+ *(unsigned char **)a = (unsigned char *)b;
+ if (**a == 0) // no-crash
+ ;
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -57,13 +57,6 @@
return Store;
}
-const ElementRegion *StoreManager::MakeElementRegion(const SubRegion *Base,
- QualType EleTy,
- uint64_t index) {
- NonLoc idx = svalBuilder.makeArrayIndex(index);
- return MRMgr.getElementRegion(EleTy, idx, Base, svalBuilder.getContext());
-}
-
const ElementRegion *StoreManager::GetElementZeroRegion(const SubRegion *R,
QualType T) {
NonLoc idx = svalBuilder.makeZeroArrayIndex();
@@ -71,161 +64,6 @@
return MRMgr.getElementRegion(T, idx, R, Ctx);
}
-const MemRegion *StoreManager::castRegion(const MemRegion *R, QualType CastToTy) {
- ASTContext &Ctx = StateMgr.getContext();
-
- // Handle casts to Objective-C objects.
- if (CastToTy->isObjCObjectPointerType())
- return R->StripCasts();
-
- if (CastToTy->isBlockPointerType()) {
- // FIXME: We may need different solutions, depending on the symbol
- // involved. Blocks can be casted to/from 'id', as they can be treated
- // as Objective-C objects. This could possibly be handled by enhancing
- // our reasoning of downcasts of symbolic objects.
- if (isa<CodeTextRegion>(R) || isa<SymbolicRegion>(R))
- return R;
-
- // We don't know what to make of it. Return a NULL region, which
- // will be interpreted as UnknownVal.
- return nullptr;
- }
-
- // Now assume we are casting from pointer to pointer. Other cases should
- // already be handled.
- QualType PointeeTy = CastToTy->getPointeeType();
- QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
-
- // Handle casts to void*. We just pass the region through.
- if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy)
- return R;
-
- // Handle casts from compatible types.
- if (R->isBoundable())
- if (const auto *TR = dyn_cast<TypedValueRegion>(R)) {
- QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
- if (CanonPointeeTy == ObjTy)
- return R;
- }
-
- // Process region cast according to the kind of the region being cast.
- switch (R->getKind()) {
- case MemRegion::CXXThisRegionKind:
- case MemRegion::CodeSpaceRegionKind:
- case MemRegion::StackLocalsSpaceRegionKind:
- case MemRegion::StackArgumentsSpaceRegionKind:
- case MemRegion::HeapSpaceRegionKind:
- case MemRegion::UnknownSpaceRegionKind:
- case MemRegion::StaticGlobalSpaceRegionKind:
- case MemRegion::GlobalInternalSpaceRegionKind:
- case MemRegion::GlobalSystemSpaceRegionKind:
- case MemRegion::GlobalImmutableSpaceRegionKind: {
- llvm_unreachable("Invalid region cast");
- }
-
- case MemRegion::FunctionCodeRegionKind:
- case MemRegion::BlockCodeRegionKind:
- case MemRegion::BlockDataRegionKind:
- case MemRegion::StringRegionKind:
- // FIXME: Need to handle arbitrary downcasts.
- case MemRegion::SymbolicRegionKind:
- case MemRegion::AllocaRegionKind:
- case MemRegion::CompoundLiteralRegionKind:
- case MemRegion::FieldRegionKind:
- case MemRegion::ObjCIvarRegionKind:
- case MemRegion::ObjCStringRegionKind:
- case MemRegion::NonParamVarRegionKind:
- case MemRegion::ParamVarRegionKind:
- case MemRegion::CXXTempObjectRegionKind:
- case MemRegion::CXXBaseObjectRegionKind:
- case MemRegion::CXXDerivedObjectRegionKind:
- return MakeElementRegion(cast<SubRegion>(R), PointeeTy);
-
- case MemRegion::ElementRegionKind: {
- // If we are casting from an ElementRegion to another type, the
- // algorithm is as follows:
- //
- // (1) Compute the "raw offset" of the ElementRegion from the
- // base region. This is done by calling 'getAsRawOffset()'.
- //
- // (2a) If we get a 'RegionRawOffset' after calling
- // 'getAsRawOffset()', determine if the absolute offset
- // can be exactly divided into chunks of the size of the
- // casted-pointee type. If so, create a new ElementRegion with
- // the pointee-cast type as the new ElementType and the index
- // being the offset divded by the chunk size. If not, create
- // a new ElementRegion at offset 0 off the raw offset region.
- //
- // (2b) If we don't a get a 'RegionRawOffset' after calling
- // 'getAsRawOffset()', it means that we are at offset 0.
- //
- // FIXME: Handle symbolic raw offsets.
-
- const ElementRegion *elementR = cast<ElementRegion>(R);
- const RegionRawOffset &rawOff = elementR->getAsArrayOffset();
- const MemRegion *baseR = rawOff.getRegion();
-
- // If we cannot compute a raw offset, throw up our hands and return
- // a NULL MemRegion*.
- if (!baseR)
- return nullptr;
-
- CharUnits off = rawOff.getOffset();
-
- if (off.isZero()) {
- // Edge case: we are at 0 bytes off the beginning of baseR. We
- // check to see if type we are casting to is the same as the base
- // region. If so, just return the base region.
- if (const auto *TR = dyn_cast<TypedValueRegion>(baseR)) {
- QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
- QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
- if (CanonPointeeTy == ObjTy)
- return baseR;
- }
-
- // Otherwise, create a new ElementRegion at offset 0.
- return MakeElementRegion(cast<SubRegion>(baseR), PointeeTy);
- }
-
- // We have a non-zero offset from the base region. We want to determine
- // if the offset can be evenly divided by sizeof(PointeeTy). If so,
- // we create an ElementRegion whose index is that value. Otherwise, we
- // create two ElementRegions, one that reflects a raw offset and the other
- // that reflects the cast.
-
- // Compute the index for the new ElementRegion.
- int64_t newIndex = 0;
- const MemRegion *newSuperR = nullptr;
-
- // We can only compute sizeof(PointeeTy) if it is a complete type.
- if (!PointeeTy->isIncompleteType()) {
- // Compute the size in **bytes**.
- CharUnits pointeeTySize = Ctx.getTypeSizeInChars(PointeeTy);
- if (!pointeeTySize.isZero()) {
- // Is the offset a multiple of the size? If so, we can layer the
- // ElementRegion (with elementType == PointeeTy) directly on top of
- // the base region.
- if (off % pointeeTySize == 0) {
- newIndex = off / pointeeTySize;
- newSuperR = baseR;
- }
- }
- }
-
- if (!newSuperR) {
- // Create an intermediate ElementRegion to represent the raw byte.
- // This will be the super region of the final ElementRegion.
- newSuperR = MakeElementRegion(cast<SubRegion>(baseR), Ctx.CharTy,
- off.getQuantity());
- }
-
- return MakeElementRegion(cast<SubRegion>(newSuperR), PointeeTy, newIndex);
- }
- }
-
- llvm_unreachable("unreachable");
-}
-
static bool regionMatchesCXXRecordType(SVal V, QualType Ty) {
const MemRegion *MR = V.getAsRegion();
if (!MR)
@@ -418,21 +256,6 @@
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);
}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -65,8 +65,34 @@
// Transfer function for Casts.
//===----------------------------------------------------------------------===//
+static bool hasSameUnqualifiedPointeeType(QualType ty1, QualType ty2) {
+ return ty1->getPointeeType().getCanonicalType().getTypePtr() ==
+ ty2->getPointeeType().getCanonicalType().getTypePtr();
+}
+
SVal SimpleSValBuilder::dispatchCast(SVal Val, QualType CastTy) {
assert(Val.getAs<Loc>() || Val.getAs<NonLoc>());
+
+ // 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()) {
+ if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(Val.getAsRegion())) {
+ QualType sr = SR->getSymbol()->getType();
+ if (!hasSameUnqualifiedPointeeType(sr, CastTy))
+ return loc::MemRegionVal(castRegion(SR, CastTy));
+ }
+ // Next fixes pointer dereference using type different from its initial
+ // one. See PR37503 for details.
+ if (const auto *ER = dyn_cast_or_null<ElementRegion>(Val.getAsRegion()))
+ return loc::MemRegionVal(castRegion(ER, CastTy));
+ }
+
return Val.getAs<Loc>() ? evalCastFromLoc(Val.castAs<Loc>(), CastTy)
: evalCastFromNonLoc(Val.castAs<NonLoc>(), CastTy);
}
@@ -135,7 +161,7 @@
// can be introduced by the frontend for corner cases, e.g
// casting from va_list* to __builtin_va_list&.
//
- if (Loc::isLocType(castTy) || castTy->isReferenceType())
+ if (Loc::isLocType(castTy))
return val;
// FIXME: Handle transparent unions where a value can be "transparently"
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -530,6 +530,169 @@
return evalCast(val, castTy, originalTy);
}
+const ElementRegion *SValBuilder::MakeElementRegion(const SubRegion *Base,
+ QualType EleTy,
+ uint64_t index /*= 0*/) {
+ NonLoc idx = makeArrayIndex(index);
+ return MemMgr.getElementRegion(EleTy, idx, Base, getContext());
+}
+
+const MemRegion *SValBuilder::castRegion(const MemRegion *R,
+ QualType CastToTy) {
+ ASTContext &Ctx = getContext();
+
+ // Handle casts to Objective-C objects.
+ if (CastToTy->isObjCObjectPointerType())
+ return R->StripCasts();
+
+ if (CastToTy->isBlockPointerType()) {
+ // FIXME: We may need different solutions, depending on the symbol
+ // involved. Blocks can be casted to/from 'id', as they can be treated
+ // as Objective-C objects. This could possibly be handled by enhancing
+ // our reasoning of downcasts of symbolic objects.
+ if (isa<CodeTextRegion>(R) || isa<SymbolicRegion>(R))
+ return R;
+
+ // We don't know what to make of it. Return a NULL region, which
+ // will be interpreted as UnknownVal.
+ return nullptr;
+ }
+
+ // Now assume we are casting from pointer to pointer. Other cases should
+ // already be handled.
+ QualType PointeeTy = CastToTy->getPointeeType();
+ QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
+
+ // Handle casts to void*. We just pass the region through.
+ if (CanonPointeeTy.getLocalUnqualifiedType() == Ctx.VoidTy)
+ return R;
+
+ // Handle casts from compatible types.
+ if (R->isBoundable())
+ if (const auto *TR = dyn_cast<TypedValueRegion>(R)) {
+ QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
+ if (CanonPointeeTy == ObjTy)
+ return R;
+ }
+
+ // Process region cast according to the kind of the region being cast.
+ switch (R->getKind()) {
+ case MemRegion::CXXThisRegionKind:
+ case MemRegion::CodeSpaceRegionKind:
+ case MemRegion::StackLocalsSpaceRegionKind:
+ case MemRegion::StackArgumentsSpaceRegionKind:
+ case MemRegion::HeapSpaceRegionKind:
+ case MemRegion::UnknownSpaceRegionKind:
+ case MemRegion::StaticGlobalSpaceRegionKind:
+ case MemRegion::GlobalInternalSpaceRegionKind:
+ case MemRegion::GlobalSystemSpaceRegionKind:
+ case MemRegion::GlobalImmutableSpaceRegionKind: {
+ llvm_unreachable("Invalid region cast");
+ }
+
+ case MemRegion::FunctionCodeRegionKind:
+ case MemRegion::BlockCodeRegionKind:
+ case MemRegion::BlockDataRegionKind:
+ case MemRegion::StringRegionKind:
+ // FIXME: Need to handle arbitrary downcasts.
+ case MemRegion::SymbolicRegionKind:
+ case MemRegion::AllocaRegionKind:
+ case MemRegion::CompoundLiteralRegionKind:
+ case MemRegion::FieldRegionKind:
+ case MemRegion::ObjCIvarRegionKind:
+ case MemRegion::ObjCStringRegionKind:
+ case MemRegion::NonParamVarRegionKind:
+ case MemRegion::ParamVarRegionKind:
+ case MemRegion::CXXTempObjectRegionKind:
+ case MemRegion::CXXBaseObjectRegionKind:
+ case MemRegion::CXXDerivedObjectRegionKind:
+ return MakeElementRegion(cast<SubRegion>(R), PointeeTy);
+
+ case MemRegion::ElementRegionKind: {
+ // If we are casting from an ElementRegion to another type, the
+ // algorithm is as follows:
+ //
+ // (1) Compute the "raw offset" of the ElementRegion from the
+ // base region. This is done by calling 'getAsRawOffset()'.
+ //
+ // (2a) If we get a 'RegionRawOffset' after calling
+ // 'getAsRawOffset()', determine if the absolute offset
+ // can be exactly divided into chunks of the size of the
+ // casted-pointee type. If so, create a new ElementRegion with
+ // the pointee-cast type as the new ElementType and the index
+ // being the offset divded by the chunk size. If not, create
+ // a new ElementRegion at offset 0 off the raw offset region.
+ //
+ // (2b) If we don't a get a 'RegionRawOffset' after calling
+ // 'getAsRawOffset()', it means that we are at offset 0.
+ //
+ // FIXME: Handle symbolic raw offsets.
+
+ const ElementRegion *elementR = cast<ElementRegion>(R);
+ const RegionRawOffset &rawOff = elementR->getAsArrayOffset();
+ const MemRegion *baseR = rawOff.getRegion();
+
+ // If we cannot compute a raw offset, throw up our hands and return
+ // a NULL MemRegion*.
+ if (!baseR)
+ return nullptr;
+
+ CharUnits off = rawOff.getOffset();
+
+ if (off.isZero()) {
+ // Edge case: we are at 0 bytes off the beginning of baseR. We
+ // check to see if type we are casting to is the same as the base
+ // region. If so, just return the base region.
+ if (const auto *TR = dyn_cast<TypedValueRegion>(baseR)) {
+ QualType ObjTy = Ctx.getCanonicalType(TR->getValueType());
+ QualType CanonPointeeTy = Ctx.getCanonicalType(PointeeTy);
+ if (CanonPointeeTy == ObjTy)
+ return baseR;
+ }
+
+ // Otherwise, create a new ElementRegion at offset 0.
+ return MakeElementRegion(cast<SubRegion>(baseR), PointeeTy);
+ }
+
+ // We have a non-zero offset from the base region. We want to determine
+ // if the offset can be evenly divided by sizeof(PointeeTy). If so,
+ // we create an ElementRegion whose index is that value. Otherwise, we
+ // create two ElementRegions, one that reflects a raw offset and the other
+ // that reflects the cast.
+
+ // Compute the index for the new ElementRegion.
+ int64_t newIndex = 0;
+ const MemRegion *newSuperR = nullptr;
+
+ // We can only compute sizeof(PointeeTy) if it is a complete type.
+ if (!PointeeTy->isIncompleteType()) {
+ // Compute the size in **bytes**.
+ CharUnits pointeeTySize = Ctx.getTypeSizeInChars(PointeeTy);
+ if (!pointeeTySize.isZero()) {
+ // Is the offset a multiple of the size? If so, we can layer the
+ // ElementRegion (with elementType == PointeeTy) directly on top of
+ // the base region.
+ if (off % pointeeTySize == 0) {
+ newIndex = off / pointeeTySize;
+ newSuperR = baseR;
+ }
+ }
+ }
+
+ if (!newSuperR) {
+ // Create an intermediate ElementRegion to represent the raw byte.
+ // This will be the super region of the final ElementRegion.
+ newSuperR = MakeElementRegion(cast<SubRegion>(baseR), Ctx.CharTy,
+ off.getQuantity());
+ }
+
+ return MakeElementRegion(cast<SubRegion>(newSuperR), PointeeTy, newIndex);
+ }
+ }
+
+ llvm_unreachable("unreachable");
+}
+
// FIXME: should rewrite according to the cast kind.
SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) {
castTy = Context.getCanonicalType(castTy);
@@ -574,8 +737,7 @@
if (Loc::isLocType(castTy) && originalTy->isIntegralOrEnumerationType()) {
if (Optional<nonloc::LocAsInteger> LV = val.getAs<nonloc::LocAsInteger>()) {
if (const MemRegion *R = LV->getLoc().getAsRegion()) {
- StoreManager &storeMgr = StateMgr.getStoreManager();
- R = storeMgr.castRegion(R, castTy);
+ R = castRegion(R, castTy);
return R ? SVal(loc::MemRegionVal(R)) : UnknownVal();
}
return LV->getLoc();
@@ -650,12 +812,10 @@
assert(Loc::isLocType(originalTy) || originalTy->isFunctionType() ||
originalTy->isBlockPointerType() || castTy->isReferenceType());
- StoreManager &storeMgr = StateMgr.getStoreManager();
-
- // Delegate to store manager to get the result of casting a region to a
- // different type. If the MemRegion* returned is NULL, this expression
- // Evaluates to UnknownVal.
- R = storeMgr.castRegion(R, castTy);
+ // Get the result of casting a region to a different type.
+ // If the MemRegion* returned is NULL, this expression evaluates to
+ // UnknownVal.
+ R = castRegion(R, castTy);
return R ? SVal(loc::MemRegionVal(R)) : UnknownVal();
}
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -178,11 +178,6 @@
const ElementRegion *GetElementZeroRegion(const SubRegion *R, QualType T);
- /// castRegion - Used by ExprEngine::VisitCast to handle casts from
- /// a MemRegion* to a specific location type. 'R' is the region being
- /// casted and 'CastToTy' the result type of the cast.
- const MemRegion *castRegion(const MemRegion *region, QualType CastToTy);
-
virtual StoreRef removeDeadBindings(Store store, const StackFrameContext *LCtx,
SymbolReaper &SymReaper) = 0;
@@ -276,9 +271,6 @@
virtual void iterBindings(Store store, BindingsHandler& f) = 0;
protected:
- const ElementRegion *MakeElementRegion(const SubRegion *baseRegion,
- 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
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -79,6 +79,12 @@
// FIXME: Make these protected again once RegionStoreManager correctly
// handles loads from different bound value types.
virtual SVal dispatchCast(SVal val, QualType castTy) = 0;
+ const ElementRegion *MakeElementRegion(const SubRegion *Base, QualType EleTy,
+ uint64_t index = 0);
+ /// castRegion - Used by ExprEngine::VisitCast to handle casts from
+ /// a MemRegion* to a specific location type. 'R' is the region being
+ /// casted and 'CastToTy' the result type of the cast.
+ const MemRegion *castRegion(const MemRegion *R, QualType CastToTy);
public:
SValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
