Skip to content

Commit 1f27c4b

Browse files
Dmitry-MeDmitry-Me
committed
Merge pull request #771 from Dmitry-Me/charUsedAsArrayIndex
Portability warning when 'char' type is used as array index
2 parents 3da997e + c339949 commit 1f27c4b

5 files changed

Lines changed: 109 additions & 10 deletions

File tree

‎lib/astutils.cpp‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,24 @@
2424
#include "tokenize.h"
2525
#include <set>
2626

27+
static bool astIsCharWithSign(const Token *tok, ValueType::Sign sign)
28+
{
29+
if (!tok)
30+
return false;
31+
const ValueType *valueType = tok->valueType();
32+
if (!valueType)
33+
return false;
34+
return valueType->type == ValueType::Type::CHAR && valueType->pointer == 0U && valueType->sign == sign;
35+
}
36+
2737
bool astIsSignedChar(const Token *tok)
2838
{
29-
return tok && tok->valueType() && tok->valueType()->sign == ValueType::Sign::SIGNED && tok->valueType()->type == ValueType::Type::CHAR && tok->valueType()->pointer == 0U;
39+
return astIsCharWithSign(tok, ValueType::Sign::SIGNED);
40+
}
41+
42+
bool astIsUnknownSignChar(const Token *tok)
43+
{
44+
return astIsCharWithSign(tok, ValueType::Sign::UNKNOWN_SIGN);
3045
}
3146

3247
bool astIsIntegral(const Token *tok, bool unknown)

‎lib/astutils.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ class Token;
2929

3030
/** Is expression a 'signed char' if no promotion is used */
3131
bool astIsSignedChar(const Token *tok);
32+
/** Is expression a 'char' if no promotion is used? */
33+
bool astIsUnknownSignChar(const Token *tok);
3234
/** Is expression of integral type? */
3335
bool astIsIntegral(const Token *tok, bool unknown);
3436
/** Is expression of floating point type? */

‎lib/checkother.cpp‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1417,7 +1417,9 @@ void CheckOther::passedByValueError(const Token *tok, const std::string &parname
14171417

14181418
void CheckOther::checkCharVariable()
14191419
{
1420-
if (!_settings->isEnabled("warning"))
1420+
const bool warning = _settings->isEnabled("warning");
1421+
const bool portability = _settings->isEnabled("portability");
1422+
if (!warning && !portability)
14211423
return;
14221424

14231425
const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
@@ -1426,12 +1428,16 @@ void CheckOther::checkCharVariable()
14261428
const Scope * scope = symbolDatabase->functionScopes[i];
14271429
for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
14281430
if (Token::Match(tok, "%var% [")) {
1429-
if (!tok->variable() || !tok->variable()->isArray())
1431+
if (!tok->variable())
1432+
continue;
1433+
if (!tok->variable()->isArray() && !tok->variable()->isPointer())
14301434
continue;
14311435
const Token *index = tok->next()->astOperand2();
1432-
if (astIsSignedChar(index) && index->getValueGE(0x80, _settings))
1433-
charArrayIndexError(tok);
1434-
} else if (Token::Match(tok, "[&|^]") && tok->astOperand2() && tok->astOperand1()) {
1436+
if (warning && tok->variable()->isArray() && astIsSignedChar(index) && index->getValueGE(0x80, _settings))
1437+
signedCharArrayIndexError(tok);
1438+
if (portability && astIsUnknownSignChar(index) && index->getValueGE(0x80, _settings))
1439+
unknownSignCharArrayIndexError(tok);
1440+
} else if (warning && Token::Match(tok, "[&|^]") && tok->astOperand2() && tok->astOperand1()) {
14351441
bool warn = false;
14361442
if (astIsSignedChar(tok->astOperand1())) {
14371443
const ValueFlow::Value *v1 = tok->astOperand1()->getValueLE(-1, _settings);
@@ -1460,17 +1466,27 @@ void CheckOther::checkCharVariable()
14601466
}
14611467
}
14621468

1463-
void CheckOther::charArrayIndexError(const Token *tok)
1469+
void CheckOther::signedCharArrayIndexError(const Token *tok)
14641470
{
14651471
reportError(tok,
14661472
Severity::warning,
1467-
"charArrayIndex",
1473+
"signedCharArrayIndex",
14681474
"Signed 'char' type used as array index.\n"
14691475
"Signed 'char' type used as array index. If the value "
14701476
"can be greater than 127 there will be a buffer underflow "
14711477
"because of sign extension.");
14721478
}
14731479

1480+
void CheckOther::unknownSignCharArrayIndexError(const Token *tok)
1481+
{
1482+
reportError(tok,
1483+
Severity::portability,
1484+
"unknownSignCharArrayIndex",
1485+
"'char' type used as array index.\n"
1486+
"'char' type used as array index. Values greater that 127 will be "
1487+
"treated depending on whether 'char' is signed or unsigned on target platform.");
1488+
}
1489+
14741490
void CheckOther::charBitOpError(const Token *tok)
14751491
{
14761492
reportError(tok,

‎lib/checkother.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,8 @@ class CPPCHECKLIB CheckOther : public Check {
218218
void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive);
219219
void passedByValueError(const Token *tok, const std::string &parname);
220220
void constStatementError(const Token *tok, const std::string &type);
221-
void charArrayIndexError(const Token *tok);
221+
void signedCharArrayIndexError(const Token *tok);
222+
void unknownSignCharArrayIndexError(const Token *tok);
222223
void charBitOpError(const Token *tok);
223224
void variableScopeError(const Token *tok, const std::string &varname);
224225
void zerodivError(const Token *tok, bool inconclusive);
@@ -280,7 +281,8 @@ class CPPCHECKLIB CheckOther : public Check {
280281
c.cstyleCastError(0);
281282
c.passedByValueError(0, "parametername");
282283
c.constStatementError(0, "type");
283-
c.charArrayIndexError(0);
284+
c.signedCharArrayIndexError(0);
285+
c.unknownSignCharArrayIndexError(0);
284286
c.charBitOpError(0);
285287
c.variableScopeError(0, "varname");
286288
c.redundantAssignmentInSwitchError(0, 0, "var");

‎test/testcharvar.cpp‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class TestCharVar : public TestFixture {
3232
void run() {
3333
settings.platform(Settings::Unspecified);
3434
settings.addEnabled("warning");
35+
settings.addEnabled("portability");
3536

3637
TEST_CASE(array_index_1);
3738
TEST_CASE(array_index_2);
@@ -67,6 +68,14 @@ class TestCharVar : public TestFixture {
6768
" char ch = 0x80;\n"
6869
" buf[ch] = 0;\n"
6970
"}");
71+
ASSERT_EQUALS("[test.cpp:5]: (portability) 'char' type used as array index.\n", errout.str());
72+
73+
check("int buf[256];\n"
74+
"void foo()\n"
75+
"{\n"
76+
" char ch = 0;\n"
77+
" buf[ch] = 0;\n"
78+
"}");
7079
ASSERT_EQUALS("", errout.str());
7180

7281
check("int buf[256];\n"
@@ -85,13 +94,68 @@ class TestCharVar : public TestFixture {
8594
"}");
8695
ASSERT_EQUALS("", errout.str());
8796

97+
check("int buf[256];\n"
98+
"void foo()\n"
99+
"{\n"
100+
" char ch = 0x80;\n"
101+
" buf[ch] = 0;\n"
102+
"}");
103+
ASSERT_EQUALS("[test.cpp:5]: (portability) 'char' type used as array index.\n", errout.str());
104+
88105
check("int buf[256];\n"
89106
"void foo(signed char ch)\n"
90107
"{\n"
91108
" buf[ch] = 0;\n"
92109
"}");
93110
ASSERT_EQUALS("", errout.str());
94111

112+
check("int buf[256];\n"
113+
"void foo(char ch)\n"
114+
"{\n"
115+
" buf[ch] = 0;\n"
116+
"}");
117+
ASSERT_EQUALS("", errout.str());
118+
119+
check("void foo(char* buf)\n"
120+
"{\n"
121+
" char ch = 0x80;"
122+
" buf[ch] = 0;\n"
123+
"}");
124+
ASSERT_EQUALS("[test.cpp:3]: (portability) 'char' type used as array index.\n", errout.str());
125+
126+
check("void foo(char* buf)\n"
127+
"{\n"
128+
" char ch = 0;"
129+
" buf[ch] = 0;\n"
130+
"}");
131+
ASSERT_EQUALS("", errout.str());
132+
133+
check("void foo(char* buf)\n"
134+
"{\n"
135+
" buf['A'] = 0;\n"
136+
"}");
137+
ASSERT_EQUALS("", errout.str());
138+
139+
check("void foo(char* buf, char ch)\n"
140+
"{\n"
141+
" buf[ch] = 0;\n"
142+
"}");
143+
ASSERT_EQUALS("", errout.str());
144+
145+
check("int flags[256];\n"
146+
"void foo(const char* str)\n"
147+
"{\n"
148+
" flags[*str] = 0;\n"
149+
"}");
150+
ASSERT_EQUALS("", errout.str());
151+
152+
check("int flags[256];\n"
153+
"void foo(const char* str)\n"
154+
"{\n"
155+
" flags[(unsigned char)*str] = 0;\n"
156+
"}");
157+
ASSERT_EQUALS("", errout.str());
158+
95159
check("void foo(const char str[])\n"
96160
"{\n"
97161
" map[str] = 0;\n"

0 commit comments

Comments
 (0)