From 2de3ebcb1e0dca39ac2f3c496d8d1d10a0ed8543 Mon Sep 17 00:00:00 2001 From: Robert Reif Date: Tue, 24 Sep 2013 06:43:03 +0200 Subject: [PATCH] CheckIO: fixed some more false negatives. Ticket: #4964 --- lib/checkio.cpp | 192 ++++++++++++++++++++++++++++--------- lib/templatesimplifier.cpp | 2 +- lib/token.cpp | 42 ++++++++ lib/token.h | 2 + lib/tokenize.cpp | 3 +- lib/tokenlist.cpp | 5 +- test/testio.cpp | 26 +++++ 7 files changed, 222 insertions(+), 50 deletions(-) diff --git a/lib/checkio.cpp b/lib/checkio.cpp index 08defc59a..111a22383 100644 --- a/lib/checkio.cpp +++ b/lib/checkio.cpp @@ -848,17 +848,54 @@ void CheckIO::checkWrongPrintfScanfArguments() case 'X': case 'o': specifier += *i; - if (argInfo.typeToken->type() == Token::eString) { + if (argInfo.typeToken->type() == Token::eString) invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); - } else if (argInfo.isKnownType() && !argInfo.isArrayOrPointer()) { - if (!Token::Match(argInfo.typeToken, "bool|short|long|int|char")) { + else if (argInfo.isKnownType()) { + if (argInfo.isArrayOrPointer() && !argInfo.element) { + // use %p on pointers and arrays invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); - } else if ((specifier[0] == 'l' && (argInfo.typeToken->str() != "long" || (specifier[1] == 'l' && !argInfo.typeToken->isLong()))) || - (specifier.find("I64") != std::string::npos && (argInfo.typeToken->str() != "long" || !argInfo.typeToken->isLong()))) { + } else if (!Token::Match(argInfo.typeToken, "bool|short|long|int|char")) invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + else { + switch (specifier[0]) { + case 'l': + if (specifier[1] == 'l') { + if (argInfo.typeToken->str() != "long" || !argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + } else if (argInfo.typeToken->str() != "long" || argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + break; + case 'j': + if (!(argInfo.typeToken->originalName() == "intmax_t" || + argInfo.typeToken->originalName() == "uintmax_t")) + invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + break; + case 'z': + if (argInfo.typeToken->originalName() != "size_t") + invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + break; + case 't': + if (argInfo.typeToken->originalName() != "ptrdiff_t") + invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + break; + case 'I': + if (specifier.find("I64") != std::string::npos) { + if (argInfo.typeToken->str() != "long" || !argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + } else if (specifier.find("I32") != std::string::npos) { + if (argInfo.typeToken->str() != "int" || argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + } else if (!(argInfo.typeToken->originalName() != "size_t" || + argInfo.typeToken->originalName() != "ptrdiff_t")) + invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + break; + default: + if (!Token::Match(argInfo.typeToken, "bool|char|short|int")) + invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); + break; + } } - } else if ((!argInfo.element && argInfo.isArrayOrPointer()) || - (argInfo.element && !argInfo.isArrayOrPointer())) { + } else if (argInfo.isArrayOrPointer()) { // use %p on pointers and arrays invalidPrintfArgTypeError_int(tok, numFormat, specifier, &argInfo); } @@ -869,24 +906,55 @@ void CheckIO::checkWrongPrintfScanfArguments() specifier += *i; if (argInfo.typeToken->type() == Token::eString) { invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); - } else if (argInfo.isKnownType() && !argInfo.isArrayOrPointer()) { - if ((argInfo.typeToken->isUnsigned() || !Token::Match(argInfo.typeToken, "bool|short|long|int")) && !Token::Match(argInfo.typeToken, "char|short")) { + } else if (argInfo.isKnownType()) { + if (argInfo.isArrayOrPointer() && !argInfo.element) { + // use %p on pointers and arrays invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); - } else if ((specifier[0] == 'l' && (argInfo.typeToken->str() != "long" || (specifier[1] == 'l' && !argInfo.typeToken->isLong()))) || - (specifier.find("I64") != std::string::npos && (argInfo.typeToken->str() != "long" || !argInfo.typeToken->isLong()))) { - // %l requires long and %ll or %I64 requires long long - invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); - } else if ((specifier[0] == 't' || (specifier[0] == 'I' && (specifier[1] == 'd' || specifier[1] == 'i'))) && - argInfo.typeToken->originalName() != "ptrdiff_t") { - // use %t or %I on ptrdiff_t - invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); - } else if (argInfo.typeToken->originalName() == "ptrdiff_t" && - (specifier[0] != 't' && !(specifier[0] == 'I' && (specifier[1] == 'd' || specifier[1] == 'i')))) { - // ptrdiff_t requires %t or %I + } else if ((argInfo.typeToken->isUnsigned() || !Token::Match(argInfo.typeToken, "bool|short|long|int")) && !Token::Match(argInfo.typeToken, "char|short")) invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + else { + switch (specifier[0]) { + case 'l': + if (specifier[1] == 'l') { + if (argInfo.typeToken->str() != "long" || !argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + else if (argInfo.typeToken->originalName() == "ptrdiff_t" || + argInfo.typeToken->originalName() == "intmax_t") + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + } else if (argInfo.typeToken->str() != "long" || argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + else if (argInfo.typeToken->originalName() == "ptrdiff_t" || + argInfo.typeToken->originalName() == "intmax_t") + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + break; + case 'j': + if (argInfo.typeToken->originalName() != "intmax_t") + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + break; + case 't': + if (argInfo.typeToken->originalName() != "ptrdiff_t") + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + break; + case 'I': + if (specifier.find("I64") != std::string::npos) { + if (argInfo.typeToken->str() != "long" || !argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + } else if (specifier.find("I32") != std::string::npos) { + if (argInfo.typeToken->str() != "int" || argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + } else if (argInfo.typeToken->originalName() != "ptrdiff_t") + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + break; + default: + if (!Token::Match(argInfo.typeToken, "bool|char|short|int")) + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + else if (argInfo.typeToken->originalName() == "ptrdiff_t" || + argInfo.typeToken->originalName() == "intmax_t") + invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); + break; + } } - } else if ((!argInfo.element && argInfo.isArrayOrPointer()) || - (argInfo.element && !argInfo.isArrayOrPointer())) { + } else if (argInfo.isArrayOrPointer()) { // use %p on pointers and arrays invalidPrintfArgTypeError_sint(tok, numFormat, specifier, &argInfo); } @@ -896,23 +964,55 @@ void CheckIO::checkWrongPrintfScanfArguments() specifier += *i; if (argInfo.typeToken->type() == Token::eString) { invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); - } else if (argInfo.isKnownType() && !argInfo.isArrayOrPointer()) { - if ((!argInfo.typeToken->isUnsigned() || !Token::Match(argInfo.typeToken, "char|short|long|int")) && argInfo.typeToken->str() != "bool") { + } else if (argInfo.isKnownType()) { + if (argInfo.isArrayOrPointer() && !argInfo.element) { + // use %p on pointers and arrays invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); - } else if ((specifier[0] == 'l' && (argInfo.typeToken->str() != "long" || (specifier[1] == 'l' && !argInfo.typeToken->isLong()))) || - (specifier.find("I64") != std::string::npos && (argInfo.typeToken->str() != "long" || !argInfo.typeToken->isLong()))) { - // %l requires long and %ll or %I64 requires long long - invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); - } else if ((specifier[0] == 'z' || (specifier[0] == 'I' && specifier[1] == 'u')) && argInfo.typeToken->originalName() != "size_t") { - // use %z or %I on size_t - invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); - } else if (argInfo.typeToken->originalName() == "size_t" && (specifier[0] != 'z' && !(specifier[0] == 'I' && specifier[1] == 'u'))) { - // size_t requires %z or %I + } else if ((!argInfo.typeToken->isUnsigned() || !Token::Match(argInfo.typeToken, "char|short|long|int")) && argInfo.typeToken->str() != "bool") invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + else { + switch (specifier[0]) { + case 'l': + if (specifier[1] == 'l') { + if (argInfo.typeToken->str() != "long" || !argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + else if (argInfo.typeToken->originalName() == "size_t" || + argInfo.typeToken->originalName() == "uintmax_t") + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + } else if (argInfo.typeToken->str() != "long" || argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + else if (argInfo.typeToken->originalName() == "size_t" || + argInfo.typeToken->originalName() == "uintmax_t") + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + break; + case 'j': + if (argInfo.typeToken->originalName() != "uintmax_t") + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + break; + case 'z': + if (argInfo.typeToken->originalName() != "size_t") + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + break; + case 'I': + if (specifier.find("I64") != std::string::npos) { + if (argInfo.typeToken->str() != "long" || !argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + } else if (specifier.find("I32") != std::string::npos) { + if (argInfo.typeToken->str() != "int" || argInfo.typeToken->isLong()) + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + } else if (argInfo.typeToken->originalName() != "size_t") + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + break; + default: + if (!Token::Match(argInfo.typeToken, "bool|char|short|int")) + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + else if (argInfo.typeToken->originalName() == "size_t" || + argInfo.typeToken->originalName() == "intmax_t") + invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); + break; + } } - } else if ((!argInfo.element && argInfo.isArrayOrPointer()) || - (argInfo.element && !argInfo.isArrayOrPointer())) { - // use %p on pointers and arrays + } else if (argInfo.isArrayOrPointer()) { invalidPrintfArgTypeError_uint(tok, numFormat, specifier, &argInfo); } done = true; @@ -932,15 +1032,17 @@ void CheckIO::checkWrongPrintfScanfArguments() specifier += *i; if (argInfo.typeToken->type() == Token::eString) invalidPrintfArgTypeError_float(tok, numFormat, specifier, &argInfo); - else if (argInfo.isKnownType() && (!argInfo.isArrayOrPointer() || argInfo.element)) { - if (!Token::Match(argInfo.typeToken, "float|double")) { + else if (argInfo.isKnownType()) { + if (argInfo.isArrayOrPointer() && !argInfo.element) { + // use %p on pointers and arrays invalidPrintfArgTypeError_float(tok, numFormat, specifier, &argInfo); - } else if ((specifier[0] == 'L' && (!argInfo.typeToken->isLong() || argInfo.typeToken->str() != "double")) || - (specifier[0] != 'L' && argInfo.typeToken->isLong())) { + } else if (!Token::Match(argInfo.typeToken, "float|double")) invalidPrintfArgTypeError_float(tok, numFormat, specifier, &argInfo); - } - } else if ((!argInfo.element && argInfo.isArrayOrPointer()) || - (argInfo.element && !argInfo.isArrayOrPointer())) { + else if ((specifier[0] == 'L' && (!argInfo.typeToken->isLong() || argInfo.typeToken->str() != "double")) || + (specifier[0] != 'L' && argInfo.typeToken->isLong())) + invalidPrintfArgTypeError_float(tok, numFormat, specifier, &argInfo); + + } else if (argInfo.isArrayOrPointer()) { // use %p on pointers and arrays invalidPrintfArgTypeError_float(tok, numFormat, specifier, &argInfo); } @@ -1252,10 +1354,8 @@ bool CheckIO::ArgumentInfo::isKnownType() const return (typeToken->isStandardType() || typeToken->next()->isStandardType() || isComplexType()); else if (functionInfo) return (typeToken->isStandardType() || functionInfo->retType); - else - return typeToken->isStandardType() || Token::Match(typeToken, "std :: string|wstring"); - return false; + return typeToken->isStandardType() || Token::Match(typeToken, "std :: string|wstring"); } void CheckIO::wrongPrintfScanfArgumentsError(const Token* tok, diff --git a/lib/templatesimplifier.cpp b/lib/templatesimplifier.cpp index 665cf7eee..9504d3db1 100644 --- a/lib/templatesimplifier.cpp +++ b/lib/templatesimplifier.cpp @@ -541,7 +541,7 @@ void TemplateSimplifier::useDefaultArgumentValues(const std::list &temp const Token *from = (*it)->next(); std::stack links; while (from && (!links.empty() || (from->str() != "," && from->str() != ">"))) { - tok->insertToken(from->str()); + tok->insertToken(from->str(), from->originalName()); tok = tok->next(); if (Token::Match(tok, "(|[")) links.push(tok); diff --git a/lib/token.cpp b/lib/token.cpp index eaea508b7..e37391b2a 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -204,6 +204,7 @@ void Token::deleteThis() _scope = _next->_scope; _function = _next->_function; _variable = _next->_variable; + _originalName = _next->_originalName; if (_link) _link->link(this); @@ -952,6 +953,47 @@ void Token::insertToken(const std::string &tokenStr, bool prepend) } } +void Token::insertToken(const std::string &tokenStr, const std::string &originalNameStr, bool prepend) +{ + Token *newToken; + + //TODO: Find a solution for the first token on the list + if (prepend && !this->previous()) + return; + + if (_str.empty()) + newToken = this; + else + newToken = new Token(tokensBack); + newToken->str(tokenStr); + newToken->_originalName = originalNameStr; + newToken->_linenr = _linenr; + newToken->_fileIndex = _fileIndex; + newToken->_progressValue = _progressValue; + + if (newToken != this) { + if (prepend) { + /*if (this->previous())*/ { + newToken->previous(this->previous()); + newToken->previous()->next(newToken); + } /*else if (tokensFront?) { + *tokensFront? = newToken; + }*/ + this->previous(newToken); + newToken->next(this); + } else { + if (this->next()) { + newToken->next(this->next()); + newToken->next()->previous(newToken); + } else if (tokensBack) { + *tokensBack = newToken; + } + this->next(newToken); + newToken->previous(this); + } + } +} + void Token::eraseTokens(Token *begin, const Token *end) { if (!begin || begin == end) diff --git a/lib/token.h b/lib/token.h index 4f913868f..86e8ab084 100644 --- a/lib/token.h +++ b/lib/token.h @@ -355,6 +355,8 @@ public: */ void insertToken(const std::string &tokenStr, bool prepend=false); + void insertToken(const std::string &tokenStr, const std::string &originalNameStr, bool prepend=false); + Token *previous() const { return _previous; } diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index b652aebc8..d2204cd4a 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -5418,6 +5418,7 @@ void Tokenizer::simplifyPlatformTypes() tok->originalName(tok->str()); tok->str("char"); } else if (Token::Match(tok, "DWORD|ULONG|COLORREF|LCID|LCTYPE|LGRPID")) { + tok->originalName(tok->str()); tok->str("long"); tok->isUnsigned(true); } else if (Token::Match(tok, "DWORD_PTR|ULONG_PTR|SIZE_T")) { @@ -9919,7 +9920,7 @@ void Tokenizer::printUnknownTypes() if (!unknowns.empty()) { std::multimap::const_iterator it; std::string last; - size_t count; + size_t count = 0; for (it = unknowns.begin(); it != unknowns.end(); ++it) { // skip types is std namespace because they are not interesting diff --git a/lib/tokenlist.cpp b/lib/tokenlist.cpp index 311562cf3..d9b172126 100644 --- a/lib/tokenlist.cpp +++ b/lib/tokenlist.cpp @@ -116,11 +116,12 @@ void TokenList::addtoken(const Token * tok, const unsigned int lineno, const uns return; if (_back) { - _back->insertToken(tok->str()); + _back->insertToken(tok->str(), tok->originalName()); } else { _front = new Token(&_back); _back = _front; _back->str(tok->str()); + _back->originalName(tok->originalName()); } _back->linenr(lineno); @@ -139,7 +140,7 @@ void TokenList::insertTokens(Token *dest, const Token *src, unsigned int n) std::stack link; while (n > 0) { - dest->insertToken(src->str()); + dest->insertToken(src->str(), src->originalName()); dest = dest->next(); // Set links diff --git a/test/testio.cpp b/test/testio.cpp index 78aede7cf..1a880ae1c 100644 --- a/test/testio.cpp +++ b/test/testio.cpp @@ -2003,6 +2003,32 @@ private: "[test.cpp:6]: (warning) %p in format string (no. 2) requires an address but the argument type is 'char'.\n" "[test.cpp:7]: (warning) %p in format string (no. 2) requires an address but the argument type is 'char'.\n", errout.str()); + check("template \n" + "struct buffer {\n" + " size_t size();\n" + "};\n" + "buffer b;\n" + "void foo() {\n" + " printf(\"%u\", b.size());\n" + "}\n", false, false, Settings::Unix64); + ASSERT_EQUALS("[test.cpp:7]: (warning) %u in format string (no. 1) requires an unsigned integer but the argument type is 'size_t {aka unsigned long}'.\n", errout.str()); + + check("DWORD a;\n" + "DWORD_PTR b;\n" + "void foo() {\n" + " printf(\"%u %u\", a, b);\n" + "}\n", false, false, Settings::Win32A); + ASSERT_EQUALS("[test.cpp:4]: (warning) %u in format string (no. 1) requires an unsigned integer but the argument type is 'DWORD {aka unsigned long}'.\n" + "[test.cpp:4]: (warning) %u in format string (no. 2) requires an unsigned integer but the argument type is 'DWORD_PTR {aka unsigned long}'.\n", errout.str()); + + check("unsigned long a[] = { 1, 2 };\n" + "void foo() {\n" + " printf(\"%d %d %x \", a[0], a[0], a[0]);\n" + "}\n", false, false, Settings::Win32A); + ASSERT_EQUALS("[test.cpp:3]: (warning) %d in format string (no. 1) requires a signed integer but the argument type is 'unsigned long'.\n" + "[test.cpp:3]: (warning) %d in format string (no. 2) requires a signed integer but the argument type is 'unsigned long'.\n" + "[test.cpp:3]: (warning) %x in format string (no. 3) requires an integer but the argument type is 'unsigned long'.\n", errout.str()); + } void testPosixPrintfScanfParameterPosition() { // #4900 - No support for parameters in format strings