From b16cde9c53cbb734a2867bd2dc61a9d0545bfea7 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Tue, 21 Apr 2026 13:33:58 -0700 Subject: [PATCH] Add a hack to support `-cc1as`. (#7083) This is (far) from robust -- particularly with repeated compiles in the same address space. But it appears to be sufficient in the short term, and we already have the relevant TODOs to factor this upstream into something that we can use here. This also somehow uncovered a bug in how we were logging failed commands -- the failed commands are destroyed when we destroy the driver object (and its diagnostics object), so we simply cannot do the logging _after_ flushing diagnostics. That's probably ok, and just doing this in the other order makes the code simpler. Assisted-by: Antigravity with Gemini --- third_party/llvm/clang_cc1.cpp | 8 +- third_party/llvm/clang_cc1.h | 8 +- toolchain/driver/clang_runner.cpp | 222 ++++++++++++------------- toolchain/driver/clang_runner.h | 4 + toolchain/driver/clang_runner_test.cpp | 41 +++++ 5 files changed, 163 insertions(+), 120 deletions(-) diff --git a/third_party/llvm/clang_cc1.cpp b/third_party/llvm/clang_cc1.cpp index 983900805222..3d5a69d27cc0 100644 --- a/third_party/llvm/clang_cc1.cpp +++ b/third_party/llvm/clang_cc1.cpp @@ -35,10 +35,10 @@ namespace Carbon { -auto RunClangCC1(const InstallPaths& installation, - llvm::IntrusiveRefCntPtr fs, - llvm::SmallVectorImpl& cc1_args, - bool enable_leaking) -> int { +auto RunClangCC1Main(const InstallPaths& installation, + llvm::IntrusiveRefCntPtr fs, + llvm::SmallVectorImpl& cc1_args, + bool enable_leaking) -> int { llvm::BumpPtrAllocator allocator; llvm::cl::ExpansionContext expansion_context( allocator, llvm::cl::TokenizeGNUCommandLine); diff --git a/third_party/llvm/clang_cc1.h b/third_party/llvm/clang_cc1.h index 3368f35cbe2b..8a2520c6a0ad 100644 --- a/third_party/llvm/clang_cc1.h +++ b/third_party/llvm/clang_cc1.h @@ -19,10 +19,10 @@ namespace Carbon { // TODO: Much of the logic here should be factored out of the CC1 // implementation in Clang's driver and into a reusable part of its libraries. // That should allow reducing the code here to a minimal amount. -auto RunClangCC1(const InstallPaths& installation, - llvm::IntrusiveRefCntPtr fs, - llvm::SmallVectorImpl& cc1_args, - bool enable_leaking) -> int; +auto RunClangCC1Main(const InstallPaths& installation, + llvm::IntrusiveRefCntPtr fs, + llvm::SmallVectorImpl& cc1_args, + bool enable_leaking) -> int; } // namespace Carbon diff --git a/toolchain/driver/clang_runner.cpp b/toolchain/driver/clang_runner.cpp index e3524ac7bdc8..1567952a4e43 100644 --- a/toolchain/driver/clang_runner.cpp +++ b/toolchain/driver/clang_runner.cpp @@ -239,10 +239,35 @@ auto ClangRunner::RunWithNoRuntimes(llvm::ArrayRef args, enable_leaking); } +auto ClangRunner::RunClangCC1(llvm::SmallVectorImpl& cstr_args, + bool enable_leaking) -> int { + if (cstr_args[1] == llvm::StringRef("-cc1")) { + CARBON_VLOG("Dispatching `-cc1` command line in-process..."); + int exit_code = + RunClangCC1Main(*installation_, fs_, cstr_args, enable_leaking); + return exit_code; + } + + // Other CC1-based invocations need to dispatch into the `clang_main` + // routine to work correctly. This means they're not reliable in a library + // context but currently there is too much logic to reasonably extract here. + // This at least allows simple cases (often when directly used on the + // command line) to work correctly. + // + // TODO: Factor the relevant code paths into a library API or move this into + // the busybox dispatch logic. + CARBON_VLOG("Calling clang_main for a cc1-based invocation..."); + // cstr_args[0] will be the `clang_path` so we don't need the prepend arg. + llvm::ToolContext tool_context = { + .Path = cstr_args[0], .PrependArg = "clang", .NeedsPrependArg = false}; + int exit_code = clang_main( + cstr_args.size(), const_cast(cstr_args.data()), tool_context); + return exit_code; +} + auto ClangRunner::RunInternal( llvm::ArrayRef args, llvm::StringRef target, std::optional target_resource_dir_path, - std::optional libunwind_path, std::optional libcxx_path, bool link_runtime_libs, bool enable_leaking) -> ErrorOr { @@ -253,30 +278,8 @@ auto ClangRunner::RunInternal( if (!args.empty() && args[0].starts_with("-cc1")) { llvm::SmallVector cstr_args = BuildCStrArgs(clang_path_.native(), args, alloc); - if (args[0] == "-cc1") { - CARBON_VLOG("Dispatching `-cc1` command line..."); - int exit_code = - RunClangCC1(*installation_, fs_, cstr_args, enable_leaking); - // TODO: Should this be forwarding the full exit code? - return exit_code == 0; - } - - // Other CC1-based invocations need to dispatch into the `clang_main` - // routine to work correctly. This means they're not reliable in a library - // context but currently there is too much logic to reasonably extract here. - // This at least allows simple cases (often when directly used on the - // command line) to work correctly. - // - // TODO: Factor the relevant code paths into a library API or move this into - // the busybox dispatch logic. - CARBON_VLOG("Calling clang_main for a cc1-based invocation..."); - // cstr_args[0] will be the `clang_path` so we don't need the prepend arg. - llvm::ToolContext tool_context = { - .Path = cstr_args[0], .PrependArg = "clang", .NeedsPrependArg = false}; - int exit_code = clang_main( - cstr_args.size(), const_cast(cstr_args.data()), tool_context); // TODO: Should this be forwarding the full exit code? - return exit_code == 0; + return RunClangCC1(cstr_args, enable_leaking) == 0; } // We start with a custom prefix of arguments to establish Carbon's default @@ -343,102 +346,97 @@ auto ClangRunner::RunInternal( llvm::SmallVector> failing_commands; int result = -1; - { - // Create the diagnostic options and parse arguments controlling them out of - // our arguments. - std::unique_ptr diagnostic_options = - clang::CreateAndPopulateDiagOpts(cstr_args); - // TODO: We don't yet support serializing diagnostics the way the actual - // `clang` command line does. Unclear if we need to or not, but it would - // need a bit more logic here to set up chained consumers. - clang::TextDiagnosticPrinter diagnostic_client(llvm::errs(), - *diagnostic_options); + // Create the diagnostic options and parse arguments controlling them out of + // our arguments. + std::unique_ptr diagnostic_options = + clang::CreateAndPopulateDiagOpts(cstr_args); - // Note that the `DiagnosticsEngine` takes ownership (via a ref count) of - // the DiagnosticIDs, unlike the other parameters. - clang::DiagnosticsEngine diagnostics( - clang::DiagnosticIDs::create(), *diagnostic_options, &diagnostic_client, - /*ShouldOwnClient=*/false); - clang::ProcessWarningOptions(diagnostics, *diagnostic_options, *fs_); + // TODO: We don't yet support serializing diagnostics the way the actual + // `clang` command line does. Unclear if we need to or not, but it would + // need a bit more logic here to set up chained consumers. + clang::TextDiagnosticPrinter diagnostic_client(llvm::errs(), + *diagnostic_options); - // Note that we configure the driver's *default* target here, not the - // expected target as that will be parsed out of the command line below. - clang::driver::Driver driver(clang_path_.native(), - llvm::sys::getDefaultTargetTriple(), - diagnostics, "clang LLVM compiler", fs_); + // Note that the `DiagnosticsEngine` takes ownership (via a ref count) of + // the DiagnosticIDs, unlike the other parameters. + clang::DiagnosticsEngine diagnostics(clang::DiagnosticIDs::create(), + *diagnostic_options, &diagnostic_client, + /*ShouldOwnClient=*/false); + clang::ProcessWarningOptions(diagnostics, *diagnostic_options, *fs_); - llvm::Triple target_triple(target); + // Note that we configure the driver's *default* target here, not the + // expected target as that will be parsed out of the command line below. + clang::driver::Driver driver(clang_path_.native(), + llvm::sys::getDefaultTargetTriple(), diagnostics, + "clang LLVM compiler", fs_); - // We need to set an SDK system root on macOS by default. Setting it here - // allows a custom sysroot to still be specified on the command line. - // - // TODO: A different system root should be used for iOS, watchOS, tvOS. - // Currently, we're only targeting macOS support though. - if (target_triple.isMacOSX()) { - // This is the default CLT system root, shown by `xcrun --show-sdk-path`. - // We hard code it here to avoid the overhead of subprocessing to `xcrun` - // on each Clang invocation, but this may need to be updated to search or - // reflect macOS versions if this changes in the future. - driver.SysRoot = "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk"; - } + llvm::Triple target_triple(target); - // If we have a target-specific resource directory, set it as the default - // here, otherwise use the installation's resource directory. - driver.ResourceDir = target_resource_dir_path - ? target_resource_dir_path->str() - : installation_->clang_resource_path().native(); - - // Configure the install directory to find other tools and data files. - // - // We directly override the detected directory as we use a synthetic path - // above. This makes it appear that our binary was in the installed binaries - // directory, and allows finding tools relative to it. - driver.Dir = installation_->llvm_install_bin(); - CARBON_VLOG("Setting bin directory to: {0}\n", driver.Dir); - - // When there's only one command being run, this will run it in-process. - // However, a `clang` invocation may cause multiple `cc1` invocations, which - // still subprocess. See `InProcess` comment at: - // https://github.com/llvm/llvm-project/blob/86ce8e4504c06ecc3cc42f002ad4eb05cac10925/clang/lib/Driver/Job.cpp#L411-L413 - // - // Note the subprocessing will effectively call `clang -cc1`, which turns - // into `carbon-busybox clang -cc1`, which results in an equivalent - // `clang_main` call. - // - // Also note that we only do `-disable-free` filtering in the in-process - // execution here, as subprocesses leaking memory won't impact this process. - auto cc1_main = [this, enable_leaking]( - llvm::SmallVectorImpl& cc1_args) -> int { - return RunClangCC1(*installation_, fs_, cc1_args, enable_leaking); - }; - driver.CC1Main = cc1_main; - - std::unique_ptr compilation( - driver.BuildCompilation(cstr_args)); - CARBON_CHECK(compilation, "Should always successfully allocate!"); - if (compilation->containsError()) { - // These should have been diagnosed by the driver. - return false; - } - - // Make sure our target detection matches Clang's. Sadly, we can't just - // reuse Clang's as it is available too late. - // TODO: Use nice diagnostics here rather than a check failure. - CARBON_CHECK(llvm::Triple(target) == llvm::Triple(driver.getTargetTriple()), - "Mismatch between the expected target '{0}' and the one " - "computed by Clang '{1}'", - target, driver.getTargetTriple()); - - CARBON_VLOG("Running Clang driver...\n"); - - result = driver.ExecuteCompilation(*compilation, failing_commands); - - // Let diagnostics fall out of scope and be destroyed. This finishes - // diagnosing any failures before we verbosely log the source of those - // failures. + // We need to set an SDK system root on macOS by default. Setting it here + // allows a custom sysroot to still be specified on the command line. + // + // TODO: A different system root should be used for iOS, watchOS, tvOS. + // Currently, we're only targeting macOS support though. + if (target_triple.isMacOSX()) { + // This is the default CLT system root, shown by `xcrun --show-sdk-path`. + // We hard code it here to avoid the overhead of subprocessing to `xcrun` + // on each Clang invocation, but this may need to be updated to search or + // reflect macOS versions if this changes in the future. + driver.SysRoot = "/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk"; } + // If we have a target-specific resource directory, set it as the default + // here, otherwise use the installation's resource directory. + driver.ResourceDir = target_resource_dir_path + ? target_resource_dir_path->str() + : installation_->clang_resource_path().native(); + + // Configure the install directory to find other tools and data files. + // + // We directly override the detected directory as we use a synthetic path + // above. This makes it appear that our binary was in the installed binaries + // directory, and allows finding tools relative to it. + driver.Dir = installation_->llvm_install_bin(); + CARBON_VLOG("Setting bin directory to: {0}\n", driver.Dir); + + // When there's only one command being run, this will run it in-process. + // However, a `clang` invocation may cause multiple `cc1` invocations, which + // still subprocess. See `InProcess` comment at: + // https://github.com/llvm/llvm-project/blob/86ce8e4504c06ecc3cc42f002ad4eb05cac10925/clang/lib/Driver/Job.cpp#L411-L413 + // + // Note the subprocessing will effectively call `clang -cc1`, which turns + // into `carbon-busybox clang -cc1`, which results in an equivalent + // `clang_main` call. + // + // Also note that we only do `-disable-free` filtering in the in-process + // execution here, as subprocesses leaking memory won't impact this process. + auto cc1_main = [this, enable_leaking]( + llvm::SmallVectorImpl& cc1_args) -> int { + return RunClangCC1(cc1_args, enable_leaking); + }; + driver.CC1Main = cc1_main; + + std::unique_ptr compilation( + driver.BuildCompilation(cstr_args)); + CARBON_CHECK(compilation, "Should always successfully allocate!"); + if (compilation->containsError()) { + // These should have been diagnosed by the driver. + return false; + } + + // Make sure our target detection matches Clang's. Sadly, we can't just + // reuse Clang's as it is available too late. + // TODO: Use nice diagnostics here rather than a check failure. + CARBON_CHECK(llvm::Triple(target) == llvm::Triple(driver.getTargetTriple()), + "Mismatch between the expected target '{0}' and the one " + "computed by Clang '{1}'", + target, driver.getTargetTriple()); + + CARBON_VLOG("Running Clang driver...\n"); + + result = driver.ExecuteCompilation(*compilation, failing_commands); + CARBON_VLOG("Execution result code: {0}\n", result); for (const auto& [command_result, failing_command] : failing_commands) { CARBON_VLOG("Failing command '{0}' with code '{1}' was:\n", diff --git a/toolchain/driver/clang_runner.h b/toolchain/driver/clang_runner.h index c957ffd0eba1..309c4d2e6307 100644 --- a/toolchain/driver/clang_runner.h +++ b/toolchain/driver/clang_runner.h @@ -14,6 +14,7 @@ #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Allocator.h" #include "llvm/Support/ThreadPool.h" #include "llvm/Support/VirtualFileSystem.h" #include "llvm/TargetParser/Triple.h" @@ -106,6 +107,9 @@ class ClangRunner : ToolRunnerBase { private: friend class ClangRuntimesBuilderBase; + auto RunClangCC1(llvm::SmallVectorImpl& cstr_args, + bool enable_leaking) -> int; + // Handles building the Clang driver and passing the arguments down to it. auto RunInternal(llvm::ArrayRef args, llvm::StringRef target, std::optional target_resource_dir_path, diff --git a/toolchain/driver/clang_runner_test.cpp b/toolchain/driver/clang_runner_test.cpp index 8567c2752e9a..10ff836ba93b 100644 --- a/toolchain/driver/clang_runner_test.cpp +++ b/toolchain/driver/clang_runner_test.cpp @@ -261,5 +261,46 @@ TEST_F(ClangRunnerTest, ParamsFile) { EXPECT_THAT(out, HasSubstr("clang version")); } +TEST_F(ClangRunnerTest, Assemble) { + std::filesystem::path test_file = *Testing::WriteTestFile("test.s", R"asm( + .file "test.s" + .section .text,"ax",@progbits,unique,1 + .globl _Z4testv + .p2align 2 + .type _Z4testv,@function +_Z4testv: + .cfi_startproc + mov w0, wzr + ret +.Lfunc_end0: + .size _Z4testv, .Lfunc_end0-_Z4testv + .cfi_endproc + .section ".note.GNU-stack","",@progbits + .addrsig)asm"); + + std::filesystem::path test_output = *Testing::WriteTestFile("test.o", ""); + + RawStringOstream verbose_out; + ClangRunner runner(&install_paths_, vfs_, &verbose_out); + std::string out; + std::string err; + EXPECT_THAT( + Testing::CallWithCapturedOutput( + out, err, + [&] { + return runner.RunWithNoRuntimes( + {"-c", test_file.string(), "--target=aarch64-unknown-linux-gnu", + "-o", test_output.string()}); + }), + IsSuccess(true)) + << "Verbose output from runner:\n" + << verbose_out.TakeStr() << "\n"; + verbose_out.clear(); + + // No output should be produced. + EXPECT_THAT(out, StrEq("")); + EXPECT_THAT(err, StrEq("")); +} + } // namespace } // namespace Carbon