Skip to content

Commit baced49

Browse files
danmardanmar
committed
restore cstyleCast, write dangerousOldStyleTypeCast for dangerous type casts
1 parent 21cea1d commit baced49

7 files changed

Lines changed: 159 additions & 68 deletions

File tree

‎lib/checkother.cpp‎

Lines changed: 107 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,44 @@ void CheckOther::suspiciousSemicolonError(const Token* tok)
290290
"Suspicious use of ; at the end of '" + (tok ? tok->str() : std::string()) + "' statement.", CWE398, Certainty::normal);
291291
}
292292

293+
/** @brief would it make sense to use dynamic_cast instead of C style cast? */
294+
static bool isDangerousTypeConversion(const Token* const tok)
295+
{
296+
const Token* from = tok->astOperand1();
297+
if (!from)
298+
return false;
299+
if (!tok->valueType() || !from->valueType())
300+
return false;
301+
if (tok->valueType()->typeScope != nullptr &&
302+
tok->valueType()->typeScope == from->valueType()->typeScope)
303+
return false;
304+
if (tok->valueType()->type == from->valueType()->type &&
305+
tok->valueType()->isPrimitive())
306+
return false;
307+
// cast from derived object to base object is safe..
308+
if (tok->valueType()->typeScope && from->valueType()->typeScope) {
309+
const Type* fromType = from->valueType()->typeScope->definedType;
310+
const Type* toType = tok->valueType()->typeScope->definedType;
311+
if (fromType && toType && fromType->isDerivedFrom(toType->name()))
312+
return false;
313+
}
314+
const bool refcast = (tok->valueType()->reference != Reference::None);
315+
if (!refcast && tok->valueType()->pointer == 0)
316+
return false;
317+
if (!refcast && from->valueType()->pointer == 0)
318+
return false;
319+
320+
if (tok->valueType()->type == ValueType::Type::VOID || from->valueType()->type == ValueType::Type::VOID)
321+
return false;
322+
if (tok->valueType()->pointer == 0 && tok->valueType()->isIntegral())
323+
// ok: (uintptr_t)ptr;
324+
return false;
325+
if (from->valueType()->pointer == 0 && from->valueType()->isIntegral())
326+
// ok: (int *)addr;
327+
return false;
328+
329+
return true;
330+
}
293331

294332
//---------------------------------------------------------------------------
295333
// For C++ code, warn if C-style casts are used on pointer types
@@ -316,40 +354,45 @@ void CheckOther::warningOldStylePointerCast()
316354
// Old style pointer casting..
317355
if (!tok->isCast() || tok->isBinaryOp())
318356
continue;
319-
const Token* from = tok->astOperand1();
320-
if (!from)
321-
continue;
322-
if (!tok->valueType() || !from->valueType())
323-
continue;
324-
if (tok->valueType()->typeScope != nullptr &&
325-
tok->valueType()->typeScope == from->valueType()->typeScope)
357+
if (isDangerousTypeConversion(tok))
326358
continue;
327-
if (tok->valueType()->type == from->valueType()->type &&
328-
tok->valueType()->isPrimitive())
329-
continue;
330-
// cast from derived object to base object is safe..
331-
if (tok->valueType()->typeScope && from->valueType()->typeScope) {
332-
const Type* fromType = from->valueType()->typeScope->definedType;
333-
const Type* toType = tok->valueType()->typeScope->definedType;
334-
if (fromType && toType && fromType->isDerivedFrom(toType->name()))
335-
continue;
359+
const Token* const errtok = tok;
360+
const Token* castTok = tok->next();
361+
while (Token::Match(castTok, "const|volatile|class|struct|union|%type%|::")) {
362+
castTok = castTok->next();
363+
if (Token::simpleMatch(castTok, "<") && castTok->link())
364+
castTok = castTok->link()->next();
336365
}
337-
const bool refcast = (tok->valueType()->reference != Reference::None);
338-
if (!refcast && tok->valueType()->pointer == 0)
366+
if (castTok == tok->next())
339367
continue;
340-
if (!refcast && from->valueType()->pointer == 0)
368+
bool isPtr = false, isRef = false;
369+
while (Token::Match(castTok, "*|const|&")) {
370+
if (castTok->str() == "*")
371+
isPtr = true;
372+
else if (castTok->str() == "&")
373+
isRef = true;
374+
castTok = castTok->next();
375+
}
376+
if ((!isPtr && !isRef) || !Token::Match(castTok, ") (| %name%|%bool%|%char%|%str%|&"))
341377
continue;
342378

343-
if (tok->valueType()->type == ValueType::Type::VOID || from->valueType()->type == ValueType::Type::VOID)
379+
if (Token::Match(tok->previous(), "%type%"))
344380
continue;
345-
if (tok->valueType()->pointer == 0 && tok->valueType()->isIntegral())
346-
// ok: (uintptr_t)ptr;
347-
continue;
348-
if (from->valueType()->pointer == 0 && from->valueType()->isIntegral())
349-
// ok: (int *)addr;
381+
382+
// skip first "const" in "const Type* const"
383+
while (Token::Match(tok->next(), "const|volatile|class|struct|union"))
384+
tok = tok->next();
385+
const Token* typeTok = tok->next();
386+
// skip second "const" in "const Type* const"
387+
if (tok->strAt(3) == "const")
388+
tok = tok->next();
389+
390+
const Token *p = tok->tokAt(4);
391+
if (p->hasKnownIntValue() && p->getKnownIntValue()==0) // Casting nullpointers is safe
350392
continue;
351393

352-
cstyleCastError(tok, tok->valueType()->pointer > 0);
394+
if (typeTok->tokType() == Token::eType || typeTok->tokType() == Token::eName)
395+
cstyleCastError(errtok, isPtr);
353396
}
354397
}
355398
}
@@ -365,14 +408,49 @@ void CheckOther::cstyleCastError(const Token *tok, bool isPtr)
365408
"which kind of cast is expected.", CWE398, Certainty::normal);
366409
}
367410

411+
void CheckOther::warningDangerousOldStyleTypeCast()
412+
{
413+
// Only valid on C++ code
414+
if (!mTokenizer->isCPP())
415+
return;
416+
if (!mSettings->severity.isEnabled(Severity::warning) && !mSettings->isPremiumEnabled("cstyleCast"))
417+
return;
418+
419+
logChecker("CheckOther::warningDangerousOldStyleTypeCast"); // warning,c++
420+
421+
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
422+
for (const Scope * scope : symbolDatabase->functionScopes) {
423+
const Token* tok;
424+
if (scope->function && scope->function->isConstructor())
425+
tok = scope->classDef;
426+
else
427+
tok = scope->bodyStart;
428+
for (; tok && tok != scope->bodyEnd; tok = tok->next()) {
429+
// Old style pointer casting..
430+
if (!tok->isCast() || tok->isBinaryOp())
431+
continue;
432+
433+
if (isDangerousTypeConversion(tok))
434+
dangerousOldStyleTypeCastError(tok, tok->valueType()->pointer > 0);
435+
}
436+
}
437+
}
438+
439+
void CheckOther::dangerousOldStyleTypeCastError(const Token *tok, bool isPtr)
440+
{
441+
const std::string type = isPtr ? "pointer" : "reference";
442+
reportError(tok, Severity::warning, "dangerousOldStyleTypeCast",
443+
"Potentially dangerous C style type cast of " + type + " to object, use dynamic_cast or static_cast",
444+
CWE398, Certainty::normal);
445+
}
446+
368447
void CheckOther::warningIntToPointerCast()
369448
{
370-
if (!mSettings->severity.isEnabled(Severity::portability))
449+
if (!mSettings->severity.isEnabled(Severity::portability) && !mSettings->isPremiumEnabled("cstyleCast"))
371450
return;
372451

373452
logChecker("CheckOther::warningIntToPointerCast"); // portability
374453

375-
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
376454
for (const Token* tok = mTokenizer->tokens(); tok; tok = tok->next()) {
377455
// pointer casting..
378456
if (!tok->isCast())
@@ -4487,6 +4565,7 @@ void CheckOther::getErrorMessages(ErrorLogger *errorLogger, const Settings *sett
44874565
c.checkComparisonFunctionIsAlwaysTrueOrFalseError(nullptr, "isless","varName",false);
44884566
c.checkCastIntToCharAndBackError(nullptr, "func_name");
44894567
c.cstyleCastError(nullptr);
4568+
c.dangerousOldStyleTypeCastError(nullptr, true);
44904569
c.intToPointerCastError(nullptr);
44914570
c.suspiciousFloatingPointCastError(nullptr);
44924571
c.passedByValueError(nullptr, false);

‎lib/checkother.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ class CPPCHECKLIB CheckOther : public Check {
8080
/** @brief Are there C-style pointer casts in a c++ file? */
8181
void warningOldStylePointerCast();
8282

83+
/** @brief Dangerous type cast */
84+
void warningDangerousOldStyleTypeCast();
85+
8386
/** @brief Casting non-hexadecimal integer literal to pointer */
8487
void warningIntToPointerCast();
8588

@@ -201,6 +204,7 @@ class CPPCHECKLIB CheckOther : public Check {
201204
void clarifyCalculationError(const Token *tok, const std::string &op);
202205
void clarifyStatementError(const Token* tok);
203206
void cstyleCastError(const Token *tok, bool isPtr = true);
207+
void dangerousOldStyleTypeCastError(const Token *tok, bool isPtr);
204208
void intToPointerCastError(const Token *tok);
205209
void suspiciousFloatingPointCastError(const Token *tok);
206210
void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive, bool toIsInt);
@@ -277,6 +281,7 @@ class CPPCHECKLIB CheckOther : public Check {
277281
// warning
278282
"- either division by zero or useless condition\n"
279283
"- access of moved or forwarded variable.\n"
284+
"- potentially dangerous C style type cast of pointer/reference to object.\n"
280285

281286
// performance
282287
"- redundant data copying for const variable\n"

‎test/cfg/googletest.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ namespace ExampleNamespace {
4444

4545
TEST(ASSERT, ASSERT)
4646
{
47-
int *a = (int*)calloc(10,sizeof(int));
47+
int *a = (int*)calloc(10,sizeof(int)); // cppcheck-suppress cstyleCast
4848
ASSERT_TRUE(a != nullptr);
4949

5050
a[0] = 10;

‎test/cfg/opencv2.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ void validCode(const char* argStr)
3030
cvStr += " World";
3131
std::cout << cvStr;
3232

33-
// cppcheck-suppress unusedAllocatedMemory
33+
// cppcheck-suppress [cstyleCast, unusedAllocatedMemory]
3434
char * pBuf = (char *)cv::fastMalloc(20);
3535
cv::fastFree(pBuf);
3636
}
@@ -43,6 +43,7 @@ void ignoredReturnValue()
4343

4444
void memleak()
4545
{
46+
// cppcheck-suppress cstyleCast
4647
const char * pBuf = (char *)cv::fastMalloc(1000);
4748
// cppcheck-suppress [uninitdata, valueFlowBailoutIncompleteVar, nullPointerOutOfMemory]
4849
std::cout << pBuf;

‎test/cfg/std.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3027,7 +3027,7 @@ void uninitvar_longjmp(void)
30273027
void uninitvar_malloc(void)
30283028
{
30293029
size_t size;
3030-
// cppcheck-suppress [uninitvar, unusedAllocatedMemory]
3030+
// cppcheck-suppress [uninitvar, cstyleCast, unusedAllocatedMemory]
30313031
int *p = (int*)std::malloc(size);
30323032
free(p);
30333033
}
@@ -3259,7 +3259,7 @@ void uninivar_bsearch(void)
32593259
const void* base;
32603260
size_t num;
32613261
size_t size;
3262-
// cppcheck-suppress uninitvar
3262+
// cppcheck-suppress [uninitvar, cstyleCast]
32633263
(void)std::bsearch(key,base,num,size,(int (*)(const void*,const void*))strcmp);
32643264
}
32653265

@@ -3269,11 +3269,11 @@ void minsize_bsearch(const void* key, const void* base,
32693269
{
32703270
const int Base[3] = {42, 43, 44};
32713271

3272-
(void)std::bsearch(key,Base,2,size,(int (*)(const void*,const void*))strcmp);
3273-
(void)std::bsearch(key,Base,3,size,(int (*)(const void*,const void*))strcmp);
3274-
(void)std::bsearch(key,Base,4,size,(int (*)(const void*,const void*))strcmp);
3272+
(void)std::bsearch(key,Base,2,size,(int (*)(const void*,const void*))strcmp); // cppcheck-suppress cstyleCast
3273+
(void)std::bsearch(key,Base,3,size,(int (*)(const void*,const void*))strcmp); // cppcheck-suppress cstyleCast
3274+
(void)std::bsearch(key,Base,4,size,(int (*)(const void*,const void*))strcmp); // cppcheck-suppress cstyleCast
32753275

3276-
(void)std::bsearch(key,base,2,size,(int (*)(const void*,const void*))strcmp);
3276+
(void)std::bsearch(key,base,2,size,(int (*)(const void*,const void*))strcmp); // cppcheck-suppress cstyleCast
32773277
}
32783278

32793279
void uninitvar_qsort(void)
@@ -3282,7 +3282,7 @@ void uninitvar_qsort(void)
32823282
size_t n;
32833283
size_t size;
32843284
// cppcheck-suppress uninitvar
3285-
(void)std::qsort(base,n,size, (int (*)(const void*,const void*))strcmp);
3285+
(void)std::qsort(base,n,size, (int (*)(const void*,const void*))strcmp); // cppcheck-suppress cstyleCast
32863286
}
32873287

32883288
void uninitvar_stable_sort(std::vector<int>& v)

‎test/cfg/windows.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ void memleak_HeapAlloc()
624624
void memleak_LocalAlloc()
625625
{
626626
LPTSTR pszBuf;
627-
// cppcheck-suppress [LocalAllocCalled, valueFlowBailoutIncompleteVar]
627+
// cppcheck-suppress [LocalAllocCalled, cstyleCast, valueFlowBailoutIncompleteVar]
628628
pszBuf = (LPTSTR)LocalAlloc(LPTR, MAX_PATH*sizeof(TCHAR));
629629
(void)LocalSize(pszBuf);
630630
(void)LocalFlags(pszBuf);
@@ -700,7 +700,7 @@ void resourceLeak_LoadLibrary()
700700
HINSTANCE hInstLib;
701701
hInstLib = ::LoadLibrary(L"My.dll");
702702
typedef BOOL (WINAPI *fpFunc)();
703-
// cppcheck-suppress unreadVariable
703+
// cppcheck-suppress [unreadVariable, cstyleCast]
704704
fpFunc pFunc = (fpFunc)GetProcAddress(hInstLib, "name");
705705
// cppcheck-suppress resourceLeak
706706
}
@@ -825,7 +825,7 @@ void invalidFunctionArg()
825825
CloseHandle(hMutex);
826826

827827
//Incorrect: 2. parameter to LoadLibraryEx() must be NULL
828-
// cppcheck-suppress [invalidFunctionArg, intToPointerCast]
828+
// cppcheck-suppress [invalidFunctionArg, cstyleCast]
829829
HINSTANCE hInstLib = LoadLibraryEx(L"My.dll", HANDLE(1), 0);
830830
FreeLibrary(hInstLib);
831831

0 commit comments

Comments
 (0)