From 194fe4c08ef02e59ebe8b61b70d338d638626427 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 10 Jan 2024 11:05:52 +0100 Subject: [PATCH] Fix linker feature detection being performed on wrong linker Bazel forces use of `lld` and `ld.gold` if found, but performed feature detection on the default linker instead. --- tools/cpp/unix_cc_configure.bzl | 56 +++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/tools/cpp/unix_cc_configure.bzl b/tools/cpp/unix_cc_configure.bzl index ac2e16206e3406..993e46dd40fb92 100644 --- a/tools/cpp/unix_cc_configure.bzl +++ b/tools/cpp/unix_cc_configure.bzl @@ -161,10 +161,9 @@ def _is_compiler_option_supported(repository_ctx, cc, option): ]) return result.stderr.find(option) == -1 -def _is_linker_option_supported(repository_ctx, cc, option, pattern): +def _is_linker_option_supported(repository_ctx, cc, force_linker_flags, option, pattern): """Checks that `option` is supported by the C linker. Doesn't %-escape the option.""" - result = repository_ctx.execute([ - cc, + result = repository_ctx.execute([cc] + force_linker_flags + [ option, "-o", "/dev/null", @@ -213,9 +212,9 @@ def _add_compiler_option_if_supported(repository_ctx, cc, option): """Returns `[option]` if supported, `[]` otherwise. Doesn't %-escape the option.""" return [option] if _is_compiler_option_supported(repository_ctx, cc, option) else [] -def _add_linker_option_if_supported(repository_ctx, cc, option, pattern): +def _add_linker_option_if_supported(repository_ctx, cc, force_linker_flags, option, pattern): """Returns `[option]` if supported, `[]` otherwise. Doesn't %-escape the option.""" - return [option] if _is_linker_option_supported(repository_ctx, cc, option, pattern) else [] + return [option] if _is_linker_option_supported(repository_ctx, cc, force_linker_flags, option, pattern) else [] def _get_no_canonical_prefixes_opt(repository_ctx, cc): # If the compiler sometimes rewrites paths in the .d files without symlinks @@ -420,16 +419,40 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools): False, ), ":") + gold_or_lld_linker_path = ( + _find_linker_path(repository_ctx, cc, "lld", is_clang) or + _find_linker_path(repository_ctx, cc, "gold", is_clang) + ) + cc_path = repository_ctx.path(cc) + if not str(cc_path).startswith(str(repository_ctx.path(".")) + "/"): + # cc is outside the repository, set -B + bin_search_flags = ["-B" + escape_string(str(cc_path.dirname))] + else: + # cc is inside the repository, don't set -B. + bin_search_flags = [] + if not gold_or_lld_linker_path: + ld_path = repository_ctx.path(tool_paths["ld"]) + if ld_path.dirname != cc_path.dirname: + bin_search_flags.append("-B" + str(ld_path.dirname)) + force_linker_flags = [] + if gold_or_lld_linker_path: + force_linker_flags.append("-fuse-ld=" + gold_or_lld_linker_path) + + # TODO: It's unclear why these flags aren't added on macOS. + if bin_search_flags and not darwin: + force_linker_flags.extend(bin_search_flags) use_libcpp = darwin or bsd is_as_needed_supported = _is_linker_option_supported( repository_ctx, cc, + force_linker_flags, "-Wl,-no-as-needed", "-no-as-needed", ) is_push_state_supported = _is_linker_option_supported( repository_ctx, cc, + force_linker_flags, "-Wl,--push-state", "--push-state", ) @@ -463,21 +486,6 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools): bazel_linklibs, False, ), ":") - gold_or_lld_linker_path = ( - _find_linker_path(repository_ctx, cc, "lld", is_clang) or - _find_linker_path(repository_ctx, cc, "gold", is_clang) - ) - cc_path = repository_ctx.path(cc) - if not str(cc_path).startswith(str(repository_ctx.path(".")) + "/"): - # cc is outside the repository, set -B - bin_search_flags = ["-B" + escape_string(str(cc_path.dirname))] - else: - # cc is inside the repository, don't set -B. - bin_search_flags = [] - if not gold_or_lld_linker_path: - ld_path = repository_ctx.path(tool_paths["ld"]) - if ld_path.dirname != cc_path.dirname: - bin_search_flags.append("-B" + str(ld_path.dirname)) coverage_compile_flags, coverage_link_flags = _coverage_flags(repository_ctx, darwin) print_resource_dir_supported = _is_compiler_option_supported( repository_ctx, @@ -610,19 +618,18 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools): ), "%{cxx_flags}": get_starlark_list(cxx_opts + _escaped_cplus_include_paths(repository_ctx)), "%{conly_flags}": get_starlark_list(conly_opts), - "%{link_flags}": get_starlark_list(( - ["-fuse-ld=" + gold_or_lld_linker_path] if gold_or_lld_linker_path else [] - ) + ( + "%{link_flags}": get_starlark_list(force_linker_flags + ( ["-Wl,-no-as-needed"] if is_as_needed_supported else [] ) + _add_linker_option_if_supported( repository_ctx, cc, + force_linker_flags, "-Wl,-z,relro,-z,now", "-z", ) + ( [ "-headerpad_max_install_names", - ] if darwin else bin_search_flags + [ + ] if darwin else [ # Gold linker only? Can we enable this by default? # "-Wl,--warn-execstack", # "-Wl,--detect-odr-violations" @@ -664,6 +671,7 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools): ["-Wl,-dead_strip"] if darwin else _add_linker_option_if_supported( repository_ctx, cc, + force_linker_flags, "-Wl,--gc-sections", "-gc-sections", ),