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

Added Rust 1.71.0 #2062

Merged
merged 7 commits into from
Jul 27, 2023
Merged

Added Rust 1.71.0 #2062

merged 7 commits into from
Jul 27, 2023

Conversation

UebelAndre
Copy link
Collaborator

@UebelAndre
Copy link
Collaborator Author

UebelAndre commented Jul 26, 2023

rust-lang/rust#73632 (comment) seems to be the fix for https://buildkite.com/bazel/rules-rust-rustlang/builds/9049#01895590-f668-4118-981a-8fd572da4147 but I'm not sure what the integration should look like for rules_rust.

@UebelAndre
Copy link
Collaborator Author

rust-lang/rust#73632 (comment) seems to be the fix for https://buildkite.com/bazel/rules-rust-rustlang/builds/9049#01895590-f668-4118-981a-8fd572da4147 but I'm not sure what the integration should look like for rules_rust.

Cc @illicitonion @scentini @krasimirgg i don’t currently use this feature. Do any of you and do you have thoughts on how to best define this symbol?

@scentini
Copy link
Collaborator

@UebelAndre Google uses it.

I believe we need to add

__attribute__((weak)) uint8_t __rust_no_alloc_shim_is_unstable = 0;

to
https://github.com/bazelbuild/rules_rust/blob/main/test/cc_common_link/global_allocator_library.cc
and https://github.com/bazelbuild/rules_rust/blob/main/test/cc_common_link/allocator_library.cc

@scentini
Copy link
Collaborator

@UebelAndre I'm taking a look at the allocator related failures.

@UebelAndre
Copy link
Collaborator Author

@UebelAndre I'm taking a look at the allocator related failures.

Danke!

@scentini
Copy link
Collaborator

Ok, I think I've got it figured out:

The ios example fails because there is an additional https://github.com/bazelbuild/rules_rust/blob/main/examples/ios/allocator_library.cc that we need to add __rust_no_alloc_shim_is_unstable to.

For the cc_common.link with global allocator failure we have guidance over at rust-lang/rust#73632 (comment):

In addition if you are using #[global_allocator], you must stop defining __rust_alloc, __rust_dealloc, __rust_realloc and __rust_alloc_zeroed as they are now directly defined by the #[global_allocator] expansion rather than as part of the allocator shim.

So over in https://github.com/bazelbuild/rules_rust/blob/main/test/cc_common_link/global_allocator_library.cc we need to remove the following section:

extern "C" uint8_t *__rg_alloc(uintptr_t size, uintptr_t align);
extern "C" __attribute__((weak))
uint8_t *__rust_alloc(uintptr_t size, uintptr_t align) {
  return __rg_alloc(size, align);
}
extern "C" void __rg_dealloc(uint8_t *ptr, uintptr_t size, uintptr_t align);
extern "C" __attribute__((weak))
void __rust_dealloc(uint8_t *ptr, uintptr_t size, uintptr_t align) {
  __rg_dealloc(ptr, size, align);
}
extern "C" uint8_t *__rg_realloc(uint8_t *ptr, uintptr_t old_size, uintptr_t align,
                       uintptr_t new_size);
extern "C" __attribute__((weak))
uint8_t *__rust_realloc(uint8_t *ptr, uintptr_t old_size, uintptr_t align,
                        uintptr_t new_size) {
  return __rg_realloc(ptr, old_size, align, new_size);
}
extern "C" uint8_t *__rg_alloc_zeroed(uintptr_t size, uintptr_t align);
extern "C" __attribute__((weak))
uint8_t *__rust_alloc_zeroed(uintptr_t size, uintptr_t align) {
  return __rg_alloc_zeroed(size, align);
}

@UebelAndre
Copy link
Collaborator Author

Would removing those affect users targeting older versions of rust?

@scentini
Copy link
Collaborator

Yes, although I think that is fine as we deliberately only provide allocator_library.cc in examples and tests, and not as an official library that comes with rules_rust. Invoking the linker directly and not via rustc is not officially supported today by the Rust project, and as long as that's the case we'll guard the functionality behind an --experimental flag and we'll expect that the user themselves provides an allocator_library.cc that works for the toolchain version they are using.

We could be a bit helpful by not removing the lines of code but rather commenting them out with a note that they are needed for versions <1.71.

@UebelAndre UebelAndre enabled auto-merge (squash) July 27, 2023 10:18
@UebelAndre UebelAndre merged commit 4bd44d0 into bazelbuild:main Jul 27, 2023
2 of 3 checks passed
@UebelAndre UebelAndre deleted the rustup branch July 27, 2023 10:19
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.

None yet

3 participants