[cfe-commits] r167992 - in /cfe/trunk: lib/Sema/SemaChecking.cpp test/Analysis/additive-folding.cpp test/SemaCXX/compare.cpp

Richard Trieu rtrieu at google.com
Wed Nov 14 14:50:24 PST 2012


Author: rtrieu
Date: Wed Nov 14 16:50:24 2012
New Revision: 167992

URL: http://llvm.org/viewvc/llvm-project?rev=167992&view=rev
Log:
Improve -Wtautological-constant-out-of-range-compare by taking into account
type conversion between integers.  This allows the warning to be more accurate.

Also, turned the warning off in an analyzer test.  The relavent test cases
are covered by the tests in Sema.

Modified:
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/Analysis/additive-folding.cpp
    cfe/trunk/test/SemaCXX/compare.cpp

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=167992&r1=167991&r2=167992&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Nov 14 16:50:24 2012
@@ -4328,38 +4328,96 @@
                                          Expr *Constant, Expr *Other,
                                          llvm::APSInt Value,
                                          bool RhsConstant) {
+  // 0 values are handled later by CheckTrivialUnsignedComparison().
+  if (Value == 0)
+    return;
+
   BinaryOperatorKind op = E->getOpcode();
   QualType OtherT = Other->getType();
   QualType ConstantT = Constant->getType();
+  QualType CommonT = E->getLHS()->getType();
   if (S.Context.hasSameUnqualifiedType(OtherT, ConstantT))
     return;
   assert((OtherT->isIntegerType() && ConstantT->isIntegerType())
          && "comparison with non-integer type");
-  // FIXME. handle cases for signedness to catch (signed char)N == 200
+
+  bool ConstantSigned = ConstantT->isSignedIntegerType();
+  bool OtherSigned = OtherT->isSignedIntegerType();
+  bool CommonSigned = CommonT->isSignedIntegerType();
+
+  bool EqualityOnly = false;
+
+  // TODO: Investigate using GetExprRange() to get tighter bounds on
+  // on the bit ranges.
   IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
-  IntRange LitRange = GetValueRange(S.Context, Value, Value.getBitWidth());
-  if (OtherRange.Width >= LitRange.Width)
-    return;
+  unsigned OtherWidth = OtherRange.Width;
+  
+  if (CommonSigned) {
+    // The common type is signed, therefore no signed to unsigned conversion.
+    if (OtherSigned) {
+      // Check that the constant is representable in type OtherT.
+      if (ConstantSigned) {
+        if (OtherWidth >= Value.getMinSignedBits())
+          return;
+      } else { // !ConstantSigned
+        if (OtherWidth >= Value.getActiveBits() + 1)
+          return;
+      }
+    } else { // !OtherSigned
+      // Check that the constant is representable in type OtherT.
+      // Negative values are out of range.
+      if (ConstantSigned) {
+        if (Value.isNonNegative() && OtherWidth >= Value.getActiveBits())
+          return;
+      } else { // !ConstantSigned
+        if (OtherWidth >= Value.getActiveBits())
+          return;
+      }
+    }
+  } else {  // !CommonSigned
+    if (!OtherSigned) {
+      if (OtherWidth >= Value.getActiveBits())
+        return;
+    } else if (OtherSigned && !ConstantSigned) {
+      // Check to see if the constant is representable in OtherT.
+      if (OtherWidth > Value.getActiveBits())
+        return;
+      // Check to see if the constant is equivalent to a negative value
+      // cast to CommonT.
+      if (S.Context.getIntWidth(ConstantT) == S.Context.getIntWidth(CommonT) &&
+          Value.isNegative() && Value.getMinSignedBits() < OtherWidth)
+        return;
+      // The constant value rests between values that OtherT can represent after
+      // conversion.  Relational comparison still works, but equality
+      // comparisons will be tautological.
+      EqualityOnly = true;
+    } else { // OtherSigned && ConstantSigned
+      assert(0 && "Two signed types converted to unsigned types.");
+    }
+  }
+
+  bool PositiveConstant = !ConstantSigned || Value.isNonNegative();
+
   bool IsTrue = true;
-  if (op == BO_EQ)
-    IsTrue = false;
-  else if (op == BO_NE)
-    IsTrue = true;
-  else if (RhsConstant) {
+  if (op == BO_EQ || op == BO_NE) {
+    IsTrue = op == BO_NE;
+  } else if (EqualityOnly) {
+    return;
+  } else if (RhsConstant) {
     if (op == BO_GT || op == BO_GE)
-      IsTrue = !LitRange.NonNegative;
+      IsTrue = !PositiveConstant;
     else // op == BO_LT || op == BO_LE
-      IsTrue = LitRange.NonNegative;
+      IsTrue = PositiveConstant;
   } else {
     if (op == BO_LT || op == BO_LE)
-      IsTrue = !LitRange.NonNegative;
+      IsTrue = !PositiveConstant;
     else // op == BO_GT || op == BO_GE
-      IsTrue = LitRange.NonNegative;
+      IsTrue = PositiveConstant;
   }
   SmallString<16> PrettySourceValue(Value.toString(10));
   S.Diag(E->getOperatorLoc(), diag::warn_out_of_range_compare)
-  << PrettySourceValue << OtherT << IsTrue
-  << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
+      << PrettySourceValue << OtherT << IsTrue
+      << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
 }
 
 /// Analyze the operands of the given comparison.  Implements the

Modified: cfe/trunk/test/Analysis/additive-folding.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/additive-folding.cpp?rev=167992&r1=167991&r2=167992&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/additive-folding.cpp (original)
+++ cfe/trunk/test/Analysis/additive-folding.cpp Wed Nov 14 16:50:24 2012
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -analyzer-constraints=range -Wno-tautological-compare -Wtautological-constant-out-of-range-compare %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -analyzer-constraints=range -Wno-tautological-compare %s
 
 void clang_analyzer_eval(bool);
 
@@ -128,10 +128,10 @@
 
 // Tautologies from outside the range of the symbol
 void tautologiesOutside(unsigned char a) {
-  clang_analyzer_eval(a <= 0x100); // expected-warning{{comparison of constant 256 with expression of type 'unsigned char' is always true}} expected-warning{{TRUE}}
-  clang_analyzer_eval(a < 0x100); // expected-warning{{comparison of constant 256 with expression of type 'unsigned char' is always true}} expected-warning{{TRUE}}
+  clang_analyzer_eval(a <= 0x100); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a < 0x100); // expected-warning{{TRUE}}
 
-  clang_analyzer_eval(a != 0x100); // expected-warning{{comparison of constant 256 with expression of type 'unsigned char' is always true}} expected-warning{{TRUE}}
+  clang_analyzer_eval(a != 0x100); // expected-warning{{TRUE}}
   clang_analyzer_eval(a != -1); // expected-warning{{TRUE}}
 
   clang_analyzer_eval(a > -1); // expected-warning{{TRUE}}

Modified: cfe/trunk/test/SemaCXX/compare.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/compare.cpp?rev=167992&r1=167991&r2=167992&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/compare.cpp (original)
+++ cfe/trunk/test/SemaCXX/compare.cpp Wed Nov 14 16:50:24 2012
@@ -223,3 +223,117 @@
   (void) (true ? b : a);
   (void) (true ? (unsigned char)b : (signed char)a);
 }
+
+// Test comparison of short to unsigned.  If tautological compare does not
+// trigger, then the signed comparision warning will.
+void test4(short s) {
+  // A is max short plus 1.  All zero and positive shorts are smaller than it.
+  // All negative shorts are cast towards the max unsigned range.  Relation
+  // comparisons are possible, but equality comparisons are tautological.
+  const unsigned A = 32768;
+  void (s < A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+  void (s > A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+  void (s <= A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+  void (s >= A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+
+  void (s == A); // expected-warning{{comparison of constant 32768 with expression of type 'short' is always false}}
+  void (s != A); // expected-warning{{comparison of constant 32768 with expression of type 'short' is always true}}
+
+  // When negative one is converted to an unsigned value, it becomes the max
+  // unsigned.  Likewise, a negative one short can also be converted to max
+  // unsigned.
+  const unsigned B = -1;
+  void (s < B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+  void (s > B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+  void (s <= B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+  void (s >= B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+  void (s == B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+  void (s != B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
+
+}
+
+void test5(bool b) {
+  (void) (b < -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always false}}
+  (void) (b > -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always true}}
+  (void) (b == -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always false}}
+  (void) (b != -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always true}}
+  (void) (b <= -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always false}}
+  (void) (b >= -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always true}}
+
+  (void) (b < -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always false}}
+  (void) (b > -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always true}}
+  (void) (b == -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always false}}
+  (void) (b != -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always true}}
+  (void) (b <= -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always false}}
+  (void) (b >= -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always true}}
+
+  (void) (b < 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always true}}
+  (void) (b > 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always false}}
+  (void) (b == 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always false}}
+  (void) (b != 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always true}}
+  (void) (b <= 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always true}}
+  (void) (b >= 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always false}}
+
+  (void) (b < 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always true}}
+  (void) (b > 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always false}}
+  (void) (b == 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always false}}
+  (void) (b != 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always true}}
+  (void) (b <= 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always true}}
+  (void) (b >= 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always false}}
+}
+
+void test6(signed char sc) {
+  (void)(sc < 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
+  (void)(sc > 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
+  (void)(sc <= 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
+  (void)(sc >= 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
+  (void)(sc == 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
+  (void)(sc != 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
+
+  (void)(200 < sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
+  (void)(200 > sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
+  (void)(200 <= sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
+  (void)(200 >= sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
+  (void)(200 == sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
+  (void)(200 != sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
+}
+
+// Test many signedness combinations.
+void test7(unsigned long other) {
+  // Common unsigned, other unsigned, constant unsigned
+  (void)((unsigned)other != (unsigned long)(0x1ffffffff)); // expected-warning{{true}}
+  (void)((unsigned)other != (unsigned long)(0xffffffff));
+  (void)((unsigned long)other != (unsigned)(0x1ffffffff));
+  (void)((unsigned long)other != (unsigned)(0xffffffff));
+
+  // Common unsigned, other signed, constant unsigned
+  (void)((int)other != (unsigned long)(0xffffffffffffffff)); // expected-warning{{different signs}}
+  (void)((int)other != (unsigned long)(0x00000000ffffffff)); // expected-warning{{true}}
+  (void)((int)other != (unsigned long)(0x000000000fffffff));
+  (void)((int)other < (unsigned long)(0x00000000ffffffff));  // expected-warning{{different signs}}
+
+  // Common unsigned, other unsigned, constant signed
+  (void)((unsigned long)other != (int)(0xffffffff));  // expected-warning{{different signs}}
+
+  // Common unsigned, other signed, constant signed
+  // Should not be possible as the common type should also be signed.
+
+  // Common signed, other signed, constant signed
+  (void)((int)other != (long)(0xffffffff));  // expected-warning{{true}}
+  (void)((int)other != (long)(0xffffffff00000000));  // expected-warning{{true}}
+  (void)((int)other != (long)(0xfffffff));
+  (void)((int)other != (long)(0xfffffffff0000000));
+
+  // Common signed, other signed, constant unsigned
+  (void)((int)other != (unsigned char)(0xffff));
+  (void)((int)other != (unsigned char)(0xff));
+
+  // Common signed, other unsigned, constant signed
+  (void)((unsigned char)other != (int)(0xff));
+  (void)((unsigned char)other != (int)(0xffff));  // expected-warning{{true}}
+
+  // Common signed, other unsigned, constant unsigned
+  (void)((unsigned char)other != (unsigned short)(0xff));
+  (void)((unsigned char)other != (unsigned short)(0x100)); // expected-warning{{true}}
+  (void)((unsigned short)other != (unsigned char)(0xff));
+}





More information about the cfe-commits mailing list