Per an astute observation from Zhongxing Xu, remove a "special case" logic in

RegionStoreManager::Retrieve() that was intended to handle conflated uses of pointers as integers.
It turns out this isn't needed, and resulted in inconsistent behavior when creating symbolic values on the following test case in 'tests/Analysis/misc-ps.m':

  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__) {}
    }
  }

When the code was analyzed with '-arch x86_64', the value assigned to '__gruev__' be would be a
symbolic integer, but for '-arch i386' the value assigned to '__gruev__' would be a symbolic region
(a blob of memory). With this change the value created is always a symbolic integer.

Since the code being removed was added to support analysis of code calling
OSAtomicCompareAndSwapXXX(), I also modified 'test/Analysis/NSString.m' to analyze the code in both
'-arch i386' and '-arch x86_64', and also added some complementary test cases to test the presence
of leaks when using OSAtomicCompareAndSwap32Barrier()/OSAtomicCompareAndSwap64Barrier() instead of
just their absence. This code change reveals that previously both RegionStore and BasicStore were
handling these cases wrong, and would never cause the analyzer to emit a leak in these cases (false
negatives). Now RegionStore gets it right, but BasicStore still gets it wrong (and hence it has been
disabled temporarily for this test case).

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

diff --git a/lib/Analysis/RegionStore.cpp b/lib/Analysis/RegionStore.cpp
index 3844d6a6149cc8ccedd214d34c894cf5d1b3aa4b..9456ab64542cf6e2d1978f5f28bed8a0c9686e90 100644 (file)
--- a/lib/Analysis/RegionStore.cpp
+++ b/lib/Analysis/RegionStore.cpp
@@ -1115,28 +1115,6 @@ SVal RegionStoreManager::RetrieveElement(const GRState* state,
     }
   }
 
-  // Special case: the current region represents a cast and it and the super
-  // region both have pointer types or intptr_t types.  If so, perform the
-  // retrieve from the super region and appropriately "cast" the value.
-  // This is needed to support OSAtomicCompareAndSwap and friends or other
-  // loads that treat integers as pointers and vis versa.
-  if (R->getIndex().isZeroConstant()) {
-    if (const TypedRegion *superTR = dyn_cast<TypedRegion>(superR)) {
-      ASTContext &Ctx = getContext();
-      if (IsAnyPointerOrIntptr(superTR->getValueType(Ctx), Ctx)) {
-        QualType valTy = R->getValueType(Ctx);
-        if (IsAnyPointerOrIntptr(valTy, Ctx)) {
-          // Retrieve the value from the super region.  This will be casted to
-          // valTy when we return to 'Retrieve'.
-          const SValuator::CastResult &cr = Retrieve(state,
-                                                     loc::MemRegionVal(superR),
-                                                     valTy);
-          return cr.getSVal();
-        }
-      }
-    }
-  }
-
   // Check if the immediate super region has a direct binding.
   if (Optional<SVal> V = getDirectBinding(B, superR)) {
     if (SymbolRef parentSym = V->getAsSymbol())

diff --git a/test/Analysis/NSString.m b/test/Analysis/NSString.m
index 0ba3cda6d580e3b9f8ca6838fa41fc4d8bb90fac..a360075645f82d1168a435497e22c7a312485c49 100644 (file)
--- a/test/Analysis/NSString.m
+++ b/test/Analysis/NSString.m
@@ -1,7 +1,13 @@
-// RUN: clang-cc -triple i386-pc-linux-gnu -analyze -checker-cfref -analyzer-store=basic -analyzer-constraints=basic -verify %s &&
-// RUN: clang-cc -triple i386-pc-linux-gnu -analyze -checker-cfref -analyzer-store=basic -analyzer-constraints=range -verify %s &&
-// RUN: clang-cc -triple i386-pc-linux-gnu -analyze -checker-cfref -analyzer-store=region -analyzer-constraints=basic -verify %s &&
-// RUN: clang-cc -triple i386-pc-linux-gnu -analyze -checker-cfref -analyzer-store=region -analyzer-constraints=range -verify %s
+// RUN: clang-cc -triple i386-apple-darwin10 -analyze -checker-cfref -analyzer-store=region -analyzer-constraints=basic -verify %s &&
+// RUN: clang-cc -triple i386-apple-darwin10 -analyze -checker-cfref -analyzer-store=region -analyzer-constraints=range -verify %s &&
+// RUN: clang-cc -DTEST_64 -triple x86_64-apple-darwin10 -analyze -checker-cfref -analyzer-store=region -analyzer-constraints=basic -verify %s &&
+// RUN: clang-cc -DTEST_64 -triple x86_64-apple-darwin10 -analyze -checker-cfref -analyzer-store=region -analyzer-constraints=range -verify %s
+
+// ==-- FIXME: -analyzer-store=basic fails on this file (false negatives). --==
+// NOTWORK: clang-cc -triple i386-apple-darwin10 -analyze -checker-cfref -analyzer-store=basic -analyzer-constraints=range -verify %s &&
+// NOTWORK: clang-cc -triple i386-apple-darwin10 -analyze -checker-cfref -analyzer-store=basic -analyzer-constraints=basic -verify %s &&
+// NOTWORK: clang-cc -DTEST_64 -triple x86_64-apple-darwin10 -analyze -checker-cfref -analyzer-store=basic -analyzer-constraints=basic -verify %s &&
+// NOTWORK: clang-cc -DTEST_64 -triple x86_64-apple-darwin10 -analyze -checker-cfref -analyzer-store=basic -analyzer-constraints=range -verify %s
 
 //===----------------------------------------------------------------------===//
 // The following code is reduced using delta-debugging from
@@ -12,7 +18,18 @@
 // both svelte and portable to non-Mac platforms.
 //===----------------------------------------------------------------------===//
 
+#ifdef TEST_64
+typedef long long int64_t;
+_Bool OSAtomicCompareAndSwap64Barrier( int64_t __oldValue, int64_t __newValue, volatile int64_t *__theValue );
+#define COMPARE_SWAP_BARRIER OSAtomicCompareAndSwap64Barrier
+typedef int64_t intptr_t;
+#else
 typedef int int32_t;
+_Bool OSAtomicCompareAndSwap32Barrier( int32_t __oldValue, int32_t __newValue, volatile int32_t *__theValue );
+#define COMPARE_SWAP_BARRIER OSAtomicCompareAndSwap32Barrier
+typedef int32_t intptr_t;
+#endif
+
 typedef const void * CFTypeRef;
 typedef const struct __CFString * CFStringRef;
 typedef const struct __CFAllocator * CFAllocatorRef;
@@ -263,7 +280,6 @@ id testSharedClassFromFunction() {
 
 // Test OSCompareAndSwap
 _Bool OSAtomicCompareAndSwapPtr( void *__oldValue, void *__newValue, void * volatile *__theValue );
-_Bool OSAtomicCompareAndSwap32Barrier( int32_t __oldValue, int32_t __newValue, volatile int32_t *__theValue );
 extern BOOL objc_atomicCompareAndSwapPtr(id predicate, id replacement, volatile id *objectLocation);
 
 void testOSCompareAndSwap() {
@@ -275,20 +291,29 @@ void testOSCompareAndSwap() {
     [old release];
 }
 
-void testOSCompareAndSwap32Barrier() {
+void testOSCompareAndSwapXXBarrier() {
   NSString *old = 0;
   NSString *s = [[NSString alloc] init]; // no-warning
-  if (!OSAtomicCompareAndSwap32Barrier((int32_t) 0, (int32_t) s, (int32_t*) &old))
+  if (!COMPARE_SWAP_BARRIER((intptr_t) 0, (intptr_t) s, (intptr_t*) &old))
     [s release];
   else    
     [old release];
 }
 
-int testOSCompareAndSwap32Barrier_id(Class myclass, id xclass) {
-  if (OSAtomicCompareAndSwap32Barrier(0, (int32_t) myclass, (int32_t*) &xclass))
+void testOSCompareAndSwapXXBarrier_positive() {
+  NSString *old = 0;
+  NSString *s = [[NSString alloc] init]; // expected-warning{{leak}}
+  if (!COMPARE_SWAP_BARRIER((intptr_t) 0, (intptr_t) s, (intptr_t*) &old))
+    return;
+  else    
+    [old release];
+}
+
+int testOSCompareAndSwapXXBarrier_id(Class myclass, id xclass) {
+  if (COMPARE_SWAP_BARRIER(0, (intptr_t) myclass, (intptr_t*) &xclass))
     return 1;
   return 0;
-}  
+}
 
 void test_objc_atomicCompareAndSwap() {
   NSString *old = 0;
@@ -299,6 +324,16 @@ void test_objc_atomicCompareAndSwap() {
     [old release];
 }
 
+void test_objc_atomicCompareAndSwap_positive() {
+  NSString *old = 0;
+  NSString *s = [[NSString alloc] init]; // expected-warning{{leak}}
+  if (!objc_atomicCompareAndSwapPtr(0, s, &old))
+    return;
+  else    
+    [old release];
+}
+
+
 // Test stringWithFormat (<rdar://problem/6815234>)
 void test_stringWithFormat() {  
   NSString *string = [[NSString stringWithFormat:@"%ld", (long) 100] retain];