[win][aarch64] Fix linking statics on Arm64EC, take 2 #142742
+197
−52
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
A-run-make
|
This PR modifies cc @jieyouxu Some changes occurred in compiler/rustc_codegen_ssa |
bjorn3
reviewed
Jun 19, 2025
dpaoliello
force-pushed
the
arm64eclinking
branch
from
8463164 to
fec0e2a
Compare
|
@bors2 try jobs=x86_64-msvc-,x86_64-mingw-,dist-aarch64-msvc |
rust-bors bot
added a commit
that referenced
this pull request
Jun 20, 2025
[win][aarch64] Fix linking statics on Arm64EC, take 2
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 #138541
Resurrects #140176 now that #141061 is merged, which removes the incompatibility with `__rust_no_alloc_shim_is_unstable`.
r? `@wesleywiser`
CC `@bjorn3`
try-job: x86_64-msvc-
|
💔 Test failed
|
bjorn3
reviewed
Jun 20, 2025
| pub trait Copy {} | ||
| impl Copy for i32 {} | ||
| #[lang = "drop_in_place"] | ||
| pub unsafe fn drop_in_place<T: ?Sized>(_: *mut T) {} |
There was a problem hiding this comment.
Can this test use mini_core? Or does that not work for run-make tests?
|
@bors2 try jobs= Edit: Seems to be rust-lang/bors#314 |
|
Unknown value for argument "jobs". |
|
You'll need to use the try-job-in-PR-description form |
|
@bors2 try |
rust-bors bot
added a commit
that referenced
this pull request
Jun 20, 2025
[win][aarch64] Fix linking statics on Arm64EC, take 2
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 #138541
Resurrects #140176 now that #141061 is merged, which removes the incompatibility with `__rust_no_alloc_shim_is_unstable`.
r? `@wesleywiser`
CC `@bjorn3`
--
try-job: x86_64-msvc-*
try-job: x86_64-mingw-*
try-job: dist-aarch64-msvc
bjorn3
reviewed
Jun 20, 2025
| @@ -755,7 +758,7 @@ pub(crate) fn extend_exported_symbols<'tcx>( | |||
| let undecorated = symbol_name_for_instance_in_crate(tcx, symbol, instantiating_crate); | |||
|
|
|||
| // Add the symbol for the kernel descriptor (with .kd suffix) | |||
| symbols.push(format!("{undecorated}.kd")); | |||
| symbols.push((format!("{undecorated}.kd"), info.kind)); | |||
There was a problem hiding this comment.
This should be SymbolExportKind::Data. According to https://llvm.org/docs/AMDGPUUsage.html#symbols, this symbol will be STT_OBJECT rather than STT_FUNC. It is some (non-executable) metadata for the function.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Arm64EC builds recently started to fail due to the linker not finding a symbol:
It turns out that
EMPTY_PANICis 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 #138541
Resurrects #140176 now that #141061 is merged, which removes the incompatibility with
__rust_no_alloc_shim_is_unstable.r? @wesleywiser
CC @bjorn3