From 0962f1d72b1606f3224a14434c7b4500a23f8728 Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Sun, 31 Jan 2021 12:00:08 +0100 Subject: [PATCH] [clangd] Quote/escape argv included in log messages. https://github.com/clangd/clangd/issues/637 --- clang-tools-extra/clangd/CompileCommands.cpp | 27 ++++++++++++++++++++++ clang-tools-extra/clangd/CompileCommands.h | 5 ++++ clang-tools-extra/clangd/QueryDriverDatabase.cpp | 4 ++-- clang-tools-extra/clangd/TUScheduler.cpp | 4 ++-- clang-tools-extra/clangd/tool/Check.cpp | 6 ++--- .../clangd/unittests/CompileCommandsTests.cpp | 12 ++++++++-- .../unittests/GlobalCompilationDatabaseTests.cpp | 5 ++-- 7 files changed, 51 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp index 96cbd88..b55d1b0 100644 --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -503,5 +503,32 @@ void ArgStripper::process(std::vector &Args) const { Args.resize(Write); } + +std::string printArgv(llvm::ArrayRef Args) { + std::string Buf; + llvm::raw_string_ostream OS(Buf); + bool Sep = false; + for (llvm::StringRef Arg : Args) { + if (Sep) + OS << ' '; + Sep = true; + if (llvm::all_of(Arg, llvm::isPrint) && + Arg.find_first_of(" \t\n\"\\") == llvm::StringRef::npos) { + OS << Arg; + continue; + } + OS << '"'; + OS.write_escaped(Arg, /*UseHexEscapes=*/true); + OS << '"'; + } + return std::move(OS.str()); +} + +std::string printArgv(llvm::ArrayRef Args) { + std::vector Refs(Args.size()); + llvm::copy(Args, Refs.begin()); + return printArgv(Refs); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h index 2ba17a0..6e958d2 100644 --- a/clang-tools-extra/clangd/CompileCommands.h +++ b/clang-tools-extra/clangd/CompileCommands.h @@ -96,6 +96,11 @@ private: std::deque Storage; // Store strings not found in option table. }; +// Renders an argv list, with arguments separated by spaces. +// Where needed, arguments are "quoted" and escaped. +std::string printArgv(llvm::ArrayRef Args); +std::string printArgv(llvm::ArrayRef Args); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp b/clang-tools-extra/clangd/QueryDriverDatabase.cpp index 0bb2c46..94faec9f 100644 --- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp +++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp @@ -222,8 +222,8 @@ extractSystemIncludesAndTarget(llvm::SmallString<128> Driver, if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None, Redirects)) { elog("System include extraction: driver execution failed with return code: " - "{0}. Args: ['{1}']", - llvm::to_string(RC), llvm::join(Args, "', '")); + "{0}. Args: [{1}]", + llvm::to_string(RC), printArgv(Args)); return llvm::None; } diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp index 1d0ca1f..1cd6699 100644 --- a/clang-tools-extra/clangd/TUScheduler.cpp +++ b/clang-tools-extra/clangd/TUScheduler.cpp @@ -648,7 +648,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags, log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}", FileName, Inputs.Version, Inputs.CompileCommand.Heuristic, Inputs.CompileCommand.Directory, - llvm::join(Inputs.CompileCommand.CommandLine, " ")); + printArgv(Inputs.CompileCommand.CommandLine)); StoreDiags CompilerInvocationDiagConsumer; std::vector CC1Args; @@ -656,7 +656,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags, Inputs, CompilerInvocationDiagConsumer, &CC1Args); // Log cc1 args even (especially!) if creating invocation failed. if (!CC1Args.empty()) - vlog("Driver produced command: cc1 {0}", llvm::join(CC1Args, " ")); + vlog("Driver produced command: cc1 {0}", printArgv(CC1Args)); std::vector CompilerInvocationDiags = CompilerInvocationDiagConsumer.take(); if (!Invocation) { diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp index e42596b..9e3e439 100644 --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -107,10 +107,10 @@ public: if (auto TrueCmd = CDB->getCompileCommand(File)) { Cmd = std::move(*TrueCmd); - log("Compile command from CDB is: {0}", llvm::join(Cmd.CommandLine, " ")); + log("Compile command from CDB is: {0}", printArgv(Cmd.CommandLine)); } else { Cmd = CDB->getFallbackCommand(File); - log("Generic fallback command is: {0}", llvm::join(Cmd.CommandLine, " ")); + log("Generic fallback command is: {0}", printArgv(Cmd.CommandLine)); } return true; @@ -140,7 +140,7 @@ public: buildCompilerInvocation(Inputs, CaptureInvocationDiags, &CC1Args); auto InvocationDiags = CaptureInvocationDiags.take(); ErrCount += showErrors(InvocationDiags); - log("internal (cc1) args are: {0}", llvm::join(CC1Args, " ")); + log("internal (cc1) args are: {0}", printArgv(CC1Args)); if (!Invocation) { elog("Failed to parse command line"); return false; diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp index d911179..2330426 100644 --- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp +++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp @@ -64,7 +64,7 @@ TEST(CommandMangler, Sysroot) { std::vector Cmd = {"clang++", "foo.cc"}; Mangler.adjust(Cmd); - EXPECT_THAT(llvm::join(Cmd, " "), + EXPECT_THAT(printArgv(Cmd), HasSubstr("-isysroot " + testPath("fake/sysroot"))); } @@ -214,7 +214,7 @@ static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) { ArgStripper S; S.strip(Arg); S.process(Args); - return llvm::join(Args, " "); + return printArgv(Args); } TEST(ArgStripperTest, Spellings) { @@ -367,6 +367,14 @@ TEST(ArgStripperTest, OrderDependent) { EXPECT_THAT(Args, ElementsAre("clang", "foo.cc")); } +TEST(PrintArgvTest, All) { + std::vector Args = { + "one", "two", "thr ee", "f\"o\"ur", "fi\\ve", "$" + }; + const char *Expected = R"(one two "thr ee" "f\"o\"ur" "fi\\ve" $)"; + EXPECT_EQ(Expected, printArgv(Args)); +} + } // namespace } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp index 7c62955..2ec6412 100644 --- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp +++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp @@ -348,7 +348,7 @@ MATCHER_P(hasArg, Flag, "") { return false; } if (!llvm::is_contained(arg->CommandLine, Flag)) { - *result_listener << "flags are " << llvm::join(arg->CommandLine, " "); + *result_listener << "flags are " << printArgv(arg->CommandLine); return false; } return true; @@ -457,8 +457,7 @@ MATCHER_P2(hasFlag, Flag, Path, "") { return false; } if (!llvm::is_contained(Cmds.front().CommandLine, Flag)) { - *result_listener << "flags are: " - << llvm::join(Cmds.front().CommandLine, " "); + *result_listener << "flags are: " << printArgv(Cmds.front().CommandLine); return false; } return true; -- 2.7.4