Skip to content

Commit 35adf09

Browse files
firewavefirewave
committed
improved errorhandling of analyzer information loading
1 parent 5fc51d1 commit 35adf09

File tree

9 files changed

+238
-44
lines changed

9 files changed

+238
-44
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: 98 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,56 @@ 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 const 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+
// cppcheck-suppress useStlAlgorithm
127+
if (e->Attribute("id", id)) {
128+
if (debug)
129+
std::cout << "discarding cached result - '" << id << "' encountered" << std::endl;
130+
errors.clear();
131+
return false;
132+
}
133+
}
134+
135+
errors.emplace_back(e);
108136
}
109137

110138
return true;
@@ -138,27 +166,43 @@ std::string AnalyzerInformation::getAnalyzerInfoFile(const std::string &buildDir
138166
filename = sourcefile;
139167
else
140168
filename = sourcefile.substr(pos + 1);
169+
// TODO: is this correct? the above code will return files ending in '.aN'. Also does not consider the ID
141170
return Path::join(buildDir, std::move(filename)) + ".analyzerinfo";
142171
}
143172

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)
173+
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)
145174
{
175+
if (mOutputStream.is_open())
176+
throw std::runtime_error("analyzer information file is already open");
177+
146178
if (buildDir.empty() || sourcefile.empty())
147179
return true;
148-
close();
149180

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

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;
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+
}
156200

157201
mOutputStream.open(analyzerInfoFile);
158-
if (mOutputStream.is_open()) {
159-
mOutputStream << "<?xml version=\"1.0\"?>\n";
160-
mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";
161-
}
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";
162206

163207
return true;
164208
}
@@ -175,6 +219,7 @@ void AnalyzerInformation::setFileInfo(const std::string &check, const std::strin
175219
mOutputStream << " <FileInfo check=\"" << check << "\">\n" << fileInfo << " </FileInfo>\n";
176220
}
177221

222+
// TODO: report detailed errors?
178223
bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) {
179224
const std::string::size_type sep1 = filesTxtLine.find(sep);
180225
if (sep1 == std::string::npos)
@@ -202,37 +247,58 @@ bool AnalyzerInformation::Info::parse(const std::string& filesTxtLine) {
202247
return true;
203248
}
204249

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)
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)
207251
{
208252
const std::string filesTxt(buildDir + "/files.txt");
209253
std::ifstream fin(filesTxt.c_str());
210254
std::string filesTxtLine;
211255
while (std::getline(fin, filesTxtLine)) {
212256
AnalyzerInformation::Info filesTxtInfo;
213-
if (!filesTxtInfo.parse(filesTxtLine)) {
214-
return;
215-
}
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 + "'";
216262

217263
const std::string xmlfile = buildDir + '/' + filesTxtInfo.afile;
218264

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

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

228288
for (const tinyxml2::XMLElement *e = rootNode->FirstChildElement(); e; e = e->NextSiblingElement()) {
229289
if (std::strcmp(e->Name(), "FileInfo") != 0)
230290
continue;
231291
const char *checkattr = e->Attribute("check");
232-
if (checkattr == nullptr)
292+
if (checkattr == nullptr) {
293+
if (debug)
294+
std::cout << "'check' attribute missing in 'FileInfo' in '" << xmlfile << "' from '" << filesTxt + "'";
233295
continue;
296+
}
234297
handler(checkattr, e, filesTxtInfo);
235298
}
236299
}
300+
301+
// TODO: error on empty file?
302+
return "";
237303
}
238304

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