From 20278d9c929d35cfd55d82e600e03f66700c6d87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 5 Aug 2019 12:41:08 +0200 Subject: [PATCH] Clarify signConversion warning message --- lib/checktype.cpp | 21 ++++++++++++++++----- lib/checktype.h | 4 ++-- test/testtype.cpp | 2 +- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/lib/checktype.cpp b/lib/checktype.cpp index 9d09f7d79..081acf6b8 100644 --- a/lib/checktype.cpp +++ b/lib/checktype.cpp @@ -70,7 +70,7 @@ void CheckType::checkTooBigBitwiseShift() // get number of bits of lhs const ValueType * const lhstype = tok->astOperand1()->valueType(); - if (!lhstype || !lhstype->isIntegral() || lhstype->pointer >= 1U) + if (!lhstype || !lhstype->isIntegral() || lhstype->pointer >= 1) continue; // C11 Standard, section 6.5.7 Bitwise shift operators, states: // The integer promotions are performed on each of the operands. @@ -237,15 +237,16 @@ void CheckType::checkSignConversion() tokens.pop(); if (!tok1) continue; - if (!tok1->getValueLE(-1,mSettings)) + const ValueFlow::Value *negativeValue = tok1->getValueLE(-1,mSettings); + if (!negativeValue) continue; if (tok1->valueType() && tok1->valueType()->sign != ValueType::Sign::UNSIGNED) - signConversionError(tok1, tok1->isNumber()); + signConversionError(tok1, negativeValue, tok1->isNumber()); } } } -void CheckType::signConversionError(const Token *tok, const bool constvalue) +void CheckType::signConversionError(const Token *tok, const ValueFlow::Value *negativeValue, const bool constvalue) { const std::string expr(tok ? tok->expressionString() : "var"); @@ -257,7 +258,17 @@ void CheckType::signConversionError(const Token *tok, const bool constvalue) else msg << "Expression '" << expr << "' can have a negative value. That is converted to an unsigned value and used in an unsigned calculation."; - reportError(tok, Severity::warning, "signConversion", msg.str(), CWE195, false); + if (!negativeValue) + reportError(tok, Severity::warning, "signConversion", msg.str(), CWE195, false); + else { + const ErrorPath &errorPath = getErrorPath(tok,negativeValue,"Negative value is converted to an unsigned value"); + reportError(errorPath, + Severity::warning, + Check::getMessageId(*negativeValue, "signConversion").c_str(), + msg.str(), + CWE195, + negativeValue->isInconclusive()); + } } diff --git a/lib/checktype.h b/lib/checktype.h index 8caa51679..e51da8d1d 100644 --- a/lib/checktype.h +++ b/lib/checktype.h @@ -81,7 +81,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 signConversionError(const Token *tok, const bool constvalue); + void signConversionError(const Token *tok, const ValueFlow::Value *negativeValue, const bool constvalue); void longCastAssignError(const Token *tok); void longCastReturnError(const Token *tok); void floatToIntegerOverflowError(const Token *tok, const ValueFlow::Value &value); @@ -91,7 +91,7 @@ private: c.tooBigBitwiseShiftError(nullptr, 32, ValueFlow::Value(64)); c.tooBigSignedBitwiseShiftError(nullptr, 31, ValueFlow::Value(31)); c.integerOverflowError(nullptr, ValueFlow::Value(1LL<<32)); - c.signConversionError(nullptr, false); + c.signConversionError(nullptr, nullptr, false); c.longCastAssignError(nullptr); c.longCastReturnError(nullptr); ValueFlow::Value f; diff --git a/test/testtype.cpp b/test/testtype.cpp index c6e120442..7450de4c6 100644 --- a/test/testtype.cpp +++ b/test/testtype.cpp @@ -194,7 +194,7 @@ private: " if (x==0) {}\n" " return (x-1)*sizeof(int);\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3]: (warning) Expression 'x-1' can have a negative value. That is converted to an unsigned value and used in an unsigned calculation.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Expression 'x-1' can have a negative value. That is converted to an unsigned value and used in an unsigned calculation.\n", errout.str()); check("unsigned int f1(signed int x, unsigned int y) {" // x is signed " return x * y;\n"