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