Skip to content

Commit

Permalink
Truncate value of increment operator (danmar#6342)
Browse files Browse the repository at this point in the history 
This PR fixes a similar bug than
[#11591](https://trac.cppcheck.net/ticket/11591) fixed in PR
danmar#6331.

---------

Signed-off-by: Francois Berder <[email protected]>
  • Loading branch information
@francois-berder
francois-berder committed Jun 26, 2024
1 parent bd536d3 commit c85f889
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 18 deletions.
14 changes: 2 additions & 12 deletions lib/valueflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<class F>
static size_t accumulateStructMembers(const Scope* scope, F f)
Expand Down Expand Up @@ -1879,7 +1869,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) {
Expand Down Expand Up @@ -5105,7 +5095,7 @@ static std::list<ValueFlow::Value> truncateValues(std::list<ValueFlow::Value> 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;
}
Expand Down
14 changes: 14 additions & 0 deletions lib/vf_common.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<MathLib::biguint>::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))
value |= ~unsignedMaxValue;

return value;
}

static nonneg int getSizeOfType(const Token *typeTok, const Settings &settings)
{
const ValueType &valueType = ValueType::parseDecl(typeTok, settings);
Expand Down
2 changes: 2 additions & 0 deletions lib/vf_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
36 changes: 32 additions & 4 deletions lib/vf_settokenvalue.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,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 -= 2;
}
}
v.intvalue = newvalue;
} else {
v.intvalue = v.intvalue + 1;
}
}
else
v.floatValue = v.floatValue + 1.0;
}
Expand All @@ -656,8 +670,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 += 2;
}
}
v.intvalue = newvalue;
} else {
v.intvalue = v.intvalue - 1;
}
}
else
v.floatValue = v.floatValue - 1.0;
}
Expand Down
15 changes: 15 additions & 0 deletions test/testcondition.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -6077,6 +6078,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)
22 changes: 20 additions & 2 deletions test/testvalueflow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -7925,7 +7925,7 @@ class TestValueFlow : public TestFixture {
ASSERT_EQUALS(false, testValueOfXImpossible(code, 3U, 1));
}

void valueFlowInc() {
void valueFlowIncDec() {
const char *code;
std::list<ValueFlow::Value> values;

Expand All @@ -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()
Expand Down

0 comments on commit c85f889

Please sign in to comment.