Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aligned-in-packed restriction can be circumvented with generic parameters #80926

Open
mahkoh opened this issue Jan 11, 2021 · 7 comments
Open
Labels
A-layout Area: Memory layout of types C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@mahkoh
Copy link
Contributor

mahkoh commented Jan 11, 2021

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=596ca7ba423417b421439fa86cc57e9c

See also rust-lang/rfcs#3060

The effective layout seems to correspond to the layout used on non-MS platforms. This is quite useful to circumvent the restriction and I'll have to make use of it to interact with certain C apis as described in the RFC issue. So please don't remove it.

@mahkoh mahkoh added the C-bug Category: This is a bug. label Jan 11, 2021
@jonas-schievink
Copy link
Contributor

I think this works as intended? Packed representation ignores field alignment requirements

@jonas-schievink
Copy link
Contributor

Ah, apparently not:

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

Not sure what the rationale is there

@jonas-schievink jonas-schievink added A-layout Area: Memory layout of types T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jan 11, 2021
@nagisa
Copy link
Member

nagisa commented Jan 12, 2021

what the rationale is there

rust-lang/rfcs#1358 (comment)

@nagisa
Copy link
Member

nagisa commented Jan 12, 2021

So please don't remove it

Do bear in mind that you are relying on what is explicitly been considered an unspecified behaviour. While this loophole existing does introduce some soft constraints for when the interaction between packed and align needs to be defined, it is not necessarily the case that Rust will be super hard-pressed to maintain the current behaviour.

@mahkoh
Copy link
Contributor Author

mahkoh commented Jan 12, 2021

Do bear in mind that you are relying on what is explicitly been considered an unspecified behaviour.

Nevertheless it is exactly the behavior documented in the reference if the sentence disallowing it were omitted.

@RalfJung RalfJung added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 31, 2021
@RalfJung
Copy link
Member

RalfJung commented Aug 28, 2023

I feel like we should just remove the aligned-in-packed check. There's probably already code out there relying on its current behavior via generics (even accidentally). And I don't think the behavior is particularly surprising either, packed acts on align types the same way it acts on all types -- it completely ignores their alignment.

Having types that can be written with generics but not monomorphic is obviously inconsistent, and means the check can at best serve as a lint anyway. We had a similar situation with repr(transparent) and that got fixed by also allowing the attribute on types where all fields are 1-ZST.

I wonder what @rust-lang/lang thinks about this -- the original goal was to avoid having to decide either way for the interaction of packed and align. Different C implementations seem to behave differently: on MSVC, __declspec(align(N)) types are fully aligned even in a packed struct; in GCC __attribute__((aligned(N))) types behave like all other types in a packed struct. But now that we inadvertently already made a decision to behave like GCC, is it really worth keeping the restriction?

@RalfJung RalfJung added the I-lang-nominated The issue / PR has been nominated for discussion during a lang team meeting. label Aug 28, 2023
@pnkfelix
Copy link
Member

Discussed at T-lang triage meeting.

  1. We think the restriction can be safely lifted, in the manner described by @RalfJung above.
  2. We would not object if the restriction were readded in the form of a best-effort lint. I.e., we do not see it as a downside if people are expected to explicitly add an #[allow] in the cases where they are putting an aligned type in a field of a packed struct, so that casual readers of the code will know something funky is going on.

@rustbot label: -I-lang-nominated

@rustbot rustbot removed the I-lang-nominated The issue / PR has been nominated for discussion during a lang team meeting. label Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout Area: Memory layout of types C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants