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

New check: Warn about nonportable code that can be optimised (signed integer overflow)

This commit is contained in:
Daniel Marjamäki 2021-01-03 21:03:07 +01:00
parent 208014bae9
commit 6375fb4f08
3 changed files with 85 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
lib/checktype.cpp
View File

@ -222,6 +222,55 @@ void CheckType::integerOverflowError(const Token *tok, const ValueFlow::Value &v
value.isInconclusive());
}
//---------------------------------------------------------------------------------
// Checking for code patterns that might be optimised differently by the compilers
//---------------------------------------------------------------------------------
void CheckType::checkIntegerOverflowOptimisations()
{
// Various patterns are defined here:
// https://kristerw.blogspot.com/2016/02/how-undefined-signed-overflow-enables.html
// These optimisations seem to generate "unwanted" output
// x + c < x -> false
// x + c <= x -> false
// x + c > x -> true
// x + c >= x -> true
if (!mSettings->isEnabled(Settings::PORTABILITY))
return;
for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
if (!Token::Match(tok, "<|<=|>=|>") || !tok->isBinaryOp())
continue;
if (Token::Match(tok->astOperand1(), "[+-]") &&
tok->astOperand1()->valueType() &&
tok->astOperand1()->valueType()->isIntegral() &&
tok->astOperand1()->valueType()->sign == ValueType::Sign::SIGNED &&
tok->astOperand1()->isBinaryOp() &&
tok->astOperand1()->astOperand2()->isNumber() &&
Token::Match(tok->astOperand1()->astOperand1(), "%var%") &&
tok->astOperand1()->astOperand1()->str() == tok->astOperand2()->str()) {
bool result;
if (tok->astOperand1()->str() == "+")
result = Token::Match(tok, ">|>=");
else
result = Token::Match(tok, "<|<=");
integerOverflowOptimisationError(tok, result ? "true" : "false");
}
}
}
void CheckType::integerOverflowOptimisationError(const Token *tok, const std::string &replace)
{
const std::string expr = tok ? tok->expressionString() : "x+c<x";
const std::string errmsg =
"There is a danger that '" + expr + "' will be optimised into '" + replace + "'. Signed integer overflow is undefined behavior.\n"
"There is a danger that '" + expr + "' will be optimised into '" + replace + "'. Your code could work differently depending on what compiler/flags/version/etc is used. Signed integer overflow is undefined behavior and assuming that it will never happen; the result of '" + expr + "' is always '" + replace + "'.";
reportError(tok, Severity::portability, "integerOverflowOptimization", errmsg, CWE190, false);
}
//---------------------------------------------------------------------------
// Checking for sign conversion when operand can be negative
//---------------------------------------------------------------------------

6
lib/checktype.h
View File

@ -54,6 +54,7 @@ public:
CheckType checkType(tokenizer, settings, errorLogger);
checkType.checkTooBigBitwiseShift();
checkType.checkIntegerOverflow();
checkType.checkIntegerOverflowOptimisations();
checkType.checkSignConversion();
checkType.checkLongCast();
checkType.checkFloatToIntegerOverflow();
@ -65,6 +66,9 @@ public:
/** @brief %Check for integer overflow */
void checkIntegerOverflow();
/** @brief Check for overflow code patterns that will be optimized */
void checkIntegerOverflowOptimisations();
/** @brief %Check for dangerous sign conversion */
void checkSignConversion();
@ -81,6 +85,7 @@ private:
void tooBigBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits);
void tooBigSignedBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits);
void integerOverflowError(const Token *tok, const ValueFlow::Value &value);
void integerOverflowOptimisationError(const Token *tok, const std::string &replace);
void signConversionError(const Token *tok, const ValueFlow::Value *negativeValue, const bool constvalue);
void longCastAssignError(const Token *tok);
void longCastReturnError(const Token *tok);
@ -108,6 +113,7 @@ private:
return "Type checks\n"
"- bitwise shift by too many bits (only enabled when --platform is used)\n"
"- signed integer overflow (only enabled when --platform is used)\n"
"- expression that can be optimised 'out' because signed integer overflow is undefined behavior.\n"
"- dangerous sign conversion, when signed value can be negative\n"
"- possible loss of information when assigning int result to long variable\n"
"- possible loss of information when returning int result as long return value\n"

30
test/testtype.cpp
View File

@ -35,6 +35,7 @@ private:
void run() OVERRIDE {
TEST_CASE(checkTooBigShift_Unix32);
TEST_CASE(checkIntegerOverflow);
TEST_CASE(checkIntegerOverflowOptimisations);
TEST_CASE(signConversion);
TEST_CASE(longCastAssign);
TEST_CASE(longCastReturn);
@ -248,6 +249,35 @@ private:
ASSERT_EQUALS("", errout.str());
}
void checkIntegerOverflowOptimisations() {
Settings settings;
settings.addEnabled("warning");
check("int f(int x) { return x + 10 > x; }", &settings);
ASSERT_EQUALS("[test.cpp:1]: (portability) There is a danger that 'x+10>x' will be optimised into 'true'. Signed integer overflow is undefined behavior.\n", errout.str());
check("int f(int x) { return x + 10 >= x; }", &settings);
ASSERT_EQUALS("[test.cpp:1]: (portability) There is a danger that 'x+10>=x' will be optimised into 'true'. Signed integer overflow is undefined behavior.\n", errout.str());
check("int f(int x) { return x + 10 < x; }", &settings);
ASSERT_EQUALS("[test.cpp:1]: (portability) There is a danger that 'x+10<x' will be optimised into 'false'. Signed integer overflow is undefined behavior.\n", errout.str());
check("int f(int x) { return x + 10 <= x; }", &settings);
ASSERT_EQUALS("[test.cpp:1]: (portability) There is a danger that 'x+10<=x' will be optimised into 'false'. Signed integer overflow is undefined behavior.\n", errout.str());
check("int f(int x) { return x - 10 > x; }", &settings);
ASSERT_EQUALS("[test.cpp:1]: (portability) There is a danger that 'x-10>x' will be optimised into 'false'. Signed integer overflow is undefined behavior.\n", errout.str());
check("int f(int x) { return x - 10 >= x; }", &settings);
ASSERT_EQUALS("[test.cpp:1]: (portability) There is a danger that 'x-10>=x' will be optimised into 'false'. Signed integer overflow is undefined behavior.\n", errout.str());
check("int f(int x) { return x - 10 < x; }", &settings);
ASSERT_EQUALS("[test.cpp:1]: (portability) There is a danger that 'x-10<x' will be optimised into 'true'. Signed integer overflow is undefined behavior.\n", errout.str());
check("int f(int x) { return x - 10 <= x; }", &settings);
ASSERT_EQUALS("[test.cpp:1]: (portability) There is a danger that 'x-10<=x' will be optimised into 'true'. Signed integer overflow is undefined behavior.\n", errout.str());
}
void signConversion() {
check("x = -4 * (unsigned)y;");
ASSERT_EQUALS("[test.cpp:1]: (warning) Expression '-4' has a negative value. That is converted to an unsigned value and used in an unsigned calculation.\n", errout.str());