Skip to content

Commit ad8fdc6

Browse files
Truncate value of increment/decrement operator
Signed-off-by: Francois Berder <[email protected]>
1 parent 32fefc6 commit ad8fdc6

File tree

6 files changed

+85
-18
lines changed

6 files changed

+85
-18
lines changed

lib/valueflow.cpp

+2-12
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)
@@ -1868,7 +1858,7 @@ struct ValueFlowAnalyzer : Analyzer {
18681858
if (dst) {
18691859
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
18701860
if (sz > 0 && sz < sizeof(MathLib::biguint)) {
1871-
long long newvalue = truncateIntValue(value->intvalue, sz, dst->sign);
1861+
long long newvalue = ValueFlow::truncateIntValue(value->intvalue, sz, dst->sign);
18721862

18731863
/* Handle overflow/underflow for value bounds */
18741864
if (value->bound != ValueFlow::Value::Bound::Point) {
@@ -5094,7 +5084,7 @@ static std::list<ValueFlow::Value> truncateValues(std::list<ValueFlow::Value> va
50945084
}
50955085

50965086
if (value.isIntValue() && sz > 0 && sz < sizeof(MathLib::biguint))
5097-
value.intvalue = truncateIntValue(value.intvalue, sz, dst->sign);
5087+
value.intvalue = ValueFlow::truncateIntValue(value.intvalue, sz, dst->sign);
50985088
}
50995089
return values;
51005090
}

lib/vf_common.cpp

+14
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() >> (value_size < sizeof(unsignedMaxValue) ? ((sizeof(unsignedMaxValue) - value_size) * 8) : 0);
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

+2
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

+32-4
Original file line numberDiff line numberDiff line change
@@ -633,8 +633,22 @@ namespace ValueFlow
633633
continue;
634634
Value v(val);
635635
if (parent == tok->previous()) {
636-
if (v.isIntValue() || v.isSymbolicValue())
637-
v.intvalue = v.intvalue + 1;
636+
if (v.isIntValue() || v.isSymbolicValue()) {
637+
const ValueType *dst = tok->valueType();
638+
if (dst) {
639+
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
640+
long long newvalue = ValueFlow::truncateIntValue(v.intvalue + 1, sz, dst->sign);
641+
if (v.bound != ValueFlow::Value::Bound::Point) {
642+
if (newvalue < v.intvalue) {
643+
v.invertBound();
644+
newvalue -= 1;
645+
}
646+
}
647+
v.intvalue = newvalue;
648+
} else {
649+
v.intvalue = v.intvalue + 1;
650+
}
651+
}
638652
else
639653
v.floatValue = v.floatValue + 1.0;
640654
}
@@ -649,8 +663,22 @@ namespace ValueFlow
649663
continue;
650664
Value v(val);
651665
if (parent == tok->previous()) {
652-
if (v.isIntValue() || v.isSymbolicValue())
653-
v.intvalue = v.intvalue - 1;
666+
if (v.isIntValue() || v.isSymbolicValue()) {
667+
const ValueType *dst = tok->valueType();
668+
if (dst) {
669+
const size_t sz = ValueFlow::getSizeOf(*dst, settings);
670+
long long newvalue = ValueFlow::truncateIntValue(v.intvalue - 1, sz, dst->sign);
671+
if (v.bound != ValueFlow::Value::Bound::Point) {
672+
if (newvalue > v.intvalue) {
673+
v.invertBound();
674+
newvalue += 1;
675+
}
676+
}
677+
v.intvalue = newvalue;
678+
} else {
679+
v.intvalue = v.intvalue - 1;
680+
}
681+
}
654682
else
655683
v.floatValue = v.floatValue - 1.0;
656684
}

test/testcondition.cpp

+15
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__)
@@ -6046,6 +6047,20 @@ class TestCondition : public TestFixture {
60466047
);
60476048
ASSERT_EQUALS("[test.cpp:13] -> [test.cpp:18]: (style) Condition 'mS.b' is always false\n", errout_str());
60486049
}
6050+
6051+
void knownConditionIncDecOperator() {
6052+
check(
6053+
"void f() {\n"
6054+
" unsigned int d = 0;\n"
6055+
" for (int i = 0; i < 4; ++i) {\n"
6056+
" if (i < 3)\n"
6057+
" ++d;\n"
6058+
" else if (--d == 0)\n"
6059+
" ;\n"
6060+
" }\n"
6061+
"}\n");
6062+
ASSERT_EQUALS("", errout_str());
6063+
}
60496064
};
60506065

60516066
REGISTER_TEST(TestCondition)

test/testvalueflow.cpp

+20-2
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)