RegionStore/BasicStore: do not return UndefinedVal for accesses to concrete addresses...

leads it up to checkers (e.g., DereferenceChecker) to guard against illegal accesses (e.g., null dereferences).

Fixes PR 5272 and <rdar://problem/6839683>.

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

diff --git a/lib/Checker/BasicStore.cpp b/lib/Checker/BasicStore.cpp
index f82e1b20be9b90ac5b2aa27b065a0d79843974c3..5221ae3495fdbec38a8009752d5e7c326f4a909f 100644 (file)
--- a/lib/Checker/BasicStore.cpp
+++ b/lib/Checker/BasicStore.cpp
@@ -194,10 +194,9 @@ SVal BasicStoreManager::Retrieve(Store store, Loc loc, QualType T) {
     }
 
     case loc::ConcreteIntKind:
-      // Some clients may call GetSVal with such an option simply because
-      // they are doing a quick scan through their Locs (potentially to
-      // invalidate their bindings).  Just return Undefined.
-      return UndefinedVal();
+      // Support direct accesses to memory.  It's up to individual checkers
+      // to flag an error.
+      return UnknownVal();
 
     default:
       assert (false && "Invalid Loc.");

diff --git a/lib/Checker/RegionStore.cpp b/lib/Checker/RegionStore.cpp
index 231be0af18d77986f4e7e271328b7af3546560a1..7808872f5dd630d09dba85157003e45e234be754 100644 (file)
--- a/lib/Checker/RegionStore.cpp
+++ b/lib/Checker/RegionStore.cpp
@@ -952,10 +952,15 @@ SVal RegionStoreManager::Retrieve(Store store, Loc L, QualType T) {
   assert(!isa<UnknownVal>(L) && "location unknown");
   assert(!isa<UndefinedVal>(L) && "location undefined");
 
-  // FIXME: Is this even possible?  Shouldn't this be treated as a null
-  //  dereference at a higher level?
-  if (isa<loc::ConcreteInt>(L))
-    return UndefinedVal();
+  // For access to concrete addresses, return UnknownVal.  Checks
+  // for null dereferences (and similar errors) are done by checkers, not
+  // the Store.
+  // FIXME: We can consider lazily symbolicating such memory, but we really
+  // should defer this when we can reason easily about symbolicating arrays
+  // of bytes.
+  if (isa<loc::ConcreteInt>(L)) {
+    return UnknownVal();
+  }
 
   const MemRegion *MR = cast<loc::MemRegionVal>(L).getRegion();
 

diff --git a/test/Analysis/misc-ps.m b/test/Analysis/misc-ps.m
index 12e51023a9ecf0ce882421ceb76e26cbd37c6130..9b923bf0f8092b9e63c89fef0a688a9cc5e6209e 100644 (file)
--- a/test/Analysis/misc-ps.m
+++ b/test/Analysis/misc-ps.m
@@ -1179,3 +1179,17 @@ void baz_pr8440(int n)
      saved_pr8440.data[i] = foo_pr8440(); // no-warning
 }
 
+// Support direct accesses to non-null memory.  Reported in:
+//  PR 5272
+//  <rdar://problem/6839683>
+int test_direct_address_load() {
+  int *p = (int*) 0x4000;
+  return *p; // no-warning
+}
+
+void pr5272_test() {
+  struct pr5272 { int var2; };
+  (*(struct pr5272*)0xBC000000).var2 = 0; // no-warning
+  (*(struct pr5272*)0xBC000000).var2 += 2; // no-warning
+}
+