From 02055821612f28500ad56f3953d6e75d3d8dcaae Mon Sep 17 00:00:00 2001 From: PKEuS Date: Sun, 18 Dec 2011 19:41:21 +0100 Subject: [PATCH] Fixed #3327 (printf with std::string as parameter) --- lib/checkother.cpp | 141 ++++++++++++++++++++++++++++++++++++++++++--- lib/checkother.h | 12 ++++ test/testother.cpp | 90 +++++++++++++++++++++++++---- 3 files changed, 224 insertions(+), 19 deletions(-) diff --git a/lib/checkother.cpp b/lib/checkother.cpp index 21a110d34..2339ee580 100644 --- a/lib/checkother.cpp +++ b/lib/checkother.cpp @@ -1230,13 +1230,35 @@ void CheckOther::invalidScanfError(const Token *tok) } //--------------------------------------------------------------------------- -// printf("%u", "xyz"); // Wrong argument type. TODO. +// printf("%u", "xyz"); // Wrong argument type // printf("%u%s", 1); // Too few arguments // printf("", 1); // Too much arguments //--------------------------------------------------------------------------- +static bool isComplexType(const Variable* var, const Token* varTypeTok) +{ + if (var->type()) + return(true); + + static std::set knownTypes; + if (knownTypes.empty()) { + knownTypes.insert("struct"); // If a type starts with the struct keyword, its a complex type + knownTypes.insert("string"); + } + + if (varTypeTok->str() == "std") + varTypeTok = varTypeTok->tokAt(2); + return(knownTypes.find(varTypeTok->str()) != knownTypes.end() && !var->isPointer() && !var->isArray()); +} + +static bool isKnownType(const Variable* var, const Token* varTypeTok) +{ + return(varTypeTok->isStandardType() || varTypeTok->next()->isStandardType() || isComplexType(var, varTypeTok)); +} void CheckOther::checkWrongPrintfScanfArguments() { + const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase(); + for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) { if (!tok->isName()) continue; @@ -1286,6 +1308,7 @@ void CheckOther::checkWrongPrintfScanfArguments() bool scan = Token::Match(tok, "sscanf|fscanf|scanf"); unsigned int numFormat = 0; bool percent = false; + const Token* argListTok2 = argListTok; for (std::string::iterator i = formatString.begin(); i != formatString.end(); ++i) { if (*i == '%') { percent = !percent; @@ -1293,6 +1316,8 @@ void CheckOther::checkWrongPrintfScanfArguments() while (i != formatString.end()) { if (*i == ']') { numFormat++; + if (argListTok) + argListTok = argListTok->nextArgument(); percent = false; break; } @@ -1308,8 +1333,11 @@ void CheckOther::checkWrongPrintfScanfArguments() if (*i == '*') { if (scan) _continue = true; - else + else { numFormat++; + if (argListTok) + argListTok = argListTok->nextArgument(); + } } ++i; } @@ -1318,18 +1346,80 @@ void CheckOther::checkWrongPrintfScanfArguments() if (_continue) continue; - if (*i != 'm') // %m is a non-standard extension that requires no parameter + if (*i != 'm') { // %m is a non-standard extension that requires no parameter numFormat++; - // TODO: Perform type checks + // Perform type checks + if (_settings->isEnabled("style") && Token::Match(argListTok, "%any% ,|)")) { // We can currently only check the type of arguments matching this simple pattern. + const Variable* variableInfo = symbolDatabase->getVariableFromVarId(argListTok->varId()); + const Token* varTypeTok = variableInfo ? variableInfo->typeStartToken() : NULL; + if (varTypeTok && varTypeTok->str() == "static") + varTypeTok = varTypeTok->next(); + + if (scan && varTypeTok) { + if ((!variableInfo->isPointer() && !variableInfo->isArray()) || varTypeTok->str() == "const") + invalidScanfArgTypeError(tok, tok->str(), numFormat); + } else if (!scan) { + switch (*i) { + case 's': + if (variableInfo && !Token::Match(argListTok, "%str%") && isKnownType(variableInfo, varTypeTok) && (!variableInfo->isPointer() && !variableInfo->isArray())) + invalidPrintfArgTypeError_s(tok, numFormat); + break; + case 'n': + if ((varTypeTok && isKnownType(variableInfo, varTypeTok) && ((!variableInfo->isPointer() && !variableInfo->isArray()) || varTypeTok->str() == "const")) || Token::Match(argListTok, "%str%")) + invalidPrintfArgTypeError_n(tok, numFormat); + break; + case 'c': + case 'd': + case 'i': + case 'u': + case 'x': + case 'X': + case 'o': + if (varTypeTok && varTypeTok->str() == "const") + varTypeTok = varTypeTok->next(); + if ((varTypeTok && isKnownType(variableInfo, varTypeTok) && !Token::Match(varTypeTok, "unsigned|signed| bool|short|long|int|char|size_t|unsigned|signed") && !variableInfo->isPointer() && !variableInfo->isArray())) + invalidPrintfArgTypeError_int(tok, numFormat, *i); + else if (Token::Match(argListTok, "%str%")) + invalidPrintfArgTypeError_int(tok, numFormat, *i); + break; + case 'p': + if (varTypeTok && varTypeTok->str() == "const") + varTypeTok = varTypeTok->next(); + if (varTypeTok && isKnownType(variableInfo, varTypeTok) && !Token::Match(varTypeTok, "unsigned|signed| short|long|int|size_t|unsigned|signed") && !variableInfo->isPointer() && !variableInfo->isArray()) + invalidPrintfArgTypeError_p(tok, numFormat); + else if (Token::Match(argListTok, "%str%")) + invalidPrintfArgTypeError_p(tok, numFormat); + break; + case 'e': + case 'E': + case 'f': + case 'g': + case 'G': + if (varTypeTok && varTypeTok->str() == "const") + varTypeTok = varTypeTok->next(); + if (varTypeTok && (isKnownType(variableInfo, varTypeTok) && !Token::Match(varTypeTok, "float|double") || variableInfo->isPointer() || variableInfo->isArray())) + invalidPrintfArgTypeError_float(tok, numFormat, *i); + else if (Token::Match(argListTok, "%str%")) + invalidPrintfArgTypeError_float(tok, numFormat, *i); + break; + default: + break; + } + } + } + + if (argListTok) + argListTok = argListTok->nextArgument(); // Find next argument + } } } // Count printf/scanf parameters.. unsigned int numFunction = 0; - while (argListTok) { + while (argListTok2) { numFunction++; - argListTok = argListTok->nextArgument(); // Find next argument + argListTok2 = argListTok2->nextArgument(); // Find next argument } // Mismatching number of parameters => warning @@ -1356,7 +1446,44 @@ void CheckOther::wrongPrintfScanfArgumentsError(const Token* tok, << numFunction << " are given"; - reportError(tok, severity, "wrongPrintfScanfArgs", errmsg.str()); + reportError(tok, severity, "wrongPrintfScanfArgNum", errmsg.str()); +} + +void CheckOther::invalidScanfArgTypeError(const Token* tok, const std::string &functionName, unsigned int numFormat) +{ + std::ostringstream errmsg; + errmsg << functionName << " argument no. " << numFormat << ": requires non-const pointers or arrays as arguments"; + reportError(tok, Severity::warning, "invalidScanfArgType", errmsg.str()); +} +void CheckOther::invalidPrintfArgTypeError_s(const Token* tok, unsigned int numFormat) +{ + std::ostringstream errmsg; + errmsg << "%s in format string (no. " << numFormat << ") requires a char* given in the argument list"; + reportError(tok, Severity::warning, "invalidPrintfArgType_s", errmsg.str()); +} +void CheckOther::invalidPrintfArgTypeError_n(const Token* tok, unsigned int numFormat) +{ + std::ostringstream errmsg; + errmsg << "%n in format string (no. " << numFormat << ") requires a pointer to an non-const integer given in the argument list"; + reportError(tok, Severity::warning, "invalidPrintfArgType_n", errmsg.str()); +} +void CheckOther::invalidPrintfArgTypeError_p(const Token* tok, unsigned int numFormat) +{ + std::ostringstream errmsg; + errmsg << "%p in format string (no. " << numFormat << ") requires an integer or pointer given in the argument list"; + reportError(tok, Severity::warning, "invalidPrintfArgType_p", errmsg.str()); +} +void CheckOther::invalidPrintfArgTypeError_int(const Token* tok, unsigned int numFormat, char c) +{ + std::ostringstream errmsg; + errmsg << "%" << c << " in format string (no. " << numFormat << ") requires an integer given in the argument list"; + reportError(tok, Severity::warning, "invalidPrintfArgType_int", errmsg.str()); +} +void CheckOther::invalidPrintfArgTypeError_float(const Token* tok, unsigned int numFormat, char c) +{ + std::ostringstream errmsg; + errmsg << "%" << c << " in format string (no. " << numFormat << ") requires a floating point number given in the argument list"; + reportError(tok, Severity::warning, "invalidPrintfArgType_float", errmsg.str()); } //--------------------------------------------------------------------------- diff --git a/lib/checkother.h b/lib/checkother.h index 9f6532160..93f59394b 100644 --- a/lib/checkother.h +++ b/lib/checkother.h @@ -179,6 +179,12 @@ public: const std::string &function, unsigned int numFormat, unsigned int numFunction); + void invalidScanfArgTypeError(const Token* tok, const std::string &functionName, unsigned int numFormat); + void invalidPrintfArgTypeError_s(const Token* tok, unsigned int numFormat); + void invalidPrintfArgTypeError_n(const Token* tok, unsigned int numFormat); + void invalidPrintfArgTypeError_p(const Token* tok, unsigned int numFormat); + void invalidPrintfArgTypeError_int(const Token* tok, unsigned int numFormat, char c); + void invalidPrintfArgTypeError_float(const Token* tok, unsigned int numFormat, char c); /** @brief %Check for assigning to the same variable twice in a switch statement*/ void checkRedundantAssignmentInSwitch(); @@ -357,6 +363,12 @@ public: c.comparisonOfBoolExpressionWithIntError(0); c.SuspiciousSemicolonError(0); c.wrongPrintfScanfArgumentsError(0,"printf",3,2); + c.invalidScanfArgTypeError(0, "scanf", 1); + c.invalidPrintfArgTypeError_s(0, 1); + c.invalidPrintfArgTypeError_n(0, 1); + c.invalidPrintfArgTypeError_p(0, 1); + c.invalidPrintfArgTypeError_int(0, 1, 'u'); + c.invalidPrintfArgTypeError_float(0, 1, 'f'); c.cctypefunctionCallError(0, "funname", "value"); } diff --git a/test/testother.cpp b/test/testother.cpp index a869e6091..26bbc76bc 100644 --- a/test/testother.cpp +++ b/test/testother.cpp @@ -2098,10 +2098,7 @@ private: " snprintf(str,10,\"%u%s\");\n" " sprintf(string1, \"%-*.*s\", 32, string2);\n" // #3364 " sprintf(string1, \"%*\", 32);\n" // #3364 - "}\n", - "test.cpp", - true - ); + "}"); ASSERT_EQUALS("[test.cpp:2]: (error) printf format string has 1 parameters but only 0 are given\n" "[test.cpp:3]: (error) printf format string has 2 parameters but only 1 are given\n" "[test.cpp:4]: (error) printf format string has 3 parameters but only 2 are given\n" @@ -2115,10 +2112,7 @@ private: " printf(\"\", 0);\n" " printf(\"%u\", 123, bar());\n" " printf(\"%u%s\", 0, bar(), 43123);\n" - "}\n", - "test.cpp", - true - ); + "}"); ASSERT_EQUALS("[test.cpp:2]: (warning) printf format string has 0 parameters but 1 are given\n" "[test.cpp:3]: (warning) printf format string has 1 parameters but 2 are given\n" "[test.cpp:4]: (warning) printf format string has 2 parameters but 3 are given\n", errout.str()); @@ -2135,11 +2129,83 @@ private: " fprintf(stderr, \"error: %m\n\");\n" // #3339 " printf(\"string: %.*s\n\", len, string);\n" // #3311 " fprintf(stderr, \"%*cText.\n\", indent, ' ');\n" // #3313 - "}\n", - "test.cpp", - true - ); + "}"); ASSERT_EQUALS("", errout.str()); + + check("void foo(char* s, const char* s2, std::string s3, int i) {\n" + " printf(\"%s%s\", s, s2);\n" + " printf(\"%s\", i);\n" + " printf(\"%i%s\", i, i);\n" + " printf(\"%s\", s3);\n" + " printf(\"%s\", \"s4\");\n" + " printf(\"%u\", s);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) %s in format string (no. 1) requires a char* given in the argument list\n" + "[test.cpp:4]: (warning) %s in format string (no. 2) requires a char* given in the argument list\n" + "[test.cpp:5]: (warning) %s in format string (no. 1) requires a char* given in the argument list\n", errout.str()); + + check("void foo(const int* cpi, const int ci, int i, int* pi, std::string s) {\n" + " printf(\"%n\", cpi);\n" + " printf(\"%n\", ci);\n" + " printf(\"%n\", i);\n" + " printf(\"%n\", pi);\n" + " printf(\"%n\", s);\n" + " printf(\"%n\", \"s4\");\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n" + "[test.cpp:3]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n" + "[test.cpp:4]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n" + "[test.cpp:6]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n" + "[test.cpp:7]: (warning) %n in format string (no. 1) requires a pointer to an non-const integer given in the argument list\n", errout.str()); + + check("class foo {};\n" + "void foo(const int* cpi, foo f, bar b, bar* bp, double d) {\n" + " printf(\"%i\", f);\n" + " printf(\"%c\", \"s4\");\n" + " printf(\"%o\", d);\n" + " printf(\"%i\", cpi);\n" + " printf(\"%u\", b);\n" + " printf(\"%u\", bp);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) %i in format string (no. 1) requires an integer given in the argument list\n" + "[test.cpp:4]: (warning) %c in format string (no. 1) requires an integer given in the argument list\n" + "[test.cpp:5]: (warning) %o in format string (no. 1) requires an integer given in the argument list\n", errout.str()); + + check("class foo {};\n" + "void foo(const int* cpi, foo f, bar b, bar* bp, char c) {\n" + " printf(\"%p\", f);\n" + " printf(\"%p\", c);\n" + " printf(\"%p\", bp);\n" + " printf(\"%p\", cpi);\n" + " printf(\"%p\", b);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) %p in format string (no. 1) requires an integer or pointer given in the argument list\n" + "[test.cpp:4]: (warning) %p in format string (no. 1) requires an integer or pointer given in the argument list\n", errout.str()); + + check("class foo {};\n" + "void foo(const int* cpi, foo f, bar b, bar* bp, double d) {\n" + " printf(\"%e\", f);\n" + " printf(\"%E\", \"s4\");\n" + " printf(\"%f\", cpi);\n" + " printf(\"%G\", bp);\n" + " printf(\"%f\", d);\n" + " printf(\"%f\", b);\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (warning) %e in format string (no. 1) requires a floating point number given in the argument list\n" + "[test.cpp:4]: (warning) %E in format string (no. 1) requires a floating point number given in the argument list\n" + "[test.cpp:5]: (warning) %f in format string (no. 1) requires a floating point number given in the argument list\n" + "[test.cpp:6]: (warning) %G in format string (no. 1) requires a floating point number given in the argument list\n", errout.str()); + + check("class foo;\n" + "void foo(foo f) {\n" + " printf(\"%u\", f);\n" + " printf(\"%f\", f);\n" + " printf(\"%p\", f);\n" + "}"); + TODO_ASSERT_EQUALS("[test.cpp:3]: (warning) %u in format string (no. 1) requires an integer given in the argument list\n" + "[test.cpp:4]: (warning) %f in format string (no. 1) requires an integer given in the argument list\n" + "[test.cpp:5]: (warning) %p in format string (no. 1) requires an integer given in the argument list\n", "", errout.str()); + } void trac1132() {