Skip to content

Commit 1653983

Browse files
firewavefirewave
committed
improved errorhandling of analyzer information loading [skip ci]
1 parent a438097 commit 1653983

File tree

10 files changed

+228
-45
lines changed

10 files changed

+228
-45
lines changed

‎cli/cmdlineparser.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,9 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
621621
mSettings.cppHeaderProbe = true;
622622
}
623623

624+
else if (std::strcmp(argv[i], "--debug-analyzerinfo") == 0)
625+
mSettings.debugainfo = true;
626+
624627
else if (std::strcmp(argv[i], "--debug-ast") == 0)
625628
mSettings.debugast = true;
626629

‎cli/cppcheckexecutor.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ int CppCheckExecutor::check_internal(const Settings& settings, Suppressions& sup
427427

428428
returnValue |= cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings, stdLogger.getCtuInfo());
429429

430-
if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) {
430+
if ((settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) && !supprs.nomsg.getSuppressions().empty()) {
431431
const bool err = reportUnmatchedSuppressions(settings, supprs.nomsg, mFiles, mFileSettings, stdLogger);
432432
if (err && returnValue == 0)
433433
returnValue = settings.exitCode;

‎lib/analyzerinfo.cpp‎

Lines changed: 89 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
#include <cstring>
2727
#include <exception>
28+
#include <iostream>
2829
#include <map>
2930
#include <sstream>
3031
#include <utility>
@@ -84,34 +85,61 @@ void AnalyzerInformation::close()
8485
}
8586
}
8687

87-
bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors)
88+
bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors, bool debug)
8889
{
8990
const tinyxml2::XMLElement * const rootNode = analyzerInfoDoc.FirstChildElement();
90-
if (rootNode == nullptr)
91+
if (rootNode == nullptr) {
92+
if (debug)
93+
std::cout << "discarding cached result - no root node found" << std::endl;
9194
return false;
95+
}
9296

93-
const char *attr = rootNode->Attribute("hash");
94-
if (!attr || attr != std::to_string(hash))
97+
if (strcmp(rootNode->Name(), "analyzerinfo") != 0) {
98+
if (debug)
99+
std::cout << "discarding cached result - unexpected root node" << std::endl;
95100
return false;
101+
}
96102

97-
// Check for invalid license error or internal error, in which case we should retry analysis
98-
for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
99-
if (std::strcmp(e->Name(), "error") == 0 &&
100-
(e->Attribute("id", "premium-invalidLicense") ||
101-
e->Attribute("id", "premium-internalError") ||
102-
e->Attribute("id", "internalError")
103-
))
104-
return false;
103+
const char * const attr = rootNode->Attribute("hash");
104+
if (!attr) {
105+
if (debug)
106+
std::cout << "discarding cached result - no 'hash' attribute found" << std::endl;
107+
return false;
108+
}
109+
if (attr != std::to_string(hash)) {
110+
if (debug)
111+
std::cout << "discarding cached result - hash mismatch" << std::endl;
112+
return false;
105113
}
106114

107115
for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
108-
if (std::strcmp(e->Name(), "error") == 0)
109-
errors.emplace_back(e);
116+
if (std::strcmp(e->Name(), "error") != 0)
117+
continue;
118+
119+
// TODO: discarding results on internalError doesn't make sense since that won't fix itself
120+
// Check for invalid license error or internal error, in which case we should retry analysis
121+
static std::array<const char*, 3> s_ids{
122+
"premium-invalidLicense",
123+
"premium-internalError",
124+
"internalError"
125+
};
126+
for (const auto* id : s_ids)
127+
{
128+
if (e->Attribute("id", id)) {
129+
if (debug)
130+
std::cout << "discarding cached result - '" << id << "' encountered" << std::endl;
131+
errors.clear();
132+
return false;
133+
}
134+
}
135+
136+
errors.emplace_back(e);
110137
}
111138

112139
return true;
113140
}
114141

142+
// TODO: report errors
115143
std::string AnalyzerInformation::getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg, int fileIndex)
116144
{
117145
const std::string id = (fileIndex > 0) ? std::to_string(fileIndex) : "";
@@ -145,24 +173,39 @@ std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir
145173
return Path::join(buildDir, std::move(filename)) + ".analyzerinfo";
146174
}
147175

148-
bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex, std::size_t hash, std::list<ErrorMessage> &errors)
176+
bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex, std::size_t hash, std::list<ErrorMessage> &errors, bool debug)
149177
{
178+
if (mOutputStream.is_open())
179+
throw std::runtime_error("analyzer information file is already open");
180+
150181
if (buildDir.empty() || sourcefile.empty())
151182
return true;
152-
close();
153183

154184
const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfg,fileIndex);
155185

156-
tinyxml2::XMLDocument analyzerInfoDoc;
157-
const tinyxml2::XMLError xmlError = analyzerInfoDoc.LoadFile(analyzerInfoFile.c_str());
158-
if (xmlError == tinyxml2::XML_SUCCESS && skipAnalysis(analyzerInfoDoc, hash, errors))
159-
return false;
186+
{
187+
tinyxml2::XMLDocument analyzerInfoDoc;
188+
const tinyxml2::XMLError xmlError = analyzerInfoDoc.LoadFile(analyzerInfoFile.c_str());
189+
if (xmlError == tinyxml2::XML_SUCCESS) {
190+
if (skipAnalysis(analyzerInfoDoc, hash, errors, debug)) {
191+
if (debug)
192+
std::cout << "skipping analysis - loaded " << errors.size() << " cached finding(s) from '" << analyzerInfoFile << "'" << std::endl;
193+
return false;
194+
}
195+
}
196+
else if (xmlError != tinyxml2::XML_ERROR_FILE_NOT_FOUND) {
197+
if (debug)
198+
std::cout << "discarding cached result - failed to load '" << analyzerInfoFile << "' (" << tinyxml2::XMLDocument::ErrorIDToName(xmlError) << ")" << std::endl;
199+
}
200+
else if (debug)
201+
std::cout << "no cached result '" << analyzerInfoFile << "' found" << std::endl;
202+
}
160203

161204
mOutputStream.open(analyzerInfoFile);
162-
if (mOutputStream.is_open()) {
163-
mOutputStream << "<?xml version=\"1.0\"?>\n";
164-
mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
165-
}
205+
if (!mOutputStream.is_open())
206+
throw std::runtime_error("failed to open '" + analyzerInfoFile + "'");
207+
mOutputStream << "<?xml version=\"1.0\"?>\n";
208+
mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
166209

167210
return true;
168211
}
@@ -179,6 +222,7 @@ void AnalyzerInformation::setFileInfo(const std::string &check, const std::strin
179222
mOutputStream << " <FileInfo check=\"" << check << "\">\n" << fileInfo << " </FileInfo>\n";
180223
}
181224

225+
// TODO: report detailed error
182226
bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) {
183227
const std::string::size_type sep1 = filesTxtLine.find(sep);
184228
if (sep1 == std::string::npos)
@@ -206,37 +250,50 @@ bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) {
206250
return true;
207251
}
208252

209-
// TODO: bail out on unexpected data
210-
void AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std::function<void(const char* checkattr, const tinyxml2::XMLElement* e, const Info& filesTxtInfo)>& handler)
253+
std::string AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std::function<void(const char* checkattr, const tinyxml2::XMLElement* e, const Info& filesTxtInfo)>& handler, bool debug)
211254
{
212255
const std::string filesTxt(buildDir + "/files.txt");
213256
std::ifstream fin(filesTxt.c_str());
214257
std::string filesTxtLine;
215258
while (std::getline(fin, filesTxtLine)) {
216259
AnalyzerInformation::Info filesTxtInfo;
217-
if (!filesTxtInfo.parse(filesTxtLine)) {
218-
return;
219-
}
260+
if (!filesTxtInfo.parse(filesTxtLine))
261+
return "failed to parse '" + filesTxtLine + "' from '" + filesTxt + "'";
262+
263+
if (filesTxtInfo.afile.empty())
264+
return "empty afile from '" + filesTxt + "'";
220265

221266
const std::string xmlfile = buildDir + '/' + filesTxtInfo.afile;
222267

223268
tinyxml2::XMLDocument doc;
224269
const tinyxml2::XMLError error = doc.LoadFile(xmlfile.c_str());
270+
if (error == tinyxml2::XML_ERROR_FILE_NOT_FOUND)
271+
return "'" + xmlfile + "' from '" + filesTxt + "' not found";
272+
225273
if (error != tinyxml2::XML_SUCCESS)
226-
return;
274+
return "failed to load '" + xmlfile + "' from '" + filesTxt + "'";
227275

228276
const tinyxml2::XMLElement * const rootNode = doc.FirstChildElement();
229277
if (rootNode == nullptr)
230-
return;
278+
return "no root node found in '" + xmlfile + "' from '" + filesTxt + "'";
279+
280+
if (strcmp(rootNode->Name(), "analyzerinfo") != 0)
281+
return "unexpected root node in '" + xmlfile + "' from '" + filesTxt + "'";
231282

232283
for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
233284
if (std::strcmp(e->Name(), "FileInfo") != 0)
234285
continue;
235286
const char *checkattr = e->Attribute("check");
236-
if (checkattr == nullptr)
287+
if (checkattr == nullptr) {
288+
if (debug)
289+
std::cout << "'check' attribute missing in 'FileInfo' in '" << xmlfile << "' from '" << filesTxt + "'";
237290
continue;
291+
}
238292
handler(checkattr, e, filesTxtInfo);
239293
}
240294
}
295+
296+
// TODO: error on empty file?
297+
return "";
241298
}
242299

‎lib/analyzerinfo.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@ class CPPCHECKLIB AnalyzerInformation {
6161

6262
/** Close current TU.analyzerinfo file */
6363
void close();
64-
bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex, std::size_t hash, std::list<ErrorMessage> &errors);
64+
/**
65+
* @throws std::runtime_error thrown if the output file is already open or the output file cannot be opened
66+
*/
67+
bool analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex, std::size_t hash, std::list<ErrorMessage> &errors, bool debug = false);
6568
void reportErr(const ErrorMessage &msg);
6669
void setFileInfo(const std::string &check, const std::string &fileInfo);
6770
static std::string getAnalyzerInfoFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, int fileIndex);
@@ -77,14 +80,14 @@ class CPPCHECKLIB AnalyzerInformation {
7780
std::string sourceFile;
7881
};
7982

80-
static void processFilesTxt(const std::string& buildDir, const std::function<void(const char* checkattr, const tinyxml2::XMLElement* e, const Info& filesTxtInfo)>& handler);
83+
static std::string processFilesTxt(const std::string& buildDir, const std::function<void(const char* checkattr, const tinyxml2::XMLElement* e, const Info& filesTxtInfo)>& handler, bool debug = false);
8184

8285
protected:
8386
static std::string getFilesTxt(const std::list<std::string> &sourcefiles, const std::string &userDefines, const std::list<FileSettings> &fileSettings);
8487

8588
static std::string getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg, int fileIndex);
8689

87-
static bool skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors);
90+
static bool skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors, bool debug = false);
8891

8992
private:
9093
std::ofstream mOutputStream;

‎lib/checkunusedfunctions.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,12 @@ void CheckUnusedFunctions::analyseWholeProgram(const Settings &settings, ErrorLo
482482
}
483483
};
484484

485-
AnalyzerInformation::processFilesTxt(buildDir, handler);
485+
const std::string err = AnalyzerInformation::processFilesTxt(buildDir, handler, settings.debugainfo);
486+
if (!err.empty()) {
487+
const ErrorMessage errmsg({}, "", Severity::error, err, "internalError", Certainty::normal); // TODO: set filename
488+
errorLogger.reportErr(errmsg);
489+
return;
490+
}
486491

487492
for (auto decl = decls.cbegin(); decl != decls.cend(); ++decl) {
488493
const std::string &functionName = stripTemplateParameters(decl->first);

‎lib/cppcheck.cpp‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -974,7 +974,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
974974
mLogger->setAnalyzerInfo(nullptr);
975975

976976
std::list<ErrorMessage> errors;
977-
analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, fileIndex, hash, errors);
977+
analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, fileIndex, hash, errors, mSettings.debugainfo);
978978
analyzerInformation->setFileInfo("CheckUnusedFunctions", mUnusedFunctionsCheck->analyzerInfo(tokenizer));
979979
analyzerInformation->close();
980980
}
@@ -1020,7 +1020,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
10201020
// Calculate hash so it can be compared with old hash / future hashes
10211021
const std::size_t hash = calculateHash(preprocessor, file.spath());
10221022
std::list<ErrorMessage> errors;
1023-
if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, fileIndex, hash, errors)) {
1023+
if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, fileIndex, hash, errors, mSettings.debugainfo)) {
10241024
while (!errors.empty()) {
10251025
mErrorLogger.reportErr(errors.front());
10261026
errors.pop_front();
@@ -1861,12 +1861,17 @@ unsigned int CppCheck::analyseWholeProgram(const std::string &buildDir, const st
18611861
}
18621862
};
18631863

1864-
AnalyzerInformation::processFilesTxt(buildDir, handler);
1865-
1866-
// Analyse the tokens
1867-
// cppcheck-suppress shadowFunction - TODO: fix this
1868-
for (Check *check : Check::instances())
1869-
check->analyseWholeProgram(ctuFileInfo, fileInfoList, mSettings, mErrorLogger);
1864+
const std::string err = AnalyzerInformation::processFilesTxt(buildDir, handler, mSettings.debugainfo);
1865+
if (!err.empty()) {
1866+
const ErrorMessage errmsg({}, "", Severity::error, err, "internalError", Certainty::normal); // TODO: set filename
1867+
mErrorLogger.reportErr(errmsg);
1868+
}
1869+
else {
1870+
// Analyse the tokens
1871+
// cppcheck-suppress shadowFunction - TODO: fix this
1872+
for (Check *check : Check::instances())
1873+
check->analyseWholeProgram(ctuFileInfo, fileInfoList, mSettings, mErrorLogger);
1874+
}
18701875

18711876
for (Check::FileInfo *fi : fileInfoList)
18721877
delete fi;

‎lib/errorlogger.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@ ErrorMessage::ErrorMessage(ErrorPath errorPath, const TokenList *tokenList, Seve
162162
// hash = calculateWarningHash(tokenList, hashWarning.str());
163163
}
164164

165+
// TODO: improve errorhandling?
165166
ErrorMessage::ErrorMessage(const tinyxml2::XMLElement * const errmsg)
166167
: severity(Severity::none),
167168
cwe(0U),

‎lib/settings.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
189189
/** @brief Are we running from DACA script? */
190190
bool daca{};
191191

192+
/** @brief Is --debug-analyzerinfo given? */
193+
bool debugainfo{};
194+
192195
/** @brief Is --debug-ast given? */
193196
bool debugast{};
194197

0 commit comments

Comments
 (0)