#[repr(align(N))] fields not allowed in #[repr(packed(M>=N))] structs

I tried this code:

use std::sync::atomic::AtomicU32;

#[repr(C, packed(4))]
struct Foo {
    bar: AtomicU32,
}

I expected that #[repr(align(4))] fields, such as AtomicU32, would be allowed inside a #[repr(packed(4))] struct, since the minimum alignment is >= the required alignment.

However, the compiler complains:

error[E0588]: packed type cannot transitively contain a `#[repr(align)]` type

Use case: I have some fixed-layout shared memory structs I need to interact with, which sometimes contain unaligned u64s. I also need to use atomics on other parts of them (hopefully only u32s).

Meta

rustc --version --verbose:

rustc 1.65.0-nightly (34a6cae28 2022-08-09)
binary: rustc
commit-hash: 34a6cae28e7013ff0e640026a8e46f315426829d
commit-date: 2022-08-09
host: x86_64-unknown-linux-gnu
release: 1.65.0-nightly
LLVM version: 14.0.6

That entire check is anyway somewhat odd since it is easily circumvented:

use std::sync::atomic::AtomicU32;

#[repr(C, packed(4))]
struct FooT<T> {
    bar: T,
}

type Foo = FooT<AtomicU32>;

fn main() {
    let x: Foo = Foo { bar: AtomicU32::new(0) };
}

So this affects bcachefs - it's a blocker for using Rust more extensively there.

In C, specifying packed and aligned will round up the size of the type to the alignment specified, so without this fixed it means we can't generate types in Rust that match the C types for our on disk data structures.

In C, specifying packed and aligned will round up the size of the type to the alignment specified,

That's not universally true, GCC and MSVC have different behavior. See the discussion here. So accepting this code in Rust would be a major interop footgun.

To make progress on this, we need a concrete proposal for what the exact behavior of align(N) fields in packed structs should be, taking into account this problem and also the fact that some code may already exist on stable that could change behavior if we adjust the layout. The comment here could be a good starting point for a proposed solution -- making the resulting layout depend on the target triple (windows-gnu vs windows-msvc).

So, if this is a blocker for someone, I suggest looking into sketching a pre-RFC. :)

No, making it depend on the target triple would not be a good solution - this is important for on the wire formats, the behavior has to be explicitly controllable. Just make it another #[repr] thing.

And this does need to be fixed; it's a blocker for C FFI. We've worked around it for now in bindgen for bcachefs, but that was a hack and we've been waiting on compiler guys to fix this properly.

You cannot have both, use it for C FFI and have it fixed independent of the target triple. That's the entire point. So pick one.

And given that the repr is called C, and a stable platform-independent repr is in the works (#105586), it seems to me that the primary role of repr(C) is to match the dominant C compiler of the given target. If that means it differs between targets, then that's unfortunate -- but it can't really be helped.

"C FFI" is a wider issue than just communicating with code linked in to the same binary. If an on disk format is defined in C, it's a bug if we cannot specify the correct representation.

The existing silent different in behaviour between gcc and Microsoft is a bug, and it's perfectly straightforward to fix it here, but you're insisting that it 'can't be helped'. No, you're the one who's got this wrong.

"Match the host" should be one option, but it should be specified, per data type, not silently guessed at.

The existing silent different in behaviour between gcc and Microsoft is a bug,

What's the basis for that claim? Who's wrong, and where's the bugreport? It's not like C has defined struct layout. "Whatever the major C compiler for that target does" is the definition of correct behavior. windows-gnu and windows-msvc are already different ABIs, you can't just call functions across that boundary anyway.

There's no basis for demanding that repr(C) be the same across different targets. And of course it already is not the same due to pointer size and endianess differences. u64 also has different alignment on different targets, meaning that #[repr(C)] struct S(u32, u64) has different size on different targets. So trying to use repr(C) as an exchange format is already misguided, there's no point in trying to keep up the pretense that it could be a portable layout.

It's also unclear what you are suggesting. Match GCC or match MSVC? I hope you're going to be there to explain to people how they are wrong when they file bugreports that Rust code struct layout doesn't match what their C compiler does? Your attitude isn't helpful here.

The whole point of packed and aligned is to be able to precisely specify the layout of a given struct.

If we have different inconsistent implementations - do you not see the issue? There's no reason Rust has to continue this brokenness - all you have to do is make it possible to specify, on a per type basis, which behavior to emulate.

If rust doesn't do the same thing as msvc for -msvc targets and as gcc on -gnu targets Rust is broken and will be considered as such by most users. If you want this behavior to change you should take it up with gcc or msvc (depending on which you consider "correct").

I think this is getting off topic.

Yes, an repr that allows specifying a consistent layout across platforms would absolutely be great. But that's not what repr(C) is and repurposing it for that would not only be a breaking change, it would leave us with the opposite problem (no way to use the system's C layout).

No, this wouldn't be repurposing repr(C)

repr(gcc_c)
repr(microsoft_c)
repr(system_c_compiler)

repr(gcc_c)
repr(microsoft_c)
repr(system_c_compiler)

And then repr(C) means "whatever the representation of the current target is".

Being able to request repr(gcc_c) on an MSVC target is an interesting feature request, but somewhat orthogonal to fixing repr(C) to match the current target.

@CAD97

This comment by @CAD97 seems relevant

Should we change #[repr(C)]? I'm split. I'm no longer as against it as I was when I thought the change was more widely applicable, but there still is the fact that a lot of code has been written (and generated) assuming the current #[repr(C)] rules, and there's no way to tell whether they expect the "match gcc on Linux, clang on macOS, and MSVC on Windows" behavior, or the current #[repr(simple)] behavior (which matches clang and gcc).

For this issue in particular, the change to using the MSVC layout on MSVC targets would be almost unobservable, except for code that uses generics to bypass the restriction of align fields in packed structs.

However, as also noted in that comment, implementing MSVC semantics would require adding a new notion of alignment -- one that trumps outer packed, somehow. I don't even know what the algorithm is.

An RFC has been proposed to lift this limitation: rust-lang/rfcs#3718

Navigation Menu

Repository navigation

#[repr(align(N))] fields not allowed in #[repr(packed(M>=N))] structs #100743

Meta

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

#[repr(align(N))] fields not allowed in #[repr(packed(M>=N))] structs #100743

Description

Meta

Activity

RalfJung commented on Aug 28, 2022

koverstreet commented on Dec 12, 2023

RalfJung commented on Dec 13, 2023

RalfJung commented on Apr 25, 2024

koverstreet commented on Apr 25, 2024

koverstreet commented on Apr 25, 2024

RalfJung commented on Apr 25, 2024

RalfJung commented on Apr 25, 2024

koverstreet commented on Apr 25, 2024

RalfJung commented on Apr 25, 2024

koverstreet commented on Apr 26, 2024

onestacked commented on Apr 26, 2024

ChrisDenton commented on Apr 26, 2024

koverstreet commented on Apr 26, 2024

koverstreet commented on Apr 26, 2024

RalfJung commented on Apr 26, 2024

RalfJung commented on Apr 26, 2024

RalfJung commented on Nov 1, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions