From 5b7342fe5ffd4cdde9ed12e5c6837c63dd260ae3 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Mon, 20 May 2024 21:43:50 +0200 Subject: [PATCH 1/3] Fix 12681: FP: knownConditionTrueFalse While truncating integer values for upper/lower bounds, underflows/overflows must be handled correctly by inverting the bound. Signed-off-by: Francois Berder --- lib/valueflow.cpp | 20 ++++++++++++++++++-- test/testcondition.cpp | 9 +++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index e9246920e4a..1eb7c620e48 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2833,8 +2833,24 @@ struct ValueFlowAnalyzer : Analyzer { const ValueType *dst = tok->valueType(); if (dst) { const size_t sz = ValueFlow::getSizeOf(*dst, settings); - if (sz > 0 && sz < 8) - value->intvalue = truncateIntValue(value->intvalue, sz, dst->sign); + if (sz > 0 && sz < 8) { + long long newvalue = truncateIntValue(value->intvalue, sz, dst->sign); + + /* Handle overflow/underflow for value bounds */ + if (value->bound != ValueFlow::Value::Bound::Point) { + if (newvalue > value->intvalue) { + if ((inc && value->bound == ValueFlow::Value::Bound::Lower) + || (!inc && value->bound == ValueFlow::Value::Bound::Upper)) + value->invertBound(); + } else if (newvalue < value->intvalue) { + if ((!inc && value->bound == ValueFlow::Value::Bound::Lower) + || (inc && value->bound == ValueFlow::Value::Bound::Upper)) + value->invertBound(); + } + } + + value->intvalue = newvalue; + } value->errorPath.emplace_back(tok, tok->str() + " is " + opName + "', new value is " + value->infoString()); } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 8005c400e50..3a7d9ab41fa 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4783,6 +4783,15 @@ class TestCondition : public TestFixture { " }\n" "}\n"); ASSERT_EQUALS("", errout_str()); + + // #12681 + check("void f(unsigned u) {\n" + " if (u > 0) {\n" + " u--;\n" + " if (u == 0) {}\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); } void alwaysTrueInfer() { From 12e80ec0581b7c9e950b4b32dbf43d3d6b6b3b5e Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Mon, 20 May 2024 23:13:05 +0200 Subject: [PATCH 2/3] fixup! Fix 12681: FP: knownConditionTrueFalse --- lib/valueflow.cpp | 11 ++--------- test/testcondition.cpp | 10 +++++++++- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 1eb7c620e48..07b30a86cdb 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2838,15 +2838,8 @@ struct ValueFlowAnalyzer : Analyzer { /* Handle overflow/underflow for value bounds */ if (value->bound != ValueFlow::Value::Bound::Point) { - if (newvalue > value->intvalue) { - if ((inc && value->bound == ValueFlow::Value::Bound::Lower) - || (!inc && value->bound == ValueFlow::Value::Bound::Upper)) - value->invertBound(); - } else if (newvalue < value->intvalue) { - if ((!inc && value->bound == ValueFlow::Value::Bound::Lower) - || (inc && value->bound == ValueFlow::Value::Bound::Upper)) - value->invertBound(); - } + if ((newvalue > value->intvalue && !inc) || (newvalue < value->intvalue && inc)) + value->invertBound(); } value->intvalue = newvalue; diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 3a7d9ab41fa..f488bdadf90 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -4784,7 +4784,7 @@ class TestCondition : public TestFixture { "}\n"); ASSERT_EQUALS("", errout_str()); - // #12681 + // #12681 check("void f(unsigned u) {\n" " if (u > 0) {\n" " u--;\n" @@ -4792,6 +4792,14 @@ class TestCondition : public TestFixture { " }\n" "}\n"); ASSERT_EQUALS("", errout_str()); + + check("void f(unsigned u) {\n" + " if (u < 0xFFFFFFFF) {\n" + " u++;\n" + " if (u == 0xFFFFFFFF) {}\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); } void alwaysTrueInfer() { From 714bf266199341999a45eb5174013b373f00f460 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Tue, 21 May 2024 14:32:48 +0200 Subject: [PATCH 3/3] fixup! fixup! Fix 12681: FP: knownConditionTrueFalse --- lib/valueflow.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 07b30a86cdb..e8cda5751e3 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -2833,7 +2833,7 @@ struct ValueFlowAnalyzer : Analyzer { const ValueType *dst = tok->valueType(); if (dst) { const size_t sz = ValueFlow::getSizeOf(*dst, settings); - if (sz > 0 && sz < 8) { + if (sz > 0 && sz < sizeof(MathLib::biguint)) { long long newvalue = truncateIntValue(value->intvalue, sz, dst->sign); /* Handle overflow/underflow for value bounds */ @@ -6058,7 +6058,7 @@ static std::list truncateValues(std::list va value.valueType = ValueFlow::Value::ValueType::INT; } - if (value.isIntValue() && sz > 0 && sz < 8) + if (value.isIntValue() && sz > 0 && sz < sizeof(MathLib::biguint)) value.intvalue = truncateIntValue(value.intvalue, sz, dst->sign); } return values;