codegen: Fix va_list - aarch64 iOS/Windows #56599
Merged
Conversation
This file contains 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
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
dlrobertson
force-pushed
the
fix_va_arg
branch
from
028d2ac
to
71eba86
Compare
dlrobertson
changed the title
dlrobertson
commented
Dec 7, 2018
src/librustc_codegen_llvm/va_arg.rs
Outdated
| bx.cx.tcx.sess.target.target.options.is_like_windows) { | ||
| ("x86", true) => { | ||
| bx.cx.tcx.sess.target.target.options.is_like_windows, | ||
| bx.cx.tcx.sess.target.target.options.is_like_osx) { |
There was a problem hiding this comment.
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.
I think you want to check target_os directly.
dlrobertson
force-pushed
the
fix_va_arg
branch
from
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.
dlrobertson
force-pushed
the
fix_va_arg
branch
from
d06b226
to
3dfd8f7
Compare
|
@bors r+ |
|
📌 Commit 3dfd8f7 has been approved by |
bors
added
S-waiting-on-bors
Centril
added a commit
to Centril/rust
that referenced
this pull request
Dec 8, 2018
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
bors
added a commit
that referenced
this pull request
Dec 8, 2018
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
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.
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.