Skip to content

Commit ddce8d5

Browse files
authored
Fix #14433 (Import project: refactor code to read compile_commands.json) (danmar#8030)
There has been multiple issues with the import lately, and (my) fixes have been a bit hacky. With this change, `command` is first split similarily to how the shell would, and is then handled like the `arguments` field. For extracting compiler options, the argument list is handled like the compiler would read `argv`. I believe this will be easier to maintain in the future.
1 parent 31da8a2 commit ddce8d5

File tree

4 files changed

+257
-141
lines changed

4 files changed

+257
-141
lines changed

.github/workflows/selfcheck.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ jobs:
121121
122122
- name: Self check (unusedFunction / no test / no gui)
123123
run: |
124-
supprs="--suppress=unusedFunction:lib/errorlogger.h:196 --suppress=unusedFunction:lib/importproject.cpp:1516 --suppress=unusedFunction:lib/importproject.cpp:1540"
124+
supprs="--suppress=unusedFunction:lib/errorlogger.h:196 --suppress=unusedFunction:lib/importproject.cpp:1530 --suppress=unusedFunction:lib/importproject.cpp:1554"
125125
./cppcheck -q --template=selfcheck --error-exitcode=1 --library=cppcheck-lib -D__CPPCHECK__ -D__GNUC__ --enable=unusedFunction,information --exception-handling -rp=. --project=cmake.output.notest_nogui/compile_commands.json --suppressions-list=.selfcheck_unused_suppressions --inline-suppr $supprs
126126
env:
127127
DISABLE_VALUEFLOW: 1

lib/importproject.cpp

Lines changed: 153 additions & 139 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,148 @@
4242

4343
#include "json.h"
4444

45+
std::string ImportProject::collectArgs(const std::string &cmd, std::vector<std::string> &args)
46+
{
47+
args.clear();
48+
49+
std::string::size_type pos = 0;
50+
const std::string::size_type end = cmd.size();
51+
std::string arg;
52+
53+
bool inDoubleQuotes = false;
54+
bool inSingleQuotes = false;
55+
56+
while (pos < end) {
57+
char c = cmd[pos++];
58+
59+
if (c == ' ') {
60+
if (inDoubleQuotes || inSingleQuotes) {
61+
arg.push_back(c);
62+
continue;
63+
}
64+
65+
if (!arg.empty())
66+
args.push_back(arg);
67+
arg.clear();
68+
69+
pos = cmd.find_first_not_of(' ', pos);
70+
71+
continue;
72+
}
73+
74+
if (c == '\"' && !inSingleQuotes) {
75+
inDoubleQuotes = !inDoubleQuotes;
76+
continue;
77+
}
78+
79+
if (c == '\'' && !inDoubleQuotes) {
80+
inSingleQuotes = !inSingleQuotes;
81+
continue;
82+
}
83+
84+
if (c == '\\' && !inSingleQuotes) {
85+
if (pos == end) {
86+
arg.push_back('\\');
87+
break;
88+
}
89+
90+
c = cmd[pos++];
91+
92+
if (!std::strchr("\\\"\' ", c))
93+
arg.push_back('\\');
94+
95+
arg.push_back(c);
96+
continue;
97+
}
98+
99+
arg.push_back(c);
100+
}
101+
102+
if (inSingleQuotes || inDoubleQuotes)
103+
return "Missing closing quote in command string";
104+
105+
if (!arg.empty())
106+
args.push_back(arg);
107+
108+
return "";
109+
}
110+
111+
void ImportProject::parseArgs(FileSettings &fs, const std::vector<std::string> &args)
112+
{
113+
const auto getOptArg = [&args](std::initializer_list<std::string> optNames,
114+
std::size_t &i) {
115+
const auto &arg = args[i];
116+
const auto *const it = std::find_if(optNames.begin(),
117+
optNames.end(),
118+
[&arg] (const std::string &optName) {
119+
return startsWith(arg, optName);
120+
});
121+
122+
if (it == optNames.end())
123+
return std::string();
124+
125+
const std::size_t optLen = it->size();
126+
if (arg.size() == optLen)
127+
return ++i >= args.size() ? std::string() : args[i];
128+
129+
return arg.substr(optLen);
130+
};
131+
132+
std::string defs;
133+
for (std::size_t i = 0; i < args.size(); i++) {
134+
std::string optArg;
135+
136+
if (!(optArg = getOptArg({ "-I", "/I" }, i)).empty()) {
137+
if (std::none_of(fs.includePaths.cbegin(), fs.includePaths.cend(),
138+
[&](const std::string &path) {
139+
return path == optArg;
140+
}))
141+
fs.includePaths.push_back(std::move(optArg));
142+
continue;
143+
}
144+
145+
if (!(optArg = getOptArg({ "-isystem" }, i)).empty()) {
146+
fs.systemIncludePaths.push_back(std::move(optArg));
147+
continue;
148+
}
149+
150+
if (!(optArg = getOptArg({ "-D", "/D" }, i)).empty()) {
151+
defs += optArg + ";";
152+
continue;
153+
}
154+
155+
if (!(optArg = getOptArg({ "-U", "/U" }, i)).empty()) {
156+
fs.undefs.insert(std::move(optArg));
157+
continue;
158+
}
159+
160+
if (!(optArg = getOptArg({ "-std=", "/std:" }, i)).empty()) {
161+
fs.standard = std::move(optArg);
162+
continue;
163+
}
164+
165+
if (!(optArg = getOptArg({ "-f" }, i)).empty()) {
166+
if (optArg == "pic")
167+
defs += "__pic__;";
168+
else if (optArg == "PIC")
169+
defs += "__PIC__;";
170+
else if (optArg == "pie")
171+
defs += "__pie__;";
172+
else if (optArg == "PIE")
173+
defs += "__PIE__;";
174+
continue;
175+
}
176+
177+
if (!(optArg = getOptArg({ "-m" }, i)).empty()) {
178+
if (optArg == "unicode")
179+
defs += "UNICODE;";
180+
continue;
181+
}
182+
}
183+
184+
fsSetDefines(fs, std::move(defs));
185+
}
186+
45187
void ImportProject::ignorePaths(const std::vector<std::string> &ipaths, bool debug)
46188
{
47189
PathMatch matcher(ipaths, Path::getCurrentPath());
@@ -210,134 +352,6 @@ ImportProject::Type ImportProject::import(const std::string &filename, Settings
210352
return ImportProject::Type::FAILURE;
211353
}
212354

213-
static std::string readUntil(const std::string &command, std::string::size_type *pos, const char until[], bool str = false)
214-
{
215-
std::string ret;
216-
bool escapedString = false;
217-
bool escape = false;
218-
for (; *pos < command.size() && (str || !std::strchr(until, command[*pos])); (*pos)++) {
219-
if (escape)
220-
escape = false;
221-
else if (command[*pos] == '\\') {
222-
if (str)
223-
escape = true;
224-
else if (command[*pos + 1] == '"') {
225-
if (escapedString)
226-
return ret + "\\\"";
227-
escapedString = true;
228-
ret += "\\\"";
229-
(*pos)++;
230-
continue;
231-
}
232-
} else if (command[*pos] == '\"')
233-
str = !str;
234-
ret += command[*pos];
235-
}
236-
return ret;
237-
}
238-
239-
static std::string unescape(const std::string &in)
240-
{
241-
std::string out;
242-
bool escape = false;
243-
for (const char c: in) {
244-
if (escape) {
245-
escape = false;
246-
if (!std::strchr("\\\"\'",c))
247-
out += "\\";
248-
out += c;
249-
} else if (c == '\\')
250-
escape = true;
251-
else
252-
out += c;
253-
}
254-
return out;
255-
}
256-
257-
void ImportProject::fsParseCommand(FileSettings& fs, const std::string& command, bool doUnescape)
258-
{
259-
std::string defs;
260-
261-
// Parse command..
262-
std::string::size_type pos = 0;
263-
while (std::string::npos != (pos = command.find(' ',pos))) {
264-
while (pos < command.size() && command[pos] == ' ')
265-
pos++;
266-
if (pos >= command.size())
267-
break;
268-
bool wholeArgQuoted = false;
269-
if (command[pos] == '"') {
270-
wholeArgQuoted = true;
271-
pos++;
272-
if (pos >= command.size())
273-
break;
274-
}
275-
if (command[pos] != '/' && command[pos] != '-')
276-
continue;
277-
pos++;
278-
if (pos >= command.size())
279-
break;
280-
const char F = command[pos++];
281-
if (std::strchr("DUI", F)) {
282-
while (pos < command.size() && command[pos] == ' ')
283-
++pos;
284-
}
285-
std::string fval = readUntil(command, &pos, " =", wholeArgQuoted);
286-
if (wholeArgQuoted && fval.back() == '\"')
287-
fval.resize(fval.size() - 1);
288-
if (F=='D') {
289-
std::string defval = readUntil(command, &pos, " ");
290-
defs += fval;
291-
if (doUnescape) {
292-
if (defval.size() >= 3 && startsWith(defval,"=\"") && defval.back()=='\"')
293-
defval = "=" + unescape(defval.substr(2, defval.size() - 3));
294-
else if (defval.size() >= 5 && startsWith(defval, "=\\\"") && endsWith(defval, "\\\""))
295-
defval = "=\"" + unescape(defval.substr(3, defval.size() - 5)) + "\"";
296-
}
297-
if (!defval.empty())
298-
defs += defval;
299-
defs += ';';
300-
} else if (F=='U')
301-
fs.undefs.insert(std::move(fval));
302-
else if (F=='I') {
303-
std::string i = std::move(fval);
304-
if (i.size() > 1 && i[0] == '\"' && i.back() == '\"')
305-
i = unescape(i.substr(1, i.size() - 2));
306-
if (std::find(fs.includePaths.cbegin(), fs.includePaths.cend(), i) == fs.includePaths.cend())
307-
fs.includePaths.push_back(std::move(i));
308-
} else if (F=='s' && startsWith(fval,"td")) {
309-
++pos;
310-
fs.standard = readUntil(command, &pos, " ");
311-
} else if (F == 'i' && fval == "system") {
312-
++pos;
313-
std::string isystem = readUntil(command, &pos, " ");
314-
fs.systemIncludePaths.push_back(std::move(isystem));
315-
} else if (F=='m') {
316-
if (fval == "unicode") {
317-
defs += "UNICODE";
318-
defs += ";";
319-
}
320-
} else if (F=='f') {
321-
if (fval == "pic") {
322-
defs += "__pic__";
323-
defs += ";";
324-
} else if (fval == "PIC") {
325-
defs += "__PIC__";
326-
defs += ";";
327-
} else if (fval == "pie") {
328-
defs += "__pie__";
329-
defs += ";";
330-
} else if (fval == "PIE") {
331-
defs += "__PIE__";
332-
defs += ";";
333-
}
334-
// TODO: support -fsigned-char and -funsigned-char?
335-
// we can only set it globally but in this context it needs to be treated per file
336-
}
337-
}
338-
fsSetDefines(fs, std::move(defs));
339-
}
340-
341355
bool ImportProject::importCompileCommands(std::istream &istr)
342356
{
343357
picojson::value compileCommands;
@@ -371,31 +385,31 @@ bool ImportProject::importCompileCommands(std::istream &istr)
371385

372386
const std::string directory = std::move(dirpath);
373387

374-
bool doUnescape = false;
375-
std::string command;
388+
std::vector<std::string> arguments;
376389
if (obj.count("arguments")) {
377-
doUnescape = false;
378390
if (obj["arguments"].is<picojson::array>()) {
379391
for (const picojson::value& arg : obj["arguments"].get<picojson::array>()) {
380-
if (arg.is<std::string>()) {
381-
std::string str = arg.get<std::string>();
382-
if (str.find(' ') != std::string::npos)
383-
str = "\"" + str + "\"";
384-
command += str + " ";
385-
}
392+
if (arg.is<std::string>())
393+
arguments.push_back(arg.get<std::string>());
386394
}
387395
} else {
388396
errors.emplace_back("'arguments' field in compilation database entry is not a JSON array");
389397
return false;
390398
}
391399
} else if (obj.count("command")) {
392-
doUnescape = true;
400+
std::string command;
393401
if (obj["command"].is<std::string>()) {
394402
command = obj["command"].get<std::string>();
395403
} else {
396404
errors.emplace_back("'command' field in compilation database entry is not a string");
397405
return false;
398406
}
407+
408+
std::string error = collectArgs(command, arguments);
409+
if (!error.empty()) {
410+
errors.emplace_back(error);
411+
return false;
412+
}
399413
} else {
400414
errors.emplace_back("no 'arguments' or 'command' field found in compilation database entry");
401415
return false;
@@ -425,7 +439,7 @@ bool ImportProject::importCompileCommands(std::istream &istr)
425439
else
426440
path = Path::simplifyPath(directory + file);
427441
FileSettings fs{path, Standards::Language::None, 0}; // file will be identified later on
428-
fsParseCommand(fs, command, doUnescape); // read settings; -D, -I, -U, -std, -m*, -f*
442+
parseArgs(fs, arguments);
429443
std::map<std::string, std::string, cppcheck::stricmp> variables;
430444
fsSetIncludePaths(fs, directory, fs.includePaths, variables);
431445
// Assign a unique index to each file path. If the file path already exists in the map,

lib/importproject.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ class CPPCHECKLIB WARN_UNUSED ImportProject {
6969
CPPCHECK_GUI
7070
};
7171

72-
static void fsParseCommand(FileSettings& fs, const std::string& command, bool doUnescape);
7372
static void fsSetDefines(FileSettings& fs, std::string defs);
7473
static void fsSetIncludePaths(FileSettings& fs, const std::string &basepath, const std::list<std::string> &in, std::map<std::string, std::string, cppcheck::stricmp> &variables);
7574

@@ -103,6 +102,8 @@ class CPPCHECKLIB WARN_UNUSED ImportProject {
103102
protected:
104103
bool importCompileCommands(std::istream &istr);
105104
bool importCppcheckGuiProject(std::istream &istr, Settings &settings, Suppressions &supprs);
105+
static std::string collectArgs(const std::string &cmd, std::vector<std::string> &args);
106+
static void parseArgs(FileSettings &fs, const std::vector<std::string> &args);
106107

107108
private:
108109
struct SharedItemsProject {

0 commit comments

Comments
 (0)