Fixed #6707 (new check: possible truncation when assigning int result to long)

lib/checktype.cpp

@@ -30,6 +30,19 @@ namespace {
     CheckType instance;
 }
 
+static bool astIsIntResult(const Token *tok)
+{
+    if (!tok)
+        return false;
+    if (tok->astOperand1())
+        return astIsIntResult(tok->astOperand1()) && astIsIntResult(tok->astOperand2());
+    // todo: handle numbers
+    if (!tok->variable())
+        return false;
+    const Token *type = tok->variable()->typeStartToken();
+    return Token::Match(type, "char|short|int") && !type->isLong();
+}
+
 static bool astGetSizeSign(const Settings *settings, const Token *tok, unsigned int *size, char *sign)
 {
     if (!tok)
@@ -287,3 +300,77 @@ void CheckType::signConversionError(const Token *tok)
                 "signConversion",
                 "Suspicious code: sign conversion of " + varname + " in calculation, even though " + varname + " can have a negative value");
 }
+
+
+//---------------------------------------------------------------------------
+// Checking for long cast of int result   const long x = var1 * var2;
+//---------------------------------------------------------------------------
+
+void CheckType::checkLongCast()
+{
+    if (!_settings->isEnabled("style"))
+        return;
+
+    // Assignments..
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (!Token::Match(tok, "%var% ="))
+            continue;
+        if (!tok->variable() || !tok->variable()->isConst() || tok->variable()->typeStartToken()->str() != "long")
+            continue;
+        if (Token::Match(tok->next()->astOperand2(), "*|<<") && astIsIntResult(tok->next()->astOperand2()))
+            longCastAssignError(tok);
+    }
+
+    // Return..
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+    const std::size_t functions = symbolDatabase->functionScopes.size();
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope * scope = symbolDatabase->functionScopes[i];
+
+        // function must return long data
+        const Token * def = scope->classDef;
+        bool islong = false;
+        while (Token::Match(def, "%type%|::")) {
+            if (def->str() == "long") {
+                islong = true;
+                break;
+            }
+            def = def->previous();
+        }
+        if (!islong)
+            continue;
+
+        // find return statement
+        // todo.. this is slow, we should only check last statement in each child scope
+        const Token *ret = nullptr;
+        for (const Token *tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
+            if (tok->str() == "return") {
+                if (!ret)
+                    ret = tok;
+                else {
+                    ret =  nullptr;
+                    break;
+                }
+            }
+        }
+
+        if (ret && Token::Match(ret->astOperand1(), "*|<<") && astIsIntResult(ret->astOperand1()))
+            longCastReturnError(ret);
+    }
+}
+
+void CheckType::longCastAssignError(const Token *tok)
+{
+    reportError(tok,
+                Severity::style,
+                "truncLongCastAssignment",
+                "possible loss of information, int result is assigned to long variable");
+}
+
+void CheckType::longCastReturnError(const Token *tok)
+{
+    reportError(tok,
+                Severity::style,
+                "truncLongCastReturn",
+                "possible loss of information, int result is returned as long value");
+}

lib/checktype.h

@@ -49,6 +49,7 @@ public:
         checkType.checkTooBigBitwiseShift();
         checkType.checkIntegerOverflow();
         checkType.checkSignConversion();
+        checkType.checkLongCast();
     }
 
     /** @brief Run checks against the simplified token list */
@@ -67,6 +68,8 @@ public:
     /** @brief %Check for dangerous sign conversion */
     void checkSignConversion();
 
+    /** @brief %Check for implicit long cast of int result */
+    void checkLongCast();
 private:
     bool isUnsigned(const Variable *var) const;
     static bool isSigned(const Variable *var);
@@ -75,12 +78,16 @@ private:
     void tooBigBitwiseShiftError(const Token *tok, int lhsbits, const ValueFlow::Value &rhsbits);
     void integerOverflowError(const Token *tok, const ValueFlow::Value &value);
     void signConversionError(const Token *tok);
+    void longCastAssignError(const Token *tok);
+    void longCastReturnError(const Token *tok);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         CheckType c(0, settings, errorLogger);
         c.tooBigBitwiseShiftError(0, 32, ValueFlow::Value(64));
         c.integerOverflowError(0, ValueFlow::Value(1LL<<32));
         c.signConversionError(0);
+        c.longCastAssignError(0);
+        c.longCastReturnError(0);
     }
 
     static std::string myName() {
@@ -91,7 +98,9 @@ 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"
-               "- dangerous sign conversion, when signed value can be negative\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";
     }
 };
 /// @}

test/testtype.cpp

@@ -37,6 +37,8 @@ private:
         TEST_CASE(checkTooBigShift);
         TEST_CASE(checkIntegerOverflow);
         TEST_CASE(signConversion);
+        TEST_CASE(longCastAssign);
+        TEST_CASE(longCastReturn);
     }
 
     void check(const char code[], Settings* settings = 0) {
@@ -134,6 +136,34 @@ private:
               "void f2() { f1(-4); }");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void longCastAssign() {
+        Settings settings;
+        settings.addEnabled("style");
+
+        check("long f(int x, int y) {\n"
+              "  const long ret = x * y;\n"
+              "  return ret;\n"
+              "}\n", &settings);
+        ASSERT_EQUALS("[test.cpp:2]: (style) possible loss of information, int result is assigned to long variable\n", errout.str());
+
+        // astIsIntResult
+        check("long f(int x, int y) {\n"
+              "  const long ret = (long)x * y;\n"
+              "  return ret;\n"
+              "}\n", &settings);
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void longCastReturn() {
+        Settings settings;
+        settings.addEnabled("style");
+
+        check("long f(int x, int y) {\n"
+              "  return x * y;\n"
+              "}\n", &settings);
+        ASSERT_EQUALS("[test.cpp:2]: (style) possible loss of information, int result is returned as long value\n", errout.str());
+    }
 };
 
 REGISTER_TEST(TestType)