diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 282f3f3e50e..123a9fce2ee 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -290,6 +290,44 @@ void CheckOther::suspiciousSemicolonError(const Token* tok) "Suspicious use of ; at the end of '" + (tok ? tok->str() : std::string()) + "' statement.", CWE398, Certainty::normal); } +/** @brief would it make sense to use dynamic_cast instead of C style cast? */ +static bool isDangerousTypeConversion(const Token* const tok) +{ + const Token* from = tok->astOperand1(); + if (!from) + return false; + if (!tok->valueType() || !from->valueType()) + return false; + if (tok->valueType()->typeScope != nullptr && + tok->valueType()->typeScope == from->valueType()->typeScope) + return false; + if (tok->valueType()->type == from->valueType()->type && + tok->valueType()->isPrimitive()) + return false; + // cast from derived object to base object is safe.. + if (tok->valueType()->typeScope && from->valueType()->typeScope) { + const Type* fromType = from->valueType()->typeScope->definedType; + const Type* toType = tok->valueType()->typeScope->definedType; + if (fromType && toType && fromType->isDerivedFrom(toType->name())) + return false; + } + const bool refcast = (tok->valueType()->reference != Reference::None); + if (!refcast && tok->valueType()->pointer == 0) + return false; + if (!refcast && from->valueType()->pointer == 0) + return false; + + if (tok->valueType()->type == ValueType::Type::VOID || from->valueType()->type == ValueType::Type::VOID) + return false; + if (tok->valueType()->pointer == 0 && tok->valueType()->isIntegral()) + // ok: (uintptr_t)ptr; + return false; + if (from->valueType()->pointer == 0 && from->valueType()->isIntegral()) + // ok: (int *)addr; + return false; + + return true; +} //--------------------------------------------------------------------------- // For C++ code, warn if C-style casts are used on pointer types @@ -314,8 +352,11 @@ void CheckOther::warningOldStylePointerCast() tok = scope->bodyStart; for (; tok && tok != scope->bodyEnd; tok = tok->next()) { // Old style pointer casting.. - if (tok->str() != "(") + if (!tok->isCast() || tok->isBinaryOp()) + continue; + if (isDangerousTypeConversion(tok)) continue; + const Token* const errtok = tok; const Token* castTok = tok->next(); while (Token::Match(castTok, "const|volatile|class|struct|union|%type%|::")) { castTok = castTok->next(); @@ -332,7 +373,7 @@ void CheckOther::warningOldStylePointerCast() isRef = true; castTok = castTok->next(); } - if ((!isPtr && !isRef) || !Token::Match(castTok, ") (| %name%|%num%|%bool%|%char%|%str%|&")) + if ((!isPtr && !isRef) || !Token::Match(castTok, ") (| %name%|%bool%|%char%|%str%|&")) continue; if (Token::Match(tok->previous(), "%type%")) @@ -351,7 +392,7 @@ void CheckOther::warningOldStylePointerCast() continue; if (typeTok->tokType() == Token::eType || typeTok->tokType() == Token::eName) - cstyleCastError(tok, isPtr); + cstyleCastError(errtok, isPtr); } } } @@ -367,6 +408,79 @@ void CheckOther::cstyleCastError(const Token *tok, bool isPtr) "which kind of cast is expected.", CWE398, Certainty::normal); } +void CheckOther::warningDangerousTypeCast() +{ + // Only valid on C++ code + if (!mTokenizer->isCPP()) + return; + if (!mSettings->severity.isEnabled(Severity::warning) && !mSettings->isPremiumEnabled("cstyleCast")) + return; + + logChecker("CheckOther::warningDangerousTypeCast"); // warning,c++ + + const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); + for (const Scope * scope : symbolDatabase->functionScopes) { + const Token* tok; + if (scope->function && scope->function->isConstructor()) + tok = scope->classDef; + else + tok = scope->bodyStart; + for (; tok && tok != scope->bodyEnd; tok = tok->next()) { + // Old style pointer casting.. + if (!tok->isCast() || tok->isBinaryOp()) + continue; + + if (isDangerousTypeConversion(tok)) + dangerousTypeCastError(tok, tok->valueType()->pointer > 0); + } + } +} + +void CheckOther::dangerousTypeCastError(const Token *tok, bool isPtr) +{ + //const std::string type = isPtr ? "pointer" : "reference"; + (void)isPtr; + reportError(tok, Severity::warning, "dangerousTypeCast", + "Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast", + CWE398, Certainty::normal); +} + +void CheckOther::warningIntToPointerCast() +{ + if (!mSettings->severity.isEnabled(Severity::portability) && !mSettings->isPremiumEnabled("cstyleCast")) + return; + + logChecker("CheckOther::warningIntToPointerCast"); // portability + + for (const Token* tok = mTokenizer->tokens(); tok; tok = tok->next()) { + // pointer casting.. + if (!tok->isCast()) + continue; + const Token* from = tok->astOperand2() ? tok->astOperand2() : tok->astOperand1(); + if (!from || !from->isNumber()) + continue; + if (!tok->valueType() || tok->valueType()->pointer == 0) + continue; + if (!MathLib::isIntHex(from->str()) && from->getKnownIntValue() != 0) { + std::string format; + if (MathLib::isDec(from->str())) + format = "decimal"; + else if (MathLib::isOct(from->str())) + format = "octal"; + else + continue; + intToPointerCastError(tok, format); + } + } +} + +void CheckOther::intToPointerCastError(const Token *tok, const std::string& format) +{ + reportError(tok, Severity::portability, "intToPointerCast", + "Casting non-zero " + format + " integer literal to pointer.", + CWE398, Certainty::normal); +} + void CheckOther::suspiciousFloatingPointCast() { if (!mSettings->severity.isEnabled(Severity::style) && !mSettings->isPremiumEnabled("suspiciousFloatingPointCast")) @@ -4393,6 +4507,8 @@ void CheckOther::runChecks(const Tokenizer &tokenizer, ErrorLogger *errorLogger) // Checks checkOther.warningOldStylePointerCast(); + checkOther.warningDangerousTypeCast(); + checkOther.warningIntToPointerCast(); checkOther.suspiciousFloatingPointCast(); checkOther.invalidPointerCast(); checkOther.checkCharVariable(); @@ -4459,6 +4575,8 @@ void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *sett c.checkComparisonFunctionIsAlwaysTrueOrFalseError(nullptr, "isless","varName",false); c.checkCastIntToCharAndBackError(nullptr, "func_name"); c.cstyleCastError(nullptr); + c.dangerousTypeCastError(nullptr, true); + c.intToPointerCastError(nullptr, "decimal"); c.suspiciousFloatingPointCastError(nullptr); c.passedByValueError(nullptr, false); c.constVariableError(nullptr, nullptr); diff --git a/lib/checkother.h b/lib/checkother.h index b32461d07a9..db907785e45 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -80,6 +80,12 @@ class CPPCHECKLIB CheckOther : public Check { /** @brief Are there C-style pointer casts in a c++ file? */ void warningOldStylePointerCast(); + /** @brief Dangerous type cast */ + void warningDangerousTypeCast(); + + /** @brief Casting non-hexadecimal integer literal to pointer */ + void warningIntToPointerCast(); + void suspiciousFloatingPointCast(); /** @brief Check for pointer casts to a type with an incompatible binary data representation */ @@ -198,6 +204,8 @@ class CPPCHECKLIB CheckOther : public Check { void clarifyCalculationError(const Token *tok, const std::string &op); void clarifyStatementError(const Token* tok); void cstyleCastError(const Token *tok, bool isPtr = true); + void dangerousTypeCastError(const Token *tok, bool isPtr); + void intToPointerCastError(const Token *tok, const std::string& format); void suspiciousFloatingPointCastError(const Token *tok); void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive, bool toIsInt); void passedByValueError(const Variable* var, bool inconclusive, bool isRangeBasedFor = false); @@ -273,6 +281,7 @@ class CPPCHECKLIB CheckOther : public Check { // warning "- either division by zero or useless condition\n" "- access of moved or forwarded variable.\n" + "- potentially dangerous C style type cast of pointer/reference to object.\n" // performance "- redundant data copying for const variable\n" @@ -281,6 +290,7 @@ class CPPCHECKLIB CheckOther : public Check { // portability "- Passing NULL pointer to function with variable number of arguments leads to UB.\n" + "- Casting non-zero integer literal in decimal or octal format to pointer.\n" // style "- C-style pointer cast in C++ code\n" diff --git a/man/checkers/cstyleCast.md b/man/checkers/cstyleCast.md new file mode 100644 index 00000000000..8bf8d0724d9 --- /dev/null +++ b/man/checkers/cstyleCast.md @@ -0,0 +1,50 @@ + +# cstyleCast + +**Message**: C-style pointer casting
+**Category**: Modernization
+**Severity**: Style
+**Language**: C++, not applicable for C code + +## Description + +Many developers feel that it's best to replace C casts with C++ casts. + +There are several advantages with C++ casts: + * they permit only certain conversions + * they express the intent + * they are easy to identify + +This checker is about C casts that converts to/from a pointer or reference. + +**Note:** More "noisy" warnings exists that warn about all C casts. For instance Clang has +`-Wold-style-cast` and there is also such checking in Misra C++. + +Dangerous conversions are covered by other warnings so this ID `cstyleCast` is primarily about +writing warnings for casts that are currently safe. + +## How to fix + +You can use C++ casts such as `static_cast` to fix these warnings. + +The `dynamic_cast` should rarelly be used to fix these warnings because dangerousTypeCast is +reported when that can be a good idea. + +Before: +```cpp +struct Base{}; +struct Derived: public Base {}; +void foo(Base* base) { + Base *p = (Base*)derived; // <- cstyleCast, cast from derived object to base object is safe now +} +``` + +After: +```cpp +struct Base{}; +struct Derived: public Base {}; +void foo(Base* base) { + Derived *p = static_cast(base); +} +``` +The `static_cast` ensures that there will not be loss of constness in the future. diff --git a/man/checkers/dangerousTypeCast.md b/man/checkers/dangerousTypeCast.md new file mode 100644 index 00000000000..79538103db5 --- /dev/null +++ b/man/checkers/dangerousTypeCast.md @@ -0,0 +1,43 @@ + +# dangerousTypeCast + +**Message**: Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast
+**Category**: Type Safety
+**Severity**: Warning
+**Language**: C++, not applicable for C code + +## Motivation + +C style casts can be dangerous in many ways: + * loss of precision + * loss of sign + * loss of constness + * invalid type conversion + +## Philosophy + +This checker warns about old style C casts that perform type conversions that can be invalid. + +This checker is not about readability. It is about safety. + +## How to fix + +You can use `dynamic_cast` or `static_cast` to fix these warnings. + +Before: +```cpp +struct Base{}; +struct Derived: public Base {}; +void foo(Base* base) { + Derived *p = (Derived*)base; // <- can be invalid +} +``` + +After: +```cpp +struct Base{}; +struct Derived: public Base {}; +void foo(Base* base) { + Derived *p = dynamic_cast(base); +} +``` diff --git a/releasenotes.txt b/releasenotes.txt index 28ed742ea86..792d2dcbc8c 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -25,4 +25,5 @@ Other: - CMake will now unconditionally use Boost.Containers if available. If CMake option `USE_BOOST` is specified it will now bail out when it is not found. - Fix checking a project that contains several project file entries for the same file. - Fixed --file-filter matching of looked up files in provided paths. +- Split up cstyleCast checker; dangerous casts produce portability/warning reports, safe casts produce style reports. - diff --git a/test/cfg/posix.c b/test/cfg/posix.c index a8075b21bcd..f54453a471c 100644 --- a/test/cfg/posix.c +++ b/test/cfg/posix.c @@ -144,6 +144,7 @@ void nullPointer_pthread_attr_setstack(const pthread_attr_t *attr) { (void) pthread_attr_setstack(NULL, NULL, 0); (void) pthread_attr_setstack(attr, NULL, 0); // cppcheck-suppress nullPointer + // cppcheck-suppress intToPointerCast (void) pthread_attr_setstack(NULL, (void*) 1, 0); } diff --git a/test/cfg/windows.cpp b/test/cfg/windows.cpp index c43bbfeb747..5951cb74c07 100644 --- a/test/cfg/windows.cpp +++ b/test/cfg/windows.cpp @@ -825,7 +825,7 @@ void invalidFunctionArg() CloseHandle(hMutex); //Incorrect: 2. parameter to LoadLibraryEx() must be NULL - // cppcheck-suppress [invalidFunctionArg, cstyleCast] + // cppcheck-suppress [invalidFunctionArg, intToPointerCast] HINSTANCE hInstLib = LoadLibraryEx(L"My.dll", HANDLE(1), 0); FreeLibrary(hInstLib); diff --git a/test/testother.cpp b/test/testother.cpp index 0eb6218361e..c126675a84b 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -109,6 +109,7 @@ class TestOther : public TestFixture { TEST_CASE(varScope43); TEST_CASE(oldStylePointerCast); + TEST_CASE(intToPointerCast); TEST_CASE(invalidPointerCast); TEST_CASE(passedByValue); @@ -1901,7 +1902,7 @@ class TestOther : public TestFixture { template void checkOldStylePointerCast_(const char* file, int line, const char (&code)[size], Standards::cppstd_t std = Standards::CPPLatest) { - const Settings settings = settingsBuilder().severity(Severity::style).cpp(std).build(); + const Settings settings = settingsBuilder().severity(Severity::warning).severity(Severity::style).cpp(std).build(); // Tokenize.. SimpleTokenizer tokenizerCpp(settings, *this); @@ -1909,78 +1910,111 @@ class TestOther : public TestFixture { CheckOther checkOtherCpp(&tokenizerCpp, &settings, this); checkOtherCpp.warningOldStylePointerCast(); + checkOtherCpp.warningDangerousTypeCast(); } void oldStylePointerCast() { - checkOldStylePointerCast("class Base;\n" + checkOldStylePointerCast("class Base{};\n" + "class Derived: public Base {};\n" + "void foo(Base* base)\n" + "{\n" + " Derived * d = (Derived *) base;\n" + "}"); + ASSERT_EQUALS("[test.cpp:5:19]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); + + checkOldStylePointerCast("class Base{};\n" + "class Derived: public Base {};\n" + "void foo(Derived* derived)\n" + "{\n" + " Base * b = (Base *) derived;\n" // <- cast from derived to base is safe => cstyleCast + "}"); + ASSERT_EQUALS("[test.cpp:5:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + + checkOldStylePointerCast("void foo(Base* base)\n" + "{\n" + " Derived * d = (Derived *) base;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3:19]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); + + checkOldStylePointerCast("class Base{};\n" + "class Derived: public Base {};\n" + "void foo(Base* base)\n" + "{\n" + " Derived * d = (const Derived *) base;\n" + "}"); + ASSERT_EQUALS("[test.cpp:5:19]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); + + checkOldStylePointerCast("class Base{};\n" + "class Derived: public Base {};\n" "void foo()\n" "{\n" - " Base * b = (Base *) derived;\n" + " Derived * d = (const Derived *) ( new Base() );\n" "}"); - ASSERT_EQUALS("[test.cpp:4:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:5:19]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); - checkOldStylePointerCast("class Base;\n" + checkOldStylePointerCast("class Base{};\n" + "class Derived: public Base {};\n" "void foo()\n" "{\n" - " Base * b = (const Base *) derived;\n" + " Derived * d = (const Derived *) new Base();\n" "}"); - ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:5:19]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); - checkOldStylePointerCast("class Base;\n" + checkOldStylePointerCast("class Base{};\n" "void foo()\n" "{\n" - " Base * b = (const Base * const) derived;\n" + " Base * b = (Base *) new short[10];\n" "}"); - ASSERT_EQUALS("[test.cpp:4:23]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:16]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); checkOldStylePointerCast("class Base;\n" "void foo()\n" "{\n" " Base * b = (volatile Base *) derived;\n" "}"); - ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); checkOldStylePointerCast("class Base;\n" "void foo()\n" "{\n" " Base * b = (volatile Base * const) derived;\n" "}"); - ASSERT_EQUALS("[test.cpp:4:26]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); checkOldStylePointerCast("class Base;\n" "void foo()\n" "{\n" " Base * b = (const volatile Base *) derived;\n" "}"); - ASSERT_EQUALS("[test.cpp:4:23]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); checkOldStylePointerCast("class Base;\n" "void foo()\n" "{\n" " Base * b = (const volatile Base * const) derived;\n" "}"); - ASSERT_EQUALS("[test.cpp:4:32]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); checkOldStylePointerCast("class Base;\n" "void foo()\n" "{\n" " Base * b = (const Base *) ( new Derived() );\n" "}"); - ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); checkOldStylePointerCast("class Base;\n" "void foo()\n" "{\n" " Base * b = (const Base *) new Derived();\n" "}"); - ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); checkOldStylePointerCast("class Base;\n" "void foo()\n" "{\n" " Base * b = (const Base *) new short[10];\n" "}"); - ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:16]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); checkOldStylePointerCast("class B;\n" "class A\n" @@ -1997,11 +2031,17 @@ class TestOther : public TestFixture { ASSERT_EQUALS("", errout_str()); // #3630 - checkOldStylePointerCast("class SomeType;\n" + checkOldStylePointerCast("class SomeType{};\n" "class X : public Base {\n" - " X() : Base((SomeType*)7) {}\n" + " X() : Base((SomeType*)7) {}\n" // <- intToPointerCast "};"); - ASSERT_EQUALS("[test.cpp:3:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("", errout_str()); + + checkOldStylePointerCast("class SomeType{};\n" + "class X : public Base {\n" + " X() : Base((SomeType*)0x7000) {}\n" // <- it's common in embedded code to cast address + "};"); + ASSERT_EQUALS("", errout_str()); checkOldStylePointerCast("class SomeType;\n" "class X : public Base {\n" @@ -2032,6 +2072,7 @@ class TestOther : public TestFixture { " std::vector v;\n" " v.push_back((Base*)new Derived);\n" "}"); + // FIXME write a dangerousTypeCast warning instead ASSERT_EQUALS("[test.cpp:5:15]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); // #7709 @@ -2055,22 +2096,22 @@ class TestOther : public TestFixture { " TT* tt = (TT*)i;\n" " TT2* tt2 = (TT2*)i;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:10:13]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:11:15]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:12:22]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:13:13]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:14:21]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:15:15]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:16:16]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:17:16]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:18:15]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:19:17]: (style) C-style pointer casting [cstyleCast]\n", + ASSERT_EQUALS("[test.cpp:10:12]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:11:14]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:12:21]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:13:12]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:14:20]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:15:15]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:16:16]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:17:16]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:18:14]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:19:16]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); // #8649 checkOldStylePointerCast("struct S {};\n" "void g(S*& s);\n" - "void f(int i) {\n" + "void f(uintptr_t i) {\n" " g((S*&)i);\n" " S*& r = (S*&)i;\n" "}\n"); @@ -2078,6 +2119,17 @@ class TestOther : public TestFixture { "[test.cpp:5:13]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + checkOldStylePointerCast("struct S {};\n" + "void g(S*& s);\n" + "void f(uint8_t i) {\n" + " g((S*&)i);\n" + " S*& r = (S*&)i;\n" + "}\n"); + // TODO: these conversions are dangerous, but it's a different issue not covered by cstyleCast. A separate checker can be added which is executed for both C and C++ code. + // clang says: 1.cpp:5:18: warning: cast to 'unsigned char *' from smaller integer type 'uint8_t' (aka 'unsigned char') [-Wint-to-pointer-cast] + ASSERT_EQUALS("[test.cpp:4:7]: (style) C-style pointer casting [cstyleCast]\n" + "[test.cpp:5:13]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + // #10823 checkOldStylePointerCast("void f(void* p) {\n" " auto h = reinterpret_cast(p);\n" @@ -2085,12 +2137,14 @@ class TestOther : public TestFixture { ASSERT_EQUALS("", errout_str()); // #5210 - checkOldStylePointerCast("void f(void* v1, void* v2) {\n" - " T** p1 = (T**)v1;\n" - " T*** p2 = (T***)v2;\n" + checkOldStylePointerCast("class Base {};\n" + "class Derived: public Base {};\n" + "void f(Base** b1, Base*** b2) {\n" + " Derived** p1 = (Derived**)b1;\n" + " Derived*** p2 = (Derived***)b2;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:2:14]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:3:15]: (style) C-style pointer casting [cstyleCast]\n", + ASSERT_EQUALS("[test.cpp:4:20]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" + "[test.cpp:5:21]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); // #12446 @@ -2105,16 +2159,16 @@ class TestOther : public TestFixture { " auto pv = (std::vector*)(p);\n" "}\n"); ASSERT_EQUALS("[test.cpp:7:15]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:8:16]: (style) C-style pointer casting [cstyleCast]\n" - "[test.cpp:9:15]: (style) C-style pointer casting [cstyleCast]\n", - errout_str()); + "[test.cpp:8:15]: (style) C-style pointer casting [cstyleCast]\n" + "[test.cpp:9:15]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); // #12447 - checkOldStylePointerCast("void f(const int& i) {\n" - " int& r = (int&)i;\n" - " r = 0;\n" + checkOldStylePointerCast("class Base {};\n" + "class Derived: public Base {};\n" + "void f(const Base& base) {\n" + " d = (const Derived&)base;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:2:12]: (style) C-style reference casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:7]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); // #11430 checkOldStylePointerCast("struct B {\n" @@ -2125,11 +2179,40 @@ class TestOther : public TestFixture { "}\n" "bool g(B& b) {\n" " using float_ptr = float*;\n" - " return N::f(float_ptr(b.data()));\n" + " return N::f(float_ptr(b.data()));\n" // <- the cast is safe "}\n"); ASSERT_EQUALS("[test.cpp:9:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); } +#define checkIntToPointerCast(...) checkIntToPointerCast_(__FILE__, __LINE__, __VA_ARGS__) + template + void checkIntToPointerCast_(const char* file, int line, const char (&code)[size]) { + + const Settings settings = settingsBuilder().severity(Severity::portability).build(); + + // Tokenize.. + SimpleTokenizer tokenizerCpp(settings, *this); + ASSERT_LOC(tokenizerCpp.tokenize(code), file, line); + + CheckOther checkOtherCpp(&tokenizerCpp, &settings, this); + checkOtherCpp.warningIntToPointerCast(); + } + + void intToPointerCast() { + // #3630 + checkIntToPointerCast("uint8_t* ptr = (uint8_t*)7;"); + ASSERT_EQUALS("[test.cpp:1:16]: (portability) Casting non-zero decimal integer literal to pointer. [intToPointerCast]\n", errout_str()); + + checkIntToPointerCast("void* ptr = (void*)7;"); + ASSERT_EQUALS("[test.cpp:1:13]: (portability) Casting non-zero decimal integer literal to pointer. [intToPointerCast]\n", errout_str()); + + checkIntToPointerCast("uint8_t* ptr = (uint8_t*)0;"); + ASSERT_EQUALS("", errout_str()); + + checkIntToPointerCast("uint8_t* ptr = (uint8_t*)0x7000;"); // <- it's common in embedded code to cast address + ASSERT_EQUALS("", errout_str()); + } + #define checkInvalidPointerCast(...) checkInvalidPointerCast_(__FILE__, __LINE__, __VA_ARGS__) template void checkInvalidPointerCast_(const char* file, int line, const char (&code)[size], bool portability = true, bool inconclusive = false) { @@ -3113,7 +3196,7 @@ class TestOther : public TestFixture { " x.dostuff();\n" " const U& y = (const U&)(x);\n" "}"); - ASSERT_EQUALS("[test.cpp:4:19]: (style) C-style reference casting [cstyleCast]\n" + ASSERT_EQUALS("[test.cpp:4:18]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" "[test.cpp:2:11]: (style) Parameter 'x' can be declared as reference to const [constParameterReference]\n", errout_str()); check("struct T : public U { void dostuff() const {}};\n" @@ -3122,13 +3205,13 @@ class TestOther : public TestFixture { " U& y = (U&)(x);\n" " y.mutate();\n" // to avoid warnings that y can be const "}"); - ASSERT_EQUALS("[test.cpp:4:12]: (style) C-style reference casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:12]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); check("struct T : public U { void dostuff() const {}};\n" "void a(T& x) {\n" " x.dostuff();\n" " const U& y = (typename const U&)(x);\n" "}"); - ASSERT_EQUALS("[test.cpp:4:0]: (style) C-style reference casting [cstyleCast]\n" + ASSERT_EQUALS("[test.cpp:4:18]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" "[test.cpp:2:11]: (style) Parameter 'x' can be declared as reference to const [constParameterReference]\n", errout_str()); check("struct T : public U { void dostuff() const {}};\n" @@ -3137,14 +3220,14 @@ class TestOther : public TestFixture { " U& y = (typename U&)(x);\n" " y.mutate();\n" // to avoid warnings that y can be const "}"); - ASSERT_EQUALS("[test.cpp:4:12]: (style) C-style reference casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:12]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); check("struct T : public U { void dostuff() const {}};\n" "void a(T& x) {\n" " x.dostuff();\n" " U* y = (U*)(&x);\n" " y->mutate();\n" // to avoid warnings that y can be const "}"); - ASSERT_EQUALS("[test.cpp:4:12]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:4:12]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); check("struct C { void f() const; };\n" // #9875 - crash "\n" @@ -3589,7 +3672,7 @@ class TestOther : public TestFixture { "void g(A* a) {\n" " const B* b = (const B*)a;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:10:19]: (style) C-style pointer casting [cstyleCast]\n" + ASSERT_EQUALS("[test.cpp:10:18]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n" "[test.cpp:6:11]: (style) Parameter 'a' can be declared as pointer to const [constParameterPointer]\n" "[test.cpp:9:11]: (style) Parameter 'a' can be declared as pointer to const [constParameterPointer]\n", errout_str()); @@ -10176,7 +10259,7 @@ class TestOther : public TestFixture { " *reg = 12;\n" " *reg = 34;\n" "}"); - ASSERT_EQUALS("[test.cpp:2:25]: style: C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("", errout_str()); check("void f(std::map& m, int key, int value) {\n" // #6379 " m[key] = value;\n" @@ -12661,7 +12744,7 @@ class TestOther : public TestFixture { "int f(S s) {\n" " return &s.i - (int*)&s;\n" "}\n"); - ASSERT_EQUALS("[test.cpp:3:19]: (style) C-style pointer casting [cstyleCast]\n", errout_str()); + ASSERT_EQUALS("[test.cpp:3:19]: (warning) Potentially invalid type conversion in old-style C cast, clarify/fix with C++ cast [dangerousTypeCast]\n", errout_str()); check("struct S { int i; };\n" "int f(S s1, S s2) {\n"