Fix #10431 FP one-definition-rule if struct in mutually exclusive #ifdef branches (danmar#7584)

chrchr-github · web-flow · web-flow · commit 8b66b1c83c3b · 2025-06-21T10:07:40.000+02:00
Co-authored-by: chrchr-github &lt;noreply@github.com&gt;
diff --git a/lib/check.h b/lib/check.h
@@ -110,7 +110,7 @@ class CPPCHECKLIB Check {
         std::string file0;
     };
 
-    virtual FileInfo * getFileInfo(const Tokenizer& /*tokenizer*/, const Settings& /*settings*/) const {
+    virtual FileInfo * getFileInfo(const Tokenizer& /*tokenizer*/, const Settings& /*settings*/, const std::string& /*currentConfig*/) const {
         return nullptr;
     }
 
diff --git a/lib/checkbufferoverrun.cpp b/lib/checkbufferoverrun.cpp
@@ -947,7 +947,7 @@ bool CheckBufferOverrun::isCtuUnsafePointerArith(const Settings &settings, const
 }
 
 /** @brief Parse current TU and extract file info */
-Check::FileInfo *CheckBufferOverrun::getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const
+Check::FileInfo *CheckBufferOverrun::getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /*currentConfig*/) const
 {
     const std::list<CTU::FileInfo::UnsafeUsage> &unsafeArrayIndex = CTU::getUnsafeUsage(tokenizer, settings, isCtuUnsafeArrayIndex);
     const std::list<CTU::FileInfo::UnsafeUsage> &unsafePointerArith = CTU::getUnsafeUsage(tokenizer, settings, isCtuUnsafePointerArith);
diff --git a/lib/checkbufferoverrun.h b/lib/checkbufferoverrun.h
@@ -72,7 +72,7 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
     void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override;
 
     /** @brief Parse current TU and extract file info */
-    Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const override;
+    Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /*currentConfig*/) const override;
 
     /** @brief Analyse all file infos for all TU */
     bool analyseWholeProgram(const CTU::FileInfo &ctu, const std::list<Check::FileInfo*> &fileInfo, const Settings& settings, ErrorLogger &errorLogger) override;
diff --git a/lib/checkclass.cpp b/lib/checkclass.cpp
@@ -3570,6 +3570,7 @@ namespace
         struct NameLoc {
             std::string className;
             std::string fileName;
+            std::string configuration;
             int lineNumber;
             int column;
             std::size_t hash;
@@ -3589,6 +3590,7 @@ namespace
             for (const NameLoc &nameLoc: classDefinitions) {
                 ret += "<class name=\"" + ErrorLogger::toxml(nameLoc.className) +
                        "\" file=\"" + ErrorLogger::toxml(nameLoc.fileName) +
+                       "\" configuration=\"" + ErrorLogger::toxml(nameLoc.configuration) +
                        "\" line=\"" + std::to_string(nameLoc.lineNumber) +
                        "\" col=\"" + std::to_string(nameLoc.column) +
                        "\" hash=\"" + std::to_string(nameLoc.hash) +
@@ -3599,7 +3601,7 @@ namespace
     };
 }
 
-Check::FileInfo *CheckClass::getFileInfo(const Tokenizer &tokenizer, const Settings& /*settings*/) const
+Check::FileInfo *CheckClass::getFileInfo(const Tokenizer &tokenizer, const Settings& /*settings*/, const std::string& currentConfig) const
 {
     if (!tokenizer.isCPP())
         return nullptr;
@@ -3656,6 +3658,7 @@ Check::FileInfo *CheckClass::getFileInfo(const Tokenizer &tokenizer, const Setti
             }
         }
         nameLoc.hash = std::hash<std::string> {}(def);
+        nameLoc.configuration = currentConfig;
 
         classDefinitions.push_back(std::move(nameLoc));
     }
@@ -3676,13 +3679,15 @@ Check::FileInfo * CheckClass::loadFileInfoFromXml(const tinyxml2::XMLElement *xm
             continue;
         const char *name = e->Attribute("name");
         const char *file = e->Attribute("file");
+        const char *configuration = e->Attribute("configuration");
         const char *line = e->Attribute("line");
         const char *col = e->Attribute("col");
         const char *hash = e->Attribute("hash");
-        if (name && file && line && col && hash) {
+        if (name && file && configuration && line && col && hash) {
             MyFileInfo::NameLoc nameLoc;
             nameLoc.className = name;
             nameLoc.fileName = file;
+            nameLoc.configuration = configuration;
             nameLoc.lineNumber = strToInt<int>(line);
             nameLoc.column = strToInt<int>(col);
             nameLoc.hash = strToInt<std::size_t>(hash);
@@ -3724,6 +3729,8 @@ bool CheckClass::analyseWholeProgram(const CTU::FileInfo &ctu, const std::list<C
             }
             if (it->second.hash == nameLoc.hash)
                 continue;
+            if (it->second.fileName == nameLoc.fileName && it->second.configuration != nameLoc.configuration)
+                continue;
             // Same location, sometimes the hash is different wrongly (possibly because of different token simplifications).
             if (it->second.isSameLocation(nameLoc))
                 continue;
diff --git a/lib/checkclass.h b/lib/checkclass.h
@@ -138,7 +138,7 @@ class CPPCHECKLIB CheckClass : public Check {
     void checkUnsafeClassRefMember();
 
     /** @brief Parse current TU and extract file info */
-    Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const override;
+    Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& currentConfig) const override;
 
     Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const override;
 
diff --git a/lib/checknullpointer.cpp b/lib/checknullpointer.cpp
@@ -620,7 +620,7 @@ namespace
     };
 }
 
-Check::FileInfo *CheckNullPointer::getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const
+Check::FileInfo *CheckNullPointer::getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /*currentConfig*/) const
 {
     const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, isUnsafeUsage);
     if (unsafeUsage.empty())
diff --git a/lib/checknullpointer.h b/lib/checknullpointer.h
@@ -93,7 +93,7 @@ class CPPCHECKLIB CheckNullPointer : public Check {
     void nullPointerError(const Token *tok, const std::string &varname, const ValueFlow::Value* value, bool inconclusive);
 
     /** @brief Parse current TU and extract file info */
-    Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const override;
+    Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /*currentConfig*/) const override;
 
     Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const override;
 
diff --git a/lib/checkuninitvar.cpp b/lib/checkuninitvar.cpp
@@ -1730,7 +1730,7 @@ static bool isVariableUsage(const Settings &settings, const Token *argtok, CTU::
     return isVariableUsage(settings, argtok, &value->value);
 }
 
-Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const
+Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /*currentConfig*/) const
 {
     const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, ::isVariableUsage);
     if (unsafeUsage.empty())
diff --git a/lib/checkuninitvar.h b/lib/checkuninitvar.h
@@ -96,7 +96,7 @@ class CPPCHECKLIB CheckUninitVar : public Check {
     void valueFlowUninit();
 
     /** @brief Parse current TU and extract file info */
-    Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const override;
+    Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /*currentConfig*/) const override;
 
     Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const override;
 
diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp
@@ -732,7 +732,7 @@ unsigned int CppCheck::checkClang(const FileWithDetails &file, int fileIndex)
                              mSettings,
                              &s_timerResults);
         tokenizer.printDebugOutput(std::cout);
-        checkNormalTokens(tokenizer, nullptr); // TODO: provide analyzer information
+        checkNormalTokens(tokenizer, nullptr, ""); // TODO: provide analyzer information
 
         // create dumpfile
         std::ofstream fdump;
@@ -1213,7 +1213,7 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
                     }
 
                     // Check normal tokens
-                    checkNormalTokens(tokenizer, analyzerInformation.get());
+                    checkNormalTokens(tokenizer, analyzerInformation.get(), currentConfig);
                 } catch (const InternalError &e) {
                     ErrorMessage errmsg = ErrorMessage::fromInternalError(e, &tokenizer.list, file.spath());
                     mErrorLogger.reportErr(errmsg);
@@ -1337,7 +1337,7 @@ void CppCheck::internalError(const std::string &filename, const std::string &msg
 // CppCheck - A function that checks a normal token list
 //---------------------------------------------------------------------------
 
-void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation)
+void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation, const std::string& currentConfig)
 {
     CheckUnusedFunctions unusedFunctionsChecker;
 
@@ -1402,7 +1402,7 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation
         if (!doUnusedFunctionOnly) {
             // cppcheck-suppress shadowFunction - TODO: fix this
             for (const Check *check : Check::instances()) {
-                if (Check::FileInfo * const fi = check->getFileInfo(tokenizer, mSettings)) {
+                if (Check::FileInfo * const fi = check->getFileInfo(tokenizer, mSettings, currentConfig)) {
                     if (analyzerInformation)
                         analyzerInformation->setFileInfo(check->name(), fi->toString());
                     if (mSettings.useSingleJob())
diff --git a/lib/cppcheck.h b/lib/cppcheck.h
@@ -186,7 +186,7 @@ class CPPCHECKLIB CppCheck {
      * @param tokenizer tokenizer instance
      * @param analyzerInformation the analyzer infomation
      */
-    void checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation);
+    void checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation, const std::string& currentConfig);
 
     /**
      * Execute addons
diff --git a/test/cli/whole-program/odr3.cpp b/test/cli/whole-program/odr3.cpp
@@ -0,0 +1,6 @@
+// #10431
+#ifdef X
+struct S { S() {} };
+#else
+struct S {};
+#endif
diff --git a/test/cli/whole-program/odr_cfg1.cpp b/test/cli/whole-program/odr_cfg1.cpp
@@ -0,0 +1,3 @@
+#ifdef X
+struct S {};
+#endif
diff --git a/test/cli/whole-program/odr_cfg2.cpp b/test/cli/whole-program/odr_cfg2.cpp
@@ -0,0 +1,3 @@
+struct S {
+    S() {}
+};
diff --git a/test/cli/whole-program_test.py b/test/cli/whole-program_test.py
@@ -254,7 +254,8 @@ def __test_checkclass(extra_args):
         '--enable=information,style',
         '--error-exitcode=1',
         'whole-program/odr1.cpp',
-        'whole-program/odr2.cpp'
+        'whole-program/odr2.cpp',
+        'whole-program/odr3.cpp'
     ]
 
     args += extra_args
@@ -336,6 +337,25 @@ def test_checkclass_project_builddir_j(tmpdir):
     os.mkdir(build_dir)
     __test_checkclass_project(tmpdir, ['-j2', '--cppcheck-build-dir={}'.format(build_dir)])
 
+def test_ctu_odr_config():
+    args = [
+        '-q',
+        '-j1',
+        '--template=simple',
+        '--enable=information,style',
+        '--error-exitcode=1',
+        'whole-program/odr_cfg1.cpp',
+        'whole-program/odr_cfg2.cpp'
+    ]
+
+    ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
+    lines = stderr.splitlines()
+    assert lines == [
+        "whole-program{}odr_cfg1.cpp:2:1: error: The one definition rule is violated, different classes/structs have the same name 'S' [ctuOneDefinitionRuleViolation]".format(os.path.sep)
+    ]
+    assert stdout == ''
+    assert ret == 1, stdout
+
 
 def __test_nullpointer_file0(extra_args):
     args = [
diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp
@@ -5245,7 +5245,7 @@ class TestBufferOverrun : public TestFixture {
         // Check code..
         std::list<Check::FileInfo*> fileInfo;
         Check& c = getCheck<CheckBufferOverrun>();
-        fileInfo.push_back(c.getFileInfo(tokenizer, settings0));
+        fileInfo.push_back(c.getFileInfo(tokenizer, settings0, ""));
         c.analyseWholeProgram(*ctu, fileInfo, settings0, *this); // TODO: check result
         while (!fileInfo.empty()) {
             delete fileInfo.back();
diff --git a/test/testclass.cpp b/test/testclass.cpp
@@ -9092,7 +9092,7 @@ class TestClass : public TestFixture {
             const std::string filename = std::to_string(fileInfo.size()) + ".cpp";
             SimpleTokenizer tokenizer{settingsDefault, *this, filename};
             ASSERT(tokenizer.tokenize(c));
-            fileInfo.push_back(check.getFileInfo(tokenizer, settingsDefault));
+            fileInfo.push_back(check.getFileInfo(tokenizer, settingsDefault, ""));
         }
 
         // Check code..
@@ -9138,7 +9138,7 @@ class TestClass : public TestFixture {
 
         // Check..
         const Check& c = getCheck<CheckClass>();
-        Check::FileInfo * fileInfo = (c.getFileInfo)(tokenizer, settings1);
+        Check::FileInfo * fileInfo = (c.getFileInfo)(tokenizer, settings1, "");
 
         delete fileInfo;
     }
diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp
@@ -4599,7 +4599,7 @@ class TestNullPointer : public TestFixture {
         // Check code..
         std::list<Check::FileInfo*> fileInfo;
         Check& c = getCheck<CheckNullPointer>();
-        fileInfo.push_back(c.getFileInfo(tokenizer, settings));
+        fileInfo.push_back(c.getFileInfo(tokenizer, settings, ""));
         c.analyseWholeProgram(*ctu, fileInfo, settings, *this); // TODO: check result
         while (!fileInfo.empty()) {
             delete fileInfo.back();
diff --git a/test/testuninitvar.cpp b/test/testuninitvar.cpp
@@ -7923,7 +7923,7 @@ class TestUninitVar : public TestFixture {
         // Check code..
         std::list<Check::FileInfo*> fileInfo;
         Check& c = getCheck<CheckUninitVar>();
-        fileInfo.push_back(c.getFileInfo(tokenizer, settings));
+        fileInfo.push_back(c.getFileInfo(tokenizer, settings, ""));
         c.analyseWholeProgram(*ctu, fileInfo, settings, *this); // TODO: check result
         while (!fileInfo.empty()) {
             delete fileInfo.back();

Original file line number	Diff line number	Diff line change
`@@ -110,7 +110,7 @@ class CPPCHECKLIB Check {`
`110`	`110`	`std::string file0;`
`111`	`111`	`};`
`112`	`112`
`113`		`- virtual FileInfo * getFileInfo(const Tokenizer& /tokenizer/, const Settings& /settings/) const {`
	`113`	`+ virtual FileInfo * getFileInfo(const Tokenizer& /tokenizer/, const Settings& /settings/, const std::string& /currentConfig/) const {`
`114`	`114`	`return nullptr;`
`115`	`115`	`}`
`116`	`116`
Original file line number	Diff line number	Diff line change
`@@ -947,7 +947,7 @@ bool CheckBufferOverrun::isCtuUnsafePointerArith(const Settings &settings, const`
`947`	`947`	`}`
`948`	`948`
`949`	`949`	`/** @brief Parse current TU and extract file info */`
`950`		`-Check::FileInfo *CheckBufferOverrun::getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const`
	`950`	`+Check::FileInfo CheckBufferOverrun::getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /currentConfig*/) const`
`951`	`951`	`{`
`952`	`952`	`const std::list<CTU::FileInfo::UnsafeUsage> &unsafeArrayIndex = CTU::getUnsafeUsage(tokenizer, settings, isCtuUnsafeArrayIndex);`
`953`	`953`	`const std::list<CTU::FileInfo::UnsafeUsage> &unsafePointerArith = CTU::getUnsafeUsage(tokenizer, settings, isCtuUnsafePointerArith);`
Original file line number	Diff line number	Diff line change
`@@ -620,7 +620,7 @@ namespace`
`620`	`620`	`};`
`621`	`621`	`}`
`622`	`622`
`623`		`-Check::FileInfo *CheckNullPointer::getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const`
	`623`	`+Check::FileInfo CheckNullPointer::getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /currentConfig*/) const`
`624`	`624`	`{`
`625`	`625`	`const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, isUnsafeUsage);`
`626`	`626`	`if (unsafeUsage.empty())`
Original file line number	Diff line number	Diff line change
`@@ -1730,7 +1730,7 @@ static bool isVariableUsage(const Settings &settings, const Token *argtok, CTU::`
`1730`	`1730`	`return isVariableUsage(settings, argtok, &value->value);`
`1731`	`1731`	`}`
`1732`	`1732`
`1733`		`-Check::FileInfo *CheckUninitVar::getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const`
	`1733`	`+Check::FileInfo CheckUninitVar::getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /currentConfig*/) const`
`1734`	`1734`	`{`
`1735`	`1735`	`const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, ::isVariableUsage);`
`1736`	`1736`	`if (unsafeUsage.empty())`