bazelbuild / rules_rust

Rust rules for Bazel
https://bazelbuild.github.io/rules_rust/
Apache License 2.0
643 stars 406 forks source link

GCC cannot find linker when rustc invoked with #1114

Open harrysarson opened 2 years ago

harrysarson commented 2 years ago

Hi,

We have a bazel setup with a cc_toolchain configured to use gold via --fuse-ld=gold. When rules rust runs rustc it forwards the linker flags configured in the cc_toolchain to rustc with --codegen 'link-args=<snipped> -fuse-ld=gold'.

However, the environment variables are such that PATH is not set and therefore gcc cannot find the gold executable (which is in /usr/bin/gold). This means I am seeing errors looking like:

error: linking with `/usr/bin/gcc` failed: exit status: 1                                                                                                                                                        |                                                                                                                                                                                                              = note: "/usr/bin/gcc" <snipped> "-fuse-ld=gold"
  = note: collect2: fatal error: cannot find 'ld'
          compilation terminated.

error: aborting due to previous error

(cannot find 'ld' is just bad error reporting from gcc. It means that gcc cannot find gold).


The only way I found to fix this is to patch rules rust like so:

diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
index 9249ff8..f415b61 100644
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -676,6 +676,7 @@ def construct_arguments(

     # Set the SYSROOT to the directory of the rust_std files passed to the toolchain
     env["SYSROOT"] = paths.dirname(toolchain.rust_std.files.to_list()[0].short_path)
+    env["PATH"] = "/usr/bin"

     # extra_rustc_flags apply to the target configuration, not the exec configuration.
     if hasattr(ctx.attr, "_extra_rustc_flags") and not is_exec_configuration(ctx):

I was wondering if there was a way to configure the PATH env variable used when running rustc? Possibly I am missing something and this is already doable without patching rules_rust, if so I am sorry!

illicitonion commented 2 years ago

That's interesting - I would have expected at least gcc to look in the same dir as itself... In my cc_toolchain (using clang not gcc), I have -fuse-ld=lld, and it leads to clang finding the file in the same directory as it named ld.lld, without any $PATH being specially set... Out of interest, if you create a symlink pointing /usr/bin/ld.gold at /usr/bin/gold, does that help?

harrysarson commented 2 years ago

Out of interest, if you create a symlink pointing /usr/bin/ld.gold at /usr/bin/gold, does that help?

Yeah, it turns out the symlink alraedy exists in my environment:

> ls /usr/bin -lh | grep gold
lrwxrwxrwx 1 root root     21 Oct 20 11:09 gold -> x86_64-linux-gnu-gold
lrwxrwxrwx 1 root root     24 Oct 20 11:09 ld.gold -> x86_64-linux-gnu-ld.gold
lrwxrwxrwx 1 root root     24 Oct 20 11:09 x86_64-linux-gnu-gold -> x86_64-linux-gnu-ld.gold
-rwxr-xr-x 1 root root   2.3M Oct 20 11:09 x86_64-linux-gnu-ld.gold
harrysarson commented 2 years ago

worth noting, this isn't specific to gold. If I remove --fuse-ld=ld then GCC still fails to find ld

illicitonion commented 2 years ago

This definitely feels like something the cc_toolchain should be setting - it's possible that we're not propagating some information from the cc_toolchain properly, or it's possible the cc_toolchain is misconfigured...

This is where we currently get the env vars to set for the linker to work: https://github.com/bazelbuild/rules_rust/blob/c435cf4478fc6e097edc5dba0e71de6608ab77d8/rust/private/rustc.bzl#L292-L296

As far as I can tell, this is doing the right thing, which suggests you may need to modify your cc_toolchain somehow to set $PATH so that when we do this env lookup $PATH is present? But if you're confident your cc_toolchain is set up right, that's where I'd add some debugging to work out if we're dropping something somewhere...

wt commented 2 years ago

I actually fixed this for the case where the gcc and ld were not in the same directory. However, if you are using gold, I didn't fix that case as I didn't know how to fix it. Let me find my commit on bazel. It should be a pretty easy fix.

https://github.com/bazelbuild/bazel/issues/13857

Specifically, the following lines need to be pulled out of the conditional block and do the right thing for the gold case: https://github.com/bazelbuild/bazel/blob/a9fb0fc53e88d0e7b334afd22fabebd00b646f4c/tools/cpp/unix_cc_configure.bzl#L435-L436

A little more context, the -B arg for gcc is used to specify the directory where it looks for binary tools like the linker. My patch simply added another -B arg if gcc and ld were in different directories. Presumably gold is in a different directory than your gcc, so I assume you would need to do the same in that case.

Also, I think it's important to note that this is not a rules_rust issue. It's a bazel issue.

wt commented 2 years ago

And here's a diff to work around this in your local rules_rust before bazel is updated to fix the problem:

commit 8b66ca12b974de5d6edc2a8d5a099978baa10cd3
Author: Wren Turkal <wturkal@twitter.com>
Date:   Thu Mar 3 12:31:53 2022 -0800

    Add binutils to binpath for gcc:

    My binutils binaries are in a different directory to gcc. I needed to
    work around a bug in bazel iteself reported at
    https://github.com/bazelbuild/bazel/issues/13857.

diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl
index b64a3cb..1a57977 100644
--- a/rust/private/rustc.bzl
+++ b/rust/private/rustc.bzl
@@ -14,6 +14,7 @@

 """Functionality for constructing actions that invoke the Rust compiler"""

+load("@bazel_skylib//lib:paths.bzl", "paths")
 load(
     "@bazel_tools//tools/build_defs/cc:action_names.bzl",
     "CPP_LINK_EXECUTABLE_ACTION_NAME",
@@ -296,6 +297,8 @@ def get_linker_and_args(ctx, attr, cc_toolchain, feature_configuration, rpaths):
         action_name = CPP_LINK_EXECUTABLE_ACTION_NAME,
         variables = link_variables,
     )
+    link_args = [i for i in link_args]
+    link_args.append("-B" + paths.dirname(cc_toolchain.ld_executable))
     link_env = cc_common.get_environment_variables(
         feature_configuration = feature_configuration,
         action_name = CPP_LINK_EXECUTABLE_ACTION_NAME,
wt commented 2 years ago

Actually, now this diff is still required in my internal rules_rust to make builds work. I think some variation of this patch needs to make it into rules_rust. I suspect with my change there may be a better way to get the -B param, but I have not looked more closely at it. This patch was a hack to workaround the problem.