Skip to content
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

"Any two types with size 0 and alignment 1 are ABI-compatible" vs the Windows ABI #552

Open
RalfJung opened this issue Jan 10, 2025 · 21 comments

Comments

@RalfJung
Copy link
Member

RalfJung commented Jan 10, 2025

So apparently ZST return types are actually returned via a return pointer on windows-gnu targets -- except of course when the return type is (). At the same time we promise "Any two types with size 0 and alignment 1 are ABI-compatible". These two things are in direct contradiction with each other. We have to either add something like "except on platforms with a ridiculous ABI, such as windows, then you are just on your own" to the ABI docs, or decide we do not follow the likely accidental ad-hoc ABI of windows-gnu, or emit a hard error for types that the ABI simply does not support.

Note that the reason for this rule of any two 1-ZST being ABI compatible is that for types like #[repr(transparent)] ZST([u8; 0]; ()), we promise that the type is ABI-compatible with the field that is being transparently wrapped, and that could be either field. So the only way to always deliver on our repr(transparent) ABI-compatibility promise is to make all 1-ZST ABI-compatible.

This came up in rust-lang/rust#135204.
Cc @programmerjake @ChrisDenton @eddyb

@RalfJung RalfJung changed the title "Any two types with size 0 and alignment 1 are ABI-compatible" vs the Windows API "Any two types with size 0 and alignment 1 are ABI-compatible" vs the Windows ABI Jan 10, 2025
@programmerjake
Copy link
Member

programmerjake commented Jan 10, 2025

could we make the function return type syntax meaningful here? so extern "win64" fn() means no sret pointer but extern "win64" fn() -> () means there is a sret pointer since the return type is explicit? or is this too big of a change?

@bjorn3
Copy link
Member

bjorn3 commented Jan 10, 2025

extern "win64" fn() is an alias for extern "win64" fn() -> () by definition:

https://doc.rust-lang.org/stable/reference/items/functions.html#r-items.fn.implicit-return.

If the output type is not explicitly stated, it is the unit type.

Also extern "win64" fn foo() -> () is the literal translation of void foo(void) on windows. extern "win64" fn foo() is just a convenience shorthand.

@programmerjake
Copy link
Member

extern "win64" fn() is an alias for extern "win64" fn() -> () by definition:

yeah, I was proposing thinking about changing that...

another probably better option would be to have #[repr(transparent)] always pick a particular field to duplicate the ABI of, e.g. it could pick the generic field, or if there isn't one it'd pick an annotated field or the first field, and a future edition could require ambiguous #[repr(transparent)] structs to annotate one field.
that way you'd have the following ABI lowering:

#[repr(C)]
struct Z([u8; 0]);

#[repr(transparent)]
struct S1 {
    a: Z,
    b: (),
}

// define dso_local void @f(ptr sret(%struct.Z) %0) local_unnamed_addr #0 {
extern "win64" f() -> S1 {
    S1 { a: Z([]), b: () }
}

#[repr(transparent)]
struct S2 {
    a: (),
    b: Z,
}

// define dso_local void @g() local_unnamed_addr #0 {
extern "win64" g() -> S2 {
    S2 { a: (), b: Z([]) }
}

@RalfJung
Copy link
Member Author

RalfJung commented Jan 10, 2025

I don't think the quirks of this... special... ABI should affect what we guarantee for other, more reasonable, ABIs.

@Diggsey
Copy link

Diggsey commented Jan 10, 2025

My understanding is that ZSTs are a non-standard extension to C by clang/gcc, hence why they don't exist under MSVC.

In other words, we don't necessarily have to follow the same ABI - our ABI would be "just as correct" for windows. The question is whether that would cause too many issues for interop with C.

We could have an ABI-modifying attribute to apply at a function level which makes the return value be returned via pointer, to handle that edgecase.

@RalfJung
Copy link
Member Author

@rust-lang/lang is there any appetite for a post-monomorphization lint or error rejecting argument types that are not supported by a particular ABI? The context here is passing ZST across extern "C" on Windows targets; MSVC does not have such types so there is no official ABI and the de-facto ABI implemented by gcc and clang is... odd.

This could be seen as similar to the lint that we already have rejecting SIMD vectors passed via extern "C" without the required target features.

We could have an ABI-modifying attribute to apply at a function level which makes the return value be returned via pointer, to handle that edgecase.

This would have to be a separate extern "ABI" string, since otherwise function pointers are unsound.

@bjorn3
Copy link
Member

bjorn3 commented Jan 10, 2025

is there any appetite for a post-monomorphization lint or error rejecting argument types that are not supported by a particular ABI?

I would love to have that. And also to have it reject any #[repr(Rust)] type for which we don't guarantee a specific ABI.

@RalfJung
Copy link
Member Author

RalfJung commented Jan 10, 2025

And also to have it reject any #[repr(Rust)] type for which we don't guarantee a specific ABI.

In the interest of minimizing breaking changes, I'd like to make a difference between "we don't guarantee this type's ABI" and "this type just can't be passed via this ABI".

Please don't expand the scope of this issue further than is absolutely necessary.

@chorman0773
Copy link
Contributor

I think the answer ought to be just "All return 1-ZSTs match c void" (not the same as c_void), and we just don't match C ZSTs in return position on windows-gnu.

@chorman0773
Copy link
Contributor

Although, now that I think about it, we should check other ABIs.
Something tells me, aggregate ZSTs might be returned in memory on some other platforms (ARM32 maybe)?

@Diggsey
Copy link

Diggsey commented Jan 11, 2025

I think there's a more general problem here where Rust is making guarantees that are not strictly within its remit.

A platform's C ABI can be arbitrarily "odd", and yet we would still want to be able to interoperate with it via extern "C" functions.

Can we relax the guarantees we're making in cases where it's outside our remit. For example, #[repr(transparent)] would still guarantee that the representation of the outer vs inner types in memory and in extern "Rust" functions is identical, but would only be "best effort" as far as how the type is passed to or returned from an extern "C" function.

At monomorphisation time we should check that an extern "C" function signature unambiguously corresponds to a valid C function definition for the target architecture.

We would return an error or warning if either of the following:

  • There are multiple "equally correct" translations to C which could be valid, with different ABIs. This would occur on windows-gnu for a function that tries to return the following type:

    #[repr(transparent)]
    struct CantReturn {
        a: (),
        b: [i32; 0]
    }
  • There is no "correct" translation to C. This would occur on windows-msvc for the following type:

    #[repr(C)]
    struct CantReturn {};

    ie. we should avoid "extending" the ABI, since it's not within our remit to do so and could cause backwards compatibility problems in future.

(These are just examples, but there might be other cases)

@chorman0773
Copy link
Contributor

BTW, if 1-ZSTs don't all have the same ABI, what's the ABI of the following?

#[repr(transparent)]
struct Transparent6;

@Lokathor
Copy link
Contributor

If there isn't a single correct answer we should be at least warning about this during compilation, if not deny by default.

@zachs18
Copy link

zachs18 commented Jan 12, 2025

BTW, if 1-ZSTs don't all have the same ABI, what's the ABI of the following?

#[repr(transparent)]
struct Transparent6;

I would expect Transparent61 would be ABI-compatible with () (unit), since the Reference currently states that, for #[repr(transparent)]

Structs and enums with this representation have the same layout and ABI as the only non-size 0 non-alignment 1 field, if present, or unit otherwise.

(source, emphasis mine). However, this does give the wrinkle that, by a strict reading, if there are 1-ZSTs that are not ABI-compatible with () (say Other1Zst), then #[repr(transparent)] struct Bar(Other1Zst); is ABI-compatible with () and not with Other1Zst.2

The meaning for #[repr(transparent)] on a struct without a non-1-ZST field was T-lang FCP'd (here), but IIUC under the assumption that all 1-ZSTs are ABI-compatible: "[...] The real question here is whether there exists some "special" 1-ZST that would be treated differently by the ABI than any other 1-ZST. I do not believe that is the case, which is why () is kind of "as good as any other". [...]" (src).

Footnotes

  1. and any #[repr(transparent)] struct with no non-1-ZST field

  2. Would #[repr(transparent)] struct _Bar<T>(T); type Bar = _Bar<Other1Zst>; be ABI-compatible with Other1Zst or () in that case? I assume Other1Zst since AFAICT we treat generic types "as if" they are non-1-ZST, but that is a fragile assumption.

@chorman0773
Copy link
Contributor

chorman0773 commented Jan 12, 2025

Would #[repr(transparent)] struct _Bar(T); type Bar = _Bar; be ABI-compatible with Other1Zst or () in that case? I assume Other1Zst since AFAICT we treat generic types "as if" they are non-1-ZST, but that is a fragile assumption

That applies pre-mono when type-checking #[repr(transparent)] - however, I'd assume that layout and ABI of _Bar would be computed post-mono, like it is for all other types - thus it would be compatible with ().

@ChrisDenton
Copy link
Member

On the one hand, using our "C" ABIs for things that don't have C equivalents on the platform is fundamentally nonsensical. On the other hand, our C ABIs unfortunately do double duty as both C ABIs and as a (limited) stable Rust ABI.

I would be in favour of starting by warning when a ZST is passed or returned (except for returning ()). In the absence of crater, this may help gauge the impact of changes here. I would want to be cautious about breaking people's code even if we did make mistakes when implementing certain ABIs.

If we were starting from scratch then I'd think either we should deny ZSTs or else we should try for cross-platform consistency as much possible. But we're not so the potential impact of changing the status quo has to be considered. Maybe if we had some form of stable Rust ABI separate from C ABIs then this would be less of an issue.

@RalfJung
Copy link
Member Author

RalfJung commented Jan 12, 2025

Can we relax the guarantees we're making in cases where it's outside our remit. For example, #[repr(transparent)] would still guarantee that the representation of the outer vs inner types in memory and in extern "Rust" functions is identical, but would only be "best effort" as far as how the type is passed to or returned from an extern "C" function.

Note that repr(C) and repr(transparent) are mutually exclusive. So I think we can always guarantee that repr(transparent) behaves like its one non-1-ZST field, simply by giving it the ABI of that field (i.e., pretending that the newtype wrapper simply wasn't there for the purpose of argument passing). I see no way this can be broken even by the weirdest C ABIs.

The issue here arises because if all fields are 1-ZST (including in the case where there is no field), it is ambiguous which field to pick.

I would be in favour of starting by warning when a ZST is passed or returned (except for returning ()). In the absence of crater, this may help gauge the impact of changes here. I would want to be cautious about breaking people's code even if we did make mistakes when implementing certain ABIs.

We do have the "improper FFI types" lint but I don't know whether or when it fires for ZST, and it can't help when generics are involved.

@CAD97
Copy link

CAD97 commented Jan 12, 2025

I've long been of the position that repr(C)/extern "C"'s "compatible with C" guarantee only extends to standard C (as implemented by the platform C compiler with all of its strict conformance flags turned on), and any extensions to standard C (e.g. data carrying enums) are defined by Rust to best-effort match common C idioms. Similarly, extern "system" provides the same guarantee, but for the system API (and thus any extensions to C that it may utilize).

My current leaning would be a complete rework of the FFI safety lint(s) into ABI safety which denies-by-default any extern "C" declaration or definition which does not have an obviously compatible signature in standard C1 (post mono, but may require incomplete types on either side). This forbids any function that returns a ZST which is not () (or an alias thereof), as standard C has no way of declaring a compatible function signature.

However, I know that ABI is cruel, we at least de facto guarantee that extern "system" is extern "C" on all non-Windows targets2, and there're surely further compatibilities that I'm forgetting that are effectively required to exist.

We do have the "improper FFI types" lint but I don't know whether or when it fires for ZST

warn(improper_ctypes) is known to simultaneously be too weak and too strong. However, it does fire for #[repr(transparent)] to (), both for unit structs and newtypes of () (that doesn't require seeing through generics). This makes no distinction for how the type is used in the FFI signature (e.g. as parameter, return value, or behind a reference).

Notably, the lint does not fire for #[repr(C)] struct CUnit(()) but does fire for #[repr(transparent)] struct TransUnit(CUnit). This means that the types under discussion should all fire the lint currently (if they're subject to being linted at all).

Footnotes

  1. One exception must be made for external linkage: the type tag (name) is not required to match (all struct/union/enum are anonymous in the C tag namespace when crossing FFI).

  2. This was a minor mistake, imo; it should've been “all unix-y targets” or at least “all current non-Windows” written in the reference.

@ChrisDenton
Copy link
Member

I've long been of the position that repr(C)/extern "C"'s "compatible with C" guarantee only extends to standard C (as implemented by the platform C compiler with all of its strict conformance flags turned on), and any extensions to standard C (e.g. data carrying enums) are defined by Rust to best-effort match common C idioms.

I think if that is the philosophy than ideally ZSTs should not have any effect on the C ABI if the ABI does not support them. I.e. returning a ZST is the same as returning void and passing in a ZST as an argument results in the argument being ignored (for the sake of ABI). This would avoid inventing an ABI extension while not preventing Rust (and other ZST supporting langs) from magicking up ZSTs when needed.

@nikomatsakis
Copy link
Contributor

So there are a lot of comments on this issue. I can't speak to the larger questions about refactoring everything but I definitely feel that post-mono lints or warnings for things that are not supported by the target ABI sounds like it would help prevent some hair-pulling moments.

@RalfJung
Copy link
Member Author

RalfJung commented Jan 14, 2025

For SIMD types we made this a future-compat lint and to my knowledge the plan is still to eventually make this a hard error -- do you think that is not the right approach? Arguably there we have to do it as otherwise it's just unsound. For the Windows ZST case, pure Rust code is fine either way, so it's not strictly speaking a soundness question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants