Fixed cppcheck-opensource#3088 (False positive: Dont report "struct or union member is never used" for structs with __attribute__((packed)) or #pragma pack(push))

Author: danmar

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())

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,19 +709,19 @@ 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
                 if (mSettings.checkConfiguration)
                     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 << "    <cpp version=\"" << mSettings.standards.getCPP() << "\"/>" << std::endl;
                     fdump << "  </standards>" << std::endl;
                     preprocessor.dump(fdump);
-                    mTokenizer.dump(fdump);
+                    tokenizer.dump(fdump);
                     fdump << "</dump>" << 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,25 +750,25 @@ 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")) {
                     std::cout << "Handling of \"simple\" rules is deprecated and will be removed in Cppcheck 2.5." << std::endl;
 
                     // 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<ErrorMessage::FileLocation> 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,

lib/preprocessor.h

@@ -91,6 +91,9 @@ class CPPCHECKLIB Preprocessor {
     void inlineSuppressions(const simplecpp::TokenList &tokens);
 
     void setDirectives(const simplecpp::TokenList &tokens);
+    void setDirectives(const std::list<Directive> &directives) {
+        mDirectives = directives;
+    }
 
     /** list of all directives met while preprocessing file */
     const std::list<Directive> &getDirectives() const {

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);

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 @@ class CPPCHECKLIB Tokenizer {
      */
     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 @@ class CPPCHECKLIB Tokenizer {
     /** Tokenizer maxtime */
     const std::time_t mMaxTime;
 #endif
+
+    const Preprocessor *mPreprocessor;
 };
 
 /// @}

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 @@ class TestUnusedVar : public TestFixture {
         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 @@ class TestUnusedVar : public TestFixture {
         TEST_CASE(volatileData); // #9280
     }
 
-    void checkStructMemberUsage(const char code[]) {
+    void checkStructMemberUsage(const char code[], const std::list<Directive> *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 @@ class TestUnusedVar : public TestFixture {
         ASSERT_EQUALS("", errout.str());
     }
 
+    void structmember15() { // #3088
+        std::list<Directive> 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"