Skip to content

Commit 1f71e8c

Browse files
firewavefirewave
committed
fixed #13673/#14076 - fixed signedCharArrayIndex detection
1 parent 107e226 commit 1f71e8c

File tree

3 files changed

+70
-5
lines changed

3 files changed

+70
-5
lines changed

‎lib/checkother.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2023,7 +2023,7 @@ void CheckOther::checkCharVariable()
20232023
if (!tok->variable()->isArray() && !tok->variable()->isPointer())
20242024
continue;
20252025
const Token *index = tok->next()->astOperand2();
2026-
if (warning && tok->variable()->isArray() && astIsSignedChar(index) && index->getValueGE(0x80, *mSettings))
2026+
if (warning && astIsSignedChar(index) && index->getValueLE(-1, *mSettings))
20272027
signedCharArrayIndexError(tok);
20282028
if (portability && astIsUnknownSignChar(index) && index->getValueGE(0x80, *mSettings))
20292029
unknownSignCharArrayIndexError(tok);

‎lib/token.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1922,6 +1922,7 @@ const ValueFlow::Value * Token::getValueLE(const MathLib::bigint val, const Sett
19221922
{
19231923
if (!mImpl->mValues)
19241924
return nullptr;
1925+
// TODO: assert the given value is actually possible - i.e. 0x80 for a signed char is not possible
19251926
return ValueFlow::findValue(*mImpl->mValues, settings, [&](const ValueFlow::Value& v) {
19261927
return !v.isImpossible() && v.isIntValue() && v.intvalue <= val;
19271928
});
@@ -1931,6 +1932,7 @@ const ValueFlow::Value * Token::getValueGE(const MathLib::bigint val, const Sett
19311932
{
19321933
if (!mImpl->mValues)
19331934
return nullptr;
1935+
// TODO: assert the given value is actually possible - i.e. 0x80 for a signed char is not possible
19341936
return ValueFlow::findValue(*mImpl->mValues, settings, [&](const ValueFlow::Value& v) {
19351937
return !v.isImpossible() && v.isIntValue() && v.intvalue >= val;
19361938
});
@@ -1940,6 +1942,7 @@ const ValueFlow::Value * Token::getValueNE(MathLib::bigint val) const
19401942
{
19411943
if (!mImpl->mValues)
19421944
return nullptr;
1945+
// TODO: assert the given value is actually possible - i.e. 0x80 for a signed char is not possible
19431946
const auto it = std::find_if(mImpl->mValues->cbegin(), mImpl->mValues->cend(), [=](const ValueFlow::Value& value) {
19441947
return value.isIntValue() && !value.isImpossible() && value.intvalue != val;
19451948
});

‎test/testcharvar.cpp‎

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ class TestCharVar : public TestFixture {
6060
"}");
6161
ASSERT_EQUALS("", errout_str());
6262

63+
check("int buf[256];\n"
64+
"void foo()\n"
65+
"{\n"
66+
" signed char ch = 0x80;\n"
67+
" buf[ch] = 0;\n"
68+
"}");
69+
ASSERT_EQUALS("[test.cpp:5:5]: (warning) Signed 'char' type used as array index. [signedCharArrayIndex]\n", errout_str());
70+
6371
check("int buf[256];\n"
6472
"void foo()\n"
6573
"{\n"
@@ -71,7 +79,7 @@ class TestCharVar : public TestFixture {
7179
check("int buf[256];\n"
7280
"void foo()\n"
7381
"{\n"
74-
" char ch = 0;\n"
82+
" unsigned char ch = 0;\n"
7583
" buf[ch] = 0;\n"
7684
"}");
7785
ASSERT_EQUALS("", errout_str());
@@ -87,10 +95,17 @@ class TestCharVar : public TestFixture {
8795
check("int buf[256];\n"
8896
"void foo()\n"
8997
"{\n"
90-
" char ch = 0x80;\n"
98+
" char ch = 0;\n"
9199
" buf[ch] = 0;\n"
92100
"}");
93-
ASSERT_EQUALS("[test.cpp:5:5]: (portability) 'char' type used as array index. [unknownSignCharArrayIndex]\n", errout_str());
101+
ASSERT_EQUALS("", errout_str());
102+
103+
check("int buf[256];\n"
104+
"void foo(unsigned char ch)\n"
105+
"{\n"
106+
" buf[ch] = 0;\n"
107+
"}");
108+
ASSERT_EQUALS("", errout_str());
94109

95110
check("int buf[256];\n"
96111
"void foo(signed char ch)\n"
@@ -106,13 +121,41 @@ class TestCharVar : public TestFixture {
106121
"}");
107122
ASSERT_EQUALS("", errout_str());
108123

124+
check("void foo(char* buf)\n"
125+
"{\n"
126+
" unsigned char ch = 0x80;"
127+
" buf[ch] = 0;\n"
128+
"}");
129+
ASSERT_EQUALS("", errout_str());
130+
131+
check("void foo(char* buf)\n"
132+
"{\n"
133+
" signed char ch = 0x80;"
134+
" buf[ch] = 0;\n"
135+
"}");
136+
ASSERT_EQUALS("[test.cpp:3:31]: (warning) Signed 'char' type used as array index. [signedCharArrayIndex]\n", errout_str());
137+
109138
check("void foo(char* buf)\n"
110139
"{\n"
111140
" char ch = 0x80;"
112141
" buf[ch] = 0;\n"
113142
"}");
114143
ASSERT_EQUALS("[test.cpp:3:24]: (portability) 'char' type used as array index. [unknownSignCharArrayIndex]\n", errout_str());
115144

145+
check("void foo(char* buf)\n"
146+
"{\n"
147+
" unsigned char ch = 0;"
148+
" buf[ch] = 0;\n"
149+
"}");
150+
ASSERT_EQUALS("", errout_str());
151+
152+
check("void foo(char* buf)\n"
153+
"{\n"
154+
" signed char ch = 0;"
155+
" buf[ch] = 0;\n"
156+
"}");
157+
ASSERT_EQUALS("", errout_str());
158+
116159
check("void foo(char* buf)\n"
117160
"{\n"
118161
" char ch = 0;"
@@ -126,6 +169,18 @@ class TestCharVar : public TestFixture {
126169
"}");
127170
ASSERT_EQUALS("", errout_str());
128171

172+
check("void foo(char* buf, unsigned char ch)\n"
173+
"{\n"
174+
" buf[ch] = 0;\n"
175+
"}");
176+
ASSERT_EQUALS("", errout_str());
177+
178+
check("void foo(char* buf, signed char ch)\n"
179+
"{\n"
180+
" buf[ch] = 0;\n"
181+
"}");
182+
ASSERT_EQUALS("", errout_str());
183+
129184
check("void foo(char* buf, char ch)\n"
130185
"{\n"
131186
" buf[ch] = 0;\n"
@@ -135,7 +190,7 @@ class TestCharVar : public TestFixture {
135190
check("int flags[256];\n"
136191
"void foo(const char* str)\n"
137192
"{\n"
138-
" flags[*str] = 0;\n"
193+
" flags[(signed char)*str] = 0;\n"
139194
"}");
140195
ASSERT_EQUALS("", errout_str());
141196

@@ -146,6 +201,13 @@ class TestCharVar : public TestFixture {
146201
"}");
147202
ASSERT_EQUALS("", errout_str());
148203

204+
check("int flags[256];\n"
205+
"void foo(const char* str)\n"
206+
"{\n"
207+
" flags[*str] = 0;\n"
208+
"}");
209+
ASSERT_EQUALS("", errout_str());
210+
149211
check("void foo(const char str[])\n"
150212
"{\n"
151213
" map[str] = 0;\n"

0 commit comments

Comments
 (0)