Skip to content

Commit acf18fd

Browse files
committed
Fix unsound compat01as03 implementation (fixes #2514)
Miri complains: > Undefined Behavior: trying to retag [...] for SharedReadWrite > permission [...], but that tag only grants SharedReadOnly permission for > this location in the UnsafeNotify01::drop_raw implementation for NotifyWaker. drop_raw should receive a `*mut Self`, but that isn't dyn compatible; it receives `&Self` instead (as documented by futures01 UnsafeNotify::drop_raw). Wrap in and access through UnsafeCell to fix the undefined behavior.
1 parent edccd29 commit acf18fd

2 files changed

Lines changed: 34 additions & 7 deletions

File tree

futures-util/src/compat/compat01as03.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use futures_core::{future::Future as Future03, stream::Stream as Stream03, task
99
#[cfg(feature = "sink")]
1010
use futures_sink::Sink as Sink03;
1111
use std::boxed::Box;
12+
use std::cell::UnsafeCell;
1213
use std::pin::Pin;
1314
use std::task::Context;
1415

@@ -319,33 +320,45 @@ where
319320
}
320321
}
321322

322-
struct NotifyWaker(task03::Waker);
323+
#[repr(transparent)]
324+
struct NotifyWaker(UnsafeCell<task03::Waker>);
323325

324326
#[derive(Clone)]
325327
struct WakerToHandle<'a>(&'a task03::Waker);
326328

327329
impl From<WakerToHandle<'_>> for NotifyHandle01 {
328330
fn from(handle: WakerToHandle<'_>) -> Self {
329-
let ptr = Box::new(NotifyWaker(handle.0.clone()));
331+
let waker_ptr: Box<task03::Waker> = Box::new(handle.0.clone());
332+
// NotifyWaker is a transparent (pointer compatible) wrapper for
333+
// task03::Waker (and wrapping in UnsafeCell is fine).
334+
let ptr: *mut NotifyWaker = Box::into_raw(waker_ptr) as *mut NotifyWaker;
330335

331-
unsafe { Self::new(Box::into_raw(ptr)) }
336+
unsafe { Self::new(ptr) }
332337
}
333338
}
334339

335340
impl Notify01 for NotifyWaker {
336341
fn notify(&self, _: usize) {
337-
self.0.wake_by_ref();
342+
unsafe { &*self.0.get() }.wake_by_ref();
338343
}
339344
}
340345

346+
unsafe impl Send for NotifyWaker {}
347+
unsafe impl Sync for NotifyWaker {}
348+
341349
unsafe impl UnsafeNotify01 for NotifyWaker {
342350
unsafe fn clone_raw(&self) -> NotifyHandle01 {
343-
WakerToHandle(&self.0).into()
351+
WakerToHandle(unsafe { &*self.0.get() }).into()
344352
}
345353

346354
unsafe fn drop_raw(&self) {
347-
let ptr: *const dyn UnsafeNotify01 = self;
348-
drop(unsafe { Box::from_raw(ptr as *mut dyn UnsafeNotify01) });
355+
/* UnsafeNotify01::drop_raw says this should receive `*mut Self`,
356+
* but that isn't dyn compatible.
357+
* miri is unhappy when a `*mut` is created from a `&` reference,
358+
* so need to go through `UnsafeCell`.
359+
*/
360+
let waker: *mut task03::Waker = self.0.get();
361+
drop(unsafe { Box::from_raw(waker) });
349362
}
350363
}
351364

futures/tests/compat_miri.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#![cfg(feature = "compat")]
2+
use futures::compat::Future01CompatExt;
3+
use futures::executor::block_on;
4+
use futures::future::TryFutureExt;
5+
6+
#[test]
7+
// from https://github.com/rust-lang/futures-rs/issues/2514
8+
fn miri_compat_two_way() {
9+
let fut = async { Ok(()) as Result<(), ()> };
10+
let fut = Box::pin(fut);
11+
let fut = fut.compat(); // 03as01
12+
let fut = fut.compat(); // 01as03
13+
block_on(fut).unwrap();
14+
}

0 commit comments

Comments
 (0)