Fix #15000: Propagate integral cast ranges to condition analysis

Impossible bounds on integral casts must not be propagated through
arithmetic with unsigned result type, because wrap-around invalidates
the bounds. '(uint32_t)x - 1u == 0xFFFFFFFFu' must not be reported as
an always-false condition, since 0u - 1u wraps to 4294967295.

- Filter impossible integral bounds from the operand values before
  interval inference in valueFlowInferCondition() when the arithmetic
  result type is an unsigned integer (+, -, *).
- Skip impossible-bounded operand pairs in the binary calculation
  branch of setTokenValue() under the same result-type predicate.
- Known and possible values, impossible point values, comparisons,
  pointer arithmetic and arithmetic with signed result type are
  preserved.

Author: jfdeverge

lib/valueflow.cpp

@@ -1087,6 +1087,24 @@ static void valueFlowImpossibleValues(TokenList& tokenList, const Settings& sett
             upper.bound = ValueFlow::Value::Bound::Lower;
             upper.setImpossible();
             setTokenValue(tok, std::move(upper), settings);
+        } else if (tok->isCast() && tok->valueType() && tok->valueType()->isIntegral() && !tok->valueType()->pointer) {
+            MathLib::bigint minValue;
+            MathLib::bigint maxValue;
+            if (!ValueFlow::getMinMaxValues(tok->valueType(), settings.platform, minValue, maxValue))
+                continue;
+
+            if (minValue > std::numeric_limits<MathLib::bigint>::min()) {
+                ValueFlow::Value lower{minValue - 1};
+                lower.bound = ValueFlow::Value::Bound::Upper;
+                lower.setImpossible();
+                setTokenValue(tok, std::move(lower), settings);
+            }
+            if (maxValue < std::numeric_limits<MathLib::bigint>::max()) {
+                ValueFlow::Value upper{maxValue + 1};
+                upper.bound = ValueFlow::Value::Bound::Lower;
+                upper.setImpossible();
+                setTokenValue(tok, std::move(upper), settings);
+            }
         } else if (astIsUnsigned(tok) && !astIsPointer(tok)) {
             std::vector<MathLib::bigint> minvalue = minUnsignedValue(tok);
             if (minvalue.empty())
@@ -5134,6 +5152,26 @@ static bool isIntegralOrPointer(const Token* tok)
     return false;
 }
 
+/**
+ * @brief Check if the token is an arithmetic operation whose result type is
+ * an unsigned integer, i.e. arithmetic that may wrap around.
+ *
+ * Impossible bounds cannot be propagated through such arithmetic because
+ * wrap-around invalidates the bounds.
+ */
+static bool isUnsignedArithmeticResult(const Token* tok)
+{
+    if (!Token::Match(tok, "+|-|*"))
+        return false;
+    const ValueType* vt = tok->valueType();
+    return vt && vt->isIntegral() && vt->pointer == 0 &&
+           vt->sign == ValueType::Sign::UNSIGNED &&
+           (vt->type == ValueType::Type::INT ||
+            vt->type == ValueType::Type::LONG ||
+            vt->type == ValueType::Type::LONGLONG ||
+            vt->type == ValueType::Type::UNKNOWN_INT);
+}
+
 static void valueFlowInferCondition(TokenList& tokenlist, const Settings& settings)
 {
     for (Token* tok = tokenlist.front(); tok; tok = tok->next()) {
@@ -5154,8 +5192,19 @@ static void valueFlowInferCondition(TokenList& tokenlist, const Settings& settin
                     }
                 }
             } else if (isIntegralOrPointer(tok->astOperand1()) && isIntegralOrPointer(tok->astOperand2())) {
+                std::list<ValueFlow::Value> lhsValues = tok->astOperand1()->values();
+                std::list<ValueFlow::Value> rhsValues = tok->astOperand2()->values();
+                // Impossible bounds cannot be propagated through arithmetic whose
+                // result type is unsigned because wrap-around invalidates the bound
+                if (isUnsignedArithmeticResult(tok)) {
+                    const auto isImpossibleIntegralBound = [](const ValueFlow::Value& v) {
+                        return v.isIntValue() && v.isImpossible() && v.bound != ValueFlow::Value::Bound::Point;
+                    };
+                    lhsValues.remove_if(isImpossibleIntegralBound);
+                    rhsValues.remove_if(isImpossibleIntegralBound);
+                }
                 std::vector<ValueFlow::Value> result =
-                    infer(makeIntegralInferModel(), tok->str(), tok->astOperand1()->values(), tok->astOperand2()->values());
+                    infer(makeIntegralInferModel(), tok->str(), lhsValues, rhsValues);
                 for (ValueFlow::Value& value : result) {
                     setTokenValue(tok, std::move(value), settings);
                 }

lib/vf_common.cpp

@@ -46,7 +46,7 @@ namespace ValueFlow
         if (!vt || !vt->isIntegral() || vt->pointer)
             return false;
 
-        std::uint8_t bits;
+        std::size_t bits;
         switch (vt->type) {
         case ValueType::Type::BOOL:
             bits = 1;
@@ -66,29 +66,37 @@ namespace ValueFlow
         case ValueType::Type::LONGLONG:
             bits = platform.long_long_bit;
             break;
+        case ValueType::Type::WCHAR_T:
+            bits = platform.sizeof_wchar_t * platform.char_bit;
+            break;
         default:
             return false;
         }
 
+        if (bits == 0) {
+            return false;
+        }
         if (bits == 1) {
             minValue = 0;
             maxValue = 1;
         } else if (bits < 62) {
             if (vt->sign == ValueType::Sign::UNSIGNED) {
                 minValue = 0;
                 maxValue = (1LL << bits) - 1;
-            } else {
+            } else if (vt->sign == ValueType::Sign::SIGNED) {
                 minValue = -(1LL << (bits - 1));
                 maxValue = (1LL << (bits - 1)) - 1;
-            }
+            } else
+                return false;
         } else if (bits == 64) {
             if (vt->sign == ValueType::Sign::UNSIGNED) {
                 minValue = 0;
-                maxValue = LLONG_MAX; // todo max unsigned value
-            } else {
+                maxValue = LLONG_MAX; // MathLib::bigint cannot represent ULLONG_MAX; conservative max for unsigned 64-bit
+            } else if (vt->sign == ValueType::Sign::SIGNED) {
                 minValue = LLONG_MIN;
                 maxValue = LLONG_MAX;
-            }
+            } else
+                return false;
         } else {
             return false;
         }

lib/vf_settokenvalue.cpp

@@ -488,6 +488,18 @@ namespace ValueFlow
                 return;
             }
 
+            // Impossible bounds cannot be propagated through arithmetic whose
+            // result type is unsigned, because wrap-around invalidates the bound
+            const ValueType* resultType = parent->valueType();
+            const bool wraps =
+                resultType && resultType->isIntegral() &&
+                resultType->sign == ValueType::Sign::UNSIGNED && resultType->pointer == 0 &&
+                (resultType->type == ValueType::Type::INT ||
+                 resultType->type == ValueType::Type::LONG ||
+                 resultType->type == ValueType::Type::LONGLONG ||
+                 resultType->type == ValueType::Type::UNKNOWN_INT);
+            const bool skipImpossibleBounds = wraps && Token::Match(parent, "+|-|*");
+
             for (const Value &value1 : parent->astOperand1()->values()) {
                 if (!isComputableValue(parent, value1))
                     continue;
@@ -500,6 +512,12 @@ namespace ValueFlow
                         continue;
                     if (!isCompatibleValues(value1, value2))
                         continue;
+                    // Skip impossible bounds on arithmetic with unsigned result type
+                    const bool operandHasImpossibleBound =
+                        (value1.isIntValue() && value1.isImpossible() && value1.bound != Value::Bound::Point) ||
+                        (value2.isIntValue() && value2.isImpossible() && value2.bound != Value::Bound::Point);
+                    if (skipImpossibleBounds && operandHasImpossibleBound)
+                        continue;
                     Value result(0);
                     combineValueProperties(value1, value2, result);
                     if (astIsFloat(parent, false)) {

test/testcondition.cpp

@@ -6612,6 +6612,99 @@ class TestCondition : public TestFixture {
                       "[test.cpp:4:13]: (style) Comparing expression of type 'const unsigned int &' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n",
                       errout_str());
 
+        // PR review: no false positive when unsigned arithmetic wraps around
+        check("void f(int x) {\n"
+              "  if ((unsigned int)x - 1u == 0xFFFFFFFFu) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("", errout_str());
+
+        check("void f(int x) {\n"
+              "  if ((unsigned short)x - 1u == 0xFFFFFFFFu) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("", errout_str());
+
+        // Signed result type after integer promotion must still warn
+        check("void f(int x) {\n"
+              "  if ((unsigned short)x + 1 == 0) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:2:29]: (style) Condition '(unsigned short)x+1==0' is always false [knownConditionTrueFalse]\n",
+                      errout_str());
+
+        // Direct cast bounds are preserved
+        check("void f(int x) {\n"
+              "  if ((unsigned int)x > 4294967295ULL) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:2:25]: (style) Comparing expression of type 'unsigned int' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n",
+                      errout_str());
+
+        // Reproduce the original typedef-heavy Simulink-generated pattern from #15000.
+        check("void f(unsigned int x) {\n"
+              "    unsigned long long tmp = ((unsigned long long)x) + 1ULL;\n"
+              "    if (tmp > 4294967295ULL)\n"
+              "        tmp = 4294967295ULL;\n"
+              "    if ((((long long)((unsigned int)tmp)) - 1LL) < 0LL) {}\n"
+              "    if ((((long long)((unsigned int)tmp)) - 1LL) > 4294967295LL) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:6:50]: (style) Condition '(((long long)((unsigned int)tmp))-1LL)>4294967295LL' is always false [knownConditionTrueFalse]\n",
+                      errout_str());
+
+        check("void f(unsigned long long tmp) {\n"
+              "    if (tmp > 4294967295ULL)\n"
+              "        tmp = 4294967295ULL;\n"
+              "    if (static_cast<long long>(static_cast<unsigned int>(tmp)) - 1LL > 4294967295LL) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:4:70]: (style) Condition 'static_cast<long long>(static_cast<unsigned int>(tmp))-1LL>4294967295LL' is always false [knownConditionTrueFalse]\n",
+                      errout_str());
+
+        // cast directly around variable: both the range-based and the declared-type
+        // analysis can prove the condition invariant; diag() must prevent duplicates
+        check("void f(unsigned char c) {\n"
+              "    if ((unsigned char)c > 255) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:2:28]: (style) Comparing expression of type 'unsigned char' against value 255. Condition is always false. [compareValueOutOfTypeRangeError]\n",
+                      errout_str());
+
+        check("void f(unsigned char c) {\n"
+              "    if ((unsigned char)c == 256) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:2:29]: (style) Comparing expression of type 'unsigned char' against value 256. Condition is always false. [compareValueOutOfTypeRangeError]\n",
+                      errout_str());
+
+        check("void f(unsigned int u) {\n"
+              "    if ((unsigned int)u > 4294967295ULL) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:2:27]: (style) Comparing expression of type 'unsigned int' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n",
+                      errout_str());
+
+        check("void f(unsigned short s) {\n"
+              "    if ((unsigned int)s > 4294967295ULL) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:2:27]: (style) Comparing expression of type 'unsigned int' against value 4294967295. Condition is always false. [compareValueOutOfTypeRangeError]\n",
+                      errout_str());
+
+        // wchar_t range is derived through ValueType::getSizeOf()
+        check("void f(wchar_t c) {\n"
+              "    if ((wchar_t)c > 0x7fffffff) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:2:20]: (style) Condition '(wchar_t)c>0x7fffffff' is always false [knownConditionTrueFalse]\n",
+                      errout_str());
+
+        check("void f(unsigned int x) {\n"
+              "    if (-(signed char)x < -129) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("[test.cpp:2:25]: (style) Condition '-(char)x<-129' is always false [knownConditionTrueFalse]\n",
+                      errout_str());
+
+        check("void f(int x) {\n"
+              "    if ((x) > 0) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("", errout_str());
+
+        check("void f(unsigned int x) {\n"
+              "    if ((unsigned int)x > 0) {}\n"
+              "}\n", settingsUnix64);
+        ASSERT_EQUALS("", errout_str());
+
         check("void f() {\n"
               "    long long ll = 1024 * 1024 * 1024;\n"
               "    if (ll * 8 < INT_MAX) {}\n"

test/testvalueflow.cpp

@@ -9604,6 +9604,49 @@ class TestValueFlow : public TestFixture {
                "}\n";
         ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1));
         ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, -1));
+
+        const Settings settingsUnix64 = settingsBuilder().platform(Platform::Type::Unix64).build();
+        code = "void f(unsigned long long x) {\n"
+               "    return (unsigned int)x;\n"
+               "}\n";
+        SimpleTokenizer tokenizer(settingsUnix64, *this);
+        ASSERT(tokenizer.tokenize(code));
+        const Token* returnTok = Token::findmatch(tokenizer.tokens(), "return (");
+        ASSERT(returnTok && returnTok->next());
+        const std::list<ValueFlow::Value>& castValues = returnTok->next()->values();
+        ASSERT(std::any_of(castValues.cbegin(), castValues.cend(), [](const ValueFlow::Value& value) {
+            return value.isImpossible() && value.intvalue == -1;
+        }));
+        ASSERT(std::any_of(castValues.cbegin(), castValues.cend(), [](const ValueFlow::Value& value) {
+            return value.isImpossible() && value.intvalue == 4294967296;
+        }));
+
+        // PR review: impossible bounds on a cast must not be propagated through
+        // arithmetic with unsigned result type, since wrap-around invalidates the bound
+        code = "void f(int x) {\n"
+               "    return (unsigned int)x - 1u;\n"
+               "}\n";
+        SimpleTokenizer tokenizer2(settingsUnix64, *this);
+        ASSERT(tokenizer2.tokenize(code));
+        const Token* minusTok = Token::findsimplematch(tokenizer2.tokens(), "-");
+        ASSERT(minusTok);
+        for (const ValueFlow::Value& value : minusTok->values()) {
+            ASSERT(!(value.isImpossible() && value.isIntValue() &&
+                     value.bound != ValueFlow::Value::Bound::Point && value.intvalue == 4294967295));
+        }
+
+        // Known values are still propagated through unsigned arithmetic
+        code = "void f(int x) {\n"
+               "    unsigned int y = 5u;\n"
+               "    return y - 1u;\n"
+               "}\n";
+        SimpleTokenizer tokenizer3(settingsUnix64, *this);
+        ASSERT(tokenizer3.tokenize(code));
+        const Token* minusTok3 = Token::findsimplematch(tokenizer3.tokens(), "-");
+        ASSERT(minusTok3);
+        ASSERT(std::any_of(minusTok3->values().cbegin(), minusTok3->values().cend(), [](const ValueFlow::Value& value) {
+            return value.isKnown() && value.isIntValue() && value.intvalue == 4;
+        }));
     }
 
     void valueFlowImpossibleIncDec()