Skip to content

Commit c85f889

Browse files
francois-berderfrancois-berder
authored
Truncate value of increment operator (#6342)
This PR fixes a similar bug than [#11591](https://trac.cppcheck.net/ticket/11591) fixed in PR #6331. --------- Signed-off-by: Francois Berder <fberder@outlook.fr>
1 parent bd536d3 commit c85f889

6 files changed

Lines changed: 85 additions & 18 deletions

File tree

‎lib/valueflow.cpp‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -411,16 +411,6 @@ void ValueFlow::combineValueProperties(const ValueFlow::Value &value1, const Val
411411
result.path = value1.path;
412412
}
413413

414-
static long long truncateIntValue(long long value, size_t value_size, const ValueType::Sign dst_sign)
415-
{
416-
const MathLib::biguint unsignedMaxValue = (1ULL << (value_size * 8)) - 1ULL;
417-
const MathLib::biguint signBit = 1ULL << (value_size * 8 - 1);
418-
value &= unsignedMaxValue;
419-
if (dst_sign == ValueType::Sign::SIGNED && (value & signBit))
420-
value |= ~unsignedMaxValue;
421-
422-
return value;
423-
}
424414

425415
template<class F>
426416
static size_t accumulateStructMembers(const Scope* scope, F f)
@@ -1879,7 +1869,7 @@ struct ValueFlowAnalyzer : Analyzer {
18791869
if (dst) {
18801870
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
18811871
if (sz > 0 && sz < sizeof(MathLib::biguint)) {
1882-
long long newvalue = truncateIntValue(value->intvalue, sz, dst->sign);
1872+
long long newvalue = ValueFlow::truncateIntValue(value->intvalue, sz, dst->sign);
18831873

18841874
/* Handle overflow/underflow for value bounds */
18851875
if (value->bound != ValueFlow::Value::Bound::Point) {
@@ -5105,7 +5095,7 @@ static std::list<ValueFlow::Value> truncateValues(std::list<ValueFlow::Value> va
51055095
}
51065096

51075097
if (value.isIntValue() && sz > 0 && sz < sizeof(MathLib::biguint))
5108-
value.intvalue = truncateIntValue(value.intvalue, sz, dst->sign);
5098+
value.intvalue = ValueFlow::truncateIntValue(value.intvalue, sz, dst->sign);
51095099
}
51105100
return values;
51115101
}

‎lib/vf_common.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,20 @@ namespace ValueFlow
9393
return true;
9494
}
9595

96+
long long truncateIntValue(long long value, size_t value_size, const ValueType::Sign dst_sign)
97+
{
98+
if (value_size == 0)
99+
return value;
100+
101+
const MathLib::biguint unsignedMaxValue = std::numeric_limits<MathLib::biguint>::max() >> ((sizeof(unsignedMaxValue) - value_size) * 8);
102+
const MathLib::biguint signBit = 1ULL << (value_size * 8 - 1);
103+
value &= unsignedMaxValue;
104+
if (dst_sign == ValueType::Sign::SIGNED && (value & signBit))
105+
value |= ~unsignedMaxValue;
106+
107+
return value;
108+
}
109+
96110
static nonneg int getSizeOfType(const Token *typeTok, const Settings &settings)
97111
{
98112
const ValueType &valueType = ValueType::parseDecl(typeTok, settings);

‎lib/vf_common.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ namespace ValueFlow
3535
{
3636
bool getMinMaxValues(const ValueType* vt, const Platform& platform, MathLib::bigint& minValue, MathLib::bigint& maxValue);
3737

38+
long long truncateIntValue(long long value, size_t value_size, const ValueType::Sign dst_sign);
39+
3840
Token * valueFlowSetConstantValue(Token *tok, const Settings &settings);
3941

4042
Value castValue(Value value, const ValueType::Sign sign, nonneg int bit);

‎lib/vf_settokenvalue.cpp‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -640,8 +640,22 @@ namespace ValueFlow
640640
continue;
641641
Value v(val);
642642
if (parent == tok->previous()) {
643-
if (v.isIntValue() || v.isSymbolicValue())
644-
v.intvalue = v.intvalue + 1;
643+
if (v.isIntValue() || v.isSymbolicValue()) {
644+
const ValueType *dst = tok->valueType();
645+
if (dst) {
646+
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
647+
long long newvalue = ValueFlow::truncateIntValue(v.intvalue + 1, sz, dst->sign);
648+
if (v.bound != ValueFlow::Value::Bound::Point) {
649+
if (newvalue < v.intvalue) {
650+
v.invertBound();
651+
newvalue -= 2;
652+
}
653+
}
654+
v.intvalue = newvalue;
655+
} else {
656+
v.intvalue = v.intvalue + 1;
657+
}
658+
}
645659
else
646660
v.floatValue = v.floatValue + 1.0;
647661
}
@@ -656,8 +670,22 @@ namespace ValueFlow
656670
continue;
657671
Value v(val);
658672
if (parent == tok->previous()) {
659-
if (v.isIntValue() || v.isSymbolicValue())
660-
v.intvalue = v.intvalue - 1;
673+
if (v.isIntValue() || v.isSymbolicValue()) {
674+
const ValueType *dst = tok->valueType();
675+
if (dst) {
676+
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
677+
long long newvalue = ValueFlow::truncateIntValue(v.intvalue - 1, sz, dst->sign);
678+
if (v.bound != ValueFlow::Value::Bound::Point) {
679+
if (newvalue > v.intvalue) {
680+
v.invertBound();
681+
newvalue += 2;
682+
}
683+
}
684+
v.intvalue = newvalue;
685+
} else {
686+
v.intvalue = v.intvalue - 1;
687+
}
688+
}
661689
else
662690
v.floatValue = v.floatValue - 1.0;
663691
}

‎test/testcondition.cpp‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ class TestCondition : public TestFixture {
123123
TEST_CASE(knownConditionCast); // #9976
124124
TEST_CASE(knownConditionIncrementLoop); // #9808
125125
TEST_CASE(knownConditionAfterBailout); // #12526
126+
TEST_CASE(knownConditionIncDecOperator);
126127
}
127128

128129
#define check(...) check_(__FILE__, __LINE__, __VA_ARGS__)
@@ -6077,6 +6078,20 @@ class TestCondition : public TestFixture {
60776078
);
60786079
ASSERT_EQUALS("[test.cpp:13] -> [test.cpp:18]: (style) Condition 'mS.b' is always false\n", errout_str());
60796080
}
6081+
6082+
void knownConditionIncDecOperator() {
6083+
check(
6084+
"void f() {\n"
6085+
" unsigned int d = 0;\n"
6086+
" for (int i = 0; i < 4; ++i) {\n"
6087+
" if (i < 3)\n"
6088+
" ++d;\n"
6089+
" else if (--d == 0)\n"
6090+
" ;\n"
6091+
" }\n"
6092+
"}\n");
6093+
ASSERT_EQUALS("", errout_str());
6094+
}
60806095
};
60816096

60826097
REGISTER_TEST(TestCondition)

‎test/testvalueflow.cpp‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ class TestValueFlow : public TestFixture {
150150
TEST_CASE(valueFlowIdempotent);
151151
TEST_CASE(valueFlowUnsigned);
152152
TEST_CASE(valueFlowMod);
153-
TEST_CASE(valueFlowInc);
153+
TEST_CASE(valueFlowIncDec);
154154
TEST_CASE(valueFlowNotNull);
155155
TEST_CASE(valueFlowSymbolic);
156156
TEST_CASE(valueFlowSymbolicIdentity);
@@ -7925,7 +7925,7 @@ class TestValueFlow : public TestFixture {
79257925
ASSERT_EQUALS(false, testValueOfXImpossible(code, 3U, 1));
79267926
}
79277927

7928-
void valueFlowInc() {
7928+
void valueFlowIncDec() {
79297929
const char *code;
79307930
std::list<ValueFlow::Value> values;
79317931

@@ -7939,6 +7939,24 @@ class TestValueFlow : public TestFixture {
79397939
values = tokenValues(code, "i ]");
79407940
ASSERT_EQUALS(1U, values.size());
79417941
ASSERT_EQUALS(0LLU, values.back().intvalue);
7942+
7943+
code = "int f() {\n"
7944+
" const int a[1] = {};\n"
7945+
" unsigned char i = 255;\n"
7946+
" return a[++i];\n"
7947+
"}\n";
7948+
values = tokenValues(code, "++");
7949+
ASSERT_EQUALS(1U, values.size());
7950+
ASSERT_EQUALS(0LLU, values.back().intvalue);
7951+
7952+
code = "int f() {\n"
7953+
" const int a[128] = {};\n"
7954+
" char b = -128;\n"
7955+
" return a[--b];\n"
7956+
"}\n";
7957+
values = tokenValues(code, "--");
7958+
ASSERT_EQUALS(1U, values.size());
7959+
ASSERT_EQUALS(127LLU, values.back().intvalue);
79427960
}
79437961

79447962
void valueFlowNotNull()

0 commit comments

Comments
 (0)