Skip to content

Commit 0962f1d

Browse files
committed
[clangd] Quote/escape argv included in log messages.
clangd/clangd#637
1 parent 095f086 commit 0962f1d

File tree

7 files changed

+51
-12
lines changed

7 files changed

+51
-12
lines changed

clang-tools-extra/clangd/CompileCommands.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,5 +503,32 @@ void ArgStripper::process(std::vector<std::string> &Args) const {
503503
Args.resize(Write);
504504
}
505505

506+
507+
std::string printArgv(llvm::ArrayRef<llvm::StringRef> Args) {
508+
std::string Buf;
509+
llvm::raw_string_ostream OS(Buf);
510+
bool Sep = false;
511+
for (llvm::StringRef Arg : Args) {
512+
if (Sep)
513+
OS << ' ';
514+
Sep = true;
515+
if (llvm::all_of(Arg, llvm::isPrint) &&
516+
Arg.find_first_of(" \t\n\"\\") == llvm::StringRef::npos) {
517+
OS << Arg;
518+
continue;
519+
}
520+
OS << '"';
521+
OS.write_escaped(Arg, /*UseHexEscapes=*/true);
522+
OS << '"';
523+
}
524+
return std::move(OS.str());
525+
}
526+
527+
std::string printArgv(llvm::ArrayRef<std::string> Args) {
528+
std::vector<llvm::StringRef> Refs(Args.size());
529+
llvm::copy(Args, Refs.begin());
530+
return printArgv(Refs);
531+
}
532+
506533
} // namespace clangd
507534
} // namespace clang

clang-tools-extra/clangd/CompileCommands.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,11 @@ class ArgStripper {
9696
std::deque<std::string> Storage; // Store strings not found in option table.
9797
};
9898

99+
// Renders an argv list, with arguments separated by spaces.
100+
// Where needed, arguments are "quoted" and escaped.
101+
std::string printArgv(llvm::ArrayRef<llvm::StringRef> Args);
102+
std::string printArgv(llvm::ArrayRef<std::string> Args);
103+
99104
} // namespace clangd
100105
} // namespace clang
101106

clang-tools-extra/clangd/QueryDriverDatabase.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,8 @@ extractSystemIncludesAndTarget(llvm::SmallString<128> Driver,
222222
if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
223223
Redirects)) {
224224
elog("System include extraction: driver execution failed with return code: "
225-
"{0}. Args: ['{1}']",
226-
llvm::to_string(RC), llvm::join(Args, "', '"));
225+
"{0}. Args: [{1}]",
226+
llvm::to_string(RC), printArgv(Args));
227227
return llvm::None;
228228
}
229229

clang-tools-extra/clangd/TUScheduler.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -648,15 +648,15 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags,
648648
log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
649649
FileName, Inputs.Version, Inputs.CompileCommand.Heuristic,
650650
Inputs.CompileCommand.Directory,
651-
llvm::join(Inputs.CompileCommand.CommandLine, " "));
651+
printArgv(Inputs.CompileCommand.CommandLine));
652652

653653
StoreDiags CompilerInvocationDiagConsumer;
654654
std::vector<std::string> CC1Args;
655655
std::unique_ptr<CompilerInvocation> Invocation = buildCompilerInvocation(
656656
Inputs, CompilerInvocationDiagConsumer, &CC1Args);
657657
// Log cc1 args even (especially!) if creating invocation failed.
658658
if (!CC1Args.empty())
659-
vlog("Driver produced command: cc1 {0}", llvm::join(CC1Args, " "));
659+
vlog("Driver produced command: cc1 {0}", printArgv(CC1Args));
660660
std::vector<Diag> CompilerInvocationDiags =
661661
CompilerInvocationDiagConsumer.take();
662662
if (!Invocation) {

clang-tools-extra/clangd/tool/Check.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,10 @@ class Checker {
107107

108108
if (auto TrueCmd = CDB->getCompileCommand(File)) {
109109
Cmd = std::move(*TrueCmd);
110-
log("Compile command from CDB is: {0}", llvm::join(Cmd.CommandLine, " "));
110+
log("Compile command from CDB is: {0}", printArgv(Cmd.CommandLine));
111111
} else {
112112
Cmd = CDB->getFallbackCommand(File);
113-
log("Generic fallback command is: {0}", llvm::join(Cmd.CommandLine, " "));
113+
log("Generic fallback command is: {0}", printArgv(Cmd.CommandLine));
114114
}
115115

116116
return true;
@@ -140,7 +140,7 @@ class Checker {
140140
buildCompilerInvocation(Inputs, CaptureInvocationDiags, &CC1Args);
141141
auto InvocationDiags = CaptureInvocationDiags.take();
142142
ErrCount += showErrors(InvocationDiags);
143-
log("internal (cc1) args are: {0}", llvm::join(CC1Args, " "));
143+
log("internal (cc1) args are: {0}", printArgv(CC1Args));
144144
if (!Invocation) {
145145
elog("Failed to parse command line");
146146
return false;

clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ TEST(CommandMangler, Sysroot) {
6464

6565
std::vector<std::string> Cmd = {"clang++", "foo.cc"};
6666
Mangler.adjust(Cmd);
67-
EXPECT_THAT(llvm::join(Cmd, " "),
67+
EXPECT_THAT(printArgv(Cmd),
6868
HasSubstr("-isysroot " + testPath("fake/sysroot")));
6969
}
7070

@@ -214,7 +214,7 @@ static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
214214
ArgStripper S;
215215
S.strip(Arg);
216216
S.process(Args);
217-
return llvm::join(Args, " ");
217+
return printArgv(Args);
218218
}
219219

220220
TEST(ArgStripperTest, Spellings) {
@@ -367,6 +367,14 @@ TEST(ArgStripperTest, OrderDependent) {
367367
EXPECT_THAT(Args, ElementsAre("clang", "foo.cc"));
368368
}
369369

370+
TEST(PrintArgvTest, All) {
371+
std::vector<llvm::StringRef> Args = {
372+
"one", "two", "thr ee", "f\"o\"ur", "fi\\ve", "$"
373+
};
374+
const char *Expected = R"(one two "thr ee" "f\"o\"ur" "fi\\ve" $)";
375+
EXPECT_EQ(Expected, printArgv(Args));
376+
}
377+
370378
} // namespace
371379
} // namespace clangd
372380
} // namespace clang

clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ MATCHER_P(hasArg, Flag, "") {
348348
return false;
349349
}
350350
if (!llvm::is_contained(arg->CommandLine, Flag)) {
351-
*result_listener << "flags are " << llvm::join(arg->CommandLine, " ");
351+
*result_listener << "flags are " << printArgv(arg->CommandLine);
352352
return false;
353353
}
354354
return true;
@@ -457,8 +457,7 @@ MATCHER_P2(hasFlag, Flag, Path, "") {
457457
return false;
458458
}
459459
if (!llvm::is_contained(Cmds.front().CommandLine, Flag)) {
460-
*result_listener << "flags are: "
461-
<< llvm::join(Cmds.front().CommandLine, " ");
460+
*result_listener << "flags are: " << printArgv(Cmds.front().CommandLine);
462461
return false;
463462
}
464463
return true;

0 commit comments

Comments
 (0)