From e6fb4d1a6550c4220fea365ff4681cc2d0585e8d Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Tue, 14 Jan 2020 21:06:41 +0100 Subject: [PATCH] Reimplement Module::ResolveCallback ABI fix without global variables (#207) --- src/binding.cc | 24 +------ src/module.rs | 88 ++++++++++++++++++++++--- src/support.rs | 164 +++++++++++++++++++++++++++++++++++++++++++++- tests/test_api.rs | 34 +++++----- 4 files changed, 261 insertions(+), 49 deletions(-) diff --git a/src/binding.cc b/src/binding.cc index 16e570c4..811fd864 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -1353,30 +1353,10 @@ int v8__Module__GetIdentityHash(const v8::Module& self) { return self.GetIdentityHash(); } -// This is an extern C calling convention compatible version of -// v8::Module::ResolveCallback. -using v8__Module__ResolveCallback = - const v8::Module* (*)(v8::Local context, - v8::Local specifier, - v8::Local referrer); - MaybeBool v8__Module__InstantiateModule(v8::Module& self, v8::Local context, - v8__Module__ResolveCallback c_cb) { - thread_local v8__Module__ResolveCallback static_cb = nullptr; - assert(static_cb == nullptr); - static_cb = c_cb; - - auto cxx_cb = [](v8::Local context, - v8::Local specifier, - v8::Local referrer) { - const auto* m = static_cb(context, specifier, referrer); - return ptr_to_maybe_local(const_cast(m)); - }; - - auto r = maybe_to_maybe_bool(self.InstantiateModule(context, cxx_cb)); - static_cb = nullptr; - return r; + v8::Module::ResolveCallback cb) { + return maybe_to_maybe_bool(self.InstantiateModule(context, cb)); } v8::Value* v8__Module__Evaluate(v8::Module& self, diff --git a/src/module.rs b/src/module.rs index 8aca7cf3..6bb9ea2a 100644 --- a/src/module.rs +++ b/src/module.rs @@ -1,19 +1,86 @@ +use std::mem::MaybeUninit; +use std::ptr::null; + use crate::support::int; +use crate::support::MapFnFrom; +use crate::support::MapFnTo; use crate::support::MaybeBool; +use crate::support::ToCFn; +use crate::support::UnitType; use crate::Context; use crate::Local; use crate::Module; use crate::String; use crate::ToLocal; use crate::Value; -use std::mem::MaybeUninit; /// Called during Module::instantiate_module. Provided with arguments: -/// (context, specifier, referrer) -/// Return null on error. -/// Hint: to tranform Local to *const Module use `&*module`. -pub type ResolveCallback = - extern "C" fn(Local, Local, Local) -> *const Module; +/// (context, specifier, referrer). Return None on error. +/// +/// Note: this callback has an unusual signature due to ABI incompatibilities +/// between Rust and C++. However end users can implement the callback as +/// follows; it'll be automatically converted. +/// +/// ```rust,ignore +/// fn my_resolve_callback<'a>( +/// context: v8::Local<'a, v8::Context>, +/// specifier: v8::Local<'a, v8::String>, +/// referrer: v8::Local<'a, v8::Module>, +/// ) -> Option> { +/// // ... +/// Some(resolved_module) +/// } +/// ``` +/// + +// System V AMD64 ABI: Local returned in a register. +#[cfg(not(target_os = "windows"))] +pub type ResolveCallback<'a> = extern "C" fn( + Local<'a, Context>, + Local<'a, String>, + Local<'a, Module>, +) -> *const Module; + +// Windows x64 ABI: Local returned on the stack. +#[cfg(target_os = "windows")] +pub type ResolveCallback<'a> = extern "C" fn( + *mut *const Module, + Local<'a, Context>, + Local<'a, String>, + Local<'a, Module>, +) -> *mut *const Module; + +impl<'a, F> MapFnFrom for ResolveCallback<'a> +where + F: UnitType + + Fn( + Local<'a, Context>, + Local<'a, String>, + Local<'a, Module>, + ) -> Option>, +{ + #[cfg(not(target_os = "windows"))] + fn mapping() -> Self { + let f = |context, specifier, referrer| { + (F::get())(context, specifier, referrer) + .map(|r| -> *const Module { &*r }) + .unwrap_or(null()) + }; + f.to_c_fn() + } + + #[cfg(target_os = "windows")] + fn mapping() -> Self { + let f = |ret_ptr, context, specifier, referrer| { + let r = (F::get())(context, specifier, referrer) + .map(|r| -> *const Module { &*r }) + .unwrap_or(null()); + unsafe { std::ptr::write(ret_ptr, r) }; // Write result to stack. + ret_ptr // Return stack pointer to the return value. + }; + f.to_c_fn() + } +} extern "C" { fn v8__Module__GetStatus(this: *const Module) -> ModuleStatus; @@ -120,12 +187,15 @@ impl Module { /// instantiation. (In the case where the callback throws an exception, that /// exception is propagated.) #[must_use] - pub fn instantiate_module( + pub fn instantiate_module<'a>( &mut self, context: Local, - callback: ResolveCallback, + callback: impl MapFnTo>, ) -> Option { - unsafe { v8__Module__InstantiateModule(self, context, callback) }.into() + unsafe { + v8__Module__InstantiateModule(self, context, callback.map_fn_to()) + } + .into() } /// Evaluates the module and its dependencies. diff --git a/src/support.rs b/src/support.rs index 2998f879..209be17d 100644 --- a/src/support.rs +++ b/src/support.rs @@ -15,7 +15,7 @@ pub use std::os::raw::c_char as char; pub use std::os::raw::c_int as int; pub use std::os::raw::c_long as long; -pub type Opaque = [usize; 0]; +pub type Opaque = [u8; 0]; pub trait Delete where @@ -273,3 +273,165 @@ impl Into> for Maybe { } } } + +pub trait UnitType +where + Self: Copy + Sized, +{ + #[inline(always)] + fn get() -> Self { + UnitValue::::get() + } +} + +impl UnitType for T where T: Copy + Sized {} + +#[derive(Copy, Clone)] +struct UnitValue(PhantomData) +where + Self: Sized; + +impl UnitValue +where + Self: Copy + Sized, +{ + const SELF: Self = Self::new_checked(); + + const fn new_checked() -> Self { + // Statically assert that T is indeed a unit type. + let size_must_be_0 = size_of::(); + let s = Self(PhantomData::); + [s][size_must_be_0] + } + + #[inline(always)] + fn get_checked(self) -> T { + // This run-time check serves just as a backup for the compile-time + // check when Self::SELF is initialized. + assert_eq!(size_of::(), 0); + unsafe { std::mem::MaybeUninit::::zeroed().assume_init() } + } + + #[inline(always)] + pub fn get() -> T { + // Accessing the Self::SELF is necessary to make the compile-time type check + // work. + Self::SELF.get_checked() + } +} + +pub struct DefaultTag; +pub struct IdenticalConversionTag; + +pub trait MapFnFrom +where + F: UnitType, + Self: Sized, +{ + fn mapping() -> Self; + + #[inline(always)] + fn map_fn_from(_: F) -> Self { + Self::mapping() + } +} + +impl MapFnFrom for F +where + Self: UnitType, +{ + #[inline(always)] + fn mapping() -> Self { + Self::get() + } +} + +pub trait MapFnTo +where + Self: UnitType, + T: Sized, +{ + fn mapping() -> T; + + #[inline(always)] + fn map_fn_to(self) -> T { + Self::mapping() + } +} + +impl MapFnTo for F +where + Self: UnitType, + T: MapFnFrom, +{ + #[inline(always)] + fn mapping() -> T { + T::map_fn_from(F::get()) + } +} + +pub trait CFnFrom +where + Self: Sized, + F: UnitType, +{ + fn mapping() -> Self; + + #[inline(always)] + fn c_fn_from(_: F) -> Self { + Self::mapping() + } +} + +macro_rules! impl_c_fn_from { + ($($arg:ident: $ty:ident),*) => { + impl CFnFrom for extern "C" fn($($ty),*) -> R + where + F: UnitType + Fn($($ty),*) -> R, + { + #[inline(always)] + fn mapping() -> Self { + extern "C" fn c_fn($($arg: $ty),*) -> R + where + F: UnitType + Fn($($ty),*) -> R, + { + (F::get())($($arg),*) + }; + c_fn:: + } + } + }; +} + +impl_c_fn_from!(); +impl_c_fn_from!(a0: A0); +impl_c_fn_from!(a0: A0, a1: A1); +impl_c_fn_from!(a0: A0, a1: A1, a2: A2); +impl_c_fn_from!(a0: A0, a1: A1, a2: A2, a3: A3); +impl_c_fn_from!(a0: A0, a1: A1, a2: A2, a3: A3, a4: A4); +impl_c_fn_from!(a0: A0, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5); +impl_c_fn_from!(a0: A0, a1: A1, a2: A2, a3: A3, a4: A4, a5: A5, a6: A6); + +pub trait ToCFn +where + Self: UnitType, + T: Sized, +{ + fn mapping() -> T; + + #[inline(always)] + fn to_c_fn(self) -> T { + Self::mapping() + } +} + +impl ToCFn for F +where + Self: UnitType, + T: CFnFrom, +{ + #[inline(always)] + fn mapping() -> T { + T::c_fn_from(F::get()) + } +} diff --git a/tests/test_api.rs b/tests/test_api.rs index df9f770d..b032ba7c 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -541,11 +541,11 @@ fn add_message_listener() { drop(g); } -extern "C" fn unexpected_module_resolve_callback( - _context: v8::Local, - _specifier: v8::Local, - _referrer: v8::Local, -) -> *const v8::Module { +fn unexpected_module_resolve_callback<'a>( + _context: v8::Local<'a, v8::Context>, + _specifier: v8::Local<'a, v8::String>, + _referrer: v8::Local<'a, v8::Module>, +) -> Option> { unreachable!() } @@ -1289,17 +1289,17 @@ fn module_instantiation_failures1() { { let mut try_catch = v8::TryCatch::new(scope); let tc = try_catch.enter(); - extern "C" fn resolve_callback( - context: v8::Local, - _specifier: v8::Local, - _referrer: v8::Local, - ) -> *const v8::Module { + fn resolve_callback<'a>( + context: v8::Local<'a, v8::Context>, + _specifier: v8::Local<'a, v8::String>, + _referrer: v8::Local<'a, v8::Module>, + ) -> Option> { let mut cbs = v8::CallbackScope::new(context); let mut hs = v8::HandleScope::new(cbs.enter()); let scope = hs.enter(); let e = v8_str(scope, "boom"); scope.isolate().throw_exception(e.into()); - std::ptr::null() + None } let result = module.instantiate_module(context, resolve_callback); assert!(result.is_none()); @@ -1318,11 +1318,11 @@ fn module_instantiation_failures1() { drop(g); } -extern "C" fn compile_specifier_as_module_resolve_callback( - context: v8::Local, - specifier: v8::Local, - _referrer: v8::Local, -) -> *const v8::Module { +fn compile_specifier_as_module_resolve_callback<'a>( + context: v8::Local<'a, v8::Context>, + specifier: v8::Local<'a, v8::String>, + _referrer: v8::Local<'a, v8::Module>, +) -> Option> { let mut cbs = v8::CallbackScope::new(context); let mut hs = v8::EscapableHandleScope::new(cbs.enter()); let scope = hs.enter(); @@ -1330,7 +1330,7 @@ extern "C" fn compile_specifier_as_module_resolve_callback( let source = v8::script_compiler::Source::new(specifier, &origin); let module = v8::script_compiler::compile_module(scope.isolate(), source).unwrap(); - &*scope.escape(module) + Some(scope.escape(module)) } #[test]