Skip to content

Commit

Permalink
Fix linker feature detection being performed on wrong linker
Browse files Browse the repository at this point in the history
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
  • Loading branch information
fmeum authored and copybara-github committed Jan 15, 2024
1 parent 67541bb commit 2bfe045
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 26 deletions.
2 changes: 1 addition & 1 deletion MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/test/tools/bzlmod/MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

56 changes: 32 additions & 24 deletions tools/cpp/unix_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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",
),
Expand Down

0 comments on commit 2bfe045

Please sign in to comment.