[analyzer] Teach constraint managers about unsigned comparisons.

In C, comparisons between signed and unsigned numbers are always done in
unsigned-space. Thus, we should know that "i >= 0U" is always true, even
if 'i' is signed. Similarly, "u >= 0" is also always true, even though '0'
is signed.

Part of <rdar://problem/13239003> (false positives related to std::vector)

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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h b/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h
index 27f3677bba2237cea1546e3642ee81ababcea569..9502900f7e35cbfbb05407ceb8bc449ba4831578 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h
@@ -81,9 +81,12 @@ public:
 
   /// Tests whether a given value is losslessly representable using this type.
   ///
-  /// Note that signedness conversions will be rejected, even with the same bit
-  /// pattern. For example, -1s8 is not in range for 'unsigned char' (u8).
-  RangeTestResultKind testInRange(const llvm::APSInt &Val) const LLVM_READONLY;
+  /// \param Val The value to test.
+  /// \param AllowMixedSign Whether or not to allow signedness conversions.
+  ///                       This determines whether -1s8 is considered in range
+  ///                       for 'unsigned char' (u8).
+  RangeTestResultKind testInRange(const llvm::APSInt &Val,
+                                  bool AllowMixedSign) const LLVM_READONLY;
   
   bool operator==(const APSIntType &Other) const {
     return BitWidth == Other.BitWidth && IsUnsigned == Other.IsUnsigned;

diff --git a/lib/StaticAnalyzer/Core/APSIntType.cpp b/lib/StaticAnalyzer/Core/APSIntType.cpp
index 884b0faa9ed45b563aad31d62bd4bd35893d16a8..c7e95268213e7ea1aeb6fb2b42cbd41669308bfb 100644 (file)
--- a/lib/StaticAnalyzer/Core/APSIntType.cpp
+++ b/lib/StaticAnalyzer/Core/APSIntType.cpp
@@ -13,20 +13,31 @@ using namespace clang;
 using namespace ento;
 
 APSIntType::RangeTestResultKind
-APSIntType::testInRange(const llvm::APSInt &Value) const {
+APSIntType::testInRange(const llvm::APSInt &Value,
+                        bool AllowSignConversions) const {
+
   // Negative numbers cannot be losslessly converted to unsigned type.
-  if (IsUnsigned && Value.isSigned() && Value.isNegative())
+  if (IsUnsigned && !AllowSignConversions &&
+      Value.isSigned() && Value.isNegative())
     return RTR_Below;
 
-  // Signed integers can be converted to signed integers of the same width
-  // or (if positive) unsigned integers with one fewer bit.
-  // Unsigned integers can be converted to unsigned integers of the same width
-  // or signed integers with one more bit.
   unsigned MinBits;
-  if (Value.isSigned())
-    MinBits = Value.getMinSignedBits() - IsUnsigned;
-  else
-    MinBits = Value.getActiveBits() + !IsUnsigned;
+  if (AllowSignConversions) {
+    if (Value.isSigned() && !IsUnsigned)
+      MinBits = Value.getMinSignedBits();
+    else
+      MinBits = Value.getActiveBits();
+
+  } else {
+    // Signed integers can be converted to signed integers of the same width
+    // or (if positive) unsigned integers with one fewer bit.
+    // Unsigned integers can be converted to unsigned integers of the same width
+    // or signed integers with one more bit.
+    if (Value.isSigned())
+      MinBits = Value.getMinSignedBits() - IsUnsigned;
+    else
+      MinBits = Value.getActiveBits() + !IsUnsigned;
+  }
 
   if (MinBits <= BitWidth)
     return RTR_Within;

diff --git a/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp b/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index 216fb3d4b0386fd5185a418b4540099f6b631147..3606e099cec2ee1f08e58bc8a5938888fb7531e1 100644 (file)
--- a/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -153,8 +153,8 @@ private:
     // The function returns false if the described range is entirely outside
     // the range of values for the associated symbol.
     APSIntType Type(getMinValue());
-    APSIntType::RangeTestResultKind LowerTest = Type.testInRange(Lower);
-    APSIntType::RangeTestResultKind UpperTest = Type.testInRange(Upper);
+    APSIntType::RangeTestResultKind LowerTest = Type.testInRange(Lower, true);
+    APSIntType::RangeTestResultKind UpperTest = Type.testInRange(Upper, true);
 
     switch (LowerTest) {
     case APSIntType::RTR_Below:
@@ -419,7 +419,7 @@ RangeConstraintManager::assumeSymNE(ProgramStateRef St, SymbolRef Sym,
                                     const llvm::APSInt &Adjustment) {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
-  if (AdjustmentType.testInRange(Int) != APSIntType::RTR_Within)
+  if (AdjustmentType.testInRange(Int, true) != APSIntType::RTR_Within)
     return St;
 
   llvm::APSInt Lower = AdjustmentType.convert(Int) - Adjustment;
@@ -439,7 +439,7 @@ RangeConstraintManager::assumeSymEQ(ProgramStateRef St, SymbolRef Sym,
                                     const llvm::APSInt &Adjustment) {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
-  if (AdjustmentType.testInRange(Int) != APSIntType::RTR_Within)
+  if (AdjustmentType.testInRange(Int, true) != APSIntType::RTR_Within)
     return NULL;
 
   // [Int-Adjustment, Int-Adjustment]
@@ -454,7 +454,7 @@ RangeConstraintManager::assumeSymLT(ProgramStateRef St, SymbolRef Sym,
                                     const llvm::APSInt &Adjustment) {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
-  switch (AdjustmentType.testInRange(Int)) {
+  switch (AdjustmentType.testInRange(Int, true)) {
   case APSIntType::RTR_Below:
     return NULL;
   case APSIntType::RTR_Within:
@@ -483,7 +483,7 @@ RangeConstraintManager::assumeSymGT(ProgramStateRef St, SymbolRef Sym,
                                     const llvm::APSInt &Adjustment) {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
-  switch (AdjustmentType.testInRange(Int)) {
+  switch (AdjustmentType.testInRange(Int, true)) {
   case APSIntType::RTR_Below:
     return St;
   case APSIntType::RTR_Within:
@@ -512,7 +512,7 @@ RangeConstraintManager::assumeSymGE(ProgramStateRef St, SymbolRef Sym,
                                     const llvm::APSInt &Adjustment) {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
-  switch (AdjustmentType.testInRange(Int)) {
+  switch (AdjustmentType.testInRange(Int, true)) {
   case APSIntType::RTR_Below:
     return St;
   case APSIntType::RTR_Within:
@@ -541,7 +541,7 @@ RangeConstraintManager::assumeSymLE(ProgramStateRef St, SymbolRef Sym,
                                     const llvm::APSInt &Adjustment) {
   // Before we do any real work, see if the value can even show up.
   APSIntType AdjustmentType(Adjustment);
-  switch (AdjustmentType.testInRange(Int)) {
+  switch (AdjustmentType.testInRange(Int, true)) {
   case APSIntType::RTR_Below:
     return NULL;
   case APSIntType::RTR_Within:

diff --git a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
index 4ff248785a55825d3028134aed02c07da610f5bd..f6404f0f7773ea1da2b1e4c2f809a80a2cdd424d 100644 (file)
--- a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -264,10 +264,14 @@ ProgramStateRef SimpleConstraintManager::assumeSymRel(ProgramStateRef state,
   APSIntType ComparisonType = std::max(WraparoundType, APSIntType(Int));
   llvm::APSInt ConvertedInt = ComparisonType.convert(Int);
 
+  // Prefer unsigned comparisons.
+  if (ComparisonType.getBitWidth() == WraparoundType.getBitWidth() &&
+      ComparisonType.isUnsigned() && !WraparoundType.isUnsigned())
+    Adjustment.setIsSigned(false);
+
   switch (op) {
   default:
-    // No logic yet for other operators.  assume the constraint is feasible.
-    return state;
+    llvm_unreachable("invalid operation not caught by assertion above");
 
   case BO_EQ:
     return assumeSymEQ(state, Sym, ConvertedInt, Adjustment);

diff --git a/test/Analysis/additive-folding-range-constraints.c b/test/Analysis/additive-folding-range-constraints.c
index 7eb55ab1e1253fe94037e7a995d59182f5589ee8..b22eb2a5b345ecbca984d7c61edc1f37cd5405f7 100644 (file)
--- a/test/Analysis/additive-folding-range-constraints.c
+++ b/test/Analysis/additive-folding-range-constraints.c
@@ -170,3 +170,135 @@ void mixedComparisons9(signed char a) {
   clang_analyzer_eval(a == 0x7F); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(a == -0x80); // expected-warning{{UNKNOWN}}
 }
+
+
+void mixedSignedness1(int a) {
+  unsigned max = UINT_MAX;
+  clang_analyzer_eval(a < max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) < max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) < max); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness2(int a) {
+  unsigned max = UINT_MAX;
+  clang_analyzer_eval(a <= max); // expected-warning{{TRUE}}
+  clang_analyzer_eval((a + 2) <= max); // expected-warning{{TRUE}}
+  clang_analyzer_eval((a + 2U) <= max); // expected-warning{{TRUE}}
+}
+
+void mixedSignedness3(unsigned a) {
+  int max = INT_MAX;
+  clang_analyzer_eval(a < max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) < max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) < max); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness4(unsigned a) {
+  int max = INT_MAX;
+  clang_analyzer_eval(a <= max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) <= max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) <= max); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness5(unsigned a) {
+  int min = INT_MIN;
+  clang_analyzer_eval(a < min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) < min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) < min); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness6(unsigned a) {
+  int min = INT_MIN;
+  clang_analyzer_eval(a <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) <= min); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness7(unsigned a) {
+  unsigned min = 0;
+  clang_analyzer_eval(a < min); // expected-warning{{FALSE}}
+  clang_analyzer_eval((a + 2) < min); // expected-warning{{FALSE}}
+  clang_analyzer_eval((a + 2U) < min); // expected-warning{{FALSE}}
+}
+
+void mixedSignedness8(unsigned a) {
+  unsigned min = 0;
+  clang_analyzer_eval(a <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) <= min); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness9(unsigned a) {
+  int min = 0;
+  clang_analyzer_eval(a < min); // expected-warning{{FALSE}}
+  clang_analyzer_eval((a + 2) < min); // expected-warning{{FALSE}}
+  clang_analyzer_eval((a + 2U) < min); // expected-warning{{FALSE}}
+}
+
+void mixedSignedness10(unsigned a) {
+  int min = 0;
+  clang_analyzer_eval(a <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) <= min); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness11(int a) {
+  int min = 0;
+  clang_analyzer_eval(a < min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) < min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) < min); // expected-warning{{FALSE}}
+}
+
+void mixedSignedness12(int a) {
+  int min = 0;
+  clang_analyzer_eval(a <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) <= min); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness13(int a) {
+  unsigned max = INT_MAX;
+  clang_analyzer_eval(a < max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) < max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) < max); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness14(int a) {
+  unsigned max = INT_MAX;
+  clang_analyzer_eval(a <= max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) <= max); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) <= max); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness15(int a) {
+  unsigned min = INT_MIN;
+  clang_analyzer_eval(a < min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) < min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) < min); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness16(int a) {
+  unsigned min = INT_MIN;
+  clang_analyzer_eval(a <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2) <= min); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval((a + 2U) <= min); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness17(int a) {
+  unsigned max = INT_MAX;
+  if (a < max)
+    return;
+
+  clang_analyzer_eval(a < 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a == 0); // expected-warning{{FALSE}}
+  clang_analyzer_eval(a == INT_MAX); // expected-warning{{UNKNOWN}}
+}
+
+void mixedSignedness18(int a) {
+  if (a >= 0)
+    return;
+
+  clang_analyzer_eval(a < 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a == (unsigned)INT_MIN); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a == UINT_MAX); // expected-warning{{UNKNOWN}}
+}

diff --git a/test/Analysis/additive-folding.cpp b/test/Analysis/additive-folding.cpp
index 33f3b9b251772e76c946e2048f3ca0be22d335d6..c2e502623e94b7a1b3baa01a611e919ac46bd1ae 100644 (file)
--- a/test/Analysis/additive-folding.cpp
+++ b/test/Analysis/additive-folding.cpp
@@ -184,6 +184,18 @@ void mixedSignedness(int a, unsigned b) {
   clang_analyzer_eval(b == uMin && b != sMin); // expected-warning{{FALSE}}
 }
 
+void mixedSignedness2(int a) {
+  if (a != -1)
+    return;
+  clang_analyzer_eval(a == UINT_MAX); // expected-warning{{TRUE}}
+}
+
+void mixedSignedness3(unsigned a) {
+  if (a != UINT_MAX)
+    return;
+  clang_analyzer_eval(a == -1); // expected-warning{{TRUE}}
+}
+
 
 void multiplicativeSanityTest(int x) {
   // At one point we were ignoring the *4 completely -- the constraint manager