[analyzer] Equality ops are like relational ops in that the arguments shouldn't be...

This was probably the original intent of r133041 (also me, a year ago).

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

diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 4a4fcf3c1fc6faee671a29f6c5faf7c0fa3d8352..057b8c0adb851837ac87c0debb3a48542a69976b 100644 (file)
--- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -345,7 +345,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
           if (const llvm::APSInt *Constant = state->getSymVal(RSym)) {
             // The symbol evaluates to a constant.
             const llvm::APSInt *rhs_I;
-            if (BinaryOperator::isRelationalOp(op))
+            if (BinaryOperator::isComparisonOp(op))
               rhs_I = &BasicVals.Convert(lhsInt.getValue(), *Constant);
             else
               rhs_I = &BasicVals.Convert(resultTy, *Constant);
@@ -494,7 +494,7 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
         // The conversion type is usually the result type, but not in the case
         // of relational expressions.
         QualType conversionType = resultTy;
-        if (BinaryOperator::isRelationalOp(op))
+        if (BinaryOperator::isComparisonOp(op))
           conversionType = lhsType;
 
         // Does the symbol simplify to a constant?  If so, "fold" the constant

diff --git a/test/Analysis/additive-folding.c b/test/Analysis/additive-folding.cpp
similarity index 71%
rename from test/Analysis/additive-folding.c
rename to test/Analysis/additive-folding.cpp
index 9a51d279ff9ca387694af2179cae6a51a2bf8079..1a87ccfe7056c4b5bc4dbe5fe7dd809432e7c9fd 100644 (file)
--- a/test/Analysis/additive-folding.c
+++ b/test/Analysis/additive-folding.cpp
@@ -201,3 +201,42 @@ void tautologyLE (unsigned a) {
     free(b);
   return; // no-warning
 }
+
+
+// PR12206/12510 - When SimpleSValBuilder figures out that a symbol is fully
+// constrained, it should cast the value to the result type in a binary
+// operation...unless the binary operation is a comparison, in which case the
+// two arguments should be the same type, but won't match the result type.
+//
+// This is easier to trigger in C++ mode, where the comparison result type is
+// 'bool' and is thus differently sized from int on pretty much every system.
+//
+// This is not directly related to additive folding, but we use SValBuilder's
+// additive folding to tickle the bug. ExprEngine will simplify fully-constrained
+// symbols, so SValBuilder will only see them if they are (a) part of an evaluated
+// SymExpr (e.g. with additive folding) or (b) generated by a checker (e.g.
+// unix.cstring's strlen() modelling).
+void PR12206(int x) {
+  // Build a SymIntExpr, dependent on x.
+  int local = x - 1;
+
+  // Constrain the value of x.
+  int value = 1 + (1 << (8 * sizeof(1 == 1))); // not representable by bool
+  if (x != value) return;
+
+  // Constant-folding will turn (local+1) back into the symbol for x.
+  // The point of this dance is to make SValBuilder be responsible for
+  // turning the symbol into a ConcreteInt, rather than ExprEngine.
+
+  // Test relational operators.
+  if ((local + 1) < 2)
+    malloc(1); // expected-warning{{never executed}}
+  if (2 > (local + 1))
+    malloc(1); // expected-warning{{never executed}}
+
+  // Test equality operators.
+  if ((local + 1) == 1) 
+    malloc(1); // expected-warning{{never executed}}
+  if (1 == (local + 1))
+    malloc(1); // expected-warning{{never executed}}
+}

diff --git a/test/Analysis/string.c b/test/Analysis/string.c
index c0814b89c17c6f9dcd66a79f95f3e8de2aca4efd..24e29ebb02b39806919777e2eeccd72dcc7e0790 100644 (file)
--- a/test/Analysis/string.c
+++ b/test/Analysis/string.c
@@ -1122,3 +1122,35 @@ void strncasecmp_embedded_null () {
        if (strncasecmp("ab\0zz", "ab\0yy", 4) != 0)
                (void)*(char*)0; // no-warning
 }
+
+//===----------------------------------------------------------------------===
+// Miscellaneous extras.
+//===----------------------------------------------------------------------===
+
+// See additive-folding.cpp for a description of this bug.
+// This test is insurance in case we significantly change how SymExprs are
+// evaluated. It isn't as good as additive-folding.cpp's version
+// because it will only actually be a test on systems where
+//   sizeof(1 == 1) < sizeof(size_t).
+// We could add a triple if it becomes necessary.
+void PR12206(const char *x) {
+  // This test is only useful under these conditions.
+  size_t comparisonSize = sizeof(1 == 1);
+  if (sizeof(size_t) <= comparisonSize) return;
+
+  // Create a value that requires more bits to store than a comparison result.
+  size_t value = 1UL;
+  value <<= 8 * comparisonSize;
+  value += 1;
+
+  // Constrain the length of x.
+  if (strlen(x) != value) return;
+
+  // Test relational operators.
+  if (strlen(x) < 2) { (void)*(char*)0; } // no-warning
+  if (2 > strlen(x)) { (void)*(char*)0; } // no-warning
+
+  // Test equality operators.
+  if (strlen(x) == 1) { (void)*(char*)0; } // no-warning
+  if (1 == strlen(x)) { (void)*(char*)0; } // no-warning
+}