Miri UB warning about compat feature

[quininer]

I tried to use miri to check a piece of code, and accidentally found that the compat feature had some UB warnings.

use futures::future::TryFutureExt;
use futures::compat::Future01CompatExt;
use futures::executor::block_on;

fn main() {
    let fut = async {
        Ok(()) as Result<(), ()>
    };
    let fut = Box::pin(fut);
    let fut = fut.compat(); // 03as01
    let fut = fut.compat(); // 01as03
    block_on(fut).unwrap();
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=8ef0b573eab227a3fb307ca73df351bb

by default, miri will report an error saying that 0x1 is not a valid function pointer. I try to fix it,

diff --git a/src/task_impl/std/mod.rs b/src/task_impl/std/mod.rs
index e82a23e5..59f7a0f0 100644
--- a/src/task_impl/std/mod.rs
+++ b/src/task_impl/std/mod.rs
@@ -42,7 +42,7 @@ pub fn get_ptr() -> Option<*mut u8> {
     // used (the default), the branch predictor will be able to optimize the
     // branching and a dynamic dispatch will be avoided, which makes the
     // compiler happier.
-    if core::is_get_ptr(0x1) {
+    if core::is_get_ptr(fake_get_ptr as _) {
         Some(CURRENT_TASK.with(|c| c.get()))
     } else {
         core::get_ptr()
@@ -53,6 +53,9 @@ fn tls_slot() -> *const Cell<*mut u8> {
     CURRENT_TASK.with(|c| c as *const _)
 }

+fn fake_get_ptr() -> *mut u8 { std::ptr::null_mut() }
+fn fake_set_ptr(_: *mut u8) {}
+
 pub fn set<'a, F, R>(task: &BorrowedTask<'a>, f: F) -> R
     where F: FnOnce() -> R
 {
@@ -61,13 +64,11 @@ pub fn set<'a, F, R>(task: &BorrowedTask<'a>, f: F) -> R
     // Note that we won't actually use these functions ever, we'll instead be
     // testing the pointer's value elsewhere and calling our own functions.
     INIT.call_once(|| unsafe {
-        let get = mem::transmute::<usize, _>(0x1);
-        let set = mem::transmute::<usize, _>(0x2);
-        init(get, set);
+        init(fake_get_ptr, fake_set_ptr);
     });

     // Same as above.
-    if core::is_get_ptr(0x1) {
+    if core::is_get_ptr(fake_get_ptr as _) {
         struct Reset(*const Cell<*mut u8>, *mut u8);

         impl Drop for Reset {

but another error will occur

error: Undefined Behavior: not granting access to tag <untagged> because incompatible item is protected: [SharedReadOnly for <4348> (call 1302)]
   --> /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:904:9
    |
904 |         Box(unsafe { Unique::new_unchecked(raw) }, alloc)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <untagged> because incompatible item is protected: [SharedReadOnly for <4348> (call 1302)]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
    = note: inside `std::boxed::Box::<dyn futures::task_impl::UnsafeNotify>::from_raw_in` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:904:9
    = note: inside `std::boxed::Box::<dyn futures::task_impl::UnsafeNotify>::from_raw` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:849:18
    = note: inside `<futures::compat::compat01as03::NotifyWaker as futures::task_impl::UnsafeNotify>::drop_raw` at /home/user/workspace/fork/futures03/futures-util/src/compat/compat01as03.rs:346:14
    = note: inside `<futures::task_impl::NotifyHandle as std::ops::Drop>::drop` at /home/user/workspace/fork/futures-rs/src/task_impl/mod.rs:688:13
    = note: inside `std::ptr::drop_in_place::<futures::task_impl::NotifyHandle> - shim(Some(futures::task_impl::NotifyHandle))` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `std::ptr::drop_in_place::<futures::task_impl::core::TaskUnpark> - shim(Some(futures::task_impl::core::TaskUnpark))` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `std::ptr::drop_in_place::<futures::task_impl::std::TaskUnpark> - shim(Some(futures::task_impl::std::TaskUnpark))` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `std::ptr::drop_in_place::<futures::task_impl::Task> - shim(Some(futures::task_impl::Task))` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `std::ptr::drop_in_place::<futures::compat::compat03as01::Current> - shim(Some(futures::compat::compat03as01::Current))` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `futures::compat::compat03as01::with_context::<std::pin::Pin<std::boxed::Box<std::future::from_generator::GenFuture<[static generator@src/main.rs:6:21: 8:6]>>>, std::result::Result<futures::poll::Async<()>, ()>, [closure@<futures::compat::Compat<std::pin::Pin<std::boxed::Box<std::future::from_generator::GenFuture<[static generator@src/main.rs:6:21: 8:6]>>>> as futures::future::Future>::poll::{closure#0}]>` at /home/user/workspace/fork/futures03/futures-util/src/compat/compat03as01.rs:201:1

If -Zmiri-track-raw-pointers is enabled,

error: Undefined Behavior: trying to reborrow for SharedReadWrite at alloc1445, but parent tag <3312> does not have an appropriate item in the borrow stack
   --> /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:324:18
    |
324 |         unsafe { &*self.as_ptr() }
    |                  ^^^^^^^^^^^^^^^ trying to reborrow for SharedReadWrite at alloc1445, but parent tag <3312> does not have an appropriate item in the borrow stack
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

    = note: inside `std::ptr::NonNull::<alloc::sync::ArcInner<futures::futures_executor::local_pool::ThreadNotify>>::as_ref` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/non_null.rs:324:18
    = note: inside `std::sync::Arc::<futures::futures_executor::local_pool::ThreadNotify>::inner` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1071:18
    = note: inside `<std::sync::Arc<futures::futures_executor::local_pool::ThreadNotify> as std::clone::Clone>::clone` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/sync.rs:1329:24
    = note: inside `<std::mem::ManuallyDrop<std::sync::Arc<futures::futures_executor::local_pool::ThreadNotify>> as std::clone::Clone>::clone` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/manually_drop.rs:50:5
    = note: inside `futures_task::waker::increase_refcount::<futures::futures_executor::local_pool::ThreadNotify>` at /home/user/workspace/fork/futures03/futures-task/src/waker.rs:36:44
    = note: inside `futures_task::waker::clone_arc_raw::<futures::futures_executor::local_pool::ThreadNotify>` at /home/user/workspace/fork/futures03/futures-task/src/waker.rs:41:5
    = note: inside `<std::task::Waker as std::clone::Clone>::clone` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/task/wake.rs:273:29
    = note: inside `futures::compat::compat01as03::<impl std::convert::From<futures::compat::compat01as03::WakerToHandle> for futures::task_impl::NotifyHandle>::from` at /home/user/workspace/fork/futures03/futures-util/src/compat/compat01as03.rs:327:40

I am not sure what this means, maybe @RalfJung can help.

[RalfJung]

This might be an instance of rust-lang/unsafe-code-guidelines#148? Unfortunately I don't have the time to dig further into this right now.

[RalfJung]

Would be good to try this again once rust-lang/miri#1952 lands and it shipped; that PR adds a (temporary) work-around for rust-lang/unsafe-code-guidelines#148, at least for self-referential generators.

[quininer]

I retried it at nightly (rustc 1.60.0-nightly 89b9f7b28 2022-01-10) and it now throws the error in another place.

error: Undefined Behavior: not granting access to tag <untagged> because incompatible item is protected: [SharedReadOnly for <4095> (call 1216)]
   --> /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/task/wake.rs:281:5
    |
281 | /     fn drop(&mut self) {
282 | |         // SAFETY: This is safe because `Waker::from_raw` is the only way
283 | |         // to initialize `drop` and `data` requiring the user to acknowledge
284 | |         // that the contract of `RawWaker` is upheld.
285 | |         unsafe { (self.waker.vtable.drop)(self.waker.data) }
286 | |     }
    | |_____^ not granting access to tag <untagged> because incompatible item is protected: [SharedReadOnly for <4095> (call 1216)]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

    = note: inside `<std::task::Waker as std::ops::Drop>::drop` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/task/wake.rs:281:5
    = note: inside `std::ptr::drop_in_place::<std::task::Waker> - shim(Some(std::task::Waker))` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `std::ptr::drop_in_place::<futures::compat::compat01as03::NotifyWaker> - shim(Some(futures::compat::compat01as03::NotifyWaker))` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `std::ptr::drop_in_place::<std::boxed::Box<dyn futures::task_impl::UnsafeNotify>> - shim(Some(std::boxed::Box<dyn futures::task_impl::UnsafeNotify>))` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
    = note: inside `std::mem::drop::<std::boxed::Box<dyn futures::task_impl::UnsafeNotify>>` at /home/user/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:909:24
    = note: inside `<futures::compat::compat01as03::NotifyWaker as futures::task_impl::UnsafeNotify>::drop_raw` at /home/user/workspace/fork/futures03/futures-util/src/compat/compat01as03.rs:346:9

[taiki-e]

I guess the SB violation is due to futures 0.1's UnsafeNotify::drop_raw.

futures-rs/src/task_impl/mod.rs

Lines 551 to 561 in 47b876f

	/// # Unsafety
	///
	/// This trait is manually encoding the memory management of the underlying
	/// handle, and as a result is quite unsafe to implement! Implementors of
	/// this trait must guarantee:
	///
	/// * Calls to `clone_raw` produce uniquely owned handles. It should be safe
	/// to drop the current handle and have the returned handle still be valid.
	/// * Calls to `drop_raw` work with `self` as a raw pointer, deallocating
	/// resources associated with it. This is a pretty unsafe operation as it's
	/// invalidating the `self` pointer, so extreme care needs to be taken.

futures-rs/src/task_impl/mod.rs

Lines 584 to 607 in 47b876f

	/// Drops this instance of `UnsafeNotify`, deallocating resources
	/// associated with it.
	///
	/// This method is intended to have a signature such as:
	///
	/// ```ignore
	/// fn drop_raw(self: *mut Self);
	/// ```
	///
	/// Unfortunately in Rust today that signature is not object safe.
	/// Nevertheless it's recommended to implement this function *as if* that
	/// were its signature. As such it is not safe to call on an invalid
	/// pointer, nor is the validity of the pointer guaranteed after this
	/// function returns.
	///
	/// # Unsafety
	///
	/// This trait is unsafe to implement, as are all these methods. This
	/// method is also unsafe to call as it's asserting the `UnsafeNotify`
	/// value is in a consistent state. In general it's recommended to
	/// review the trait documentation as well as the implementation for `Arc`
	/// in this crate. When in doubt ping the `futures` authors to clarify
	/// an unsafety question here.
	unsafe fn drop_raw(&self);

futures-rs/src/task_impl/std/mod.rs

Lines 642 to 657 in 47b876f

	// Safe implementation of `UnsafeNotify` for `Arc` in the standard library.
	//
	// Note that this is a very unsafe implementation! The crucial pieces is that
	// these two values are considered equivalent:
	//
	// * Arc<T>
	// * *const ArcWrapped<T>
	//
	// We don't actually know the layout of `ArcWrapped<T>` as it's an
	// implementation detail in the standard library. We can work, though, by
	// casting it through and back an `Arc<T>`.
	//
	// This also means that you won't actually fine `UnsafeNotify for Arc<T>`
	// because it's the wrong level of indirection. These methods are sort of
	// receiving Arc<T>, but not an owned version. It's... complicated. We may be
	// one of the first users of unsafe trait objects!