From 5ea79cdd63dfc7b6254a12a3f54bfd811b3478f2 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Mon, 28 Oct 2024 00:27:15 -0400 Subject: [PATCH 1/4] Add vtable to Rooted when T has a Traceable RootKind value. Signed-off-by: Josh Matthews --- mozjs-sys/src/jsgc.rs | 150 +++++++++++++++++++++++++++------------ mozjs-sys/src/jsglue.cpp | 4 ++ mozjs-sys/src/jsimpls.rs | 57 ++++++++------- 3 files changed, 142 insertions(+), 69 deletions(-) diff --git a/mozjs-sys/src/jsgc.rs b/mozjs-sys/src/jsgc.rs index a80ab14c21f..382fb0ae756 100644 --- a/mozjs-sys/src/jsgc.rs +++ b/mozjs-sys/src/jsgc.rs @@ -2,91 +2,145 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +use crate::glue::CallPropertyDescriptorTracer; +use crate::jsapi::js::TraceValueArray; use crate::jsapi::JS; use crate::jsapi::{jsid, JSFunction, JSObject, JSScript, JSString, JSTracer}; use crate::jsid::VoidId; use std::cell::UnsafeCell; -use std::ffi::c_void; +use std::ffi::{c_char, c_void}; use std::mem; use std::ptr; /// A trait for JS types that can be registered as roots. pub trait RootKind { - #[allow(non_snake_case)] - /// Returns the rooting kind for `Self`. - fn rootKind() -> JS::RootKind; + type Vtable; + const VTABLE: Self::Vtable; + const KIND: JS::RootKind; } impl RootKind for *mut JSObject { - #[inline(always)] - fn rootKind() -> JS::RootKind { - JS::RootKind::Object - } + type Vtable = (); + const VTABLE: Self::Vtable = (); + const KIND: JS::RootKind = JS::RootKind::Object; } impl RootKind for *mut JSFunction { - #[inline(always)] - fn rootKind() -> JS::RootKind { - JS::RootKind::Object - } + type Vtable = (); + const VTABLE: Self::Vtable = (); + const KIND: JS::RootKind = JS::RootKind::Object; } impl RootKind for *mut JSString { - #[inline(always)] - fn rootKind() -> JS::RootKind { - JS::RootKind::String - } + type Vtable = (); + const VTABLE: Self::Vtable = (); + const KIND: JS::RootKind = JS::RootKind::String; } impl RootKind for *mut JS::Symbol { - #[inline(always)] - fn rootKind() -> JS::RootKind { - JS::RootKind::Symbol - } + type Vtable = (); + const VTABLE: Self::Vtable = (); + const KIND: JS::RootKind = JS::RootKind::Symbol; } impl RootKind for *mut JS::BigInt { - #[inline(always)] - fn rootKind() -> JS::RootKind { - JS::RootKind::BigInt - } + type Vtable = (); + const VTABLE: Self::Vtable = (); + const KIND: JS::RootKind = JS::RootKind::BigInt; } impl RootKind for *mut JSScript { - #[inline(always)] - fn rootKind() -> JS::RootKind { - JS::RootKind::Script - } + type Vtable = (); + const VTABLE: Self::Vtable = (); + const KIND: JS::RootKind = JS::RootKind::Script; } impl RootKind for jsid { - #[inline(always)] - fn rootKind() -> JS::RootKind { - JS::RootKind::Id - } + type Vtable = (); + const VTABLE: Self::Vtable = (); + const KIND: JS::RootKind = JS::RootKind::Id; } impl RootKind for JS::Value { - #[inline(always)] - fn rootKind() -> JS::RootKind { - JS::RootKind::Value - } + type Vtable = (); + const VTABLE: Self::Vtable = (); + const KIND: JS::RootKind = JS::RootKind::Value; } impl RootKind for JS::PropertyDescriptor { - #[inline(always)] - fn rootKind() -> JS::RootKind { - JS::RootKind::Traceable + type Vtable = *const RootedVFTable; + const VTABLE: Self::Vtable = &::VTABLE; + const KIND: JS::RootKind = JS::RootKind::Traceable; +} + +/// A vtable for use in RootedTraceable, which must be present for stack roots using +/// RootKind::Traceable. The C++ tracing implementation uses a virtual trace function +/// which is only present for C++ Rooted values that use the Traceable root kind. +#[repr(C)] +pub struct RootedVFTable { + #[cfg(windows)] + pub padding: [usize; 1], + #[cfg(not(windows))] + pub padding: [usize; 2], + pub trace: unsafe extern "C" fn(this: *mut c_void, trc: *mut JSTracer, name: *const c_char), +} + +impl RootedVFTable { + #[cfg(windows)] + pub const PADDING: [usize; 1] = [0]; + #[cfg(not(windows))] + pub const PADDING: [usize; 2] = [0, 0]; +} + +/// `Rooted` with a T that uses the Traceable RootKind uses dynamic dispatch on the C++ side +/// for custom tracing. This trait provides trace logic via a vtable when creating a Rust instance +/// of the object. +pub unsafe trait TraceableTrace: Sized + RootKind { + const VTABLE: RootedVFTable = RootedVFTable { + padding: RootedVFTable::PADDING, + trace: Self::trace, + }; + + unsafe extern "C" fn trace(this: *mut c_void, trc: *mut JSTracer, _name: *const c_char) { + let rooted = this as *mut Rooted; + let rooted = rooted.as_mut().unwrap(); + Self::do_trace(&mut rooted.ptr, trc); } + + /// Used by `TraceableTrace` implementer to trace its contents. + /// Corresponds to virtual `trace` call in a `Rooted` that inherits from + /// StackRootedTraceableBase (C++). + fn do_trace(&mut self, trc: *mut JSTracer); +} + +unsafe impl TraceableTrace for JS::PropertyDescriptor { + fn do_trace(&mut self, trc: *mut JSTracer) { + unsafe { + CallPropertyDescriptorTracer(trc, self); + } + } +} + +// The C++ representation of Rooted inherits from StackRootedBase, which +// contains the actual pointers that get manipulated. The Rust representation +// also uses the pattern, which is critical to ensuring that the right pointers +// to Rooted values are used, since some Rooted values are prefixed with +// a vtable pointer, and we don't want to store pointers to that vtable where +// C++ expects a StackRootedBase. +#[repr(C)] +#[derive(Debug)] +pub struct RootedBase { + pub stack: *mut *mut RootedBase, + pub prev: *mut RootedBase, } // Annoyingly, bindgen can't cope with SM's use of templates, so we have to roll our own. #[repr(C)] #[derive(Debug)] -pub struct Rooted { - pub stack: *mut *mut Rooted<*mut c_void>, - pub prev: *mut Rooted<*mut c_void>, +pub struct Rooted { + pub vtable: T::Vtable, + pub base: RootedBase, pub ptr: T, } @@ -214,8 +268,16 @@ impl ValueArray { } impl RootKind for ValueArray { - fn rootKind() -> JS::RootKind { - JS::RootKind::Traceable + type Vtable = *const RootedVFTable; + const VTABLE: Self::Vtable = &::VTABLE; + const KIND: JS::RootKind = JS::RootKind::Traceable; +} + +unsafe impl TraceableTrace for ValueArray { + fn do_trace(&mut self, trc: *mut JSTracer) { + unsafe { + TraceValueArray(trc, N, self.get_mut_ptr()); + } } } diff --git a/mozjs-sys/src/jsglue.cpp b/mozjs-sys/src/jsglue.cpp index 16d2f2623b8..7a9518da2d9 100644 --- a/mozjs-sys/src/jsglue.cpp +++ b/mozjs-sys/src/jsglue.cpp @@ -943,6 +943,10 @@ void CallValueRootTracer(JSTracer* trc, JS::Value* valp, const char* name) { JS::TraceRoot(trc, valp, name); } +void CallPropertyDescriptorTracer(JSTracer* trc, JS::PropertyDescriptor* desc) { + desc->trace(trc); +} + bool IsDebugBuild() { #ifdef JS_DEBUG return true; diff --git a/mozjs-sys/src/jsimpls.rs b/mozjs-sys/src/jsimpls.rs index 2eb11d37e46..e5eee43aa44 100644 --- a/mozjs-sys/src/jsimpls.rs +++ b/mozjs-sys/src/jsimpls.rs @@ -20,7 +20,7 @@ use crate::jsapi::JSPropertySpec_Kind; use crate::jsapi::JSPropertySpec_Name; use crate::jsapi::JS; use crate::jsapi::JS::Scalar::Type; -use crate::jsgc::RootKind; +use crate::jsgc::{RootKind, RootedBase}; use crate::jsid::VoidId; use crate::jsval::UndefinedValue; @@ -383,42 +383,49 @@ impl JSNativeWrapper { } } -impl JS::Rooted { - pub fn new_unrooted() -> JS::Rooted { - JS::Rooted { - stack: ptr::null_mut(), - prev: ptr::null_mut(), - ptr: unsafe { std::mem::zeroed() }, - } +impl RootedBase { + unsafe fn add_to_root_stack(&mut self, cx: *mut JSContext, kind: JS::RootKind) { + let stack = Self::get_root_stack(cx, kind); + self.stack = stack; + self.prev = *stack; + + *stack = self as *mut _ as usize as _; } - unsafe fn get_rooting_context(cx: *mut JSContext) -> *mut JS::RootingContext { - cx as *mut JS::RootingContext + unsafe fn remove_from_root_stack(&mut self) { + assert!(*self.stack == self as *mut _ as usize as _); + *self.stack = self.prev; } - unsafe fn get_root_stack(cx: *mut JSContext) -> *mut *mut JS::Rooted<*mut c_void> - where - T: RootKind, - { - let kind = T::rootKind() as usize; + unsafe fn get_root_stack(cx: *mut JSContext, kind: JS::RootKind) -> *mut *mut RootedBase { + let kind = kind as usize; let rooting_cx = Self::get_rooting_context(cx); &mut (*rooting_cx).stackRoots_[kind] as *mut _ as *mut _ } - pub unsafe fn add_to_root_stack(&mut self, cx: *mut JSContext) - where - T: RootKind, - { - let stack = Self::get_root_stack(cx); - self.stack = stack; - self.prev = *stack; + unsafe fn get_rooting_context(cx: *mut JSContext) -> *mut JS::RootingContext { + cx as *mut JS::RootingContext + } +} - *stack = self as *mut _ as usize as _; +impl JS::Rooted { + pub fn new_unrooted() -> JS::Rooted { + JS::Rooted { + vtable: T::VTABLE, + base: RootedBase { + stack: ptr::null_mut(), + prev: ptr::null_mut(), + }, + ptr: unsafe { std::mem::zeroed() }, + } + } + + pub unsafe fn add_to_root_stack(&mut self, cx: *mut JSContext) { + self.base.add_to_root_stack(cx, T::KIND) } pub unsafe fn remove_from_root_stack(&mut self) { - assert!(*self.stack == self as *mut _ as usize as _); - *self.stack = self.prev; + self.base.remove_from_root_stack() } } From bab789d9d9148fe4ba7062ea770059e5a3a6d58b Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Mon, 28 Oct 2024 02:00:30 -0400 Subject: [PATCH 2/4] Update version number. Signed-off-by: Josh Matthews --- mozjs-sys/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mozjs-sys/Cargo.toml b/mozjs-sys/Cargo.toml index 0848caa909d..3c4bd8940a4 100644 --- a/mozjs-sys/Cargo.toml +++ b/mozjs-sys/Cargo.toml @@ -2,7 +2,7 @@ name = "mozjs_sys" description = "System crate for the Mozilla SpiderMonkey JavaScript engine." repository.workspace = true -version = "0.128.3-0" +version = "0.128.3-1" authors = ["Mozilla"] links = "mozjs" build = "build.rs" From 0d5ff0d5a55d8a746dae571d2c4ce73eca4e7c12 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Mon, 28 Oct 2024 02:00:43 -0400 Subject: [PATCH 3/4] Automatically use Traceable-compatible Rooted with any T that implements TraceableTrace. Signed-off-by: Josh Matthews --- mozjs-sys/src/jsgc.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/mozjs-sys/src/jsgc.rs b/mozjs-sys/src/jsgc.rs index 382fb0ae756..e9d18deb053 100644 --- a/mozjs-sys/src/jsgc.rs +++ b/mozjs-sys/src/jsgc.rs @@ -68,7 +68,7 @@ impl RootKind for JS::Value { const KIND: JS::RootKind = JS::RootKind::Value; } -impl RootKind for JS::PropertyDescriptor { +impl RootKind for T { type Vtable = *const RootedVFTable; const VTABLE: Self::Vtable = &::VTABLE; const KIND: JS::RootKind = JS::RootKind::Traceable; @@ -96,7 +96,7 @@ impl RootedVFTable { /// `Rooted` with a T that uses the Traceable RootKind uses dynamic dispatch on the C++ side /// for custom tracing. This trait provides trace logic via a vtable when creating a Rust instance /// of the object. -pub unsafe trait TraceableTrace: Sized + RootKind { +pub unsafe trait TraceableTrace: Sized { const VTABLE: RootedVFTable = RootedVFTable { padding: RootedVFTable::PADDING, trace: Self::trace, @@ -267,12 +267,6 @@ impl ValueArray { } } -impl RootKind for ValueArray { - type Vtable = *const RootedVFTable; - const VTABLE: Self::Vtable = &::VTABLE; - const KIND: JS::RootKind = JS::RootKind::Traceable; -} - unsafe impl TraceableTrace for ValueArray { fn do_trace(&mut self, trc: *mut JSTracer) { unsafe { From c9f0f45a9862cf439f28408f529659145d17dc45 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Mon, 28 Oct 2024 02:31:45 -0400 Subject: [PATCH 4/4] Make TraceableTrace::do_trace unsafe. Signed-off-by: Josh Matthews --- mozjs-sys/src/jsgc.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/mozjs-sys/src/jsgc.rs b/mozjs-sys/src/jsgc.rs index e9d18deb053..1141dd06112 100644 --- a/mozjs-sys/src/jsgc.rs +++ b/mozjs-sys/src/jsgc.rs @@ -111,14 +111,12 @@ pub unsafe trait TraceableTrace: Sized { /// Used by `TraceableTrace` implementer to trace its contents. /// Corresponds to virtual `trace` call in a `Rooted` that inherits from /// StackRootedTraceableBase (C++). - fn do_trace(&mut self, trc: *mut JSTracer); + unsafe fn do_trace(&mut self, trc: *mut JSTracer); } unsafe impl TraceableTrace for JS::PropertyDescriptor { - fn do_trace(&mut self, trc: *mut JSTracer) { - unsafe { - CallPropertyDescriptorTracer(trc, self); - } + unsafe fn do_trace(&mut self, trc: *mut JSTracer) { + CallPropertyDescriptorTracer(trc, self); } } @@ -268,10 +266,8 @@ impl ValueArray { } unsafe impl TraceableTrace for ValueArray { - fn do_trace(&mut self, trc: *mut JSTracer) { - unsafe { - TraceValueArray(trc, N, self.get_mut_ptr()); - } + unsafe fn do_trace(&mut self, trc: *mut JSTracer) { + TraceValueArray(trc, N, self.get_mut_ptr()); } }