From caf7db4bc9bb0d11f582333ebd473b190e037228 Mon Sep 17 00:00:00 2001 From: Pointerbender Date: Tue, 4 Oct 2022 16:43:45 +0200 Subject: [PATCH] fix soundness hole in join macros add a miri regression test update failing tests (join sizes increased due to fix) --- futures-macro/src/join.rs | 13 ++++++------ futures/tests/async_await_macros.rs | 8 ++++---- futures/tests/future_join.rs | 32 +++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 10 deletions(-) create mode 100644 futures/tests/future_join.rs diff --git a/futures-macro/src/join.rs b/futures-macro/src/join.rs index d427da27a0..94e356f729 100644 --- a/futures-macro/src/join.rs +++ b/futures-macro/src/join.rs @@ -38,6 +38,7 @@ fn bind_futures(fut_exprs: Vec, span: Span) -> (Vec, Vec TokenStream { let poll_futures = future_names.iter().map(|fut| { quote! { __all_done &= __futures_crate::future::Future::poll( - unsafe { __futures_crate::Pin::new_unchecked(&mut #fut) }, __cx).is_ready(); + #fut.as_mut(), __cx).is_ready(); } }); let take_outputs = future_names.iter().map(|fut| { quote! { - unsafe { __futures_crate::Pin::new_unchecked(&mut #fut) }.take_output().unwrap(), + #fut.as_mut().take_output().unwrap(), } }); @@ -96,17 +97,17 @@ pub(crate) fn try_join(input: TokenStream) -> TokenStream { let poll_futures = future_names.iter().map(|fut| { quote! { if __futures_crate::future::Future::poll( - unsafe { __futures_crate::Pin::new_unchecked(&mut #fut) }, __cx).is_pending() + #fut.as_mut(), __cx).is_pending() { __all_done = false; - } else if unsafe { __futures_crate::Pin::new_unchecked(&mut #fut) }.output_mut().unwrap().is_err() { + } else if #fut.as_mut().output_mut().unwrap().is_err() { // `.err().unwrap()` rather than `.unwrap_err()` so that we don't introduce // a `T: Debug` bound. // Also, for an error type of ! any code after `err().unwrap()` is unreachable. #[allow(unreachable_code)] return __futures_crate::task::Poll::Ready( __futures_crate::Err( - unsafe { __futures_crate::Pin::new_unchecked(&mut #fut) }.take_output().unwrap().err().unwrap() + #fut.as_mut().take_output().unwrap().err().unwrap() ) ); } @@ -118,7 +119,7 @@ pub(crate) fn try_join(input: TokenStream) -> TokenStream { // an `E: Debug` bound. // Also, for an ok type of ! any code after `ok().unwrap()` is unreachable. #[allow(unreachable_code)] - unsafe { __futures_crate::Pin::new_unchecked(&mut #fut) }.take_output().unwrap().ok().unwrap(), + #fut.as_mut().take_output().unwrap().ok().unwrap(), } }); diff --git a/futures/tests/async_await_macros.rs b/futures/tests/async_await_macros.rs index 8c51d66234..7579f333ac 100644 --- a/futures/tests/async_await_macros.rs +++ b/futures/tests/async_await_macros.rs @@ -352,14 +352,14 @@ fn join_size() { let ready = future::ready(0i32); join!(ready) }; - assert_eq!(mem::size_of_val(&fut), 12); + assert_eq!(mem::size_of_val(&fut), 24); let fut = async { let ready1 = future::ready(0i32); let ready2 = future::ready(0i32); join!(ready1, ready2) }; - assert_eq!(mem::size_of_val(&fut), 20); + assert_eq!(mem::size_of_val(&fut), 40); } #[test] @@ -368,14 +368,14 @@ fn try_join_size() { let ready = future::ready(Ok::(0)); try_join!(ready) }; - assert_eq!(mem::size_of_val(&fut), 16); + assert_eq!(mem::size_of_val(&fut), 24); let fut = async { let ready1 = future::ready(Ok::(0)); let ready2 = future::ready(Ok::(0)); try_join!(ready1, ready2) }; - assert_eq!(mem::size_of_val(&fut), 28); + assert_eq!(mem::size_of_val(&fut), 48); } #[test] diff --git a/futures/tests/future_join.rs b/futures/tests/future_join.rs new file mode 100644 index 0000000000..f5df9d7775 --- /dev/null +++ b/futures/tests/future_join.rs @@ -0,0 +1,32 @@ +use futures::executor::block_on; +use futures::future::Future; +use std::task::Poll; + +/// This tests verifies (through miri) that self-referencing +/// futures are not invalidated when joining them. +#[test] +fn futures_join_macro_self_referential() { + block_on(async { futures::join!(yield_now(), trouble()) }); +} + +async fn trouble() { + let lucky_number = 42; + let problematic_variable = &lucky_number; + + yield_now().await; + + // problematic dereference + let _ = { *problematic_variable }; +} + +fn yield_now() -> impl Future { + let mut yielded = false; + std::future::poll_fn(move |cx| { + if core::mem::replace(&mut yielded, true) { + Poll::Ready(()) + } else { + cx.waker().wake_by_ref(); + Poll::Pending + } + }) +}