Skip to content

Commit 34606a2

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

File tree

9 files changed

+231
-51
lines changed

9 files changed

+231
-51
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

‎lib/analyzerinfo.cpp‎

Lines changed: 93 additions & 39 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,45 +85,69 @@ 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

115142
std::string AnalyzerInformation::getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg, int fileIndex)
116143
{
117-
const std::string id = (fileIndex > 0) ? std::to_string(fileIndex) : "";
118144
std::string line;
119-
const std::string end(sep + cfg + sep + id + sep + Path::simplifyPath(sourcefile));
120145
while (std::getline(filesTxt,line)) {
121-
if (line.size() <= end.size() + 2U)
122-
continue;
123-
if (!endsWith(line, end.c_str(), end.size()))
124-
continue;
125-
return line.substr(0,line.find(sep));
146+
AnalyzerInformation::Info filesTxtInfo;
147+
if (!filesTxtInfo.parse(line))
148+
continue; // TODO: report error?
149+
if (filesTxtInfo.sourceFile == sourcefile && filesTxtInfo.cfg == cfg && filesTxtInfo.fileIndex == fileIndex)
150+
return filesTxtInfo.afile;
126151
}
127152
return "";
128153
}
@@ -145,24 +170,39 @@ std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir
145170
return Path::join(buildDir, std::move(filename)) + ".analyzerinfo";
146171
}
147172

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)
173+
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)
149174
{
175+
if (mOutputStream.is_open())
176+
throw std::runtime_error("analyzer information file is already open");
177+
150178
if (buildDir.empty() || sourcefile.empty())
151179
return true;
152-
close();
153180

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

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;
183+
{
184+
tinyxml2::XMLDocument analyzerInfoDoc;
185+
const tinyxml2::XMLError xmlError = analyzerInfoDoc.LoadFile(analyzerInfoFile.c_str());
186+
if (xmlError == tinyxml2::XML_SUCCESS) {
187+
if (skipAnalysis(analyzerInfoDoc, hash, errors, debug)) {
188+
if (debug)
189+
std::cout << "skipping analysis - loaded " << errors.size() << " cached finding(s) from '" << analyzerInfoFile << "'" << std::endl;
190+
return false;
191+
}
192+
}
193+
else if (xmlError != tinyxml2::XML_ERROR_FILE_NOT_FOUND) {
194+
if (debug)
195+
std::cout << "discarding cached result - failed to load '" << analyzerInfoFile << "' (" << tinyxml2::XMLDocument::ErrorIDToName(xmlError) << ")" << std::endl;
196+
}
197+
else if (debug)
198+
std::cout << "no cached result '" << analyzerInfoFile << "' found" << std::endl;
199+
}
160200

161201
mOutputStream.open(analyzerInfoFile);
162-
if (mOutputStream.is_open()) {
163-
mOutputStream << "<?xml version=\"1.0\"?>\n";
164-
mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
165-
}
202+
if (!mOutputStream.is_open())
203+
throw std::runtime_error("failed to open '" + analyzerInfoFile + "'");
204+
mOutputStream << "<?xml version=\"1.0\"?>\n";
205+
mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
166206

167207
return true;
168208
}
@@ -179,6 +219,7 @@ void AnalyzerInformation::setFileInfo(const std::string &check, const std::strin
179219
mOutputStream << " <FileInfo check=\"" << check << "\">\n" << fileInfo << " </FileInfo>\n";
180220
}
181221

222+
// TODO: report detailed errors?
182223
bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) {
183224
const std::string::size_type sep1 = filesTxtLine.find(sep);
184225
if (sep1 == std::string::npos)
@@ -206,37 +247,50 @@ bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) {
206247
return true;
207248
}
208249

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)
250+
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)
211251
{
212252
const std::string filesTxt(buildDir + "/files.txt");
213253
std::ifstream fin(filesTxt.c_str());
214254
std::string filesTxtLine;
215255
while (std::getline(fin, filesTxtLine)) {
216256
AnalyzerInformation::Info filesTxtInfo;
217-
if (!filesTxtInfo.parse(filesTxtLine)) {
218-
return;
219-
}
257+
if (!filesTxtInfo.parse(filesTxtLine))
258+
return "failed to parse '" + filesTxtLine + "' from '" + filesTxt + "'";
259+
260+
if (filesTxtInfo.afile.empty())
261+
return "empty afile from '" + filesTxt + "'";
220262

221263
const std::string xmlfile = buildDir + '/' + filesTxtInfo.afile;
222264

223265
tinyxml2::XMLDocument doc;
224266
const tinyxml2::XMLError error = doc.LoadFile(xmlfile.c_str());
267+
if (error == tinyxml2::XML_ERROR_FILE_NOT_FOUND)
268+
return "'" + xmlfile + "' from '" + filesTxt + "' not found";
269+
225270
if (error != tinyxml2::XML_SUCCESS)
226-
return;
271+
return "failed to load '" + xmlfile + "' from '" + filesTxt + "'";
227272

228273
const tinyxml2::XMLElement * const rootNode = doc.FirstChildElement();
229274
if (rootNode == nullptr)
230-
return;
275+
return "no root node found in '" + xmlfile + "' from '" + filesTxt + "'";
276+
277+
if (strcmp(rootNode->Name(), "analyzerinfo") != 0)
278+
return "unexpected root node in '" + xmlfile + "' from '" + filesTxt + "'";
231279

232280
for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
233281
if (std::strcmp(e->Name(), "FileInfo") != 0)
234282
continue;
235283
const char *checkattr = e->Attribute("check");
236-
if (checkattr == nullptr)
284+
if (checkattr == nullptr) {
285+
if (debug)
286+
std::cout << "'check' attribute missing in 'FileInfo' in '" << xmlfile << "' from '" << filesTxt + "'";
237287
continue;
288+
}
238289
handler(checkattr, e, filesTxtInfo);
239290
}
240291
}
292+
293+
// TODO: error on empty file?
294+
return "";
241295
}
242296

‎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)