-
Notifications
You must be signed in to change notification settings - Fork 14k
codegen: Fix va_list - aarch64 iOS/Windows #56599
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
Conversation
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
028d2ac to
71eba86
Compare
src/librustc_codegen_llvm/va_arg.rs
Outdated
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.
This is correct by circumstance, but is not ideal. Technically if there was a aarch64-apple-darwin spec (there isn't one currently) it would match here, but not in libcore. So we'd end up generating a generic Aarch64 ABI VaList structure in libcore and then use the char * variant of the va_arg instruction here 😳. Is there a better way to match against IOS here other than using the target_os string or is_like_osx?
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.
CC @rust-lang/compiler ^
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.
I think you want to check target_os directly.
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.
👍 updated
71eba86 to
d06b226
Compare
According to the Apple developer docs: > The type va_list is an alias for char * rather than for the struct > type specified in the generic PCS. The current implementation uses the generic Aarch64 structure for VaList for Aarch64 iOS. Windows always uses the char * variant of the va_list.
d06b226 to
3dfd8f7
Compare
|
@bors r+ |
|
📌 Commit 3dfd8f7 has been approved by |
codegen: Fix va_list - aarch64 iOS/Windows ## Summary Fix code generated for `VaList` on Aarch64 iOS/Windows. ## Details According to the [Apple - ARM64 Function Calling Conventions]: > ... the type va_list is an alias for char * rather than for the struct > type specified in the generic PCS. The current implementation uses the generic Aarch64 structure for `VaList` for Aarch64 iOS. Switch to using the `char *` variant of the `VaList` and use the corresponding `emit_ptr_va_arg` for the `va_arg` intrinsic. Windows always uses the `char *` variant of the `VaList`. Update the `va_arg` intrinsic to use `emit_ptr_va_arg`. [Apple - ARM64 Function Calling Conventions]: https://developer.apple.com/library/archive/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html
Rollup of 6 pull requests Successful merges: - #56248 (Suggest an appropriate token when encountering `pub Ident<'a>`) - #56597 (Improve the usage message for `-Z dump-mir`.) - #56599 (codegen: Fix va_list - aarch64 iOS/Windows) - #56602 (Fix the just-introduced ptr::hash docs) - #56620 (resolve: Reduce some clutter in import ambiguity errors) - #56621 (Add missing comma in Generators) Failed merges: r? @ghost
Summary
Fix code generated for
VaListon Aarch64 iOS/Windows.Details
According to the Apple - ARM64 Function Calling Conventions:
The current implementation uses the generic Aarch64 structure for
VaListfor Aarch64 iOS. Switch to using the
char *variant of theVaListand use the corresponding
emit_ptr_va_argfor theva_argintrinsic.Windows always uses the
char *variant of theVaList. Update theva_argintrinsic to use
emit_ptr_va_arg.