[cfe-commits] r129366 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp test/Analysis/out-of-bounds.c

[Ted Kremenek]

Author: kremenek
Date: Tue Apr 12 12:21:33 2011
New Revision: 129366

URL: http://llvm.org/viewvc/llvm-project?rev=129366&view=rev
Log:
ArrayBoundCheckerV2: don't arbitrarily warn about indexing before the 0-index of a symbolic region.  In many cases that isn't really the base offset.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
    cfe/trunk/test/Analysis/out-of-bounds.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp?rev=129366&r1=129365&r2=129366&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp Tue Apr 12 12:21:33 2011
@@ -62,6 +62,22 @@
 };
 }
 
+static SVal computeExtentBegin(SValBuilder &svalBuilder, 
+                               const MemRegion *region) {
+  while (true)
+    switch (region->getKind()) {
+      default:
+        return svalBuilder.makeZeroArrayIndex();        
+      case MemRegion::SymbolicRegionKind:
+        // FIXME: improve this later by tracking symbolic lower bounds
+        // for symbolic regions.
+        return UnknownVal();
+      case MemRegion::ElementRegionKind:
+        region = cast<SubRegion>(region)->getSuperRegion();
+        continue;
+    }
+}
+
 void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
                                         CheckerContext &checkerContext) const {
 
@@ -84,31 +100,36 @@
   if (!rawOffset.getRegion())
     return;
 
-  // CHECK LOWER BOUND: Is byteOffset < 0?  If so, we are doing a load/store
+  // CHECK LOWER BOUND: Is byteOffset < extent begin?  
+  //  If so, we are doing a load/store
   //  before the first valid offset in the memory region.
 
-  SVal lowerBound
-    = svalBuilder.evalBinOpNN(state, BO_LT, rawOffset.getByteOffset(),
-                              svalBuilder.makeZeroArrayIndex(),
-                              svalBuilder.getConditionType());
+  SVal extentBegin = computeExtentBegin(svalBuilder, rawOffset.getRegion());
+  
+  if (isa<NonLoc>(extentBegin)) {
+    SVal lowerBound
+      = svalBuilder.evalBinOpNN(state, BO_LT, rawOffset.getByteOffset(),
+                                cast<NonLoc>(extentBegin),
+                                svalBuilder.getConditionType());
 
-  NonLoc *lowerBoundToCheck = dyn_cast<NonLoc>(&lowerBound);
-  if (!lowerBoundToCheck)
-    return;
+    NonLoc *lowerBoundToCheck = dyn_cast<NonLoc>(&lowerBound);
+    if (!lowerBoundToCheck)
+      return;
     
-  const GRState *state_precedesLowerBound, *state_withinLowerBound;
-  llvm::tie(state_precedesLowerBound, state_withinLowerBound) =
+    const GRState *state_precedesLowerBound, *state_withinLowerBound;
+    llvm::tie(state_precedesLowerBound, state_withinLowerBound) =
       state->assume(*lowerBoundToCheck);
 
-  // Are we constrained enough to definitely precede the lower bound?
-  if (state_precedesLowerBound && !state_withinLowerBound) {
-    reportOOB(checkerContext, state_precedesLowerBound, OOB_Precedes);
-    return;
-  }
+    // Are we constrained enough to definitely precede the lower bound?
+    if (state_precedesLowerBound && !state_withinLowerBound) {
+      reportOOB(checkerContext, state_precedesLowerBound, OOB_Precedes);
+      return;
+    }
   
-  // Otherwise, assume the constraint of the lower bound.
-  assert(state_withinLowerBound);
-  state = state_withinLowerBound;
+    // Otherwise, assume the constraint of the lower bound.
+    assert(state_withinLowerBound);
+    state = state_withinLowerBound;
+  }
   
   do {
     // CHECK UPPER BOUND: Is byteOffset >= extent(baseRegion)?  If so,
@@ -232,9 +253,11 @@
   while (region) {
     switch (region->getKind()) {
       default: {
-        if (const SubRegion *subReg = dyn_cast<SubRegion>(region))
+        if (const SubRegion *subReg = dyn_cast<SubRegion>(region)) {
+          offset = getValue(offset, svalBuilder);
           if (!offset.isUnknownOrUndef())
             return RegionRawOffsetV2(subReg, offset);
+        }
         return RegionRawOffsetV2();
       }
       case MemRegion::ElementRegionKind: {

Modified: cfe/trunk/test/Analysis/out-of-bounds.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/out-of-bounds.c?rev=129366&r1=129365&r2=129366&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/out-of-bounds.c (original)
+++ cfe/trunk/test/Analysis/out-of-bounds.c Tue Apr 12 12:21:33 2011
@@ -146,3 +146,12 @@
   if (x > 99)
     buf[x] = 1; 
 }
+
+// Don't warn when indexing below the start of a symbolic region's whose
+// base extent we don't know.
+int *get_symbolic();
+void test_index_below_symboloc() {
+  int *buf = get_symbolic();
+  buf[-1] = 0; // no-warning;
+}
+