From 7825191b606c2fa8235e5d191aa89f7c38d702a6 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Wed, 22 Oct 2025 10:53:28 -0700 Subject: [PATCH] [clang][deps] Fix a use-after-free from expanding response files In 436861645247 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 --- .../DependencyScannerImpl.cpp | 4 ++-- .../DependencyScannerImpl.h | 3 ++- .../DependencyScanningWorker.cpp | 6 +++-- clang/test/ClangScanDeps/response-file.c | 6 +++-- clang/tools/clang-scan-deps/ClangScanDeps.cpp | 24 ++++++++++++------- clang/tools/clang-scan-deps/Opts.td | 2 ++ 6 files changed, 30 insertions(+), 15 deletions(-) diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp index b0096d8e6b08b..05d566922a441 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp @@ -382,7 +382,8 @@ DignosticsEngineWithDiagOpts::DignosticsEngineWithDiagOpts( std::pair, std::unique_ptr> buildCompilation(ArrayRef ArgStrs, DiagnosticsEngine &Diags, - IntrusiveRefCntPtr FS) { + IntrusiveRefCntPtr FS, + llvm::BumpPtrAllocator &Alloc) { SmallVector Argv; Argv.reserve(ArgStrs.size()); for (const std::string &Arg : ArgStrs) @@ -393,7 +394,6 @@ buildCompilation(ArrayRef 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))); diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h index 71c6731803597..5657317565e8d 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h @@ -105,7 +105,8 @@ struct TextDiagnosticsPrinterWithOutput { std::pair, std::unique_ptr> buildCompilation(ArrayRef ArgStrs, DiagnosticsEngine &Diags, - IntrusiveRefCntPtr FS); + IntrusiveRefCntPtr FS, + llvm::BumpPtrAllocator &Alloc); std::unique_ptr createCompilerInvocation(ArrayRef CommandLine, diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 95154212603ac..0a1cf6b18b11c 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -78,8 +78,10 @@ static bool forEachDriverJob( IntrusiveRefCntPtr FS, llvm::function_ref 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()) { diff --git a/clang/test/ClangScanDeps/response-file.c b/clang/test/ClangScanDeps/response-file.c index c08105c127202..f905438e86af6 100644 --- a/clang/test/ClangScanDeps/response-file.c +++ b/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 diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index e41f4eb7999ae..c11a34870b204 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/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,14 +886,16 @@ 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 = @@ -898,7 +903,7 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) { 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()); diff --git a/clang/tools/clang-scan-deps/Opts.td b/clang/tools/clang-scan-deps/Opts.td index 03011f9ae1f75..7a63b18f6d462 100644 --- a/clang/tools/clang-scan-deps/Opts.td +++ b/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>;