remove the memcpy-on-equal-ptrs assumption

[RalfJung]

One of the libc we support, musl, defines memcpy with restrict pointers. This in fact matches the definition in the C standard. Calling that memcpy with overlapping pointers is clearly UB, who knows what the compiler did when optimizing this memcpy -- it certainly assumed source and destination to be disjoint.

Lucky enough, it does not seem like we actually need this assumption that memcpy(p, p, n) is always allowed. clang and GCC need it since they use memcpy to compile C assignments, but we use memmove for similar code. There are no known cases where LLVM introduces calls to memcpy on equal pointers itself. (And if there were, that would be a soundness bug in rustc due to the musl issue mentioned above.)

This does mean we must make sure to never call the LLVM memcpy builtin on equal ranges even though the LangRef says that is allowed. Currently that is the case so we just need to make sure it remains the case. :) Cc @rust-lang/opsem @rust-lang/wg-llvm

[rustbot]

r? @cuviper

(rustbot has picked a reviewer for you, use r? to override)

[the8472]

There are no known cases where LLVM introduces calls to memcpy on equal pointers itself.

A black swan has not been observed, therefore all swans are white? Or do we have stronger confirmation that LLVM won't do this?

[RalfJung]

I didn't say it's great but I don't know any better way than asking @nikic. 🤷 LLVM does not even document anywhere what it assumes about the libc memcpy, so it's hard to say anything with reasonable amounts of certainty.

That said, the main reason I am proposing this is that if we do see a black swan, then the bit of text we have here doesn't actually help. We are unsound on musl. So I want to have the text removed from the docs to make it clear that this would be a soundness bug.

[bjorn3]

In the StatementKind::Assign implementation I can only find memcpy, not memmove:

rust/compiler/rustc_codegen_ssa/src/mir/operand.rs

Line 424 in b29a1e0

base::memcpy_ty(bx, dest.llval, dest.align, r, source_align, dest.layout, flags)

[bjorn3]

Looks like rustc indeed emits llvm.memcpy for your example, but LLVM itself turns it into memmove: https://godbolt.org/z/3PKP47z9a

[RalfJung]

In the StatementKind::Assign implementation I can only find memcpy, not memmove:

Our semantics for Assign are somewhat subtle. I'd like to fix that eventually but oh well... also see #68364.

Basically, if the assignment is of a type with Scalar or ScalarPair layout (and the scalars must be initialized), then the two sides may overlap. The LLVM backend will implement this as a load+store. For other types MIR Assign already requires disjointness (and Miri checks that) so we can use memcpy without relying on the "equal pointer" clause.

[RalfJung]

Looks like rustc indeed emits llvm.memcpy for your example, but LLVM itself turns it into memmove: https://godbolt.org/z/3PKP47z9a

The MIR we generate has a temporary. This gets turned into LLVM IR with two memcpy to and from the temporary, which apparently LLVM optimizes into a memmove that skips the temporary. So all is good, as far as I can see.

[cuviper]

While removing the "equal" part, you added "NULL or dangling" -- isn't that a new assumption? Or are we already living by that elsewhere?

[RalfJung]

That's a clarification. We already have "if the n parameter is 0, the function is assumed to not be UB", which was already meant to include all pointers, including NULL or dangling. I just made it more explicit.

[cuviper]

@bors r+ rollup

[bors]

📌 Commit 7304220 has been approved by cuviper

It is now in the queue for this repository.