Skip to content

fix #13333: better handling when source file is repeated in compile_commands.json #7469

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

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ cli/executor.o: cli/executor.cpp cli/executor.h lib/addoninfo.h lib/checkers.h l
cli/filelister.o: cli/filelister.cpp cli/filelister.h lib/config.h lib/filesettings.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/standards.h lib/utils.h
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/filelister.cpp

cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/config.h lib/errortypes.h lib/filesettings.h lib/mathlib.h lib/path.h lib/platform.h lib/standards.h
cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/config.h lib/errortypes.h lib/filesettings.h lib/mathlib.h lib/path.h lib/platform.h lib/standards.h lib/utils.h
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/main.cpp

cli/processexecutor.o: cli/processexecutor.cpp cli/executor.h cli/processexecutor.h lib/addoninfo.h lib/check.h lib/checkers.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
Expand Down
21 changes: 20 additions & 1 deletion cli/cmdlineparser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,24 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
}
}

// enforce the language since markup files are special and do not adhere to the enforced language
for (auto& fs : fileSettings)
{
if (mSettings.library.markupFile(fs.filename())) {
// enforce the language since markup files are special and do not adhere to the enforced language
fs.file.setLang(Standards::Language::C);
}

fs.computeHash();
}

if (!mSettings.recheckProjectDuplicates) {
auto it = fileSettings.begin();
while (it != fileSettings.end()) {
fileSettings.erase(std::remove_if(std::next(it), fileSettings.end(), [&](const FileSettings& fs) {
return fs.hash == it->hash;
}), fileSettings.end());
++it;
}
}

// sort the markup last
Expand Down Expand Up @@ -635,6 +647,9 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
else if (std::strcmp(argv[i], "--debug-clang-output") == 0)
mSettings.debugClangOutput = true;

else if (std::strcmp(argv[i], "--recheck-project-duplicates") == 0)
mSettings.recheckProjectDuplicates = true;

// Show debug messages for ignored files
else if (std::strcmp(argv[i], "--debug-ignore") == 0)
mSettings.debugignore = true;
Expand Down Expand Up @@ -1702,6 +1717,10 @@ void CmdLineParser::printHelp() const
" be considered for evaluation.\n"
" --config-excludes-file=<file>\n"
" A file that contains a list of config-excludes\n"
" --recheck-project-duplicates\n"
" When using the --project option, files with identical\n"
" names and compiler flags are deduplicated by default.\n"
" This option forces cppcheck to check all imported files.\n"
" --disable=<id> Disable individual checks.\n"
" Please refer to the documentation of --enable=<id>\n"
" for further details.\n"
Expand Down
25 changes: 14 additions & 11 deletions lib/analyzerinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,13 @@ void AnalyzerInformation::writeFilesTxt(const std::string &buildDir, const std::
fout << afile << ".a" << (++fileCount[afile]) << ":" << userDefines << ":" << Path::simplifyPath(f) << '\n';
}

std::ios savedFlags(nullptr);
savedFlags.copyfmt(fout);
for (const FileSettings &fs : fileSettings) {
const std::string afile = getFilename(fs.filename());
fout << afile << ".a" << (++fileCount[afile]) << ":" << fs.cfg << ":" << Path::simplifyPath(fs.filename()) << std::endl;
fout << afile << ".a" << (++fileCount[afile]) << ":" << std::hex << fs.hash << ":" << Path::simplifyPath(fs.filename()) << std::endl;
}
fout.copyfmt(savedFlags);
}

void AnalyzerInformation::close()
Expand All @@ -73,7 +76,7 @@ void AnalyzerInformation::close()
}
}

static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t hash, std::list<ErrorMessage> &errors)
static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t filehash, std::list<ErrorMessage> &errors)
{
tinyxml2::XMLDocument doc;
const tinyxml2::XMLError error = doc.LoadFile(analyzerInfoFile.c_str());
Expand All @@ -85,7 +88,7 @@ static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t hash,
return false;

const char *attr = rootNode->Attribute("hash");
if (!attr || attr != std::to_string(hash))
if (!attr || attr != std::to_string(filehash))
return false;

for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
Expand All @@ -96,10 +99,10 @@ static bool skipAnalysis(const std::string &analyzerInfoFile, std::size_t hash,
return true;
}

std::string AnalyzerInformation::getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg)
std::string AnalyzerInformation::getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfghash)
{
std::string line;
const std::string end(':' + cfg + ':' + Path::simplifyPath(sourcefile));
const std::string end(':' + cfghash + ':' + Path::simplifyPath(sourcefile));
while (std::getline(filesTxt,line)) {
if (line.size() <= end.size() + 2U)
continue;
Expand All @@ -110,11 +113,11 @@ std::string AnalyzerInformation::getAnalyzerInfoFileFromFilesTxt(std::istream& f
return "";
}

std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg)
std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfghash)
{
std::ifstream fin(Path::join(buildDir, "files.txt"));
if (fin.is_open()) {
const std::string& ret = getAnalyzerInfoFileFromFilesTxt(fin, sourcefile, cfg);
const std::string& ret = getAnalyzerInfoFileFromFilesTxt(fin, sourcefile, cfghash);
if (!ret.empty())
return Path::join(buildDir, ret);
}
Expand All @@ -128,21 +131,21 @@ std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir
return Path::join(buildDir, filename) + ".analyzerinfo";
}

bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t hash, std::list<ErrorMessage> &errors)
bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfgHash, std::size_t fileHash, std::list<ErrorMessage> &errors)
{
if (buildDir.empty() || sourcefile.empty())
return true;
close();

mAnalyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfg);
mAnalyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfgHash);

if (skipAnalysis(mAnalyzerInfoFile, hash, errors))
if (skipAnalysis(mAnalyzerInfoFile, fileHash, errors))
return false;

mOutputStream.open(mAnalyzerInfoFile);
if (mOutputStream.is_open()) {
mOutputStream << "<?xml version=\"1.0\"?>\n";
mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
mOutputStream << "<analyzerinfo hash=\"" << fileHash << "\">\n";
} else {
mAnalyzerInfoFile.clear();
}
Expand Down
6 changes: 3 additions & 3 deletions lib/analyzerinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ class CPPCHECKLIB AnalyzerInformation {

/** Close current TU.analyzerinfo file */
void close();
bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t hash, std::list<ErrorMessage> &errors);
bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfgHash, std::size_t fileHash, std::list<ErrorMessage> &errors);
void reportErr(const ErrorMessage &msg);
void setFileInfo(const std::string &check, const std::string &fileInfo);
static std::string getAnalyzerInfoFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg);
static std::string getAnalyzerInfoFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfghash);
protected:
static std::string getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg);
static std::string getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfghash);
private:
std::ofstream mOutputStream;
std::string mAnalyzerInfoFile;
Expand Down
37 changes: 23 additions & 14 deletions lib/cppcheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,13 +331,17 @@ static std::vector<std::string> split(const std::string &str, const std::string
return ret;
}

static std::string getDumpFileName(const Settings& settings, const std::string& filename)
static std::string getDumpFileName(const Settings& settings, const std::string& filename, const std::string &cfgHash = "")
{
std::string extension;
if (settings.dump || !settings.buildDir.empty())
extension = ".dump";
else
extension = "." + std::to_string(settings.pid) + ".dump";

if (!settings.dump && settings.buildDir.empty())
extension += "." + std::to_string(settings.pid);

if (!cfgHash.empty())
extension += "." + cfgHash;

extension += ".dump";

if (!settings.dump && !settings.buildDir.empty())
return AnalyzerInformation::getAnalyzerInfoFile(settings.buildDir, filename, "") + extension;
Expand All @@ -352,11 +356,12 @@ static std::string getCtuInfoFileName(const std::string &dumpFile)
static void createDumpFile(const Settings& settings,
const FileWithDetails& file,
std::ofstream& fdump,
std::string& dumpFile)
std::string& dumpFile,
const std::string &cfgHash = "")
{
if (!settings.dump && settings.addons.empty())
return;
dumpFile = getDumpFileName(settings, file.spath());
dumpFile = getDumpFileName(settings, file.spath(), cfgHash);

fdump.open(dumpFile);
if (!fdump.is_open())
Expand Down Expand Up @@ -775,7 +780,7 @@ unsigned int CppCheck::check(const FileWithDetails &file)
unsigned int CppCheck::check(const FileWithDetails &file, const std::string &content)
{
std::istringstream iss(content);
return checkFile(file, "", &iss);
return checkFile(file, "", "", &iss);
}

unsigned int CppCheck::check(const FileSettings &fs)
Expand Down Expand Up @@ -811,7 +816,9 @@ unsigned int CppCheck::check(const FileSettings &fs)
}
// need to pass the externally provided ErrorLogger instead of our internal wrapper
CppCheck temp(tempSettings, mSuppressions, mErrorLoggerDirect, mUseGlobalSuppressions, mExecuteCommand);
const unsigned int returnValue = temp.checkFile(fs.file, fs.cfg);
std::stringstream hash;
hash << std::hex << fs.hash;
const unsigned int returnValue = temp.checkFile(fs.file, fs.cfg, hash.str());
if (mUnusedFunctionsCheck)
mUnusedFunctionsCheck->updateFunctionData(*temp.mUnusedFunctionsCheck);
while (!temp.mFileInfo.empty()) {
Expand Down Expand Up @@ -846,7 +853,7 @@ static std::size_t calculateHash(const Preprocessor& preprocessor, const simplec
return preprocessor.calculateHash(tokens, toolinfo.str());
}

unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string &cfgname, std::istream* fileStream)
unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string &cfgname, const std::string &cfgHash, std::istream* fileStream)
{
// TODO: move to constructor when CppCheck no longer owns the settings
if (mSettings.checks.isEnabled(Checks::unusedFunction) && !mUnusedFunctionsCheck)
Expand Down Expand Up @@ -995,9 +1002,9 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string

if (analyzerInformation) {
// Calculate hash so it can be compared with old hash / future hashes
const std::size_t hash = calculateHash(preprocessor, tokens1, mSettings, mSuppressions);
const std::size_t fileHash = calculateHash(preprocessor, tokens1, mSettings, mSuppressions);
std::list<ErrorMessage> errors;
if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, hash, errors)) {
if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgHash, fileHash, errors)) {
while (!errors.empty()) {
mErrorLogger.reportErr(errors.front());
errors.pop_front();
Expand Down Expand Up @@ -1061,7 +1068,7 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
// write dump file xml prolog
std::ofstream fdump;
std::string dumpFile;
createDumpFile(mSettings, file, fdump, dumpFile);
createDumpFile(mSettings, file, fdump, dumpFile, cfgHash);
if (fdump.is_open()) {
fdump << getLibraryDumpData();
fdump << dumpProlog;
Expand Down Expand Up @@ -1794,7 +1801,9 @@ void CppCheck::executeAddonsWholeProgram(const std::list<FileWithDetails> &files
}

for (const auto &f: fileSettings) {
const std::string &dumpFileName = getDumpFileName(mSettings, f.filename());
std::stringstream hash;
hash << std::hex << f.hash;
const std::string &dumpFileName = getDumpFileName(mSettings, f.filename(), hash.str());
ctuInfoFiles.push_back(getCtuInfoFileName(dumpFileName));
}

Expand Down
2 changes: 1 addition & 1 deletion lib/cppcheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class CPPCHECKLIB CppCheck {
* @param fileStream stream the file content can be read from
* @return number of errors found
*/
unsigned int checkFile(const FileWithDetails& file, const std::string &cfgname, std::istream* fileStream = nullptr);
unsigned int checkFile(const FileWithDetails& file, const std::string &cfgname, const std::string &cfgHash = "", std::istream* fileStream = nullptr);

/**
* @brief Check normal tokens
Expand Down
32 changes: 32 additions & 0 deletions lib/filesettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "path.h"
#include "platform.h"
#include "standards.h"
#include "utils.h"

#include <list>
#include <set>
Expand Down Expand Up @@ -82,12 +83,42 @@ class FileWithDetails
struct CPPCHECKLIB FileSettings {
explicit FileSettings(std::string path)
: file(std::move(path))
, hash(0)
{}

FileSettings(std::string path, Standards::Language lang, std::size_t size)
: file(std::move(path), lang, size)
, hash(0)
{}

void computeHash() {
hash = std::hash<std::string>{}(file.path());

hash ^= std::hash<std::string>{}(standard);

for (const auto &undef : undefs) {
hash = rotateLeft(hash, 1);
hash ^= std::hash<std::string>{}(undef);
}

for (const auto &includePath : includePaths) {
hash = rotateLeft(hash, 1);
hash ^= std::hash<std::string>{}(includePath);
}

for (const auto &systemIncludePath : systemIncludePaths) {
hash = rotateLeft(hash, 1);
hash ^= std::hash<std::string>{}(systemIncludePath);
}

for (const auto &define : splitString(defines, ';')) {
hash = rotateLeft(hash, 1);
hash ^= std::hash<std::string>{}(define);
}

hash ^= std::hash<std::string>{}(cfg);
}

std::string cfg;
FileWithDetails file;
const std::string& filename() const
Expand All @@ -110,6 +141,7 @@ struct CPPCHECKLIB FileSettings {
std::list<std::string> systemIncludePaths;
std::string standard;
Platform::Type platformType = Platform::Type::Unspecified;
std::size_t hash;
// TODO: get rid of these
bool msc{};
bool useMfc{};
Expand Down
3 changes: 3 additions & 0 deletions lib/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
/** @brief Do not filter duplicated errors. */
bool emitDuplicates{};

/** @brief Recheck project files with identical path and config. */
bool recheckProjectDuplicates{};

/** @brief Name of the language that is enforced. Empty per default. */
Standards::Language enforcedLang{};

Expand Down
6 changes: 6 additions & 0 deletions lib/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,12 @@ static inline T* empty_if_null(T* p)
return default_if_null(p, "");
}

template<typename T>
static inline T rotateLeft(T value, std::size_t amount)
{
return (value << amount) | (value >> ((8 * sizeof(T)) - (amount % (8 * sizeof(T)))));
}

/**
* Split string by given sperator.
* @param str The string to split
Expand Down
11 changes: 11 additions & 0 deletions man/cppcheck.1.xml
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ man(1), man(7), http://www.tldp.org/HOWTO/Man-Page/
<arg choice="opt">
<option>--config-excludes-file=&lt;file&gt;</option>
</arg>
<arg choice="opt">
<option>--recheck-project-duplicates</option>
</arg>
<arg choice="opt">
<option>--include=&lt;file&gt;</option>
</arg>
Expand Down Expand Up @@ -441,6 +444,14 @@ First given path is searched for contained header files first. If paths are rela
<para>A file that contains a list of config-excludes.</para>
</listitem>
</varlistentry>
<varlistentry>
<term>
<option>--recheck-project-duplicates</option>
</term>
<listitem>
<para>When using the --project option, files with identical names and compiler flags are deduplicated by default. This option forces cppcheck to check all imported files.</para>
</listitem>
</varlistentry>
<varlistentry>
<term>
<option>--include=&lt;file&gt;</option>
Expand Down
12 changes: 12 additions & 0 deletions test/cli/duplicates/compile_commands_duplicates.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
{
"directory": "./.",
"command": "/usr/bin/c++ -std=gnu++11 -c ./main.cpp",
"file": "./main.cpp"
},
{
"directory": "./.",
"command": "/usr/bin/c++ -std=gnu++11 -c ./main.cpp",
"file": "./main.cpp"
}
]
12 changes: 12 additions & 0 deletions test/cli/duplicates/compile_commands_no_duplicates.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
{
"directory": "./.",
"command": "/usr/bin/c++ -std=gnu++11 -c ./main.cpp",
"file": "./main.cpp"
},
{
"directory": "./.",
"command": "/usr/bin/c++ -DHELLO -std=gnu++11 -c ./main.cpp",
"file": "./main.cpp"
}
]
Loading
Loading