From 4bb99a7887b1e026b715f5815b24e3e436152e4b Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sun, 22 May 2016 22:29:21 +0200 Subject: [PATCH] Improved CheckSizeof::checkSizeofForPointerSize(): - Support cast in front of malloc() call - Support sizeof(type) pattern (#4428) --- lib/checksizeof.cpp | 33 +++++++++++++++++++-------------- test/testsizeof.cpp | 31 +++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/lib/checksizeof.cpp b/lib/checksizeof.cpp index 0ee27b018..4c82c991e 100644 --- a/lib/checksizeof.cpp +++ b/lib/checksizeof.cpp @@ -120,20 +120,15 @@ void CheckSizeof::checkSizeofForPointerSize() // Once leaving those tests, it is mandatory to have: // - variable matching the used pointer // - tokVar pointing on the argument where sizeof may be used - if (Token::Match(tok, "%var% = malloc|alloca (")) { - variable = tok; + if (Token::Match(tok->tokAt(2), "malloc|alloca|calloc (")) { + if (Token::Match(tok, "%var% =")) + variable = tok; + else if (tok->strAt(1) == ")" && Token::Match(tok->linkAt(1)->tokAt(-2), "%var% =")) + variable = tok->linkAt(1)->tokAt(-2); + else if (tok->link() && Token::Match(tok, "> ( malloc|alloca|calloc (") && Token::Match(tok->link()->tokAt(-3), "%var% =")) + variable = tok->link()->tokAt(-3); tokSize = tok->tokAt(4); tokFunc = tok->tokAt(2); - } else if (Token::Match(tok, "%var% = calloc (")) { - variable = tok; - tokSize = tok->tokAt(4)->nextArgument(); - tokFunc = tok->tokAt(2); - } else if (Token::Match(tok, "return malloc|alloca (")) { - tokSize = tok->tokAt(3); - tokFunc = tok->next(); - } else if (Token::simpleMatch(tok, "return calloc (")) { - tokSize = tok->tokAt(3)->nextArgument(); - tokFunc = tok->next(); } else if (Token::simpleMatch(tok, "memset (") && tok->strAt(-1) != ".") { variable = tok->tokAt(2); tokSize = variable->nextArgument(); @@ -151,6 +146,9 @@ void CheckSizeof::checkSizeofForPointerSize() continue; } + if (tokFunc && tokFunc->str() == "calloc") + tokSize = tokSize->nextArgument(); + if (tokFunc && tokSize) { for (const Token* tok2 = tokSize; tok2 != tokFunc->linkAt(1); tok2 = tok2->next()) { if (Token::simpleMatch(tok2, "/ sizeof")) @@ -192,6 +190,14 @@ void CheckSizeof::checkSizeofForPointerSize() if (tokSize->str() != "sizeof") continue; + // Now check for the sizeof usage: Does the level of pointer indirection match? + if (tokSize->linkAt(1)->strAt(-1) == "*") { + if (variable && variable->valueType() && variable->valueType()->pointer == 1) + sizeofForPointerError(variable, variable->str()); + else if (variable2 && variable2->valueType() && variable2->valueType()->pointer == 1) + sizeofForPointerError(variable2, variable2->str()); + } + if (Token::simpleMatch(tokSize, "sizeof ( &")) tokSize = tokSize->tokAt(3); else if (Token::Match(tokSize, "sizeof (|&")) @@ -205,9 +211,8 @@ void CheckSizeof::checkSizeofForPointerSize() if (Token::Match(tokSize, "%var% [|(")) continue; - // Now check for the sizeof usage. Once here, everything using sizeof(varid) or sizeof(&varid) + // Now check for the sizeof usage again. Once here, everything using sizeof(varid) or sizeof(&varid) // looks suspicious - // Do it for first variable if (variable && tokSize->varId() == variable->varId()) sizeofForPointerError(variable, variable->str()); if (variable2 && tokSize->varId() == variable2->varId()) diff --git a/test/testsizeof.cpp b/test/testsizeof.cpp index 461f6fc69..0d91f5f3c 100644 --- a/test/testsizeof.cpp +++ b/test/testsizeof.cpp @@ -349,12 +349,38 @@ private: "}"); ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + check("void f() {\n" + " int *x = (int*)malloc(sizeof(x));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = static_cast(malloc(sizeof(x)));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + check("void f() {\n" " int *x = malloc(sizeof(&x));\n" " free(x);\n" "}"); ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + check("void f() {\n" + " int *x = malloc(sizeof(int*));\n" + " free(x);\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'x' used instead of size of its data.\n", errout.str()); + + check("void f() {\n" + " int *x = malloc(sizeof(int));\n" + " free(x);\n" + " int **y = malloc(sizeof(int*));\n" + " free(y);\n" + "}"); + ASSERT_EQUALS("", errout.str()); + check("void f() {\n" " int *x = malloc(100 * sizeof(x));\n" " free(x);\n" @@ -517,6 +543,11 @@ private: "}"); ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'buf1' used instead of size of its data.\n", errout.str()); + check("int fun(const char *buf2) {\n" + " return strncmp(buf1, buf2, sizeof(char*)) == 0;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) Size of pointer 'buf2' used instead of size of its data.\n", errout.str()); + // #ticket 3874 check("void f()\n" "{\n"