Skip to content

Commit 1a9b11f

Browse files
danmardanmar
committed
tweak checker to match documentation
1 parent dff9a37 commit 1a9b11f

3 files changed

Lines changed: 106 additions & 113 deletions

File tree

‎lib/checkother.cpp‎

Lines changed: 38 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -296,10 +296,6 @@ void CheckOther::suspiciousSemicolonError(const Token* tok)
296296
//---------------------------------------------------------------------------
297297
void CheckOther::warningOldStylePointerCast()
298298
{
299-
// Only valid on C++ code
300-
if (!mTokenizer->isCPP())
301-
return;
302-
303299
if (!mSettings->severity.isEnabled(Severity::style) && !mSettings->isPremiumEnabled("cstyleCast"))
304300
return;
305301

@@ -316,42 +312,42 @@ void CheckOther::warningOldStylePointerCast()
316312
// Old style pointer casting..
317313
if (!tok->isCast() || tok->isBinaryOp())
318314
continue;
319-
const Token* castTok = tok->next();
320-
while (Token::Match(castTok, "const|volatile|class|struct|union|%type%|::")) {
321-
castTok = castTok->next();
322-
if (Token::simpleMatch(castTok, "<") && castTok->link())
323-
castTok = castTok->link()->next();
324-
}
325-
if (castTok == tok->next())
315+
const Token* from = tok->astOperand1();
316+
if (!from)
326317
continue;
327-
bool isPtr = false, isRef = false;
328-
while (Token::Match(castTok, "*|const|&")) {
329-
if (castTok->str() == "*")
330-
isPtr = true;
331-
else if (castTok->str() == "&")
332-
isRef = true;
333-
castTok = castTok->next();
334-
}
335-
if ((!isPtr && !isRef) || !Token::Match(castTok, ") (| %name%|%num%|%bool%|%char%|%str%|&"))
318+
if (!tok->valueType() || !from->valueType())
336319
continue;
337-
338-
if (Token::Match(tok->previous(), "%type%"))
320+
if (tok->valueType()->type == from->valueType()->type &&
321+
tok->valueType()->typeScope == from->valueType()->typeScope)
339322
continue;
323+
const bool refcast = (tok->valueType()->reference != Reference::None);
324+
if (!refcast && tok->valueType()->pointer == 0)
325+
continue;
326+
if (!refcast && from->valueType()->pointer == 0) {
327+
// casting non-zero integer literal (decimal/octal) to pointer is suspicious
328+
if (mSettings->severity.isEnabled(Severity::portability) &&
329+
tok->valueType()->pointer > 0 &&
330+
from->isNumber() &&
331+
!MathLib::isIntHex(from->str()) &&
332+
from->getKnownIntValue() != 0)
333+
intToPointerCastError(tok);
334+
continue;
335+
}
340336

341-
// skip first "const" in "const Type* const"
342-
while (Token::Match(tok->next(), "const|volatile|class|struct|union"))
343-
tok = tok->next();
344-
const Token* typeTok = tok->next();
345-
// skip second "const" in "const Type* const"
346-
if (tok->strAt(3) == "const")
347-
tok = tok->next();
337+
// Use C++ cast: Only valid on C++ code
338+
if (!mTokenizer->isCPP())
339+
continue;
348340

349-
const Token *p = tok->tokAt(4);
350-
if (p->hasKnownIntValue() && p->getKnownIntValue()==0) // Casting nullpointers is safe
341+
if (tok->valueType()->type == ValueType::Type::VOID || from->valueType()->type == ValueType::Type::VOID)
342+
continue;
343+
if (tok->valueType()->pointer == 0 && tok->valueType()->isIntegral())
344+
// ok: (uintptr_t)ptr;
345+
continue;
346+
if (from->valueType()->pointer == 0 && from->valueType()->isIntegral())
347+
// ok: (int *)addr;
351348
continue;
352349

353-
if (typeTok->tokType() == Token::eType || typeTok->tokType() == Token::eName)
354-
cstyleCastError(tok, isPtr);
350+
cstyleCastError(tok, tok->valueType()->pointer > 0);
355351
}
356352
}
357353
}
@@ -367,6 +363,14 @@ void CheckOther::cstyleCastError(const Token *tok, bool isPtr)
367363
"which kind of cast is expected.", CWE398, Certainty::normal);
368364
}
369365

366+
367+
void CheckOther::intToPointerCastError(const Token *tok)
368+
{
369+
reportError(tok, Severity::portability, "intToPointerCast",
370+
"Casting non-zero integer literal in decimal or octal format to pointer.",
371+
CWE398, Certainty::normal);
372+
}
373+
370374
void CheckOther::suspiciousFloatingPointCast()
371375
{
372376
if (!mSettings->severity.isEnabled(Severity::style) && !mSettings->isPremiumEnabled("suspiciousFloatingPointCast"))
@@ -4459,6 +4463,7 @@ void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *sett
44594463
c.checkComparisonFunctionIsAlwaysTrueOrFalseError(nullptr, "isless","varName",false);
44604464
c.checkCastIntToCharAndBackError(nullptr, "func_name");
44614465
c.cstyleCastError(nullptr);
4466+
c.intToPointerCastError(nullptr);
44624467
c.suspiciousFloatingPointCastError(nullptr);
44634468
c.passedByValueError(nullptr, false);
44644469
c.constVariableError(nullptr, nullptr);

‎lib/checkother.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ class CPPCHECKLIB CheckOther : public Check {
198198
void clarifyCalculationError(const Token *tok, const std::string &op);
199199
void clarifyStatementError(const Token* tok);
200200
void cstyleCastError(const Token *tok, bool isPtr = true);
201+
void intToPointerCastError(const Token *tok);
201202
void suspiciousFloatingPointCastError(const Token *tok);
202203
void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive, bool toIsInt);
203204
void passedByValueError(const Variable* var, bool inconclusive, bool isRangeBasedFor = false);
@@ -281,6 +282,7 @@ class CPPCHECKLIB CheckOther : public Check {
281282

282283
// portability
283284
"- Passing NULL pointer to function with variable number of arguments leads to UB.\n"
285+
"- Casting non-zero integer literal in decimal or octal format to pointer.\n"
284286

285287
// style
286288
"- C-style pointer cast in C++ code\n"

‎test/testother.cpp‎

Lines changed: 66 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1901,7 +1901,7 @@ class TestOther : public TestFixture {
19011901
template<size_t size>
19021902
void checkOldStylePointerCast_(const char* file, int line, const char (&code)[size], Standards::cppstd_t std = Standards::CPPLatest) {
19031903

1904-
const Settings settings = settingsBuilder().severity(Severity::style).cpp(std).build();
1904+
const Settings settings = settingsBuilder().severity(Severity::style).severity(Severity::portability).cpp(std).build();
19051905

19061906
// Tokenize..
19071907
SimpleTokenizer tokenizerCpp(settings, *this);
@@ -1912,75 +1912,44 @@ class TestOther : public TestFixture {
19121912
}
19131913

19141914
void oldStylePointerCast() {
1915-
checkOldStylePointerCast("class Base;\n"
1916-
"void foo()\n"
1915+
checkOldStylePointerCast("class Base{};\n"
1916+
"class Derived: public Base {};\n"
1917+
"void foo(Derived* derived)\n"
19171918
"{\n"
19181919
" Base * b = (Base *) derived;\n"
19191920
"}");
1920-
ASSERT_EQUALS("[test.cpp:4:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1921+
ASSERT_EQUALS("[test.cpp:5:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
19211922

1922-
checkOldStylePointerCast("class Base;\n"
1923-
"void foo()\n"
1923+
checkOldStylePointerCast("class Base{};\n"
1924+
"class Derived: public Base {};\n"
1925+
"void foo(Derived* derived)\n"
19241926
"{\n"
19251927
" Base * b = (const Base *) derived;\n"
19261928
"}");
1927-
ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1928-
1929-
checkOldStylePointerCast("class Base;\n"
1930-
"void foo()\n"
1931-
"{\n"
1932-
" Base * b = (const Base * const) derived;\n"
1933-
"}");
1934-
ASSERT_EQUALS("[test.cpp:4:23]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1935-
1936-
checkOldStylePointerCast("class Base;\n"
1937-
"void foo()\n"
1938-
"{\n"
1939-
" Base * b = (volatile Base *) derived;\n"
1940-
"}");
1941-
ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1942-
1943-
checkOldStylePointerCast("class Base;\n"
1944-
"void foo()\n"
1945-
"{\n"
1946-
" Base * b = (volatile Base * const) derived;\n"
1947-
"}");
1948-
ASSERT_EQUALS("[test.cpp:4:26]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1949-
1950-
checkOldStylePointerCast("class Base;\n"
1951-
"void foo()\n"
1952-
"{\n"
1953-
" Base * b = (const volatile Base *) derived;\n"
1954-
"}");
1955-
ASSERT_EQUALS("[test.cpp:4:23]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1929+
ASSERT_EQUALS("[test.cpp:5:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
19561930

1957-
checkOldStylePointerCast("class Base;\n"
1958-
"void foo()\n"
1959-
"{\n"
1960-
" Base * b = (const volatile Base * const) derived;\n"
1961-
"}");
1962-
ASSERT_EQUALS("[test.cpp:4:32]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1963-
1964-
checkOldStylePointerCast("class Base;\n"
1931+
checkOldStylePointerCast("class Base{};\n"
1932+
"class Derived: public Base {};\n"
19651933
"void foo()\n"
19661934
"{\n"
1967-
" Base * b = (const Base *) ( new Derived() );\n"
1935+
" Base * b = (Base *) ( new Derived() );\n"
19681936
"}");
1969-
ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1937+
ASSERT_EQUALS("[test.cpp:5:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
19701938

1971-
checkOldStylePointerCast("class Base;\n"
1939+
checkOldStylePointerCast("class Base{};\n"
1940+
"class Derived: public Base {};\n"
19721941
"void foo()\n"
19731942
"{\n"
1974-
" Base * b = (const Base *) new Derived();\n"
1943+
" Base * b = (Base *) new Derived();\n"
19751944
"}");
1976-
ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1945+
ASSERT_EQUALS("[test.cpp:5:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
19771946

1978-
checkOldStylePointerCast("class Base;\n"
1947+
checkOldStylePointerCast("class Base{};\n"
19791948
"void foo()\n"
19801949
"{\n"
1981-
" Base * b = (const Base *) new short[10];\n"
1950+
" Base * b = (Base *) new short[10];\n"
19821951
"}");
1983-
ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1952+
ASSERT_EQUALS("[test.cpp:4:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
19841953

19851954
checkOldStylePointerCast("class B;\n"
19861955
"class A\n"
@@ -1997,17 +1966,23 @@ class TestOther : public TestFixture {
19971966
ASSERT_EQUALS("", errout_str());
19981967

19991968
// #3630
2000-
checkOldStylePointerCast("class SomeType;\n"
1969+
checkOldStylePointerCast("class SomeType{};\n"
20011970
"class X : public Base {\n"
20021971
" X() : Base((SomeType*)7) {}\n"
20031972
"};");
2004-
ASSERT_EQUALS("[test.cpp:3:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1973+
ASSERT_EQUALS("[test.cpp:3:16]: (portability) Casting non-zero integer literal in decimal or octal format to pointer. [intToPointerCast]\n", errout_str());
1974+
1975+
checkOldStylePointerCast("class SomeType{};\n"
1976+
"class X : public Base {\n"
1977+
" X() : Base((SomeType*)0x7000) {}\n" // <- it's common in embedded code to cast address
1978+
"};");
1979+
ASSERT_EQUALS("", errout_str());
20051980

20061981
checkOldStylePointerCast("class SomeType;\n"
20071982
"class X : public Base {\n"
20081983
" X() : Base((SomeType*)var) {}\n"
20091984
"};");
2010-
ASSERT_EQUALS("[test.cpp:3:16]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
1985+
ASSERT_EQUALS("", errout_str());
20111986

20121987
checkOldStylePointerCast("class SomeType;\n"
20131988
"class X : public Base {\n"
@@ -2032,7 +2007,7 @@ class TestOther : public TestFixture {
20322007
" std::vector<Base*> v;\n"
20332008
" v.push_back((Base*)new Derived);\n"
20342009
"}");
2035-
ASSERT_EQUALS("[test.cpp:5:15]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
2010+
// FIXME ASSERT_EQUALS("[test.cpp:5:15]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
20362011

20372012
// #7709
20382013
checkOldStylePointerCast("typedef struct S S;\n"
@@ -2055,27 +2030,38 @@ class TestOther : public TestFixture {
20552030
" TT* tt = (TT*)i;\n"
20562031
" TT2* tt2 = (TT2*)i;\n"
20572032
"}\n");
2058-
ASSERT_EQUALS("[test.cpp:10:13]: (style) C-style pointer casting [cstyleCast]\n"
2059-
"[test.cpp:11:15]: (style) C-style pointer casting [cstyleCast]\n"
2060-
"[test.cpp:12:22]: (style) C-style pointer casting [cstyleCast]\n"
2061-
"[test.cpp:13:13]: (style) C-style pointer casting [cstyleCast]\n"
2062-
"[test.cpp:14:21]: (style) C-style pointer casting [cstyleCast]\n"
2033+
ASSERT_EQUALS("[test.cpp:10:12]: (style) C-style pointer casting [cstyleCast]\n"
2034+
"[test.cpp:11:14]: (style) C-style pointer casting [cstyleCast]\n"
2035+
"[test.cpp:12:21]: (style) C-style pointer casting [cstyleCast]\n"
2036+
"[test.cpp:13:12]: (style) C-style pointer casting [cstyleCast]\n"
2037+
"[test.cpp:14:20]: (style) C-style pointer casting [cstyleCast]\n"
20632038
"[test.cpp:15:15]: (style) C-style pointer casting [cstyleCast]\n"
20642039
"[test.cpp:16:16]: (style) C-style pointer casting [cstyleCast]\n"
20652040
"[test.cpp:17:16]: (style) C-style pointer casting [cstyleCast]\n"
2066-
"[test.cpp:18:15]: (style) C-style pointer casting [cstyleCast]\n"
2067-
"[test.cpp:19:17]: (style) C-style pointer casting [cstyleCast]\n",
2041+
"[test.cpp:18:14]: (style) C-style pointer casting [cstyleCast]\n"
2042+
"[test.cpp:19:16]: (style) C-style pointer casting [cstyleCast]\n",
20682043
errout_str());
20692044

20702045
// #8649
20712046
checkOldStylePointerCast("struct S {};\n"
20722047
"void g(S*& s);\n"
2073-
"void f(int i) {\n"
2048+
"void f(uintptr_t i) {\n"
20742049
" g((S*&)i);\n"
20752050
" S*& r = (S*&)i;\n"
20762051
"}\n");
2077-
ASSERT_EQUALS("[test.cpp:4:7]: (style) C-style pointer casting [cstyleCast]\n"
2078-
"[test.cpp:5:13]: (style) C-style pointer casting [cstyleCast]\n",
2052+
ASSERT_EQUALS("",
2053+
errout_str());
2054+
2055+
checkOldStylePointerCast("struct S {};\n"
2056+
"void g(S*& s);\n"
2057+
"void f(uint8_t i) {\n"
2058+
" g((S*&)i);\n"
2059+
" S*& r = (S*&)i;\n"
2060+
"}\n");
2061+
// TODO: these conversions are dangerous, but a separate checker would make sense which
2062+
// also checks C code.
2063+
// clang says: 1.cpp:5:18: warning: cast to 'unsigned char *' from smaller integer type 'uint8_t' (aka 'unsigned char') [-Wint-to-pointer-cast]
2064+
TODO_ASSERT_EQUALS("bad cast from uint8_t to pointer", "",
20792065
errout_str());
20802066

20812067
// #10823
@@ -2085,12 +2071,14 @@ class TestOther : public TestFixture {
20852071
ASSERT_EQUALS("", errout_str());
20862072

20872073
// #5210
2088-
checkOldStylePointerCast("void f(void* v1, void* v2) {\n"
2089-
" T** p1 = (T**)v1;\n"
2090-
" T*** p2 = (T***)v2;\n"
2074+
checkOldStylePointerCast("class Base {};\n"
2075+
"class Derived: public Base {};\n"
2076+
"void f(Derived** d1, Derived*** d2) {\n"
2077+
" Base** p1 = (Base**)d1;\n"
2078+
" Base*** p2 = (Base***)d2;\n"
20912079
"}\n");
2092-
ASSERT_EQUALS("[test.cpp:2:14]: (style) C-style pointer casting [cstyleCast]\n"
2093-
"[test.cpp:3:15]: (style) C-style pointer casting [cstyleCast]\n",
2080+
ASSERT_EQUALS("[test.cpp:4:17]: (style) C-style pointer casting [cstyleCast]\n"
2081+
"[test.cpp:5:18]: (style) C-style pointer casting [cstyleCast]\n",
20942082
errout_str());
20952083

20962084
// #12446
@@ -2104,17 +2092,15 @@ class TestOther : public TestFixture {
21042092
" auto pu = (union U*)p;\n"
21052093
" auto pv = (std::vector<int>*)(p);\n"
21062094
"}\n");
2107-
ASSERT_EQUALS("[test.cpp:7:15]: (style) C-style pointer casting [cstyleCast]\n"
2108-
"[test.cpp:8:16]: (style) C-style pointer casting [cstyleCast]\n"
2109-
"[test.cpp:9:15]: (style) C-style pointer casting [cstyleCast]\n",
2110-
errout_str());
2095+
ASSERT_EQUALS("", errout_str()); // There are other checkers and compiler warnings that warn about all C style casts
21112096

21122097
// #12447
2113-
checkOldStylePointerCast("void f(const int& i) {\n"
2114-
" int& r = (int&)i;\n"
2115-
" r = 0;\n"
2098+
checkOldStylePointerCast("class Base {};\n"
2099+
"class Derived: public Base {};\n"
2100+
"void f(const Derived& derived) {\n"
2101+
" Base& b = (Base&)derived;\n"
21162102
"}\n");
2117-
ASSERT_EQUALS("[test.cpp:2:12]: (style) C-style reference casting [cstyleCast]\n", errout_str());
2103+
ASSERT_EQUALS("[test.cpp:4:13]: (style) C-style reference casting [cstyleCast]\n", errout_str());
21182104

21192105
// #11430
21202106
checkOldStylePointerCast("struct B {\n"
@@ -2125,9 +2111,9 @@ class TestOther : public TestFixture {
21252111
"}\n"
21262112
"bool g(B& b) {\n"
21272113
" using float_ptr = float*;\n"
2128-
" return N::f(float_ptr(b.data()));\n"
2114+
" return N::f(float_ptr(b.data()));\n" // <- the cast is safe
21292115
"}\n");
2130-
ASSERT_EQUALS("[test.cpp:9:17]: (style) C-style pointer casting [cstyleCast]\n", errout_str());
2116+
ASSERT_EQUALS("", errout_str());
21312117
}
21322118

21332119
#define checkInvalidPointerCast(...) checkInvalidPointerCast_(__FILE__, __LINE__, __VA_ARGS__)

0 commit comments

Comments
 (0)