Skip to content

Commit b4d455d

Browse files
pfultz2pfultz2
authored
Fix 11349: FP negativeIndex for clamped array index (#4627)
* Fix 11349: FP negativeIndex for clamped array index * Format * Use emplace_back * Use default constructor
1 parent 5b687cb commit b4d455d

4 files changed

Lines changed: 135 additions & 1 deletion

File tree

‎lib/utils.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ bool contains(const std::initializer_list<T>& r, const U& x)
6262
return std::find(r.begin(), r.end(), x) != r.end();
6363
}
6464

65+
template<class T, class ... Ts>
66+
inline std::array<T, sizeof...(Ts) + 1> makeArray(T x, Ts... xs)
67+
{
68+
return {std::move(x), std::move(xs)...};
69+
}
70+
6571
// Enum hash for C++11. This is not needed in C++14
6672
struct EnumClassHash {
6773
template<typename T>

‎lib/valueflow.cpp‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1731,6 +1731,15 @@ static bool isConvertedToIntegral(const Token* tok, const Settings* settings)
17311731
return vt.type != ValueType::UNKNOWN_INT && vt.isIntegral();
17321732
}
17331733

1734+
static bool isSameToken(const Token* tok1, const Token* tok2)
1735+
{
1736+
if (tok1->exprId() != 0 && tok1->exprId() == tok2->exprId())
1737+
return true;
1738+
if (tok1->hasKnownIntValue() && tok2->hasKnownIntValue())
1739+
return tok1->values().front().intvalue == tok2->values().front().intvalue;
1740+
return false;
1741+
}
1742+
17341743
static void valueFlowImpossibleValues(TokenList* tokenList, const Settings* settings)
17351744
{
17361745
for (Token* tok = tokenList->front(); tok; tok = tok->next()) {
@@ -1757,7 +1766,50 @@ static void valueFlowImpossibleValues(TokenList* tokenList, const Settings* sett
17571766
value.setImpossible();
17581767
setTokenValue(tok, value, settings);
17591768
}
1760-
if (Token::simpleMatch(tok, "%") && tok->astOperand2() && tok->astOperand2()->hasKnownIntValue()) {
1769+
if (Token::simpleMatch(tok, "?") && Token::Match(tok->astOperand1(), "<|<=|>|>=")) {
1770+
const Token* condTok = tok->astOperand1();
1771+
const Token* branchesTok = tok->astOperand2();
1772+
1773+
auto tokens = makeArray(condTok->astOperand1(), condTok->astOperand2());
1774+
auto branches = makeArray(branchesTok->astOperand1(), branchesTok->astOperand2());
1775+
bool flipped = false;
1776+
if (std::equal(tokens.begin(), tokens.end(), branches.rbegin(), &isSameToken))
1777+
flipped = true;
1778+
else if (!std::equal(tokens.begin(), tokens.end(), branches.begin(), &isSameToken))
1779+
continue;
1780+
const bool isMin = Token::Match(condTok, "<|<=") ^ flipped;
1781+
std::vector<ValueFlow::Value> values;
1782+
for (const Token* tok2 : tokens) {
1783+
if (tok2->hasKnownIntValue()) {
1784+
values.emplace_back();
1785+
} else {
1786+
ValueFlow::Value symValue{};
1787+
symValue.valueType = ValueFlow::Value::ValueType::SYMBOLIC;
1788+
symValue.tokvalue = tok2;
1789+
values.push_back(symValue);
1790+
std::copy_if(tok2->values().begin(),
1791+
tok2->values().end(),
1792+
std::back_inserter(values),
1793+
[](const ValueFlow::Value& v) {
1794+
if (!v.isKnown())
1795+
return false;
1796+
return v.isSymbolicValue();
1797+
});
1798+
}
1799+
}
1800+
for (ValueFlow::Value& value : values) {
1801+
value.setImpossible();
1802+
if (isMin) {
1803+
value.intvalue++;
1804+
value.bound = ValueFlow::Value::Bound::Lower;
1805+
} else {
1806+
value.intvalue--;
1807+
value.bound = ValueFlow::Value::Bound::Upper;
1808+
}
1809+
setTokenValue(tok, value, settings);
1810+
}
1811+
1812+
} else if (Token::simpleMatch(tok, "%") && tok->astOperand2() && tok->astOperand2()->hasKnownIntValue()) {
17611813
ValueFlow::Value value{tok->astOperand2()->values().front()};
17621814
value.bound = ValueFlow::Value::Bound::Lower;
17631815
value.setImpossible();

‎test/testbufferoverrun.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ class TestBufferOverrun : public TestFixture {
205205
TEST_CASE(array_index_negative3);
206206
TEST_CASE(array_index_negative4);
207207
TEST_CASE(array_index_negative5); // #10526
208+
TEST_CASE(array_index_negative6); // #11349
208209
TEST_CASE(array_index_for_decr);
209210
TEST_CASE(array_index_varnames); // FP: struct member #1576, FN: #1586
210211
TEST_CASE(array_index_for_continue); // for,continue
@@ -2227,6 +2228,19 @@ class TestBufferOverrun : public TestFixture {
22272228
ASSERT_EQUALS("", errout.str());
22282229
}
22292230

2231+
// #11349
2232+
void array_index_negative6()
2233+
{
2234+
check("void f(int i) {\n"
2235+
" int j = i;\n"
2236+
" const int a[5] = {};\n"
2237+
" const int k = j < 0 ? 0 : j;\n"
2238+
" (void)a[k];\n"
2239+
" if (i == -3) {}\n"
2240+
"}\n");
2241+
ASSERT_EQUALS("", errout.str());
2242+
}
2243+
22302244
void array_index_for_decr() {
22312245
check("void f()\n"
22322246
"{\n"

‎test/testvalueflow.cpp‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ class TestValueFlow : public TestFixture {
160160
TEST_CASE(valueFlowSymbolicIdentity);
161161
TEST_CASE(valueFlowSymbolicStrlen);
162162
TEST_CASE(valueFlowSmartPointer);
163+
TEST_CASE(valueFlowImpossibleMinMax);
163164
}
164165

165166
static bool isNotTokValue(const ValueFlow::Value &val) {
@@ -7631,6 +7632,67 @@ class TestValueFlow : public TestFixture {
76317632
"}\n";
76327633
ASSERT_EQUALS(false, testValueOfX(code, 5U, 0));
76337634
}
7635+
7636+
void valueFlowImpossibleMinMax()
7637+
{
7638+
const char* code;
7639+
7640+
code = "void f(int a, int b) {\n"
7641+
" int x = a < b ? a : b;\n"
7642+
" return x;\n"
7643+
"}\n";
7644+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", 1));
7645+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "b", 1));
7646+
7647+
code = "void f(int a, int b) {\n"
7648+
" int x = a > b ? a : b;\n"
7649+
" return x;\n"
7650+
"}\n";
7651+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1));
7652+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "b", -1));
7653+
7654+
code = "void f(int a, int b) {\n"
7655+
" int x = a > b ? b : a;\n"
7656+
" return x;\n"
7657+
"}\n";
7658+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", 1));
7659+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "b", 1));
7660+
7661+
code = "void f(int a, int b) {\n"
7662+
" int x = a < b ? b : a;\n"
7663+
" return x;\n"
7664+
"}\n";
7665+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1));
7666+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "b", -1));
7667+
7668+
code = "void f(int a) {\n"
7669+
" int x = a < 0 ? a : 0;\n"
7670+
" return x;\n"
7671+
"}\n";
7672+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", 1));
7673+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, 1));
7674+
7675+
code = "void f(int a) {\n"
7676+
" int x = a > 0 ? a : 0;\n"
7677+
" return x;\n"
7678+
"}\n";
7679+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1));
7680+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, -1));
7681+
7682+
code = "void f(int a) {\n"
7683+
" int x = a > 0 ? 0 : a;\n"
7684+
" return x;\n"
7685+
"}\n";
7686+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", 1));
7687+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, 1));
7688+
7689+
code = "void f(int a) {\n"
7690+
" int x = a < 0 ? 0 : a;\n"
7691+
" return x;\n"
7692+
"}\n";
7693+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "a", -1));
7694+
ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, -1));
7695+
}
76347696
};
76357697

76367698
REGISTER_TEST(TestValueFlow)

0 commit comments

Comments
 (0)