Improve check: Check for 64-bit portability issues when returning pointers/integers

lib/check64bit.cpp

@@ -47,9 +47,35 @@ void Check64BitPortability::pointerassignment()
     if (!_settings->isEnabled("portability"))
         return;
 
+    const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
+
+    // Check return values
+    for (std::list<Scope>::const_iterator scope = symbolDatabase->scopeList.begin(); scope != symbolDatabase->scopeList.end(); ++scope) {
+        if (scope->type != Scope::eFunction || scope->function == 0 || !scope->function->hasBody) // We only look for functions with a body
+            continue;
+
+        bool retPointer = false;
+        if (scope->function->token->strAt(-1) == "*") // Function returns a pointer
+            retPointer = true;
+        else if (Token::Match(scope->function->token->previous(), "int|long|DWORD")) // Function returns an integer
+            ;
+        else
+            continue;
+
+        for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
+            if (Token::Match(tok, "return %var% [;+]")) {
+                const Variable *var = symbolDatabase->getVariableFromVarId(tok->next()->varId());
+                if (retPointer && isint(var))
+                    returnIntegerError(tok);
+                else if (!retPointer && isaddr(var))
+                    returnPointerError(tok);
+            }
+        }
+    }
+
+    // Check assignements
     for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
         if (Token::Match(tok, "[;{}] %var% = %var% [;+]")) {
-            const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
 
             const Variable *var1(symbolDatabase->getVariableFromVarId(tok->next()->varId()));
             const Variable *var2(symbolDatabase->getVariableFromVarId(tok->tokAt(3)->varId()));
@@ -90,3 +116,25 @@ void Check64BitPortability::assignmentIntegerToAddressError(const Token *tok)
                 "they are of different width. In worst case you end up assigning 32-bit integer to 64-bit pointer. The safe "
                 "way is to always assign address to pointer.");
 }
+
+void Check64BitPortability::returnPointerError(const Token *tok)
+{
+    reportError(tok, Severity::portability,
+                "CastAddressToIntegerAtReturn",
+                "Returning an address value in a function with integer return type is not portable.\n"
+                "Returning an address value in a function with integer (int/long/etc) return type is not portable across "
+                "different platforms and compilers. For example in 32-bit Windows and Linux they are same width, but in "
+                "64-bit Windows and Linux they are of different width. In worst case you end up casting 64-bit address down "
+                "to 32-bit integer. The safe way is to always return an integer.");
+}
+
+void Check64BitPortability::returnIntegerError(const Token *tok)
+{
+    reportError(tok, Severity::portability,
+                "CastIntegerToAddressAtReturn",
+                "Returning an integer in a function that returns a pointer is not portable.\n"
+                "Returning an integer (int/long/etc) in a function that returns a pointer is not portable across different "
+                "platforms and compilers. For example in 32-bit Windows and Linux they are same width, but in 64-bit Windows "
+                "and Linux they are of different width. In worst case you end up casting 32-bit integer down to 64-bit pointer. "
+                "The safe way is to always return a pointer.");
+}

lib/check64bit.h

@@ -63,11 +63,15 @@ private:
 
     void assignmentAddressToIntegerError(const Token *tok);
     void assignmentIntegerToAddressError(const Token *tok);
+    void returnIntegerError(const Token *tok);
+    void returnPointerError(const Token *tok);
 
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
         Check64BitPortability c(0, settings, errorLogger);
         c.assignmentAddressToIntegerError(0);
         c.assignmentIntegerToAddressError(0);
+        c.returnIntegerError(0);
+        c.returnPointerError(0);
     }
 
     std::string myName() const {
@@ -76,7 +80,8 @@ private:
 
     std::string classInfo() const {
         return "Check if there is 64-bit portability issues:\n"
-               "* assign address to/from int/long";
+               "* assign address to/from int/long\n"
+               "* casting address from/to integer when returning from function";
     }
 };
 /// @}

test/test64bit.cpp

@@ -38,6 +38,7 @@ private:
         TEST_CASE(structmember);
         TEST_CASE(ptrcompare);
         TEST_CASE(ptrarithmetic);
+        TEST_CASE(returnIssues);
     }
 
     void check(const char code[]) {
@@ -86,7 +87,7 @@ private:
               "    int *a = p;\n"
               "    return a;\n"
               "}\n");
-        ASSERT_EQUALS("", errout.str());
+        ASSERT_EQUALS("[test.cpp:4]: (portability) Returning an address value in a function with integer return type is not portable.\n", errout.str());
 
         check("void foo(int x)\n"
               "{\n"
@@ -138,6 +139,33 @@ private:
               "}\n");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void returnIssues() {
+        check("void* foo(int i) {\n"
+              "    return i;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (portability) Returning an integer in a function that returns a pointer is not portable.\n", errout.str());
+
+        check("void* foo(int* i) {\n"
+              "    return i;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int foo(int i) {\n"
+              "    return i;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int foo(char* c) {\n"
+              "    return c;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2]: (portability) Returning an address value in a function with integer return type is not portable.\n", errout.str());
+
+        check("std::string foo(char* c) {\n"
+              "    return c;\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 REGISTER_TEST(Test64BitPortability)