Skip to content

Commit 322f04e

Browse files
firewavefirewave
committed
improved errorhandling of analyzer information loading
1 parent 08fb0fc commit 322f04e

9 files changed

Lines changed: 237 additions & 44 deletions

‎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: 97 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>
@@ -82,29 +83,55 @@ void AnalyzerInformation::close()
8283
}
8384
}
8485

85-
bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors)
86+
bool AnalyzerInformation::skipAnalysis(const tinyxml2::XMLDocument &analyzerInfoDoc, std::size_t hash, std::list<ErrorMessage> &errors, bool debug)
8687
{
8788
const tinyxml2::XMLElement * const rootNode = analyzerInfoDoc.FirstChildElement();
88-
if (rootNode == nullptr)
89+
if (rootNode == nullptr) {
90+
if (debug)
91+
std::cout << "discarding cached result - no root node found" << std::endl;
8992
return false;
93+
}
9094

91-
const char *attr = rootNode->Attribute("hash");
92-
if (!attr || attr != std::to_string(hash))
95+
if (strcmp(rootNode->Name(), "analyzerinfo") != 0) {
96+
if (debug)
97+
std::cout << "discarding cached result - unexpected root node" << std::endl;
9398
return false;
99+
}
94100

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

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

110137
return true;
@@ -138,27 +165,43 @@ std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir
138165
filename = sourcefile;
139166
else
140167
filename = sourcefile.substr(pos + 1);
168+
// TODO: is this correct? the above code will return files ending in '.aN'. Also does not consider the ID
141169
return Path::join(buildDir, std::move(filename)) + ".analyzerinfo";
142170
}
143171

144-
bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId, std::size_t hash, std::list<ErrorMessage> &errors)
172+
bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::string &sourcefile, const std::string &cfg, std::size_t fsFileId, std::size_t hash, std::list<ErrorMessage> &errors, bool debug)
145173
{
174+
if (mOutputStream.is_open())
175+
throw std::runtime_error("analyzer information file is already open");
176+
146177
if (buildDir.empty() || sourcefile.empty())
147178
return true;
148-
close();
149179

150180
const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(buildDir,sourcefile,cfg,fsFileId);
151181

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

157200
mOutputStream.open(analyzerInfoFile);
158-
if (mOutputStream.is_open()) {
159-
mOutputStream << "<?xml version=\"1.0\"?>\n";
160-
mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
161-
}
201+
if (!mOutputStream.is_open())
202+
throw std::runtime_error("failed to open '" + analyzerInfoFile + "'");
203+
mOutputStream << "<?xml version=\"1.0\"?>\n";
204+
mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
162205

163206
return true;
164207
}
@@ -175,6 +218,7 @@ void AnalyzerInformation::setFileInfo(const std::string &check, const std::strin
175218
mOutputStream << " <FileInfo check=\"" << check << "\">\n" << fileInfo << " </FileInfo>\n";
176219
}
177220

221+
// TODO: report detailed errors?
178222
bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) {
179223
const std::string::size_type sep1 = filesTxtLine.find(sep);
180224
if (sep1 == std::string::npos)
@@ -202,37 +246,58 @@ bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) {
202246
return true;
203247
}
204248

205-
// TODO: bail out on unexpected data
206-
void AnalyzerInformation::processFilesTxt(const std::string& buildDir, const std::function<void(const char* checkattr, const tinyxml2::XMLElement* e, const Info& filesTxtInfo)>& handler)
249+
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)
207250
{
208251
const std::string filesTxt(buildDir + "/files.txt");
209252
std::ifstream fin(filesTxt.c_str());
210253
std::string filesTxtLine;
211254
while (std::getline(fin, filesTxtLine)) {
212255
AnalyzerInformation::Info filesTxtInfo;
213-
if (!filesTxtInfo.parse(filesTxtLine)) {
214-
return;
215-
}
256+
if (!filesTxtInfo.parse(filesTxtLine))
257+
return "failed to parse '" + filesTxtLine + "' from '" + filesTxt + "'";
258+
259+
if (filesTxtInfo.afile.empty())
260+
return "empty afile from '" + filesTxt + "'";
216261

217262
const std::string xmlfile = buildDir + '/' + filesTxtInfo.afile;
218263

219264
tinyxml2::XMLDocument doc;
220265
const tinyxml2::XMLError error = doc.LoadFile(xmlfile.c_str());
266+
if (error == tinyxml2::XML_ERROR_FILE_NOT_FOUND) {
267+
/* FIXME: this can currently not be reported as an error because:
268+
* - --clang does not generate any analyzer information - see #14456
269+
* - markup files might not generate analyzer information
270+
* - files with preprocessor errors might not generate analyzer information
271+
*/
272+
if (debug)
273+
std::cout << "'" + xmlfile + "' from '" + filesTxt + "' not found";
274+
return "";
275+
}
276+
221277
if (error != tinyxml2::XML_SUCCESS)
222-
return;
278+
return "failed to load '" + xmlfile + "' from '" + filesTxt + "'";
223279

224280
const tinyxml2::XMLElement * const rootNode = doc.FirstChildElement();
225281
if (rootNode == nullptr)
226-
return;
282+
return "no root node found in '" + xmlfile + "' from '" + filesTxt + "'";
283+
284+
if (strcmp(rootNode->Name(), "analyzerinfo") != 0)
285+
return "unexpected root node in '" + xmlfile + "' from '" + filesTxt + "'";
227286

228287
for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
229288
if (std::strcmp(e->Name(), "FileInfo") != 0)
230289
continue;
231290
const char *checkattr = e->Attribute("check");
232-
if (checkattr == nullptr)
291+
if (checkattr == nullptr) {
292+
if (debug)
293+
std::cout << "'check' attribute missing in 'FileInfo' in '" << xmlfile << "' from '" << filesTxt + "'";
233294
continue;
295+
}
234296
handler(checkattr, e, filesTxtInfo);
235297
}
236298
}
299+
300+
// TODO: error on empty file?
301+
return "";
237302
}
238303

‎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, std::size_t fsFileId, 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, std::size_t fsFileId, 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, std::size_t fsFileId);
@@ -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::list<FileSettings> &fileSettings);
8487

8588
static std::string getAnalyzerInfoFileFromFilesTxt(std::istream& filesTxt, const std::string &sourcefile, const std::string &cfg, int fsFileId);
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);
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
@@ -973,7 +973,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
973973
mLogger->setAnalyzerInfo(nullptr);
974974

975975
std::list<ErrorMessage> errors;
976-
analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, file.fsFileId(), hash, errors);
976+
analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, file.fsFileId(), hash, errors, mSettings.debugainfo);
977977
analyzerInformation->setFileInfo("CheckUnusedFunctions", mUnusedFunctionsCheck->analyzerInfo(tokenizer));
978978
analyzerInformation->close();
979979
}
@@ -1019,7 +1019,7 @@ unsigned int CppCheck::checkInternal(const FileWithDetails& file, const std::str
10191019
// Calculate hash so it can be compared with old hash / future hashes
10201020
const std::size_t hash = calculateHash(preprocessor, file.spath());
10211021
std::list<ErrorMessage> errors;
1022-
if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, file.fsFileId(), hash, errors)) {
1022+
if (!analyzerInformation->analyzeFile(mSettings.buildDir, file.spath(), cfgname, file.fsFileId(), hash, errors, mSettings.debugainfo)) {
10231023
while (!errors.empty()) {
10241024
mErrorLogger.reportErr(errors.front());
10251025
errors.pop_front();
@@ -1860,12 +1860,17 @@ unsigned int CppCheck::analyseWholeProgram(const std::string &buildDir, const st
18601860
}
18611861
};
18621862

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

18701875
for (Check::FileInfo *fi : fileInfoList)
18711876
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)