[cfe-commits] r156062 - in /cfe/trunk: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp test/Analysis/additive-folding.c test/Analysis/additive-folding.cpp test/Analysis/string.c

Jordy Rose jediknil at belkadan.com
Thu May 3 00:34:01 PDT 2012


Author: jrose
Date: Thu May  3 02:34:01 2012
New Revision: 156062

URL: http://llvm.org/viewvc/llvm-project?rev=156062&view=rev
Log:
[analyzer] Equality ops are like relational ops in that the arguments shouldn't be converted to the result type. Fixes PR12206 and dupe PR12510.

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

Added:
    cfe/trunk/test/Analysis/additive-folding.cpp
      - copied, changed from r156061, cfe/trunk/test/Analysis/additive-folding.c
Removed:
    cfe/trunk/test/Analysis/additive-folding.c
Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
    cfe/trunk/test/Analysis/string.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp?rev=156062&r1=156061&r2=156062&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp Thu May  3 02:34:01 2012
@@ -345,7 +345,7 @@
           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 @@
         // 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

Removed: cfe/trunk/test/Analysis/additive-folding.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/additive-folding.c?rev=156061&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/additive-folding.c (original)
+++ cfe/trunk/test/Analysis/additive-folding.c (removed)
@@ -1,203 +0,0 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.deadcode.UnreachableCode,unix.Malloc -verify -analyzer-constraints=basic %s
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,experimental.deadcode.UnreachableCode,unix.Malloc -verify -analyzer-constraints=range %s
-
-// These are used to trigger warnings.
-typedef typeof(sizeof(int)) size_t;
-void *malloc(size_t);
-void free(void *);
-#define NULL ((void*)0)
-#define UINT_MAX (~0U)
-
-//---------------
-//  Plus/minus
-//---------------
-
-void separateExpressions (int a) {
-  int b = a + 1;
-  --b;
-
-  void *buf = malloc(1);
-  if (a != 0 && b == 0)
-    return; // expected-warning{{never executed}}
-  free(buf);
-}
-
-void oneLongExpression (int a) {
-  // Expression canonicalization should still allow this to work, even though
-  // the first term is on the left.
-  int b = 15 + a + 15 - 10 - 20;
-
-  void *buf = malloc(1);
-  if (a != 0 && b == 0)
-    return; // expected-warning{{never executed}}
-  free(buf);
-}
-
-void mixedTypes (int a) {
-  void *buf = malloc(1);
-
-  // Different additive types should not cause crashes when constant-folding.
-  // This is part of PR7406.
-  int b = a + 1LL;
-  if (a != 0 && (b-1) == 0) // not crash
-    return; // expected-warning{{never executed}}
-
-  int c = a + 1U;
-  if (a != 0 && (c-1) == 0) // not crash
-    return; // expected-warning{{never executed}}
-
-  free(buf);
-}
-
-//---------------
-//  Comparisons
-//---------------
-
-// Equality and inequality only
-void eq_ne (unsigned a) {
-  void *b = NULL;
-  if (a == UINT_MAX)
-    b = malloc(1);
-  if (a+1 != 0)
-    return; // no-warning
-  if (a-1 != UINT_MAX-1)
-    return; // no-warning
-  free(b);
-}
-
-void ne_eq (unsigned a) {
-  void *b = NULL;
-  if (a != UINT_MAX)
-    b = malloc(1);
-  if (a+1 == 0)
-    return; // no-warning
-  if (a-1 == UINT_MAX-1)
-    return; // no-warning
-  free(b);
-}
-
-// Mixed typed inequalities (part of PR7406)
-// These should not crash.
-void mixed_eq_ne (int a) {
-  void *b = NULL;
-  if (a == 1)
-    b = malloc(1);
-  if (a+1U != 2)
-    return; // no-warning
-  if (a-1U != 0)
-    return; // expected-warning{{never executed}}
-  free(b);
-}
-
-void mixed_ne_eq (int a) {
-  void *b = NULL;
-  if (a != 1)
-    b = malloc(1);
-  if (a+1U == 2)
-    return; // no-warning
-  if (a-1U == 0)
-    return; // expected-warning{{never executed}}
-  free(b);
-}
-
-
-// Simple order comparisons with no adjustment
-void baselineGT (unsigned a) {
-  void *b = NULL;
-  if (a > 0)
-    b = malloc(1);
-  if (a == 0)
-    return; // no-warning
-  free(b);
-}
-
-void baselineGE (unsigned a) {
-  void *b = NULL;
-  if (a >= UINT_MAX)
-    b = malloc(1);
-  if (a == UINT_MAX)
-    free(b);
-  return; // no-warning
-}
-
-void baselineLT (unsigned a) {
-  void *b = NULL;
-  if (a < UINT_MAX)
-    b = malloc(1);
-  if (a == UINT_MAX)
-    return; // no-warning
-  free(b);
-}
-
-void baselineLE (unsigned a) {
-  void *b = NULL;
-  if (a <= 0)
-    b = malloc(1);
-  if (a == 0)
-    free(b);
-  return; // no-warning
-}
-
-
-// Adjustment gives each of these an extra solution!
-void adjustedGT (unsigned a) {
-  void *b = NULL;
-  if (a-1 > UINT_MAX-1)
-    b = malloc(1);
-  return; // expected-warning{{leak}}
-}
-
-void adjustedGE (unsigned a) {
-  void *b = NULL;
-  if (a-1 >= UINT_MAX-1)
-    b = malloc(1);
-  if (a == UINT_MAX)
-    free(b);
-  return; // expected-warning{{leak}}
-}
-
-void adjustedLT (unsigned a) {
-  void *b = NULL;
-  if (a+1 < 1)
-    b = malloc(1);
-  return; // expected-warning{{leak}}
-}
-
-void adjustedLE (unsigned a) {
-  void *b = NULL;
-  if (a+1 <= 1)
-    b = malloc(1);
-  if (a == 0)
-    free(b);
-  return; // expected-warning{{leak}}
-}
-
-
-// Tautologies
-void tautologyGT (unsigned a) {
-  void *b = malloc(1);
-  if (a > UINT_MAX)
-    return; // no-warning
-  free(b);
-}
-
-void tautologyGE (unsigned a) {
-  void *b = malloc(1);
-  if (a >= 0) // expected-warning{{always true}}
-    free(b);
-  return; // no-warning
-}
-
-void tautologyLT (unsigned a) {
-  void *b = malloc(1);
-  if (a < 0) // expected-warning{{always false}}
-    return; // expected-warning{{never executed}}
-  free(b);
-}
-
-void tautologyLE (unsigned a) {
-  void *b = malloc(1);
-  if (a <= UINT_MAX)
-    free(b);
-  return; // no-warning
-}

Copied: cfe/trunk/test/Analysis/additive-folding.cpp (from r156061, cfe/trunk/test/Analysis/additive-folding.c)
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/additive-folding.cpp?p2=cfe/trunk/test/Analysis/additive-folding.cpp&p1=cfe/trunk/test/Analysis/additive-folding.c&r1=156061&r2=156062&rev=156062&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/additive-folding.c (original)
+++ cfe/trunk/test/Analysis/additive-folding.cpp Thu May  3 02:34:01 2012
@@ -201,3 +201,42 @@
     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}}
+}

Modified: cfe/trunk/test/Analysis/string.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/string.c?rev=156062&r1=156061&r2=156062&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/string.c (original)
+++ cfe/trunk/test/Analysis/string.c Thu May  3 02:34:01 2012
@@ -1122,3 +1122,35 @@
 	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
+}





More information about the cfe-commits mailing list