repr(C) is unsound on MSVC targets

[mahkoh]

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

[jyn514]

@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.

[mahkoh]

@jyn514 Yes

[mahkoh]

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.

[jonas-schievink]

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

[jonas-schievink]

Does this reproduce with clang?

[mahkoh]

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

• https://doc.rust-lang.org/reference/type-layout.html#reprc-structs
• https://doc.rust-lang.org/stable/std/alloc/struct.Layout.html#method.extend

Does this reproduce with clang?

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

• https://bugs.llvm.org/show_bug.cgi?id=48787
• https://bugs.llvm.org/show_bug.cgi?id=48777
• https://bugs.llvm.org/show_bug.cgi?id=48776
• https://bugs.llvm.org/show_bug.cgi?id=48835
• https://bugs.llvm.org/show_bug.cgi?id=48842
• https://bugs.llvm.org/show_bug.cgi?id=48849
• https://bugs.llvm.org/show_bug.cgi?id=48850
• https://bugs.llvm.org/show_bug.cgi?id=48875

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

[rylev]

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

@rustbot ping windows

[rustbot]

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

[rylev]

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

[Lokathor]

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.

[mahkoh]

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>()
}

[mahkoh]

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

[Lokathor]

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

[mahkoh]

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.

[sivadeilra]

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."

[mahkoh]

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.

[ghost]

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.

[mahkoh]

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.

[sivadeilra]

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.

[ghost]

@AngelicosPhosphoros I believe the issue here is the behaviour that may or may not be intentional is not desirable, per #81996 (comment).

(FWIW I've already linked the nomicon here.)

[Stargateur]

Note that C doesn't have any standard ABI, the only thing C require is field order. So, repr(C) from rust is a "best try" to match the "most common" layout from a platform, not a easy task. I say that cause I read thing like "the C ABI mandates on that particular target" C mandate nothing or any particular target about ABI.

Also, BTW MSVC is not a C compiler compliant "MSVC is not a C compiler" is a running gag in stackoverflow C community.

[oli-obk]

Small update after checking in with some very helpful MSVC folk:

• int arr[0] is treated as int* arr due to legacy reasons
• it causes problems within MSVC C++ itself
• it is unlikely to ever change due to the fact that it would break their own ABI

So, basically, considering this feature is

• broken on clang in MSVC compat mode
• broken in rustc in #[repr(C)] while compiling for MSVC
• broken in MSVC itself during C++ interop
• not really clear whether it's the right thing to do for clang and gcc

Maybe we should not look into fixing this at all, but instead start reporting a future incompatibility error and strongly suggest people move away from it, irrespective on what platform they are compiling against? Even if we never make it a hard error, a strongly worded message like this may help move people away from it.

So in short: [T; 0] could be made not FFI safe and thus not legal within repr(C) structs or extern function signatures.

[talchas]

So in short: [T; 0] could be made not FFI safe and thus not legal within repr(C) structs or extern function signatures.

Doing this in general would break the most obvious way to represent flexible array member (either the spec-supported char x[] or the old char x[0]) in rust, which MSVC supports, or the align hack usage. MSVC only breaks for the case of a struct with nothing but the single zero-length array. Loudly warning /that/ seems fine though.

[RalfJung]

(prepare for more accidental closures of this issue when that commit lands on the beta and stable branches -- at least that's how things went in the past)

[RalfJung]

This issue would probably benefit from being split up into one issue per problem following this summary as well as this comment.

[RalfJung]

Looking at it, it actually seems to be largely two issues -- enums with too big discriminants, and then everything around size 0. I have opened #124403 for the enums, so this issue is now about the case of structs / unions with no fields / zero-sized fields.

[RalfJung]

In general it probably makes sense to consider this together with #100743, in the wider question of -- how do we deal with the differences between MSVC's layout algorithm and the one everyone else uses. There seem to be two differences that surfaced so far: how to deal with structs/unions where all fields have size 0, and how to deal with explicitly aligned types occurring as (potentially nested) fields of packed types.

1. Make enough things hard errors so that what is allowed is consistent between GCC and MSVC. For aligned-field-in-packed, this is what we tried to do, but we failed at that. For zero-sized cases, this would require disallowing a bunch of things, like repr(C) unit structs and repr(C) generic newtpyes wrapping an arbitrary T (since the newtype could be wrapping an empty array). We could also do these checks during monomorphization, then we can allow newtpyes, but monomorphization-time checks are generally not great.
2. Compute layout using GCC/clang rules, but emit a lint for cases where that differs from MSVC. This would have to be either best-effort or monomorphization-time though, due to the problems mentioned above with generics. (We have precedent for a monomorphization-time lint: the lint that detects "big copies".)
3. Compute layout using the rules of the dominant C compiler for a target, i.e., use MSVC rules for MSVC targets. This could still be combined with a lint for cases where layout differs from what it would be on GCC.

Option 1 seems unlikely as there's a lot of code we'd have to exclude to be sure that we are in the fragment where all our current targets agree. Option 1 is also extremely unsatisfying for the aligned-field-in-packed-struct case as it leaves a gap in what can be expressed with repr(C) types in Rust.

Option 3 seems closer to the original goal of repr(C) to me, in particular given that repr(crabi) is in the works for defining a deterministic cross-platform layout algorithm. But option 3 would be a breaking change for certain types which could make it impossible.