Clarify signConversion warning message

danmar · danmar · commit 20278d9c929d · 2019-08-05T12:41:08.000+02:00
diff --git 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
@@ -81,7 +81,7 @@ class CPPCHECKLIB CheckType : public Check {
     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 @@ class CPPCHECKLIB CheckType : public Check {
         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
@@ -194,7 +194,7 @@ class TestType : public TestFixture {
               "  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"
