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.

jieyouxu · 2025-03-04T15:06:01Z

I can't reproduce this locally on arm64 linux. Neither with nor without lld.

I'm not 100% sure, but maybe this symbol comes from LLVM compiler-rt?

https://github.com/llvm/llvm-project/blob/794ba171a43baaf9de9739440ddb582500527031/compiler-rt/lib/builtins/cpu_model/aarch64.c#L59-L64

Though the symbol visibility of that is hidden so idk.

For staticlib we still keep checking symbols that don't contain rust as substring.

bjorn3 · 2025-03-04T16:13:59Z

Though the symbol visibility of that is hidden so idk.

Looks like symbol.is_global() doesn't take symbol visibility into account, just linkage.

I've changed symbols_check to ignore symbols that don't have rust in the name, but keep symbols_check_archive check all symbols. I've also changed the test back to running on all targets.

jieyouxu · 2025-03-04T16:58:36Z

That seems reasonble. I'd like to see if there's any nasty surprises since the queue is currently backed up a lot, so

@bors try

…bol, r=<try> 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

bors · 2025-03-04T16:59:50Z

⌛ Trying commit bf70cce with merge 6255ed0...

bors · 2025-03-04T19:37:59Z

💔 Test failed - checks-actions

bjorn3 · 2025-03-05T14:53:48Z

@bors try

bors · 2025-03-05T14:55:02Z

⌛ Trying commit 6d4de52 with merge ed25e8e...

…bol, r=<try> 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

bors · 2025-03-05T17:31:07Z

💔 Test failed - checks-actions

jieyouxu · 2025-03-05T17:32:54Z

Hm, is that from a dependency 🤔

bjorn3 · 2025-03-06T13:15:08Z

.refptr. and __imp_ are implementation details of the PE/COFF object file formats. I've changed the test to only run on ELF for which I know that this kind of weird things don't happen.

@rustbot ready

jieyouxu · 2025-03-06T14:52:37Z

@bjorn3 I think you forgot to push. But r=wesleywiser and me afterwards; that sounds reasonable.

bjorn3 · 2025-03-06T15:09:43Z

I did push, but it seems that github is still processing the push.

bjorn3 · 2025-03-06T15:10:42Z

git commit --amend && git push --force-with-lease fixed it.

@bors r=wesleywiser,jieyouxu

bors · 2025-03-06T15:10:46Z

📌 Commit 701dc0f has been approved by wesleywiser,jieyouxu

It is now in the queue for this repository.

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