From 0583763cc606cdb7b0389c37361b558881140b2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 30 Jun 2020 10:59:57 +0200 Subject: [PATCH] Fixed #3088 (False positive: Dont report "struct or union member is never used" for structs with __attribute__((packed)) or #pragma pack(push)) --- lib/checkunusedvar.cpp | 12 ++++++++++++ lib/cppcheck.cpp | 31 ++++++++++++++++--------------- lib/preprocessor.h | 3 +++ lib/tokenize.cpp | 4 +++- lib/tokenize.h | 10 ++++++++++ test/testunusedvar.cpp | 16 +++++++++++++++- 6 files changed, 59 insertions(+), 17 deletions(-) diff --git a/lib/checkunusedvar.cpp b/lib/checkunusedvar.cpp index 3951b9e87..d54d29f98 100644 --- a/lib/checkunusedvar.cpp +++ b/lib/checkunusedvar.cpp @@ -21,6 +21,7 @@ #include "checkunusedvar.h" #include "astutils.h" +#include "preprocessor.h" #include "settings.h" #include "symboldatabase.h" #include "token.h" @@ -1340,6 +1341,17 @@ void CheckUnusedVar::checkStructMemberUsage() // Packed struct => possibly used by lowlevel code. Struct members might be required by hardware. if (scope.bodyEnd->isAttributePacked()) continue; + if (const Preprocessor *preprocessor = mTokenizer->getPreprocessor()) { + bool isPacked = false; + for (const Directive &d: preprocessor->getDirectives()) { + if (d.str == "#pragma pack(1)" && d.file == mTokenizer->list.getFiles().front() && d.linenr < scope.bodyStart->linenr()) { + isPacked=true; + break; + } + } + if (isPacked) + continue; + } // Bail out if struct/union contains any functions if (!scope.functionList.empty()) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 0b4349eb8..f948e8b78 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -688,16 +688,17 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string continue; } - Tokenizer mTokenizer(&mSettings, this); + Tokenizer tokenizer(&mSettings, this); + tokenizer.setPreprocessor(&preprocessor); if (mSettings.showtime != SHOWTIME_MODES::SHOWTIME_NONE) - mTokenizer.setTimerResults(&s_timerResults); + tokenizer.setTimerResults(&s_timerResults); try { // Create tokens, skip rest of iteration if failed { Timer timer("Tokenizer::createTokens", mSettings.showtime, &s_timerResults); simplecpp::TokenList tokensP = preprocessor.preprocess(tokens1, mCurrentConfig, files, true); - mTokenizer.createTokens(std::move(tokensP)); + tokenizer.createTokens(std::move(tokensP)); } hasValidConfig = true; @@ -708,7 +709,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string mErrorLogger.reportOut("Checking " + fixedpath + ": " + mCurrentConfig + "..."); } - if (!mTokenizer.tokens()) + if (!tokenizer.tokens()) continue; // skip rest of iteration if just checking configuration @@ -716,11 +717,11 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string continue; // Check raw tokens - checkRawTokens(mTokenizer); + checkRawTokens(tokenizer); // Simplify tokens into normal form, skip rest of iteration if failed Timer timer2("Tokenizer::simplifyTokens1", mSettings.showtime, &s_timerResults); - bool result = mTokenizer.simplifyTokens1(mCurrentConfig); + bool result = tokenizer.simplifyTokens1(mCurrentConfig); timer2.stop(); if (!result) continue; @@ -733,13 +734,13 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string fdump << " " << std::endl; fdump << " " << std::endl; preprocessor.dump(fdump); - mTokenizer.dump(fdump); + tokenizer.dump(fdump); fdump << "" << std::endl; } // Skip if we already met the same simplified token list if (mSettings.force || mSettings.maxConfigs > 1) { - const unsigned long long checksum = mTokenizer.list.calculateChecksum(); + const unsigned long long checksum = tokenizer.list.calculateChecksum(); if (checksums.find(checksum) != checksums.end()) { if (mSettings.debugwarnings) purgedConfigurationMessage(filename, mCurrentConfig); @@ -749,11 +750,11 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string } // Check normal tokens - checkNormalTokens(mTokenizer); + checkNormalTokens(tokenizer); // Analyze info.. if (!mSettings.buildDir.empty()) - checkUnusedFunctions.parseTokens(mTokenizer, filename.c_str(), &mSettings); + checkUnusedFunctions.parseTokens(tokenizer, filename.c_str(), &mSettings); // simplify more if required, skip rest of iteration if failed if (mSimplify && hasRule("simple")) { @@ -761,13 +762,13 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string // if further simplification fails then skip rest of iteration Timer timer3("Tokenizer::simplifyTokenList2", mSettings.showtime, &s_timerResults); - result = mTokenizer.simplifyTokenList2(); + result = tokenizer.simplifyTokenList2(); timer3.stop(); if (!result) continue; if (!Settings::terminated()) - executeRules("simple", mTokenizer); + executeRules("simple", tokenizer); } } catch (const simplecpp::Output &o) { @@ -779,16 +780,16 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string } catch (const InternalError &e) { std::list locationList; if (e.token) { - ErrorMessage::FileLocation loc(e.token, &mTokenizer.list); + ErrorMessage::FileLocation loc(e.token, &tokenizer.list); locationList.push_back(loc); } else { - ErrorMessage::FileLocation loc(mTokenizer.list.getSourceFilePath(), 0, 0); + ErrorMessage::FileLocation loc(tokenizer.list.getSourceFilePath(), 0, 0); ErrorMessage::FileLocation loc2(filename, 0, 0); locationList.push_back(loc2); locationList.push_back(loc); } ErrorMessage errmsg(locationList, - mTokenizer.list.getSourceFilePath(), + tokenizer.list.getSourceFilePath(), Severity::error, e.errorMessage, e.id, diff --git a/lib/preprocessor.h b/lib/preprocessor.h index 8a0dfd888..71b88dd50 100644 --- a/lib/preprocessor.h +++ b/lib/preprocessor.h @@ -91,6 +91,9 @@ public: void inlineSuppressions(const simplecpp::TokenList &tokens); void setDirectives(const simplecpp::TokenList &tokens); + void setDirectives(const std::list &directives) { + mDirectives = directives; + } /** list of all directives met while preprocessing file */ const std::list &getDirectives() const { diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp index b72ba5225..a92301a78 100644 --- a/lib/tokenize.cpp +++ b/lib/tokenize.cpp @@ -157,8 +157,9 @@ Tokenizer::Tokenizer() : mCodeWithTemplates(false), //is there any templates? mTimerResults(nullptr) #ifdef MAXTIME - ,mMaxTime(std::time(0) + MAXTIME) + , mMaxTime(std::time(0) + MAXTIME) #endif + , mPreprocessor(nullptr) { } @@ -175,6 +176,7 @@ Tokenizer::Tokenizer(const Settings *settings, ErrorLogger *errorLogger) : #ifdef MAXTIME ,mMaxTime(std::time(0) + MAXTIME) #endif + , mPreprocessor(nullptr) { // make sure settings are specified assert(mSettings); diff --git a/lib/tokenize.h b/lib/tokenize.h index 36ea66bd5..628628ae6 100644 --- a/lib/tokenize.h +++ b/lib/tokenize.h @@ -37,6 +37,7 @@ class TimerResults; class Token; class TemplateSimplifier; class ErrorLogger; +class Preprocessor; namespace simplecpp { class TokenList; @@ -560,6 +561,13 @@ public: */ static const Token * isFunctionHead(const Token *tok, const std::string &endsWith, bool cpp); + void setPreprocessor(const Preprocessor *preprocessor) { + mPreprocessor = preprocessor; + } + const Preprocessor *getPreprocessor() const { + return mPreprocessor; + } + private: /** @@ -956,6 +964,8 @@ private: /** Tokenizer maxtime */ const std::time_t mMaxTime; #endif + + const Preprocessor *mPreprocessor; }; /// @} diff --git a/test/testunusedvar.cpp b/test/testunusedvar.cpp index c2cfb1f3b..7450c38fd 100644 --- a/test/testunusedvar.cpp +++ b/test/testunusedvar.cpp @@ -17,6 +17,7 @@ */ #include "checkunusedvar.h" +#include "preprocessor.h" #include "settings.h" #include "testsuite.h" #include "tokenize.h" @@ -55,6 +56,7 @@ private: TEST_CASE(structmember12); // #7179 - FP unused structmember TEST_CASE(structmember13); // #3088 - __attribute__((packed)) TEST_CASE(structmember14); // #6508 - (struct x){1,2,..} + TEST_CASE(structmember15); // #3088 - #pragma pack(1) TEST_CASE(structmember_sizeof); TEST_CASE(localvar1); @@ -211,12 +213,17 @@ private: TEST_CASE(volatileData); // #9280 } - void checkStructMemberUsage(const char code[]) { + void checkStructMemberUsage(const char code[], const std::list *directives=nullptr) { // Clear the error buffer.. errout.str(""); + Preprocessor preprocessor(settings, nullptr); + if (directives) + preprocessor.setDirectives(*directives); + // Tokenize.. Tokenizer tokenizer(&settings, this); + tokenizer.setPreprocessor(&preprocessor); std::istringstream istr(code); tokenizer.tokenize(istr, "test.cpp"); @@ -473,6 +480,13 @@ private: ASSERT_EQUALS("", errout.str()); } + void structmember15() { // #3088 + std::list directives; + directives.emplace_back("test.cpp", 1, "#pragma pack(1)"); + checkStructMemberUsage("\nstruct Foo { int x; int y; };", &directives); + ASSERT_EQUALS("", errout.str()); + } + void structmember_extern() { // extern struct => no false positive checkStructMemberUsage("extern struct AB\n"