Add infrastructure to "compute the ABI of a Rust type, described as a C type"

[RalfJung]

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" fn on sparc64 targets does not respect repr(transparent) rust#115336
• extern "C" fn on mips64 targets does not respect repr(transparent) rust#115404
• riscv64gc-unknown-linux-gnu extern "C" ABI violates repr(transparent) on unions rust#115481
• loongarch64-unknown-linux-gnu extern "C" ABI violates repr(transparent) on unions rust#115509
• The sparc64, riscv64, loongarch64 extern "C" fn ABIs are all wrong when aligned/packed structs are involved rust#115609
• extern "C" fn ABI on aarch64 violates repr(transparent) rust#115664

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:

• File an issue describing the proposal.
• A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
  • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
  • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
• Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

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.

[rustbot]

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.

cc @rust-lang/compiler @rust-lang/compiler-contributors

[RalfJung]

I think the entire ABI/PassMode system and in particular CastTy will actually need a larger refactoring, otherwise I don't see how we can fix issues such as rust-lang/rust#115609.

[oli-obk]

@rustbot second

[RalfJung]

Which of the various variants we discussed does this second? In particular we discussed that (a variant of) CAbiType above might entirely replace the current Abi type.

Or do we leave that to experimentation during the implementation?

[oli-obk]

Incremental progress towards (and including) replacing Abi. As long as a compiler team member is alright with all the steps, this MCP is the overarching plan, but may be halted before actually replacing Abi if new insights during the implementation speak against it

[RalfJung]

@eddyb mentions that another shape this could take is to have a method on TyAndLayout that produces the "list of ABI fields" of this type -- i.e., the list of fields that this type has for the purpose of call ABI (which is separate from the list of declared fields, found in FieldsShape, due to things like newtypes disappearing in the ABI).

ABI adjustments could then even be given a wrapper around TyAndLayout that doesn't give access to anything but such ABI-stable information.

[RalfJung]

After more discussion with @eddyb we came to no final conclusion but exchanged some promising ideas:

• The layout.abi field is poorly named. It wasn't even intended to be about the function call ABI, it was intended to be about how such values are treated by codegen backends internally (as SSA values vs in-memory) -- entirely an optimization. Proposed new name: storage: Storage, or StorageKind or StorageClass; explicitly document that it is merely an optimization; maybe rename the Aggregate variant to Memory. Maybe have a Zst variant for "no storage needed"? Maybe that could be the same variant as Uninhabited? (though is_uninhabited has to still work, but I don't see why that would be a "storage"/"ABI" property)
• Decouple ABI computation from storage. In particular, make one-field #[repr(C)] use Storage::Scalar! Instead ABI is provided via two functions (or an equivalent API): get_abi_kind which can return Scalar, Vector, Aggregate (maybe Aggregate { kind: Array | Struct | PackedStruct | Union }?); get_abi_fields which only works on Aggregates and returns the fields of this struct as far as ABI is concerned (different from FieldsShape since it will skip newtypes and maybe some ZST).

[RalfJung]

When we are refactoring the ABI adjustments anyway, it might make sense to make the entire thing properly declarative: currently we are setting a "default" PassMode for all types initially, and then we have various "adjustment" passes that patch things up. That's very fragile (causing issues like rust-lang/rust#115666 and rust-lang/rust#118124) and it becomes hard to tell what the effective ABI of a given function even is. It is very easy to just forget to adjust some types, leading to code that seemingly works but has the wrong ABI. On wasm this led to us now having a de-facto ABI that nobody ever wanted but that is hard to change. It'd be better if the ABI "adjustments" had to just declare explicitly for each type how it should be passed.

[apiraino]

@rustbot label -final-comment-period +major-change-accepted

[RalfJung]

Opened an issue to track this: rust-lang/rust#119183