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

[kadircet]

Summary

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

[sammccall]

I do half wonder whether we're going to get 3 steps further and then crash again when we call Command.front() :-)

This change looks good.

It also seems tempting to inject an empty check in GlobalCompilationDatabase (where we convert vector<CompileCommand> -> optional.
But I can see a perverse situation where e.g. background indexing would "just fail" at the driver level today, but the change would lead to using a fallback command and attempting to parse some binary file. (Ugh, garbage in compile_commands...)

[kadircet]

I do half wonder whether we're going to get 3 steps further and then crash again when we call Command.front() :-)

I actually couldn't find any other places this could happen today (we seem to check for non-emptiness of compile commands most of the time). But I hadn't look into clang, apparently createInvocationFromCommandLine also assumes that.
Adding an assert and pushing the check to callers, as in theory they should probably check for it long before since many tools perform some sort of command line mangling. LMK if the check should reside in the callee instead.

[kadircet]

• Also handle OOB access while creating compiler invocation.

clang-tools-extra/clangd/CompileCommands.cpp

CommandMangler CommandMangler::detect() {
   return Result;
 }
 
 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(
   // 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)
                : /*everything*/ 0,
       /*FlagsToExclude=*/driver::options::NoDriverOption |
           (IsCLMode ? 0 : driver::options::CLOption));
 
   llvm::SmallVector<unsigned, 1> IndicesToDrop;

clang-tools-extra/clangd/Compiler.cpp

void disableUnsupportedOptions(CompilerInvocation &CI) {
   CI.getFrontendOpts().PluginArgs.clear();
   CI.getFrontendOpts().ProgramAction = frontend::ParseSyntaxOnly;
   CI.getFrontendOpts().ActionName.clear();
 }
 
 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());
 
   auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
   llvm::IntrusiveRefCntPtr<DiagnosticsEngine> CommandLineDiagsEngine =
       CompilerInstance::createDiagnostics(new DiagnosticOptions, &D, false);
   std::unique_ptr<CompilerInvocation> CI = createInvocationFromCommandLine(

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

TEST(CommandMangler, StripsMultipleArch) {
 
   // Single arch option is preserved.
   Args = {"clang", "-arch", "foo", "/Users/foo.cc"};
   Mangler.adjust(Args, "/Users/foo.cc");
   EXPECT_EQ(
       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

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

TU.ExtraArgs = {"-Xclang", "-load",
                   "-Xclang", "my_plugin2"};
   auto Opts = buildCompilerInvocation(TU.inputs(FS), Diags)->getFrontendOpts();
   EXPECT_THAT(Opts.Plugins, IsEmpty());
   EXPECT_THAT(Opts.PluginArgs, IsEmpty());
   EXPECT_THAT(Opts.AddPluginActions, IsEmpty());
   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

clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

 #include "llvm/Support/Host.h"
 using namespace clang;
 using namespace llvm::opt;
 
 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.
     Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions);
   }
 
   SmallVector<const char *, 16> Args(ArgList.begin(), ArgList.end());