Conflicting documentation on the alignment requirements for ptr::copy

[sgrif]

The docs for ptr::drop_in_place state:

Unaligned values cannot be dropped in place, they must be copied to an aligned location first:

And include an example which uses ptr::copy to copy the value from an unaligned pointer into an aligned one. However, the docs for ptr::copy state:

Both src and dst must be properly aligned.

Either the docs for ptr::copy are wrong, and src does not need to be aligned, or the example for drop_in_place invokes UB and should be updated to use read_unaligned

[Centril]

cc @ubsan @RalfJung

(It's also notable that the example in std::ptr::read_unaligned is wrong when it uses let unaligned = &x.unaligned as *const u32; as is seen if you move the let statement out of the unsafe { ... } block; we should fix that ASAP)

[RalfJung]

Either the docs for ptr::copy are wrong, and src does not need to be aligned, or the example for drop_in_place invokes UB and should be updated to use read_unaligned

Good catch!

What we'd really want is copy_unaligned, I think... we don't want an intermediate value on the stack.

(It's also notable that the example in std::ptr::read_unaligned is wrong when it uses let unaligned = &x.unaligned as *const u32; as is seen if you move the let statement out of the unsafe { ... } block; we should fix that ASAP)

Feel free to suggest code that we can use instead.

We have several doctests that need rust-lang/rfcs#2582. They have been there since 1.0 and longer, probably. But nobody listens to me when I say that there is existing code that we need to support ASAP.

[sgrif]

The example given is just using ptr::copy to put it on the stack anyway. If you're having to copy the value at all, there's really no reason to use ptr::drop_in_place at that point, since the only reason to use it instead of ptr::read is to avoid the copy (which is optimized out in release mode anyway). I think we can probably just remove the unaligned example entirely and change the documentation to read:

to_drop must be properly aligned. To drop an unaligned pointer, use ptr::read_unaligned instead.

I think we should probably mention the caveat WRT the optimizer eliminating the copy (or can it even do that in this case? The docs mention that the drop glue generated for packed structs does this copy automatically, I'm curious if it's doing so in a way that is UB or if the copy is just the price you have to pay here.

It's also worth noting that there's no way to drop_in_place an unsized value through an unaligned pointer, but I'm not sure that's worth bringing up at all in the docs, since there's no reasonable way to actually get that pointer in the first place.

[RalfJung]

The example given is just using ptr::copy to put it on the stack anyway.

Oh, good point. Yeah then you can basically just make it let copy = to_drop.read_unaligned(), right?
I wonder why MaybeUninit/mem::uninitialized ever got used here?

I think we can probably just remove the unaligned example entirely and change the documentation to read:

Well but wouldn't it make sense for that text to come with an example?

I think we should probably mention the caveat WRT the optimizer eliminating the copy (or can it even do that in this case?

It cannot, because then it would be introducing alignment errors.

It's also worth noting that there's no way to drop_in_place an unsized value through an unaligned pointer, but I'm not sure that's worth bringing up at all in the docs, since there's no reasonable way to actually get that pointer in the first place.

Uh... yeah. With copy_unaligned it could work.

Doesn't copy_unaligned seem like a reasonable thing to have either way?

[sgrif]

I wonder why MaybeUninit/mem::uninitialized ever got used here?

Presumably because they wanted the example to continue to use drop_in_place

Well but wouldn't it make sense for that text to come with an example?

Sure, we could -- it just wouldn't ever call the function being documented.

Doesn't copy_unaligned seem like a reasonable thing to have either way?

Yes, it's just unrelated to dropping unaligned pointers since the only place you can reasonably copy them to is the stack. Unless you happen to already have an valid pointer of the proper size and alignment sitting around

[hanna-kruppe]

I'm not actually sure unalgined DSTs are so unreasonable. For example, unsizing repr(packed) wrapper around an array is only stopped by this rule:

= note: the last field of a packed struct may only have a dynamically sized type if it does not need drop to be run

Which could be solved if we had e.g. a way to express "does not have drop glue" as a bound ([T] : Copy doesn't work for obvious reasons).

In any case, I think "copy unaligned" functions make sense but we might want to consider multiple variants for src only / dest only / both unaligned. This can be expressed in LLVM IR though in a brief test I couldn't demonstrate that it currently makes a codegen difference (but I could only try on x86_64, where alignment rarely matters for codegen).

[RalfJung]

Sure, we could -- it just wouldn't ever call the function being documented.

🤣 fair. I guess all of this is basically saying an in-place drop of unaligned data is just not possible. Do an "out-of-place" drop instead.

[RalfJung]

Actually looks like the use of an unaligned pointer in the doctest just caused #62103 to fail to land. ;)

[RalfJung]

Proposed fix at #62351.