Skip to content

Commit 8b66b1c

Browse files
chrchr-githubweb-flow
chrchr-github
and
web-flow
authored
Fix #10431 FP one-definition-rule if struct in mutually exclusive #ifdef branches (#7584)
Co-authored-by: chrchr-github <noreply@github.com>
1 parent 84f910b commit 8b66b1c

19 files changed

+60
-21
lines changed

‎lib/check.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ class CPPCHECKLIB Check {
110110
std::string file0;
111111
};
112112

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 {
114114
return nullptr;
115115
}
116116

‎lib/checkbufferoverrun.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -947,7 +947,7 @@ bool CheckBufferOverrun::isCtuUnsafePointerArith(const Settings &settings, const
947947
}
948948

949949
/** @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
951951
{
952952
const std::list<CTU::FileInfo::UnsafeUsage> &unsafeArrayIndex = CTU::getUnsafeUsage(tokenizer, settings, isCtuUnsafeArrayIndex);
953953
const std::list<CTU::FileInfo::UnsafeUsage> &unsafePointerArith = CTU::getUnsafeUsage(tokenizer, settings, isCtuUnsafePointerArith);

‎lib/checkbufferoverrun.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class CPPCHECKLIB CheckBufferOverrun : public Check {
7272
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override;
7373

7474
/** @brief Parse current TU and extract file info */
75-
Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const override;
75+
Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /*currentConfig*/) const override;
7676

7777
/** @brief Analyse all file infos for all TU */
7878
bool analyseWholeProgram(const CTU::FileInfo &ctu, const std::list<Check::FileInfo*> &fileInfo, const Settings& settings, ErrorLogger &errorLogger) override;

‎lib/checkclass.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3570,6 +3570,7 @@ namespace
35703570
struct NameLoc {
35713571
std::string className;
35723572
std::string fileName;
3573+
std::string configuration;
35733574
int lineNumber;
35743575
int column;
35753576
std::size_t hash;
@@ -3589,6 +3590,7 @@ namespace
35893590
for (const NameLoc &nameLoc: classDefinitions) {
35903591
ret += "<class name=\"" + ErrorLogger::toxml(nameLoc.className) +
35913592
"\" file=\"" + ErrorLogger::toxml(nameLoc.fileName) +
3593+
"\" configuration=\"" + ErrorLogger::toxml(nameLoc.configuration) +
35923594
"\" line=\"" + std::to_string(nameLoc.lineNumber) +
35933595
"\" col=\"" + std::to_string(nameLoc.column) +
35943596
"\" hash=\"" + std::to_string(nameLoc.hash) +
@@ -3599,7 +3601,7 @@ namespace
35993601
};
36003602
}
36013603

3602-
Check::FileInfo *CheckClass::getFileInfo(const Tokenizer &tokenizer, const Settings& /*settings*/) const
3604+
Check::FileInfo *CheckClass::getFileInfo(const Tokenizer &tokenizer, const Settings& /*settings*/, const std::string& currentConfig) const
36033605
{
36043606
if (!tokenizer.isCPP())
36053607
return nullptr;
@@ -3656,6 +3658,7 @@ Check::FileInfo *CheckClass::getFileInfo(const Tokenizer &tokenizer, const Setti
36563658
}
36573659
}
36583660
nameLoc.hash = std::hash<std::string> {}(def);
3661+
nameLoc.configuration = currentConfig;
36593662

36603663
classDefinitions.push_back(std::move(nameLoc));
36613664
}
@@ -3676,13 +3679,15 @@ Check::FileInfo * CheckClass::loadFileInfoFromXml(const tinyxml2::XMLElement *xm
36763679
continue;
36773680
const char *name = e->Attribute("name");
36783681
const char *file = e->Attribute("file");
3682+
const char *configuration = e->Attribute("configuration");
36793683
const char *line = e->Attribute("line");
36803684
const char *col = e->Attribute("col");
36813685
const char *hash = e->Attribute("hash");
3682-
if (name && file && line && col && hash) {
3686+
if (name && file && configuration && line && col && hash) {
36833687
MyFileInfo::NameLoc nameLoc;
36843688
nameLoc.className = name;
36853689
nameLoc.fileName = file;
3690+
nameLoc.configuration = configuration;
36863691
nameLoc.lineNumber = strToInt<int>(line);
36873692
nameLoc.column = strToInt<int>(col);
36883693
nameLoc.hash = strToInt<std::size_t>(hash);
@@ -3724,6 +3729,8 @@ bool CheckClass::analyseWholeProgram(const CTU::FileInfo &ctu, const std::list<C
37243729
}
37253730
if (it->second.hash == nameLoc.hash)
37263731
continue;
3732+
if (it->second.fileName == nameLoc.fileName && it->second.configuration != nameLoc.configuration)
3733+
continue;
37273734
// Same location, sometimes the hash is different wrongly (possibly because of different token simplifications).
37283735
if (it->second.isSameLocation(nameLoc))
37293736
continue;

‎lib/checkclass.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ class CPPCHECKLIB CheckClass : public Check {
138138
void checkUnsafeClassRefMember();
139139

140140
/** @brief Parse current TU and extract file info */
141-
Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const override;
141+
Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& currentConfig) const override;
142142

143143
Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const override;
144144

‎lib/checknullpointer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ namespace
620620
};
621621
}
622622

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
624624
{
625625
const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, isUnsafeUsage);
626626
if (unsafeUsage.empty())

‎lib/checknullpointer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class CPPCHECKLIB CheckNullPointer : public Check {
9393
void nullPointerError(const Token *tok, const std::string &varname, const ValueFlow::Value* value, bool inconclusive);
9494

9595
/** @brief Parse current TU and extract file info */
96-
Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const override;
96+
Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /*currentConfig*/) const override;
9797

9898
Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const override;
9999

‎lib/checkuninitvar.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1730,7 +1730,7 @@ static bool isVariableUsage(const Settings &settings, const Token *argtok, CTU::
17301730
return isVariableUsage(settings, argtok, &value->value);
17311731
}
17321732

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
17341734
{
17351735
const std::list<CTU::FileInfo::UnsafeUsage> &unsafeUsage = CTU::getUnsafeUsage(tokenizer, settings, ::isVariableUsage);
17361736
if (unsafeUsage.empty())

‎lib/checkuninitvar.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class CPPCHECKLIB CheckUninitVar : public Check {
9696
void valueFlowUninit();
9797

9898
/** @brief Parse current TU and extract file info */
99-
Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings) const override;
99+
Check::FileInfo *getFileInfo(const Tokenizer &tokenizer, const Settings &settings, const std::string& /*currentConfig*/) const override;
100100

101101
Check::FileInfo * loadFileInfoFromXml(const tinyxml2::XMLElement *xmlElement) const override;
102102

‎lib/cppcheck.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ unsigned int CppCheck::checkClang(const FileWithDetails &file, int fileIndex)
732732
mSettings,
733733
&s_timerResults);
734734
tokenizer.printDebugOutput(std::cout);
735-
checkNormalTokens(tokenizer, nullptr); // TODO: provide analyzer information
735+
checkNormalTokens(tokenizer, nullptr, ""); // TODO: provide analyzer information
736736

737737
// create dumpfile
738738
std::ofstream fdump;
@@ -1213,7 +1213,7 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
12131213
}
12141214

12151215
// Check normal tokens
1216-
checkNormalTokens(tokenizer, analyzerInformation.get());
1216+
checkNormalTokens(tokenizer, analyzerInformation.get(), currentConfig);
12171217
} catch (const InternalError &e) {
12181218
ErrorMessage errmsg = ErrorMessage::fromInternalError(e, &tokenizer.list, file.spath());
12191219
mErrorLogger.reportErr(errmsg);
@@ -1337,7 +1337,7 @@ void CppCheck::internalError(const std::string &filename, const std::string &msg
13371337
// CppCheck - A function that checks a normal token list
13381338
//---------------------------------------------------------------------------
13391339

1340-
void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation)
1340+
void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation, const std::string& currentConfig)
13411341
{
13421342
CheckUnusedFunctions unusedFunctionsChecker;
13431343

@@ -1402,7 +1402,7 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation
14021402
if (!doUnusedFunctionOnly) {
14031403
// cppcheck-suppress shadowFunction - TODO: fix this
14041404
for (const Check *check : Check::instances()) {
1405-
if (Check::FileInfo * const fi = check->getFileInfo(tokenizer, mSettings)) {
1405+
if (Check::FileInfo * const fi = check->getFileInfo(tokenizer, mSettings, currentConfig)) {
14061406
if (analyzerInformation)
14071407
analyzerInformation->setFileInfo(check->name(), fi->toString());
14081408
if (mSettings.useSingleJob())

‎lib/cppcheck.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ class CPPCHECKLIB CppCheck {
186186
* @param tokenizer tokenizer instance
187187
* @param analyzerInformation the analyzer infomation
188188
*/
189-
void checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation);
189+
void checkNormalTokens(const Tokenizer &tokenizer, AnalyzerInformation* analyzerInformation, const std::string& currentConfig);
190190

191191
/**
192192
* Execute addons

‎test/cli/whole-program/odr3.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// #10431
2+
#ifdef X
3+
struct S { S() {} };
4+
#else
5+
struct S {};
6+
#endif

‎test/cli/whole-program/odr_cfg1.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#ifdef X
2+
struct S {};
3+
#endif

‎test/cli/whole-program/odr_cfg2.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
struct S {
2+
S() {}
3+
};

‎test/cli/whole-program_test.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,8 @@ def __test_checkclass(extra_args):
254254
'--enable=information,style',
255255
'--error-exitcode=1',
256256
'whole-program/odr1.cpp',
257-
'whole-program/odr2.cpp'
257+
'whole-program/odr2.cpp',
258+
'whole-program/odr3.cpp'
258259
]
259260

260261
args += extra_args
@@ -336,6 +337,25 @@ def test_checkclass_project_builddir_j(tmpdir):
336337
os.mkdir(build_dir)
337338
__test_checkclass_project(tmpdir, ['-j2', '--cppcheck-build-dir={}'.format(build_dir)])
338339

340+
def test_ctu_odr_config():
341+
args = [
342+
'-q',
343+
'-j1',
344+
'--template=simple',
345+
'--enable=information,style',
346+
'--error-exitcode=1',
347+
'whole-program/odr_cfg1.cpp',
348+
'whole-program/odr_cfg2.cpp'
349+
]
350+
351+
ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
352+
lines = stderr.splitlines()
353+
assert lines == [
354+
"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)
355+
]
356+
assert stdout == ''
357+
assert ret == 1, stdout
358+
339359

340360
def __test_nullpointer_file0(extra_args):
341361
args = [

‎test/testbufferoverrun.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5245,7 +5245,7 @@ class TestBufferOverrun : public TestFixture {
52455245
// Check code..
52465246
std::list<Check::FileInfo*> fileInfo;
52475247
Check& c = getCheck<CheckBufferOverrun>();
5248-
fileInfo.push_back(c.getFileInfo(tokenizer, settings0));
5248+
fileInfo.push_back(c.getFileInfo(tokenizer, settings0, ""));
52495249
c.analyseWholeProgram(*ctu, fileInfo, settings0, *this); // TODO: check result
52505250
while (!fileInfo.empty()) {
52515251
delete fileInfo.back();

‎test/testclass.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9092,7 +9092,7 @@ class TestClass : public TestFixture {
90929092
const std::string filename = std::to_string(fileInfo.size()) + ".cpp";
90939093
SimpleTokenizer tokenizer{settingsDefault, *this, filename};
90949094
ASSERT(tokenizer.tokenize(c));
9095-
fileInfo.push_back(check.getFileInfo(tokenizer, settingsDefault));
9095+
fileInfo.push_back(check.getFileInfo(tokenizer, settingsDefault, ""));
90969096
}
90979097

90989098
// Check code..
@@ -9138,7 +9138,7 @@ class TestClass : public TestFixture {
91389138

91399139
// Check..
91409140
const Check& c = getCheck<CheckClass>();
9141-
Check::FileInfo * fileInfo = (c.getFileInfo)(tokenizer, settings1);
9141+
Check::FileInfo * fileInfo = (c.getFileInfo)(tokenizer, settings1, "");
91429142

91439143
delete fileInfo;
91449144
}

‎test/testnullpointer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4599,7 +4599,7 @@ class TestNullPointer : public TestFixture {
45994599
// Check code..
46004600
std::list<Check::FileInfo*> fileInfo;
46014601
Check& c = getCheck<CheckNullPointer>();
4602-
fileInfo.push_back(c.getFileInfo(tokenizer, settings));
4602+
fileInfo.push_back(c.getFileInfo(tokenizer, settings, ""));
46034603
c.analyseWholeProgram(*ctu, fileInfo, settings, *this); // TODO: check result
46044604
while (!fileInfo.empty()) {
46054605
delete fileInfo.back();

‎test/testuninitvar.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7923,7 +7923,7 @@ class TestUninitVar : public TestFixture {
79237923
// Check code..
79247924
std::list<Check::FileInfo*> fileInfo;
79257925
Check& c = getCheck<CheckUninitVar>();
7926-
fileInfo.push_back(c.getFileInfo(tokenizer, settings));
7926+
fileInfo.push_back(c.getFileInfo(tokenizer, settings, ""));
79277927
c.analyseWholeProgram(*ctu, fileInfo, settings, *this); // TODO: check result
79287928
while (!fileInfo.empty()) {
79297929
delete fileInfo.back();

0 commit comments

Comments
 (0)