Skip to content

Commit

Permalink
Pass absolute path to gold when using Clang
Browse files Browse the repository at this point in the history
Prior to this change Bazel's C++ toolchain autoconfiguration generated
toolchain that passed `-fuse-ld=gold` when gold linker was detected.
After this change this will be `-fuse-ld=/usr/bin/ld.gold` or similar
depending on what clang reported.

Gcc will still use -fuse-ld=gold (as it doesn't accept absolute
path argument to `-fuse-ld`).

This is to make the Clang autogenerated toolchain not depend on PATH if
we can help it. Gcc is still leaking the PATH.

The leak:

1) CC is installed in a non-standard location, or is checked-in into the
   workspace.
2) CC is passed to Bazel's cc_configure, Bazel autoconfigures the
   toolchain using CC.
3) Remote worker has CC installed in the same non-standard location, or
   gets CC in action inputs.
4) If Host system has gold on its PATH (potentially to the surprise
   of the user, user might assume that since they provide CC explicitly
   only that installation is used), Bazel will autoconfigure the
   toolchain to use gold linker.
5) Remote system doesn't have gold installed, build fails.

Alternative solutions:
* Pass -B$(dirname $(which gcc)) to CC and pass empty PATH to all
  Bazel's C++ autoconfiguration commands
* Do not detect gold automatically at all, require environment variable
  to be explicitly set to enable gold

Both are backwards incompatible changes. Solution in this PR is
backwards compatible. It's also not fixing the issue for Gcc.

Closes #8580.

PiperOrigin-RevId: 256623542
  • Loading branch information
hlopko authored and copybara-github committed Jul 5, 2019
1 parent 29eecb5 commit cdd0c3c
Showing 1 changed file with 38 additions and 8 deletions.
46 changes: 38 additions & 8 deletions tools/cpp/unix_cc_configure.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -167,21 +167,51 @@ def _is_linker_option_supported(repository_ctx, cc, option, pattern):
])
return result.stderr.find(pattern) == -1

def _is_gold_supported(repository_ctx, cc):
"""Checks that `gold` is supported by the C compiler."""
def _find_gold_linker_path(repository_ctx, cc):
"""Checks if `gold` is supported by the C compiler.
Args:
repository_ctx: repository_ctx.
cc: path to the C compiler.
Returns:
String to put as value to -fuse-ld= flag, or None if gold couldn't be found.
"""
result = repository_ctx.execute([
cc,
"-fuse-ld=gold",
str(repository_ctx.path("tools/cpp/empty.cc")),
"-o",
"/dev/null",
# Some macos clang versions don't fail when setting -fuse-ld=gold, adding
# these lines to force it to. This also means that we will not detect
# gold when only a very old (year 2010 and older) is present.
"-Wl,--start-lib",
"-Wl,--end-lib",
str(repository_ctx.path("tools/cpp/empty.cc")),
"-fuse-ld=gold",
"-v",
])
return result.return_code == 0
if result.return_code != 0:
return None

for line in result.stderr.splitlines():
if line.find("gold") == -1:
continue
for flag in line.split(" "):
if flag.find("gold") == -1:
continue

# flag is '-fuse-ld=gold' for GCC or "/usr/lib/ld.gold" for Clang
# strip space, single quote, and double quotes
flag = flag.strip(" \"'")

# remove -fuse-ld= from GCC output so we have only the flag value part
flag = flag.replace("-fuse-ld=", "")
return flag
auto_configure_warning(
"CC with -fuse-ld=gold returned 0, but its -v output " +
"didn't contain 'gold', falling back to the default linker.",
)
return None

def _add_compiler_option_if_supported(repository_ctx, cc, option):
"""Returns `[option]` if supported, `[]` otherwise. Doesn't %-escape the option."""
Expand Down Expand Up @@ -348,7 +378,7 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools):
"",
False,
), ":")
supports_gold_linker = _is_gold_supported(repository_ctx, cc)
gold_linker_path = _find_gold_linker_path(repository_ctx, cc)
cc_path = repository_ctx.path(cc)
if not str(cc_path).startswith(str(repository_ctx.path(".")) + "/"):
# cc is outside the repository, set -B
Expand Down Expand Up @@ -457,7 +487,7 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools):
),
"%{cxx_flags}": get_starlark_list(cxx_opts + _escaped_cplus_include_paths(repository_ctx)),
"%{link_flags}": get_starlark_list((
["-fuse-ld=gold"] if supports_gold_linker else []
["-fuse-ld=" + gold_linker_path] if gold_linker_path else []
) + _add_linker_option_if_supported(
repository_ctx,
cc,
Expand Down Expand Up @@ -532,6 +562,6 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools):
"%{dbg_compile_flags}": get_starlark_list(["-g"]),
"%{coverage_compile_flags}": coverage_compile_flags,
"%{coverage_link_flags}": coverage_link_flags,
"%{supports_start_end_lib}": "True" if supports_gold_linker else "False",
"%{supports_start_end_lib}": "True" if gold_linker_path else "False",
},
)

0 comments on commit cdd0c3c

Please sign in to comment.