[clang-tools-extra] 444a5f3 - [clangd] Bail-out when an empty compile flag is encountered

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-09-20T16:51:56+02:00
New Revision: 444a5f304f6c2c332f18392d2458d74664e98498

URL: https://github.com/llvm/llvm-project/commit/444a5f304f6c2c332f18392d2458d74664e98498
DIFF: https://github.com/llvm/llvm-project/commit/444a5f304f6c2c332f18392d2458d74664e98498.diff

LOG: [clangd] Bail-out when an empty compile flag is encountered

Fixes https://github.com/clangd/clangd/issues/865

Differential Revision: https://reviews.llvm.org/D109894

Added: 
    

Modified: 
    clang-tools-extra/clangd/CompileCommands.cpp
    clang-tools-extra/clangd/Compiler.cpp
    clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
    clang-tools-extra/clangd/unittests/CompilerTests.cpp
    clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index f9e283beca64..d707bf69eded 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -198,22 +198,26 @@ CommandMangler CommandMangler::forTests() { return CommandMangler(); }
 void CommandMangler::adjust(std::vector<std::string> &Cmd,
                             llvm::StringRef File) const {
   trace::Span S("AdjustCompileFlags");
+  // Most of the modifications below assumes the Cmd starts with a driver name.
+  // We might consider injecting a generic driver name like "cc" or "c++", but
+  // a Cmd missing the driver is probably rare enough in practice and errnous.
+  if (Cmd.empty())
+    return;
   auto &OptTable = clang::driver::getDriverOptTable();
   // OriginalArgs needs to outlive ArgList.
   llvm::SmallVector<const char *, 16> OriginalArgs;
   OriginalArgs.reserve(Cmd.size());
   for (const auto &S : Cmd)
     OriginalArgs.push_back(S.c_str());
-  bool IsCLMode =
-      !OriginalArgs.empty() &&
-      driver::IsClangCL(driver::getDriverMode(
-          OriginalArgs[0], llvm::makeArrayRef(OriginalArgs).slice(1)));
+  bool IsCLMode = driver::IsClangCL(driver::getDriverMode(
+      OriginalArgs[0], llvm::makeArrayRef(OriginalArgs).slice(1)));
   // ParseArgs propagates missig arg/opt counts on error, but preserves
   // everything it could parse in ArgList. So we just ignore those counts.
   unsigned IgnoredCount;
   // Drop the executable name, as ParseArgs doesn't expect it. This means
   // indices are actually of by one between ArgList and OriginalArgs.
-  auto ArgList = OptTable.ParseArgs(
+  llvm::opt::InputArgList ArgList;
+  ArgList = OptTable.ParseArgs(
       llvm::makeArrayRef(OriginalArgs).drop_front(), IgnoredCount, IgnoredCount,
       /*FlagsToInclude=*/
       IsCLMode ? (driver::options::CLOption | driver::options::CoreOption)

diff  --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp
index 44a1b1a77db5..809119cda6b1 100644
--- a/clang-tools-extra/clangd/Compiler.cpp
+++ b/clang-tools-extra/clangd/Compiler.cpp
@@ -83,6 +83,8 @@ void disableUnsupportedOptions(CompilerInvocation &CI) {
 std::unique_ptr<CompilerInvocation>
 buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D,
                         std::vector<std::string> *CC1Args) {
+  if (Inputs.CompileCommand.CommandLine.empty())
+    return nullptr;
   std::vector<const char *> ArgStrs;
   for (const auto &S : Inputs.CompileCommand.CommandLine)
     ArgStrs.push_back(S.c_str());

diff  --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
index c8749ec7f487..4cb6ef9a1661 100644
--- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -396,6 +396,13 @@ TEST(CommandMangler, StripsMultipleArch) {
       llvm::count_if(Args, [](llvm::StringRef Arg) { return Arg == "-arch"; }),
       1);
 }
+
+TEST(CommandMangler, EmptyArgs) {
+  const auto Mangler = CommandMangler::forTests();
+  std::vector<std::string> Args = {};
+  // Make sure we don't crash.
+  Mangler.adjust(Args, "foo.cc");
+}
 } // namespace
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/unittests/CompilerTests.cpp b/clang-tools-extra/clangd/unittests/CompilerTests.cpp
index 9a85bfeb2bb8..ab9e85e983d1 100644
--- a/clang-tools-extra/clangd/unittests/CompilerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompilerTests.cpp
@@ -102,6 +102,17 @@ TEST(BuildCompilerInvocation, DropsPlugins) {
   EXPECT_EQ(Opts.ProgramAction, frontend::ActionKind::ParseSyntaxOnly);
   EXPECT_TRUE(Opts.ActionName.empty());
 }
+
+TEST(BuildCompilerInvocation, EmptyArgs) {
+  MockFS FS;
+  IgnoreDiagnostics Diags;
+  TestTU TU;
+  auto Inputs = TU.inputs(FS);
+  Inputs.CompileCommand.CommandLine.clear();
+
+  // No crash.
+  EXPECT_EQ(buildCompilerInvocation(Inputs, Diags), nullptr);
+}
 } // namespace
 } // namespace clangd
 } // namespace clang

diff  --git a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
index 2a14b7aab52e..c5627d13a7a7 100644
--- a/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ b/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -30,6 +30,7 @@ std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
     ArrayRef<const char *> ArgList, IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS, bool ShouldRecoverOnErorrs,
     std::vector<std::string> *CC1Args) {
+  assert(!ArgList.empty());
   if (!Diags.get()) {
     // No diagnostics engine was provided, so create our own diagnostics object
     // with the default options.