From 1581a7fab759b43a5659b1c1222b38c9cc659422 Mon Sep 17 00:00:00 2001 From: Francois Berder Date: Wed, 24 Apr 2024 21:57:04 +0200 Subject: [PATCH] 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/testvalueflow.cpp | 22 ++++++++++++++++++++-- 5 files changed, 70 insertions(+), 18 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index b687f7b9f04b..8ddd9d799d6f 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 7505e89c36d8..d61b83676c2f 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 050b99609690..d67dfc6e722a 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 51275fe469df..391fdc947a6f 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/testvalueflow.cpp b/test/testvalueflow.cpp index f417a643b4aa..b39822365e32 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()