Fix #13876 (Document cstyleCast) [ci skip]

lib/checkother.cpp

@@ -296,10 +296,6 @@ void CheckOther::suspiciousSemicolonError(const Token* tok)
 //---------------------------------------------------------------------------
 void CheckOther::warningOldStylePointerCast()
 {
-    // Only valid on C++ code
-    if (!mTokenizer->isCPP())
-        return;
-
     if (!mSettings->severity.isEnabled(Severity::style) && !mSettings->isPremiumEnabled("cstyleCast"))
         return;
 
@@ -314,44 +310,44 @@ 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;
-            const Token* castTok = tok->next();
-            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())
+            const Token* from = tok->astOperand1();
+            if (!from)
                 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 ((!isPtr && !isRef) || !Token::Match(castTok, ") (| %name%|%num%|%bool%|%char%|%str%|&"))
+            if (!tok->valueType() || !from->valueType())
                 continue;
-
-            if (Token::Match(tok->previous(), "%type%"))
+            if (tok->valueType()->type == from->valueType()->type &&
+                tok->valueType()->typeScope == from->valueType()->typeScope)
                 continue;
+            const bool refcast = (tok->valueType()->reference != Reference::None);
+            if (!refcast && tok->valueType()->pointer == 0)
+                continue;
+            if (!refcast && from->valueType()->pointer == 0) {
+                // casting non-zero integer literal (decimal/octal) to pointer is suspicious
+                if (mSettings->severity.isEnabled(Severity::portability) &&
+                    tok->valueType()->pointer > 0 &&
+                    from->isNumber() &&
+                    !MathLib::isIntHex(from->str()) &&
+                    from->getKnownIntValue() != 0)
+                    intToPointerCastError(tok);
+                continue;
+            }
 
-            // skip first "const" in "const Type* const"
-            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"
-            if (tok->strAt(3) == "const")
-                tok = tok->next();
+            // Use C++ cast: Only valid on C++ code
+            if (!mTokenizer->isCPP())
+                continue;
 
-            const Token *p = tok->tokAt(4);
-            if (p->hasKnownIntValue() && p->getKnownIntValue()==0) // Casting nullpointers is safe
+            if (tok->valueType()->type == ValueType::Type::VOID || from->valueType()->type == ValueType::Type::VOID)
+                continue;
+            if (tok->valueType()->pointer == 0 && tok->valueType()->isIntegral())
+                // ok: (uintptr_t)ptr;
+                continue;
+            if (from->valueType()->pointer == 0 && from->valueType()->isIntegral())
+                // ok: (int *)addr;
                 continue;
 
-            if (typeTok->tokType() == Token::eType || typeTok->tokType() == Token::eName)
-                cstyleCastError(tok, isPtr);
+            cstyleCastError(tok, tok->valueType()->pointer > 0);
         }
     }
 }
@@ -367,6 +363,14 @@ void CheckOther::cstyleCastError(const Token *tok, bool isPtr)
                 "which kind of cast is expected.", CWE398, Certainty::normal);
 }
 
+
+void CheckOther::intToPointerCastError(const Token *tok)
+{
+    reportError(tok, Severity::portability, "intToPointerCast",
+                "Casting non-zero integer literal in decimal or octal format to pointer.",
+                CWE398, Certainty::normal);
+}
+
 void CheckOther::suspiciousFloatingPointCast()
 {
     if (!mSettings->severity.isEnabled(Severity::style) && !mSettings->isPremiumEnabled("suspiciousFloatingPointCast"))
@@ -4459,6 +4463,7 @@ void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *sett
     c.checkComparisonFunctionIsAlwaysTrueOrFalseError(nullptr, "isless","varName",false);
     c.checkCastIntToCharAndBackError(nullptr, "func_name");
     c.cstyleCastError(nullptr);
+    c.intToPointerCastError(nullptr);
     c.suspiciousFloatingPointCastError(nullptr);
     c.passedByValueError(nullptr, false);
     c.constVariableError(nullptr, nullptr);

lib/checkother.h

@@ -198,6 +198,7 @@ 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 intToPointerCastError(const Token *tok);
     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);
@@ -281,6 +282,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"

man/checkers/cstyleCast.md

@@ -0,0 +1,81 @@
+
+# cstyleCast
+
+**Message**: C-style pointer casting<br/>
+**Category**: Type Safety<br/>
+**Severity**: Style<br/>
+**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 can be dangerous.
+
+This checker is not about readability. It is about safety.
+
+This checker focus on invalid type conversions:
+If there is loss of sign or precision in the cast; it's hard to know for Cppcheck if that is intentional.
+If there is loss of constness; there are other checkers available and these apply to C code also.
+Therefore we only warn about pointer/reference casts that could be invalid type conversions.
+
+## Other tools / checkers
+
+Extended checking; if you want more warnings that warn about all C style casts:
+ * Misra C++ 8.2.2, cover all C style casts.
+ * C style cast compiler warnings (i.e. clang reports -Wold-style-cast)
+
+Complementing; checking for loss of constness etc:
+ * Misra C 11.8
+ * Loss of constness compiler warnings (i.e. clang reports -Wcast-qual).
+
+## Examples
+
+### Compliant C style cast
+
+The goal is to not warn about "safe" and/or "intentional" casts.
+
+For example:
+```cpp
+int *p = (int*)0;
+```
+A static_cast can be used however technically it's the same as the C style cast.
+
+### Non compliant C style cast
+
+A cast from a base class pointer to a derived class pointer is potentially unsafe:
+```cpp
+struct Base{};
+struct Derived: public Base {};
+void foo(Base* base) {
+    Derived *p = (Derived*)base; // <- dangerous
+}
+```
+
+## How to fix
+
+You can use `dynamic_cast` or `static_cast` to fix warnings.
+
+Before:
+```cpp
+struct Base{};
+struct Derived: public Base {};
+void foo(Base* base) {
+    Derived *p = (Derived*)base; // <- dangerous
+}
+```
+
+After:
+```cpp
+struct Base{};
+struct Derived: public Base {};
+void foo(Base* base) {
+    Derived *p = dynamic_cast<Derived*>(base);
+}
+```