From 9e50b7cb68a37a874aaa7e34ce072d94da39dc6f Mon Sep 17 00:00:00 2001 From: Daniel Marjamaki Date: Sun, 23 Oct 2011 19:17:29 +0200 Subject: [PATCH 01/16] Preprocessor: updates to 'normal' preprocessing --- lib/preprocessor.cpp | 17 ++++++++++------- lib/preprocessor.h | 2 +- test/testpreprocessor.cpp | 26 ++++++++++++++------------ 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 7e4717564..a2fa46d3d 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -1701,11 +1701,14 @@ static bool openHeader(std::string &filename, const std::list &incl } -std::string Preprocessor::handleIncludes(const std::string &code, const std::string &filePath, const std::list &includePaths, std::map &defs) +std::string Preprocessor::handleIncludes(const std::string &code, const std::string &filePath, const std::list &includePaths, std::map &defs) { const std::string path(filePath.substr(0, 1 + filePath.find_last_of("\\/"))); + // current #if indent level. unsigned int indent = 0; + + // how deep does the #if match? this can never be bigger than "indent". unsigned int indentmatch = 0; std::ostringstream ostr; @@ -1739,6 +1742,10 @@ std::string Preprocessor::handleIncludes(const std::string &code, const std::str if (indent == indentmatch && defs.find(getdef(line,false)) == defs.end()) indentmatch++; ++indent; + } else if (line.compare(0,4,"#if ") == 0) { + if (indent == indentmatch && match_cfg_def(defs, line.substr(4))) + indentmatch++; + ++indent; } else if (line.compare(0,5,"#else") == 0) { if (indentmatch == indent) indentmatch = indent - 1; @@ -1752,16 +1759,12 @@ std::string Preprocessor::handleIncludes(const std::string &code, const std::str if (line.compare(0,8,"#define ")==0) { // no value if (line.find_first_of("( ", 8) == std::string::npos) - defs[line.substr(8)] = 1; + defs[line.substr(8)] = ""; // define value else if (line.find("(") == std::string::npos) { const std::string::size_type pos = line.find(" ", 8); - const std::string val(line.substr(pos + 1)); - int i; - std::istringstream istr2(val); - istr2 >> i; - defs[line.substr(8,pos-8)] = i; + defs[line.substr(8,pos-8)] = line.substr(pos+1); } } diff --git a/lib/preprocessor.h b/lib/preprocessor.h index 34b16a24b..a89d21552 100644 --- a/lib/preprocessor.h +++ b/lib/preprocessor.h @@ -217,7 +217,7 @@ public: * @param defs defines (only values) * \return resulting string */ - std::string handleIncludes(const std::string &code, const std::string &filePath, const std::list &includePaths, std::map &defs); + std::string handleIncludes(const std::string &code, const std::string &filePath, const std::list &includePaths, std::map &defs); private: void missingInclude(const std::string &filename, unsigned int linenr, const std::string &header, bool userheader); diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index 3aa8323db..c05f7c0fc 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -2827,13 +2827,13 @@ private: void handleIncludes_def() { const std::string filePath("test.c"); const std::list includePaths; - std::map defs; + std::map defs; Preprocessor preprocessor; // ifdef { defs.clear(); - defs["A"] = 1; + defs["A"] = ""; { const std::string code("#ifdef A\n123\n#endif\n"); const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); @@ -2848,7 +2848,7 @@ private: // ifndef { defs.clear(); - defs["A"] = 1; + defs["A"] = ""; { const std::string code("#ifndef A\n123\n#endif\n"); const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); @@ -2868,15 +2868,17 @@ private: const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); ASSERT_EQUALS("\n\n123\n\n" "\n\n\n\n", actual); } - /* - // define X 123 - #if - { - defs.clear(); - const std::string code("#define X 123\n#if X==123\n456\n#endif\n"); - const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); - ASSERT_EQUALS("\n\n456\n\n", actual); - } - */ + + // define X 123 - #if + { + defs.clear(); + const std::string code("#define X 123\n" + "#if X==123\n" + "456\n" + "#endif\n"); + const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); + ASSERT_EQUALS("\n\n456\n\n", actual); + } } }; From 5b97cc14403079e0a3d0862c2a4d73d7fcef34c7 Mon Sep 17 00:00:00 2001 From: Thomas Jarosch Date: Sun, 23 Oct 2011 20:38:03 +0200 Subject: [PATCH 02/16] Bugfix: Update token properties on string changes --- lib/token.cpp | 16 +++++++++++--- lib/token.h | 4 ++++ test/testtoken.cpp | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/lib/token.cpp b/lib/token.cpp index 9cb9453b8..5bcba4de0 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -54,10 +54,8 @@ Token::~Token() } -void Token::str(const std::string &s) +void Token::update_property_info() { - _str = s; - if (!_str.empty()) { _isName = bool(_str[0] == '_' || std::isalpha(_str[0])); @@ -72,15 +70,27 @@ void Token::str(const std::string &s) _isBoolean = true; else _isBoolean = false; + } else { + _isName = false; + _isNumber = false; + _isBoolean = false; } +} +void Token::str(const std::string &s) +{ + _str = s; _varId = 0; + + update_property_info(); } void Token::concatStr(std::string const& b) { _str.erase(_str.length() - 1); _str.append(b.begin() + 1, b.end()); + + update_property_info(); } std::string Token::strValue() const diff --git a/lib/token.h b/lib/token.h index 605c0f596..02c05a3b3 100644 --- a/lib/token.h +++ b/lib/token.h @@ -440,6 +440,10 @@ private: unsigned int _fileIndex; unsigned int _linenr; + /** Updates internal property cache like _isName or _isBoolean. + Called after any _str() modification. */ + void update_property_info(); + /** * A value from 0-100 that provides a rough idea about where in the token * list this token is located. diff --git a/test/testtoken.cpp b/test/testtoken.cpp index caca72443..991ab129c 100644 --- a/test/testtoken.cpp +++ b/test/testtoken.cpp @@ -46,6 +46,13 @@ private: TEST_CASE(matchNumeric); TEST_CASE(matchBoolean); TEST_CASE(matchOr); + + TEST_CASE(updateProperties) + TEST_CASE(isNameGuarantees1) + TEST_CASE(isNameGuarantees2) + TEST_CASE(isNameGuarantees3) + TEST_CASE(isNameGuarantees4) + TEST_CASE(isNameGuarantees5) } void nextprevious() { @@ -253,6 +260,51 @@ private: givenACodeSampleToTokenize op("+"); ASSERT_EQUALS(true, Token::Match(op.tokens(), "%op%")); } + + void updateProperties() { + Token tok(NULL); + tok.str("foobar"); + + ASSERT_EQUALS(true, tok.isName()); + ASSERT_EQUALS(false, tok.isNumber()); + + tok.str("123456"); + + ASSERT_EQUALS(false, tok.isName()); + ASSERT_EQUALS(true, tok.isNumber()); + } + + void isNameGuarantees1() { + Token tok(NULL); + tok.str("Name"); + ASSERT_EQUALS(true, tok.isName()); + } + + void isNameGuarantees2() { + Token tok(NULL); + tok.str("_name"); + ASSERT_EQUALS(true, tok.isName()); + } + + void isNameGuarantees3() { + Token tok(NULL); + tok.str("_123"); + ASSERT_EQUALS(true, tok.isName()); + } + + void isNameGuarantees4() { + Token tok(NULL); + tok.str("123456"); + ASSERT_EQUALS(false, tok.isName()); + ASSERT_EQUALS(true, tok.isNumber()); + } + + void isNameGuarantees5() { + Token tok(NULL); + tok.str("a123456"); + ASSERT_EQUALS(true, tok.isName()); + ASSERT_EQUALS(false, tok.isNumber()); + } }; REGISTER_TEST(TestToken) From 1ccb57e5953d12f49dd01dfce3faea9d18dd5d66 Mon Sep 17 00:00:00 2001 From: Thomas Jarosch Date: Sun, 23 Oct 2011 21:21:42 +0200 Subject: [PATCH 03/16] Document and test Token::concatStr() --- lib/token.h | 4 ++++ test/testtoken.cpp | 13 +++++++++++++ 2 files changed, 17 insertions(+) diff --git a/lib/token.h b/lib/token.h index 02c05a3b3..6927d7dec 100644 --- a/lib/token.h +++ b/lib/token.h @@ -48,6 +48,10 @@ public: void str(const std::string &s); + /** + * Concatenate two strings. "hello" "world" -> "hello world" + * Note: Automatically cuts of the last/first character. + */ void concatStr(std::string const& b); const std::string &str() const { diff --git a/test/testtoken.cpp b/test/testtoken.cpp index 991ab129c..7044538e5 100644 --- a/test/testtoken.cpp +++ b/test/testtoken.cpp @@ -48,6 +48,7 @@ private: TEST_CASE(matchOr); TEST_CASE(updateProperties) + TEST_CASE(updatePropertiesConcatStr) TEST_CASE(isNameGuarantees1) TEST_CASE(isNameGuarantees2) TEST_CASE(isNameGuarantees3) @@ -274,6 +275,18 @@ private: ASSERT_EQUALS(true, tok.isNumber()); } + void updatePropertiesConcatStr() { + Token tok(NULL); + tok.str("true"); + + ASSERT_EQUALS(true, tok.isBoolean()); + + tok.concatStr("123"); + + ASSERT_EQUALS(false, tok.isBoolean()); + ASSERT_EQUALS("tru23", tok.str()); + } + void isNameGuarantees1() { Token tok(NULL); tok.str("Name"); From e63adfaaf83a3cde2fe618d330fb05932f223752 Mon Sep 17 00:00:00 2001 From: Reijo Tomperi Date: Sun, 23 Oct 2011 23:13:03 +0300 Subject: [PATCH 04/16] Update codeblocks project file --- cppcheck.cbp | 56 +++++++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/cppcheck.cbp b/cppcheck.cbp index c17789ed4..ccb1c5d78 100644 --- a/cppcheck.cbp +++ b/cppcheck.cbp @@ -35,10 +35,6 @@ - - - - @@ -47,6 +43,8 @@ + + @@ -64,13 +62,13 @@ - - + + @@ -85,17 +83,13 @@ + + - - - - - - @@ -112,20 +106,24 @@ + + + + + + - - - - + + @@ -140,19 +138,14 @@ - + + - - - - - - @@ -161,6 +154,9 @@ + + + @@ -173,14 +169,16 @@ + + + - @@ -189,6 +187,7 @@ + @@ -197,10 +196,7 @@ - - - @@ -215,12 +211,6 @@ - - - - - - From 55a079a7c4053649fc585a52d697533805f23467 Mon Sep 17 00:00:00 2001 From: Thomas Jarosch Date: Sun, 23 Oct 2011 22:46:55 +0200 Subject: [PATCH 05/16] Clarify Token::concatStr() documentation --- lib/token.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/token.h b/lib/token.h index 6927d7dec..37d8a3979 100644 --- a/lib/token.h +++ b/lib/token.h @@ -49,8 +49,8 @@ public: void str(const std::string &s); /** - * Concatenate two strings. "hello" "world" -> "hello world" - * Note: Automatically cuts of the last/first character. + * Concatenate two (quoted) strings. Automatically cuts of the last/first character. + * Example: "hello ""world" -> "hello world". Used by the token simplifier. */ void concatStr(std::string const& b); From 36ef8e771fe80833e34ca5e8c10acb42a677c160 Mon Sep 17 00:00:00 2001 From: Reijo Tomperi Date: Mon, 24 Oct 2011 00:36:57 +0300 Subject: [PATCH 06/16] Improve null pointer dereference test coverage --- test/testnullpointer.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index c6c938e56..a156e616b 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -53,6 +53,7 @@ private: TEST_CASE(snprintf_with_non_zero_size); TEST_CASE(printf_with_invalid_va_argument); TEST_CASE(scanf_with_invalid_va_argument); + TEST_CASE(nullpointer_in_return); } void check(const char code[], bool inconclusive = false, bool cpp11 = false) { @@ -175,6 +176,19 @@ private: check(code, true); ASSERT_EQUALS("[test.cpp:5]: (error) Possible null pointer dereference: tok - otherwise it is redundant to check if tok is null at line 3\n", errout.str()); } + + check("int foo(const Token *tok)\n" + "{\n" + " while (tok){;}\n" + "}\n", true); + ASSERT_EQUALS("", errout.str()); + + check("int foo(const Token *tok)\n" + "{\n" + " while (tok){;}\n" + " char a[2] = {0,0};\n" + "}\n", true); + ASSERT_EQUALS("", errout.str()); } void nullpointer1() { @@ -1479,6 +1493,20 @@ private: "}"); ASSERT_EQUALS("[test.cpp:3]: (error) Possible null pointer dereference: iVal\n", errout.str()); } + + void nullpointer_in_return() { + check("int foo() {\n" + " int* iVal = 0;\n" + " if(g()) iVal = g();\n" + " return iVal[0];\n" + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Possible null pointer dereference: iVal\n", errout.str()); + + check("int foo(int* iVal) {\n" + " return iVal[0];\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestNullPointer) From df5d26901cabc0b64c96082dce39020ce5bc3617 Mon Sep 17 00:00:00 2001 From: Edoardo Prezioso Date: Mon, 24 Oct 2011 02:52:55 +0200 Subject: [PATCH 07/16] Add new warning option to check for dead code and change the order of some struct members to reduce structure padding. --- Makefile | 2 +- lib/token.cpp | 10 +++++----- lib/token.h | 10 ++++++---- lib/tokenize.cpp | 47 +++++++++++++++++------------------------------ lib/tokenize.h | 24 ++++++++++++------------ tools/dmake.cpp | 21 +++++++++++---------- 6 files changed, 52 insertions(+), 62 deletions(-) diff --git a/Makefile b/Makefile index e04b0acdb..0772a2724 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ ifndef HAVE_RULES endif ifndef CXXFLAGS - CXXFLAGS=-Wall -Wextra -Wshadow -pedantic -Wno-long-long -Wfloat-equal -Wcast-qual -Woverloaded-virtual -Wsign-promo -Wabi -Winline -Wredundant-decls -Wpacked -Wmissing-format-attribute -Wmissing-declarations -D_GLIBCXX_DEBUG -g + CXXFLAGS=-pedantic -Wall -Wextra -Wabi -Wcast-qual -Wfloat-equal -Winline -Wmissing-declarations -Wmissing-format-attribute -Wno-long-long -Woverloaded-virtual -Wpacked -Wredundant-decls -Wshadow -Wsign-promo -Wunreachable-code -D_GLIBCXX_DEBUG -g endif ifeq ($(HAVE_RULES),yes) diff --git a/lib/token.cpp b/lib/token.cpp index 5bcba4de0..fd42abb6a 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -30,7 +30,9 @@ Token::Token(Token **t) : tokensBack(t), - _str(""), + _next(0), + _previous(0), + _link(0), _isName(false), _isNumber(false), _isBoolean(false), @@ -40,12 +42,10 @@ Token::Token(Token **t) : _isLong(false), _isUnused(false), _varId(0), - _next(0), - _previous(0), - _link(0), _fileIndex(0), _linenr(0), - _progressValue(0) + _progressValue(0), + _str("") { } diff --git a/lib/token.h b/lib/token.h index 37d8a3979..581911b27 100644 --- a/lib/token.h +++ b/lib/token.h @@ -428,7 +428,10 @@ private: static int firstWordLen(const char *str); - std::string _str; + Token *_next; + Token *_previous; + Token *_link; + bool _isName; bool _isNumber; bool _isBoolean; @@ -438,9 +441,6 @@ private: bool _isLong; bool _isUnused; unsigned int _varId; - Token *_next; - Token *_previous; - Token *_link; unsigned int _fileIndex; unsigned int _linenr; @@ -453,6 +453,8 @@ private: * list this token is located. */ unsigned int _progressValue; + + std::string _str; }; /// @} diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index 92cfca2ae..779dde147 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -42,41 +42,28 @@ //--------------------------------------------------------------------------- -Tokenizer::Tokenizer() - : _settings(0), _errorLogger(0) +Tokenizer::Tokenizer() : + _tokens(0), //no tokens to start with + _tokensBack(0), + _settings(0), + _errorLogger(0), + _symbolDatabase(0), + _varId(0), + _codeWithTemplates(false) //is there any templates? { - // No tokens to start with - _tokens = 0; - _tokensBack = 0; - - // is there any templates? - _codeWithTemplates = false; - - // symbol database - _symbolDatabase = NULL; - - // variable count - _varId = 0; } -Tokenizer::Tokenizer(const Settings *settings, ErrorLogger *errorLogger) - : _settings(settings), _errorLogger(errorLogger) +Tokenizer::Tokenizer(const Settings *settings, ErrorLogger *errorLogger) : + _tokens(0), //no tokens to start with + _tokensBack(0), + _settings(settings), + _errorLogger(errorLogger), + _symbolDatabase(0), + _varId(0), + _codeWithTemplates(false) //is there any templates? { // make sure settings are specified assert(_settings); - - // No tokens to start with - _tokens = 0; - _tokensBack = 0; - - // is there any templates? - _codeWithTemplates = false; - - // symbol database - _symbolDatabase = NULL; - - // variable count - _varId = 0; } Tokenizer::~Tokenizer() @@ -738,9 +725,9 @@ Token * Tokenizer::deleteInvalidTypedef(Token *typeDef) } struct Space { - bool isNamespace; std::string className; const Token * classEnd; + bool isNamespace; }; static Token *splitDefinitionFromTypedef(Token *tok) diff --git a/lib/tokenize.h b/lib/tokenize.h index f7dad0fe1..389302580 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -728,33 +728,33 @@ private: /** Token list */ Token *_tokens, *_tokensBack; - /** sizeof information for known types */ - std::map _typeSize; - - /** filenames for the tokenized source code (source + included) */ - std::vector _files; - /** settings */ const Settings * _settings; /** errorlogger */ ErrorLogger * const _errorLogger; + /** Symbol database that all checks etc can use */ + mutable SymbolDatabase *_symbolDatabase; + /** E.g. "A" for code where "#ifdef A" is true. This is used to print additional information in error situations. */ std::string _configuration; + /** sizeof information for known types */ + std::map _typeSize; + + /** filenames for the tokenized source code (source + included) */ + std::vector _files; + + /** variable count */ + unsigned int _varId; + /** * was there any templates? templates that are "unused" are * removed from the token list */ bool _codeWithTemplates; - - /** Symbol database that all checks etc can use */ - mutable SymbolDatabase *_symbolDatabase; - - /** variable count */ - unsigned int _varId; }; /// @} diff --git a/tools/dmake.cpp b/tools/dmake.cpp index 96512250b..0fde96654 100644 --- a/tools/dmake.cpp +++ b/tools/dmake.cpp @@ -216,21 +216,22 @@ int main(int argc, char **argv) // The _GLIBCXX_DEBUG doesn't work in cygwin makeConditionalVariable(fout, "CXXFLAGS", + "-pedantic " "-Wall " "-Wextra " - "-Wshadow " - "-pedantic " - "-Wno-long-long " - "-Wfloat-equal " - "-Wcast-qual " - "-Woverloaded-virtual " - "-Wsign-promo " "-Wabi " + "-Wcast-qual " + "-Wfloat-equal " "-Winline " - "-Wredundant-decls " - "-Wpacked " - "-Wmissing-format-attribute " "-Wmissing-declarations " + "-Wmissing-format-attribute " + "-Wno-long-long " + "-Woverloaded-virtual " + "-Wpacked " + "-Wredundant-decls " + "-Wshadow " + "-Wsign-promo " + "-Wunreachable-code " // "-Wsign-conversion " // "-Wconversion " "-D_GLIBCXX_DEBUG " From 17aea0a997ea1262361ce49fc7eabdd151187453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 24 Oct 2011 07:09:14 +0200 Subject: [PATCH 08/16] dmake: disabled -Wunreachable-code because there was too many warnings --- Makefile | 2 +- tools/dmake.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 0772a2724..55ff9ba17 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ ifndef HAVE_RULES endif ifndef CXXFLAGS - CXXFLAGS=-pedantic -Wall -Wextra -Wabi -Wcast-qual -Wfloat-equal -Winline -Wmissing-declarations -Wmissing-format-attribute -Wno-long-long -Woverloaded-virtual -Wpacked -Wredundant-decls -Wshadow -Wsign-promo -Wunreachable-code -D_GLIBCXX_DEBUG -g + CXXFLAGS=-pedantic -Wall -Wextra -Wabi -Wcast-qual -Wfloat-equal -Winline -Wmissing-declarations -Wmissing-format-attribute -Wno-long-long -Woverloaded-virtual -Wpacked -Wredundant-decls -Wshadow -Wsign-promo -D_GLIBCXX_DEBUG -g endif ifeq ($(HAVE_RULES),yes) diff --git a/tools/dmake.cpp b/tools/dmake.cpp index 0fde96654..bf33dbc35 100644 --- a/tools/dmake.cpp +++ b/tools/dmake.cpp @@ -231,7 +231,7 @@ int main(int argc, char **argv) "-Wredundant-decls " "-Wshadow " "-Wsign-promo " - "-Wunreachable-code " +// "-Wunreachable-code " // "-Wsign-conversion " // "-Wconversion " "-D_GLIBCXX_DEBUG " From 8f0248040c7ba4760a130d66ae9f379b5dee103b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 24 Oct 2011 07:17:52 +0200 Subject: [PATCH 09/16] webreport: updated scp command --- webreport.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webreport.sh b/webreport.sh index 5d28bb220..e7d9debe6 100755 --- a/webreport.sh +++ b/webreport.sh @@ -13,5 +13,5 @@ mv doxyoutput/html devinfo/doxyoutput # Detect duplicate code.. ~/pmd-4.2.5/bin/cpd.sh lib/ > devinfo/cpd.txt -#scp -r devinfo/ hyd_danmar,cppcheck@web.sourceforge.net:/home/groups/c/cp/cppcheck/htdocs +#scp -r devinfo/ danielmarjamaki,cppcheck@web.sourceforge.net:/home/groups/c/cp/cppcheck/htdocs From 3de70a72448fae4bfb6fe19df5561ae9059b3cc5 Mon Sep 17 00:00:00 2001 From: Daniel Marjamaki Date: Mon, 24 Oct 2011 07:37:47 +0200 Subject: [PATCH 10/16] Preprocessor: Better 'normal' preprocessing. Simple handling of '#elif' --- lib/preprocessor.cpp | 5 +++++ test/testpreprocessor.cpp | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index a2fa46d3d..8aa26161f 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -1746,6 +1746,11 @@ std::string Preprocessor::handleIncludes(const std::string &code, const std::str if (indent == indentmatch && match_cfg_def(defs, line.substr(4))) indentmatch++; ++indent; + } else if (line.compare(0,6,"#elif ") == 0) { + if (indentmatch == indent) + indentmatch = indent - 1; // TODO: Make sure all remaining #elif and #else are skipped + else if (indentmatch == indent - 1 && match_cfg_def(defs, line.substr(6))) + indentmatch = indent; } else if (line.compare(0,5,"#else") == 0) { if (indentmatch == indent) indentmatch = indent - 1; diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index c05f7c0fc..6f25660eb 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -2869,7 +2869,7 @@ private: ASSERT_EQUALS("\n\n123\n\n" "\n\n\n\n", actual); } - // define X 123 - #if + // #define => #if { defs.clear(); const std::string code("#define X 123\n" @@ -2879,6 +2879,19 @@ private: const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); ASSERT_EQUALS("\n\n456\n\n", actual); } + + // #elif + { + defs.clear(); + defs["B"] = ""; + const std::string code("#if defined(A)\n" + "123\n" + "#elif defined(B)\n" + "456\n" + "#endif"); + const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); + ASSERT_EQUALS("\n\n\n456\n\n", actual); + } } }; From 08ba378730df5a2eb2b0a039263fa8c674fdedd2 Mon Sep 17 00:00:00 2001 From: Daniel Marjamaki Date: Mon, 24 Oct 2011 08:11:44 +0200 Subject: [PATCH 11/16] Preprocessor: Improved 'normal' preprocessing. better handling of multiple #elif and #else blocks --- lib/preprocessor.cpp | 53 ++++++++++++++++++++++++++++----------- test/testpreprocessor.cpp | 32 ++++++++++++++++++----- 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 8aa26161f..24fba4648 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -1711,6 +1711,10 @@ std::string Preprocessor::handleIncludes(const std::string &code, const std::str // how deep does the #if match? this can never be bigger than "indent". unsigned int indentmatch = 0; + // has there been a true #if condition at the current indentmatch level? + // then no more #elif or #else can be true before the #endif is seen. + bool elseIsTrue = true; + std::ostringstream ostr; std::istringstream istr(code); std::string line; @@ -1735,31 +1739,52 @@ std::string Preprocessor::handleIncludes(const std::string &code, const std::str << handleIncludes(read(fin, filename, NULL), filename, includePaths, defs) << std::endl << "#endfile"; } else if (line.compare(0,7,"#ifdef ") == 0) { - if (indent == indentmatch && defs.find(getdef(line,true)) != defs.end()) + if (indent == indentmatch && defs.find(getdef(line,true)) != defs.end()) { + elseIsTrue = false; indentmatch++; + } ++indent; + + if (indent == indentmatch + 1) + elseIsTrue = true; } else if (line.compare(0,8,"#ifndef ") == 0) { - if (indent == indentmatch && defs.find(getdef(line,false)) == defs.end()) + if (indent == indentmatch && defs.find(getdef(line,false)) == defs.end()) { + elseIsTrue = false; indentmatch++; + } ++indent; + + if (indent == indentmatch + 1) + elseIsTrue = true; } else if (line.compare(0,4,"#if ") == 0) { - if (indent == indentmatch && match_cfg_def(defs, line.substr(4))) + if (indent == indentmatch && match_cfg_def(defs, line.substr(4))) { + elseIsTrue = false; indentmatch++; + } ++indent; - } else if (line.compare(0,6,"#elif ") == 0) { - if (indentmatch == indent) - indentmatch = indent - 1; // TODO: Make sure all remaining #elif and #else are skipped - else if (indentmatch == indent - 1 && match_cfg_def(defs, line.substr(6))) - indentmatch = indent; - } else if (line.compare(0,5,"#else") == 0) { - if (indentmatch == indent) - indentmatch = indent - 1; - else if (indentmatch == indent - 1) - indentmatch = indent; + + if (indent == indentmatch + 1) + elseIsTrue = true; + } else if (line.compare(0,6,"#elif ") == 0 || line.compare(0,5,"#else") == 0) { + if (!elseIsTrue) { + if (indentmatch == indent) + indentmatch = indent - 1; + } else { + if (indentmatch == indent) + indentmatch = indent - 1; + else if (indentmatch == indent - 1) { + if (line.compare(0,5,"#else")==0 || match_cfg_def(defs,line.substr(6))) { + indentmatch = indent; + elseIsTrue = false; + } + } + } } else if (line == "#endif") { --indent; - if (indentmatch > indent) + if (indentmatch > indent) { indentmatch = indent; + elseIsTrue = false; + } } else if (indentmatch == indent) { if (line.compare(0,8,"#define ")==0) { // no value diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index 6f25660eb..37c192c9f 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -2882,15 +2882,35 @@ private: // #elif { - defs.clear(); - defs["B"] = ""; const std::string code("#if defined(A)\n" - "123\n" + "1\n" "#elif defined(B)\n" - "456\n" + "2\n" + "#elif defined(C)\n" + "3\n" + "#else\n" + "4\n" "#endif"); - const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); - ASSERT_EQUALS("\n\n\n456\n\n", actual); + { + defs.clear(); + defs["A"] = ""; + defs["C"] = ""; + const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); + ASSERT_EQUALS("\n1\n\n\n\n\n\n\n\n", actual); + } + + { + defs.clear(); + defs["B"] = ""; + const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); + ASSERT_EQUALS("\n\n\n2\n\n\n\n\n\n", actual); + } + + { + defs.clear(); + const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); + ASSERT_EQUALS("\n\n\n\n\n\n\n4\n\n", actual); + } } } }; From 0eb4e3032ab21bb50bc59f62c53f8086f6bd94e8 Mon Sep 17 00:00:00 2001 From: Daniel Marjamaki Date: Mon, 24 Oct 2011 19:51:00 +0200 Subject: [PATCH 12/16] Preprocessor: handle '#undef' better. Ticket: #2131 --- lib/preprocessor.cpp | 4 ++++ test/testpreprocessor.cpp | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 24fba4648..660ae270b 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -1798,6 +1798,10 @@ std::string Preprocessor::handleIncludes(const std::string &code, const std::str } } + else if (line.compare(0,7,"#undef ") == 0) { + defs.erase(line.substr(7)); + } + else { ostr << line; } diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index 37c192c9f..d580af178 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -2912,6 +2912,22 @@ private: ASSERT_EQUALS("\n\n\n\n\n\n\n4\n\n", actual); } } + + // #undef + { + const std::string code("#ifndef X\n" + "#define X\n" + "123\n" + "#endif\n"); + + defs.clear(); + const std::string actual1(preprocessor.handleIncludes(code,filePath,includePaths,defs)); + + defs.clear(); + const std::string actual(preprocessor.handleIncludes(code + "#undef X\n" + code, filePath, includePaths, defs)); + + ASSERT_EQUALS(actual1 + "\n" + actual1, actual); + } } }; From 7fa58b455b8e5552257c74adc1ef9e54d5a14eb7 Mon Sep 17 00:00:00 2001 From: Daniel Marjamaki Date: Mon, 24 Oct 2011 20:59:46 +0200 Subject: [PATCH 13/16] Preprocessor: Make it possible to use the 'normal' preprocessor by using special command -DCPPCHECK-TEST. Ticket: #2131 --- lib/preprocessor.cpp | 47 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/lib/preprocessor.cpp b/lib/preprocessor.cpp index 660ae270b..a03816c72 100644 --- a/lib/preprocessor.cpp +++ b/lib/preprocessor.cpp @@ -854,13 +854,44 @@ void Preprocessor::preprocess(std::istream &srcCodeStream, std::string &processe processedFile = ostr.str(); } - handleIncludes(processedFile, filename, includePaths); + if (_settings && _settings->userDefines.compare(0,14,"CPPCHECK-TEST;") == 0) { + std::map defs; - processedFile = replaceIfDefined(processedFile); + // TODO: break out this code. There is other similar code. + std::string::size_type pos1 = 0; + while (pos1 != std::string::npos) { + const std::string::size_type pos2 = _settings->userDefines.find_first_of(";=", pos1); + const std::string::size_type pos3 = _settings->userDefines.find(";", pos1); - // Get all possible configurations.. - if (!_settings || (_settings && _settings->userDefines.empty())) - resultConfigurations = getcfgs(processedFile, filename); + std::string name, value; + if (pos2 == std::string::npos) + name = _settings->userDefines.substr(pos1); + else + name = _settings->userDefines.substr(pos1, pos2 - pos1); + if (pos2 != pos3) { + if (pos3 == std::string::npos) + value = _settings->userDefines.substr(pos2+1); + else + value = _settings->userDefines.substr(pos2+1, pos3 - pos2 - 1); + } + + defs[name] = value; + + pos1 = pos3; + if (pos1 != std::string::npos) + pos1++; + } + + processedFile = handleIncludes(processedFile, filename, includePaths, defs); + } else { + handleIncludes(processedFile, filename, includePaths); + + processedFile = replaceIfDefined(processedFile); + + // Get all possible configurations.. + if (!_settings || (_settings && _settings->userDefines.empty())) + resultConfigurations = getcfgs(processedFile, filename); + } } @@ -1735,7 +1766,7 @@ std::string Preprocessor::handleIncludes(const std::string &code, const std::str continue; } - ostr << "#file " << filename << "\n" + ostr << "#file \"" << filename << "\"\n" << handleIncludes(read(fin, filename, NULL), filename, includePaths, defs) << std::endl << "#endfile"; } else if (line.compare(0,7,"#ifdef ") == 0) { @@ -1802,9 +1833,7 @@ std::string Preprocessor::handleIncludes(const std::string &code, const std::str defs.erase(line.substr(7)); } - else { - ostr << line; - } + ostr << line; } // A line has been read.. From 3413ffef3e1a61d022ae73ef85ef1bee6b8be0a0 Mon Sep 17 00:00:00 2001 From: Thomas Jarosch Date: Mon, 24 Oct 2011 21:22:04 +0200 Subject: [PATCH 14/16] Refactor readlink() buffer check to also handle readlinkat() --- lib/checkbufferoverrun.cpp | 63 ++++++++++++++++++++++---------------- lib/checkbufferoverrun.h | 7 +++-- test/testbufferoverrun.cpp | 51 ++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 28 deletions(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index 8bec6e724..b1abc5342 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -107,10 +107,10 @@ void CheckBufferOverrun::possibleBufferOverrunError(const Token *tok, const std: "The source buffer is larger than the destination buffer so there is the potential for overflowing the destination buffer."); } -void CheckBufferOverrun::possibleReadlinkBufferOverrunError(const Token* tok, const std::string &varname) +void CheckBufferOverrun::possibleReadlinkBufferOverrunError(const Token* tok, const std::string &funcname, const std::string &varname) { - const std::string errmsg = "readlink() might return the full size of '" + varname + "'. Lower the supplied size by one.\n" - "readlink() might return the full size of '" + varname + "'. Lower the supplied size by one. " + const std::string errmsg = funcname + "() might return the full size of '" + varname + "'. Lower the supplied size by one.\n" + + funcname + "() might return the full size of '" + varname + "'. Lower the supplied size by one. " "If a " + varname + "[len] = '\\0'; statement follows, it will overrun the buffer."; reportInconclusiveError(tok, Severity::warning, "possibleReadlinkBufferOverrun", errmsg); @@ -1192,29 +1192,12 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo outOfBoundsError(tok->tokAt(4), "snprintf size", true, n, total_size); } - // readlink() - if (_settings->standards.posix && Token::Match(tok, "readlink ( %any% , %varid% , %num% )", arrayInfo.varid())) { - const MathLib::bigint n = MathLib::toLongNumber(tok->strAt(6)); - if (total_size > 0 && n > total_size) - outOfBoundsError(tok->tokAt(4), "readlink() buf size", true, n, total_size); - - if (_settings->inconclusive) { - // readlink() never terminates the buffer, check the end of the scope for buffer termination. - bool found_termination = false; - const Token *scope_end = scope_begin->link(); - for (const Token *tok2 = tok->tokAt(8); tok2 && tok2 != scope_end; tok2 = tok2->next()) { - if (Token::Match(tok2, "%varid% [ %any% ] = 0 ;", tok->tokAt(4)->varId())) { - found_termination = true; - break; - } - } - - if (!found_termination) { - bufferNotZeroTerminatedError(tok, tok->tokAt(4)->str(), "readlink"); - } else if (n == total_size) { - possibleReadlinkBufferOverrunError(tok, tok->tokAt(4)->str()); - } - } + // readlink() / readlinkat() buffer usage + if (_settings->standards.posix) { + if (Token::Match(tok, "readlink ( %any% , %varid% , %num% )", arrayInfo.varid())) + checkReadlinkBufferUsage(tok, scope_begin, total_size, false); + else if(Token::Match(tok, "readlinkat ( %any , %any% , %varid% , %num% )", arrayInfo.varid())) + checkReadlinkBufferUsage(tok, scope_begin, total_size, true); } // undefined behaviour: result of pointer arithmetic is out of bounds @@ -1227,6 +1210,34 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo } } +void CheckBufferOverrun::checkReadlinkBufferUsage(const Token* tok, const Token *scope_begin, const MathLib::bigint total_size, const bool is_readlinkat) +{ + unsigned int param_offset = is_readlinkat ? 2 : 0; + const std::string funcname = is_readlinkat ? "readlinkat" : "readlink"; + + const MathLib::bigint n = MathLib::toLongNumber(tok->strAt(6 + param_offset)); + if (total_size > 0 && n > total_size) + outOfBoundsError(tok->tokAt(4 + param_offset), funcname + "() buf size", true, n, total_size); + + if (!_settings->inconclusive) + return; + + // readlink()/readlinkat() never terminates the buffer, check the end of the scope for buffer termination. + bool found_termination = false; + const Token *scope_end = scope_begin->link(); + for (const Token *tok2 = tok->tokAt(8 + param_offset); tok2 && tok2 != scope_end; tok2 = tok2->next()) { + if (Token::Match(tok2, "%varid% [ %any% ] = 0 ;", tok->tokAt(4 + param_offset)->varId())) { + found_termination = true; + break; + } + } + + if (!found_termination) { + bufferNotZeroTerminatedError(tok, tok->tokAt(4 + param_offset)->str(), funcname); + } else if (n == total_size) { + possibleReadlinkBufferOverrunError(tok, funcname, tok->tokAt(4 + param_offset)->str()); + } +} //--------------------------------------------------------------------------- // Checking local variables in a scope diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h index 93c2e39c6..7e6dba1cb 100644 --- a/lib/checkbufferoverrun.h +++ b/lib/checkbufferoverrun.h @@ -195,6 +195,9 @@ public: /** Helper function used when parsing for-loops */ void parse_for_body(const Token *tok2, const ArrayInfo &arrayInfo, const std::string &strindex, bool condition_out_of_bounds, unsigned int counter_varid, const std::string &min_counter_value, const std::string &max_counter_value); + /** Check readlink or readlinkat() buffer usage */ + void checkReadlinkBufferUsage(const Token *tok, const Token *scope_begin, const MathLib::bigint total_size, const bool is_readlinkat); + /** * Helper function for checkFunctionCall - check a function parameter * \param tok token for the function name @@ -223,7 +226,7 @@ public: void pointerOutOfBoundsError(const Token *tok, const std::string &object); // UB when result of calculation is out of bounds void arrayIndexThenCheckError(const Token *tok, const std::string &indexName); void possibleBufferOverrunError(const Token *tok, const std::string &src, const std::string &dst, bool cat); - void possibleReadlinkBufferOverrunError(const Token *tok, const std::string &varname); + void possibleReadlinkBufferOverrunError(const Token *tok, const std::string &funcname, const std::string &varname); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) { CheckBufferOverrun c(0, settings, errorLogger); @@ -241,7 +244,7 @@ public: c.pointerOutOfBoundsError(0, "array"); c.arrayIndexThenCheckError(0, "index"); c.possibleBufferOverrunError(0, "source", "destination", false); - c.possibleReadlinkBufferOverrunError(0, "buffer"); + c.possibleReadlinkBufferOverrunError(0, "readlink", "buffer"); } std::string myName() const { diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 4b0dc2f07..1e604fa47 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -236,6 +236,7 @@ private: TEST_CASE(bufferNotZeroTerminated); TEST_CASE(readlink); + TEST_CASE(readlinkat); } @@ -3480,6 +3481,56 @@ private: "}\n"); ASSERT_EQUALS("", errout.str()); } + + void readlinkat() { + check("void f()\n" + "{\n" + " int dirfd = 42;\n" + " char buf[255];\n" + " ssize_t len = readlinkat(dirfd, path, buf, sizeof(buf)-1);\n" + " printf(\"%s\n\", buf);\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (warning) The buffer 'buf' is not zero-terminated after the call to readlinkat().\n", errout.str()); + + check("void f()\n" + "{\n" + " int dirfd = 42;\n" + " char buf[255];\n" + " ssize_t len = readlinkat(dirfd, path, buf, sizeof(buf)-1);\n" + " buf[len] = 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("void f()\n" + "{\n" + " int dirfd = 42;\n" + " char buf[10];\n" + " ssize_t len = readlinkat(dirf, path, buf, 255);\n" + " buf[len] = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (error) readlinkat() buf size is out of bounds: Supplied size 255 is larger than actual size of 10\n", errout.str()); + + check("void f()\n" + "{\n" + " int dirfd = 42;\n" + " char buf[255];\n" + " ssize_t len = readlinkat(dirfd, path, buf, sizeof(buf));\n" + " buf[len] = 0;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:5]: (warning) readlinkat() might return the full size of 'buf'. Lower the supplied size by one.\n", errout.str()); + + check("void f()\n" + "{\n" + " int dirfd = 42;\n" + " char buf[255];\n" + " ssize_t len = readlinkat(dirfd, path, buf, sizeof(buf)-1);\n" + " if (len == -1) {\n" + " return;\n" + " }\n" + " buf[len] = 0;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestBufferOverrun) From f0d8fd72353c70a94e6bef1a3bcb83ccd00876eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 24 Oct 2011 21:56:43 +0200 Subject: [PATCH 15/16] Preprocessor: updated tests --- test/testpreprocessor.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/testpreprocessor.cpp b/test/testpreprocessor.cpp index d580af178..a2c5c66e1 100644 --- a/test/testpreprocessor.cpp +++ b/test/testpreprocessor.cpp @@ -2866,7 +2866,7 @@ private: const std::string code("#ifndef X\n#define X\n123\n#endif\n" "#ifndef X\n#define X\n123\n#endif\n"); const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); - ASSERT_EQUALS("\n\n123\n\n" "\n\n\n\n", actual); + ASSERT_EQUALS("\n#define X\n123\n\n" "\n\n\n\n", actual); } // #define => #if @@ -2877,7 +2877,7 @@ private: "456\n" "#endif\n"); const std::string actual(preprocessor.handleIncludes(code,filePath,includePaths,defs)); - ASSERT_EQUALS("\n\n456\n\n", actual); + ASSERT_EQUALS("#define X 123\n\n456\n\n", actual); } // #elif @@ -2926,7 +2926,7 @@ private: defs.clear(); const std::string actual(preprocessor.handleIncludes(code + "#undef X\n" + code, filePath, includePaths, defs)); - ASSERT_EQUALS(actual1 + "\n" + actual1, actual); + ASSERT_EQUALS(actual1 + "#undef X\n" + actual1, actual); } } }; From a076b24dc6af8af9ba7519f07079b420da35130c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Mon, 24 Oct 2011 21:57:49 +0200 Subject: [PATCH 16/16] astyle formatting --- lib/checkbufferoverrun.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp index b1abc5342..c79923706 100644 --- a/lib/checkbufferoverrun.cpp +++ b/lib/checkbufferoverrun.cpp @@ -1196,7 +1196,7 @@ void CheckBufferOverrun::checkScope(const Token *tok, const ArrayInfo &arrayInfo if (_settings->standards.posix) { if (Token::Match(tok, "readlink ( %any% , %varid% , %num% )", arrayInfo.varid())) checkReadlinkBufferUsage(tok, scope_begin, total_size, false); - else if(Token::Match(tok, "readlinkat ( %any , %any% , %varid% , %num% )", arrayInfo.varid())) + else if (Token::Match(tok, "readlinkat ( %any , %any% , %varid% , %num% )", arrayInfo.varid())) checkReadlinkBufferUsage(tok, scope_begin, total_size, true); }