Author: Sam McCall Date: 2021-01-31T12:00:08+01:00 New Revision: 0962f1d72b1606f3224a14434c7b4500a23f8728
URL: https://github.com/llvm/llvm-project/commit/0962f1d72b1606f3224a14434c7b4500a23f8728 DIFF: https://github.com/llvm/llvm-project/commit/0962f1d72b1606f3224a14434c7b4500a23f8728.diff LOG: [clangd] Quote/escape argv included in log messages. https://github.com/clangd/clangd/issues/637 Added: Modified: clang-tools-extra/clangd/CompileCommands.cpp clang-tools-extra/clangd/CompileCommands.h clang-tools-extra/clangd/QueryDriverDatabase.cpp clang-tools-extra/clangd/TUScheduler.cpp clang-tools-extra/clangd/tool/Check.cpp clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp index 96cbd8806ff6..b55d1b03dee6 100644 --- a/clang-tools-extra/clangd/CompileCommands.cpp +++ b/clang-tools-extra/clangd/CompileCommands.cpp @@ -503,5 +503,32 @@ void ArgStripper::process(std::vector<std::string> &Args) const { Args.resize(Write); } + +std::string printArgv(llvm::ArrayRef<llvm::StringRef> 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<std::string> Args) { + std::vector<llvm::StringRef> 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 2ba17a0e6c0d..6e958d271c87 100644 --- a/clang-tools-extra/clangd/CompileCommands.h +++ b/clang-tools-extra/clangd/CompileCommands.h @@ -96,6 +96,11 @@ class ArgStripper { std::deque<std::string> 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<llvm::StringRef> Args); +std::string printArgv(llvm::ArrayRef<std::string> Args); + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp b/clang-tools-extra/clangd/QueryDriverDatabase.cpp index 0bb2c46189b2..94faec9f3ed9 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 1d0ca1fee29d..1cd669945198 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<std::string> 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<Diag> CompilerInvocationDiags = CompilerInvocationDiagConsumer.take(); if (!Invocation) { diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp index e42596bb4bf4..9e3e439ae70d 100644 --- a/clang-tools-extra/clangd/tool/Check.cpp +++ b/clang-tools-extra/clangd/tool/Check.cpp @@ -107,10 +107,10 @@ class Checker { 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 @@ class Checker { 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 d91117956b09..23304267ae1a 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<std::string> 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<llvm::StringRef> 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 7c62955b1a72..2ec64128485b 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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits