Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate value of increment operator #6342

Merged
merged 3 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 2 additions & 12 deletions lib/valueflow.cpp
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 @@ -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) {
Expand Down Expand Up @@ -5094,7 +5084,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
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() >> (value_size < sizeof(unsignedMaxValue) ? ((sizeof(unsignedMaxValue) - value_size) * 8) : 0);
danmar marked this conversation as resolved.
Show resolved Hide resolved
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -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 -= 2;
}
}
v.intvalue = newvalue;
} else {
v.intvalue = v.intvalue + 1;
}
}
else
v.floatValue = v.floatValue + 1.0;
}
Expand All @@ -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 += 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
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 @@ -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)
22 changes: 20 additions & 2 deletions test/testvalueflow.cpp
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
Loading