repr(C) on MSVC targets does not always match MSVC type layout when ZST are involved

Question

repr(C) on MSVC targets does not always match MSVC type layout when ZST are involved

mahkoh opened this issue 5 years ago · 120 comments

Also see this summary below.

Consider

#![allow(dead_code)]

use std::mem;

#[no_mangle]
pub fn sizeof_empty_struct_1() -> usize {
    #[repr(C)]
    struct EmptyS1 {
        f: [i64; 0],
    }

    // Expected: 4
    // Actual: 0
    mem::size_of::<EmptyS1>()
}

#[no_mangle]
pub fn sizeof_empty_struct_2() -> usize {
    #[repr(C, align(8))]
    struct X {
        i: i32,
    }

    #[repr(C)]
    struct EmptyS2 {
        x: [X; 0],
    }

    // Expected: 8
    // Actual: 0
    mem::size_of::<EmptyS2>()
}

#[no_mangle]
pub fn sizeof_enum() -> usize {
    #[repr(C)]
    enum E {
        A = 1111111111111111111
    }

    // Expected: 4
    // Actual: 8
    mem::size_of::<E>()
}

#[no_mangle]
pub fn sizeof_empty_union_1() -> usize {
    #[repr(C)]
    union EmptyU1 {
        f: [i8; 0],
    }

    // Expected: 1
    // Actual: 0
    mem::size_of::<EmptyU1>()
}

#[no_mangle]
pub fn sizeof_empty_union_2() -> usize {
    #[repr(C)]
    union EmptyU2 {
        f: [i64; 0],
    }

    // Expected: 8
    // Actual: 0
    mem::size_of::<EmptyU2>()
}

and the corresponding MSVC output: https://godbolt.org/z/csv4qc

The behavior of MSVC is described here as far as it is known to me: https://github.com/mahkoh/repr-c/blob/a04e931b67eed500aea672587492bd7335ea549d/repc/impl/src/builder/msvc.rs#L215-L236

Answer 1 · 2021-02-11T16:08:54.000Z

@mahkoh can you reproduce this without no_mangle? no_mangle should really require unsafe, it can cause unsoundness if it overlaps with another linker symbol.

Answer 2 · 2021-02-11T16:10:00.000Z

@jyn514 Yes

Answer 3 · 2021-02-11T16:13:35.000Z

repr(C) has served two purposes:

A representation with a know layout algorithm which is the same across all targets
A representation that is compatible with C types

The first purpose is advertised both in the reference and in stdlib code e.g. in Layout. It is probably used in many other places.

The second purpose is also advertised in the reference.

However, these purposes are not compatible as shown above.

Answer 4 · 2021-02-11T16:29:24.000Z

The layout algorithm is not the same across all targets, it is always supposed to be whatever the C ABI mandates on that particular target

Answer 5 · 2021-02-11T16:29:45.000Z

Does this reproduce with clang?

Answer 6 · 2021-02-11T16:40:43.000Z

The layout algorithm is not the same across all targets, it is always supposed to be whatever the C ABI mandates on that particular target

The layout algorithms used by the C compilers are not the same. But repr(C) is advertised with a specific layout algorithm that is the same across all targets. Namely in these places

Does this reproduce with clang?

Clang contains many bugs in their MSVC-compatible layout algorithm. It should not be used as a reference:

The output of clang is incompatible with both MSVC and rustc: https://github.com/llvm/llvm-project/blob/661f9e2a92302b1c7140528423fdbfc133a68b41/clang/lib/AST/RecordLayoutBuilder.cpp#L3076-L3087

Answer 7 · 2021-02-12T09:17:24.000Z

Let's make sure the windows notification group is aware of this:

@rustbot ping windows

Answer 8 · 2021-02-12T09:17:26.000Z

Hey Windows Group! This bug has been identified as a good "Windows candidate".
In case it's useful, here are some instructions for tackling these sorts of
bugs. Maybe take a look?
Thanks! <3

cc @arlosi @danielframpton @gdr-at-ms @kennykerr @luqmana @lzybkr @nico-abram @retep998 @rylev @sivadeilra

Answer 9 · 2021-02-12T09:21:18.000Z

Assigning P-high as discussed as part of the Prioritization Working Group procedure and removing I-prioritize.

Answer 10 · 2021-02-12T13:56:34.000Z

So to summarize:

repr(C) is inaccurate in the presence of Zero-sized Types.
A repr(C) enum with a tag value in excess of the normal tag maximum will (inaccurately) use a larger tag rather than error.

Answer 11 · 2021-02-12T14:16:56.000Z

One more thing

#[repr(C, packed(1))]
struct X {
    c: u8,
    m: std::arch::x86_64::__m128,
}

#[no_mangle]
pub fn f() -> usize {
    // Expected 32
    // Actual 17
    std::mem::size_of::<X>()
}

Answer 12 · 2021-02-12T14:17:33.000Z

That is, if we assume repr(packed(1)) to have the same intended effect as #pragma pack(1).

Answer 13 · 2021-02-12T14:18:48.000Z

Yikes, I think a lot of folks do assume that, yeah.

Answer 14 · 2021-02-12T14:24:48.000Z

Basically all types that have a __declspec(align) annotation (such as __m128) in the MSVC stdlib but no such annotation in Rust are broken because MSVC implements the concept of required alignments which are unaffected by #pragma pack annotations.

This particular problem could be fixed by adding such an annotation to __m128 on MSVC targets. The definition of __m128 is broken anyway because it has to be 16 byte aligned on MSVC targets but is defined as 4 byte aligned in the stdlib.

So in the end this is not an inherent problem of the layout algorithm because you're not supposed to be able to write this anyway.

Answer 15 · 2021-02-12T17:08:48.000Z

C/C++ do not permit zero-sized structs or arrays. Aggregates (structures and classes) that have no members still have a non-zero size. MSVC, Clang, and GCC all have different extensions to control the behavior of zero-sized arrays.

It is unfortunate that #[repr(C)] means two things: C ABI compatible, and sequential layout. Maybe a new #[repr(stable)] could be added, which would request sequential layout but would not require interop with standard C ABI.

In the near term, perhaps the best thing is to add a new diagnostic, which is "you asked for #[repr(C)], but you have ZSTs in here, and that might be a problem."

Answer 16 · 2021-02-12T17:58:41.000Z

C/C++ do not permit zero-sized structs or arrays. Aggregates (structures and classes) that have no members still have a non-zero size.

GCC and Clang do in fact accept even completely empty structs and unions and such types have size 0 when compiled with these compilers. (Except that Clang tries to emulate MSVC when compiling for *-msvc targets.) The current implementation of repr(C) seems to be correct in all cases accepted by rustc except on msvc targets.

Maybe a new #[repr(stable)] could be added, which would request sequential layout but would not require interop with standard C ABI.

That seems to be the most pragmatic solution. Alternatively one could deprecate repr(C) completely and replace it by repr(NativeC) and repr(stable).

In the near term, perhaps the best thing is to add a new diagnostic, which is "you asked for #[repr(C)], but you have ZSTs in here, and that might be a problem."

Maybe only warn on msvc targets.

Answer 17 · 2021-02-12T18:27:16.000Z

GCC and Clang do in fact accept even completely empty structs and unions and such types have size 0 when compiled with these compilers.

In C++ they have size 1:

$ clang -std=c++20 -xc++ - -o /dev/null -c <<<'extern "C" { struct EmptyStruct {}; union EmptyUnion {}; }'
<stdin>:1:14: warning: empty struct has size 0 in C, size 1 in C++ [-Wextern-c-compat]
extern "C" { struct EmptyStruct {}; union EmptyUnion {}; }
             ^
<stdin>:1:37: warning: empty union has size 0 in C, size 1 in C++ [-Wextern-c-compat]
extern "C" { struct EmptyStruct {}; union EmptyUnion {}; }
                                    ^
2 warnings generated.

Actually the nomicon says:

ZSTs are still zero-sized, even though this is not a standard behavior in C, and is explicitly contrary to the behavior of an empty type in C++, which says they should still consume a byte of space.

So it seems that this (unsound) behavior is even documented.

Answer 18 · 2021-02-12T18:34:09.000Z

In C++ they have size 1:

repr(C) is about C compatibility not about C++ compatibility.

So it seems that this (unsound) behavior is even documented.

Otherwise empty structs in msvc have size at least 4 bytes not 1 byte in C mode.

Answer 19 · 2021-02-13T19:50:27.000Z

GCC and Clang do in fact accept even completely empty structs and unions and such types have size 0 when compiled with these compilers.

These are non-standard extensions, and they deviate from the C/C++ specification.

If #[repr(C)] means "has a representation that is equivalent to that generated by a conformant C compiler", then Rust's current behavior is fine. If #[repr(C)] means #[repr(C_with_msvc_and_clang_extensions)], then that is something different.

I understand the desire for compatibility with the de-facto standard behavior of these compilers, from a practical point of view. At the same time, these are areas where they do deviate from the language standard. Figuring out the best solution for Rust will require some careful consideration, and the short-term solution of "make it work like Clang / MSVC / GCC" should not be chosen without due consideration.

Answer 20 · 2021-02-13T20:16:18.000Z

If #[repr(C)] means "has a representation that is equivalent to that generated by a conformant C compiler",

That is obviously not what it means. First of all, there is no such thing as the representation of a type when compiled with a conformant C compiler because the C specification does not specify the representation of types. Two conformant C compilers on the same OS can have different type layouts. Therefore, even if we only want to talk about the representation of types that have an equivalent in standard's compliant C, we have to first decide which ABI we are targeting. And this ABI is most of the time dependent on the compiler even on the same OS.

Second of all, Rust has for a long time supported non-standard extensions such as repr(packed).

Figuring out the best solution for Rust will require some careful consideration, and the short-term solution of "make it work like Clang / MSVC / GCC" should not be chosen without due consideration.

I hope you are not implying that I suggested that repr(C) should work like any one of these compilers. I said it should work like the native compiler on the target the Rust program is compiled for. Please explain why this would be controversial. This is also the approach taken by the Clang developers.

Answer 21 · 2021-02-13T20:39:06.000Z

... there is no such thing as the the representation of a type when compiled with a conformant C compiler

The gray area I was pointing out was, when you ask a C compiler to compile non-conformant code, there is no possible well-defined behavior that corresponds to the specification.

Please explain why this would be controversial.

Because that line of reasoning boils down to "whatever a specific set of C implementations do". Specification-by-implementation is always a slippery slope. When you go down that path, you often wind up in a place where different implementations have conflicting behavior, and now you have no clear way to specify your own behavior.

If we look at the specific question of "what should #[repr(C)] mean with a ZST?", then we have several options:

Reject it as invalid, because this is invalid C.
Issue a warning, saying that this is invalid C. The warning should indicate that Rust will still treat this as a ZST. For people who are using #[repr(C)] as a way to interop with C++ (and there are many), this may save someone a lot of hassle, because the C++ side will compute sizeof(T) == 1, not zero.
Define a Rust attribute that specifies the size, e.g. #[repr(C, size = 0)]. This may seem redundant, but it would inform the compiler that you know that you're asking for behavior that is non-standard, and you're giving it the information on how to interpret the non-standard behavior.
Define a Rust attribute that specifies, within a particular scope (such as a module), that #[repr(C)] means compatibility with a specific C implementation, e.g. #[C_repr_compatibility = "clang")] or some such. I'm intentionally using an ugly, bespoke syntax because I don't care about the syntax. I only care about the semantics -- that where the developer is using behavior that is not standard C, that they must specify (to the Rust compiler) what they need / expect.

I am not criticizing your very reasonable desire to solve this issue. All I'm doing is pointing out that solving this requires more thinking than "do whatever Clang / GCC / MSVC" does. There are many other C implementations (such as those for embedded systems), and I do hope that Rust, as a language, does not specify its own behavior in terms of "do what some other language implementation does".

I think the best solution would be:

When Rust encounters a ZST marked with #[repr(C)], it issues a warning. This is exactly like what Clang, GCC, and MSVC do already when you attempt to compile zero-sized arrays in those languages.
The developer can either suppress this warning using #[allow(nonstandard_C_extension)] or #[repr(C, compatible_with = "clang")] or some such. Tools such as bindgen can then add these annotations, for exact compatibility with the compilers that Clang supports.

Answer 22 · 2021-02-13T21:16:41.000Z

The gray area I was pointing out was, when you ask a C compiler to compile non-conformant code, there is no possible well-defined behavior that corresponds to the specification.

Like I already explained, repr(C) is not about the C specification. It's about the ABI.

Because that line of reasoning boils down to "whatever a specific set of C implementations do". Specification-by-implementation is always a slippery slope.

This has nothing to do with non-standard extensions. The behavior of bit-fields, a standard C construct, differs widely between targets and compilers and is essentially only defined by its implementation.

If we look at the specific question of "what should #[repr(C)] mean with a ZST?", then we have several options:

You've omitted the option I've suggested. Namely to conform to the behavior of the native OS compiler. This is the option taken by Clang with overwhelming success, allowing it to be used as the C compiler on all major platforms with a very high degree of interoperability [1] [2] [3]. Explain why this does not work for Rust and why it would not lead to equal success.

Reject it as invalid, because this is invalid C.

We do not care about valid C. We care about making Rust usable in as many places as possible. In particular place that are currently dominated by C and C++. To achieve this goal we have to be able to easily interact with code written in C and C++. Such code uses non-standard extensions.

Your other suggested options all restrict the usability of Rust in these places.

There are many other C implementations (such as those for embedded systems)

Your argument boils down to "on the same target, there can be multiple incompatible C implementations". But this has nothing at all to do with non-standard extensions such a ZSTs. What you are saying is that we cannot have repr(C) at all because there is no authorative definition of the layout of C types on any platform. This goes contrary to what Herb Sutter said in N4028 where he basically follows my argument by saying

Because the ABI is per “platform,” it is the OS platform owner, not every C++ implementation, who is
responsible for defining what the C++ ABI means on its platform. For example, for the platform “Windows x86 32-bit,” Microsoft’s Windows team (possibly delegating to their native compiler team) would be ultimately responsible for specifying the Windows C++ ABI.

[1] https://docs.microsoft.com/en-us/cpp/build/clang-support-msbuild?view=msvc-160
[2] https://www.phoronix.com/scan.php?page=news_item&px=OpenMandrova-Clang-Euro-LLVM
[3] https://www.kernel.org/doc/html/latest/kbuild/llvm.html

Answer 23 · 2021-02-13T21:24:53.000Z

If we follow your argument and solutions, then it will not be possible to use Rust in the Linux kernel which makes significant use of GCC extensions. As does the userspace API, though to a lesser degree. The MSVC standard library also makes use of extensions:

~/opt/msvc$ rg 'declspec\(align' | wc -l
64
~/opt/msvc$ rg 'pragma pack' | wc -l               
966

So no more winapi for Rust.

Answer 24 · 2021-02-13T21:33:49.000Z

My "argument" is simply that we should help the developer get the behavior they were expecting. Since ZSTs are a non-standard part of C, it is reasonable to ask the developer for more information -- basically, confirming what behavior they want.

You are attributing strawman arguments (such as "let's break Rust in Linux kernel") to me that I have not proposed, which is not helping. I am not proposing anything that prevents using Rust in some particular scenario, such as the Linux kernel. I am trying to work through the problem from first principles, and I don't think we are actually in any sort of fundamental disagreement. I would ask you for a little more patience while we work through these issues; I am not attacking you or trying to propose something absurd. The only major difference seems to be that I am focusing more on the C specification than on the de-facto standards implied by the implementations.

From my experience, it's good to be cautious about designing language features and semantics. Often, you don't get a chance to correct a mistake, so it's worth it to move carefully on things like this and avoid mistakes. If that takes some conversation to converge on a solution, that seems fine to me.

You responded to almost everything that I posted except for my suggestions at the bottom. What do you think of my suggestions, of 1) generating a warning when #[repr(C)] is applied to ZSTs, and 2) providing Rust attribute that specify the behavior that is wanted?

One situation I am worried about, is that C and C++ differ in what sizeof means for a struct that has no fields. For the exact same code, GCC and G++ give different answers. (Clang gives the same answers.) MSVC (in C mode) rejects with a hard error structs that have no fields, while MSVC (in C++ mode) generates a struct of size 1. Since the same compilers give different answers even when targeting the same platforms, I think there is a good reason to flag this same struct definition with a warning in Rust, and require the user to specify what behavior they want.

Answer 25 · 2021-02-13T22:17:56.000Z

My "argument" is simply that we should help the developer get the behavior they were expecting. Since ZSTs are a non-standard part of C, it is reasonable to ask the developer for more information -- basically, confirming what behavior they want.

Why only for non-standard C? The behavior of GCC and MSVC is significantly different even for standard C.

You are attributing strawman arguments (such as "let's break Rust in Linux kernel") to me that I have not proposed, which is not helping.

It is quite distressing that I open this issue to increase the compatibility of Rust with MSVC and you're floating the idea that we could instead solve the problem by decreasing the compatibility of Rust with all other platforms by disallowing certain constructs that are currently broken only on MSVC targets.

I am not proposing anything that prevents using Rust in some particular scenario, such as the Linux kernel.

The options and solutions you've proposed do that.

Reject it as invalid, because this is invalid C.

This obviously would prevent using Rust in the Linux kernel. Not necessarily because of empty structs but because of other constructs that fall in the same category of non-standard C.
The warning should indicate that Rust will still treat this as a ZST.

This makes Rust incompatible with C code compiled with MSVC.
Define a Rust attribute that specifies the size, e.g. #[repr(C, size = 0)].

Ditto.
Define a Rust attribute that specifies, within a particular scope (such as a module), that #[repr(C)] means compatibility with a specific C implementation, e.g. #[C_repr_compatibility = "clang")]

Ditto. Or if we instead write #[C_repr_compatibility = "msvc")], then it will be unsound on non-msvc targets.

Meanwhile you've ignored the option that would create the highest degree of compatibility: Do what the C compiler that compiled the C code that we are trying to interact with does.

You responded to almost everything that I posted except for my suggestions at the bottom. What do you think of my suggestions, of 1) generating a warning when #[repr(C)] is applied to ZSTs, and 2) providing Rust attribute that specify the behavior that is wanted?

I do not believe a warning is necessary. Simply follow the behavior of the native OS compiler.
That would make the Rust code platform dependent. If I have C code that compiles with both MSVC and GCC, then Rust code that also compiles on both platforms must be able to soundly interface with the C code.

One situation I am worried about, is that C and C++ differ in what sizeof means for a struct that has no fields.

There would be no problem with adding repr(C_plus_plus) if only to handle this one case.

Answer 26 · 2021-02-13T23:48:01.000Z

It is quite distressing that I open this issue to increase the compatibility of Rust with MSVC and you're floating the idea that we could instead solve the problem by decreasing the compatibility of Rust with all other platforms by disallowing certain constructs that are currently broken only on MSVC targets.

It is common (when investigating issues) to enumerate options precisely so that we can identify which ones are bad ones, and which ones are good ones. Listing options is not the same thing as endorsing them.

This [using #[repr(C, size = 0)] makes Rust incompatible with C code compiled with MSVC.

It doesn't even matter with MSVC, because MSVC (in C mode) rejects the program entirely, because it is not conformant C code. With this code:

#include <stdio.h>

struct EmptyStruct {
};

int EmptyArray[0];

int main() {
    printf("size of empty struct = %d\n", (int)sizeof(struct EmptyStruct));
    printf("size of empty array = %d\n", (int)sizeof(EmptyArray));
}

I get this:

test.c(4): error C2016: C requires that a struct or union have at least one member
test.c(6): error C2466: cannot allocate an array of constant size 0

You say we should "do what the compiler does for that platform". In this specific case, there is no single behavior from the compilers. MSVC rejects the program entirely. Should Rust also reject the program entirely? Clang and GCC do something different from MSVC, so how do we follow your own suggestion and do what the compilers do?

If you mean that Rust should do what a specific compiler does when targeting a specific platform, then, which one? And how do you select that behavior?

Answer 27 · 2021-02-13T23:56:14.000Z

It doesn't even matter with MSVC, because MSVC (in C mode) rejects the program entirely, because it is not conformant C code. With this code:

For empty structs it does but recall the example from the OP: https://godbolt.org/z/jzrasb

For constructs that are rejected by the C compiler there is no problem because we will not be interfacing with such C code. In this case we can emit a warning and leave the layout unspecified.

If you mean that Rust should do what a specific compiler does when targeting a specific platform, then, which one? And how do you select that behavior?

I've compiled such a list here: https://github.com/mahkoh/rfcs/blob/bitfield/text/3064-bitfield.md#appendix-a

On non-windows platforms there is not much of a problem because the differences between Clang and GCC are mostly bugs in Clang. The only interesting case are the -windows-gnu targets. Here we might want to add an annotation that opts into MSVC layout so that people can interact with MSVC libraries.

Answer 28 · 2021-02-14T18:01:42.000Z

Thank you @sivadeilra and @mahkoh so much for the discussion and for providing analysis and options for a clearer evaluation.
Just chiming in to remind that unsound issues are definitively regarded with high priority and this one will probably need a bit of thought on how to be tackled.

labeling this for T-compiler.

Answer 29 · 2021-02-14T18:03:46.000Z

While I agree that soundness issues are critical, let's also keep in mind that this has been the behavior since before 1.0. I don't think there is a need for any short-term solutions.

Answer 30 · 2021-02-14T18:28:12.000Z

@mahkoh can you explain why you think this is unsound? Rustc certainly differs from MSVC in the layout, but as far as I can tell that can only go wrong if you use unsafe code and assume the layout is the same for both. I don't know if rust guarentees that for ZSTs.

Answer 31 · 2021-02-14T18:31:08.000Z

and assume the layout is the same for both. I don't know if rust guarentees that for ZSTs.

Can you point me to where the guarantee is that repr(C) structs have the same layout as structs in C?

Answer 32 · 2021-02-14T19:05:17.000Z

Minor Note: This is actually a T-lang issue to resolve before it's a T-compiler issue.

T-lang really needs to specify how repr(C) works for all compilers of rust, and then the compiler team can update rustc. But this needs to be an official language decision.

Answer 33 · 2021-02-14T20:04:34.000Z

and assume the layout is the same for both. I don't know if rust guarentees that for ZSTs.

Can you point me to where the guarantee is that repr(C) structs have the same layout as structs in C?

Well right, that's sort of my point - if rust doesn't guarantee that, how can having a different layout be unsound?

Answer 34 · 2021-02-14T20:12:36.000Z

If rust makes no guarantee about the compatibility of repr(C) and C layouts, and you have not linked any such guarantees, then the C in repr(C) is rather misleading and should be renamed to repr(simple) or something like that. If this is what you are arguing then there is no unsoundness here.

Answer 35 · 2021-02-15T09:03:16.000Z

If rust makes no guarantee about the compatibility of repr(C) and C layouts

I think the statement that there's no guarantee applies only to zero-sized types. And in part helps to allow things like:

#[repr(C)]
struct Foo<'a> {
    some: Stuff,
    etc: *const Blah,
    marker: PhantomData<&'a ()>,
}

It's pretty clearly a bug otherwise, though.

Answer 36 · 2021-02-15T09:11:57.000Z

A fixed layout can be selected with the #[repr] attribute … [#[repr(C)] example] … This will force a struct to be laid out like the equivalent definition in C.
— RFC79

The C representation is designed for dual purposes. One purpose is for creating types that are interoperable with the C Language. The second purpose is to create types that you can soundly perform operations on that rely on data layout such as reinterpreting values as a different type.
— Type Layout - The Rust Reference

Both of these only refer to the C language, by my reading of the C spec most of the layout details for a struct are then implementation defined, a compliant compiler could ~~sort members alphabetically before layout~~ use a members name to determine padding around it; so it really does seem like Rust needs to either:

document that repr(C) is only compatible with C compilers using the same algorithm as specified in the reference (which seems to exclude MSVC when zero-length-array extensions are in use)
change the documentation and implementation to take into account the "platform compilers" implementation details

Answer 37 · 2021-02-15T12:14:09.000Z

A fixed layout can be selected with the #[repr] attribute … [#[repr(C)] example] … This will force a struct to be laid out like the equivalent definition in C.
— RFC79

This RFC does not mention the algorithm in the reference. Where does this algorithm come from? Who decided to add this algorithm to the reference? Where is the corresponding RFC?

Answer 38 · 2021-02-15T12:45:34.000Z

Apparently this was added in rust-lang/reference#156 as part of rust-lang/rfcs#2195. But this RFC does not at all talk about the layout of repr(C) structs or unions and the reference PR goes much further than what was discussed in the RFC. So if only the correct procedure had been followed, we would not find ourselves in this situation.

Answer 39 · 2021-02-15T16:42:10.000Z

Mahkoh all the correct procedure has been followed.

The reference doesn't prescribe the operation of rustc it just reflects it. Even if the reference said nothing at all there would be some algorithm being performed within the compiler. The reference is just describing that.

Answer 40 · 2021-02-15T16:56:22.000Z

Then, in your opinion, it would do no harm to add a comment to the Rust reference that

The layout of repr(Rust) structs is the same as the layout of repr(C) structs.
The layout of &dyn Trait is (data_ptr, v_table_ptr)
The layout of &[T] is (data_ptr, size)

After all, that is how rustc behaves and the reference has no normative function.

Answer 41 · 2021-02-15T17:37:12.000Z

Well point 1 would be a very wrong thing to say

2 and 3 is technically a possible thing to say without it being inaccurate, but I think that the Reference maintainers would reject such a PR because there's not really much that a person can do with that knowledge. It would still be UB to, for example, directly transmute a &[i32] to &[f32].

The reference is not now, and likely it will not ever be, normative. It's a documentation of the language that inevitably lags behind the language, sometimes quite a lot behind.

Answer 42 · 2021-02-15T17:51:46.000Z

2 and 3 is technically a possible thing to say without it being inaccurate, but I think that the Reference maintainers would reject such a PR because there's not really much that a person can do with that knowledge.

Of course it would be useful, for example, you could pass &[i32] to a C function accepting an equivalent C type.

Answer 43 · 2021-02-15T20:00:58.000Z

@DianaNites I think the point was technically &[i32] and &[f32] are allowed to have different layouts (i.e swap the positions of the length and the pointer) such that transmuting between them would be unsound (without going through into_raw_parts and from_raw_parts)

Answer 44 · 2021-02-15T20:12:47.000Z

@DianaNites

Invoking undefined behavior via compiler intrinsics.

transmute is an intrinsic, and code can't rely on the layout of a repr rust struct being stable. in practice some layouts are stable, but you can't rely on that. eg: all code from before the field reordering update trying to guess the layout of a rust struct would just be wrong with a modern compiler.

also note, very important:

Warning: The following list is not exhaustive.

any number of things can be UB without being on that list.

@mahkoh the layout of a slice is not actually stable. I think you already know this. the most the reference could say is that it's a specific way for a given version of the compiler. Not that it will always be that way. There's a reason that slice layout is in the (far less worked on) unsafe code guidelines.

Answer 45 · 2021-02-15T20:16:44.000Z

@mahkoh the layout of a slice is not actually stable. the most the reference could say is that it's a specific way for a given version of the compiler.

Since the reference does not say that repr(C) as described in the reference is only valid for the current version of the compiler, then, following your argument, this must mean that repr(C) as described in the reference is stable. Is that what you are saying?

Answer 46 · 2021-02-15T20:24:07.000Z

I've been preparing an RFC to address this. It's a work in progress but feel free to comment. https://gist.github.com/mahkoh/eb95f518a8a4b25237ffd2edd2730c6b

Answer 47 · 2021-02-15T20:52:03.000Z

Yes, repr C is stable. Yes, changing how it works would be a breaking change. Neither of those points are up for debate, that's just the facts. However, breaking changes are allowed without a semver bump when it's related to soundness issues.

What is left to discuss is how we can fix things with as small of a breaking change as possible.

Answer 48 · 2021-02-15T20:54:13.000Z

Yes, repr C is stable.

Where was it decided that the algorithm in the reference is the stable algorithm instead of having the algorithm depend on the platform?

Answer 49 · 2021-02-15T21:01:28.000Z

Again, I would really like to stress this point: the reference isn't the source of truth. The reference is just describing what the compiler does (as best it can). When the reference and the compiler disagree, unless T-compiler says that it's a bug, then the reference is wrong.

Answer 50 · 2021-02-15T21:04:35.000Z

Again, I would really like to stress this point: the reference isn't the source of truth. The reference is just describing what the compiler does (as best it can).

You're contradicting yourself. Previously you said that the layout algorithm of &[T] cannot be added to the reference because it is not stable. Yet here you are implying that it can be added to the reference because the reference merely describes what the compiler does.

Answer 51 · 2021-02-16T17:53:44.000Z

Is there a summary of the core issue here? The OP is basically a long code example, so it's not obvious to me what the problem is, and there are a lot of comments for me to sort through.

Answer 52 · 2021-02-16T17:54:58.000Z

@mahkoh

You're contradicting yourself. Previously you said that the layout algorithm of &[T] cannot be added to the reference because it is not stable. Yet here you are implying that it can be added to the reference because the reference merely describes what the compiler does.

Whether intentional or not, these comments read hostile to me, as do others you have made on this thread. Please try for a collaborative tone, we're all trying to solve the problem (or in my case, even understand what it is).

Also, this particular comment feels personal -- perhaps a structure like "But there is a contradiction here. On the one hand... on the other...", for example, would be better.

Answer 53 · 2021-02-16T17:57:32.000Z

@nikomatsakis TLDR: Currently, on MSVC targets, repr(C) on a struct can, in select situations, generate struct layouts that do not match what the C compiler would also generate. If you were to then actually interface these structs with said C code you'd get layout mismatch UB.

Answer 54 · 2021-02-16T17:58:23.000Z

Is there a summary of the core issue here? The OP is basically a long code example, so it's not obvious to me what the problem is, and there are a lot of comments for me to sort through.

enums in MSVC are always 4 bytes. Enum constants are truncated to 4 bytes
Structs in MSVC whose elements all have size 0 have a size equal to the required alignment of the struct but at least 4 bytes. The required alignment is the alignment requested by #[repr(align)] on the struct or any of its fields, recursively.
Unions in MSVC whose elements all have size 0 have a size equal to its alignment.

Answer 55 · 2021-02-16T18:05:31.000Z

Whether intentional or not, these comments read hostile to me, as do others you have made on this thread. Please try for a collaborative tone, we're all trying to solve the problem (or in my case, even understand what it is).

@nikomatsakis There is no hostility intended. We're in a somewhat heated debate discussing an issue, not each other. Any attacks on each other's ideas are restricted to this debate.

Since you've already chimed in, please settle this question for us so that we can move past it:

Is the rust reference intended to only document stable parts of the rust language and rustc (as long as the corresponding text in the reference is not qualified with "currently" or "might change in the future" or "is not stable".)
If so, where was it decided that the layout algorithm for repr(C) structs currently documented in the reference is stable?
Otherwise, does this mean that the layout algorithm for repr(C) structs in the reference is not stable and can be changed without that being considered a breaking change?

Answer 56 · 2021-02-16T18:15:25.000Z

The Rust Reference only documents Stable Rust. It also ships with the stable compiler as part of its documentation of each stable version. There's a separate document (The Unstable Book) for Nightly things. Source: This is the stated position of the main two people who do some of the most work on the Reference when they were asked about this exact issue ~2 weeks ago (I believe it was Feb 1) during their T-docs meeting.
I don't know precisely where, presumably it had a stabilization PR just like all other things that go through the normal stabilization process.
The current algorithm is being relied on by unsafe code that's already in the field. Changing it might break unsafe code in the ecosystem, and if so such breaks would probably be totally silent breaks. In other words, no compile errors would be generated, you'd just suddenly get runtime UB. Because unsafe code is checked by hand instead of by the compiler we must be extra careful with any changes in this area.

Answer 57 · 2021-02-16T18:18:54.000Z

The Rust Reference only documents Stable Rust. It also ships with the stable compiler as part of its documentation of each stable version. There's a separate document (The Unstable Book) for Nightly things. Source: This is the stated position of the main two people who do some of the most work on the Reference when they were asked about this exact issue ~2 weeks ago (I believe it was Feb 1) during their T-docs meeting.

We agree on this.

The current algorithm is being relied on by unsafe code that's already in the field. Changing it might break unsafe code in the ecosystem, and if so such breaks would probably be totally silent breaks. In other words, no compile errors would be generated, you'd just suddenly get runtime UB. Because unsafe code is checked by hand instead of by the compiler we must be extra careful with any changes in this area.

Of course in those areas that are affected by this and which interact with actual C, the behavior is already undefined.

I don't know precisely where, presumably it had a stabilization PR just like all other things that go through the normal stabilization process.

Like I pointed out above, this algorithm was add to the reference in a PR that implemented a completely unrelated RFC as a sort of drive-by. I've not seen any stabilization PR related to this issue. Please link it if you can find it.

Answer 58 · 2021-02-16T18:20:14.000Z

@mahkoh

There is no hostility intended. We're in a somewhat heated debate discussing an issue, not each other. Any attacks on each other's ideas are restricted to this debate.

Intent doesn't change the net effect. And when something feels "heated", that's all the more reason to err on the side of de-escalating wherever possible. Again, we're all here trying to make things better, so let's remember that we're trying to collaborate, not attack or score points, please. It's possible to express a preference for one idea over another, or point out the issues with something, in a collaborative rather than adversarial fashion.

Answer 59 · 2021-02-16T18:27:06.000Z

If the reference didn't exist, or was a blank document, or simply didn't describe any algorithm for repr C, it would still be very dangerous to change how repr C works. Because people use Repr C for a lot more than interaction with C code. They use it to make unsafe code that fiddles with bytes and pointers and things like that do whatever they want to do. It's perhaps unfortunate, but that's the state of the ecosystem.

Answer 60 · 2021-02-16T18:35:06.000Z

@joshtriplett I will not go into this kind of debate here and I don't think you're intending to go into this kind of debate either. There is already too much too much discussion of minor side issues here and this debate would be completely off topic. Do not take this as me ignoring your comment or silently agreeing or silently disagreeing with it.

Answer 61 · 2021-02-16T18:41:39.000Z

If the reference didn't exist, or was a blank document, or simply didn't describe any algorithm for repr C, it would still be very dangerous to change how repr C works. Because people use Repr C for a lot more than interaction with C code. They use it to make unsafe code that fiddles with bytes and pointers and things like that do whatever they want to do. It's perhaps unfortunate, but that's the state of the ecosystem.

That is true but let's remember that this would only affect windows targets (excluding MinGW) and only those that use zero-sized types in repr(C). The effects could be negligible. Maybe one could evaluate the crates on crates.io. I'm mostly concerned about repr(C) on types with generic type parameters. However, such types are also unsound on MSVC targets because of

#[repr(C, packed)]
struct X<T> {
    t: T,
}

#[repr(C, align(16))]
struct Y {
    i: i32,
}

fn main() {
	// expected: 16
	// actual: 1
    println!("{}", std::mem::align_of::<X<Y>>());
}

(If the code assumes packed to be equivalent to #pragma pack(1).)

Note that this only affects types with generic type parameters because repr(align) in repr(packed) is otherwise not permitted precisely to prevent this issue.

Answer 62 · 2021-02-16T18:48:05.000Z

Well I wrote such a thing as a joke once and I barely needed it, so I'm sure someone has made such a thing for real.

Answer 63 · 2021-02-16T18:54:00.000Z

We discussed this in the @rust-lang/lang meeting today. Our conclusions:

repr(C) is intended to mean "match the default C ABI for the target". (This applies to structs, enums, or anything else to which you can apply #[repr(C)].) If repr(C) for MSVC targets does not match the C ABI of MSVC, that's a bug and should be fixed. If the reference describes repr(C) differently, the reference should be fixed.
We'd like to see documentation for repr(C) (including the reference) linking to appropriate definitions for the C structure layout ABIs.

Answer 64 · 2021-02-16T20:39:28.000Z

@joshtriplett I'd like to suggest that Rust should report a lint / warning, if #[repr(C)] is applied to any zero-sized types. It should be suppressible. I think a warning is good for two reasons:

It's a violation of the C spec, and different compilers handle this differently. It's good to inform the user, since they're already in the danger zone.
If the user is using #[repr(C)] for interop with C/C++, not just C, then they may be very surprised when the layout is different from what was expected in C++ (sizeof == 1). I know it's an obscure case, but a warning could save someone a lot of hassle, especially because this is an area where no single compiler has enough information to detect the mismatch.

I think the main scenario that will come up with #[repr(C)] and ZSTs is PhantomData or similar marker types. Placing a #[allow(repr_C_zero_sized_types)] (or whatever) on PhantomData should be sufficient.

Answer 65 · 2021-02-16T20:44:14.000Z

Yeah, PhantomData is supposed to be phantom, it shouldn't affect even repr C layout.

Answer 66 · 2021-02-16T21:18:17.000Z

@sivadeilra Agreed, a warning would be welcome. It would match GCC and Clang which both warn about it under pedantic mode. ICC also follows GCC and Clang in behavior, although it does not report it.

If repr(C) for MSVC targets does not match the C ABI of MSVC, that's a bug and should be fixed.

That means rustc will hard error for MSVC targets, right?

Answer 67 · 2021-02-16T22:49:56.000Z

@ojeda No, it'd mean that we need to fix rustc on MSVC targets to match the ABI of MSVC.

Answer 68 · 2021-02-16T22:50:56.000Z

@sivadeilra That might potentially be reasonable, but I'd ask that you file that as a separate issue (also tagged T-lang), so that we can consider it independently.

Answer 69 · 2021-02-16T23:12:16.000Z

@joshtriplett That's the thing: there is no match for that in MSVC (for C) since the compiler does not accept it.

Answer 70 · 2021-02-19T17:40:46.000Z

Drive-by comment from a Windows user who doesn't do much FFI.

I honestly think that the fact that #[repr(C)] is a consistent, obvious layout the same between all targets is more useful than matching the C ABI of the platform. If you're doing FFI it's already important to be double checking that your layouts are correct, because you're unsafely promising that they are, and getting it wrong is pain to diagnose.

Rust also uses ZSTs much more than C or C++ code does. It's specifically called out that these types are treated as zero sized, zero stride in documentation, even though this isn't legal in standard (or MSVC) C.

Silently changing the #[repr(C)] layout of ZSTs would be roughly similar to telling every Rust-only use of #[repr(C)] that they need to switch to using #[repr(trivial)] or potentially break on MSVC targets.

Also, what about [[no_unique_address]]_C++? Once MSVC gets support for that C++20 attribute, it'll be important that we can express true ZST members on the platform.

My argument here is that #[repr(C)] isn't unsound on any target. Potentially surprising, yes, but not unsound. Any unsoundness requires using unsafe to link to FFI using a different C-with-extensions ABI, and assuming that it's the same.

My proposal is that repr(C) ideally be described something like

#[repr(C)] causes a type to be laid out using the following simple C-like layout:

(inorder, with Rust standard size and align)
(maybe even give the const generic fn repr_C)

For types with a standard C analogue, this is layout compatible with the platform C ABI. However, this also supports some nonstandard extensions, such as

Zero sized types. These are treated as they are in Rust: they align the layout but do not take any space. (This may be different than platform, which often gives types a minimum size 1.)

Generic types are laid out after being monomorphized as if they were not generic.

(other extensions?)

#[repr(packed(N))] forces the alignment of all members to be no greater than N. (This may differ from the behavior of #pragma pack on some platforms.)

I agree that the enum case should at a minimum lint when it would be bumped to be bigger than c_int.

Answer 71 · 2021-02-19T17:48:27.000Z

Which so brings to mind the fact that GPU users are using repr C to match various graphics layout specs.

So we do need a repr(simple) layout of some sort which is the same anywhere, regardless of what the local repr(C) does.

Answer 72 · 2021-02-19T21:03:19.000Z

I honestly think that the fact that #[repr(C)] is a consistent, obvious layout the same between all targets is more useful than matching the C ABI of the platform. If you're doing FFI it's already important to be double checking that your layouts are correct,

If you only target a single platform, perhaps. However, for those writing portable software, it means they will need to think about all the possible layouts for all targets out there. The idea is to leave that burden to the compiler instead.

We need to differentiate several use cases:

"Exporting" Rust items in a consistent way in memory. That is, your program is the one controlling the layout. This may support all Rust features since you are not trying to interface with existing layouts. The default representation doesn't work here, in that this one needs to be fully deterministic (for whatever features it supports).
"Importing" types from other languages through C headers. That is, your program does not control the layout. Here you want to match what other libraries are already exporting, which are almost always expressed in terms of C header files, regardless of the internal language they use.
Other cases where your program does not control the layout, yet it is not given as a C header file. For instance, memory-mapping an existing format, non-C FFI, etc.

#[repr(C)] is mostly intended for the second case, yet it also tries to cover a bit of the others, e.g. it gives a layout to an enum with fields. In hindsight, perhaps #[repr(C)] should have been stricter overall, providing other use cases via other reprs.

2021 edition? :)

Answer 73 · 2021-02-23T19:22:34.000Z

Following up on this with further discussion from the @rust-lang/lang meeting today:

As stated in #81996 (comment) , repr(C) is intended to be C-compatible, not just a generic "simple layout".
We're potentially open to a separate repr(inorder) or repr(stable) or similar, but that'd need to be a separate proposal, with its own precisely defined semantics.
We think it may be possible to fix repr(C) to match the behavior of C, without breaking established usage of ZSTs in Rust, and in particular, without breaking things like PhantomData. A general rule that should work: we can fix the behavior of zero-sized arrays and repr(C)/repr(align) ZSTs as fields of repr(C) structures, but we can continue to say that repr(Rust) 1-ZSTs (ZSTs with alignment 1, which includes PhantomData/PhantomPinned) do not affect layout at all. (This would require careful definition, to ensure that [u8; 0] does affect layout but PhantomData doesn't, but that definition shouldn't need to special-case PhantomData.)

Answer 74 · 2021-02-23T22:28:49.000Z

@mahkoh What's the correct behavior for multiple zero-sized arrays?

#[repr(C)]
struct S {
    x: [i32; 0],
    y: [i32; 0],
}

With the C

struct S
{
    int32_t x[0];
    int32_t y[0];
} S;

int SIZEOF_S = sizeof(S);

I get that Clang and GCC produce a zero-sized type, and MSVC errors (illegal zero-sized array). This is legal Rust, so erroring out on Windows only is a definite no-go. We can't match platform C here, because the implementation doesn't even allow this structure.

Completely unrelated, I want to note that Rust enums, even with #[repr(C)] on a C-like enum, are very semantically different from C enums. As such, the "match platform C" argument for enums is much weaker there. C enums are better represented in Rust as a newtype around the properly sized integer and associated constants.

Answer 75 · 2021-02-24T01:44:14.000Z

With the C

Zero-length arrays are a GCC extension and are not intended to be used like that anyway. Standard C has flexible array members to fill a similar use case as the GCC extension.

This is legal Rust, so erroring out on Windows only is a definite no-go.

Whether it is legal Rust or not is depends on how Rust wants to define #[repr(C)]. It already gives an error for zero-variant enums; it could also give it for other cases such as when there is no way to match the platform.

Answer 76 · 2021-02-24T01:49:03.000Z

It's currently legal Rust. I've even seen it used for alignment reasons. E.g.

/// Aligns `T` to the alignment of `Aligner`, similar to `alignas(alignof(T))` in C++
#[repr(C)]
pub struct AlignedTo<Aligner, T> {
     _align: [Aligner; 0],
     pub value: T,
}

I suspect code that does this is going to be broken by fixing this issue regardless... This is part of the unfortunate consequence of repr(C) being both the "stable layout" and "C compatibility".

Answer 77 · 2021-02-27T20:38:19.000Z

Zero-length arrays are a GCC extension and are not intended to be used like that anyway. Standard C has flexible array members to fill a similar use case as the GCC extension.

If zero-length arrays are both nonstandard C and is an extension to be used for FAM: why do we care what the layout is at all? Such a type should never exist on the stack at all anyway, and only behind a pointer. And the correct way to model FAM on the Rust side of FFI is as [T] (if you want a fat pointer^[1]), or an aligned extern type (unstable).

Even if we say that #[repr(C)] matches the platform ABI as closely as possible, and not just the subset of the platform ABI that's expressible in standard C^[2], imho it doesn't make sense to just assume that zero-sized arrays are a purposeful part of the system ABI just because the platform compiler supports them and it's de-facto stable/supported. C is a language of "we'll allow you to do this, but all bets are off if you do." If the extension for zero-sized arrays is defined for use on the heap as FAM, and no mention of their use on the stack is made, their use on the stack is an artifact of implementation and not a designed feature.

I'm going to make a hard claim here: #[repr(C)] should match the system C ABI, but only for the subset of the ABI accessible with standard C. This aligns with the choices made by extern "C-unwind"^[3]. IIUC, that means that empty structures and zero-sized arrays do not have to match the system ABI, as they are not expressible in standard C.

This isn't to say that C extensions are inexpressible (in a cross-platform manner), though! The ecosystem could provide a crate translating system ABI extensions to standard C to Rust's #[repr(C)] extensions to standard C. For example, you could use @mahkoh's own repc crate to provide a #[repc::repr_system_C] attribute that added extra padding members as required to match the system ABI.

[1]: and you can layer zero-cost abstractions over rust tail slices to get typed thin pointers on stable already.

[2]: the RFC for extern "C-unwind" goes in the other direction: extern "C" does not support unwinding, even if the platform ABI natively supports it (e.g. on MSVC), and you have to opt-in to unwinding support with extern "C-unwind"^[3].

[3]: This establishes precedent that "C" in Rust refers to standard C, not "system ABI". In fact, extern "system" doesn't even support unwinding, you have to use extern "system-unwind" for that.

Answer 78 · 2021-03-02T13:29:12.000Z

If zero-length arrays are both nonstandard C and is an extension to be used for FAM: why do we care what the layout is at all?

Zero-length arrays are the only way in Rust to express FAMs. The C compilers of all targets supported by rustc produce the same layout in situations where they permit zero-length arrays and FAMs in the same position. Therefore there is no ambiguity.

Such a type should never exist on the stack at all anyway, and only behind a pointer.

Whether such types can exist on the stack is immaterial, their layout must be known either way. Even so, structures with FAMs do exist on the stack. I do not understand why you would think otherwise.

And the correct way to model FAM on the Rust side of FFI is as [T] (if you want a fat pointer[1]), or an aligned extern type (unstable).

Why is this the correct way? The size of the FAM is often only known after inspecting the surrounding structure. This does not play well with using a slice.

Even if we say that #[repr(C)] matches the platform ABI as closely as possible, and not just the subset of the platform ABI that's expressible in standard C[2], imho it doesn't make sense to just assume that zero-sized arrays are a purposeful part of the system ABI just because the platform compiler supports them and it's de-facto stable/supported.

We do not need to assume this. Zero-sized arrays have been purposefully used for decades to express structures that end with a variable-sized array. FAMs were only introduced in C99. This is fact.

C is a language of "we'll allow you to do this, but all bets are off if you do." If the extension for zero-sized arrays is defined for use on the heap as FAM, and no mention of their use on the stack is made, their use on the stack is an artifact of implementation and not a designed feature.

This is immaterial as the layout must be known either way. Furthermore structures with FAMs are used on the stack and there is nothing ambiguous about this usage.

If you think about how a struct with a FAM is created in the first place, you'll see that there is nothing special about using it on the stack.

I'm going to make a hard claim here: #[repr(C)] should match the system C ABI, but only for the subset of the ABI accessible with standard C.

How did you arrive at this?

This aligns with the choices made by extern "C-unwind"[3]. IIUC, that means that empty structures and zero-sized arrays do not have to match the system ABI, as they are not expressible in standard C.

Why should unwinding serve as a precedent for type layout?

This isn't to say that C extensions are inexpressible (in a cross-platform manner), though! The ecosystem could provide a crate translating system ABI extensions to standard C to Rust's #[repr(C)] extensions to standard C. For example, you could use @mahkoh's own repc crate to provide a #[repc::repr_system_C] attribute that added extra padding members as required to match the system ABI.

How would you imagine such an attribute would work?

Answer 79 · 2021-03-02T23:59:37.000Z

Zero-length arrays are the only way in Rust to express FAMs.

This is literally not true. I literally wrote two crates [erasable, slice-dst] to make working with FAMs (and thin-pointer FAMs) in stable Rust tractable. FAMs are currently used in rust-analyzer's syntax tree, rowan. They used to be served by these crates, and currently by an inlined subset of triomphe (because rowan doesn't need weak counts). Here's all of the machinery on the playground. It even is all fully accepted under Miri's current implementation of Stacked Borrows.

(I don't really like advertising my crates in rust-lang/rust, but I guess they're relevant enough here.)

Even further, zero-length arrays cannot be "the only way in Rust to express FAMs," because they aren't. Zero length arrays in Rust are how to express a zero-length array, or extra alignment at a point in the structure that otherwise wouldn't be there. If bindgen is emitting zero-length arrays for FAMs, that is a bug in bindgen (or a mistake in using bindgen), not a deficiency of Rust (in my opinion). (More info below.)

Why is [slices] the correct way [to represent FAM]? The size of the FAM is often only known after inspecting the surrounding structure. This does not play well with using a slice.

I beg to differ. I spent 6 months of my free time developing the crates to prove it, on stable Rust. I've linked them above. They're available under the exact same license as rustc itself, so you'll have no legal issues using the crates. I'm also super happy to support people using these crates to make their pointer usage safer. (These libraries also got me into grad school, so I'm super proud of them.)

FAMs were only introduced in C99. This is fact.

And C99 has been around for decades. Rust has no target where the "platform C compiler" doesn't support C99 and FAM. _{(Correct me if I'm wrong, I'm not 100% certain on that one.)} If FAM and zero sized arrays are ABI equivalent, why haven't the projects you cared about switched to using standard FAM in the past two decades?

If you need to use a header older than C99, I see no issue in requiring the user of bindgen to provide a C99 version of the header using FAMs instead of zero-sized arrays. In my opinion, currently bindgen should probably emit FAMs as opaque types (in the bindgen usage of opaque).

Whether such types can exist on the stack is immaterial, their layout must be known either way. Even so, structures with FAMs do exist on the stack. I do not understand why you would think otherwise.

They don't on the Rust side of the FFI barrier, because Rust has no alloca/unsized_locals. A prefix of the type can exist on the stack, but then why does it matter what the layout of the zero-sized array "FAM" part of it is, then?

If you think about how a struct with a FAM is created in the first place, you'll see that there is nothing special about using it on the stack.

Given that I wrote a crate that does exactly that, I think I know how to create a FAM in Rust. Alloc uninit space for it, and carefully move every item into place while keeping track of what's where for panic safety. Or ptr::copy_nonoverlapping if the array items are Copy. In C it goes something like,

Fam* ptr = malloc( sizeof(Fam) + length * sizeof(Item) );
ptr->length = length;
memcpy( &(ptr->array), sourceArray, length * sizeof(Item) );

If you want to insist, you could also

Fam header;
header.length = length;
Fam* ptr = malloc( sizeof(header) + length * sizeof(Item) );
memcpy( ptr, &header, sizeof(header) );
memcpy( &(ptr->array), sourceArray, length * sizeof(Item) );

but still, the thing on the stack is not the FAM, it's a prefix of the type without the FAM.

Why should unwinding serve as a precedent for type layout?

Not directly, but it serves as a precedent for how the Rust project treats the term "C" for the purposes of FFI. If you're not doing FFI, it doesn't matter if #[repr(C)] layout exactly matches the behavior of MSVC or not. Even though MSVC C always supports native unwinding, Rust's extern "C" doesn't. This is a departure from "MSVC C" to match standard C.

The exact same logic can apply to #[repr(C)]. Even though "MSVC C" supports using a zero-sized array to mark FAM, Rust's #[repr(C)] doesn't. This is a departure from "MSVC C" to match standard C.

This isn't even removing any functionality, either. C FAMs are just as usable as they ever were in Rust: barely. Creating a reference to them is a no-go (as you cut off the FAM part) so you're stuck with pointer operations.

The only thing that changing the layout really does is make passing around FAM sized prefixes on the stack over FFI easier. And to reiterate: the sized prefix is logically a different type.

And to remind you: bindgen currently emits zero-sized arrays in the Rust sense to force alignment within structures sometimes. Making zero-sized arrays always mean FAM and be not zero-sized sometimes (i.e. on MSVC targets) would break every existing bindgen'd code that currently uses zero-sized arrays for alignment.

How would you imagine #[repc::repr_system_C] would work?

It would calculate the "system C" layout of the type as described. If it potentially differs on any platform (say, the structure contains a zero-sized array, or is itself empty), then it would add extra #[cfg]'d members to make the Rust layout match the platform layout for the appropriate #[cfg]s. (The Rust #[repr(C)] layout algorithm is still simple to describe and a candidate to be provided as a std function, and knowing it, you can create a struct with whatever layout you want.)

How did you arrive at [the conclusion that #[repr(C)] should only respect standard C]?

To be completely honest: by starting from a position that changing the size of [T; N] sometimes due to nonstandard C extensions is silly and undesirable, and would break who knows how much code using arrays for alignment on MSVC to fix some small number passing FAM-prefixes as by-copy function arguments. Plus, MSVC is getting support for [[no_unique_address]]_C++20 at some point, and matching those structures (which have a defined layout) over FFI would be impossible if #[repr(C)] ZSTs were "upgraded" on MSVC to take up space in #[repr(C)] contexts. [[no_unique_address]] doesn't take you out of "standard layout" mode in C++ (IIRC), so those type with [[no_unique_address]] members are safe for FFI.

But this is also backed up by the unwind-wg's interpretation of what "C" means to Rust. Per their conclusions, "C" doesn't mean "platform C," because extern "C" doesn't support unwinding ever, even if the "platform C ABI" does. They found it more important to keep Rust's behavior consistent on different platforms than to match the platform-native capabilities by default. I agree with them, and applying that same logic here restricts #[repr(C)]'s guarantees to standard C (which luckily happens to have the same layout on all platforms LLVM targets), and leave the extensions to standard C up to Rust to specify for Rust.

I come from a position of using #[repr(C)] in Rust-focused code, not C-focused code. To be fair, that probably biases me towards keeping #[repr(C)] consistent between platforms. But doing so is consistent with other decisions that Rust has made.

IMHO, the fact that bindgen emits something that's wrong means that we should fix bindgen, not change the language such that bindgen is correct.

And by the way: I may be only two years older than C99, but I have written C code using FAMs. I'm familiar enough with using the standard C99 functionality for FAMs to use them without immediately invoking UB. While it's good to not assume that anyone in the Rust space is familiar with dark corners of performant C, it's also not great to assume that nobody's gazed into the dark abyss.

(And to be clear: I have no opposition to the enum fix. I think having a hard error or at least warning for enum discriminant sizes over the inferred size of a #[repr(C)] enum to be completely fair. It's only the (ab)use of zero-sized arrays as FAM that I take issue to.)

Answer 80 · 2021-03-03T08:27:41.000Z

This is literally not true. I literally wrote two crates [erasable, slice-dst] to make working with FAMs (and thin-pointer FAMs) in stable Rust tractable.

If you want to reduce the issue to absurdity, then repr(C) is completely unnecessary. You can simply use an appropriately sized and aligned byte array and pointer operations. (PS: After reading the rest of your comment it appears that you assume that the offset of a FAM in a struct is always trivial to calculate. If we assume this position then your comment makes more sense to me because no help from the type system is necessary to access FAMs.)

Even further, zero-length arrays cannot be "the only way in Rust to express FAMs," because they aren't.

They absolutely are.

I beg to differ. I spent 6 months of my free time developing the crates to prove it, on stable Rust. I've linked them above.

I only took at short look at the readmes but I saw nothing that relates to FAMs. Please link an example of how these crates are used to create bindings.

And C99 has been around for decades. Rust has no target where the "platform C compiler" doesn't support C99 and FAM. (Correct me if I'm wrong, I'm not 100% certain on that one.) If FAM and zero sized arrays are ABI equivalent, why haven't the projects you cared about switched to using standard FAM in the past two decades?

That's a very academic take. You don't want to deal with legacy technology so you tell everyone to rewrite their software and drop support for C89 compilers. I also never claimed that they are ABI equivalent only that they are equivalent on all platforms that are supported by rustc, which is a very limited number of platforms.

They don't on the Rust side of the FFI barrier, because Rust has no alloca/unsized_locals. A prefix of the type can exist on the stack, but then why does it matter what the layout of the zero-sized array "FAM" part of it is, then?

Alloca is not necessary to store a struct with a FAM on the stack. You're making it sound like you've never once seen such a struct on the stack even though you've spent several months creating code that provides C-ABI compatible types in Rust. Have you never come across something like

union {
	struct {
		int size;
		long elements[];
	};
	char buf[1024];
} x;
read(fd, &x, sizeof(x));
x.elements[1] = 1;

The "layout" of the FAM part of a struct is significant because it can affect the size and alignment of the surrounding structure and because it is necessary to know the offset of the FAM from the start of the struct to access the array.

In C it goes something like,

Fam* ptr = malloc( sizeof(Fam) + length * sizeof(Item) );
ptr->length = length;
memcpy( &(ptr->array), sourceArray, length * sizeof(Item) );

That's incorrect in the general case. The array can start at an offset smaller than sizeof(header). For example

struct X {
	int a;
	char b;
	char c[];
};
sizeof(struct X) == 8;
offsetof(struct X, c) == 5;

https://godbolt.org/z/x3Grh8

The rest of your comment makes more sense to me if I assume that you thought that the offset of a FAM in a struct is always the same as the size of the struct. If that were the case then working with them would be much easier and I would agree that it isn't strictly necessary for rustc to provide a direct way to wrap them.

The exact same logic can apply to #[repr(C)].

Which logic? So far you've only stated that some decision was made for unwinding. You've not shown me the logic that was used to arrive at this decision.

This isn't even removing any functionality, either. C FAMs are just as usable as they ever were in Rust: barely. Creating a reference to them is a no-go (as you cut off the FAM part) so you're stuck with pointer operations.

Creating bindings for C structs with FAMs works perfectly fine in Rust. While it doesn't allow you to index the array directly, creating an appropriate slice at runtime is the simple part. The type system does the hard part: telling you where the array starts.

Making zero-sized arrays always mean FAM and be not zero-sized sometimes (i.e. on MSVC targets) would break every existing bindgen'd code that currently uses zero-sized arrays for alignment.

What? Zero-sized arrays in C are zero-sized on all platforms targeted by rustc. Rustc and the C compilers are in perfect agreement here. This issue is about the size of structs.

It would calculate the "system C" layout of the type as described.

That's not possible using any mechanism currently provided by rustc. How would the implementation of the attribute be able to calculate the layout of a struct if it doesn't have access to the types of the fields? How would rustc know which type layouts to pass to the implementation (if such a mechanism existed) if the declaration of the struct has to modify the body of struct to generate the desired layout?

To be completely honest: by starting from a position that changing the size of [T; N] sometimes due to nonstandard C extensions is silly and undesirable

Nobody is arguing for this at least not for N = 0. There are some cases in which MSVC and other compilers disagree on the size of [T; N] but only for N > 0. Currently it is however not possible to write equivalent code in Rust. Otherwise I would have opened an appropriate issue.

Plus, MSVC is getting support for [[no_unique_address]]C++20 at some point,

MSVC already produces different type layouts for the same code in C++ and C mode regardless of no_unique_address. repr(C) is about C layout not C++ layout. Adding repr(CPP) is orthogonal to this issue.

Answer 81 · 2021-03-03T09:15:15.000Z

Hey folks, not to minimod, but i feel this comment thread is a bit too hostile to be productive.
As much as I enjoy me some github drama, let's chill out, aye? No need for personal attacks.

Answer 82 · 2021-03-03T09:36:54.000Z

While it is great to have many different opinions, unfortunately, in every discussion regarding C interoperability there is an endless number of opinions, stated with great certainty, that are nevertheless based on an incomplete or incorrect understanding of the C side of things. In a more sane world you would correct these opinions and the people would then admit that their opinions were based on faulty information and would study up on the details before continuing to make factual statements. Here on the other hand such opinions are applauded from the sidelines (in the form of "upvotes") and it is I who will be shut down for not having a sufficiently large amount of tolerance for being drowned out by such opinions without recourse.

Answer 83 · 2021-03-03T09:37:50.000Z

While I personally don't see neither "personal attacks" nor "github drama" in this technical discussion, I'd relink the relevant comments of @joshtriplett (this and this) with the insights from discussions of the rust-lang team, hoping they'll help.

EDIT @mahkoh apologies if you have the feeling you're been "drowned out" by some social feature of github that doesn't contribute a lot to the content

Answer 84 · 2021-03-04T12:26:06.000Z

imho it doesn't make sense to just assume that zero-sized arrays are a purposeful part of the system ABI just because the platform compiler supports them and it's de-facto stable/supported.

If it is documented (like in this case), I think it is reasonable to assume it. Some compilers have gone great lengths to match the platform compiler -- even when documentation is lacking.

Now, GCC recommends using the ISO approach for the flexible array member use case; however, people have used zero-length arrays for other uses, like marking a point in a struct.

In any case, regardless of what is documented or not, anyone that wants to integrate Rust in an existing C project or use a C library that uses zero-length arrays in their headers is likely interested in seeing this somehow taken care of.

C is a language of "we'll allow you to do this, but all bets are off if you do."

Note that in C you are not allowed to use undefined behavior etc. in a strictly conforming program. When you do use it, nevertheless, the C standard doesn't say "all bets are off" -- it can be very well that the nonportable behavior is documented and useful.

If the extension for zero-sized arrays is defined for use on the heap as FAM, and no mention of their use on the stack is made, their use on the stack is an artifact of implementation and not a designed feature.

The C standard provides an example of a struct being used without allocation. It also explains that while accessing the flexible array member might be undefined behavior, it might be that the size of the struct is big enough to make it legitimate.

Furthermore, it is important to note that quite a few compilers (GCC, Clang, MSVC, Intel's, IBM's...) have extensions to initialize them on the stack and/or statically with an arbitrary number of elements, which makes them quite more useful for the non-allocation cases.

Then there is also the example @mahkoh gave, where the maximum size is known; although I guess one could argue setting the size explicitly would be preferred.

why do we care what the layout is at all?

To be useful to users, of course! :-)

For instance, in the Linux kernel we have uses of both flexible array members and zero-length arrays (and not just for the flexible array member use case). While I am not certain whether those definitions would end up being needed or not if Rust gets used in the kernel, it is an example that there are projects out there that may be using them.

Per their conclusions, "C" doesn't mean "platform C,"

The language team seemed to go in the other direction last week, so this is confusing...

dark corners of performant C ... dark abyss.

I would not call flexible array members a dark corner of C; they are fairly well-known and used out there.

Answer 85 · 2021-03-04T14:03:20.000Z

While I agree that using zero-sized arrays is currently a very reasonable (and probably the best) way to express flexible array members in Rust, I still don't think it need to be the "only way" -- people also use them to express real zero-sized arrays instead of FAMs in #[repr(C)] types (and they expect zero-sized arrays don't affect the size of #[repr(C)] structs), and the 0 in the type name might be confusing (even in C, it's non-standard), or at least not ideal (zero-sized arrays are not advertised for this purpose in Rust).

Could a new (and maybe magic) type that is specific for FAMs (and has desirable layout effects) be clearer and less breaking?

Answer 86 · 2021-03-05T21:38:59.000Z

Ah, ugh. I just noticed that ~~under default settings,~~ (edit: nope, only if you disable #[derive(Copy)]) bindgen translates the following C:

union SomeUnion {
    int32_t int32;
    double float64;
    void *ptr;
};

into something like the following Rust:

#[repr(C)]
pub struct __BindgenUnionField<T>(::core::marker::PhantomData<T>);
impl<T> __BindgenUnionField<T> {
    #[inline]
    pub const fn new() -> Self {
        __BindgenUnionField(::core::marker::PhantomData)
    }
    #[inline]
    pub unsafe fn as_ref(&self) -> &T {
        ::core::mem::transmute(self)
    }
    #[inline]
    pub unsafe fn as_mut(&mut self) -> &mut T {
        ::core::mem::transmute(self)
    }
}

#[repr(C)]
pub struct SomeUnion {
    pub int32: __BindgenUnionField<i32>,
    pub float64: __BindgenUnionField<f64>,
    pub ptr: __BindgenUnionField<*mut core::ffi::c_void>,
    pub bindgen_union_field: u64,
}

Things to note here:

SomeUnion in rust is a #[repr(C)] struct (not a union).
__BindgenUnionField is #[repr(C)] and not #[repr(transparent)]
The usage of this is something like *some_union.ptr.as_ref() for reads and such. bindgen_union_field is just aligned storage.
All of the __BindgenUnionFields preceding the storage must be at the same byte offset, e.g. 0, for this to work (even if it works, it is pretty dodgy¹)

In an ideal world, bindgen would do something better here:

ideally, just using a #[repr(C)] union (edit: with ManuallyDrop fields for anything non-Copy).
failing that, using #[repr(transparent)] on __BindgenUnionField would probably avoid issues under any of the proposed changes to the layout zero-sized repr(C) fields.

(I assume these is not done for version compat, and there may be a flag you can pass to bindgen that will do it)

Unfortunately, it doesn't really matter if bindgen fixes this: It's exceedingly common to pregenerate bindgen output and check the result in directly, rather than running bindgen at build time².

That is, there's a body of code out there that uses this that will break more-or-less silently³ if the size of a currently-zst repr(C) struct changes to non-zero on some platforms. To make matters worse, this code is probably quite passively maintained (this is the case for most -sys crates IME), and fairly unlikely to get updates, especially if the breakage is only on one platform (windows in particular tends to get ignored for open source 😢).

All that said, this doesn't contain zero len arrays, just zero sized structs, so perhaps it would avoid fallout from this?

¹ The current formulation of stacked borrows doesn't support turning &T into &U where size_of::<U>() > size_of::<T>(), see rust-lang/unsafe-code-guidelines#256 and rust-lang/unsafe-code-guidelines#134. That said, there are reasons to believe this may be fixed in future iterations of stacked borrows, as it... seems to often cause problems.

² This is both in order to improve compile times (bindgen is a slow build dependency), and to avoid a dependency on llvm tools and libclang, which are often problematic on Windows.

Yes, this requires the C library to be well behaved and have types and functions that aren't particularly target specific.

³ If the user emitted layout tests, they would fail, but these... actually fail on any ABI other than the one that generated the bindings, even if the structs work, since they check hardcoded byte offsets rather than anything like computing based on size/align of fundamental C types.

I understand why this is the case, but it still sucks, and means that we really can't expect "this already was failing in crater" to be an indication of much.

Answer 87 · 2021-03-05T21:46:39.000Z

Ah, I may have spoke too soon. This does actually require disabling bindgen's emission of a #[derive(Copy, Clone)]. If you allow it to do that (as it does by default), then it will use #[repr(C)] union. This means the situation is probably less dire than I expected, since it's not the default flags. It would still be nice not to break this code though, since it's unlikely to be fixed.

Answer 88 · 2021-03-07T06:44:54.000Z

Note that this change would make the ref-cast crate as currently implemented unsound (for an edge case where it's not really useful anyway).

Specifically, ref-cast currently assumes that #[repr(C)] is equivalent w.r.t. layout to #[repr(transparent)] for structs wrapping a singular field. With this proposed change, this would no longer be true for zero-sized arrays.

Specifically,

#[derive(RefCast)]
#[repr(C)]
struct UhOh([usize; 0]);

let array: &[usize; 0] = &[];
// array is an arbitrary pointer to ZST
let uhoh: &UhOh = UhOh::refcast(array);
// uhoh is a dangling reference to some nonzero number of bytes, with this change

Answer 89 · 2021-03-07T13:24:25.000Z

@apiraino:

While I personally don't see neither "personal attacks" nor "github drama" in this technical discussion [...]

EDIT @mahkoh apologies if you have the feeling you're been "drowned out" by some social feature of github that doesn't contribute a lot to the content

Sometimes I do feel like I'm being (unintentionally) gaslit. Thank you for saying that there is at least one other person in this forum who shares my perception. I guess I haven't gone completely crazy yet.

@ojeda, @hyd-dev:

Is this discussion regarding FAMs that significant? As you said, people are using zero-sized arrays for force alignment and positioning within records. But people are also using unit-structs for this purpose

struct X {
	char c;
	char d __attribute__((aligned(8)));
};

#[repr(C, align(8))]
struct Aligner;

struct X {
	c: u8,
	_aligner: Aligner,
	d: u8,
}

In particular because the alignment of u64 is not 8 on all platforms, using an [u64; 0] here instead of a unit struct would be incorrect. Maybe unit structs could also be special cased but then what about

#[repr(C, align(4))]
struct Aligner1;

#[repr(C, align(8))]
struct Aligner2;

#[repr(C)]
struct Aligner3(Aligner1, Aligner2);

to express the maximum of two alignments.

@thomcc

Yes this is a showstopper. The ossification of repr(C) is probably too advanced at this point to change it. There are many cases in which bindgen produces incorrect or platform dependent code while attempting to hack around the lack of native equivalents to C constructs. Whatever repr replaces repr(C) as the native C representation should be powerful enough to make the implementation of bindgen trivial so that this kind of ossification doesn't happen again in the future.

Answer 90 · 2021-03-08T17:10:49.000Z

But people are also using unit-structs for this purpose

@mahkoh To be honest, I don't know how unit structs could go wrong. Am I missing something? Unit structs match all people's expectations in the examples: they're still zero-sized, therefore can still be used for zero-sized purposes. So why worrying about special casing them?

Answer 91 · 2021-03-08T17:19:43.000Z

@hyd-dev I do not think that this discussion is useful anymore because @thomcc's examples have already shown that repr(C) cannot be changed.

Answer 92 · 2021-03-08T17:29:08.000Z

@hyd-dev I do not think that this discussion is useful anymore because @thomcc's examples have already shown that repr(C) cannot be changed.

@mahkoh I think I understand now. Thanks for your explanation.

(Note that my proposal doesn't change #[repr(C)] to be backward-incompatible. I think the new type can (have some special/magical properties/limitations to) be backward-compatible.)

Answer 93 · 2021-03-16T17:37:07.000Z

@hyd-dev Can you point to which proposal you mean?

We discussed this again in today's @rust-lang/lang meeting. Summary: repr(C) was always intended to match the underlying C ABI, and to the extent that isn't already the case, we would very much like to know if it's possible to change repr(C) to match the underlying C ABI without breaking existing users. If there's a way to do that, we'd be happy to see that change made.

If it turns out that there are irreconcilable problems here, where existing code is relying on properties of repr(C) that differ from the C ABI but that we can't fix compatibly, we could consider an edition-based transition here: we could introduce a fixed C repr, and then using an edition to let us call that repr(C) in the future (rather than calling it the equivalent of repr(C_fixed) forever and telling people repr(C) doesn't work as expected), without breaking existing code.

Answer 94 · 2021-03-16T18:40:13.000Z

@hyd-dev Can you point to which proposal you mean?

I meant #81996 (comment).

That is, a new type that is specific for flexible array members, for example, core::ffi::Fam<T>. I proposed that because as far as I'm aware, this issue shows there are problems only with zero-length arrays that are used to express flexible array members, but in Rust, zero-length arrays are commonly used to express real zero-sized arrays. They're expected to be zero-sized in many cases (though not all cases). However, a new type can:

Be clearer. Zero-length arrays are never advertised to be used to express flexible array members (at least in official documentations that I've read carefully).
Affect the layout of the #[repr(C)] struct, union or other similar thing that contains it to match the native C ABI.

For backward compatibility:

It's a new type, so none of existing code will break immediately.
The new type can have several special restrictions, for example, being only allowed as the tail member in #[repr(C)] structs or fields in #[repr(C)] unions, and not being allowed to be used alone (in local variables, function parameters (maybe?), or as references), in order not to cause unsoundness in existing code that works on arbitrary types, such as ref-cast.

Answer 95 · 2021-03-16T19:02:43.000Z

The new type can have several special restrictions, for example, being only allowed as the tail member in #[repr(C)] structs or fields in #[repr(C)] unions, and not being allowed to be used alone (in function parameters, local variables or as references), in order not to cause unsoundness in existing code that works on arbitrary types, such as ref-cast.

Does this restriction need to be "infectious"? In other words, say struct Foo is repr(C) and has a Fam as tail member; is Foo itself now also a tail-field-only kind of type?

Answer 96 · 2021-03-16T19:13:16.000Z

I can't imagine it not being that way.

Answer 97 · 2021-03-16T19:16:27.000Z

So it's a bit like unsizedness... but what does that mean for generics?

struct Bar<F>(F, i32);

// In another crate
type T = Bar<Foo>; // using my `Foo` from above

Answer 98 · 2021-03-16T19:18:52.000Z

Does this restriction need to be "infectious"? In other words, say struct Foo is repr(C) and has a Fam as tail member; is Foo itself now also a tail-field-only kind of type?

That seems necessary.

but what does that mean for generics?

Maybe that can be forbidden? That is, not allowing such types to be used as generic type parameter.

Answer 99 · 2021-03-16T19:24:15.000Z

This discussion is pointless as the issue has nothing to do with flexible array members but with records whose fields all have size 0.

Answer 100 · 2021-03-16T19:29:00.000Z

but with records whose fields all have size 0.

Only with zero-length array members or structs that contains zero-length array. As far as I can tell from this thread, MSVC does not allow other zero-sized types (EDIT: I meant the things when using as fields. My wording may be inaccurate here. Apologize.).