[cfe-commits] r118852 - in /cfe/trunk: lib/Checker/BasicStore.cpp lib/Checker/RegionStore.cpp test/Analysis/misc-ps.m

[Ted Kremenek]

Author: kremenek
Date: Thu Nov 11 17:10:10 2010
New Revision: 118852

URL: http://llvm.org/viewvc/llvm-project?rev=118852&view=rev
Log:
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>.

Modified:
    cfe/trunk/lib/Checker/BasicStore.cpp
    cfe/trunk/lib/Checker/RegionStore.cpp
    cfe/trunk/test/Analysis/misc-ps.m

Modified: cfe/trunk/lib/Checker/BasicStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/BasicStore.cpp?rev=118852&r1=118851&r2=118852&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/BasicStore.cpp (original)
+++ cfe/trunk/lib/Checker/BasicStore.cpp Thu Nov 11 17:10:10 2010
@@ -194,10 +194,9 @@
     }
 
     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.");

Modified: cfe/trunk/lib/Checker/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Checker/RegionStore.cpp?rev=118852&r1=118851&r2=118852&view=diff
==============================================================================
--- cfe/trunk/lib/Checker/RegionStore.cpp (original)
+++ cfe/trunk/lib/Checker/RegionStore.cpp Thu Nov 11 17:10:10 2010
@@ -952,10 +952,15 @@
   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();
 

Modified: cfe/trunk/test/Analysis/misc-ps.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps.m?rev=118852&r1=118851&r2=118852&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/misc-ps.m (original)
+++ cfe/trunk/test/Analysis/misc-ps.m Thu Nov 11 17:10:10 2010
@@ -1179,3 +1179,17 @@
      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
+}
+