From 27dce075e037461f18377d04112c45f3d01108de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Debrard=20S=C3=A9bastien?= Date: Sat, 22 Jan 2011 19:21:56 +0100 Subject: [PATCH] Fixed #155 (check size of a variable whose type is a sized array) --- lib/checkother.cpp | 45 ++++++++++++++++++++++++ lib/checkother.h | 7 ++++ test/testother.cpp | 85 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 0d0808ece..2ff551e57 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -78,6 +78,35 @@ void CheckOther::checkFflushOnInputStream() } } + +void CheckOther::checkSizeofForArrayParameter() +{ + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) + { + if (Token::Match(tok, "sizeof ( %var% )") || Token::Match(tok, "sizeof %var% ")) + { + int tokIdx = 1; + if (tok->tokAt(tokIdx)->str() == "(") + { + ++tokIdx; + } + if (tok->tokAt(tokIdx)->varId() > 0) + { + const Token *declTok = Token::findmatch(_tokenizer->tokens(), "%varid%", tok->tokAt(tokIdx)->varId()); + if (declTok) + { + if ((Token::simpleMatch(declTok->next(), "[")) && !(Token::simpleMatch(declTok->next()->link(), "] = {")) && !(Token::simpleMatch(declTok->next()->link(), "] ;"))) + { + sizeofForArrayParameterError(tok); + } + } + } + } + } +} + + + //--------------------------------------------------------------------------- // switch (x) // { @@ -454,6 +483,22 @@ void CheckOther::invalidScanf() } } +void CheckOther::sizeofForArrayParameterError(const Token *tok) +{ + reportError(tok, Severity::error, + "sizeofwithsilentarraypointer", "Using sizeof for array given as function argument " + "returns the size of pointer.\n" + "Giving array as function parameter and then using sizeof-operator for the array " + "argument. In this case the sizeof-operator returns the size of pointer (in the " + " system). It does not return the size of the whole array in bytes as might be " + "expected. For example, this code:\n" + " int f(char a[100]) {\n" + " return sizeof(a);\n" + " }\n" + " returns 4 (in 32-bit systems) or 8 (in 64-bit systems) instead of 100 (the " + "size of the array in bytes)." + ); +} void CheckOther::invalidScanfError(const Token *tok) { reportError(tok, Severity::warning, diff --git a/lib/checkother.h b/lib/checkother.h index 9e49d8dce..0d0798dd7 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -62,6 +62,7 @@ public: checkOther.sizeofCalculation(); checkOther.checkRedundantAssignmentInSwitch(); checkOther.checkAssignmentInAssert(); + checkOther.checkSizeofForArrayParameter(); } /** @brief Run checks against the simplified token list */ @@ -170,6 +171,9 @@ public: /** @brief %Check for filling zero bytes with memset() */ void checkMemsetZeroBytes(); + /** @brief %Check for using sizeof with array given as function argument */ + void checkSizeofForArrayParameter(); + // Error messages.. void cstyleCastError(const Token *tok); void dangerousUsageStrtolError(const Token *tok); @@ -193,6 +197,7 @@ public: void misusedScopeObjectError(const Token *tok, const std::string &varname); void catchExceptionByValueError(const Token *tok); void memsetZeroBytesError(const Token *tok, const std::string &varname); + void sizeofForArrayParameterError(const Token *tok); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { @@ -205,6 +210,7 @@ public: c.mathfunctionCallError(0); c.fflushOnInputStreamError(0, "stdin"); c.misusedScopeObjectError(NULL, "varname"); + c.sizeofForArrayParameterError(0); // style/warning c.cstyleCastError(0); @@ -247,6 +253,7 @@ public: "* using fflush() on an input stream\n" "* scoped object destroyed immediately after construction\n" "* assignment in an assert statement\n" + "* sizeof for array given as function argument\n" // style "* C-style pointer cast in cpp file\n" diff --git a/test/testother.cpp b/test/testother.cpp index 935c7df32..934ab01ca 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -97,6 +97,8 @@ private: TEST_CASE(catchExceptionByValue); TEST_CASE(memsetZeroBytes); + + TEST_CASE(sizeofWithSilentArrayPointer); } void check(const char code[], const char *filename = NULL) @@ -118,6 +120,7 @@ private: checkOther.sizeofCalculation(); checkOther.checkRedundantAssignmentInSwitch(); checkOther.checkAssignmentInAssert(); + checkOther.checkSizeofForArrayParameter(); // Simplify token list.. tokenizer.simplifyTokenList(); @@ -1659,6 +1662,88 @@ private: " bytes of \"p\". Second and third arguments might be inverted.\n", errout.str()); ASSERT_EQUALS("", errout.str()); } + + void sizeofWithSilentArrayPointer() + { + check("void f() {\n" + " int a[10];\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " unsigned int a = 2;\n" + " unsigned int b = 2;\n" + " int c[(a+b)];\n" + " std::cout << sizeof(c) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " unsigned int a = { 2 };\n" + " unsigned int b[] = { 0 };\n" + " int c[a[b[0]]];\n" + " std::cout << sizeof(c) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + + check("void f() {\n" + " unsigned int a[] = { 1 };\n" + " unsigned int b = 2;\n" + " int c[(a[0]+b)];\n" + " std::cout << sizeof(c) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int a[] = { 1, 2, 3 };\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " int a[3] = { 1, 2, 3 };\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + check("void f( int a[]) {\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (error) Using sizeof for array given as " + "function argument returns the size of pointer.\n", errout.str()); + + check("void f( int a[]) {\n" + " std::cout << sizeof a / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (error) Using sizeof for array given as " + "function argument returns the size of pointer.\n", errout.str()); + + check("void f( int a[3] ) {\n" + " std::cout << sizeof(a) / sizeof(int) << std::endl;\n" + "}\n" + ); + ASSERT_EQUALS("[test.cpp:2]: (error) Using sizeof for array given as " + "function argument returns the size of pointer.\n", errout.str()); + + check("void f(int *p) {\n" + " p[0] = 0;\n" + " sizeof(p);\n" + "}\n" + ); + ASSERT_EQUALS("", errout.str()); + + } + }; REGISTER_TEST(TestOther)