Mangle rustc_std_internal_symbols functions #127173

bjorn3 · 2024-06-30T15:57:46Z

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

rustbot · 2024-06-30T15:57:54Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-06-30T15:57:56Z

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

bors · 2024-06-30T19:31:59Z

☔ The latest upstream changes (presumably #127162) made this pull request unmergeable. Please resolve the merge conflicts.

rustbot · 2024-06-30T20:27:11Z

The Miri subtree was changed

cc @rust-lang/miri

fee1-dead · 2024-07-01T03:57:31Z

r? compiler

bjorn3 · 2024-07-01T17:41:51Z

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.

bors · 2024-07-03T14:47:05Z

☔ 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.

addisoncrump · 2025-03-19T12:32:34Z

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.

RalfJung · 2025-03-20T06:49:50Z

@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.

jieyouxu · 2025-03-20T06:52:14Z

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.

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.

addisoncrump · 2025-03-20T07:36:25Z

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.

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 🙃

it's impossible to find and has no visibility

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.

addisoncrump · 2025-03-20T07:55:38Z

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

Freax13 · 2025-03-22T06:34:34Z

This PR broke panic handlers that also happen to have #[no_mangle] on them:

#[panic_handler]
#[no_mangle]
fn panic(info: &PanicInfo) -> ! {
    todo!()
}

error: linking with `rust-lld` failed: exit status: 1
  |
  = note:  "rust-lld" "-flavor" "gnu" "--script=linker.ld" "--gc-sections" "/tmp/nix-shell-268986-0/rustcOzjusq/symbols.o" "<4 object files omitted>" "--as-needed" "-Bstatic" "/home/freax13/code/bootloader/target/x86_64-bootloader/release/deps/{libxmas_elf-66bc2f3042db6d0a.rlib,libzero-4ac7a3e5a275490c.rlib,libx86_64-071c4d87b3f41b86.rlib,libvolatile-3494b28e1872b638.rlib,libbitflags-c139daa6402e315d.rlib,libbit_field-44e684cd92ae48a9.rlib,libusize_conversions-f8d542993493606b.rlib,libfixedvec-047d254b673107be.rlib,libbootloader-23a5442f6f2666f3.rlib,librlibc-fbafd460f1ad3018.rlib,librustc_std_workspace_core-8032938bf1041ab0.rlib,libcore-6b618e1a3fa2eac5.rlib,libcompiler_builtins-e7ef8faa94df0138.rlib}.rlib" "-L" "/tmp/nix-shell-268986-0/rustcOzjusq/raw-dylibs" "-Bdynamic" "--eh-frame-hdr" "-z" "noexecstack" "-L" "/home/freax13/code/bootloader/target/x86_64-bootloader/release/build/bootloader-b78ad0a097d73cb6/out" "-o" "/home/freax13/code/bootloader/target/x86_64-bootloader/release/deps/bootloader-29b7d286398a0461" "--gc-sections" "-O1"
  = note: some arguments are omitted. use `--verbose` to show all linker arguments
  = note: rust-lld: error: undefined symbol: __rustc::rust_begin_unwind
          >>> referenced by panicking.rs:75 (src/panicking.rs:75)
          >>>               core-6b618e1a3fa2eac5.core.ba490f4106a6f0a6-cgu.12.rcgu.o:(core::panicking::panic_fmt::hf5a770ebcc6a8913) in archive /home/freax13/code/bootloader/target/x86_64-bootloader/release/deps/libcore-6b618e1a3fa2eac5.rlib

Removing the #[no_mangle] attribute makes it work again. Is this considered a bug?

rust-osdev/bootloader v0.9 is affected by this issue: rust-osdev/bootloader#499.

tgross35 · 2025-03-22T06:39:13Z

This PR broke panic handlers that also happen to have #[no_mangle] on them:

This was brought up on Zulip, would you mind following up there? #t-compiler > no_mangle rust_begin_unwind

zmodem · 2025-04-01T09:21:53Z

fwiw, chromium is also broken by this due to defining the allocator functions in c++: https://crbug.com/407024458

bjorn3 · 2025-04-01T09:33:01Z

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.

RalfJung · 2025-04-01T09:53:00Z

Can't Chromium define chromium_rust_alloc etc symbols in C++, and then in Rust have a #[global_allocator] that dispatches to those symbols?

The __rust_alloc etc symbols are a rustc implementation detail and should never be implemented by anything else.

zmodem · 2025-04-01T11:32:18Z

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.

If the fix Ralf suggested works out, I think we're good.

Can't Chromium define chromium_rust_alloc etc symbols in C++, and then in Rust have a #[global_allocator] that dispatches to those symbols?

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 :)

anforowicz · 2025-04-01T17:32:02Z

Can't Chromium define chromium_rust_alloc etc symbols in C++, and then in Rust have a #[global_allocator] that dispatches to those symbols?

IIUC #[global_allocator] will be analyzed and used when rustc drives the final linking step (*). This is not what happens in Chromium where clang drives the final linking for most executables.

(*) Because linking is the only step when rustc can find the single crate that provides a #[global_allocator]. When compiling other crates rustc won't be aware that this attribute is present in some other crate.

The __rust_alloc etc symbols are a rustc implementation detail and should never be implemented by anything else.

FWIW having Chromium provide a weak definition of __rust_alloc has been discussed as a work around in #73632.

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.

I don't know. Is there another mechanism that Chromium can use to ask rustc to use a Chromium-specific allocator (PartitionAlloc) when rustc is used to build rlibs, but the final linking is driven by clang? If not, then we would appreciate a temporary exemption to unblock rolling Rust toolchain to a newer version in Chromium.

(Chromium doesn't use the stable release of rustc, because Chromium uses a recent nightly release of clang and therefore needs to also use a recent version of rustc so that LTO can see the same version of LLVM IR.)

RalfJung · 2025-04-01T18:15:07Z

If you're using a nightly rustc, maybe we can gate this temporary work-around on a -Z flag.

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. :)

bjorn3 · 2025-04-01T20:18:46Z

#[global_allocator] has actually been expanding to direct definitions of __rust_alloc and co. Only when #[global_allocator] is not used, do __rust_alloc and co get generated right before linking.

zmodem · 2025-04-02T06:59:43Z

If you're using a nightly rustc, maybe we can gate this temporary work-around on a -Z flag.

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

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

rustbot assigned fee1-dead Jun 30, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 30, 2024