[analyzer] Use a more generic MemRegion.getAsOffset to evaluate bin operators on...

In addition to enabling more code reuse, this suppresses some false positives by allowing us to
compare an element region to its base. See the ptr-arith.cpp test cases for an example.

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

diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index ee627f2baa352e9c74255426c3baf47f0e785c4e..cb9e451b24a2fd7ccd61257130d5e91c8f4fb165 100644 (file)
--- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -507,6 +507,53 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
   }
 }
 
+static SVal evalBinOpFieldRegionFieldRegion(const FieldRegion *LeftFR,
+                                            const FieldRegion *RightFR,
+                                            BinaryOperator::Opcode op,
+                                            QualType resultTy,
+                                            SimpleSValBuilder &SVB) {
+  // Only comparisons are meaningful here!
+  if (!BinaryOperator::isComparisonOp(op))
+    return UnknownVal();
+
+  // Next, see if the two FRs have the same super-region.
+  // FIXME: This doesn't handle casts yet, and simply stripping the casts
+  // doesn't help.
+  if (LeftFR->getSuperRegion() != RightFR->getSuperRegion())
+    return UnknownVal();
+
+  const FieldDecl *LeftFD = LeftFR->getDecl();
+  const FieldDecl *RightFD = RightFR->getDecl();
+  const RecordDecl *RD = LeftFD->getParent();
+
+  // Make sure the two FRs are from the same kind of record. Just in case!
+  // FIXME: This is probably where inheritance would be a problem.
+  if (RD != RightFD->getParent())
+    return UnknownVal();
+
+  // We know for sure that the two fields are not the same, since that
+  // would have given us the same SVal.
+  if (op == BO_EQ)
+    return SVB.makeTruthVal(false, resultTy);
+  if (op == BO_NE)
+    return SVB.makeTruthVal(true, resultTy);
+
+  // Iterate through the fields and see which one comes first.
+  // [C99 6.7.2.1.13] "Within a structure object, the non-bit-field
+  // members and the units in which bit-fields reside have addresses that
+  // increase in the order in which they are declared."
+  bool leftFirst = (op == BO_LT || op == BO_LE);
+  for (RecordDecl::field_iterator I = RD->field_begin(),
+       E = RD->field_end(); I!=E; ++I) {
+    if (*I == LeftFD)
+      return SVB.makeTruthVal(leftFirst, resultTy);
+    if (*I == RightFD)
+      return SVB.makeTruthVal(!leftFirst, resultTy);
+  }
+
+  llvm_unreachable("Fields not found in parent record's definition");
+}
+
 // FIXME: all this logic will change if/when we have MemRegion::getLocation().
 SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
                                   BinaryOperator::Opcode op,
@@ -699,14 +746,10 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
       }
     }
 
-    // FIXME: If/when there is a getAsRawOffset() for FieldRegions, this
-    // ElementRegion path and the FieldRegion path below should be unified.
-    if (const ElementRegion *LeftER = dyn_cast<ElementRegion>(LeftMR)) {
-      // First see if the right region is also an ElementRegion.
-      const ElementRegion *RightER = dyn_cast<ElementRegion>(RightMR);
-      if (!RightER)
-        return UnknownVal();
-
+    // Handle special cases for when both regions are element regions.
+    const ElementRegion *RightER = dyn_cast<ElementRegion>(RightMR);
+    const ElementRegion *LeftER = dyn_cast<ElementRegion>(LeftMR);
+    if (RightER && LeftER) {
       // Next, see if the two ERs have the same super-region and matching types.
       // FIXME: This should do something useful even if the types don't match,
       // though if both indexes are constant the RegionRawOffset path will
@@ -738,17 +781,29 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
         // evalBinOpNN expects the two indexes to already be the right type.
         return evalBinOpNN(state, op, *LeftIndex, *RightIndex, resultTy);
       }
+    }
 
-      // If the element indexes aren't comparable, see if the raw offsets are.
-      RegionRawOffset LeftOffset = LeftER->getAsArrayOffset();
-      RegionRawOffset RightOffset = RightER->getAsArrayOffset();
+    // Special handling of the FieldRegions, even with symbolic offsets.
+    const FieldRegion *RightFR = dyn_cast<FieldRegion>(RightMR);
+    const FieldRegion *LeftFR = dyn_cast<FieldRegion>(LeftMR);
+    if (RightFR && LeftFR) {
+      SVal R = evalBinOpFieldRegionFieldRegion(LeftFR, RightFR, op, resultTy,
+                                               *this);
+      if (!R.isUnknown())
+        return R;
+    }
 
-      if (LeftOffset.getRegion() != NULL &&
-          LeftOffset.getRegion() == RightOffset.getRegion()) {
-        CharUnits left = LeftOffset.getOffset();
-        CharUnits right = RightOffset.getOffset();
+    // Compare the regions using the raw offsets.
+    RegionOffset LeftOffset = LeftMR->getAsOffset();
+    RegionOffset RightOffset = RightMR->getAsOffset();
 
-        switch (op) {
+    if (LeftOffset.getRegion() != NULL &&
+        LeftOffset.getRegion() == RightOffset.getRegion() &&
+        !LeftOffset.hasSymbolicOffset() && !RightOffset.hasSymbolicOffset()) {
+      int64_t left = LeftOffset.getOffset();
+      int64_t right = RightOffset.getOffset();
+
+      switch (op) {
         default:
           return UnknownVal();
         case BO_LT:
@@ -763,60 +818,7 @@ SVal SimpleSValBuilder::evalBinOpLL(ProgramStateRef state,
           return makeTruthVal(left == right, resultTy);
         case BO_NE:
           return makeTruthVal(left != right, resultTy);
-        }
       }
-
-      // If we get here, we have no way of comparing the ElementRegions.
-    }
-
-    // See if both regions are fields of the same structure.
-    // FIXME: This doesn't handle nesting, inheritance, or Objective-C ivars.
-    if (const FieldRegion *LeftFR = dyn_cast<FieldRegion>(LeftMR)) {
-      // Only comparisons are meaningful here!
-      if (!BinaryOperator::isComparisonOp(op))
-        return UnknownVal();
-
-      // First see if the right region is also a FieldRegion.
-      const FieldRegion *RightFR = dyn_cast<FieldRegion>(RightMR);
-      if (!RightFR)
-        return UnknownVal();
-
-      // Next, see if the two FRs have the same super-region.
-      // FIXME: This doesn't handle casts yet, and simply stripping the casts
-      // doesn't help.
-      if (LeftFR->getSuperRegion() != RightFR->getSuperRegion())
-        return UnknownVal();
-
-      const FieldDecl *LeftFD = LeftFR->getDecl();
-      const FieldDecl *RightFD = RightFR->getDecl();
-      const RecordDecl *RD = LeftFD->getParent();
-
-      // Make sure the two FRs are from the same kind of record. Just in case!
-      // FIXME: This is probably where inheritance would be a problem.
-      if (RD != RightFD->getParent())
-        return UnknownVal();
-
-      // We know for sure that the two fields are not the same, since that
-      // would have given us the same SVal.
-      if (op == BO_EQ)
-        return makeTruthVal(false, resultTy);
-      if (op == BO_NE)
-        return makeTruthVal(true, resultTy);
-
-      // Iterate through the fields and see which one comes first.
-      // [C99 6.7.2.1.13] "Within a structure object, the non-bit-field
-      // members and the units in which bit-fields reside have addresses that
-      // increase in the order in which they are declared."
-      bool leftFirst = (op == BO_LT || op == BO_LE);
-      for (RecordDecl::field_iterator I = RD->field_begin(),
-           E = RD->field_end(); I!=E; ++I) {
-        if (*I == LeftFD)
-          return makeTruthVal(leftFirst, resultTy);
-        if (*I == RightFD)
-          return makeTruthVal(!leftFirst, resultTy);
-      }
-
-      llvm_unreachable("Fields not found in parent record's definition");
     }
 
     // At this point we're not going to get a good answer, but we can try

diff --git a/test/Analysis/ptr-arith.c b/test/Analysis/ptr-arith.c
index 35faff4a170978e1c7083b4f5b2c085a0db07da3..4a15bc24b90689bf56ca872944048f857af9e0c1 100644 (file)
--- a/test/Analysis/ptr-arith.c
+++ b/test/Analysis/ptr-arith.c
@@ -280,3 +280,19 @@ void canonical_equal(int *lhs, int *rhs) {
   // FIXME: Should be FALSE.
   clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
 }
+
+void compare_element_region_and_base(int *p) {
+  int *q = p - 1;
+  clang_analyzer_eval(p == q); // expected-warning{{FALSE}}
+}
+
+struct Point {
+  int x;
+  int y;
+};
+void symbolicFieldRegion(struct Point *points, int i, int j) {
+  clang_analyzer_eval(&points[i].x == &points[j].x);// expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(&points[i].x == &points[i].y);// expected-warning{{FALSE}}
+  clang_analyzer_eval(&points[i].x < &points[i].y);// expected-warning{{TRUE}}
+}
+

diff --git a/test/Analysis/ptr-arith.cpp b/test/Analysis/ptr-arith.cpp
new file mode 100644 (file)
index 0000000..5f09518
--- /dev/null
+++ b/test/Analysis/ptr-arith.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+// expected-no-diagnostics
+struct X {
+  int *p;
+  int zero;
+  void foo () {
+    reset(p - 1);
+  }
+  void reset(int *in) {
+    while (in != p) // Loop must be entered.
+      zero = 1;
+  }
+};
+
+int test (int *in) {
+  X littleX;
+  littleX.zero = 0;
+  littleX.p = in;
+  littleX.foo();
+  return 5/littleX.zero; // no-warning
+}
+