E0587 error on packed and aligned structures from C

[ahomescu]

When converting C structures that are both packed and aligned using either C2Rust or bindgen, such as:

struct xregs_state {
	struct fxregs_state		i387;
	struct xstate_header		header;
	u8				extended_state_area[0];
} __attribute__ ((packed, aligned (64)));

the code that either tool produces looks something like:

#[repr(C, packed(64))]
#[repr(align(64))]
pub struct xregs_state {
    pub i387: fxregs_state,
    pub header: xstate_header,
    pub extended_state_area: __IncompleteArrayField<u8>,
}

This Rust code fails to compile due to error E0587:

error[E0587]: type has conflicting packed and align representation hints
    --> .../out/bindings.rs:3894:1
     |
3894 | / pub struct xregs_state {
3895 | |     pub i387: fxregs_state,
3896 | |     pub header: xstate_header,
3897 | |     pub extended_state_area: __IncompleteArrayField<u8>,
3898 | | }
     | |_^

We can work around this in C2Rust by emitting an aligned outer/packed inner structure pair (I think bindgen could do the same), but I'm wondering if it would be better to fix this on the Rust language/compiler side.

[jonas-schievink]

The error message could be improved to say why these hints are incompatible (that is, if this error shouldn't be removed outright)

[TheDan64]

Note that the aligned outer/packed inner workaround is still really limited. If the struct contains any struct fields with their own alignment, then you'll still hit this error. (The xregs_state example does have this exact problem, as its i387: fxregs_state field has an alignment of 16)

So I think it makes sense for rust to support this for repr C/FFI purposes at the very least.

[Stargateur]

note that u8 extended_state_area[0]; is not valid C code according to standard and should not be used in 2019, u8 extended_state_area[]; instead.

[retep998]

Having structs with repr(align(N)) (transitively) inside a struct that is repr(packed(N)) is currently not allowed because gcc and VC++ differ in their behavior. Given:

#[repr(C, align(4))]
struct Foo(u8);
#[repr(C, packed(1))]
struct Bar(Foo);

What is align_of::<Bar>()? If we follow gcc's behavior it is 1, but if we follow VC++'s behavior it is 4. Things get even hairier when you consider MinGW which is gcc and therefore has gcc behavior despite targeting Windows which should obey the C ABI dictated by VC++.

[Stargateur]

@retep998

you consider MinGW which is gcc and therefore has gcc behavior despite targeting Windows which should obey the C ABI dictated by VC++.

Actually no, gcc can do whatever it want even targeting windows, the "only" requirement is that the final program work. Also, a structure defined in user side can only be use by user side, so gcc don't have to follow msvc rule to work on window in the user side. OFC, when gcc need to call Windows API they need to follow their rules, also note I don't talk about msvc rules but those of Windows API (they have probably the same anyway).

All of this to say, C ABI is not a standard thing, __attribute__ ((packed, aligned (64))) is clearly not defined by the standard and compiler are allow to have extension, there are totally implemented behavior.

[retep998]

Actually no, gcc can do whatever it want even targeting windows, the "only" requirement is that the final program work.

If MinGW compiled binaries want to do anything they have to be able to call into dlls compiled by VC++, which means they have to match the ABI that VC++ uses. Whether the C ABI is standardized or whether the language features in question are even standard is irrelevant. If there is a dll out there that uses #pragma pack in combination with __declspec(align(N)) then you need to be able to call into it regardless of whether you are in the VC++ world or the MinGW world.

Having pc-windows-msvc and pc-windows-gnu differ in their behavior around this would be a massive footgun, but at the same time having pc-windows-gnu differ from all other -gnu targets would also be a footgun. The lang team hasn't decided on a solution to this yet, and so this feature combination remains illegal.

[Stargateur]

Yes, but all of this is implemented side, C doesn't care of all of that, dll is not a thing in C standard, if MinGW want to handle dll produce by VC++ is their problem not C problem. I disagree about Rust trying to bet about what all C compiler should produce to have a FFI interface. C is design to be very flexible on what a compiler can do. So you are hurting a wall. Unless you want something like #[repr(C, gcc, 5.6, linux)], I don't think you can find a magic solution. It's look like you blame gcc for this even if gcc is allowed to do this even on windows and still be a valid C compiler.

The "best guess" is already a very good thing to have in the Rust side. I find that Rust don't have the ressource to handle every case of every implementation of C (specially with Microsoft, MSVC is not a true C compiler)

[retep998]

(specially with Microsoft, MSVC is not a true C compiler)

What is or isn't a "true" C compiler is not important nor is it relevant.

Rust needs to be able to interface with system libraries, which means that Rust needs to match the ABI used by system libraries. Therefore on Windows Rust has to match the VC++ ABI because that is the ABI used by system libraries. If MinGW differs from this then Rust is in a tight spot because pc-windows-gnu still needs to be able to match the ABI used by system libraries.

In my opinion, having the behavior of mixing repr(packed(N)) and repr(align(N)) differ based on target is a huge footgun due to pc-windows-gnu. Instead I'm in favor of allowing the user to explicitly choose which behavior they want, whether that be through alternative versions of repr(align(N)) and repr(packed(N)) or some other method.

[TheDan64]

What if repr(packed, align(N)) was allowed to compile on every target except windows? And it just remained an error there until a decision on windows was made? This would at least open it up to the most common use cases while not committing to a windows representation.