From ad8fdc64255d932749c6b9f965bd3d2d590c42fa Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Wed, 24 Apr 2024 21:57:04 +0200 Subject: [PATCH 1/3] Truncate value of increment/decrement operator Signed-off-by: Francois Berder --- lib/valueflow.cpp | 14 ++------------ lib/vf_common.cpp | 14 ++++++++++++++ lib/vf_common.h | 2 ++ lib/vf_settokenvalue.cpp | 36 ++++++++++++++++++++++++++++++++---- test/testcondition.cpp | 15 +++++++++++++++ test/testvalueflow.cpp | 22 ++++++++++++++++++++-- 6 files changed, 85 insertions(+), 18 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index b687f7b9f04..8ddd9d799d6 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -411,16 +411,6 @@ void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const Val result.path = value1.path; } -static long long truncateIntValue(long long value, size_t value_size, const ValueType::Sign dst_sign) -{ - const MathLib::biguint unsignedMaxValue = (1ULL << (value_size * 8)) - 1ULL; - const MathLib::biguint signBit = 1ULL << (value_size * 8 - 1); - value &= unsignedMaxValue; - if (dst_sign == ValueType::Sign::SIGNED && (value & signBit)) - value |= ~unsignedMaxValue; - - return value; -} template static size_t accumulateStructMembers(const Scope* scope, F f) @@ -1868,7 +1858,7 @@ struct ValueFlowAnalyzer : Analyzer { if (dst) { const size_t sz = ValueFlow::getSizeOf(*dst, settings); if (sz > 0 && sz < sizeof(MathLib::biguint)) { - long long newvalue = truncateIntValue(value->intvalue, sz, dst->sign); + long long newvalue = ValueFlow::truncateIntValue(value->intvalue, sz, dst->sign); /* Handle overflow/underflow for value bounds */ if (value->bound != ValueFlow::Value::Bound::Point) { @@ -5094,7 +5084,7 @@ static std::list truncateValues(std::list va } if (value.isIntValue() && sz > 0 && sz < sizeof(MathLib::biguint)) - value.intvalue = truncateIntValue(value.intvalue, sz, dst->sign); + value.intvalue = ValueFlow::truncateIntValue(value.intvalue, sz, dst->sign); } return values; } diff --git a/lib/vf_common.cpp b/lib/vf_common.cpp index 7505e89c36d..d61b83676c2 100644 --- a/lib/vf_common.cpp +++ b/lib/vf_common.cpp @@ -93,6 +93,20 @@ namespace ValueFlow return true; } + long long truncateIntValue(long long value, size_t value_size, const ValueType::Sign dst_sign) + { + if (value_size == 0) + return value; + + const MathLib::biguint unsignedMaxValue = std::numeric_limits::max() >> (value_size < sizeof(unsignedMaxValue) ? ((sizeof(unsignedMaxValue) - value_size) * 8) : 0); + const MathLib::biguint signBit = 1ULL << (value_size * 8 - 1); + value &= unsignedMaxValue; + if (dst_sign == ValueType::Sign::SIGNED && (value & signBit)) + value |= ~unsignedMaxValue; + + return value; + } + static nonneg int getSizeOfType(const Token *typeTok, const Settings &settings) { const ValueType &valueType = ValueType::parseDecl(typeTok, settings); diff --git a/lib/vf_common.h b/lib/vf_common.h index 050b9960969..d67dfc6e722 100644 --- a/lib/vf_common.h +++ b/lib/vf_common.h @@ -35,6 +35,8 @@ namespace ValueFlow { bool getMinMaxValues(const ValueType* vt, const Platform& platform, MathLib::bigint& minValue, MathLib::bigint& maxValue); + long long truncateIntValue(long long value, size_t value_size, const ValueType::Sign dst_sign); + Token * valueFlowSetConstantValue(Token *tok, const Settings &settings); Value castValue(Value value, const ValueType::Sign sign, nonneg int bit); diff --git a/lib/vf_settokenvalue.cpp b/lib/vf_settokenvalue.cpp index 51275fe469d..391fdc947a6 100644 --- a/lib/vf_settokenvalue.cpp +++ b/lib/vf_settokenvalue.cpp @@ -633,8 +633,22 @@ namespace ValueFlow continue; Value v(val); if (parent == tok->previous()) { - if (v.isIntValue() || v.isSymbolicValue()) - v.intvalue = v.intvalue + 1; + if (v.isIntValue() || v.isSymbolicValue()) { + const ValueType *dst = tok->valueType(); + if (dst) { + const size_t sz = ValueFlow::getSizeOf(*dst, settings); + long long newvalue = ValueFlow::truncateIntValue(v.intvalue + 1, sz, dst->sign); + if (v.bound != ValueFlow::Value::Bound::Point) { + if (newvalue < v.intvalue) { + v.invertBound(); + newvalue -= 1; + } + } + v.intvalue = newvalue; + } else { + v.intvalue = v.intvalue + 1; + } + } else v.floatValue = v.floatValue + 1.0; } @@ -649,8 +663,22 @@ namespace ValueFlow continue; Value v(val); if (parent == tok->previous()) { - if (v.isIntValue() || v.isSymbolicValue()) - v.intvalue = v.intvalue - 1; + if (v.isIntValue() || v.isSymbolicValue()) { + const ValueType *dst = tok->valueType(); + if (dst) { + const size_t sz = ValueFlow::getSizeOf(*dst, settings); + long long newvalue = ValueFlow::truncateIntValue(v.intvalue - 1, sz, dst->sign); + if (v.bound != ValueFlow::Value::Bound::Point) { + if (newvalue > v.intvalue) { + v.invertBound(); + newvalue += 1; + } + } + v.intvalue = newvalue; + } else { + v.intvalue = v.intvalue - 1; + } + } else v.floatValue = v.floatValue - 1.0; } diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 90597974cd9..c5c80c2ebcd 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -123,6 +123,7 @@ class TestCondition : public TestFixture { TEST_CASE(knownConditionCast); // #9976 TEST_CASE(knownConditionIncrementLoop); // #9808 TEST_CASE(knownConditionAfterBailout); // #12526 + TEST_CASE(knownConditionIncDecOperator); } #define check(...) check_(__FILE__, __LINE__, __VA_ARGS__) @@ -6046,6 +6047,20 @@ class TestCondition : public TestFixture { ); ASSERT_EQUALS("[test.cpp:13] -> [test.cpp:18]: (style) Condition 'mS.b' is always false\n", errout_str()); } + + void knownConditionIncDecOperator() { + check( + "void f() {\n" + " unsigned int d = 0;\n" + " for (int i = 0; i < 4; ++i) {\n" + " if (i < 3)\n" + " ++d;\n" + " else if (--d == 0)\n" + " ;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("", errout_str()); + } }; REGISTER_TEST(TestCondition) diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index f417a643b4a..b39822365e3 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -150,7 +150,7 @@ class TestValueFlow : public TestFixture { TEST_CASE(valueFlowIdempotent); TEST_CASE(valueFlowUnsigned); TEST_CASE(valueFlowMod); - TEST_CASE(valueFlowInc); + TEST_CASE(valueFlowIncDec); TEST_CASE(valueFlowNotNull); TEST_CASE(valueFlowSymbolic); TEST_CASE(valueFlowSymbolicIdentity); @@ -7925,7 +7925,7 @@ class TestValueFlow : public TestFixture { ASSERT_EQUALS(false, testValueOfXImpossible(code, 3U, 1)); } - void valueFlowInc() { + void valueFlowIncDec() { const char *code; std::list values; @@ -7939,6 +7939,24 @@ class TestValueFlow : public TestFixture { values = tokenValues(code, "i ]"); ASSERT_EQUALS(1U, values.size()); ASSERT_EQUALS(0LLU, values.back().intvalue); + + code = "int f() {\n" + " const int a[1] = {};\n" + " unsigned char i = 255;\n" + " return a[++i];\n" + "}\n"; + values = tokenValues(code, "++"); + ASSERT_EQUALS(1U, values.size()); + ASSERT_EQUALS(0LLU, values.back().intvalue); + + code = "int f() {\n" + " const int a[128] = {};\n" + " char b = -128;\n" + " return a[--b];\n" + "}\n"; + values = tokenValues(code, "--"); + ASSERT_EQUALS(1U, values.size()); + ASSERT_EQUALS(127LLU, values.back().intvalue); } void valueFlowNotNull() From ab7ffc1127e798d542655c7461d5aadc9fd5b36a Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Fri, 7 Jun 2024 18:10:50 +0200 Subject: [PATCH 2/3] fixup! Truncate value of increment/decrement operator --- lib/vf_settokenvalue.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/vf_settokenvalue.cpp b/lib/vf_settokenvalue.cpp index 391fdc947a6..a6076a9904a 100644 --- a/lib/vf_settokenvalue.cpp +++ b/lib/vf_settokenvalue.cpp @@ -641,7 +641,7 @@ namespace ValueFlow if (v.bound != ValueFlow::Value::Bound::Point) { if (newvalue < v.intvalue) { v.invertBound(); - newvalue -= 1; + newvalue -= 2; } } v.intvalue = newvalue; @@ -671,7 +671,7 @@ namespace ValueFlow if (v.bound != ValueFlow::Value::Bound::Point) { if (newvalue > v.intvalue) { v.invertBound(); - newvalue += 1; + newvalue += 2; } } v.intvalue = newvalue; From 20a5d257e14060134d14c4b1630438df709971d4 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Fri, 14 Jun 2024 16:55:15 +0200 Subject: [PATCH 3/3] fixup! fixup! Truncate value of increment/decrement operator --- lib/vf_common.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/vf_common.cpp b/lib/vf_common.cpp index d61b83676c2..ae1f4a43d8f 100644 --- a/lib/vf_common.cpp +++ b/lib/vf_common.cpp @@ -98,7 +98,7 @@ namespace ValueFlow if (value_size == 0) return value; - const MathLib::biguint unsignedMaxValue = std::numeric_limits::max() >> (value_size < sizeof(unsignedMaxValue) ? ((sizeof(unsignedMaxValue) - value_size) * 8) : 0); + const MathLib::biguint unsignedMaxValue = std::numeric_limits::max() >> ((sizeof(unsignedMaxValue) - value_size) * 8); const MathLib::biguint signBit = 1ULL << (value_size * 8 - 1); value &= unsignedMaxValue; if (dst_sign == ValueType::Sign::SIGNED && (value & signBit))