Proposal
Platform ABIs are typically defined in terms of C types. When adding support for a platform in Rust, the ABI adjustment code needs to figure out how to map Rust types into such a description. For many ABIs this is fairly simple, but some ABIs do complicated type-directed traversals, and that easily leads to issues:
Literally every single ABI that we have implemented and that does something non-trivial got it wrong! This is clearly a systematic issue. I don't have high confidence that after fixing the above bugs, those targets are "good" -- these ABIs tend to special-case specific arcane conditions and it is really hard to be sure that tests like rust-lang/rust#115372 cover all of them.
So I propose that we systematically attack this issue, by introducing a type for "C ABI descriptions" that roughly looks like this (this is a first draft, this will almost certainly need to be extended/adjusted):
/// The C type that describes the ABI of a Rust type.
enum CAbiType {
Primitive(abi::Primitive),
Struct(Vec<CAbiType>),
PackedStruct(Vec<CAbiType>),
Union(Vec<CAbiType>),
Array { elem: Box<CAbiType>, len: usize },
/// A kind of type that doesn't exist in C such as a zero-sized type, an unsized type,
/// or some enums. This can never exist as the field of a struct/union/array,
/// it is used only as a top-level ABI type.
/// (This is the most likely place for changes to be needed.)
Opaque,
}
We could then have a single shared function that computes the CAbiType for a Rust TyAndLayout, and all targets that need to make subtle adjustments should be ported to use that shared function instead of traversing the type/layout themselves. This shared function would, in particular, know to skip repr(transparent) wrappers.
Alternative
Instead of making this a completely new type that is derived from TyAndLayout, we could also alter the existing Abi type to be able to represent C structs, unions, and arrays. The intent of that Abi type was to capture everything relevant for the ABI, but unfortunately it turns out that the current Abi::Aggregate case just loses too much information.
Mentors or Reviewers
I won't have time to implement this or to serve as the sole mentor, but I'd be happy to co-mentor with someone else.
Process
The main points of the Major Change Process are as follows:
You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
Proposal
Platform ABIs are typically defined in terms of C types. When adding support for a platform in Rust, the ABI adjustment code needs to figure out how to map Rust types into such a description. For many ABIs this is fairly simple, but some ABIs do complicated type-directed traversals, and that easily leads to issues:
extern "C" fnon sparc64 targets does not respect repr(transparent) rust#115336extern "C" fnon mips64 targets does not respect repr(transparent) rust#115404extern "C"ABI violates repr(transparent) on unions rust#115481extern "C"ABI violates repr(transparent) on unions rust#115509extern "C" fnABIs are all wrong when aligned/packed structs are involved rust#115609extern "C" fnABI on aarch64 violates repr(transparent) rust#115664Literally every single ABI that we have implemented and that does something non-trivial got it wrong! This is clearly a systematic issue. I don't have high confidence that after fixing the above bugs, those targets are "good" -- these ABIs tend to special-case specific arcane conditions and it is really hard to be sure that tests like rust-lang/rust#115372 cover all of them.
So I propose that we systematically attack this issue, by introducing a type for "C ABI descriptions" that roughly looks like this (this is a first draft, this will almost certainly need to be extended/adjusted):
We could then have a single shared function that computes the
CAbiTypefor a RustTyAndLayout, and all targets that need to make subtle adjustments should be ported to use that shared function instead of traversing the type/layout themselves. This shared function would, in particular, know to skiprepr(transparent)wrappers.Alternative
Instead of making this a completely new type that is derived from
TyAndLayout, we could also alter the existingAbitype to be able to represent C structs, unions, and arrays. The intent of thatAbitype was to capture everything relevant for the ABI, but unfortunately it turns out that the currentAbi::Aggregatecase just loses too much information.Mentors or Reviewers
I won't have time to implement this or to serve as the sole mentor, but I'd be happy to co-mentor with someone else.
Process
The main points of the Major Change Process are as follows:
@rustbot second.-C flag, then full team check-off is required.@rfcbot fcp mergeon either the MCP or the PR.You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.