diff --git a/lib/check64bit.cpp b/lib/check64bit.cpp index a89644849..ead4971f6 100644 --- a/lib/check64bit.cpp +++ b/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::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."); +} diff --git a/lib/check64bit.h b/lib/check64bit.h index e72fcb434..8ff3617c1 100644 --- a/lib/check64bit.h +++ b/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"; } }; /// @} diff --git a/test/test64bit.cpp b/test/test64bit.cpp index 914307b4f..4554cb57d 100644 --- a/test/test64bit.cpp +++ b/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)