diff --git a/clang/include/clang/Analysis/PathSensitive/MemRegion.h b/clang/include/clang/Analysis/PathSensitive/MemRegion.h index cc7c44c06a80..7f8c5c29037f 100644 --- a/clang/include/clang/Analysis/PathSensitive/MemRegion.h +++ b/clang/include/clang/Analysis/PathSensitive/MemRegion.h @@ -59,9 +59,10 @@ private: protected: MemRegion(Kind k) : kind(k) {} virtual ~MemRegion(); - ASTContext &getContext() const; public: + ASTContext &getContext() const; + virtual void Profile(llvm::FoldingSetNodeID& ID) const = 0; virtual MemRegionManager* getMemRegionManager() const = 0; diff --git a/clang/lib/Analysis/RegionStore.cpp b/clang/lib/Analysis/RegionStore.cpp index 4e83720f9fa1..b4eb4b8e194a 100644 --- a/clang/lib/Analysis/RegionStore.cpp +++ b/clang/lib/Analysis/RegionStore.cpp @@ -781,6 +781,16 @@ SVal RegionStoreManager::EvalBinOp(const GRState *state, // Loading values from regions. //===----------------------------------------------------------------------===// +static bool IsReinterpreted(QualType RTy, QualType UsedTy, ASTContext &Ctx) { + RTy = Ctx.getCanonicalType(RTy); + UsedTy = Ctx.getCanonicalType(UsedTy); + + if (RTy == UsedTy) + return false; + + return !(Loc::IsLocType(RTy) && Loc::IsLocType(UsedTy)); +} + SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { assert(!isa(L) && "location unknown"); @@ -805,13 +815,14 @@ SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { if (isa(MR)) { ASTContext &Ctx = getContext(); SVal idx = ValMgr.makeIntVal(0, Ctx.IntTy); + assert(!T.isNull()); MR = MRMgr.getElementRegion(T, idx, MR, Ctx); } // FIXME: Perhaps this method should just take a 'const MemRegion*' argument // instead of 'Loc', and have the other Loc cases handled at a higher level. const TypedRegion *R = cast(MR); - assert(R && "bad region"); + QualType RTy = R->getValueType(getContext()); // FIXME: We should eventually handle funny addressing. e.g.: // @@ -822,7 +833,13 @@ SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) { // // Such funny addressing will occur due to layering of regions. - QualType RTy = R->getValueType(getContext()); + ASTContext &Ctx = getContext(); + if (!T.isNull() && IsReinterpreted(RTy, T, Ctx)) { + SVal idx = ValMgr.makeIntVal(0, Ctx.IntTy); + R = MRMgr.getElementRegion(T, idx, R, Ctx); + RTy = T; + assert(RTy == R->getValueType(Ctx)); + } if (RTy->isStructureType()) return RetrieveStruct(state, R); @@ -929,8 +946,11 @@ SVal RegionStoreManager::RetrieveElement(const GRState* state, } // Check if the super region has a binding. - if (B.lookup(superR)) { - // We do not extract the bit value from super region for now. + if (const SVal *V = B.lookup(superR)) { + if (SymbolRef parentSym = V->getAsSymbol()) + return ValMgr.getDerivedRegionValueSymbolVal(parentSym, R); + + // Other cases: give up. return UnknownVal(); } diff --git a/clang/lib/Analysis/Store.cpp b/clang/lib/Analysis/Store.cpp index 2910f49c80c9..bbda565cec5b 100644 --- a/clang/lib/Analysis/Store.cpp +++ b/clang/lib/Analysis/Store.cpp @@ -238,16 +238,20 @@ const GRState *StoreManager::InvalidateRegion(const GRState *state, } const TypedRegion *TR = cast(R); + QualType T = TR->getValueType(Ctx); - QualType T; - - // If the region is cast to another type, use that type. + // If the region is cast to another type, use that type. if (const QualType *CastTy = getCastType(state, R)) { assert(!(*CastTy)->isObjCObjectPointerType()); - T = (*CastTy)->getAsPointerType()->getPointeeType(); - } else - T = TR->getValueType(Ctx); + QualType NewT = (*CastTy)->getAsPointerType()->getPointeeType(); + // The only exception is if the original region had a location type as its + // value type we always want to treat the region as binding to a location. + // This issue can arise when pointers are casted to integers and back. + if (!Loc::IsLocType(T) || Loc::IsLocType(NewT)) + T = NewT; + } + if (Loc::IsLocType(T) || (T->isIntegerType() && T->isScalarType())) { SVal V = ValMgr.getConjuredSymbolVal(E, T, Count); return Bind(state, ValMgr.makeLoc(TR), V); diff --git a/clang/test/Analysis/misc-ps-region-store-i386.m b/clang/test/Analysis/misc-ps-region-store-i386.m index f501dbe7ad3b..c2c4d5b9413c 100644 --- a/clang/test/Analysis/misc-ps-region-store-i386.m +++ b/clang/test/Analysis/misc-ps-region-store-i386.m @@ -1,29 +1,14 @@ // RUN: clang-cc -triple i386-apple-darwin9 -analyze -checker-cfref --analyzer-store=region --verify -fblocks %s -typedef struct _BStruct { void *grue; } BStruct; -void testB_aux(void *ptr); -void testB(BStruct *b) { - { - int *__gruep__ = ((int *)&((b)->grue)); - int __gruev__ = *__gruep__; - int __gruev2__ = *__gruep__; - if (__gruev__ != __gruev2__) { - int *p = 0; - *p = 0xDEADBEEF; - } - - testB_aux(__gruep__); - } - { - int *__gruep__ = ((int *)&((b)->grue)); - int __gruev__ = *__gruep__; - int __gruev2__ = *__gruep__; - if (__gruev__ != __gruev2__) { - int *p = 0; - *p = 0xDEADBEEF; - } - - if (~0 != __gruev__) {} - } +// Here is a case where a pointer is treated as integer, invalidated as an +// integer, and then used again as a pointer. This test just makes sure +// we don't crash. +typedef unsigned uintptr_t; +void test_pointer_invalidated_as_int_aux(uintptr_t* ptr); +void test_pointer_invalidated_as_int() { + void *x; + test_pointer_invalidated_as_int_aux((uintptr_t*) &x); + // Here we have a pointer to integer cast. + uintptr_t y = (uintptr_t) x; } diff --git a/clang/test/Analysis/misc-ps-region-store-x86_64.m b/clang/test/Analysis/misc-ps-region-store-x86_64.m index 2f74904d9c32..154ffaf3a003 100644 --- a/clang/test/Analysis/misc-ps-region-store-x86_64.m +++ b/clang/test/Analysis/misc-ps-region-store-x86_64.m @@ -1,31 +1,14 @@ // RUN: clang-cc -triple x86_64-apple-darwin9 -analyze -checker-cfref --analyzer-store=region --verify -fblocks %s -// This test case appears in misc-ps-region-store-i386.m, but fails under x86_64. -// The reason is that 'int' is smaller than a pointer on a 64-bit architecture, -// and we aren't reasoning yet about just the first 32-bits of the pointer. -typedef struct _BStruct { void *grue; } BStruct; -void testB_aux(void *ptr); -void testB(BStruct *b) { - { - int *__gruep__ = ((int *)&((b)->grue)); - int __gruev__ = *__gruep__; - int __gruev2__ = *__gruep__; - if (__gruev__ != __gruev2__) { - int *p = 0; - *p = 0xDEADBEEF; // no-warning - } - - testB_aux(__gruep__); - } - { - int *__gruep__ = ((int *)&((b)->grue)); - int __gruev__ = *__gruep__; - int __gruev2__ = *__gruep__; - if (__gruev__ != __gruev2__) { - int *p = 0; - *p = 0xDEADBEEF; // expected-warning{{null}} - } - - if (~0 != __gruev__) {} - } +// Here is a case where a pointer is treated as integer, invalidated as an +// integer, and then used again as a pointer. This test just makes sure +// we don't crash. +typedef unsigned long uintptr_t; +void test_pointer_invalidated_as_int_aux(uintptr_t* ptr); +void test_pointer_invalidated_as_int() { + void *x; + test_pointer_invalidated_as_int_aux((uintptr_t*) &x); + // Here we have a pointer to integer cast. + uintptr_t y = (uintptr_t) x; } + diff --git a/clang/test/Analysis/misc-ps-region-store.m b/clang/test/Analysis/misc-ps-region-store.m index 245273b22085..c5341a013888 100644 --- a/clang/test/Analysis/misc-ps-region-store.m +++ b/clang/test/Analysis/misc-ps-region-store.m @@ -89,9 +89,28 @@ typedef struct _BStruct { void *grue; } BStruct; void testB_aux(void *ptr); void testB(BStruct *b) { - // This case has moved to 'misc-ps-region-store-i386.m' and - // 'misc-ps-region-store-x86_64.m'. It succeeds under x86_64. When it - // passes it both, pull it in here. + { + int *__gruep__ = ((int *)&((b)->grue)); + int __gruev__ = *__gruep__; + int __gruev2__ = *__gruep__; + if (__gruev__ != __gruev2__) { + int *p = 0; + *p = 0xDEADBEEF; // no-warning + } + + testB_aux(__gruep__); + } + { + int *__gruep__ = ((int *)&((b)->grue)); + int __gruev__ = *__gruep__; + int __gruev2__ = *__gruep__; + if (__gruev__ != __gruev2__) { + int *p = 0; + *p = 0xDEADBEEF; // no-warning + } + + if (~0 != __gruev__) {} + } } void testB_2(BStruct *b) {