E0587 error on packed and aligned structures from C #59154

ahomescu · 2019-03-13T04:40:04Z

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.

The text was updated successfully, but these errors were encountered:

jonas-schievink · 2019-03-13T10:25:28Z

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

TheDan64 · 2019-03-14T18:50:56Z

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 · 2019-03-25T11:34:25Z

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 · 2019-03-25T22:54:11Z

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 · 2019-03-25T23:12:49Z

@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 · 2019-03-26T00:22:06Z

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 · 2019-03-26T00:45:05Z

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 · 2019-03-26T03:21:52Z

(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 · 2019-03-26T13:40:30Z

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.

retep998 · 2019-03-26T16:12:15Z

On the contrary I think Windows is the common use case and repr(packed, align(N)) should only be allowed there with VC++ semantics until a decision on other targets is made.

rinon · 2019-03-26T19:04:53Z

I'm not sure why you think Windows is necessarily the common case? The initial example above is from the Linux kernel headers and is blocking development of Rust kernel modules that use that and other similar structs. We need some way of being ABI compatible with the Linux kernel for the same reason that we need to be ABI compatible with VC++ libraries.

retep998 · 2019-03-26T19:30:18Z

I'm not sure why you think Windows is necessarily the common case?

For the same reason that some people think non-Windows is the common case.

More seriously, I am opposed to implementing either behavior until the lang team decides on a solution that allows both behaviors to be used. I want this to be resolved just as much as you do, but I'm not willing to throw some platforms under the bus for the sake of other platforms.

rinon · 2019-03-26T20:49:03Z

I think the point was that there is only an ABI ambiguity on Windows and the ABI behaviour doesn't necessarily need to be consistent across platforms. Defining the same semantics for packed+aligned on both pc-windows-gnu or pc-windows-msvc would be a problem, but leaving those targets undefined as they currently are and defining default semantics for other targets would at least allow this combination to be used in cases where there is no ambiguity in ABI implementations. Windows would be no worse off than currently and other platforms would be able to interface with packed+aligned structs.

All that said, though, it sounds like we do need to start an RFC on defining a syntax for controlling layout for the combination of packed structs and aligned fields.

joshtriplett · 2019-03-26T23:20:39Z

We discussed this on Discord, and here's a rough sketch of some ideas and next steps:

It looks like clang matches the Windows ABI for x86_64-pc-windows-msvc and matches the GCC ABI for x86_64-pc-windows-gnu.
Ideally, the MinGW ABI should match the Windows ABI to allow calling system libraries.
- Someone should reach out to the MinGW folks, and find out if they'd be willing to match the Windows ABI as defined by Visual C++ to allow interoperability with system libraries.
repr(C) must interoperate with the C compiler for the target. So, if we can't get -gnu to use the same ABI as -msvc, we need to match what -gnu does when targeting -gnu.
However, we also need a way to interoperate with the system libraries.
We already have https://doc.rust-lang.org/beta/nomicon/ffi.html#foreign-calling-conventions to specify different calling conventions for functions in an extern block.
How about adding repr(system) to designate a structure as following the same convention as system libraries, which on Windows will mean following the Windows ABI even on -gnu targets.

SamB · 2022-09-07T15:38:45Z

Did anyone talk to the MinGW and/or GCC people about the ABI issue yet? If so, links to relevant conversations would be good.

…nner packed struct, as Rust doesn't allow packed and aligned types (see rust-lang/rust#59154).

…uct (#233) Align by aligning a wrapper struct around an inner packed struct, as Rust doesn't allow packed and aligned types (see rust-lang/rust#59154).

bertschingert · 2024-01-19T02:58:51Z

We've been discussing this issue in the bcachefs IRC recently because it's become a blocker for continuing to integrate Rust more into bcachefs. It seems like the thing holding it back is the difference in behavior between platforms.

From the example given it seems like the difference comes down to whose alignment wins when you have a parent/outer struct with an embedded child struct. One compiler uses the child struct's alignment, the other the parent.

One idea we discussed: what about creating a version of the #[repr(align(N))] attribute to specify which semantics to use? For example, a new attribute: #[repr(align_override_packed(N))] that can be used when the behavior of the child overriding the parent alignment is desired?

#[repr(align(N))] #[repr(packed)] can be used together to get the opposite behavior, parent/outer alignment wins.

The exact names don't matter at this point--just that we can come up with something reasonable to move forward. I'd appreciate feedback on this idea.

Also, I take it this will require an RFC at some point to either introduce a new attribute, or change the behavior of the current attributes? I am willing to try to get that started by initiating a pre-RFC discussion, if that'd be helpful.

RalfJung · 2024-04-25T06:02:14Z

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.

They should hit a different error, no? "packed type cannot transitively contain a #[repr(align)] type" is not the same error as "conflicting packed and align representation hints". There's another bug for the "cannot contain" issue: #100743.

@bertschingert @ojeda does bcachefs/Rust-for-Linux need #59154 or #100743?

This was referenced Mar 13, 2019

E0587 error on packed and aligned structures from C rust-lang/rust-bindgen#1538

Open

Fixed invalid module format & recent nightly support fishinabarrel/linux-kernel-module-rust#67

Closed

jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints A-ffi Area: Foreign Function Interface (FFI) labels Mar 13, 2019

pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 13, 2019

retep998 added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Mar 26, 2019

TheDan64 mentioned this issue Apr 19, 2019

Structs which are both packed and aligned fail to compile immunant/c2rust#75

Closed

emilio mentioned this issue May 2, 2019

repr(C,packed(4)) type generated that transitively contains repr(align(4)), causing a build error rust-lang/rust-bindgen#1556

Closed

This was referenced Dec 27, 2019

Rust 2020: Lessons learned by transpiling C to Rust guevara/read-it-later#6061

Open

Rust 2020: Lessons learned by transpiling C to Rust guevara/read-it-later#6062

Open

Rust 2020: Lessons learned by transpiling C to Rust guevara/read-it-later#6063

Open

crlf0710 added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jun 11, 2020

caizixian mentioned this issue Jun 13, 2021

Add perf event support mmtk/mmtk-core#315

Merged

robe2 mentioned this issue May 2, 2023

towards mingw/msys2 windows support: type has conflicting packed and align representation hints pgcentralfoundation/pgrx#1132

Open

kkysen mentioned this issue May 8, 2023

Fix missing aligns memorysafety/rav1d#232

Merged

kkysen added a commit to memorysafety/rav1d that referenced this issue May 8, 2023

struct refmvs_block: Align by aligning a wrapper struct around an i…

f776605

…nner packed struct, as Rust doesn't allow packed and aligned types (see rust-lang/rust#59154).

kkysen mentioned this issue May 8, 2023

struct refmvs_block: Align a wrapper struct for an inner packed struct memorysafety/rav1d#233

Merged

This was referenced Dec 12, 2023

Packed&aligned structs? #73988

Closed

#[repr(packed(N))] (tracking issue for RFC 1399) #33158

Closed

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

Open

koverstreet mentioned this issue Jan 11, 2024

Rust dependency updates & Debian koverstreet/bcachefs-tools#202

Closed

ojeda mentioned this issue Jan 19, 2024

bindgen wanted features & bugfixes Rust-for-Linux/linux#353

Open

21 tasks

ChrisDenton mentioned this issue Jul 30, 2024

Incorrect field offset when using #[repr(C, packed(8))] #128373

Closed

kurtmcmillan mentioned this issue Aug 7, 2024

Incorrect field offset when C struct is __attribute__((packed, aligned(8))) rust-lang/rust-bindgen#2878

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

E0587 error on packed and aligned structures from C #59154

E0587 error on packed and aligned structures from C #59154

ahomescu commented Mar 13, 2019

jonas-schievink commented Mar 13, 2019 •

edited

Loading

TheDan64 commented Mar 14, 2019

Stargateur commented Mar 25, 2019

retep998 commented Mar 25, 2019

Stargateur commented Mar 25, 2019

retep998 commented Mar 26, 2019 •

edited

Loading

Stargateur commented Mar 26, 2019

retep998 commented Mar 26, 2019 •

edited

Loading

TheDan64 commented Mar 26, 2019

retep998 commented Mar 26, 2019

rinon commented Mar 26, 2019

retep998 commented Mar 26, 2019

rinon commented Mar 26, 2019

joshtriplett commented Mar 26, 2019

SamB commented Sep 7, 2022

bertschingert commented Jan 19, 2024

RalfJung commented Apr 25, 2024

E0587 error on packed and aligned structures from C #59154

E0587 error on packed and aligned structures from C #59154

Comments

ahomescu commented Mar 13, 2019

jonas-schievink commented Mar 13, 2019 • edited Loading

TheDan64 commented Mar 14, 2019

Stargateur commented Mar 25, 2019

retep998 commented Mar 25, 2019

Stargateur commented Mar 25, 2019

retep998 commented Mar 26, 2019 • edited Loading

Stargateur commented Mar 26, 2019

retep998 commented Mar 26, 2019 • edited Loading

TheDan64 commented Mar 26, 2019

retep998 commented Mar 26, 2019

rinon commented Mar 26, 2019

retep998 commented Mar 26, 2019

rinon commented Mar 26, 2019

joshtriplett commented Mar 26, 2019

SamB commented Sep 7, 2022

bertschingert commented Jan 19, 2024

RalfJung commented Apr 25, 2024

jonas-schievink commented Mar 13, 2019 •

edited

Loading

retep998 commented Mar 26, 2019 •

edited

Loading

retep998 commented Mar 26, 2019 •

edited

Loading