More test cases revealed that the logic in StoreManager::InvalidateRegion() needs...

This patch causes:
- StoreManager::InvalidateRegion() to not used the casted type of a region if
  it would cause a pointer type to be invalidated as a non-pointer type.
- Pushes RegionStore::RetrieveElement() further by handling retrievals from
  symbolic arrays that have been invalidated.  This uses the new SymbolDerived
  construct that was recently introduced.

The result is that the failing test in misc-ps-region-store-x86_64.m now passes.
Both misc-ps-region-store-x86_64.m and misc-ps-region-store-i386.m contain a
test case that motivated this change.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@75730 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Analysis/PathSensitive/MemRegion.h b/include/clang/Analysis/PathSensitive/MemRegion.h
index cc7c44c06a801df10304fa7b3952d3ccddadf7e1..7f8c5c29037f06ec406206e0df4b01e4f4292dc9 100644 (file)
--- a/include/clang/Analysis/PathSensitive/MemRegion.h
+++ b/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/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp
index 4e83720f9fa11cc1585dd82b4a01bdd47db2a638..b4eb4b8e194ae81a42e80be0d9f1eba309742bde 100644 (file)
--- a/lib/Analysis/RegionStore.cpp
+++ b/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<UnknownVal>(L) && "location unknown");
@@ -805,13 +815,14 @@ SVal RegionStoreManager::Retrieve(const GRState *state, Loc L, QualType T) {
   if (isa<SymbolicRegion>(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<TypedRegion>(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/lib/Analysis/Store.cpp b/lib/Analysis/Store.cpp
index 2910f49c80c95d71c9c712424181c921e6272403..bbda565cec5b4f783042f2e9b13d26ac13414fde 100644 (file)
--- a/lib/Analysis/Store.cpp
+++ b/lib/Analysis/Store.cpp
@@ -238,16 +238,20 @@ const GRState *StoreManager::InvalidateRegion(const GRState *state,
   }
 
   const TypedRegion *TR = cast<TypedRegion>(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/test/Analysis/misc-ps-region-store-i386.m b/test/Analysis/misc-ps-region-store-i386.m
index f501dbe7ad3b7c705091546433defc32325200dd..c2c4d5b9413c3f177084ebb748155f69d5758e00 100644 (file)
--- a/test/Analysis/misc-ps-region-store-i386.m
+++ b/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/test/Analysis/misc-ps-region-store-x86_64.m b/test/Analysis/misc-ps-region-store-x86_64.m
index 2f74904d9c32670bd406711f69a1d49019454120..154ffaf3a003a3023b684ec6f4a146524fc7684c 100644 (file)
--- a/test/Analysis/misc-ps-region-store-x86_64.m
+++ b/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/test/Analysis/misc-ps-region-store.m b/test/Analysis/misc-ps-region-store.m
index 245273b2208546142769eb47fc2b4a9c0bf71e1f..c5341a01388897ac92aef1d5d64cad9d89f6100c 100644 (file)
--- a/test/Analysis/misc-ps-region-store.m
+++ b/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) {