From 2bfe045ff2d6550e443625128b0dfeb2941ebfbc Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 15 Jan 2024 03:31:07 -0800 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. Fixes #20834 Closes #20833. PiperOrigin-RevId: 598565598 Change-Id: I4890f278c5cc33d4e6a6ebb10d796fb1c22f9ba6 --- MODULE.bazel.lock | 2 +- src/test/tools/bzlmod/MODULE.bazel.lock | 2 +- tools/cpp/unix_cc_configure.bzl | 56 ++++++++++++++----------- 3 files changed, 34 insertions(+), 26 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 5e23ce9dcb25d8..70427499942c9c 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -2249,7 +2249,7 @@ "general": { "bzlTransitiveDigest": "uLzmPag+drHdv4SHM6VTDi4HKycGdoUKF422fH5bqrg=", "accumulatedFileDigests": { - "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "38e96ee0c3217da5ada5c377da7df10d9ff874e53bf6a2f85052d61303761483", + "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "4cb6b549865c67eda0658814b8e6209ce6a24ccb6563a325c6b52a285c72a832", "@@//:MODULE.bazel": "7e00b434061feab70b94deee240eb765c14046a07988b8b6f1eb5d7596b4a4f5" }, "envVariables": {}, diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index 7feb5ba14cdaa6..dafcd9d8a11279 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock @@ -1020,7 +1020,7 @@ }, "@@bazel_tools//tools/cpp:cc_configure.bzl%cc_configure_extension": { "general": { - "bzlTransitiveDigest": "uZaFg/HFA09XE0yk4B2fascwbA381z3FwFF64AD5a9w=", + "bzlTransitiveDigest": "2LC5INJ/KSraqEsbNl9rSibnM7ApBho21h1gyUQbjPg=", "accumulatedFileDigests": {}, "envVariables": {}, "generatedRepoSpecs": { 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", ),