Meeting Proposal: Valid uses of addr_of! (and requirements of places)

[saethlin]

Summary

The implementation of addr_of! is just:

pub macro addr_of($place:expr) {
    &raw const $place
}

Then the documentation for ptr::addr_of! says:

Creating a reference with &/&mut is only allowed if the pointer is properly aligned and points to initialized data.
...
This macro can create a raw pointer without creating a reference first.

Note, however, that the expr in addr_of!(expr) is still subject to all the usual rules.

The first paragraph of the documentation suggests that addr_of! is useful for creating misaligned pointers. Then the second paragraph warns about "all the usual rules", which are not defined anywhere. So reading this documentation, one can come away with either impression about whether this program is well-defined or not:

struct Misalignment {
    a: u32,
}

fn main() {
    let items: [Misalignment; 2] = [Misalignment { a: 0 }, Misalignment { a: 1 }]; 
    unsafe {
        let ptr: *const Misalignment = items.as_ptr().cast::<u8>().add(1).cast::<Misalignment>();
        let _ptr = core::ptr::addr_of!((*ptr).a);
    }   
}

Do "the usual rules" forbid the misaligned dereference inside of addr_of!? Or is addr_of! exempted because that's the whole reason it exists?

The goal of this meeting is to decide if we are able to bless this use of addr_of!, whether we probably need it to be UB, or if we wish to delay this decision. If we wish to delay this decision, then we also have the goal of deciding what guidance we should offer to our users in the meantime.

Reading

• Don't check for misaligned raw pointer derefs inside Rvalue::AddressOf rust#112026
• Should raw pointer deref/projections have to be in-bounds? unsafe-code-guidelines#319

HackMD: https://hackmd.io/YjSceBPISBiGwsiqt9cDdw

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]

I think this should be slightly broader -- basically this is about when exactly which properties of a place are required to avoid UB. Currently we have "whenever a place is constructed, it must be aligned and dereferenceable". That's also what Miri implements. In MiniRust we have "arbitrary places can be constructed; when doing a place projection, it must be inbounds (same rules as ptr::offset); when loading from a place or turning it into a reference, it must be aligned and dereferenceable; when turning it into a raw pointer, anything goes". Other options are conceivable.

I'm happy to help draft the discussion document.

[saethlin]

Yeah we should definitely work together. I'll start something then we can iterate on it together.

But mostly here I'm just saving a link to this very interesting footnote for myself: rust-lang/rust#112504 (comment)

[RalfJung]

I realized we don't even have a UCG issue for the "place validity invariant", so here is one: rust-lang/unsafe-code-guidelines#418. That's basically what we want to resolve in this meeting, I think.

[saethlin]

I've started a writeup here, but it's only about my questions about addr_of! so far. I think others should amend it to also address place validity in general: https://hackmd.io/@r7Ge5hUhSrqXom8A7Ga3XQ/HknsMDID2

[RalfJung]

Summary:
nobody in the room was opposed to change the rules of * expression evaluation such that, when operating on a raw pointer, nothing at all happens -- we just turn the pointer value into a place, with no chance of UB.

There was some concern that addr_of!((*ptr).field) can still be UB due to doing out-of-bounds pointer arithmetic (so addr_of! might still have more UB than expected), but:

• this is considered rarer and less surprising than violating alignment, so overall the proposed change still helps
• we almost definitely want to remove the "dereferenceable" check from raw-pointer * (for cases like dirent); keeping the "aligned" check at that point seems entirely ad-hoc and unnecessary

We agreed someone should make a PR against the reference to reflect this change, FCP'd by the team. We don't have anyone assigned to that task yet.

[saethlin]

I think there are more tasks than that? Keeping this list more for myself than anything:

• Update The Reference with t-opsem FCP: replace 'UB on raw ptr deref' with UB on place projection/access reference#1387
• Update the docs of addr_of! after The Reference: update and clarify addr_of docs rust#117572
• Update MiniRust (it already didn't do any checks on *; re-do the way we treat alignment around places minirust/minirust#140 introduced "check alignment of base pointer")
• Update Miri @RalfJung don't UB on dangling ptr deref, instead check inbounds on projections rust#114330
• Update CheckAlignment @saethlin Update the alignment checks to match rust-lang/reference#1387 rust#113343

[chorman0773]

I can take doing the addr_of! docs PR.

[RalfJung]

Right there's a bunch of follow-ups to do. I can take care of Miri.

FWIW I updated MiniRust to do the dereferenceable+aligned check on references but not raw pointers.

[RalfJung]

The reference and Miri are updated. :)

From the todo list above, we still have

• to update the addr_of! docs, @chorman0773 do you still plan to do that?
• to update the CheckAlignment pass, @saethlin I guess that's Update the alignment checks to match rust-lang/reference#1387 rust#113343 ?

[chorman0773]

Yep. Now that the reference is updated, I can do that.

[saethlin]

Yeah, I'll reshape that PR to match what is checked elsewhere.

[RalfJung]

All the action items tracked above have PRs on their way. So I don't think we need to still keep this issue here open.

Glad to have this long-standing open issue finally fully resolved. :)