rules_rust icon indicating copy to clipboard operation
rules_rust copied to clipboard

updates needed for native-links-modifiers RFC stabilization

Open krasimirgg opened this issue 2 years ago • 0 comments

Related to https://github.com/bazelbuild/rules_rust/issues/637. As part of the stabilization of the native-link-modifiers RFC, rust-lang commit https://github.com/rust-lang/rust/commit/1004783ef9bdcf006f0ed33badacf83a5934feb2 changes the way rustc lowers -l flags to the linker invocation: an invocation like rustc … -lstatic=lib:

  • before produced -Wl,--whole-archive -llib -Wl,--no-whole-archive
  • after produces a plain -llib linker flag fragment.

After this change, it's now possible to instruct rustc to produce a whole-archive linker invocation fragment by using the new whole-archive linker modifier (rustc … -lstatic:+whole-archive=lib ...). This behavior change is stabilized in recent rustc nightlies.

This is a good rustc change since it gives fine-grained control over the exact linker flavor (e.g., as a whole archive) of native dependencies to final artifacts, which will allow us to make progress on https://github.com/bazelbuild/rules_rust/issues/637.

However the new behavior causes the linking of final rules_rust targets to fail in some cases containing hybrid code interfacing between Rust and native libraries, where it was working previously: with the behavior before native libraries were passed as whole-archives to the linker which effectively disabled the linker backward reference diagnostics and made their linker flag fragments largely order-independent. With the new behavior, these turn into hard linker errors sometimes, e.g.,ld: error: backward reference detected. when linking with ld with --warn-backrefs. Internally we've got a few dozed cases, fixing them might take a while and potentially may need additional rules_rust work to adapt our linker flag generation with the new rustc behavior.

To give us some time to diagnose and fix these issues, I'd like to start by introducing a stopgap attribute to allow us to switch back to the old behavior for problematic targets and selectively migrate them to the new behavior one by one. Specifically, I'd like to add a new experimental_use_whole_archive_for_native_deps: bool; default: false attribute to common rust_target attributes. If explicitly set to true for a final target, its effect would be to use the whole-archive modifier for native dependencies (e.g., -lstatic:+whole-archive=lib instead of the standard -lstatic=lib).

I'd like to use this issue to track whatever additional work needs to be done to adapt rules_rust for the new rustc behavior and the eventual removal of the stopgap attribute.

krasimirgg avatar Apr 19 '22 09:04 krasimirgg