Add ptr::{read,write}_unaligned

[Amanieu]

Rendered

[eddyb]

What about passing the alignment explicitly, with the functions proposed here being only for an alignment of 1?
That way, instead of always doing stack copies, they can generate optimal LLVM IR for that given alignment.
They could also be used with larger alignments to potentially hint that, e.g. SIMD can be used, although that works better for variants of the ptr::copy{,_nonoverlapping} functions.

Now is that better than having the alignment as part of the pointer?
I don't know, #[align(1) *mut u32 crossed my mind and it's good enough for some cases, but doesn't work with generics (would have to be something that works with polymorphic constants, i.e. part of the typesystem).

[Amanieu]

I think these use cases can be covered by using a newtype with an explicit alignment attribute:

#[align(1)]
struct UnalignedU32(u32);

This makes more sense since the alignment is attached to the type that is pointed to rather than the pointer itself.

[eddyb]

I was thinking about taking a reference to a packed field, to be clear. Otherwise we could do:

#[align(1)]
struct Unaligned<T>(T);
struct AlignAs<T, U>([U; 0], Unaligned<T>);

[joshtriplett]

On architectures that allow unaligned accesses without faulting, these should become aliases for direct pointer reads and writes. These should only become a full call to copy_nonoverlapping or similar on architectures that don't allow unaligned accesses.

[bluss]

@joshtriplett llvm already compiles copy_nonoverlapping that way, what I've seen.

[brson]

It looks like the only concern here is the ability to tell Rust how to optimize for alignments other than 1, and there's no way to do it for references to pre-existing packed struct fields.

@eddyb Do you feel strongly about the importance of specifying alignment to these functions? This otherwise looks like a simple RFC to push through.

[comex]

Might it be better to explicitly specify an Unaligned<T> and have these functions accept *Unaligned<T> instead of *T?

Putting unaligned addresses in a *T is valid but a potential footgun, since it's easy to accidentally dereference it or convert it to a reference type; better to encourage use of separate types. Also, this sort of violates orthogonality otherwise: ptr::write_unaligned(p, val) is the unaligned equivalent to ptr::write(p, val), but there'd be no unaligned equivalent to *p = val, i.e. a write that runs the destructor on the old value first. An Unaligned struct could make this "just work" (I think?) by implementing Deref.

[parched]

I think I agree with @comex here, alignment should be handled by the type system, not manually by users. Although, I don't think unaligned types like Unaligned<T> make sense, it might not even be in memory, only pointers to unaligned types make sense.

[Amanieu]

@comex You can't run a destructor on an unaligned object because that would create a misaligned &mut T that is passed to the drop impl.

These function are merely convenience wrappers around memcpy and are only useful in unsafe code.

[comex]

Oh, good point. And in fact, after some searching, this RFC purports to ban generic packed structs altogether due to the issue with Drop on their contents, though that rule doesn't seem to be implemented...

[brson]

Based on @Amanieu's statement that "these function are merely convenience wrappers around memcpy and are only useful in unsafe code", I tend to think we don't need a lot of ceremony here to protect users from themselves. This is surfacing an unsafe feature for a specialized purpose. Those that need to do a lot of unaligned work with some guarantees can develop those abstractions and test them in practice, rather than us speculate about what would be useful.

Still hoping to hear from @eddyb about the alignment.

[eddyb]

Sorry, that ping got lost among the other messages.

If the type-based approach can be used with the existing functions with some pointer casting, then we don't even need these new functions AFAICT, and if they can't be used, they're a bit limited.

The problem with non-orthogonal functions is that you might also want unaligned (or differently aligned?) volatile variants of this, for example. There's potential candidates you'd keep adding.

If none of those concerns actually apply, then I have nothing against this RFC in its current form.

[Amanieu]

I don't think there is much of a use case for unaligned volatile accesses. These will likely be broken up into multiple memory accesses by the hardware, which somewhat defeats the point of volatile.

[brson]

I'm inclined to move this to FCP to see if we can stir up more opinions.

It looks to me like these are the major points:

• read_unaligned and write_unaligned are very simple wrappers around existing features, but these make it obvious what low-level tool to reach for for unaligned access.
• These don't provide any sort of abstraction to attempt to protect the user from themselves. One might e.g. want all unaligned values to be wrapped in Unaligned.
• On the other hand, these are low-level building blocks, and we don't have much insight into what people should be doing with them. An Unaligned type can be built from these.
• This solution pessimistically assumes the alignment is 1, and doesn't support any other alignments.
• It may be preferable to use attributes to give types alignment like #[align(1)] NewType(...).
• There are concerns about combination with other features, e.g. what if somebody wants volatile + unaligned access, then we'll need new functions, but it's been asserted that volatile + unaligned at least is unlikely to be a real case.

My inclination is that this is useful as-is for a pretty common case, and low risk. But I also think the case both for and against are not that strong. We don't need this in tree but it would be good to have some signaling that one can do unaligned load and store, and this is how.

@rfcbot fcp merge

[rfcbot]

Team member @brson has proposed to merge this. The next step is review by the rest of the tagged teams:

• @BurntSushi
• @Kimundi
• @alexcrichton
• @aturon
• @brson
• @sfackler

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[hsivonen]

Having this would have saved me time, so +1 for having obvious ways to dereference unaligned pointers to SIMD types in particular.

One bikeshed question though: Is there a good reason for these being functions as opposed to methods on pointer types like the read_volatile() and write_volatile()?

[japaric]

Is there a good reason for these being functions as opposed to methods on pointer types like the read_volatile() and write_volatile()?

read_volatile() and write_volatile() are free functions in core::ptr, not methods.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period is now complete.

[alexcrichton]

Looks like no new issues came up, so merging! Thanks again for the RFC @Amanieu!

Tracking issue: rust-lang/rust#37955

[bluss]

I realize Rust is not C, but I'll just remind us of the difference here. I'm late now, but something like this could have been part of the RFC.

With a background in C, it's strange to even pass around raw pointers that are not well aligned for their pointee type. (Casting to create such a pointer is UB in C!). In Rust it's a useful feeling anyway, because almost all consumers of raw pointers have the same requirement (well aligned for pointee type). But we make note that this is not an invariant of the raw pointer type itself in Rust. Yet, many implementations may feel it's more precise to store unaligned pointers as *const u8 or similar type.

[bluss]

forget(v) is missing in the reference implementation here; this should move v into the new location, just like write does.

[aturon]

cc @rust-lang/lang re: @bluss's comment, which has some overlap with the unsafe guidelines work. Just want to make sure everyone is aware/on board.

[chriskrycho]

Please update the Rendered link to point to the final text if you get a chance. Thanks!

[sfackler]

Updated