[clang][deps] Fix a use-after-free from expanding response files

In 4368616 we accidentally moved uses of command-line args saved into a
bump pointer allocator during response file expansion out of scope of the
allocator. Also, the test that should have caught this (at least with asan) was
not working correctly because clang-scan-deps was expanding response files
itself during argument adjustment rather than the underlying scanner library.

rdar://162720059

Author: benlangmuir

clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp

@@ -382,7 +382,8 @@ DignosticsEngineWithDiagOpts::DignosticsEngineWithDiagOpts(
 
 std::pair<std::unique_ptr<driver::Driver>, std::unique_ptr<driver::Compilation>>
 buildCompilation(ArrayRef<std::string> ArgStrs, DiagnosticsEngine &Diags,
-                 IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS) {
+                 IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
+                 llvm::BumpPtrAllocator &Alloc) {
   SmallVector<const char *, 256> Argv;
   Argv.reserve(ArgStrs.size());
   for (const std::string &Arg : ArgStrs)
@@ -393,7 +394,6 @@ buildCompilation(ArrayRef<std::string> ArgStrs, DiagnosticsEngine &Diags,
       "clang LLVM compiler", FS);
   Driver->setTitle("clang_based_tool");
 
-  llvm::BumpPtrAllocator Alloc;
   bool CLMode = driver::IsClangCL(
       driver::getDriverMode(Argv[0], ArrayRef(Argv).slice(1)));
 

clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h

@@ -105,7 +105,8 @@ struct TextDiagnosticsPrinterWithOutput {
 
 std::pair<std::unique_ptr<driver::Driver>, std::unique_ptr<driver::Compilation>>
 buildCompilation(ArrayRef<std::string> ArgStrs, DiagnosticsEngine &Diags,
-                 IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS);
+                 IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
+                 llvm::BumpPtrAllocator &Alloc);
 
 std::unique_ptr<CompilerInvocation>
 createCompilerInvocation(ArrayRef<std::string> CommandLine,

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

@@ -78,8 +78,10 @@ static bool forEachDriverJob(
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS,
     llvm::function_ref<bool(const driver::Command &Cmd)> Callback) {
   // Compilation holds a non-owning a reference to the Driver, hence we need to
-  // keep the Driver alive when we use Compilation.
-  auto [Driver, Compilation] = buildCompilation(ArgStrs, Diags, FS);
+  // keep the Driver alive when we use Compilation. Arguments to commands may be
+  // owned by Alloc when expanded from response files.
+  llvm::BumpPtrAllocator Alloc;
+  auto [Driver, Compilation] = buildCompilation(ArgStrs, Diags, FS, Alloc);
   if (!Compilation)
     return false;
   for (const driver::Command &Job : Compilation->getJobs()) {

clang/test/ClangScanDeps/response-file.c

@@ -1,10 +1,12 @@
-// Check that the scanner can handle a response file input.
+// Check that the scanner can handle a response file input. Uses -verbatim-args
+// to ensure response files are expanded by the scanner library and not the
+// argumeent adjuster in clang-scan-deps.
 
 // RUN: rm -rf %t
 // RUN: split-file %s %t
 // RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
 
-// RUN: clang-scan-deps -format experimental-full -compilation-database %t/cdb.json > %t/deps.json
+// RUN: clang-scan-deps -verbatim-args -format experimental-full -compilation-database %t/cdb.json > %t/deps.json
 
 // RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
 

clang/tools/clang-scan-deps/ClangScanDeps.cpp

@@ -106,6 +106,7 @@ static constexpr bool DoRoundTripDefault = false;
 #endif
 
 static bool RoundTripArgs = DoRoundTripDefault;
+static bool VerbatimArgs = false;
 
 static void ParseArgs(int argc, char **argv) {
   ScanDepsOptTable Tbl;
@@ -239,6 +240,8 @@ static void ParseArgs(int argc, char **argv) {
 
   RoundTripArgs = Args.hasArg(OPT_round_trip_args);
 
+  VerbatimArgs = Args.hasArg(OPT_verbatim_args);
+
   if (const llvm::opt::Arg *A = Args.getLastArgNoClaim(OPT_DASH_DASH))
     CommandLine.assign(A->getValues().begin(), A->getValues().end());
 }
@@ -883,22 +886,24 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
 
   llvm::cl::PrintOptionValues();
 
-  // Expand response files in advance, so that we can "see" all the arguments
-  // when adjusting below.
-  Compilations = expandResponseFiles(std::move(Compilations),
-                                     llvm::vfs::getRealFileSystem());
+  if (!VerbatimArgs) {
+    // Expand response files in advance, so that we can "see" all the arguments
+    // when adjusting below.
+    Compilations = expandResponseFiles(std::move(Compilations),
+                                       llvm::vfs::getRealFileSystem());
 
-  Compilations = inferTargetAndDriverMode(std::move(Compilations));
+    Compilations = inferTargetAndDriverMode(std::move(Compilations));
 
-  Compilations = inferToolLocation(std::move(Compilations));
+    Compilations = inferToolLocation(std::move(Compilations));
+  }
 
   // The command options are rewritten to run Clang in preprocessor only mode.
   auto AdjustingCompilations =
       std::make_unique<tooling::ArgumentsAdjustingCompilations>(
           std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
 
-  AdjustingCompilations->appendArgumentsAdjuster(
+  auto ArgsAdjuster =
       [&ResourceDirCache](const tooling::CommandLineArguments &Args,
                           StringRef FileName) {
         std::string LastO;
@@ -960,7 +965,10 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
         }
         AdjustedArgs.insert(AdjustedArgs.end(), FlagsEnd, Args.end());
         return AdjustedArgs;
-      });
+      };
+
+  if (!VerbatimArgs)
+    AdjustingCompilations->appendArgumentsAdjuster(ArgsAdjuster);
 
   SharedStream Errs(llvm::errs());
 

clang/tools/clang-scan-deps/Opts.td

@@ -44,4 +44,6 @@ def verbose : F<"v", "Use verbose output">;
 
 def round_trip_args : F<"round-trip-args", "verify that command-line arguments are canonical by parsing and re-serializing">;
 
+def verbatim_args : F<"verbatim-args", "Pass commands to the scanner verbatim without adjustments">;
+
 def DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>;