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

Meeting proposal: When accessing a field that is at a "very aligned" offset, can the required alignment be higher than the type of the field indicates? #11

Closed
RalfJung opened this issue Jul 28, 2023 · 3 comments
Labels
meeting-proposal Proposal for a discussion topic at a team meeting

Comments

@RalfJung
Copy link
Member

RalfJung commented Jul 28, 2023

Summary

Sometimes when accessing a field, the alignment requirement is lower than the type of the field might indicate: specifically, this happens for fields of repr(packed) structs.

Can the alignment requirement ever be higher? For instance, in this type

#[repr(C)]
struct S { x: u8, y: u32 }

the x field will always be at a 4-aligned address when occurring inside a well-aligned S instance. Therefore, Miri will make (*ptr).x UB if that load is not 4-aligned. This matches codegen which generates an align 4 load in this example.

Is that what we want, or should the alignment requirements of a field access in a struct always be upper-bounded by the alignment of that field?

Reading

https://hackmd.io/rCcTOjQ4SnOb9I2Zt8SqZQ

Comment policy

These issues are meant to be used as an "announcements channel" regarding the proposal, and not as a
place to discuss the technical details. Feel free to subscribe to updates. We'll post comments when
reviewing the proposal in meetings or making a scheduling decision. In the meantime, if you have
questions or ideas, ping the proposers on Zulip (or elsewhere).

@RalfJung RalfJung added the meeting-proposal Proposal for a discussion topic at a team meeting label Jul 28, 2023
@RalfJung RalfJung changed the title When accessing a field that is at a "very aligned" offset, can the required alignment be higher than the type of the field indicates? Meeting proposal: When accessing a field that is at a "very aligned" offset, can the required alignment be higher than the type of the field indicates? Aug 19, 2023
@RalfJung
Copy link
Member Author

I will write up an FCP proposing to limit alignment requirements to field type. There wasn't a clear consensus though so we might have to keep discussing asynchronously.

The main issue is that we don't know what kind of code out there would be affected by this, and how tricky it would be to get back the lost alignment assumption in case someone needs it. In monomorphic code a read_aligned<const ALIGN: usize> operation goes a long way, but in generic code that could be tricky.

@RalfJung
Copy link
Member Author

Actually while trying to write up the current rules for rust-lang/reference#1387 I realized there is a way to state them that isn't even that complicated. So I am no longer convinced we want to allow these examples. I do think we want to treat all 3 examples consistently, but there's a way to disallow them that's not so bad...

@RalfJung
Copy link
Member Author

RalfJung commented Aug 25, 2023

As the one who proposed this meeting, and given the voices during the meeting that said they were worried about allowing this code (which has no clear motivation to be allowed besides "the rules are confusing"), only to realize later that we'd really like the optimization potential enabled by the UB -- I no longer think I want to change anything here, for now. If rust-lang/reference#1387 lands as-is, I think am happy with our alignment rules. (I'll know for sure after implementing this in Miri, but the MiniRust implementation makes me optimistic.)

So, I don't think I will be going for any FCP here. (An FCP for "change nothing" just seemed odd.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meeting-proposal Proposal for a discussion topic at a team meeting
Projects
None yet
Development

No branches or pull requests

1 participant