From 442aeb40c4ed4f729e6f2706a5af392c8fb7f6f3 Mon Sep 17 00:00:00 2001 From: chrchr Date: Fri, 16 Feb 2024 11:44:10 +0100 Subject: [PATCH 1/6] Fix #12446 FN cstyleCast with scope operator --- lib/checkother.cpp | 10 +++++++++- test/testother.cpp | 14 ++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index af2ca4ea432..c38ad1df22c 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -313,8 +313,16 @@ void CheckOther::warningOldStylePointerCast() tok = scope->bodyStart; for (; tok && tok != scope->bodyEnd; tok = tok->next()) { // Old style pointer casting.. - if (!Token::Match(tok, "( const|volatile| const|volatile|class|struct| %type% * *| *| const|&| ) (| %name%|%num%|%bool%|%char%|%str%|&")) + const Token* castTok = tok->next(); + while (Token::Match(castTok, "const|volatile|class|struct|union|%type%|::")) + castTok = castTok->next(); + if (!Token::simpleMatch(castTok, "*")) continue; + while (Token::Match(castTok, "*|const|&")) + castTok = castTok->next(); + if (!Token::Match(castTok, ") (| %name%|%num%|%bool%|%char%|%str%|&")) + continue; + if (Token::Match(tok->previous(), "%type%")) continue; diff --git a/test/testother.cpp b/test/testother.cpp index 2c949a47877..2904c480234 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -1895,6 +1895,20 @@ class TestOther : public TestFixture { ASSERT_EQUALS("[test.cpp:2]: (style) C-style pointer casting\n" "[test.cpp:3]: (style) C-style pointer casting\n", errout.str()); + + // #12446 + checkOldStylePointerCast("namespace N { struct S {}; }\n" + "union U {\n" + " int i;\n" + " char c[4];\n" + "};\n" + "void f(void* p) {\n" + " auto ps = (N::S*)p;\n" + " auto pu = (union U*)p;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:7]: (style) C-style pointer casting\n" + "[test.cpp:8]: (style) C-style pointer casting\n", + errout.str()); } #define checkInvalidPointerCast(...) checkInvalidPointerCast_(__FILE__, __LINE__, __VA_ARGS__) From 40336c42249e6c3675123d7d27cb337c9d3c74e9 Mon Sep 17 00:00:00 2001 From: chrchr Date: Fri, 16 Feb 2024 11:50:57 +0100 Subject: [PATCH 2/6] Fix --- lib/checkother.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index c38ad1df22c..e803adb59f8 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -313,10 +313,12 @@ void CheckOther::warningOldStylePointerCast() tok = scope->bodyStart; for (; tok && tok != scope->bodyEnd; tok = tok->next()) { // Old style pointer casting.. + if (tok->str() != "(") + continue; const Token* castTok = tok->next(); while (Token::Match(castTok, "const|volatile|class|struct|union|%type%|::")) castTok = castTok->next(); - if (!Token::simpleMatch(castTok, "*")) + if (castTok == tok->next() || !Token::simpleMatch(castTok, "*")) continue; while (Token::Match(castTok, "*|const|&")) castTok = castTok->next(); @@ -327,7 +329,7 @@ void CheckOther::warningOldStylePointerCast() continue; // skip first "const" in "const Type* const" - while (Token::Match(tok->next(), "const|volatile|class|struct")) + while (Token::Match(tok->next(), "const|volatile|class|struct|union")) tok = tok->next(); const Token* typeTok = tok->next(); // skip second "const" in "const Type* const" From 8a0acf80003c889bbf4ab277e5c472cf7c67611b Mon Sep 17 00:00:00 2001 From: chrchr Date: Fri, 16 Feb 2024 12:03:25 +0100 Subject: [PATCH 3/6] Skip template arguments --- lib/checkother.cpp | 5 ++++- test/testother.cpp | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index e803adb59f8..8eaea623500 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -316,8 +316,11 @@ void CheckOther::warningOldStylePointerCast() if (tok->str() != "(") continue; const Token* castTok = tok->next(); - while (Token::Match(castTok, "const|volatile|class|struct|union|%type%|::")) + while (Token::Match(castTok, "const|volatile|class|struct|union|%type%|::")) { castTok = castTok->next(); + if (Token::simpleMatch(castTok, "<") && castTok->link()) + castTok = castTok->link()->next(); + } if (castTok == tok->next() || !Token::simpleMatch(castTok, "*")) continue; while (Token::Match(castTok, "*|const|&")) diff --git a/test/testother.cpp b/test/testother.cpp index 2904c480234..3f938af2e85 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -1905,9 +1905,11 @@ class TestOther : public TestFixture { "void f(void* p) {\n" " auto ps = (N::S*)p;\n" " auto pu = (union U*)p;\n" + " auto pv = (std::vector*)(p);\n" "}\n"); ASSERT_EQUALS("[test.cpp:7]: (style) C-style pointer casting\n" - "[test.cpp:8]: (style) C-style pointer casting\n", + "[test.cpp:8]: (style) C-style pointer casting\n" + "[test.cpp:9]: (style) C-style pointer casting\n", errout.str()); } From 548fd546cc6b21c2f6cdce5e32e262f3040f6683 Mon Sep 17 00:00:00 2001 From: chrchr Date: Mon, 19 Feb 2024 11:52:27 +0100 Subject: [PATCH 4/6] Fix #12447 FN cstyleCast when casting to reference --- lib/checkother.cpp | 23 +++++++++++++++-------- lib/checkother.h | 2 +- test/testother.cpp | 7 +++++++ 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 8eaea623500..81a55471e7b 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -321,11 +321,17 @@ void CheckOther::warningOldStylePointerCast() if (Token::simpleMatch(castTok, "<") && castTok->link()) castTok = castTok->link()->next(); } - if (castTok == tok->next() || !Token::simpleMatch(castTok, "*")) - continue; - while (Token::Match(castTok, "*|const|&")) + if (castTok == tok->next()) + continue; + bool isPtr = false, isRef = false; + while (Token::Match(castTok, "*|const|&")) { + if (castTok->str() == "*") + isPtr = true; + else if (castTok->str() == "&") + isRef = true; castTok = castTok->next(); - if (!Token::Match(castTok, ") (| %name%|%num%|%bool%|%char%|%str%|&")) + } + if ((!isPtr && !isRef) || !Token::Match(castTok, ") (| %name%|%num%|%bool%|%char%|%str%|&")) continue; if (Token::Match(tok->previous(), "%type%")) @@ -344,16 +350,17 @@ void CheckOther::warningOldStylePointerCast() continue; if (typeTok->tokType() == Token::eType || typeTok->tokType() == Token::eName) - cstyleCastError(tok); + cstyleCastError(tok, isPtr); } } } -void CheckOther::cstyleCastError(const Token *tok) +void CheckOther::cstyleCastError(const Token *tok, bool isPtr) { + const std::string type = isPtr ? "pointer" : "reference"; reportError(tok, Severity::style, "cstyleCast", - "C-style pointer casting\n" - "C-style pointer casting detected. C++ offers four different kinds of casts as replacements: " + "C-style " + type + " casting\n" + "C-style " + type + " casting detected. C++ offers four different kinds of casts as replacements: " "static_cast, const_cast, dynamic_cast and reinterpret_cast. A C-style cast could evaluate to " "any of those automatically, thus it is considered safer if the programmer explicitly states " "which kind of cast is expected.", CWE398, Certainty::normal); diff --git a/lib/checkother.h b/lib/checkother.h index 5f52e3092d9..1b7d080e7e5 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -239,7 +239,7 @@ class CPPCHECKLIB CheckOther : public Check { void checkCastIntToCharAndBackError(const Token *tok, const std::string &strFunctionName); void clarifyCalculationError(const Token *tok, const std::string &op); void clarifyStatementError(const Token* tok); - void cstyleCastError(const Token *tok); + void cstyleCastError(const Token *tok, bool isPtr = true); 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); void constVariableError(const Variable *var, const Function *function); diff --git a/test/testother.cpp b/test/testother.cpp index 3f938af2e85..0e00beefdb9 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -1911,6 +1911,13 @@ class TestOther : public TestFixture { "[test.cpp:8]: (style) C-style pointer casting\n" "[test.cpp:9]: (style) C-style pointer casting\n", errout.str()); + + // #12447 + checkOldStylePointerCast("void f(const int& i) {\n" + " int& r = (int&)i;\n" + " r = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2]: (style) C-style reference casting\n", errout.str()); } #define checkInvalidPointerCast(...) checkInvalidPointerCast_(__FILE__, __LINE__, __VA_ARGS__) From 02463cdf270c0d8a87a82c6c7cebcb872c0e97b0 Mon Sep 17 00:00:00 2001 From: chrchr Date: Mon, 19 Feb 2024 12:18:23 +0100 Subject: [PATCH 5/6] Fix test --- test/testother.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/testother.cpp b/test/testother.cpp index 0e00beefdb9..ca846987cab 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2891,7 +2891,9 @@ class TestOther : public TestFixture { " x.dostuff();\n" " const U& y = (const U&)(x);\n" "}"); - ASSERT_EQUALS("[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (style) C-style reference casting\n", + "[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", + errout.str()); check("struct T : public U { void dostuff() const {}};\n" "void a(T& x) {\n" " x.dostuff();\n" From b4ea5f76d6681142486987db95e4cc80d3f94da4 Mon Sep 17 00:00:00 2001 From: chrchr Date: Mon, 19 Feb 2024 12:22:00 +0100 Subject: [PATCH 6/6] Fix --- test/testother.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/testother.cpp b/test/testother.cpp index ca846987cab..8b316536fda 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2891,7 +2891,7 @@ class TestOther : public TestFixture { " x.dostuff();\n" " const U& y = (const U&)(x);\n" "}"); - ASSERT_EQUALS("[test.cpp:4]: (style) C-style reference casting\n", + ASSERT_EQUALS("[test.cpp:4]: (style) C-style reference casting\n" "[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", errout.str()); check("struct T : public U { void dostuff() const {}};\n" @@ -2900,20 +2900,22 @@ class TestOther : public TestFixture { " U& y = (U&)(x);\n" " y.mutate();\n" // to avoid warnings that y can be const "}"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (style) C-style reference casting\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:2]: (style) Parameter 'x' can be declared as reference to const\n", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (style) C-style reference casting\n" + "[test.cpp:2]: (style) Parameter 'x' can be declared as reference to const\n", + errout.str()); check("struct T : public U { void dostuff() const {}};\n" "void a(T& x) {\n" " x.dostuff();\n" " U& y = (typename U&)(x);\n" " y.mutate();\n" // to avoid warnings that y can be const "}"); - ASSERT_EQUALS("", errout.str()); + ASSERT_EQUALS("[test.cpp:4]: (style) C-style reference casting\n", errout.str()); check("struct T : public U { void dostuff() const {}};\n" "void a(T& x) {\n" " x.dostuff();\n"