slice: avoid calling memcmp with size 0

[RalfJung]

This is more of an invitation for discussion that "I am sure we need to change this": according to C (C18 §7.1.4), it is UB to call memcmp with an "invalid" pointer. In Rust, we consider ptr::invalid(42) a perfectly valid address for an empty slice of u8. Whether C considers such a pointer (roughly equivalent to (int*)42) to be "invalid" is not entirely clear to me. The section on "Cast operators" in C is very short so I assume int-to-ptr casts are specified (or rather, mostly left unspecified) elsewhere. In the future things might get worse if t-opsem decides that more references are valid for size 0 (including OOB/UAF pointers) -- those are definitely "invalid" in C. But then, it is not clear which part of those rules apply when the function is called from another language.

The safe thing to do is to avoid calling memcmp when the size is 0. However I don't know the cost of this (in terms of performance, in particular).

Cc @rust-lang/libs

[rustbot]

r? @scottmcm

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

[Amanieu]

I would prefer if we did the same thing we already do for memcpy and just require the C library to provide a reasonable implementation that doesn't do anything when given a length of 0.

[RalfJung]

We don't do that for memcpy though, LLVM does. We just call LLVM's memcpy intrinsic (not the C function!) which is documented to be a NOP on size 0. It is up to LLVM to implement those semantics. Other codegen backends might implement our copy_nonoverlapping intrinsics differently.

We can of course start making such an assumption directly from the Rust side as well. Then that should be documented somewhere though. Also, does that mean all Rust code calling memcmp gets to make the same assumption? For Miri's sake I'd rather not have different rules for the standard library and other Rust code.

Cc @rust-lang/opsem

[bjorn3]

What about adding a new intrinsic for this which a codegen backend may lower to memcmp if it knows it is fine to call it with size 0?

[RalfJung]

Sure, then Miri could apply different rules. However in terms of documenting this, that would just move the responsibility about this assumption from t-libs to t-compiler.

[bjorn3]

LLVM assigns defined behavior to memcmp with size 0 ¹. While I can't figure out how to make it insert a memcmp call with size 0, I don't think it is unrealistic for this to happen now or in the future. And when it happens t-compiler still has the burden of this assumption even if t-libs decided to avoid making this assumption in the standard library.

Footnotes

1. https://github.com/llvm/llvm-project/blob/9418c40af7ec6913112b82d6f1d6e8a8c43af6c0/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp#L625-L626 ↩

[RalfJung]

LLVM assigns defined behavior to memcmp with size 0 1.

Yeah but that's their intrinsic, not ours. ;)

[scottmcm]

Yeah but that's their intrinsic, not ours. ;)

Should we add a rustc intrinsic for memcmp, then, like we have https://doc.rust-lang.org/nightly/std/intrinsics/fn.copy_nonoverlapping.html instead of calling memcpy via extern "C"?

That would let the length check be a codegen issue, rather than a library issue.

---

(I visited this from my review queue, but I tweaked a bunch of labels because it seemed like this wasn't just wanting on code review, but more a policy discussion. Feel free to set it back when it's in a "just confirm the code" state.)

[RalfJung]

That would help in one case: if we don't want Rust code calling extern "C" fn memcmp to also get a guarantee that on size 0 this is never UB.

I don't know which team is the one to make that call... T-lang?

[Amanieu]

We discussed this in the libs meeting today. We don't want to add an unnecessary check for something that will never be a real issue in practice. All real memcmp implementations will not touch any memory or have any UB when passed a size of 0. The only real requirement is that the pointers are non-null, which we already guarantee.

We see 2 potential ways to resolve this:

• Add an intrinsic for memcmp like we do for memcpy, which has additional requirements on top of C's.
• Argue that every memory address is a valid zero-sized allocation (a concept which doesn't exist in C) and therefore the pointers passed to memcmp are always valid. No code changes are needed.

[scottmcm]

I'll make a PR to make an intrinsic for it. That'll give a nice place to write our expectations for it, and have it up to the backend to guarantee those things as needed.

EDIT: PR #114382

[RalfJung]

I was pointed to https://doc.rust-lang.org/nightly/core/index.html#how-to-use-the-core-library as a good place to document this assumption. Do you want to do that in the same PR or should I make a separate PR?

[RalfJung]

I opened #114412 for the documentation part. So this PR can be closed now.