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

Respect #[global_allocator] in cc_common.link builds #1926

Merged
merged 6 commits into from
Apr 20, 2023

Conversation

scentini
Copy link
Collaborator

@scentini scentini commented Apr 19, 2023

rustc generates allocator symbols at link time. When we link via cc_common.link those symbols are missing because rustc is not driving the linking anymore. We compensate by providing the symbols via the rust_toolchain.allocator_library.
Rust also allows a global allocator to be specified (via #[global_allocator]). The symbols rustc generates at link time are different based on whether the system allocator is used, or a global allocator was specified.

Currently, when linking via cc_common.link, #[global_allocator] is completely ignored. This PR makes using a global allocator actually work:

  1. Adding a rust_toolchain.global_allocator_library attribute that can point to a set of symbols needed when #[global_allocator] is used.
  2. Adding a build flag --@rules_rust//rust/settings:experimental_use_global_allocator flag that is only observed when --@rules_rust//rust/settings:experimental_use_cc_common_link==True. At this point this flag only applies to the target configuration.

Unrelated to the goal of this PR, I also modified visibility of a rust_toolchain in test/toolchain/toolchain_test.bzl to address failures of Bazel rolling releases tasks (https://buildkite.com/bazel/rules-rust-rustlang/builds/8457#018799ff-5908-4b57-918b-a582ed12a248)

@scentini scentini marked this pull request as ready for review April 19, 2023 16:00
Copy link
Collaborator

@krasimirgg krasimirgg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@@ -270,6 +272,7 @@ def BUILD_for_rust_toolchain(
exec_triple (triple): The rust-style target that this compiler runs on
target_triple (triple): The rust-style target triple of the tool
allocator_library (str, optional): Target that provides allocator functions when rust_library targets are embedded in a cc_binary.
global_allocator_library (str, optional): Target that provides allocator functions global allocator is used with cc_common_link.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Target that provides allocator functions when ...
nit: could you clarify that this applies only to non-exec mode

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@scentini scentini enabled auto-merge (squash) April 20, 2023 12:22
@scentini scentini merged commit c6ad23a into bazelbuild:main Apr 20, 2023
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

Successfully merging this pull request may close these issues.

2 participants