From b8043bcd76687e33722c0f13bdc2ac04f7615513 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 15 Jan 2024 03:31:07 -0800 Subject: [PATCH 1/2] 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 --- 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 6d6e7171edc98b..5e7b3b79f427c9 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", ), From 3d4117f514a3279333c11d4afe004e2b8334dca8 Mon Sep 17 00:00:00 2001 From: iancha1992 Date: Mon, 22 Jan 2024 15:45:31 -0800 Subject: [PATCH 2/2] Updated the lockfiles --- MODULE.bazel.lock | 2 +- src/test/tools/bzlmod/MODULE.bazel.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 985f3b8cb8a5aa..badb20420084f0 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -2180,7 +2180,7 @@ "general": { "bzlTransitiveDigest": "OaXuahb8Ga1zYeFJL4rrJjYmP9XovLouv9Tp+aqzEX0=", "accumulatedFileDigests": { - "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "a35aced942408a91b1d681edb1049ebf55c7ddad7f382d6a45d44953510f4d96", + "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "a76d8c53ac208e3b6335eefa542a24b2e1a70fcfccb5d79fd80cea2cc2947ab4", "@@//:MODULE.bazel": "1ce286c2e04a814940dcb741cddee8772a3d8b97d22e12ee12e116dac57f1cd3" }, "envVariables": {}, diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index 6887b750690452..b7493f2c371bf4 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock @@ -1016,7 +1016,7 @@ }, "@@bazel_tools//tools/cpp:cc_configure.bzl%cc_configure_extension": { "general": { - "bzlTransitiveDigest": "O9sf6ilKWU9Veed02jG9o2HM/xgV/UAyciuFBuxrFRY=", + "bzlTransitiveDigest": "mcsWHq3xORJexV5/4eCvNOLxFOQKV6eli3fkr+tEaqE=", "accumulatedFileDigests": {}, "envVariables": {}, "generatedRepoSpecs": {