UB in arc_from_vec_opt test

[RalfJung]

Miri reports UB in the new arc_from_vec_opt test:

  0.145300   test arc::arc_from_vec_opt ... error: Undefined Behavior: `ptr_offset_from` called on pointers into different allocations
  0.000096      --> alloc_miri_test/../library/alloc/tests/arc.rs:222:13
  0.000025       |
  0.000018   222 |             arc.as_ptr().cast::<u8>().offset_from(addr),
  0.000016       |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called on pointers into different allocations
  0.000016       |
  0.000016       = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
  0.000016       = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
  0.000016       = note: BACKTRACE:
  0.000143       = note: inside `arc::arc_from_vec_opt` at alloc_miri_test/../library/alloc/tests/arc.rs:222:13
  0.000020   note: inside closure at alloc_miri_test/../library/alloc/tests/arc.rs:215:23
  0.000007      --> alloc_miri_test/../library/alloc/tests/arc.rs:215:23
  0.000008       |
  0.000007   214 | #[test]
  0.000006       | ------- in this procedural macro expansion
  0.000007   215 | fn arc_from_vec_opt() {
  0.000007       |                       ^

On first sight, the error seems legitimate.

That test got added in #104205.
Cc @clubby789

[RalfJung]

Probably what happens is that in Miri, Global.shrink will always move the data to a new location, and then the way the test checks whether that happened causes UB. You should not call offset_from unless you are already 100% sure that both pointer point to the same allocation -- and in this case, you cannot be sure of that, since allocators do not promise that shrink keeps the same allocation.

In fact, it might be the case that even if shrink keeps the data in-place, the allocation has a new provenance -- it is considered a new allocation at the same location, but all old pointers are invalid now and must be 'refreshed'. So this code probably has UB even if shrink keeps the allocation in-place. I recall this was discussed somewhere at some point, and I think the C standard contains wording along these lines, but I forgot the details... Cc @rust-lang/wg-unsafe-code-guidelines

The fix is easy -- just do let addr = v.as_ptr().addr() and later compare that with arc.as_ptr().addr(). The test will still fail in Miri though, and in other allocators that do not shrink in-place.

[RalfJung]

In fact, it might be the case that even if shrink keeps the data in-place, the allocation has a new provenance

Yeah, the C standard is quite explicit about this, so for now we should follow suit. Opened #104568.

[clubby789]

Might it be a good idea to cfg(not(miri)) the pointer comparison itself, so that Miri can check the actual implementation, even if it can't check the final assertion.

I did suspect that other platforms might not have this behaviour - #104563 seems to show this. It might be best to feature gate the test and opt to only platforms with allocators that work like this.

[RalfJung]

Yes, but to be clear there are two bugs here:

• the test can fail in some platforms, such as Miri, when realloc doesn't shrink in-place
• the test has UB on all platforms because the two pointers are into different allocations (realloc conceptually creates a new allocation, even if it remains in-place), but offset_from is explicitly documented to only work on pointers within the same allocation

[clubby789]

Is the v.as_ptr().addr() comparison still UB? I'm currently working on a patch that

• Uses the .addr() cast for comparison
• Turns off this feature for non-GNU targets (since that's the only one I know for sure has the shrink-in-place behaviour)
• Disables the test on non GNU, and the comparison itself on Miri

With that in mnd though, I'm wondering if this opt should just be removed entirely. The perf wins I got in testing were pretty small for the amount of maintenance headache this seems likely to cause

[RalfJung]

addr() returns regular integers, you can compare them in whatever way you want.