From e00394ea92ce17f9d349b2a1443f2923a672bd8d Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Thu, 19 Feb 2026 09:00:40 -0800 Subject: [PATCH] Teach the `link` subcommand to accept Clang-style `LDFLAGS` (#6741) Add an optional additional set of positional parameters that can be passed to the `link` subcommand for Clang-style (or GCC-style) `LDFLAGS`. These can _also_ contain object files, etc., and in fact it is useful to allow them to contain object files in order to integrate the `carbon link` subcommand into a build system that mixes both link flags and object files. This at least happens with Bazel, and I suspect is common. Eventually, it would be nice to have sufficient semantics to handle all the varieties of links we want without resorting to this escape hatch, but that's likely a long way away and so it seems especially useful to allow falling back to Clang's flags as needed for now. This does somewhat directly surface the Clang implementation detail in the command line syntax, but I don't see a lot of good alternatives. --------- Co-authored-by: Jon Ross-Perkins --- toolchain/diagnostics/kind.def | 2 + toolchain/driver/BUILD | 1 + toolchain/driver/driver_test.cpp | 47 +++++++++++++- toolchain/driver/link_subcommand.cpp | 61 ++++++++++++++++--- toolchain/driver/link_subcommand.h | 2 + .../link/fail_object_files_missing.carbon | 15 +++++ .../testdata/link/fail_output_missing.carbon | 15 +++++ 7 files changed, 131 insertions(+), 12 deletions(-) create mode 100644 toolchain/driver/testdata/link/fail_object_files_missing.carbon create mode 100644 toolchain/driver/testdata/link/fail_output_missing.carbon diff --git a/toolchain/diagnostics/kind.def b/toolchain/diagnostics/kind.def index e2f983c7ba3e..624c75d1a012 100644 --- a/toolchain/diagnostics/kind.def +++ b/toolchain/diagnostics/kind.def @@ -36,6 +36,8 @@ CARBON_DIAGNOSTIC_KIND(FailureBuildingRuntimes) CARBON_DIAGNOSTIC_KIND(FailureRunningClang) CARBON_DIAGNOSTIC_KIND(FailureRunningClangToLink) CARBON_DIAGNOSTIC_KIND(FormatMultipleFilesToOneOutput) +CARBON_DIAGNOSTIC_KIND(LinkObjectFilesMissing) +CARBON_DIAGNOSTIC_KIND(LinkOutputOptionMissing) CARBON_DIAGNOSTIC_KIND(ToolFuzzingDisallowed) // ============================================================================ diff --git a/toolchain/driver/BUILD b/toolchain/driver/BUILD index 43f3113b5603..70a1537dd138 100644 --- a/toolchain/driver/BUILD +++ b/toolchain/driver/BUILD @@ -248,6 +248,7 @@ cc_test( "//common:error_test_helpers", "//common:filesystem", "//common:raw_string_ostream", + "//testing/base:capture_std_streams", "//testing/base:file_helpers", "//testing/base:global_exe_path", "//testing/base:gtest_main", diff --git a/toolchain/driver/driver_test.cpp b/toolchain/driver/driver_test.cpp index b95ce388c5f4..94d72eea8cb9 100644 --- a/toolchain/driver/driver_test.cpp +++ b/toolchain/driver/driver_test.cpp @@ -22,6 +22,7 @@ #include "llvm/Support/Error.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/JSON.h" +#include "testing/base/capture_std_streams.h" #include "testing/base/file_helpers.h" #include "testing/base/global_exe_path.h" #include "toolchain/testing/yaml_test_helpers.h" @@ -30,6 +31,7 @@ namespace Carbon { namespace { using ::testing::_; +using Testing::CallWithCapturedOutput; using ::testing::ContainsRegex; using ::testing::HasSubstr; using Testing::IsSuccess; @@ -248,10 +250,11 @@ TEST_F(DriverTest, Link) { << test_error_stream_.TakeStr(); // Now link this into a binary. Note that we suppress building runtimes on - // demand here as no runtimes should be needed for the empty program. + // demand here as no runtimes should be needed for the empty program. We also + // pass some system library link flags through to the underlying Clang layer. EXPECT_TRUE(driver_ .RunCommand({"--no-build-runtimes", "link", "--output=test", - "test.o"}) + "test.o", "--", "-lc", "-lm"}) .success); EXPECT_THAT(test_error_stream_.TakeStr(), StrEq("")); @@ -266,6 +269,46 @@ TEST_F(DriverTest, Link) { EXPECT_TRUE(result->getBinary()->isObject()); } +TEST_F(DriverTest, LinkWithFlagLikeFiles) { + auto scope = ScopedTempWorkingDir(); + + // First compile a file to get a linkable object. + MakeTestFile("fn Run() {}", "test.carbon"); + ASSERT_TRUE( + driver_.RunCommand({"compile", "--no-prelude-import", "test.carbon"}) + .success) + << test_error_stream_.TakeStr(); + + // Rename it to a flag-like name. + Filesystem::Cwd().Rename("test.o", Filesystem::Cwd(), "--test.o").Check(); + + // Link this into a binary and pass flags to the Clang link invocation even + // though we use `--` before the object file input list to handle weirdly + // named objects. + // + // TODO: This works correctly in the Carbon link subcommand, but Clang itself + // fails to pass object files to LLD in a way that supports flag-shaped object + // file names. The last flag being `-Wl,--` tries to work around this by + // passing a `--` to the linker before the object files. However, LLD in turn + // appears to have bugs parsing command lines in this shape. We should get + // these fixed and then can remove the `-Wl,--` hack and the test should + // actually pass. + std::string out; + std::string err; + EXPECT_FALSE(CallWithCapturedOutput(out, err, [&] { + return driver_.RunCommand({"--no-build-runtimes", "link", + "--output=test", "--", "--test.o", + "--", "-lc", "-lm", "-Wl,--"}); + }).success); + EXPECT_THAT(test_error_stream_.TakeStr(), StrEq("")); + EXPECT_THAT(out, StrEq("")); + // This error seems to stem from incorrectly handling `--` in the LLD command + // line. See above; this should go away when the underlying bugs are fixed. + EXPECT_THAT(err, + HasSubstr("error: completed parsing all 1 configured positional " + "arguments, but found a subsequent `--`")); +} + TEST_F(DriverTest, ConfigJson) { EXPECT_TRUE(driver_.RunCommand({"config", "--json"}).success); EXPECT_THAT(test_error_stream_.TakeStr(), StrEq("")); diff --git a/toolchain/driver/link_subcommand.cpp b/toolchain/driver/link_subcommand.cpp index 715e30013dfe..1ebdca573f99 100644 --- a/toolchain/driver/link_subcommand.cpp +++ b/toolchain/driver/link_subcommand.cpp @@ -15,12 +15,12 @@ auto LinkOptions::Build(CommandLine::CommandBuilder& b) -> void { .name = "OBJECT_FILE", .help = R"""( The input object files. + +If empty, there must be extra Clang link arguments that provide object files +intermingled with linking flags. )""", }, - [&](auto& arg_b) { - arg_b.Required(true); - arg_b.Append(&object_filenames); - }); + [&](auto& arg_b) { arg_b.Append(&object_filenames); }); b.AddStringOption( { @@ -28,12 +28,28 @@ The input object files. .value_name = "FILE", .help = R"""( The linked file name. The output is always a linked binary. + +If not provided, there must be extra Clang link arguments that include +specifying the output of the link. This allows supporting build systems that +intermingle the output flag with arbitrary other linker flags that need to use +legacy parsing logic. )""", }, - [&](auto& arg_b) { - arg_b.Required(true); - arg_b.Set(&output_filename); - }); + [&](auto& arg_b) { arg_b.Set(&output_filename); }); + + b.AddStringPositionalArg( + { + .name = "EXTRA_CLANG_LINK_ARGS", + .help = R"""( +Extra arguments to pass to Clang when forming the link command. This is +primarily useful for expanding `LDFLAGS` or other baseline linking flags in a +build system. + +These can also be used to pass object files to the link in the event your build +system mixes object files and linker flags. +)""", + }, + [&](auto& arg_b) { arg_b.Append(&extra_clang_args); }); codegen_options.Build(b); } @@ -69,8 +85,33 @@ auto LinkSubcommand::Run(DriverEnv& driver_env) -> DriverResult { llvm::formatv("--target={0}", options_.codegen_options.target).str(); clang_args.push_back(target_arg); - clang_args.push_back("-o"); - clang_args.push_back(options_.output_filename); + if (!options_.output_filename.empty()) { + clang_args.push_back("-o"); + clang_args.push_back(options_.output_filename); + } else if (options_.extra_clang_args.empty()) { + CARBON_DIAGNOSTIC(LinkOutputOptionMissing, Error, + "no output specified to a link command and no extra " + "Clang options that can provide an output"); + driver_env.emitter.Emit(LinkOutputOptionMissing); + return {.success = false}; + } + + if (options_.object_filenames.empty() && options_.extra_clang_args.empty()) { + CARBON_DIAGNOSTIC(LinkObjectFilesMissing, Error, + "no object files provided to link command and no extra " + "Clang options that could provide them"); + driver_env.emitter.Emit(LinkObjectFilesMissing); + return {.success = false}; + } + + // Note that we append any extra Clang args before our object filenames. This + // allows us to propagate object filenames that collide with Clang flags using + // `--` before the filenames. While in theory, this could create a problem in + // the presence of mixtures of object files in the two lists and the order + // being dependent, we don't expect that in practice. + clang_args.append(options_.extra_clang_args.begin(), + options_.extra_clang_args.end()); + clang_args.push_back("--"); clang_args.append(options_.object_filenames.begin(), options_.object_filenames.end()); diff --git a/toolchain/driver/link_subcommand.h b/toolchain/driver/link_subcommand.h index 5eb127629b07..dcb0765e0d25 100644 --- a/toolchain/driver/link_subcommand.h +++ b/toolchain/driver/link_subcommand.h @@ -23,6 +23,8 @@ struct LinkOptions { CodegenOptions codegen_options; llvm::StringRef output_filename; llvm::SmallVector object_filenames; + + llvm::SmallVector extra_clang_args; }; // Implements the link subcommand of the driver. diff --git a/toolchain/driver/testdata/link/fail_object_files_missing.carbon b/toolchain/driver/testdata/link/fail_object_files_missing.carbon new file mode 100644 index 000000000000..24670fbd51a1 --- /dev/null +++ b/toolchain/driver/testdata/link/fail_object_files_missing.carbon @@ -0,0 +1,15 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// ARGS: --include-diagnostic-kind link --output=foo +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/link/fail_object_files_missing.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/link/fail_object_files_missing.carbon +// CHECK:STDERR: error: no object files provided to link command and no extra Clang options that could provide them [LinkObjectFilesMissing] +// CHECK:STDERR: + +// --- foo.carbon diff --git a/toolchain/driver/testdata/link/fail_output_missing.carbon b/toolchain/driver/testdata/link/fail_output_missing.carbon new file mode 100644 index 000000000000..293931f80d61 --- /dev/null +++ b/toolchain/driver/testdata/link/fail_output_missing.carbon @@ -0,0 +1,15 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// ARGS: --include-diagnostic-kind link foo.o bar.o +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/driver/testdata/link/fail_output_missing.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/driver/testdata/link/fail_output_missing.carbon +// CHECK:STDERR: error: no output specified to a link command and no extra Clang options that can provide an output [LinkOutputOptionMissing] +// CHECK:STDERR: + +// --- foo.carbon