Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion lib/check.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/checkbufferoverrun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/checkbufferoverrun.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 9 additions & 2 deletions lib/checkclass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3563,6 +3563,7 @@ namespace
struct NameLoc {
std::string className;
std::string fileName;
std::string configuration;
int lineNumber;
int column;
std::size_t hash;
Expand All @@ -3582,6 +3583,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) +
Expand All @@ -3592,7 +3594,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;
Expand Down Expand Up @@ -3649,6 +3651,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));
}
Expand All @@ -3669,13 +3672,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);
Expand Down Expand Up @@ -3717,6 +3722,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;
Expand Down
2 changes: 1 addition & 1 deletion lib/checkclass.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion lib/checknullpointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion lib/checknullpointer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion lib/checkuninitvar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion lib/checkuninitvar.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
8 changes: 4 additions & 4 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,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;
Expand Down Expand Up @@ -1203,7 +1203,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);
Expand Down Expand Up @@ -1327,7 +1327,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;

Expand Down Expand Up @@ -1392,7 +1392,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())
Expand Down
2 changes: 1 addition & 1 deletion lib/cppcheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,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
Expand Down
6 changes: 6 additions & 0 deletions test/cli/whole-program/odr3.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// #10431
#ifdef X
struct S { S() {} };
#else
struct S {};
#endif
3 changes: 3 additions & 0 deletions test/cli/whole-program/odr_cfg1.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#ifdef X
struct S {};
#endif
3 changes: 3 additions & 0 deletions test/cli/whole-program/odr_cfg2.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
struct S {
S() {}
};
22 changes: 21 additions & 1 deletion test/cli/whole-program_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = [
Expand Down
2 changes: 1 addition & 1 deletion test/testbufferoverrun.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions test/testclass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9084,7 +9084,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..
Expand Down Expand Up @@ -9130,7 +9130,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;
}
Expand Down
2 changes: 1 addition & 1 deletion test/testnullpointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4587,7 +4587,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();
Expand Down
2 changes: 1 addition & 1 deletion test/testuninitvar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading