walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Fixed #7847 (Can't detect shift negative values when some op is executed)

This commit is contained in:
Daniel Marjamäki 2016-12-22 09:40:19 +01:00
parent d79688c40b
commit df6ae9f3b4
3 changed files with 18 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
lib/checkother.cpp
View File

@ -2303,6 +2303,8 @@ static bool isNegative(const Token *tok, const Settings *settings)
void CheckOther::checkNegativeBitwiseShift() void CheckOther::checkNegativeBitwiseShift()
{ {
const bool portability = _settings->isEnabled("portability");
for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) { for (const Token* tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (!tok->astOperand1() || !tok->astOperand2()) if (!tok->astOperand1() || !tok->astOperand2())
continue; continue;
@ -2312,15 +2314,8 @@ void CheckOther::checkNegativeBitwiseShift()
// don't warn if lhs is a class. this is an overloaded operator then // don't warn if lhs is a class. this is an overloaded operator then
if (_tokenizer->isCPP()) { if (_tokenizer->isCPP()) {
const Token *rhs = tok->astOperand1(); const ValueType * lhsType = tok->astOperand1()->valueType();
while (Token::Match(rhs, "::|.")) if (!lhsType || !lhsType->isIntegral())
rhs = rhs->astOperand2();
if (!rhs)
continue;
if (!rhs->isNumber() && !rhs->variable())
continue;
if (rhs->variable() &&
(!rhs->variable()->typeStartToken() || !rhs->variable()->typeStartToken()->isStandardType()))
continue; continue;
} }
@ -2336,7 +2331,7 @@ void CheckOther::checkNegativeBitwiseShift()
continue; continue;
// Get negative rhs value. preferably a value which doesn't have 'condition'. // Get negative rhs value. preferably a value which doesn't have 'condition'.
if (isNegative(tok->astOperand1(), _settings)) if (portability && isNegative(tok->astOperand1(), _settings))
negativeBitwiseShiftError(tok, 1); negativeBitwiseShiftError(tok, 1);
else if (isNegative(tok->astOperand2(), _settings)) else if (isNegative(tok->astOperand2(), _settings))
negativeBitwiseShiftError(tok, 2); negativeBitwiseShiftError(tok, 2);
@ -2346,8 +2341,11 @@ void CheckOther::checkNegativeBitwiseShift()
void CheckOther::negativeBitwiseShiftError(const Token *tok, int op) void CheckOther::negativeBitwiseShiftError(const Token *tok, int op)
{ {
if (op == 1) // LHS if (op == 1)
reportError(tok, Severity::error, "shiftNegative", "Shifting a negative value is undefined behaviour", CWE758, false); // LHS - this is used by intention in various software, if it
// is used often in a project and works as expected then this is
// a portability issue
reportError(tok, Severity::portability, "shiftNegativeLHS", "Shifting a negative value is technically undefined behaviour", CWE758, false);
else // RHS else // RHS
reportError(tok, Severity::error, "shiftNegative", "Shifting by a negative value is undefined behaviour", CWE758, false); reportError(tok, Severity::error, "shiftNegative", "Shifting by a negative value is undefined behaviour", CWE758, false);
} }

1
lib/checkother.h
View File

@ -268,6 +268,7 @@ private:
c.misusedScopeObjectError(nullptr, "varname"); c.misusedScopeObjectError(nullptr, "varname");
c.invalidPointerCastError(nullptr, "float", "double", false); c.invalidPointerCastError(nullptr, "float", "double", false);
c.negativeBitwiseShiftError(nullptr, 1); c.negativeBitwiseShiftError(nullptr, 1);
c.negativeBitwiseShiftError(nullptr, 2);
c.checkPipeParameterSizeError(nullptr, "varname", "dimension"); c.checkPipeParameterSizeError(nullptr, "varname", "dimension");
c.raceAfterInterlockedDecrementError(nullptr); c.raceAfterInterlockedDecrementError(nullptr);

10
test/testother.cpp
View File

@ -4618,13 +4618,17 @@ private:
" std::cout << 3 << -1 ;\n" " std::cout << 3 << -1 ;\n"
"}"); "}");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void foo() {\n"
" x = (-10+2) << 3;\n"
"}");
ASSERT_EQUALS("[test.cpp:2]: (portability) Shifting a negative value is technically undefined behaviour\n", errout.str());
check("x = y ? z << $-1 : 0;\n"); check("x = y ? z << $-1 : 0;\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
// Negative LHS // Negative LHS
check("const int x = -1 >> 2;"); check("const int x = -1 >> 2;");
ASSERT_EQUALS("[test.cpp:1]: (error) Shifting a negative value is undefined behaviour\n", errout.str()); ASSERT_EQUALS("[test.cpp:1]: (portability) Shifting a negative value is technically undefined behaviour\n", errout.str());
// #6383 - unsigned type // #6383 - unsigned type
check("const int x = (unsigned int)(-1) >> 2;"); check("const int x = (unsigned int)(-1) >> 2;");
@ -4637,8 +4641,8 @@ private:
"int shift4() { return -1 << 1 ;}\n"); "int shift4() { return -1 << 1 ;}\n");
ASSERT_EQUALS("[test.cpp:1]: (error) Shifting by a negative value is undefined behaviour\n" ASSERT_EQUALS("[test.cpp:1]: (error) Shifting by a negative value is undefined behaviour\n"
"[test.cpp:2]: (error) Shifting by a negative value is undefined behaviour\n" "[test.cpp:2]: (error) Shifting by a negative value is undefined behaviour\n"
"[test.cpp:3]: (error) Shifting a negative value is undefined behaviour\n" "[test.cpp:3]: (portability) Shifting a negative value is technically undefined behaviour\n"
"[test.cpp:4]: (error) Shifting a negative value is undefined behaviour\n", errout.str()); "[test.cpp:4]: (portability) Shifting a negative value is technically undefined behaviour\n", errout.str());
} }
void incompleteArrayFill() { void incompleteArrayFill() {