`addr_of[_mut]!` docs should be more precise about what's sound

[joshlf]

This example comes from this URLO thread, which in turn was inspired by this zerocopy issue.

The addr_of! and addr_of_mut! docs imply that they can handle unaligned pointers so long as a reference is never materialized from that pointer (which would be insta-UB). However, running the following program under Miri fails:

fn main() {
    #[repr(C, packed)]
    struct Unalign<T>(T);
    
    #[repr(C)]
    struct Foo {
        a: u8,
        b: u16,
    }
    
    // Has alignment `align_of::<T>()`, and the `Unalign<T>`
    // is at byte offset 1; so long as `align_of::<T>() > 1`,
    // the contained `T` is misaligned.
    #[repr(C)]
    struct Misalign<T>(u8, Unalign<T>, [T; 0]);
    
    let u = Misalign(0, Unalign(Foo{ a: 1, b: 2 }), []);
    let u_ptr: *const Unalign<Foo> = &u.1;
    // Sound because `Unalign` contains a `T` and nothing else.
    let f_ptr: *const Foo = u_ptr.cast();
    // Should be sound because we never construct a reference.
    let addr_of_b: *const u16 = unsafe { core::ptr::addr_of!((*f_ptr).b) };
    println!("{:?}", addr_of_b);
}

Here's the failure:

error: Undefined Behavior: accessing memory with alignment 1, but alignment 2 is required
  --> src/main.rs:22:42
   |
22 |     let addr_of_b: *const u16 = unsafe { core::ptr::addr_of!((*f_ptr).b) };
   |                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ accessing memory with alignment 1, but alignment 2 is required
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `main` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:1980:5: 1980:22
   = note: this error originates in the macro `core::ptr::addr_of` (in Nightly builds, run with -Z macro-backtrace for more info)

@cuviper pointed out my issue:

The example shows an unaligned field (since it's packed), but that doesn't excuse everything. "Note, however, that the expr in addr_of!(expr) is still subject to all the usual rules." -- I think this would include alignment for (*f_ptr), although the docs only call out null pointers.

Miri is happy with addr_of!((*u_ptr).0.b).

The docs strongly imply that this would be sound. It does this in two ways. First, when contrasting references and raw pointers, it specifically calls out that references need to be aligned:

Creating a reference with &/&mut is only allowed if the pointer is properly aligned and points to initialized data. For cases where those requirements do not hold, raw pointers should be used instead.

Second, it seems to enumerate the cases in which addr_of! would be unsound, and operating on unaligned pointers isn't on the list:

Note, however, that the expr in addr_of!(expr) is still subject to all the usual rules. In particular, addr_of!(*ptr::null()) is Undefined Behavior because it dereferences a null pointer.

IMO the docs as currently written are both vague and misleading with respect to what's allowed. It would be good if the docs were more precise on exactly what is and isn't allowed. I was pretty confident in the soundness of the code I'd written, and only discovered the issue thanks to Miri.

[ChrisDenton]

Maybe @rust-lang/opsem have some thoughts on precise wording that should be used.

[RalfJung]

The docs strongly imply that this would be sound.

That is unfortunate, since when I read them I find them fairly clearly stating this to be not sound. :/ Our mental models must be quite different here.

I wonder, if the code did addr_of!(***ptr), would you be surprised that the two loads from memory that are happening here are still subject to the usual alignment rules? Your case is really another instance of the same thing -- each of these three * operators requires alignment. It's as simple as that. The addr_of! only becomes relevant after the place expression ***ptr was evaluated to a place.

Sadly there are many inaccurate mental models out there about how place and value expression evaluation interacts. You are by far not the first to be confused by these docs. That's why we added the part about addr_of!(*ptr::null()) being UB -- I would assume that conflicts with your model, so my hope was that people with the wrong model would read that part and then realize they took a wrong turn somewhere. (Or maybe your model makes that UB but allows your code... but then I can't even imagine what your model is like, unfortunately.)

I think the proper model here is beautiful and I'd love to teach it to everyone... but maybe, this one time, we don't actually have to. If rust-lang/reference#1387 gets accepted, your code will become sound. addr_of!(**ptr::null()) will still be UB but I don't think anyone is surprised by that. The "wrong" model isn't actually wrong any more, it is just unnecessarily complicated in my eyes, but it should produce the right predictions. At least I hope so.

[saethlin]

I think that any invalid example involving ptr::null() is too easily hand-waved away; null is special-cased for reads so it does not necessarily register as surprising that it is also not allowed here.

[joshlf]

The docs strongly imply that this would be sound.

That is unfortunate, since when I read them I find them fairly clearly stating this to be not sound. :/ Our mental models must be quite different here.

Yeah, and I wish I had your mental model! I know what it's like to be on the other side of "I don't see what's confusing about this for you", so I'll try as best as I can to articulate how I interpreted the docs.

I wonder, if the code did addr_of!(***ptr), would you be surprised that the two loads from memory that are happening here are still subject to the usual alignment rules? Your case is really another instance of the same thing -- each of these three * operators requires alignment. It's as simple as that. The addr_of! only becomes relevant after the place expression ***ptr was evaluated to a place.

Maybe I'm missing something, but you said that there are two loads - doesn't that mean that the final dereference (since there are three of them) isn't performing a load? (And, by implication, that final dereference doesn't need to be properly aligned?)

Maybe this is the issue: In my mental model, "dereference" doesn't necessarily mean "load". A few examples of why that's my mental model:

• In C, you can do ptr->foo = bar - that's a dereference but not a load of the referent of ptr
• (This one I'm less sure about) Also in C, you can do *ptr = foo - also a dereference but not a load of the referent
• In Rust, doing &*ptr is the way you spell "cast this to a reference", and I've always assumed this doesn't actually perform a load of the referent of ptr? (though, as the docs mention, materializing an invalid reference is UB, so it's still dangerous for other reasons)

For that reason, my mental model of something like addr_of!((*ptr).foo) is "give me the address that I'd be storing into if I did (*ptr).foo = bar". That's reinforced by code like the following:

fn main() {
    let mut x = (0usize,);
    let x_ptr: *mut (usize,) = &mut x;
    unsafe { (*x_ptr).0 = 1 };
    println!("{}", x.0); // prints 1
}

My reading of this example is that, if *x_ptr loaded from x_ptr, then it would create a temporary, and so the assignment would have no effect. The fact that it has an effect implies to me that *x_ptr is closer to a "place" (I'm using that term loosely to mean "thing I can assign into", not necessarily in its precise Rust meaning).

In your example of addr_of!(***ptr), I'd assume that *ptr is loaded - let's call it ptr2 - and then **ptr (in other words, *ptr2) is loaded - let's call it ptr3 - and then ***ptr (in other words, *ptr3) constructs a place expression but does not result in a load from the memory location addressed by ptr3.

Sadly there are many inaccurate mental models out there about how place and value expression evaluation interacts. You are by far not the first to be confused by these docs. That's why we added the part about addr_of!(*ptr::null()) being UB -- I would assume that conflicts with your model, so my hope was that people with the wrong model would read that part and then realize they took a wrong turn somewhere. (Or maybe your model makes that UB but allows your code... but then I can't even imagine what your model is like, unfortunately.)

Honestly, I think I read the docs like this:

Docs: "Note, however, that the expr in addr_of!(expr) is still subject to all the usual rules."
Me: "Hmmm, I'm not sure what "all the usual rules" are."
Docs: "In particular, addr_of!(*ptr::null()) is Undefined Behavior because it dereferences a null pointer."
Me: "Oh, I guess that's the only requirement, cool. Not sure why they said "rules" plural. Oh well."

I think the proper model here is beautiful and I'd love to teach it to everyone... but maybe, this one time, we don't actually have to. If rust-lang/reference#1387 gets accepted, your code will become sound. addr_of!(**ptr::null()) will still be UB but I don't think anyone is surprised by that. The "wrong" model isn't actually wrong any more, it is just unnecessarily complicated in my eyes, but it should produce the right predictions. At least I hope so.

Yeah I would absolutely love to have the correct model in my head! If I had to speculate, I'd say that part of the disconnect between folks like you - who work on this stuff, and especially its internals - and folks like me - who merely consume these semantics - is that it'd be a lot harder for your mental model to be wrong-but-accidentally-not-causing-any-problems. For me, it's easy to feel like I understand things, and so long as I come to the correct conclusions (where "correct" means something like "write code that other programmers and Miri agree is sound"), I never realize I'm wrong until I come to edge cases like these. By contrast, it'd be a lot harder for you to persist with an incorrect mental model because, when working on the compiler internals, much smaller/finer-grained errors in your model have practical consequences than is the case for me.

I think about how confusing it used to be for me to reason about indexing/off-by-one errors/etc when I was first learning to program, and now it's not just something I'm good at, it feels obvious and easy. It's hard for me to empathize with what it'd be like not to have an intuitive grasp of those concepts. I think this is basically a Hard Problem. I don't really know of any way past it besides a) lots of navel gazing about how you came to your current understanding and, b) trying out your explanations on people (and then seeing if they can expand on the model they've acquired from those explanations, and if those expansions are correct).

On a more pragmatic note, I think there are two pieces of low-hanging fruit here that would make things a lot better:

• Canonical locations for definitions, and more linking between docs. E.g., "all the usual rules" doesn't help me if I don't already know what that refers to. It was only while writing this bullet point that I thought to look and discovered that "place" is a well-documented term.
• In the canonical definitions, lists of examples (e.g., this code is sound, this code is unsound, this code is sound, this code is unsound, etc). That gives folks a way of testing their own understanding.

[digama0]

Maybe I'm missing something, but you said that there are two loads - doesn't that mean that the final dereference (since there are three of them) isn't performing a load? (And, by implication, that final dereference doesn't need to be properly aligned?)

The first sentence is true, but the "by implication" is not. The final dereference produces a place, exactly as you have described, but constructing a place already adds the requirement of being properly aligned, same as you would get if you put a & on the front. This is what the example involving *ptr::null() was supposed to indicate: the place itself has to be valid, even though a load is not actually happening because it is wrapped in addr_of!(_).

We found this behavior confusing and foot-gunny as well, which is why rust-lang/reference#1387 documents the new behavior, that constructing a place on its own doesn't add any requirements.

[RalfJung]

In my mental model, "dereference" doesn't necessarily mean "load".

Okay so it sounds like our models are closer than I thought! The one missing bit is that according to https://doc.rust-lang.org/reference/behavior-considered-undefined.html it is the deref, not the load, where alignment is checked. I do agree "the usual rules" without further explanation is terrible docs...

[joshlf]

Sounds good!

A few more clarifying questions:

First, do field accesses generate loads? E.g.:

let p: *const (u8, (u16, (u32, u64))) = ...;
addr_of!((*p).1.1.1);

Does this addr_of! expression perform any loads? If not, then under the new model, is this also sound even if p is unaligned?

Second, I assume that indirecting through a reference generates a load unconditionally? E.g.:

let p: *const (u8, &'static (u16, (u32, u64))) = ...;
addr_of!((*p).1.1.1);

I assume that this generates a single load (namely, it loads the value of the p.1 (the &'static) so that it can then calculate the offset of .1.1 within the type (u16, (u32, u64)) and add that offset to the address stored in p.1)? I assume that, under the new model, p can not be unaligned because this generates a load from p.1?

[RalfJung]

Under the rules are currently documented and enforced by Miri, loads are irrelevant for alignment. Aligned is checked on *, i.e. on derefs. This implies that all loaded-from values are aligned, so a separate check at load time is not needed.

Under rust-lang/reference#1387, it is only loads that generate alignment requirements. Field accesses do not require alignment, but they do require "inbounds" (ptr.offset rules).

For the second point, yes -- that code desugars to addr_of!((*(*p).1).1.1) which desugars to addr_of!((*load(*load(p)).1).1.1) and each load requires alignment. (The first load here is just the load from the local itself which is always aligned since the compiler ensures to allocate it in an aligned way.

[saethlin]

addr_of! applies to a Place expression. A Place can only contain one Deref. Ergo, even if in surface Rust you pile a lot of code into one expr, you get loads for all but the last (in evaluation order) Deref. The difference with references and raw pointers is that a Deref of a pointer must be explicit, reference Deref may be implicit.

I don't know a better way to explain this. I feel like we want to avoid the explanation that a Place can only contain one Deref. That's why your example has a load through the reference, it desugars into multiple Places.