From: Jordan Rose Date: Sat, 23 Mar 2013 01:21:33 +0000 (+0000) Subject: [analyzer] Teach constraint managers about unsigned comparisons. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4708b3dde86b06f40927ae9cf30a2de83949a8f2;p=clang [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 (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 27f3677bba..9502900f7e 100644 --- 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 884b0faa9e..c7e9526821 100644 --- 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 216fb3d4b0..3606e099ce 100644 --- 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 4ff248785a..f6404f0f77 100644 --- 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 7eb55ab1e1..b22eb2a5b3 100644 --- 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 33f3b9b251..c2e502623e 100644 --- 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