Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

updates needed for native-links-modifiers RFC stabilization #1268

Open
krasimirgg opened this issue Apr 19, 2022 · 0 comments
Open

updates needed for native-links-modifiers RFC stabilization #1268

krasimirgg opened this issue Apr 19, 2022 · 0 comments
Assignees

Comments

@krasimirgg
Copy link
Collaborator

Related to #637.
As part of the stabilization of the native-link-modifiers RFC, rust-lang commit rust-lang/rust@1004783 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 #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 krasimirgg self-assigned this Apr 19, 2022
krasimirgg added a commit to krasimirgg/rules_rust that referenced this issue Apr 19, 2022
scentini pushed a commit that referenced this issue Apr 20, 2022
#1269)

This adds a new stopgap attribute enabling us to fall back to the old rustc behavior wrt whole-archive handling, see #1268.
goffrie pushed a commit to goffrie/rules_rust that referenced this issue Apr 21, 2022
bazelbuild#1269)

This adds a new stopgap attribute enabling us to fall back to the old rustc behavior wrt whole-archive handling, see bazelbuild#1268.
scentini pushed a commit that referenced this issue May 16, 2022
This isn't needed anymore after #1333 I believe. This is part of the work on #1268.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant