-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix linking statics on Arm64EC #140176
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
base: master
Are you sure you want to change the base?
Fix linking statics on Arm64EC #140176
Conversation
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
Some changes occurred in compiler/rustc_codegen_ssa |
#
#
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me. Is it possible to write a regression test? (Either a full-scale integration test that links an exported static from one crate to another or a test that imports a static and dumps the symbol name or something?)
r? @wesleywiser |
@rustbot author |
Reminder, once the PR becomes ready for a review, use |
This PR modifies cc @jieyouxu |
Good call, found another bug: we need to correctly export variables as @rustbot ready |
#
This comment has been minimized.
This comment has been minimized.
Thanks @dpaoliello! @bors r+ rollup Also nominating for potential beta backport as this fixes a serious issue for the Tier 2 |
Fix linking statics on Arm64EC Arm64EC builds recently started to fail due to the linker not finding a symbol: ``` symbols.o : error LNK2001: unresolved external symbol #_ZN3std9panicking11EMPTY_PANIC17hc8d2b903527827f1E (EC Symbol) C:\Code\hello-world\target\arm64ec-pc-windows-msvc\debug\deps\hello_world.exe : fatal error LNK1120: 1 unresolved externals ``` It turns out that `EMPTY_PANIC` is a new static variable that was being exported then imported from the standard library, but when exporting LLVM didn't prepend the name with `#` (as only functions are prefixed with this character), whereas Rust was prefixing with `#` when attempting to import it. The fix is to have Rust not prefix statics with `#` when importing. Adding tests discovered another issue: we need to correctly mark static exported from dylibs with `DATA`, otherwise MSVC's linker assumes they are functions and complains that there is no exit thunk for them. Fixes rust-lang#138541
…enton Rollup of 9 pull requests Successful merges: - rust-lang#139192 (mention provenance in the pointer::wrapping_offset docs) - rust-lang#140176 (Fix linking statics on Arm64EC) - rust-lang#140404 (rm `TypeVistable` impls for `Canonical`) - rust-lang#140437 (enable msa feature for mips in codegen tests) - rust-lang#140438 (Add `rust.debug-assertions-tools` option) - rust-lang#140446 (chore: fix some tests) - rust-lang#140470 (CI: rfl: move job forward to Linux v6.15-rc4) - rust-lang#140476 (chore: delete unused ui/auxiliary crates) - rust-lang#140481 (Require sanitizers be enabled for asan_odr_windows.rs) r? `@ghost` `@rustbot` modify labels: rollup
Possibly failed in rollup? #140495 (comment) I'll kick off some try jobs to try to check whether this PR was the cause. @bors r- |
@bors try |
Fix linking statics on Arm64EC Arm64EC builds recently started to fail due to the linker not finding a symbol: ``` symbols.o : error LNK2001: unresolved external symbol #_ZN3std9panicking11EMPTY_PANIC17hc8d2b903527827f1E (EC Symbol) C:\Code\hello-world\target\arm64ec-pc-windows-msvc\debug\deps\hello_world.exe : fatal error LNK1120: 1 unresolved externals ``` It turns out that `EMPTY_PANIC` is a new static variable that was being exported then imported from the standard library, but when exporting LLVM didn't prepend the name with `#` (as only functions are prefixed with this character), whereas Rust was prefixing with `#` when attempting to import it. The fix is to have Rust not prefix statics with `#` when importing. Adding tests discovered another issue: we need to correctly mark static exported from dylibs with `DATA`, otherwise MSVC's linker assumes they are functions and complains that there is no exit thunk for them. Fixes rust-lang#138541 --- try-job: dist-aarch64-msvc try-job: dist-x86_64-msvc try-job: x86_64-msvc-1 try-job: x86_64-msvc-2
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
@bors rollup=iffy (linkage gaming) |
I can repro this locally, will investigate. @rustbot author |
☔ The latest upstream changes (presumably #140520) made this pull request unmergeable. Please resolve the merge conflicts. |
This was discussed in today's compiler triage meeting. The team will revisit the backport decision once the PR merges. |
@bors try |
Fix linking statics on Arm64EC Arm64EC builds recently started to fail due to the linker not finding a symbol: ``` symbols.o : error LNK2001: unresolved external symbol #_ZN3std9panicking11EMPTY_PANIC17hc8d2b903527827f1E (EC Symbol) C:\Code\hello-world\target\arm64ec-pc-windows-msvc\debug\deps\hello_world.exe : fatal error LNK1120: 1 unresolved externals ``` It turns out that `EMPTY_PANIC` is a new static variable that was being exported then imported from the standard library, but when exporting LLVM didn't prepend the name with `#` (as only functions are prefixed with this character), whereas Rust was prefixing with `#` when attempting to import it. The fix is to have Rust not prefix statics with `#` when importing. Adding tests discovered another issue: we need to correctly mark static exported from dylibs with `DATA`, otherwise MSVC's linker assumes they are functions and complains that there is no exit thunk for them. CI found another bug: we only apply `DllImport` to non-local statics that aren't foreign items (i.e., in an `extern` block), that is we want to use `DllImport` for statics coming from other Rust crates. However, `__rust_no_alloc_shim_is_unstable` is a static generated by the Rust compiler if required, but downstream crates consider it a foreign item since it is declared in an `extern "Rust"` block, thus they do not apply `DllImport` to it and so fails to link if it is exported by the previous crate as `DATA`. The fix is to apply `DllImport` to foreign items that are marked with the `rustc_std_internal_symbol` attribute (i.e., we assume they aren't actually foreign and will be in some Rust crate). Fixes rust-lang#138541 --- try-job: dist-aarch64-msvc try-job: dist-x86_64-msvc try-job: x86_64-msvc-1 try-job: x86_64-msvc-2
Arm64EC builds recently started to fail due to the linker not finding a symbol:
It turns out that
EMPTY_PANIC
is a new static variable that was being exported then imported from the standard library, but when exporting LLVM didn't prepend the name with#
(as only functions are prefixed with this character), whereas Rust was prefixing with#
when attempting to import it.The fix is to have Rust not prefix statics with
#
when importing.Adding tests discovered another issue: we need to correctly mark static exported from dylibs with
DATA
, otherwise MSVC's linker assumes they are functions and complains that there is no exit thunk for them.CI found another bug: we only apply
DllImport
to non-local statics that aren't foreign items (i.e., in anextern
block), that is we want to useDllImport
for statics coming from other Rust crates. However,__rust_no_alloc_shim_is_unstable
is a static generated by the Rust compiler if required, but downstream crates consider it a foreign item since it is declared in anextern "Rust"
block, thus they do not applyDllImport
to it and so fails to link if it is exported by the previous crate asDATA
. The fix is to applyDllImport
to foreign items that are marked with therustc_std_internal_symbol
attribute (i.e., we assume they aren't actually foreign and will be in some Rust crate).Fixes #138541
try-job: dist-aarch64-msvc
try-job: dist-x86_64-msvc
try-job: x86_64-msvc-1
try-job: x86_64-msvc-2