diff --git a/library/proc_macro/src/bridge/client.rs b/library/proc_macro/src/bridge/client.rs index cdb2bac260757..144de5822c50c 100644 --- a/library/proc_macro/src/bridge/client.rs +++ b/library/proc_macro/src/bridge/client.rs @@ -2,6 +2,7 @@ use super::*; +use std::cell::Cell; use std::marker::PhantomData; macro_rules! define_handles { @@ -256,7 +257,7 @@ macro_rules! define_client_side { api_tags::Method::$name(api_tags::$name::$method).encode(&mut b, &mut ()); reverse_encode!(b; $($arg),*); - b = bridge.dispatch.call(b); + b = (bridge.dispatch)(b); let r = Result::<_, PanicMessage>::decode(&mut &b[..], &mut ()); @@ -270,48 +271,64 @@ macro_rules! define_client_side { } with_api!(self, self, define_client_side); -enum BridgeState<'a> { +enum BridgeState { /// No server is currently connected to this client. NotConnected, /// A server is connected and available for requests. - Connected(Bridge<'a>), + Connected(Bridge), /// Access to the bridge is being exclusively acquired /// (e.g., during `BridgeState::with`). InUse, } -enum BridgeStateL {} +impl BridgeState { + /// Sets the thread-local `BridgeState` to `replacement` while + /// running `f`, which gets the old `BridgeState`, mutably. + /// + /// The state will be restored after `f` exits, even + /// by panic, including modifications made to it by `f`. + fn replace_during(replacement: Self, f: impl FnOnce(&mut Self) -> R) -> R { + /// Wrapper that ensures that a cell always gets filled + /// (with the original state, optionally changed by `f`), + /// even if `f` had panicked. + struct PutBackOnDrop<'a, T> { + cell: &'a Cell, + value: Option, + } -impl<'a> scoped_cell::ApplyL<'a> for BridgeStateL { - type Out = BridgeState<'a>; -} + impl<'a, T> Drop for PutBackOnDrop<'a, T> { + fn drop(&mut self) { + self.cell.set(self.value.take().unwrap()); + } + } -thread_local! { - static BRIDGE_STATE: scoped_cell::ScopedCell = - scoped_cell::ScopedCell::new(BridgeState::NotConnected); -} + thread_local! { + static BRIDGE_STATE: Cell = Cell::new(BridgeState::NotConnected); + } + BRIDGE_STATE.with(|state| { + let mut put_back_on_drop = + PutBackOnDrop { cell: state, value: Some(state.replace(replacement)) }; + + f(put_back_on_drop.value.as_mut().unwrap()) + }) + } -impl BridgeState<'_> { /// Take exclusive control of the thread-local /// `BridgeState`, and pass it to `f`, mutably. + /// /// The state will be restored after `f` exits, even /// by panic, including modifications made to it by `f`. /// /// N.B., while `f` is running, the thread-local state /// is `BridgeState::InUse`. - fn with(f: impl FnOnce(&mut BridgeState<'_>) -> R) -> R { - BRIDGE_STATE.with(|state| { - state.replace(BridgeState::InUse, |mut state| { - // FIXME(#52812) pass `f` directly to `replace` when `RefMutL` is gone - f(&mut *state) - }) - }) + fn with(f: impl FnOnce(&mut Self) -> R) -> R { + Self::replace_during(BridgeState::InUse, f) } } -impl Bridge<'_> { +impl Bridge { pub(crate) fn is_available() -> bool { BridgeState::with(|state| match state { BridgeState::Connected(_) | BridgeState::InUse => true, @@ -337,10 +354,10 @@ impl Bridge<'_> { })); }); - BRIDGE_STATE.with(|state| state.set(BridgeState::Connected(self), f)) + BridgeState::replace_during(BridgeState::Connected(self), |_| f()) } - fn with(f: impl FnOnce(&mut Bridge<'_>) -> R) -> R { + fn with(f: impl FnOnce(&mut Self) -> R) -> R { BridgeState::with(|state| match state { BridgeState::NotConnected => { panic!("procedural macro API is used outside of a procedural macro"); @@ -367,7 +384,7 @@ pub struct Client { // FIXME(eddyb) use a reference to the `static COUNTERS`, instead of // a wrapper `fn` pointer, once `const fn` can reference `static`s. pub(super) get_handle_counters: extern "C" fn() -> &'static HandleCounters, - pub(super) run: extern "C" fn(Bridge<'_>, F) -> Buffer, + pub(super) run: extern "C" fn(Bridge, F) -> Buffer, pub(super) f: F, } @@ -375,7 +392,7 @@ pub struct Client { /// deserializing input and serializing output. // FIXME(eddyb) maybe replace `Bridge::enter` with this? fn run_client DecodeMut<'a, 's, ()>, R: Encode<()>>( - mut bridge: Bridge<'_>, + mut bridge: Bridge, f: impl FnOnce(A) -> R, ) -> Buffer { // The initial `cached_buffer` contains the input. @@ -418,7 +435,7 @@ fn run_client DecodeMut<'a, 's, ()>, R: Encode<()>>( impl Client crate::TokenStream> { pub const fn expand1(f: fn(crate::TokenStream) -> crate::TokenStream) -> Self { extern "C" fn run( - bridge: Bridge<'_>, + bridge: Bridge, f: impl FnOnce(crate::TokenStream) -> crate::TokenStream, ) -> Buffer { run_client(bridge, |input| f(crate::TokenStream(input)).0) @@ -432,7 +449,7 @@ impl Client crate::TokenStream> { f: fn(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, ) -> Self { extern "C" fn run( - bridge: Bridge<'_>, + bridge: Bridge, f: impl FnOnce(crate::TokenStream, crate::TokenStream) -> crate::TokenStream, ) -> Buffer { run_client(bridge, |(input, input2)| { diff --git a/library/proc_macro/src/bridge/closure.rs b/library/proc_macro/src/bridge/closure.rs deleted file mode 100644 index 06f76d2fc9140..0000000000000 --- a/library/proc_macro/src/bridge/closure.rs +++ /dev/null @@ -1,28 +0,0 @@ -//! Closure type (equivalent to `&mut dyn FnMut(A) -> R`) that's `repr(C)`. - -use std::marker::PhantomData; - -#[repr(C)] -pub struct Closure<'a, A, R> { - call: unsafe extern "C" fn(*mut Env, A) -> R, - env: *mut Env, - // Ensure Closure is !Send and !Sync - _marker: PhantomData<*mut &'a mut ()>, -} - -struct Env; - -impl<'a, A, R, F: FnMut(A) -> R> From<&'a mut F> for Closure<'a, A, R> { - fn from(f: &'a mut F) -> Self { - unsafe extern "C" fn call R>(env: *mut Env, arg: A) -> R { - (*(env as *mut _ as *mut F))(arg) - } - Closure { call: call::, env: f as *mut _ as *mut Env, _marker: PhantomData } - } -} - -impl<'a, A, R> Closure<'a, A, R> { - pub fn call(&mut self, arg: A) -> R { - unsafe { (self.call)(self.env, arg) } - } -} diff --git a/library/proc_macro/src/bridge/mod.rs b/library/proc_macro/src/bridge/mod.rs index f7c9df6564f87..0368a4f2243c9 100644 --- a/library/proc_macro/src/bridge/mod.rs +++ b/library/proc_macro/src/bridge/mod.rs @@ -199,8 +199,6 @@ macro_rules! reverse_decode { mod buffer; #[forbid(unsafe_code)] pub mod client; -#[allow(unsafe_code)] -mod closure; #[forbid(unsafe_code)] mod handle; #[macro_use] @@ -221,13 +219,13 @@ use rpc::{Decode, DecodeMut, Encode, Reader, Writer}; /// field of `client::Client`. The client holds its copy of the `Bridge` /// in TLS during its execution (`Bridge::{enter, with}` in `client.rs`). #[repr(C)] -pub struct Bridge<'a> { +pub struct Bridge { /// Reusable buffer (only `clear`-ed, never shrunk), primarily /// used for making requests, but also for passing input to client. cached_buffer: Buffer, /// Server-side function that the client uses to make requests. - dispatch: closure::Closure<'a, Buffer, Buffer>, + dispatch: extern "C" fn(Buffer) -> Buffer, /// If 'true', always invoke the default panic hook force_show_panics: bool, diff --git a/library/proc_macro/src/bridge/scoped_cell.rs b/library/proc_macro/src/bridge/scoped_cell.rs index 2cde1f65adf9c..99d3c468b1c7d 100644 --- a/library/proc_macro/src/bridge/scoped_cell.rs +++ b/library/proc_macro/src/bridge/scoped_cell.rs @@ -41,9 +41,10 @@ impl ScopedCell { /// Sets the value in `self` to `replacement` while /// running `f`, which gets the old value, mutably. + /// /// The old value will be restored after `f` exits, even /// by panic, including modifications made to it by `f`. - pub fn replace<'a, R>( + pub fn replace_during<'a, R>( &self, replacement: >::Out, f: impl for<'b, 'c> FnOnce(RefMutL<'b, 'c, T>) -> R, @@ -76,6 +77,6 @@ impl ScopedCell { /// Sets the value in `self` to `value` while running `f`. pub fn set(&self, value: >::Out, f: impl FnOnce() -> R) -> R { - self.replace(value, |_| f()) + self.replace_during(value, |_| f()) } } diff --git a/library/proc_macro/src/bridge/server.rs b/library/proc_macro/src/bridge/server.rs index 2e0400d32a0af..185b9c9377efd 100644 --- a/library/proc_macro/src/bridge/server.rs +++ b/library/proc_macro/src/bridge/server.rs @@ -85,14 +85,16 @@ macro_rules! define_dispatcher_impl { ($($name:ident { $(fn $method:ident($($arg:ident: $arg_ty:ty),* $(,)?) $(-> $ret_ty:ty)?;)* }),* $(,)?) => { - // FIXME(eddyb) `pub` only for `ExecutionStrategy` below. - pub trait DispatcherTrait { - // HACK(eddyb) these are here to allow `Self::$name` to work below. + // HACK(eddyb) only a (private) trait because inherent impls can't have + // associated types (and `Self::AssocType` is used within `$arg_ty`). + // While `with_api!` allows customizing `Self`, it would have to be + // extended to allow `Self::` to become ` as Types>::`. + trait DispatcherPrivateHelperTrait { $(type $name;)* fn dispatch(&mut self, b: Buffer) -> Buffer; } - impl DispatcherTrait for Dispatcher> { + impl DispatcherPrivateHelperTrait for Dispatcher> { $(type $name = as Types>::$name;)* fn dispatch(&mut self, mut b: Buffer) -> Buffer { let Dispatcher { handle_store, server } = self; @@ -128,39 +130,27 @@ macro_rules! define_dispatcher_impl { } with_api!(Self, self_, define_dispatcher_impl); +// FIXME(eddyb) a trait alias of `FnMut(Buffer) -> Buffer` would allow +// replacing the non-dynamic mentions of that trait, as well. +pub type DynDispatch<'a> = &'a mut dyn FnMut(Buffer) -> Buffer; + pub trait ExecutionStrategy { - fn run_bridge_and_client( + fn cross_thread_dispatch( &self, - dispatcher: &mut impl DispatcherTrait, - input: Buffer, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, - client_data: D, - force_show_panics: bool, + server_thread_dispatch: impl FnMut(Buffer) -> Buffer, + with_client_thread_dispatch: impl FnOnce(DynDispatch<'_>) -> Buffer + Send + 'static, ) -> Buffer; } pub struct SameThread; impl ExecutionStrategy for SameThread { - fn run_bridge_and_client( + fn cross_thread_dispatch( &self, - dispatcher: &mut impl DispatcherTrait, - input: Buffer, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, - client_data: D, - force_show_panics: bool, + mut server_thread_dispatch: impl FnMut(Buffer) -> Buffer, + with_client_thread_dispatch: impl FnOnce(DynDispatch<'_>) -> Buffer + Send + 'static, ) -> Buffer { - let mut dispatch = |b| dispatcher.dispatch(b); - - run_client( - Bridge { - cached_buffer: input, - dispatch: (&mut dispatch).into(), - force_show_panics, - _marker: marker::PhantomData, - }, - client_data, - ) + with_client_thread_dispatch(&mut server_thread_dispatch) } } @@ -170,13 +160,10 @@ impl ExecutionStrategy for SameThread { pub struct CrossThread1; impl ExecutionStrategy for CrossThread1 { - fn run_bridge_and_client( + fn cross_thread_dispatch( &self, - dispatcher: &mut impl DispatcherTrait, - input: Buffer, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, - client_data: D, - force_show_panics: bool, + mut server_thread_dispatch: impl FnMut(Buffer) -> Buffer, + with_client_thread_dispatch: impl FnOnce(DynDispatch<'_>) -> Buffer + Send + 'static, ) -> Buffer { use std::sync::mpsc::channel; @@ -184,24 +171,14 @@ impl ExecutionStrategy for CrossThread1 { let (res_tx, res_rx) = channel(); let join_handle = thread::spawn(move || { - let mut dispatch = |b| { + with_client_thread_dispatch(&mut |b| { req_tx.send(b).unwrap(); res_rx.recv().unwrap() - }; - - run_client( - Bridge { - cached_buffer: input, - dispatch: (&mut dispatch).into(), - force_show_panics, - _marker: marker::PhantomData, - }, - client_data, - ) + }) }); for b in req_rx { - res_tx.send(dispatcher.dispatch(b)).unwrap(); + res_tx.send(server_thread_dispatch(b)).unwrap(); } join_handle.join().unwrap() @@ -211,13 +188,10 @@ impl ExecutionStrategy for CrossThread1 { pub struct CrossThread2; impl ExecutionStrategy for CrossThread2 { - fn run_bridge_and_client( + fn cross_thread_dispatch( &self, - dispatcher: &mut impl DispatcherTrait, - input: Buffer, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, - client_data: D, - force_show_panics: bool, + mut server_thread_dispatch: impl FnMut(Buffer) -> Buffer, + with_client_thread_dispatch: impl FnOnce(DynDispatch<'_>) -> Buffer + Send + 'static, ) -> Buffer { use std::sync::{Arc, Mutex}; @@ -231,7 +205,7 @@ impl ExecutionStrategy for CrossThread2 { let server_thread = thread::current(); let state2 = state.clone(); let join_handle = thread::spawn(move || { - let mut dispatch = |b| { + let r = with_client_thread_dispatch(&mut |b| { *state2.lock().unwrap() = State::Req(b); server_thread.unpark(); loop { @@ -240,17 +214,7 @@ impl ExecutionStrategy for CrossThread2 { break b.take(); } } - }; - - let r = run_client( - Bridge { - cached_buffer: input, - dispatch: (&mut dispatch).into(), - force_show_panics, - _marker: marker::PhantomData, - }, - client_data, - ); + }); // Wake up the server so it can exit the dispatch loop. drop(state2); @@ -266,7 +230,7 @@ impl ExecutionStrategy for CrossThread2 { State::Req(b) => b.take(), _ => continue, }; - b = dispatcher.dispatch(b.take()); + b = server_thread_dispatch(b.take()); *state.lock().unwrap() = State::Res(b); join_handle.thread().unpark(); } @@ -275,6 +239,60 @@ impl ExecutionStrategy for CrossThread2 { } } +fn run_bridge_and_client( + strategy: &impl ExecutionStrategy, + server_thread_dispatch: impl FnMut(Buffer) -> Buffer, + input: Buffer, + run_client: extern "C" fn(Bridge, D) -> Buffer, + client_data: D, + force_show_panics: bool, +) -> Buffer { + enum OptionDynDispatchL {} + + impl<'a> scoped_cell::ApplyL<'a> for OptionDynDispatchL { + type Out = Option>; + } + + thread_local! { + /// Dispatch callback held in server TLS, and using server ABI, but + /// on the client thread (which can differ from the server thread). + // + // FIXME(eddyb) how redundant is this with the (also) thread-local + // client-side `BridgeState`? Some of that indirection can probably + // be removed, as long as concerns around further isolation methods + // (IPC and/or wasm) are considered. + static CLIENT_THREAD_DISPATCH: scoped_cell::ScopedCell = + scoped_cell::ScopedCell::new(None); + } + + strategy.cross_thread_dispatch(server_thread_dispatch, move |client_thread_dispatch| { + CLIENT_THREAD_DISPATCH.with(|dispatch_slot| { + dispatch_slot.set(Some(client_thread_dispatch), || { + extern "C" fn dispatch(b: Buffer) -> Buffer { + CLIENT_THREAD_DISPATCH.with(|dispatch_slot| { + dispatch_slot.replace_during(None, |mut client_thread_dispatch| { + client_thread_dispatch.as_mut().expect( + "proc_macro::bridge::server: dispatch used on \ + thread lacking an active server-side dispatcher", + )(b) + }) + }) + } + + run_client( + Bridge { + cached_buffer: input, + dispatch, + force_show_panics, + _marker: marker::PhantomData, + }, + client_data, + ) + }) + }) + }) +} + fn run_server< S: Server, I: Encode>>, @@ -285,7 +303,7 @@ fn run_server< handle_counters: &'static client::HandleCounters, server: S, input: I, - run_client: extern "C" fn(Bridge<'_>, D) -> Buffer, + run_client: extern "C" fn(Bridge, D) -> Buffer, client_data: D, force_show_panics: bool, ) -> Result { @@ -295,8 +313,9 @@ fn run_server< let mut b = Buffer::new(); input.encode(&mut b, &mut dispatcher.handle_store); - b = strategy.run_bridge_and_client( - &mut dispatcher, + b = run_bridge_and_client( + strategy, + |b| dispatcher.dispatch(b), b, run_client, client_data, diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index 64e25f803b27f..72b7f5f6612d8 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -156,7 +156,7 @@ pub(crate) fn maybe_download_ci_llvm(builder: &Builder<'_>) { let llvm_lib = llvm_root.join("lib"); for entry in t!(fs::read_dir(&llvm_lib)) { let lib = t!(entry).path(); - if lib.ends_with(".so") { + if lib.extension().map_or(false, |ext| ext == "so") { fix_bin_or_dylib(builder, &lib); } } @@ -284,7 +284,7 @@ fn fix_bin_or_dylib(builder: &Builder<'_>, fname: &Path) { entries }; patchelf.args(&[OsString::from("--set-rpath"), rpath_entries]); - if !fname.ends_with(".so") { + if !fname.extension().map_or(false, |ext| ext == "so") { // Finally, set the corret .interp for binaries let dynamic_linker_path = nix_deps_dir.join("nix-support/dynamic-linker"); // FIXME: can we support utf8 here? `args` doesn't accept Vec, only OsString ...