UB inside `Weak::as_ptr`

[vorner]

I tried this code:

use std::sync::Weak;

fn main() {
    println!("{:?}", Weak::into_raw(Weak::<usize>::new()));
}

When I run it under miri, I get this:

error: Undefined Behavior: inbounds test failed: 0xffffffffffffffff is not a valid pointer
    --> /home/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:2367:45
     |
2367 |     unsafe { data_offset_align(align_of_val(&*ptr)) }
     |                                             ^^^^^ inbounds test failed: 0xffffffffffffffff is not a valid pointer
     |
     = 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 `alloc::sync::data_offset::<usize>` at /home/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:2367:45
     = note: inside `std::sync::Weak::<usize>::as_ptr` at /home/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1575:26
     = note: inside `std::sync::Weak::<usize>::into_raw` at /home/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1609:22
note: inside `main` at src/main.rs:4:22
    --> src/main.rs:4:22
     |
4    |     println!("{:?}", Weak::into_raw(Weak::<usize>::new()));
     |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
     = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
     = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/vorner/.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/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
     = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
     = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/vorner/.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::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/vorner/.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::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:396:14
     = note: inside `std::rt::lang_start_internal` at /home/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:51:25
     = note: inside `std::rt::lang_start::<()>` at /home/vorner/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:65:5

Creating a reference from such pointer looks suspicious indeed. I could send a pull request that adds a condition there, but I want to ask first if this is being done because „we are std, we can“, or if this is something that really should be fixed.

Meta

rustc --version --verbose:

rustc 1.50.0-nightly (f74583445 2020-12-18)
binary: rustc
commit-hash: f74583445702e2e27ec4415376f2c540a83d7ded
commit-date: 2020-12-18
host: x86_64-unknown-linux-gnu
release: 1.50.0-nightly

(but I've looked at the git sources and the thing is still there)

[camelid]

I think it should only be UB to dereference the pointer you get from into_raw, so I'm not sure why Miri thinks there's UB happening. Just in case, @rustbot prioritize

[vorner]

Because that pointer is being turned into a reference to feed to align_of_val internally. And references are supposed to be always valid.

[SNCPlay42]

Looks like #73845 fixed this for Rc but missed it in Arc.

[vitalyd]

Yes, seems this needs to use align_of_val_raw.

[camelid]

I renamed the issue to reflect that the bug is actually in Weak::as_ptr (which Weak::into_raw calls).

[RalfJung]

Cc @CAD97

[RalfJung]

Looks like #73845 fixed this for Rc but missed it in Arc.

Assuming you use the default flags for Miri, that change should not be required to make Miri pass, so I'd be surprised if doing the same change for Rc would make any difference here. If it does, that seems like a bug in Miri that I should investigate.