Should RelPtr use wrapping_add instead of add?

[jgarvin]

I am still enough of a novice at unsafe rust that I don't know the answer. I'm getting miri errors running cargo miri test for threadstack on HEAD (see below). A regular cargo test passes fine.

Miri doesn't seem to like that RelPtr is using add, which calls offset, which based on the documentation seems to assume that you won't go outside the bounds of the current 'object'. wrapping_add doesn't have this restriction though.

Edit: for context the reason I'm using RelPtr is I need a pointer that can point to a member of the struct containing the pointer, that stays valid after the struct is moved. The reason I need that is that ThreadStackWithInitialValue is created with thread_local!, which doesn't seem to create its objects in place -- they get created on the stack and then are moved/copied into the thread local location. So if I have the struct store a regular pointer to its member it becomes invalid.

yyyyyyy@xxxxxx:~/threadstack$ cargo miri test
   Compiling threadstack v0.4.1 (/home/yyyyyyy/threadstack)
    Finished test [unoptimized + debuginfo] target(s) in 0.05s
     Running target/x86_64-unknown-linux-gnu/debug/deps/threadstack-2fa8c731864a0d1d

running 4 tests
test tests::it_works ... warning: thread support is experimental and incomplete: weak memory effects are not emulated.

error: Undefined Behavior: inbounds test failed: pointer must be in-bounds at offset 2120, but is outside bounds of alloc70015 which has size 32
   --> /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/const_ptr.rs:225:18
    |
225 |         unsafe { intrinsics::offset(self, count) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ inbounds test failed: pointer must be in-bounds at offset 2120, but is outside bounds of alloc70015 which has size 32
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
            
    = note: inside `std::ptr::const_ptr::<impl *const u8>::offset` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/const_ptr.rs:225:18
    = note: inside `rel_ptr::<impl rel_ptr::Delta for isize>::add` at /home/yyyyyyy/.cargo/registry/src/g.yxqyang.asia-1ecc6299db9ec823/rel-ptr-0.2.3/src/lib.rs:178:17
    = note: inside `rel_ptr::RelPtr::<u32>::as_raw_unchecked_impl` at /home/yyyyyyy/.cargo/registry/src/g.yxqyang.asia-1ecc6299db9ec823/rel-ptr-0.2.3/src/lib.rs:358:26
    = note: inside `rel_ptr::RelPtr::<u32>::as_ref_unchecked` at /home/yyyyyyy/.cargo/registry/src/g.yxqyang.asia-1ecc6299db9ec823/rel-ptr-0.2.3/src/lib.rs:403:11
note: inside `<ThreadStackWithInitialValue<u32> as IsThreadStack<u32>>::get_value_impl` at src/lib.rs:206:9
   --> src/lib.rs:206:9
    |
206 |         p.as_ref_unchecked()
    |         ^^^^^^^^^^^^^^^^^^^^
note: inside closure at src/lib.rs:282:13
   --> src/lib.rs:282:13
    |
282 |             $crate::IsThreadStack::get_value_impl(stack, &stack_lifetime_hack)
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
395 |                 let_ref_thread_stack_value!(stack_value, STACK);
    |                 ------------------------------------------------ in this macro invocation
    = note: inside `std::thread::LocalKey::<ThreadStackWithInitialValue<u32>>::try_with::<[closure@src/lib.rs:281:36: 283:10], &u32>` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:272:16
    = note: inside `std::thread::LocalKey::<ThreadStackWithInitialValue<u32>>::with::<[closure@src/lib.rs:281:36: 283:10], &u32>` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/local.rs:248:9
note: inside closure at src/lib.rs:281:29
   --> src/lib.rs:281:29
    |
281 |           let $new_variable = s.with(|stack| unsafe {
    |  _____________________________^
282 | |             $crate::IsThreadStack::get_value_impl(stack, &stack_lifetime_hack)
283 | |         });
    | |__________^
...
395 |                   let_ref_thread_stack_value!(stack_value, STACK);
    |                   ------------------------------------------------ in this macro invocation
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<[closure@src/lib.rs:389:18: 401:10], ()>` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
    = note: inside closure at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/mod.rs:474:17
    = note: inside `<std::panic::AssertUnwindSafe<[closure@std::thread::Builder::spawn_unchecked<[closure@src/lib.rs:389:18: 401:10], ()>::{closure#0}::{closure#0}]> as std::ops::FnOnce<()>>::call_once` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:344:9
    = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<[closure@std::thread::Builder::spawn_unchecked<[closure@src/lib.rs:389:18: 401:10], ()>::{closure#0}::{closure#0}]>, ()>` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
    = note: inside `std::panicking::r#try::<(), std::panic::AssertUnwindSafe<[closure@std::thread::Builder::spawn_unchecked<[closure@src/lib.rs:389:18: 401:10], ()>::{closure#0}::{closure#0}]>>` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:343:19
    = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<[closure@std::thread::Builder::spawn_unchecked<[closure@src/lib.rs:389:18: 401:10], ()>::{closure#0}::{closure#0}]>, ()>` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:431:14
    = note: inside closure at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/thread/mod.rs:473:30
    = note: inside `<[closure@std::thread::Builder::spawn_unchecked<[closure@src/lib.rs:389:18: 401:10], ()>::{closure#0}] as std::ops::FnOnce<()>>::call_once - shim(vtable)` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `<std::boxed::Box<dyn std::ops::FnOnce()> as std::ops::FnOnce<()>>::call_once` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1548:9
    = note: inside `<std::boxed::Box<std::boxed::Box<dyn std::ops::FnOnce()>> as std::ops::FnOnce<()>>::call_once` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1548:9
    = note: inside `std::sys::unix::thread::Thread::new::thread_start` at /home/yyyyyyy/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys/unix/thread.rs:71:17
    = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error; 1 warning emitted

error: test failed, to rerun pass '--lib'
yyyyyyy@xxxxxx:~/threadstack$ cargo test
   Compiling threadstack v0.4.1 (/home/yyyyyyy/threadstack)
    Finished test [unoptimized + debuginfo] target(s) in 0.47s
     Running target/debug/deps/threadstack-eea1ff6fd5726554

running 4 tests
test tests::it_works ... ok
test tests::it_works_no_initial ... ok
test tests::no_initial_value_test ... ok
test tests::revert_to_no_initial ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests threadstack

running 4 tests
test src/lib.rs - declare_thread_stacks (line 82) ... ok
test src/lib.rs - clone_thread_stack_value (line 313) ... ok
test src/lib.rs - let_ref_thread_stack_value (line 253) ... ok
test src/lib.rs - push_thread_stack_value (line 336) ... ok

test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.29s

[RustyYato]

What's happened is that RelPtr is likely unsound, :( I should put a warning. This has to do with provenance. Currently RelPtr tries to access data that is outside the provenance of &RelPtr, the only way around this is to use raw pointers all the way down. When I created RelPtr these rules were looser. I'm not sure if I have time to work on this for now, I'll try and get something out for the weekend.

There is no fix on stable, on nightly you can use raw_ref_macros, and that will work. I think you are right that I need to use wrapping_offset. Thanks!

---

Old (wrong) answer

---

The only advantage of wrapping_add over add is that you can temporarily go out of bounds. This is not the case for you. You will still get the MIRI error if I change to wrapping_add.

inbounds test failed: pointer must be in-bounds at offset 2120, but is outside bounds of alloc70015 which has size 32
                                                          ^^^^ note the offset, it looks way out of bounds

what likely happened is that your RelPtr was invalidated (probably by a move, judging solely on the offset).