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

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](https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=b0313f404815ab198fbcee5b2d10d42d).

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?

### Examples

#### Example 1

Is this code UB?

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

unsafe fn foo(x: *const S) -> u8 {
    unsafe { (*x).x }
}

fn main() { unsafe {
    let mem = [0u64; 16];
    let odd_ptr = std::ptr::addr_of!(mem).cast::<u8>().add(1);
    // `odd_ptr` is now not aligned enough for `S`.
    // If accessing field `x` can exploit that it is at offset 0
    // in a 4-aligned struct, that field access requires alignment 4,
    // thus making this UB.
    foo(odd_ptr.cast());
} }

#### Example 2

What about this code?

#![feature(strict_provenance)]
#[repr(align(16))]
pub struct S { f: [f32; 4] }

unsafe fn bar(x: *const S) -> f32 {
    assert!(x.addr() % 4 == 0); // it *is* 4-aligned
    unsafe { (*x).f[0] }
}

fn main() { unsafe {
    let mem = [0u64; 16];
    let non_16_aligned_ptr = std::ptr::addr_of!(mem).cast::<u8>().add(4);
    // `bar` might want to exploit that `S` objects are always 16-aligned,
    // e.g. to do some vector instruction shenanigans.
    // But then this call should be UB.
    bar(non_16_aligned_ptr.cast());
} }

The latter example is inspired by [this issue](https://github.com/rust-lang/rust/issues/54028).
However that issue did not use raw pointers; we could exploit that local variables of type `S` must be 16-aligned to still generate aligned loads in that case.

#### Example 3

#[repr(C, align(16))]
pub struct Aligned {
    _pad: [u8; 11],
    packed: Packed,
}
#[repr(packed)]
pub struct Packed {
    _pad: [u8; 5],
    x: u64,
    y: u64,
}

For a pointer `ptr: *mut Aligned`, what alignment does `(*ptr).packed.x` require? Currently codegen and MiniRust agree it is just 1, but if `ptr` was a reference we would be guaranteed that `x` is 16-aligned.
So allowing this example violates the idea that if a load is guaranteed to have a certain alignment when being done based on a reference, then the same alignment is required when using a raw pointer.
(Miri requires 16, but that is based on [rules we want to revise](https://github.com/rust-lang/reference/pull/1387): aligned is checked when `*src` happens, before any of the projections occur. Once that lands, Miri will also require just 1.)

### Alternatives

If we want to allow all of these examples, we could change the rule for computing the alignment requirement of a field obtained via place projection. Given a base place with alignment `base_align` and a field with offset `offset` and field type with alignment requirement `field_align`, currently we do

- `base_align.restrict_for_offset(offset)`

This rule implies that alignment is violated for examples 1 and 2, but not for 3!

We could instead do

- `base_align.restrict_for_offset(offset).min(field_align)`

Then alignment would not be violated for any of the examples.

Note that just `base_align.min(field_align)` is insufficient as demonstrated by the [example in this PR](https://github.com/rust-lang/rust/pull/53998): the field storing an enum discriminant is special and can actually be *less* aligned than its type indicates, which is okay since a reference to it can never be created.

Disallowing the last example requires a lot more complicated rules for what happens with a place during field projections, to keep track of known remainders modulo some power of 2.

### Status quo

MiniRust currently declares examples 1 and 2 UB, but allows example 3.
Miri, adjusted for [the decision to remove the UB on raw pointer deref](https://github.com/rust-lang/reference/pull/1387), behaves the same.

And codegen also behaves like that.
Making examples 1 and 2 UB happened in an effort to fix [this issue](https://github.com/rust-lang/rust/issues/54028), however, note that that issue did not involve raw pointers.
There was [not a lot of discussion in the PR that changed this](https://github.com/rust-lang/rust/pull/54547), so it seems likely that it was not realized that this PR introduced extra UB for some raw pointer accesses.
Miri and MiniRust just copied what codegen is doing.

### Pros and Cons

Making any of these examples UB has the big downside that we'll need to say something very complicated in the reference.
[After this PR](https://github.com/rust-lang/reference/pull/1387), it just says that it is UB to

> Accessing (loading from or storing to) a place that is dangling or unaligned.

We'd have to add a careful explanation here of what "unaligned" means: the required alignment is computed by looking at all the field projections that lead to the current place, and starting with the original place, restricting the known maximal alignment with each projection until arriving at the place that is being loaded from.
(In the spec we still need something complicated here to support fields of packed structs. However, that something complicated is only needed to explain why certain code *is allowed*. Unexpectedly *removing* UB is a lot better than complicated rules unexpectedly *adding* UB.)
Given the history that lead us to removing the UB on raw pointer derefs (delaying it to the load/store instead), the author thinks that having this kind of subtle UB is just not sustainable -- people will never guess that these are the rules they have to follow.

Note that even if we always restrict alignment to the field type, we can *still* inform LLVM about higher alignment on loads from references -- those must be fully aligned always.
So only raw pointer code can potentially lose any optimizations here.
(For codegen this would mean tracking whether a place is derived from a "safe" source, such as a local or deref of a reference, or an "unsafe" source, such as a raw pointer, and only emitting the stronger alignment requirement for "safe" sources.)

So, the downsides of making any of these examples UB are big.
The upsides are slim: code that wants the higher alignment annotations in the above examples can simply cast the raw pointer to a reference *before* projecting (as in, create a reference from the base pointer, not the field pointer).

Overall, the author considers this to be another case of aligning the UB rules with programmer expectations: the extra UB is too surprising, too complicated, and not useful enough to justify its existence.
The spec technically gets a bit less pretty when allowing these examples, but it's worth it for reducing the risk of UB in real-world code.

# Questions (during the meeting)

## cast to reference timing

> The upsides are slim: code that wants the higher alignment annotations in the above examples can simply cast the raw pointer to a reference *before* projecting.

Mario: It could also cast the raw pointer to a reference *after*, since it's just an arithmetic fact about the address being derived, no?

Ralf: If there's no control flow, sure. What I meant is that it is the base pointer that must be cast to a reference, not the field pointer.

## repr(Rust) field reorderings

Connor: A basic implementation of `repr(Rust)` structs is to sort fields by alignment. This may cause a field that was highly aligned in `repr(C)` to become less aligned in `repr(Rust)` (or in edge cases, the opposite). Should there be more possible UB using `repr(C)` than in (potential reorderings of) `repr(Rust)`?

Example of reordering to lessen field alignment:

pub struct Foo(u8, u32, u16);

With `repr(C)` layout, all 4 fields are 4 aligned in offset, but with `repr(Rust)` layout (according at least to the basic algorithm of stable-sort by descending alignment), only .1, and .2 are (and `u8` is 2 aligned -> the layout becomes `[1---2-0P]`).

Example of reordering that increases field alignment:

pub struct Bar(u32, u8, u8, u16);

.3 is reordered to right after .0 using the basic sorting algorithm, so it's offset-alignment increases from 2 to 4.

## Under-aligned fields outside of `#[repr(packed)]`

Jakob: The text claims

> Note that just base_align.min(field_align) is insufficient as demonstrated by the example in this PR: the field storing an enum discriminant is special

The existence of the discriminant as a field seems like a rustc-ism to me, not something we should write into the spec. Are there any examples of this occurring in cases that are actually inherent to the semantics?

## Replaces surprising UB with surprising deoptimization

TC: One thing worth pointing out is that allowing these examples does replace a case of suprising UB with a surprising deoptimization; I'm not sure people would expect that the raw pointer versions would have worse optimization than casting to references.

It wasn't about raw pointers, but the issue [#54028](https://github.com/rust-lang/rust/issues/54028) was about a surprising missed optimization rather than surprising UB.

Ralf: That was surprising missed optimization *on local variables* that were just allocated with sufficient alignment. I'm not sure we can extrapolate from there to raw pointer cases.

Jakob:

let _r = (*x).x;
// But this already didn't work
let p2 = (x + offset_of!(S.x)).cast();
let _r = *p2;

Mario: If people writing unsafe code have been listening to our advice, they might be using raw pointers everywhere rather than using references.

## Bringing old behavior back in explicit/targetted way

pnkfelix: If we do this (get rid of the UB cases for unsafe pointers), will there be a way for someone to re-express the alignment constraint to bring back better codegen for code using unsafe pointers? What will it look like? (New methods asserting the alignment on `ptr`, for example?)

Ben: Do you mean like `intrinsics::assume`?

Ralf: We could have `fn read_aligned<T, const ALIGN: usize>(ptr: *const T) -> T`.
- Connor: In generic code, getting the orignal behaviour may require `generic_const_exprs` and propagating trivial bounds (`align_of::<Foo<T>>()`) everywhere.

Ben: The referenced issue does not support this discussion very well at all. I think LLVM is *entirely* to blame for the surprise.

Mario: Maybe we could have a type, like Zig does, for aligned pointers.

## Is example 2 really suprising?

TC: Maybe this is in the eye of the beholder, but is it really suprising for this to be UB if `S` isn't 16-aligned? I've always assumed it would be UB to access fields of an unaligned struct.

#![feature(strict_provenance)]
#[repr(align(16))]
pub struct S { f: [f32; 4] }

unsafe fn bar(x: *const S) -> f32 {
    assert!(x.addr() % 4 == 0); // it *is* 4-aligned
    unsafe { (*x).f[0] }
}

Ben: Perhaps it was me who suggested this is surprising? I have distinctly heard C programmers refer to what we'd call a place projection in C as "well it's just an address computation" and I thought boy it would be nice of Rust to support that.