RegionStore/BasicStore: do not return UndefinedVal for accesses to concrete addresses; instead return UnknownVal. This

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

3 files changed, 26 insertions, 8 deletions

diff --git a/lib/Checker/BasicStore.cpp b/lib/Checker/BasicStore.cpp
index f82e1b20be..5221ae3495 100644
--- 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 231be0af18..7808872f5d 100644
--- 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 12e51023a9..9b923bf0f8 100644
--- 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
+}
+