-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Mangle rustc_std_internal_symbols functions #127173
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
Mangle rustc_std_internal_symbols functions #127173
Conversation
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
Some changes occurred in compiler/rustc_codegen_gcc |
This comment has been minimized.
This comment has been minimized.
aea9a3c
to
db57b2a
Compare
This comment has been minimized.
This comment has been minimized.
db57b2a
to
4859689
Compare
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #127162) made this pull request unmergeable. Please resolve the merge conflicts. |
4859689
to
10bad43
Compare
This comment has been minimized.
This comment has been minimized.
The Miri subtree was changed cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
r? compiler |
Opened rust-lang/miri#3724 with some miri changes that can be made independently of this PR. |
Use the symbol_name query instead of trying to infer from the link_name attribute This prevents the calculated name from going out of sync with exported_symbols. It also avoids having to special case the panic_impl lang item. It also makes it easier to fix miri with rust-lang/rust#127173.
☔ The latest upstream changes (presumably #125507) made this pull request unmergeable. Please resolve the merge conflicts. |
Use the symbol_name query instead of trying to infer from the link_name attribute This prevents the calculated name from going out of sync with exported_symbols. It also avoids having to special case the panic_impl lang item. It also makes it easier to fix miri with rust-lang#127173.
e1d353a
to
f8f4b88
Compare
This comment has been minimized.
This comment has been minimized.
f8f4b88
to
266c7c2
Compare
This comment has been minimized.
This comment has been minimized.
266c7c2
to
9c91546
Compare
Oh man, this broke some things for us. We have a pipeline where we need to link a static Rust library to a full Rust program (we had been doing symbol renaming to get this functional) as we need different sanitization levels between the two. cc @tokatoka @rmalmain, this is our CI failure w/ libafl_libfuzzer. |
@addisoncrump It sounds like you are deep in unspecified territory and are bypassing things that have been explicitly introduced to avoid unstable implementation details leaking out. This is definitely off-label usage of rustc. Might be worth having a discussion on Zulip or so to figure out if there isn't a less cursed way to support your usecase. |
Please don't comment on an already merged PR, it's impossible to find and has no visibility. Consider opening an issue or zulip thread if you want to discuss your use case. |
Yup, we slammed this one together because of some niche linkage requirements from a sanitizer. I'll ask on Zulip, but we had previously discussed this with others and this was the best conclusion we came up with 🙃
Sure, we figured this change was generally accepted and our use case is outside of what is considered normally permissible. Just left a comment as that: a comment. No action required, we work around it. |
I've opened a Zulip thread for this discussion: https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Linking.20Rust.20libraries.20with.20different.20sanitizers |
This PR broke panic handlers that also happen to have #[panic_handler]
#[no_mangle]
fn panic(info: &PanicInfo) -> ! {
todo!()
}
Removing the rust-osdev/bootloader v0.9 is affected by this issue: rust-osdev/bootloader#499. |
This was brought up on Zulip, would you mind following up there? #t-compiler > no_mangle rust_begin_unwind |
fwiw, chromium is also broken by this due to defining the allocator functions in c++: https://crbug.com/407024458 |
Right, I did at some point see that Chromium defined them in C++, but completely forgot about it. Are you able to fix this on your end before the next stable release of rustc? If not I could add a temporary exception for mangling of the allocator symbols. This exception will have to be removed at some point though. |
Can't Chromium define The |
If the fix Ralf suggested works out, I think we're good.
I'm not an expert on our Rust build, but yes that sounds like a good solution. Based on https://source.chromium.org/chromium/chromium/src/+/main:build/rust/std/remap_alloc.cc;l=61 it sounds like that was already the plan, but we didn't get to it yet :) |
IIUC (*) Because linking is the only step when
FWIW having Chromium provide a weak definition of
I don't know. Is there another mechanism that Chromium can use to ask (Chromium doesn't use the stable release of |
If you're using a nightly rustc, maybe we can gate this temporary work-around on a But yeah someone will have to push on #73632 to solve this properly. That might make a good project goal for 2025h2, if the Chromium team is willing to put in the work. :) |
|
That would be very helpful for us. Thanks! |
Culprit PRs: - rust-lang/rust#138384 - rust-lang/rust#127173 - rust-lang/rust#138659, Resolves #3961, #3976 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.
…bol, r=wesleywiser,jieyouxu Mangle rustc_std_internal_symbols functions This reduces the risk of issues when using a staticlib or rust dylib compiled with a different rustc version in a rust program. Currently this will either (in the case of staticlib) cause a linker error due to duplicate symbol definitions, or (in the case of rust dylibs) cause rustc_std_internal_symbols functions to be silently overridden. As rust gets more commonly used inside the implementation of libraries consumed with a C interface (like Spidermonkey, Ruby YJIT (curently has to do partial linking of all rust code to hide all symbols not part of the C api), the Rusticl OpenCL implementation in mesa) this is becoming much more of an issue. With this PR the only symbols remaining with an unmangled name are rust_eh_personality (LLVM doesn't allow renaming it) and `__rust_no_alloc_shim_is_unstable`. Helps mitigate rust-lang#104707 try-job: aarch64-gnu-debug try-job: aarch64-apple try-job: x86_64-apple-1 try-job: x86_64-mingw-1 try-job: i686-mingw-1 try-job: x86_64-msvc-1 try-job: i686-msvc-1 try-job: test-various try-job: armhf-gnu
Since [1] this symbol is mangled, meaning it is not easy to call directly. A better fix will come in [2] but for now, just disable that portion of the test. [1]: rust-lang#127173 [2]: rust-lang/compiler-builtins#802
Since [1] this symbol is mangled, meaning it is not easy to call directly. A better fix will come in [2] but for now, just disable that portion of the test. [1]: rust-lang#127173 [2]: rust-lang/compiler-builtins#802
This adds a version of the allocator support libraries implemented in rust, to account for the new internal allocator symbol mangling applied by rustc (rust-lang/rust#127173). This mechanism relies on unstable language features and requires a nightly rustc from 2025-04-05 or later. It is designed to be compatible with: * all allocator flavors (the default and global) * all flavors of `no_std` * both situations for when a `rust_library` is used as a dependency of a `cc_binary` and when the `experimental_use_cc_common_link` setting is used. Rustc generates references to internal allocator symbols when building rust libraries. At link time, rustc generates the definitions of these symbols. When rustc is not used as the final linker, we need to generate the definitions ourselves. This happens for example when a `rust_library` is used as a dependency of a `rust_binary`, or when the `experimental_use_cc_common_link` setting is used. For older versions of rustc, the allocator symbol definitions can be provided via the `rust_toolchain`'s `allocator_library` or `global_allocator_library` attributes, with sample targets like `//ffi/cc/allocator_library` and `//ffi/cc/global_allocator_library`. Recent versions of rustc started mangling these allocator symbols (rust-lang/rust#127173). The mangling uses a scheme that is specific to the exact version of the compiler. This makes the cc allocator library definitions ineffective. To work around this, we provide rust versions of the symbol definitions annotated with an unstable language attribute that instructs rustc to mangle them consistently. Because of the usage of unstable language features, this is only compatible with nightly versions of the compiler. Since the new symbol definitions are written in rust, we cannot just attach them as attributes on the `rust_toolchain` as the old cc versions, as that would create a build graph cycle: 1. any `rust_library` depends on a `rust_toolchain`, 2. the `rust_toolchain` depends on the allocator library, 3. so the allocator library cannot just be a `rust_library` directly. The bootstrapping cycle can be avoided by defining a separate internal "initial" rust toolchain specifically for building the rust allocator libraries, and use a transition to attach the generated allocator libraries to the "main" rust toolchain. But that duplicates the whole subgraph of the build around the rust toolchains, repository and supporting tools used for them. Instead, we define a new custom `rust_allocator_libraries` rule, which builds the new rust allocator libraries and exposes them via a new `AllocatorLibrariesInfo` provider. Since we cannot attach such a target as an attribute to the `rust_toolchain`, we attach it as a new `allocator_libraries` common attribute to the rust rules. We ported the cc versions into the new rust implementations of the (default) allocator and global allocator flavors in `//ffi/rs/allocator_library` and `//ffi/rs/global_allocator_library`. The rust standard libraries themselves have references to the allocator libraries. For correct linking order of the standard libraries, we need to establish the new rust allocator libraries as link-time dependencies of the standard libraries. The specific set of linker inputs depends on the no_std flavor and the choice of (default) allocator vs. global allocator. We establish the linking dependencies by extending the `rust_toolchain` `_make_libstd_and_allocator_ccinfo` feature to let us inject a custom allocator library as a standard library dependency and using that in the `rust_allocator_libraries` implementation to generate the final standard libraries linker graph. The new functionality is opt-in and gated via: 1. the new setting `//rust/settings:experimental_use_allocator_libraries_with_mangled_symbols`, which supplies the default value for the repository, and 2. the new `rust_toolchain` attribute `experimental_use_allocator_libraries_with_mangled_symbols`, which lets the user override the choice of using the new-style rust-based allocator symbols, or the old-style cc-based ones for a specific toolchain. For testing, I created variants for the cc_common.link / no_std / global_allocator test matrix. I found it useful to be able to declare `target_settings` via the `rust.repository_set` rule, similarly to the `target_compatible_with`. This lets us include additional toolchains in the same repository that are distinguished by the setting, like in: https://github.com/bazelbuild/rules_rust/pull/3403/files#diff-164dd740b73baa94f9a84ed0d4a7ec29f8adeb34ff8430492747e2443f7f39d3
This reduces the risk of issues when using a staticlib or rust dylib compiled with a different rustc version in a rust program. Currently this will either (in the case of staticlib) cause a linker error due to duplicate symbol definitions, or (in the case of rust dylibs) cause rustc_std_internal_symbols functions to be silently overridden. As rust gets more commonly used inside the implementation of libraries consumed with a C interface (like Spidermonkey, Ruby YJIT (curently has to do partial linking of all rust code to hide all symbols not part of the C api), the Rusticl OpenCL implementation in mesa) this is becoming much more of an issue. With this PR the only symbols remaining with an unmangled name are rust_eh_personality (LLVM doesn't allow renaming it) and
__rust_no_alloc_shim_is_unstable
.Helps mitigate #104707
try-job: aarch64-gnu-debug
try-job: aarch64-apple
try-job: x86_64-apple-1
try-job: x86_64-mingw-1
try-job: i686-mingw-1
try-job: x86_64-msvc-1
try-job: i686-msvc-1
try-job: test-various
try-job: armhf-gnu