Hi Ted,
Here is another fix for this bug. Instead of recovering from a wrong
invalidation, this patch aims to invalidate the region correctly. It
uses the cast-to type to invalidate the region when available. To
avoid invalid cast-to type like 'void*' or 'id', region store now only
records non-generic casts of regions.
On Sat, Jul 11, 2009 at 12:38 PM, Ted Kremenek<[email protected]> wrote:
> Author: kremenek
> Date: Fri Jul 10 23:38:49 2009
> New Revision: 75356
>
> URL: http://llvm.org/viewvc/llvm-project?rev=75356&view=rev
> Log:
> Handle insidious corner case exposed by RegionStoreManager when handling
> void* values that are bound
> to symbolic regions and then treated like integers.
>
> Modified:
> cfe/trunk/lib/Analysis/GRExprEngine.cpp
> cfe/trunk/test/Analysis/misc-ps.m
>
> Modified: cfe/trunk/lib/Analysis/GRExprEngine.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/GRExprEngine.cpp?rev=75356&r1=75355&r2=75356&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Analysis/GRExprEngine.cpp (original)
> +++ cfe/trunk/lib/Analysis/GRExprEngine.cpp Fri Jul 10 23:38:49 2009
> @@ -1110,6 +1110,19 @@
> }
> else {
> SVal V = state->getSVal(cast<Loc>(location), Ex->getType());
> +
> + // Casts can create weird scenarios where a location must be implicitly
> + // converted to something else. For example:
> + //
> + // void *x;
> + // int *y = (int*) &x; // void** -> int* cast.
> + // invalidate(y); // 'x' now binds to a symbolic region
> + // int z = *y;
> + //
> + if (isa<Loc>(V) && !Loc::IsLocType(Ex->getType())) {
> + V = EvalCast(V, Ex->getType());
> + }
> +
> MakeNode(Dst, Ex, Pred, state->bindExpr(Ex, V), K, tag);
> }
> }
>
> Modified: cfe/trunk/test/Analysis/misc-ps.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps.m?rev=75356&r1=75355&r2=75356&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/misc-ps.m (original)
> +++ cfe/trunk/test/Analysis/misc-ps.m Fri Jul 10 23:38:49 2009
> @@ -350,3 +350,24 @@
> return;
> }
>
> +// RegionStoreManager previously crashed on this example. The problem is
> that
> +// the value bound to the field of b->grue after the call to testB_aux is
> +// a symbolic region. The second '*__gruep__' involves performing a load
> +// from a 'int*' that really is a 'void**'. The loaded location must be
> +// implicitly converted to an integer that wraps a location. Previosly we
> would
> +// get a crash here due to an assertion failure.
> +typedef struct _BStruct { void *grue; } BStruct;
> +void testB_aux(void *ptr);
> +void testB(BStruct *b) {
> + {
> + int *__gruep__ = ((int *)&((b)->grue));
> + int __gruev__ = *__gruep__;
> + testB_aux(__gruep__);
> + }
> + {
> + int *__gruep__ = ((int *)&((b)->grue));
> + int __gruev__ = *__gruep__;
> + if (~0 != __gruev__) {}
> + }
> +}
> +
>
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
Index: include/clang/Analysis/PathSensitive/Store.h
===================================================================
--- include/clang/Analysis/PathSensitive/Store.h （版本 75362）
+++ include/clang/Analysis/PathSensitive/Store.h （工作副本）
@@ -145,6 +145,10 @@
return state;
}
+ virtual const QualType *getCastType(const GRState *state, const MemRegion *R){
+ return 0;
+ }
+
/// EvalBinOp - Perform pointer arithmetic.
virtual SVal EvalBinOp(const GRState *state, BinaryOperator::Opcode Op,
Loc lhs, NonLoc rhs, QualType resultTy) {
Index: lib/Analysis/RegionStore.cpp
===================================================================
--- lib/Analysis/RegionStore.cpp （版本 75362）
+++ lib/Analysis/RegionStore.cpp （工作副本）
@@ -327,6 +327,10 @@
const GRState *setCastType(const GRState *state, const MemRegion* R,
QualType T);
+ const QualType *getCastType(const GRState *state, const MemRegion *R) {
+ return state->get<RegionCasts>(R);
+ }
+
static inline RegionBindingsTy GetRegionBindings(Store store) {
return RegionBindingsTy(static_cast<const RegionBindingsTy::TreeTy*>(store));
}
@@ -348,7 +352,26 @@
};
} // end anonymous namespace
+static bool isGenericPtr(ASTContext &Ctx, QualType Ty) {
+ if (Ty->isObjCIdType() || Ty->isObjCQualifiedIdType())
+ return true;
+ while (true) {
+ Ty = Ctx.getCanonicalType(Ty);
+ if (Ty->isVoidType())
+ return true;
+
+ if (const PointerType *PT = Ty->getAsPointerType()) {
+ Ty = PT->getPointeeType();
+ continue;
+ }
+
+ break;
+ }
+
+ return false;
+}
+
//===----------------------------------------------------------------------===//
// RegionStore creation.
//===----------------------------------------------------------------------===//
@@ -1251,6 +1274,8 @@
const GRState *RegionStoreManager::setCastType(const GRState *state,
const MemRegion* R, QualType T) {
+ if (isGenericPtr(getContext(), T))
+ return state;
return state->set<RegionCasts>(R, T);
}
Index: lib/Analysis/Store.cpp
===================================================================
--- lib/Analysis/Store.cpp （版本 75362）
+++ lib/Analysis/Store.cpp （工作副本）
@@ -235,8 +235,16 @@
const TypedRegion *TR = cast<TypedRegion>(R);
- QualType T = TR->getValueType(Ctx);
+ QualType T;
+ // If the region is cast to another type, use that type.
+ const QualType *CastTy = getCastType(state, R);
+ if (CastTy) {
+ T = (*CastTy)->getAsPointerType()->getPointeeType();
+ }
+ else
+ T = TR->getValueType(Ctx);
+
if (Loc::IsLocType(T) || (T->isIntegerType() && T->isScalarType())) {
SVal V = ValMgr.getConjuredSymbolVal(E, T, Count);
return Bind(state, ValMgr.makeLoc(TR), V);
Index: lib/Analysis/GRExprEngine.cpp
===================================================================
--- lib/Analysis/GRExprEngine.cpp （版本 75362）
+++ lib/Analysis/GRExprEngine.cpp （工作副本）
@@ -1119,9 +1119,9 @@
// invalidate(y); // 'x' now binds to a symbolic region
// int z = *y;
//
- if (isa<Loc>(V) && !Loc::IsLocType(Ex->getType())) {
- V = EvalCast(V, Ex->getType());
- }
+ //if (isa<Loc>(V) && !Loc::IsLocType(Ex->getType())) {
+ // V = EvalCast(V, Ex->getType());
+ // }
MakeNode(Dst, Ex, Pred, state->bindExpr(Ex, V), K, tag);
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
