diff --git a/.github/workflows/test262.yml b/.github/workflows/test262.yml index 95822eae107..38a1613eb1b 100644 --- a/.github/workflows/test262.yml +++ b/.github/workflows/test262.yml @@ -46,7 +46,7 @@ jobs: run: | cd boa mkdir -p ../results/test262 - cargo run --release --bin boa_tester -- run -v -o ../results/test262 + cargo run --release --bin boa_tester -- run -v --disable-parallelism -o ../results/test262 cd .. - name: Compare results diff --git a/Cargo.lock b/Cargo.lock index b35b89ecf19..c60d0dd1275 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2859,7 +2859,7 @@ dependencies = [ [[package]] name = "oscars" version = "0.1.0" -source = "git+https://github.com/boa-dev/oscars.git?branch=main#6af6e1a2b1689c7eeff9675352a8d3bccf5abc11" +source = "git+https://github.com/shruti2522/oscars.git?branch=size_class#13a3514748d4b57d099d1ebbc353d842f95a59f2" dependencies = [ "arrayvec", "either", @@ -2874,7 +2874,7 @@ dependencies = [ [[package]] name = "oscars_derive" version = "0.1.0" -source = "git+https://github.com/boa-dev/oscars.git?branch=main#6af6e1a2b1689c7eeff9675352a8d3bccf5abc11" +source = "git+https://github.com/shruti2522/oscars.git?branch=size_class#13a3514748d4b57d099d1ebbc353d842f95a59f2" dependencies = [ "cfg-if", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index 59e1a8a76d1..65c35e5505d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -118,7 +118,7 @@ rand = "0.10.2" num-integer = "0.1.47" ryu-js = "1.0.3" tap = "1.0.1" -thiserror = { version = "2.0.20", default-features = false } +thiserror = { version = "2.0.18", default-features = false } typeid = "1.0.3" dashmap = "6.2.1" num_enum = "0.7.6" diff --git a/core/engine/benches/full.rs b/core/engine/benches/full.rs index 6ee42cb1a96..78cf1f27d7a 100644 --- a/core/engine/benches/full.rs +++ b/core/engine/benches/full.rs @@ -18,13 +18,11 @@ static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc; fn create_realm(c: &mut Criterion) { c.bench_function("Create Realm", move |b| { - let root_shape = RootShape::new(&boa_gc::MutationContext::global()); + let root_shape = RootShape::new(&unsafe { boa_gc::MutationContext::global() }); b.iter(|| { - Realm::create( - &DefaultHooks, - &root_shape, - &boa_gc::MutationContext::global(), - ) + Realm::create(&DefaultHooks, &root_shape, &unsafe { + boa_gc::MutationContext::global() + }) }); }); } diff --git a/core/engine/src/builtins/array/array_iterator.rs b/core/engine/src/builtins/array/array_iterator.rs index 2490214b261..f1e67020809 100644 --- a/core/engine/src/builtins/array/array_iterator.rs +++ b/core/engine/src/builtins/array/array_iterator.rs @@ -95,13 +95,28 @@ impl ArrayIterator { /// /// [spec]: https://tc39.es/ecma262/#sec-%arrayiteratorprototype%.next pub(crate) fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { - let object = this.as_object(); - let mut array_iterator = object - .as_ref() - .and_then(JsObject::downcast_mut::) + let object = this + .as_object() + .filter(|o| o.is::()) .ok_or_else(|| JsNativeError::typ().with_message("`this` is not an ArrayIterator"))?; - let index = array_iterator.next_index; - if array_iterator.done { + + // Extract needed fields into a scoped block so the RefMut borrow is dropped + // before any context call. Holding a RefMut<'_, T> across context operations + // is a use-after-free: GC can collect the backing object while the guard is live. + let (index, done, array, kind) = { + let array_iterator = object + .downcast_ref::() + .expect("already checked that it is an ArrayIterator"); + ( + array_iterator.next_index, + array_iterator.done, + array_iterator.array.clone(), + array_iterator.kind, + ) + }; + // RefMut dropped here — safe to use context below. + + if done { return Ok(create_iter_result_object( JsValue::undefined(), true, @@ -109,7 +124,7 @@ impl ArrayIterator { )); } - let len = if let Some(f) = array_iterator.array.downcast_ref::() { + let len = if let Some(f) = array.downcast_ref::() { let buf = f.viewed_array_buffer().as_buffer(); let Some(buf) = buf .bytes(std::sync::atomic::Ordering::SeqCst) @@ -122,26 +137,32 @@ impl ArrayIterator { f.array_length(buf.len()) } else { - array_iterator.array.length_of_array_like(context)? + array.length_of_array_like(context)? }; if index >= len { - array_iterator.done = true; + object.downcast_mut::().expect("already checked").done = true; return Ok(create_iter_result_object( JsValue::undefined(), true, context, )); } - array_iterator.next_index = index + 1; - match array_iterator.kind { + + // Write back the incremented index (no borrow held during context calls above). + object + .downcast_mut::() + .expect("already checked") + .next_index = index + 1; + + match kind { PropertyNameKind::Key => Ok(create_iter_result_object(index.into(), false, context)), PropertyNameKind::Value => { - let element_value = array_iterator.array.get(index, context)?; + let element_value = array.get(index, context)?; Ok(create_iter_result_object(element_value, false, context)) } PropertyNameKind::KeyAndValue => { - let element_value = array_iterator.array.get(index, context)?; + let element_value = array.get(index, context)?; let result = Array::create_array_from_list([index.into(), element_value], context); Ok(create_iter_result_object(result.into(), false, context)) } diff --git a/core/engine/src/builtins/array/from_async.rs b/core/engine/src/builtins/array/from_async.rs index 43c8dbe8c36..cecf5fa0071 100644 --- a/core/engine/src/builtins/array/from_async.rs +++ b/core/engine/src/builtins/array/from_async.rs @@ -124,6 +124,7 @@ impl Array { // Coroutine yielded. We need to allocate it for a future execution. JsPromise::resolve(value, context)?.await_native( NativeCoroutine::from_copy_closure_with_captures( + context.gc_collector(), from_array_like, coroutine_state, ), @@ -174,6 +175,7 @@ impl Array { CoroutineState::Continue(value) => { JsPromise::resolve(value, context)?.await_native( NativeCoroutine::from_copy_closure_with_captures( + context.gc_collector(), from_async_iterator, coroutine_state, ), diff --git a/core/engine/src/builtins/async_generator/mod.rs b/core/engine/src/builtins/async_generator/mod.rs index 298fc9f29c1..c662e5edc80 100644 --- a/core/engine/src/builtins/async_generator/mod.rs +++ b/core/engine/src/builtins/async_generator/mod.rs @@ -582,6 +582,7 @@ impl AsyncGenerator { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, generator, context| { // a. Assert: generator.[[AsyncGeneratorState]] is draining-queue. assert_eq!( @@ -614,6 +615,7 @@ impl AsyncGenerator { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, generator, context| { // a. Assert: generator.[[AsyncGeneratorState]] is draining-queue. assert_eq!( diff --git a/core/engine/src/builtins/eval/mod.rs b/core/engine/src/builtins/eval/mod.rs index fc9726a0236..e5325ee6d6b 100644 --- a/core/engine/src/builtins/eval/mod.rs +++ b/core/engine/src/builtins/eval/mod.rs @@ -283,7 +283,7 @@ impl Eval { false, false, context.interner_mut(), - mc, + &mc, in_with, spanned_source_text, // TODO: Could give more information from previous shadow stack. @@ -351,7 +351,7 @@ impl Eval { let global = frame.realm.environment(); frame .environments - .push_lexical(lexical_scope.num_bindings_non_local(), global, mc); + .push_lexical(lexical_scope.num_bindings_non_local(), &global, mc); } context diff --git a/core/engine/src/builtins/finalization_registry/tests.rs b/core/engine/src/builtins/finalization_registry/tests.rs index ac40d55e302..0c4802e0093 100644 --- a/core/engine/src/builtins/finalization_registry/tests.rs +++ b/core/engine/src/builtins/finalization_registry/tests.rs @@ -1,5 +1,3 @@ -// oscars_backend currently implements `boa_gc::force_collect()` as a no-op, which prevents -// the FinalizationRegistry callbacks from being triggered during these tests. #[cfg(not(feature = "oscars_backend"))] mod miri { diff --git a/core/engine/src/builtins/function/mod.rs b/core/engine/src/builtins/function/mod.rs index aff4b780f61..0e0034ea960 100644 --- a/core/engine/src/builtins/function/mod.rs +++ b/core/engine/src/builtins/function/mod.rs @@ -677,7 +677,7 @@ impl BuiltInFunctionObject { function.scopes(), function.contains_direct_eval(), context.interner_mut(), - mc, + &mc, ); let saved = context.vm.frame_mut().environments.pop_to_global(); @@ -1079,7 +1079,7 @@ pub(crate) fn function_call( let mc = context.gc_collector(); let frame = context.vm.frame_mut(); let global = frame.realm.environment(); - let index = frame.environments.push_lexical(1, global, mc); + let index = frame.environments.push_lexical(1, &global, mc); frame.environments.put_lexical_value( BindingLocatorScope::Stack(index), 0, @@ -1097,7 +1097,7 @@ pub(crate) fn function_call( frame.environments.push_function( scope, FunctionSlots::new(this, function_object.clone(), None), - global, + &global, mc, ); } @@ -1191,7 +1191,7 @@ fn function_construct( let mc = context.gc_collector(); let frame = context.vm.frame_mut(); let global = frame.realm.environment(); - let index = frame.environments.push_lexical(1, global, mc); + let index = frame.environments.push_lexical(1, &global, mc); frame.environments.put_lexical_value( BindingLocatorScope::Stack(index), 0, @@ -1220,7 +1220,7 @@ fn function_construct( .clone(), ), ), - global, + &global, mc, ); } diff --git a/core/engine/src/builtins/function/tests.rs b/core/engine/src/builtins/function/tests.rs index 82d03f49a8c..a533ecade07 100644 --- a/core/engine/src/builtins/function/tests.rs +++ b/core/engine/src/builtins/function/tests.rs @@ -150,6 +150,7 @@ fn closure_capture_clone() { ctx.realm(), ctx.gc_collector(), NativeFunction::from_copy_closure_with_captures( + ctx.gc_collector(), |_, _, captures, context| { let (string, object) = &captures; diff --git a/core/engine/src/builtins/generator/mod.rs b/core/engine/src/builtins/generator/mod.rs index 9acf67076b0..92ac2ebe727 100644 --- a/core/engine/src/builtins/generator/mod.rs +++ b/core/engine/src/builtins/generator/mod.rs @@ -287,6 +287,7 @@ impl Generator { // 2. If state is completed, return CreateIterResultObject(undefined, true). GeneratorState::Completed => { r#gen.state = GeneratorState::Completed; + drop(r#gen); return Ok(create_iter_result_object( JsValue::undefined(), true, @@ -323,6 +324,7 @@ impl Generator { } CompletionRecord::Return(value) => { r#gen.state = GeneratorState::Completed; + drop(r#gen); Ok(create_iter_result_object(value, true, context)) } CompletionRecord::Throw(err) => { @@ -374,6 +376,7 @@ impl Generator { // b. Once a generator enters the completed state it never leaves it and its // associated execution context is never resumed. Any execution state associated // with generator can be discarded at this point. + drop(r#gen); // a. If abruptCompletion.[[Type]] is return, then if let Ok(value) = abrupt_completion { @@ -414,6 +417,7 @@ impl Generator { } CompletionRecord::Return(value) => { r#gen.state = GeneratorState::Completed; + drop(r#gen); Ok(create_iter_result_object(value, true, context)) } CompletionRecord::Throw(err) => { diff --git a/core/engine/src/builtins/intl/collator/mod.rs b/core/engine/src/builtins/intl/collator/mod.rs index bdf8ceea4be..9a28d2c9251 100644 --- a/core/engine/src/builtins/intl/collator/mod.rs +++ b/core/engine/src/builtins/intl/collator/mod.rs @@ -340,31 +340,46 @@ impl Collator { JsNativeError::typ() .with_message("`resolvedOptions` can only be called on a `Collator` object") })?; - let collator_obj = this.clone(); - let mut collator = this.downcast_mut::().ok_or_else(|| { - JsNativeError::typ() - .with_message("`resolvedOptions` can only be called on a `Collator` object") - })?; // 3. If collator.[[BoundCompare]] is undefined, then // a. Let F be a new built-in function object as defined in 10.3.3.1. // b. Set F.[[Collator]] to collator. // c. Set collator.[[BoundCompare]] to F. - let bound_compare = if let Some(f) = collator.bound_compare.clone() { + // + // SAFETY: We must NOT hold a downcast_mut borrow across context.realm() / + // context.gc_collector() calls, as those can trigger a GC collection that + // frees the backing object while the mutable borrow guard is live (UAF). + // + // Pattern: read [[BoundCompare]] in a scoped block, drop the borrow, build the + // function with no borrow held, then take a fresh borrow only to write back. + let existing = { + let collator = this.downcast_ref::().ok_or_else(|| { + JsNativeError::typ() + .with_message("`resolvedOptions` can only be called on a `Collator` object") + })?; + collator.bound_compare.clone() + }; // borrow dropped here + + let bound_compare = if let Some(f) = existing { f } else { + // Build the bound compare function with no borrow held on `this` + let collator_obj = this.clone(); let bound_compare = FunctionObjectBuilder::new( context.realm(), context.gc_collector(), // 10.3.3.1. Collator Compare Functions // https://tc39.es/ecma402/#sec-collator-compare-functions NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, collator, context| { // 1. Let collator be F.[[Collator]]. // 2. Assert: Type(collator) is Object and collator has an [[InitializedCollator]] internal slot. - let collator = collator - .downcast_ref::() - .js_expect("checked above that the object was a collator object")?; + // + // SAFETY: We must resolve the string arguments (which run JS / + // may trigger GC) BEFORE borrowing `collator` via downcast_ref. + // Holding a Ref<'_, T> across a GC point is a use-after-free + // because GC can collect the backing object while the borrow is live. // 3. If x is not provided, let x be undefined. // 5. Let X be ? ToString(x). @@ -382,8 +397,12 @@ impl Collator { .iter() .collect::>(); - // 7. Return CompareStrings(collator, X, Y). + // Borrow collator AFTER all GC-triggering work is done. + let collator = collator + .downcast_ref::() + .js_expect("checked above that the object was a collator object")?; + // 7. Return CompareStrings(collator, X, Y). let result = collator.collator.as_borrowed().compare_utf16(&x, &y) as i32; Ok(result.into()) @@ -394,7 +413,15 @@ impl Collator { .length(2) .build(); - collator.bound_compare = Some(bound_compare.clone()); + // take a fresh borrow to write back [[BoundCompare]]. No context calls + // follow this so the borrow is safe. + this.downcast_mut::() + .ok_or_else(|| { + JsNativeError::typ() + .with_message("`resolvedOptions` can only be called on a `Collator` object") + })? + .bound_compare = Some(bound_compare.clone()); + bound_compare }; @@ -414,15 +441,32 @@ impl Collator { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/Collator/resolvedOptions fn resolved_options(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { // 1. Let collator be the this value. - // 2. Perform ? RequireInternalSlot(collator, [[InitializedCollator]]). - let object = this.as_object(); - let collator = object - .as_ref() - .and_then(JsObject::downcast_ref::) - .ok_or_else(|| { - JsNativeError::typ() - .with_message("`resolvedOptions` can only be called on a `Collator` object") - })?; + // 2. Perform ? RequireInternalSlot(collator, [[InitializedCollator]]). + // + // SAFETY: Extract all data from `collator` into owned values inside a scoped block + // so the Ref<'_, Collator> borrow guard is dropped BEFORE we touch `context`. + // GC allocations (context.gc_collector(), create_data_property_or_throw) can + // trigger a collection cycle; holding a Ref<'_, T> across a GC point is a UAF. + let (locale_str, usage, sensitivity, ignore_punctuation, collation, numeric, case_first) = { + let object = this.as_object(); + let collator = object + .as_ref() + .and_then(JsObject::downcast_ref::) + .ok_or_else(|| { + JsNativeError::typ() + .with_message("`resolvedOptions` can only be called on a `Collator` object") + })?; + // Copy/clone all cheap fields; Ref is dropped at end of this block. + ( + collator.locale.to_string(), + collator.usage, + collator.sensitivity, + collator.ignore_punctuation, + collator.collation, + collator.numeric, + collator.case_first, + ) + }; // ← Ref<'_, Collator> dropped here, safe to use context below // 3. Let options be OrdinaryObjectCreate(%Object.prototype%). let options = context.intrinsics().templates().ordinary_object().create( @@ -439,19 +483,15 @@ impl Collator { // ii. If %Collator%.[[RelevantExtensionKeys]] does not contain extensionKey, then // 1. Let v be undefined. // d. If v is not undefined, then - // i. Perform ! CreateDataPropertyOrThrow(options, p, v). + // i. Perform ! CreateDataPropertyOrThrow(options, p, v). // 5. Return options. options - .create_data_property_or_throw( - js_string!("locale"), - js_string!(collator.locale.to_string()), - context, - ) + .create_data_property_or_throw(js_string!("locale"), js_string!(locale_str), context) .js_expect("operation must not fail per the spec")?; options .create_data_property_or_throw( js_string!("usage"), - match collator.usage { + match usage { Usage::Search => js_string!("search"), Usage::Sort => js_string!("sort"), }, @@ -461,7 +501,7 @@ impl Collator { options .create_data_property_or_throw( js_string!("sensitivity"), - match collator.sensitivity { + match sensitivity { Sensitivity::Base => js_string!("base"), Sensitivity::Accent => js_string!("accent"), Sensitivity::Case => js_string!("case"), @@ -473,24 +513,23 @@ impl Collator { options .create_data_property_or_throw( js_string!("ignorePunctuation"), - collator.ignore_punctuation, + ignore_punctuation, context, ) .js_expect("operation must not fail per the spec")?; options .create_data_property_or_throw( js_string!("collation"), - collator - .collation + collation .map(|co| js_string!(co.as_str())) .unwrap_or(js_string!("default")), context, ) .js_expect("operation must not fail per the spec")?; options - .create_data_property_or_throw(js_string!("numeric"), collator.numeric, context) + .create_data_property_or_throw(js_string!("numeric"), numeric, context) .js_expect("operation must not fail per the spec")?; - if let Some(kf) = collator.case_first { + if let Some(kf) = case_first { options .create_data_property_or_throw( js_string!("caseFirst"), @@ -500,7 +539,6 @@ impl Collator { .js_expect("operation must not fail per the spec")?; } - // 5. Return options. Ok(options.into()) } } diff --git a/core/engine/src/builtins/intl/date_time_format/mod.rs b/core/engine/src/builtins/intl/date_time_format/mod.rs index 716584937be..0b9526e7412 100644 --- a/core/engine/src/builtins/intl/date_time_format/mod.rs +++ b/core/engine/src/builtins/intl/date_time_format/mod.rs @@ -199,6 +199,11 @@ impl BuiltInConstructor for DateTimeFormat { FormatDefaults::Date, context, )?; + let prototype = get_prototype_from_constructor( + new_target_inner, + StandardConstructors::date_time_format, + context, + )?; let date_time_format = JsObject::from_proto_and_data(context.gc_collector(), prototype, dtf); @@ -272,6 +277,7 @@ impl DateTimeFormat { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, dtf, context| { // 1. Let dtf be F.[[DateTimeFormat]]. // 2. Assert: dtf is an Object and dtf has an [[InitializedDateTimeFormat]] internal slot. @@ -445,56 +451,75 @@ impl DateTimeFormat { // a. Assert: conversion is number. // b. Set v to 𝔽(v). // ii. Perform ! CreateDataPropertyOrThrow(options, p, v). - let result = { + let ( + locale_str, + calendar_algorithm, + numbering_system, + time_zone_str, + hour_cycle, + date_style, + time_style, + fractional_second_digits, + ) = { let dtf = dtf_object.borrow(); let dtf = dtf.data(); + let time_zone_str = match &dtf.time_zone { + FormatTimeZone::UtcOffset(offset) => { + let seconds = offset.to_seconds(); + let hours = seconds / 3600; + let minutes = (seconds.abs() % 3600) / 60; + JsString::from(format!("{hours:+03}:{minutes:02}")) + } + FormatTimeZone::Identifier((_tz, id)) => { + JsString::from(context.timezone_provider().identifier(*id).map_err(|_| { + js_error!( + TypeError: + "could not fetch identifier for resolved timezone" + ) + })?) + } + }; + + ( + dtf.locale.to_string(), + dtf.calendar_algorithm + .as_ref() + .map(|ca| js_string!(ca.as_str())), + dtf.numbering_system + .as_ref() + .map(|nu| js_string!(nu.as_str())), + time_zone_str, + dtf.hour_cycle, + dtf.date_style, + dtf.time_style, + dtf.fractional_second_digits, + ) + }; + + let result = { let mut options = ObjectInitializer::new(context); options.property( js_string!("locale"), - js_string!(dtf.locale.to_string()), + js_string!(locale_str), Attribute::all(), ); - if let Some(ca) = &dtf.calendar_algorithm { - options.property( - js_string!("calendar"), - js_string!(ca.as_str()), - Attribute::all(), - ); + if let Some(ca) = calendar_algorithm { + options.property(js_string!("calendar"), ca, Attribute::all()); } - if let Some(nu) = &dtf.numbering_system { - options.property( - js_string!("numberingSystem"), - js_string!(nu.as_str()), - Attribute::all(), - ); + if let Some(nu) = numbering_system { + options.property(js_string!("numberingSystem"), nu, Attribute::all()); } - let time_zone_str = match &dtf.time_zone { - FormatTimeZone::UtcOffset(offset) => { - let seconds = offset.to_seconds(); - let hours = seconds / 3600; - let minutes = (seconds.abs() % 3600) / 60; - JsString::from(format!("{hours:+03}:{minutes:02}")) - } - FormatTimeZone::Identifier((_tz, id)) => JsString::from( - options - .context() - .timezone_provider() - .identifier(*id) - .map_err(|_| { - js_error!( - TypeError: - "could not fetch identifier for resolved timezone" - ) - })?, - ), - }; - options.property(js_string!("timeZone"), time_zone_str, Attribute::all()); + options.property( + js_string!("timeZone"), + js_string!(time_zone_str), + Attribute::all(), + ); - if let Some(hc) = &dtf.hour_cycle { + if let Some(hc) = hour_cycle { options.property( js_string!("hourCycle"), js_string!(hc.as_str()), @@ -509,7 +534,7 @@ impl DateTimeFormat { // dateStyle nor timeStyle is set; the constructor already guarantees this by // rejecting explicit component options alongside a style, so the value is // `None` whenever a style is present. - if let Some(fsd) = dtf.fractional_second_digits { + if let Some(fsd) = fractional_second_digits { options.property( js_string!("fractionalSecondDigits"), fsd.digits(), @@ -517,11 +542,11 @@ impl DateTimeFormat { ); } - if let Some(ds) = dtf.date_style { + if let Some(ds) = date_style { options.property(js_string!("dateStyle"), ds.to_js_string(), Attribute::all()); } - if let Some(ts) = dtf.time_style { + if let Some(ts) = time_style { options.property(js_string!("timeStyle"), ts.to_js_string(), Attribute::all()); } diff --git a/core/engine/src/builtins/intl/list_format/mod.rs b/core/engine/src/builtins/intl/list_format/mod.rs index a1c2228f09d..c6063c3888d 100644 --- a/core/engine/src/builtins/intl/list_format/mod.rs +++ b/core/engine/src/builtins/intl/list_format/mod.rs @@ -228,25 +228,32 @@ impl ListFormat { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/ListFormat/format fn format(this: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult { // 1. Let lf be the this value. - // 2. Perform ? RequireInternalSlot(lf, [[InitializedListFormat]]). - let object = this.as_object(); - let lf = object - .as_ref() - .and_then(|o| o.downcast_ref::()) - .ok_or_else(|| { - JsNativeError::typ() - .with_message("`format` can only be called on a `ListFormat` object") - })?; - - // 3. Let stringList be ? StringListFromIterable(list). + // 2. Perform ? RequireInternalSlot(lf, [[InitializedListFormat]]). + // Validate the `this` type BEFORE collecting strings, but do NOT hold the + // borrow across the iterator call below: `string_list_from_iterable` runs + // arbitrary JS (iterator protocol) which can trigger GC and free/move the + // GC-managed object that `downcast_ref` borrows from, causing a UAF. + let object = this.as_object().filter(|o| o.is::()).ok_or_else(|| { + JsNativeError::typ() + .with_message("`format` can only be called on a `ListFormat` object") + })?; + + // 3. Let stringList be ? StringListFromIterable(list). // TODO: support for UTF-16 unpaired surrogates formatting + // SAFETY: We must collect strings first (which runs JS / may trigger GC) and + // only THEN borrow `lf`. Holding a `Ref<'_, T>` across a GC point is a UAF. let strings = string_list_from_iterable(args.get_or_undefined(0), context)?; + // Borrow `lf` only after all GC-triggering operations are complete. + let lf = object + .downcast_ref::() + .expect("already checked above that the object is a ListFormat"); + let formatted = lf .native .format_to_string(strings.into_iter().map(|s| s.to_std_string_escaped())); - // 4. Return ! FormatList(lf, stringList). + // 4. Return ! FormatList(lf, stringList). Ok(js_string!(formatted).into()) } @@ -283,8 +290,9 @@ impl ListFormat { fn with_part( &mut self, part: writeable::Part, - mut f: impl FnMut(&mut Self::SubPartsWrite) -> fmt::Result, - ) -> fmt::Result { + mut f: impl FnMut(&mut Self::SubPartsWrite) -> core::fmt::Result, + ) -> core::fmt::Result { + assert_eq!(part.category, "list"); let mut string = CoreWriteAsPartsWrite(String::new()); f(&mut string)?; if string.0.is_empty() || part.category != "list" { @@ -296,41 +304,56 @@ impl ListFormat { } // 1. Let lf be the this value. - // 2. Perform ? RequireInternalSlot(lf, [[InitializedListFormat]]). - let object = this.as_object(); - let lf = object - .as_ref() - .and_then(|o| o.downcast_ref::()) - .ok_or_else(|| { - JsNativeError::typ() - .with_message("`formatToParts` can only be called on a `ListFormat` object") - })?; - - // 3. Let stringList be ? StringListFromIterable(list). + // 2. Perform ? RequireInternalSlot(lf, [[InitializedListFormat]]). + // Validate the `this` type BEFORE collecting strings, but do NOT hold the + // borrow across the iterator call below: `string_list_from_iterable` runs + // arbitrary JS (iterator protocol) which can trigger GC and free/move the + // GC-managed object that `downcast_ref` borrows from, causing a UAF. + let object = this.as_object().filter(|o| o.is::()).ok_or_else(|| { + JsNativeError::typ() + .with_message("`formatToParts` can only be called on a `ListFormat` object") + })?; + + // 3. Let stringList be ? StringListFromIterable(list). // TODO: support for UTF-16 unpaired surrogates formatting - let strings = string_list_from_iterable(args.get_or_undefined(0), context)? + // SAFETY: Collect the JS strings first (runs JS / may trigger GC), before + // borrowing `lf`. A Ref<'_, T> must NOT be held across any GC point. + let strings: Vec = string_list_from_iterable(args.get_or_undefined(0), context)? .into_iter() - .map(|s| s.to_std_string_escaped()); + .map(|s| s.to_std_string_escaped()) + .collect(); - // 4. Return ! FormatListToParts(lf, stringList). + // 4. Return ! FormatListToParts(lf, stringList). // Abstract operation `FormatListToParts ( listFormat, list )` // https://tc39.es/ecma402/#sec-formatlisttoparts - // 1. Let parts be ! CreatePartsFromList(listFormat, list). - let mut parts = PartsCollector(Vec::new()); - lf.native - .format(strings) - .write_to_parts(&mut parts) - .map_err(|e| JsNativeError::typ().with_message(e.to_string()))?; - - // 2. Let result be ! ArrayCreate(0). + // 1. Let parts be ! CreatePartsFromList(listFormat, list). + // + // SAFETY: Perform the pure native formatting inside a scoped block so that + // the Ref<'_, ListFormat> borrow guard is dropped BEFORE we re-enter context + // (Array::array_create, context.gc_collector, create_data_property_or_throw). + // All of those can trigger a GC collection cycle, which would be a UAF if we + // still held the Ref + let parts = { + let lf = object + .downcast_ref::() + .expect("already checked above that the object is a ListFormat"); + let mut collector = PartsCollector(Vec::new()); + lf.native + .format(strings.into_iter()) + .write_to_parts(&mut collector) + .map_err(|e| JsNativeError::typ().with_message(e.to_string()))?; + collector.0 + }; // Ref<'_, ListFormat> dropped here; safe to use context below + + // 2. Let result be ! ArrayCreate(0). let result = Array::array_create(0, None, context) .js_expect("creating an empty array with default proto must not fail")?; // 3. Let n be 0. // 4. For each Record { [[Type]], [[Value]] } part in parts, do - for (n, (typ, value)) in parts.0.into_iter().enumerate() { + for (n, part) in parts.into_iter().enumerate() { // a. Let O be OrdinaryObjectCreate(%Object.prototype%). let o = context.intrinsics().templates().ordinary_object().create( context.gc_collector(), @@ -338,15 +361,15 @@ impl ListFormat { vec![], ); - // b. Perform ! CreateDataPropertyOrThrow(O, "type", part.[[Type]]). - o.create_data_property_or_throw(js_string!("type"), js_string!(typ), context) + // b. Perform ! CreateDataPropertyOrThrow(O, "type", part.[[Type]]). + o.create_data_property_or_throw(js_string!("type"), js_string!(part.0), context) .js_expect("operation must not fail per the spec")?; - // c. Perform ! CreateDataPropertyOrThrow(O, "value", part.[[Value]]). - o.create_data_property_or_throw(js_string!("value"), js_string!(value), context) + // c. Perform ! CreateDataPropertyOrThrow(O, "value", part.[[Value]]). + o.create_data_property_or_throw(js_string!("value"), js_string!(part.1), context) .js_expect("operation must not fail per the spec")?; - // d. Perform ! CreateDataPropertyOrThrow(result, ! ToString(n), O). + // d. Perform ! CreateDataPropertyOrThrow(result, ! ToString(n), O). result .create_data_property_or_throw(n, o, context) .js_expect("operation must not fail per the spec")?; @@ -370,15 +393,27 @@ impl ListFormat { /// [mdn]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/ListFormat/resolvedOptions fn resolved_options(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { // 1. Let lf be the this value. - // 2. Perform ? RequireInternalSlot(lf, [[InitializedListFormat]]). - let object = this.as_object(); - let lf = object - .as_ref() - .and_then(|o| o.downcast_ref::()) - .ok_or_else(|| { - JsNativeError::typ() - .with_message("`resolvedOptions` can only be called on a `ListFormat` object") - })?; + // 2. Perform ? RequireInternalSlot(lf, [[InitializedListFormat]]). + // + // SAFETY: Extract all data from `lf` into owned values inside a scoped block so + // the Ref<'_, ListFormat> borrow guard is dropped BEFORE we touch `context`. + // GC allocations (context.gc_collector(), js_string!, create_data_property_or_throw) + // can trigger a collection cycle; holding a Ref<'_, T> across a GC point is a + // use-after-free because the GC may collect the backing object while the borrow + // is live. + let (locale_str, typ, style) = { + let object = this.as_object(); + let lf = object + .as_ref() + .and_then(|o| o.downcast_ref::()) + .ok_or_else(|| { + JsNativeError::typ().with_message( + "`resolvedOptions` can only be called on a `ListFormat` object", + ) + })?; + // Clone/copy out the cheap data we need; Ref is dropped at end of this block. + (lf.locale.to_string(), lf.typ, lf.style) + }; // ← Ref<'_, ListFormat> dropped here, safe to use context below // 3. Let options be OrdinaryObjectCreate(%Object.prototype%). let options = context.intrinsics().templates().ordinary_object().create( @@ -391,18 +426,14 @@ impl ListFormat { // a. Let p be the Property value of the current row. // b. Let v be the value of lf's internal slot whose name is the Internal Slot value of the current row. // c. Assert: v is not undefined. - // d. Perform ! CreateDataPropertyOrThrow(options, p, v). + // d. Perform ! CreateDataPropertyOrThrow(options, p, v). options - .create_data_property_or_throw( - js_string!("locale"), - js_string!(lf.locale.to_string()), - context, - ) + .create_data_property_or_throw(js_string!("locale"), js_string!(locale_str), context) .js_expect("operation must not fail per the spec")?; options .create_data_property_or_throw( js_string!("type"), - match lf.typ { + match typ { ListFormatType::Conjunction => js_string!("conjunction"), ListFormatType::Disjunction => js_string!("disjunction"), ListFormatType::Unit => js_string!("unit"), @@ -413,7 +444,7 @@ impl ListFormat { options .create_data_property_or_throw( js_string!("style"), - match lf.style { + match style { ListLength::Wide => js_string!("long"), ListLength::Short => js_string!("short"), ListLength::Narrow => js_string!("narrow"), @@ -423,7 +454,6 @@ impl ListFormat { ) .js_expect("operation must not fail per the spec")?; - // 5. Return options. Ok(options.into()) } } diff --git a/core/engine/src/builtins/intl/number_format/mod.rs b/core/engine/src/builtins/intl/number_format/mod.rs index b6ab664acb4..2965522cee1 100644 --- a/core/engine/src/builtins/intl/number_format/mod.rs +++ b/core/engine/src/builtins/intl/number_format/mod.rs @@ -611,6 +611,7 @@ impl NumberFormat { // Number Format Functions // NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, nf, context| { // 1. Let nf be F.[[NumberFormat]]. // 2. Assert: Type(nf) is Object and nf has an [[InitializedNumberFormat]] internal slot. @@ -752,8 +753,108 @@ impl NumberFormat { // a. Set nf to ? UnwrapNumberFormat(nf). // 3. Perform ? RequireInternalSlot(nf, [[InitializedNumberFormat]]). let nf = unwrap_number_format(this, context)?; - let nf = nf.borrow(); - let nf = nf.data(); + + let ( + locale_str, + numbering_system, + style, + currency, + currency_display, + currency_sign, + unit, + unit_display, + minimum_integer_digits, + fraction_digits, + significant_digits, + use_grouping, + notation_str, + compact_display_str, + sign_display_str, + rounding_increment, + rounding_priority_str, + trailing_zero_display_str, + ) = { + let nf_borrow = nf.borrow(); + let nf_data = nf_borrow.data(); + + let (currency, currency_display, currency_sign, unit, unit_display) = + match &nf_data.unit_options { + UnitFormatOptions::Currency { + currency, + display, + sign, + } => ( + Some(currency.to_js_string()), + Some(display.to_js_string()), + Some(sign.to_js_string()), + None, + None, + ), + UnitFormatOptions::Unit { unit, display } => ( + None, + None, + None, + Some(unit.to_js_string()), + Some(display.to_js_string()), + ), + UnitFormatOptions::Decimal | UnitFormatOptions::Percent => { + (None, None, None, None, None) + } + }; + + let use_grouping = match nf_data.use_grouping { + GroupingStrategy::Auto => js_string!("auto").into(), + GroupingStrategy::Never => JsValue::from(false), + GroupingStrategy::Always => js_string!("always").into(), + GroupingStrategy::Min2 => js_string!("min2").into(), + _ => { + return Err(JsNativeError::typ() + .with_message("unsupported useGrouping value") + .into()); + } + }; + + let (notation, compact_display) = match &nf_data.formatter { + Formatter::Standard(_) => (NotationKind::Standard, None), + Formatter::Scientific(_) => (NotationKind::Scientific, None), + Formatter::Engineering(_) => (NotationKind::Engineering, None), + Formatter::Compact { display, .. } => (NotationKind::Compact, Some(*display)), + }; + + let sign_display_str = match nf_data.sign_display { + SignDisplay::Auto => js_string!("auto"), + SignDisplay::Never => js_string!("never"), + SignDisplay::Always => js_string!("always"), + SignDisplay::ExceptZero => js_string!("exceptZero"), + SignDisplay::Negative => js_string!("negative"), + _ => { + return Err(JsNativeError::typ() + .with_message("unsupported signDisplay value") + .into()); + } + }; + + ( + nf_data.locale.to_string(), + js_string!(nf_data.numbering_system.as_str()), + nf_data.unit_options.style().to_js_string(), + currency, + currency_display, + currency_sign, + unit, + unit_display, + nf_data.digit_options.minimum_integer_digits, + nf_data.digit_options.rounding_type.fraction_digits(), + nf_data.digit_options.rounding_type.significant_digits(), + use_grouping, + notation.to_js_string(), + compact_display.map(|d| d.to_js_string()), + sign_display_str, + nf_data.digit_options.rounding_increment.to_u16(), + nf_data.digit_options.rounding_priority.to_js_string(), + nf_data.digit_options.trailing_zero_display.to_js_string(), + ) + }; // 4. Let options be OrdinaryObjectCreate(%Object.prototype%). // 5. For each row of Table 12, except the header row, in table order, do @@ -767,62 +868,33 @@ impl NumberFormat { let mut options = ObjectInitializer::new(context); options.property( js_string!("locale"), - js_string!(nf.locale.to_string()), + js_string!(locale_str), Attribute::all(), ); options.property( js_string!("numberingSystem"), - js_string!(nf.numbering_system.as_str()), + numbering_system, Attribute::all(), ); - options.property( - js_string!("style"), - nf.unit_options.style().to_js_string(), - Attribute::all(), - ); + options.property(js_string!("style"), style, Attribute::all()); - match &nf.unit_options { - UnitFormatOptions::Currency { - currency, - display, - sign, - } => { - options.property( - js_string!("currency"), - currency.to_js_string(), - Attribute::all(), - ); - options.property( - js_string!("currencyDisplay"), - display.to_js_string(), - Attribute::all(), - ); - options.property( - js_string!("currencySign"), - sign.to_js_string(), - Attribute::all(), - ); - } - UnitFormatOptions::Unit { unit, display } => { - options.property(js_string!("unit"), unit.to_js_string(), Attribute::all()); - options.property( - js_string!("unitDisplay"), - display.to_js_string(), - Attribute::all(), - ); - } - UnitFormatOptions::Decimal | UnitFormatOptions::Percent => {} + if let (Some(c), Some(d), Some(s)) = (currency, currency_display, currency_sign) { + options.property(js_string!("currency"), c, Attribute::all()); + options.property(js_string!("currencyDisplay"), d, Attribute::all()); + options.property(js_string!("currencySign"), s, Attribute::all()); + } else if let (Some(u), Some(d)) = (unit, unit_display) { + options.property(js_string!("unit"), u, Attribute::all()); + options.property(js_string!("unitDisplay"), d, Attribute::all()); } options.property( js_string!("minimumIntegerDigits"), - nf.digit_options.minimum_integer_digits, + minimum_integer_digits, Attribute::all(), ); - if let Some(Extrema { minimum, maximum }) = nf.digit_options.rounding_type.fraction_digits() - { + if let Some(Extrema { minimum, maximum }) = fraction_digits { options .property( js_string!("minimumFractionDigits"), @@ -836,9 +908,7 @@ impl NumberFormat { ); } - if let Some(Extrema { minimum, maximum }) = - nf.digit_options.rounding_type.significant_digits() - { + if let Some(Extrema { minimum, maximum }) = significant_digits { options .property( js_string!("minimumSignificantDigits"), @@ -852,69 +922,33 @@ impl NumberFormat { ); } - let use_grouping = match nf.use_grouping { - GroupingStrategy::Auto => js_string!("auto").into(), - GroupingStrategy::Never => JsValue::from(false), - GroupingStrategy::Always => js_string!("always").into(), - GroupingStrategy::Min2 => js_string!("min2").into(), - _ => { - return Err(JsNativeError::typ() - .with_message("unsupported useGrouping value") - .into()); - } - }; - options.property(js_string!("useGrouping"), use_grouping, Attribute::all()); - let (notation, compact_display) = match &nf.formatter { - Formatter::Standard(_) => (NotationKind::Standard, None), - Formatter::Scientific(_) => (NotationKind::Scientific, None), - Formatter::Engineering(_) => (NotationKind::Engineering, None), - Formatter::Compact { display, .. } => (NotationKind::Compact, Some(*display)), - }; + options.property(js_string!("notation"), notation_str, Attribute::all()); - options.property( - js_string!("notation"), - notation.to_js_string(), - Attribute::all(), - ); - - if let Some(display) = compact_display { - options.property( - js_string!("compactDisplay"), - display.to_js_string(), - Attribute::all(), - ); + if let Some(display_str) = compact_display_str { + options.property(js_string!("compactDisplay"), display_str, Attribute::all()); } - let sign_display = match nf.sign_display { - SignDisplay::Auto => js_string!("auto"), - SignDisplay::Never => js_string!("never"), - SignDisplay::Always => js_string!("always"), - SignDisplay::ExceptZero => js_string!("exceptZero"), - SignDisplay::Negative => js_string!("negative"), - _ => { - return Err(JsNativeError::typ() - .with_message("unsupported signDisplay value") - .into()); - } - }; - options - .property(js_string!("signDisplay"), sign_display, Attribute::all()) + .property( + js_string!("signDisplay"), + sign_display_str, + Attribute::all(), + ) .property( js_string!("roundingIncrement"), - nf.digit_options.rounding_increment.to_u16(), + rounding_increment, Attribute::all(), ) .property( js_string!("roundingPriority"), - nf.digit_options.rounding_priority.to_js_string(), + rounding_priority_str, Attribute::all(), ) .property( js_string!("trailingZeroDisplay"), - nf.digit_options.trailing_zero_display.to_js_string(), + trailing_zero_display_str, Attribute::all(), ); diff --git a/core/engine/src/builtins/intl/plural_rules/mod.rs b/core/engine/src/builtins/intl/plural_rules/mod.rs index d1a7bde8b1a..ff7a7fd5b62 100644 --- a/core/engine/src/builtins/intl/plural_rules/mod.rs +++ b/core/engine/src/builtins/intl/plural_rules/mod.rs @@ -179,17 +179,17 @@ impl PluralRules { fn select(this: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult { // 1. Let pr be the this value. // 2. Perform ? RequireInternalSlot(pr, [[InitializedPluralRules]]). - let object = this.as_object(); - let plural_rules = object - .as_ref() - .and_then(|o| o.downcast_ref::()) - .ok_or_else(|| { - JsNativeError::typ() - .with_message("`select` can only be called on an `Intl.PluralRules` object") - })?; + let object = this.as_object().filter(|o| o.is::()).ok_or_else(|| { + JsNativeError::typ() + .with_message("`select` can only be called on an `Intl.PluralRules` object") + })?; let n = args.get_or_undefined(0).to_number(context)?; + let plural_rules = object + .downcast_ref::() + .expect("already checked that it is a PluralRules object"); + Ok(plural_category_to_js_string(resolve_plural(&plural_rules, n).category).into()) } @@ -206,15 +206,10 @@ impl PluralRules { fn select_range(this: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult { // 1. Let pr be the this value. // 2. Perform ? RequireInternalSlot(pr, [[InitializedPluralRules]]). - let object = this.as_object(); - let plural_rules = object - .as_ref() - .and_then(|o| o.downcast_ref::()) - .ok_or_else(|| { - JsNativeError::typ().with_message( - "`select_range` can only be called on an `Intl.PluralRules` object", - ) - })?; + let object = this.as_object().filter(|o| o.is::()).ok_or_else(|| { + JsNativeError::typ() + .with_message("`select_range` can only be called on an `Intl.PluralRules` object") + })?; // 3. If start is undefined or end is undefined, throw a TypeError exception. let x = args.get_or_undefined(0); @@ -230,6 +225,10 @@ impl PluralRules { // 5. Let y be ? ToNumber(end). let y = y.to_number(context)?; + let plural_rules = object + .downcast_ref::() + .expect("already checked that it is a PluralRules object"); + // 6. Return ? ResolvePluralRange(pr, x, y). // ResolvePluralRange(pr, x, y) // @@ -300,26 +299,56 @@ impl PluralRules { fn resolved_options(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { // 1. Let pr be the this value. // 2. Perform ? RequireInternalSlot(pr, [[InitializedPluralRules]]). - let object = this.as_object(); - let plural_rules = object - .as_ref() - .and_then(|o| o.downcast_ref::()) - .ok_or_else(|| { - JsNativeError::typ().with_message( - "`resolved_options` can only be called on an `Intl.PluralRules` object", - ) - })?; + let ( + locale_str, + rule_type, + notation, + minimum_integer_digits, + fraction_digits, + significant_digits, + rounding_increment, + rounding_mode, + rounding_priority, + trailing_zero_display, + plural_categories, + ) = { + let object = this.as_object(); + let plural_rules = object + .as_ref() + .and_then(|o| o.downcast_ref::()) + .ok_or_else(|| { + JsNativeError::typ().with_message( + "`resolved_options` can only be called on an `Intl.PluralRules` object", + ) + })?; + + ( + plural_rules.locale.to_string(), + plural_rules.rule_type, + plural_rules.notation, + plural_rules.format_options.minimum_integer_digits, + plural_rules.format_options.rounding_type.fraction_digits(), + plural_rules + .format_options + .rounding_type + .significant_digits(), + plural_rules.format_options.rounding_increment.to_u16(), + plural_rules.format_options.rounding_mode, + plural_rules.format_options.rounding_priority, + plural_rules.format_options.trailing_zero_display, + plural_rules + .native + .rules() + .categories() + .map(|category| plural_category_to_js_string(category).into()) + .collect::>(), + ) + }; // 3. Let options be OrdinaryObjectCreate(%Object.prototype%). // 4. Let pluralCategories be a List of Strings containing all possible results of // PluralRuleSelect for the selected locale pr.[[Locale]], sorted according to the following // order: "zero", "one", "two", "few", "many", "other". - let plural_categories = plural_rules - .native - .rules() - .categories() - .map(|category| plural_category_to_js_string(category).into()); - // 5. For each row of Table 30, except the header row, in table order, do // a. Let p be the Property value of the current row. // b. If p is "pluralCategories", then @@ -335,12 +364,12 @@ impl PluralRules { options .property( js_string!("locale"), - js_string!(plural_rules.locale.to_string()), + js_string!(locale_str), Attribute::all(), ) .property( js_string!("type"), - match plural_rules.rule_type { + match rule_type { PluralRuleType::Cardinal => js_string!("cardinal"), PluralRuleType::Ordinal => js_string!("ordinal"), _ => js_string!("unknown"), @@ -349,18 +378,16 @@ impl PluralRules { ) .property( js_string!("notation"), - plural_rules.notation.to_js_string(), + notation.to_js_string(), Attribute::all(), ) .property( js_string!("minimumIntegerDigits"), - plural_rules.format_options.minimum_integer_digits, + minimum_integer_digits, Attribute::all(), ); - if let Some(Extrema { minimum, maximum }) = - plural_rules.format_options.rounding_type.fraction_digits() - { + if let Some(Extrema { minimum, maximum }) = fraction_digits { options .property( js_string!("minimumFractionDigits"), @@ -374,11 +401,7 @@ impl PluralRules { ); } - if let Some(Extrema { minimum, maximum }) = plural_rules - .format_options - .rounding_type - .significant_digits() - { + if let Some(Extrema { minimum, maximum }) = significant_digits { options .property( js_string!("minimumSignificantDigits"), @@ -401,12 +424,12 @@ impl PluralRules { ) .property( js_string!("roundingIncrement"), - plural_rules.format_options.rounding_increment.to_u16(), + rounding_increment, Attribute::all(), ) .property( js_string!("roundingMode"), - match plural_rules.format_options.rounding_mode { + match rounding_mode { SignedRoundingMode::Unsigned(UnsignedRoundingMode::Expand) => { js_string!("expand") } @@ -432,15 +455,12 @@ impl PluralRules { ) .property( js_string!("roundingPriority"), - js_string!(plural_rules.format_options.rounding_priority.to_js_string()), + js_string!(rounding_priority.to_js_string()), Attribute::all(), ) .property( js_string!("trailingZeroDisplay"), - plural_rules - .format_options - .trailing_zero_display - .to_js_string(), + trailing_zero_display.to_js_string(), Attribute::all(), ); diff --git a/core/engine/src/builtins/intl/segmenter/iterator.rs b/core/engine/src/builtins/intl/segmenter/iterator.rs index fb361570826..1c165f39c9d 100644 --- a/core/engine/src/builtins/intl/segmenter/iterator.rs +++ b/core/engine/src/builtins/intl/segmenter/iterator.rs @@ -108,54 +108,71 @@ impl SegmentIterator { fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { // 1. Let iterator be the this value. // 2. Perform ? RequireInternalSlot(iterator, [[IteratingSegmenter]]). - let object = this.as_object(); - let mut iter = object - .as_ref() - .and_then(JsObject::downcast_mut::) - .ok_or_else(|| { - JsNativeError::typ() - .with_message("`next` can only be called on a `Segment Iterator` object") - })?; - - // 5. Let startIndex be iterator.[[IteratedStringNextSegmentCodeUnitIndex]]. - let start = iter.next_segment_index; - - // 4. Let string be iterator.[[IteratedString]]. - // 6. Let endIndex be ! FindBoundary(segmenter, string, startIndex, after). - let Some((end, is_word_like)) = iter.string.get(start..).and_then(|string| { - // 3. Let segmenter be iterator.[[IteratingSegmenter]]. - let segmenter = iter - .segmenter - .downcast_ref::() - .js_expect("segment iterator object should contain a segmenter") - .ok()?; - let mut segments = segmenter.native.segment(string.variant()); - // the first elem is always 0. - segments.next(); - segments - .next() - .map(|end| (start + end, segments.is_word_like())) - }) else { - // 7. If endIndex is not finite, then - // a. Return CreateIterResultObject(undefined, true). - return Ok(create_iter_result_object( - JsValue::undefined(), - true, - context, - )); + let object = this.as_object().filter(|o| o.is::()).ok_or_else(|| { + JsNativeError::typ() + .with_message("`next` can only be called on a `Segment Iterator` object") + })?; + + // Extract all data inside a scoped block so the mutable borrow is dropped + // before we pass `context` to `create_segment_data_object` / `create_iter_result_object` + // (those can trigger GC, and holding a RefMut across a GC point is a UAF). + let result: Option<(JsString, usize, usize, Option)> = { + let mut iter = object + .downcast_mut::() + .expect("already checked that it is a Segment Iterator object"); + + // 5. Let startIndex be iterator.[[IteratedStringNextSegmentCodeUnitIndex]]. + let start = iter.next_segment_index; + + // 4. Let string be iterator.[[IteratedString]]. + // 6. Let endIndex be ! FindBoundary(segmenter, string, startIndex, after). + let maybe_end = iter.string.get(start..).and_then(|string| { + // 3. Let segmenter be iterator.[[IteratingSegmenter]]. + let segmenter = iter + .segmenter + .downcast_ref::() + .js_expect("segment iterator object should contain a segmenter") + .ok()?; + let mut segments = segmenter.native.segment(string.variant()); + // the first elem is always 0. + segments.next(); + segments + .next() + .map(|end| (start + end, segments.is_word_like())) + }); + + if let Some((end, is_word_like)) = maybe_end { + // 8. Set iterator.[[IteratedStringNextSegmentCodeUnitIndex]] to endIndex. + iter.next_segment_index = end; + Some((iter.string.clone(), start, end, is_word_like)) + } else { + None + } }; - // 8. Set iterator.[[IteratedStringNextSegmentCodeUnitIndex]] to endIndex. - iter.next_segment_index = end; - - // 9. Let segmentData be ! CreateSegmentDataObject(segmenter, string, startIndex, endIndex). - let segment_data = - create_segment_data_object(iter.string.clone(), start..end, is_word_like, context); - - // 10. Return CreateIterResultObject(segmentData, false). - Ok(create_iter_result_object( - segment_data.into(), - false, - context, - )) + // RefMut<'_, SegmentIterator> is dropped here — safe to use context below. + + match result { + None => { + // 7. If endIndex is not finite, then + // a. Return CreateIterResultObject(undefined, true). + Ok(create_iter_result_object( + JsValue::undefined(), + true, + context, + )) + } + Some((string, start, end, is_word_like)) => { + // 9. Let segmentData be ! CreateSegmentDataObject(segmenter, string, startIndex, endIndex). + let segment_data = + create_segment_data_object(string, start..end, is_word_like, context); + + // 10. Return CreateIterResultObject(segmentData, false). + Ok(create_iter_result_object( + segment_data.into(), + false, + context, + )) + } + } } } diff --git a/core/engine/src/builtins/intl/segmenter/mod.rs b/core/engine/src/builtins/intl/segmenter/mod.rs index 120f17d0d26..5fe4caffaab 100644 --- a/core/engine/src/builtins/intl/segmenter/mod.rs +++ b/core/engine/src/builtins/intl/segmenter/mod.rs @@ -266,15 +266,22 @@ impl Segmenter { fn resolved_options(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { // 1. Let segmenter be the this value. // 2. Perform ? RequireInternalSlot(segmenter, [[InitializedSegmenter]]). - let object = this.as_object(); - let segmenter = object - .as_ref() - .and_then(JsObject::downcast_ref::) - .ok_or_else(|| { - JsNativeError::typ().with_message( - "`resolved_options` can only be called on an `Intl.Segmenter` object", - ) - })?; + let (locale_str, granularity_str) = { + let object = this.as_object(); + let segmenter = object + .as_ref() + .and_then(JsObject::downcast_ref::) + .ok_or_else(|| { + JsNativeError::typ().with_message( + "`resolved_options` can only be called on an `Intl.Segmenter` object", + ) + })?; + + ( + segmenter.locale.to_string(), + segmenter.native.granularity().to_string(), + ) + }; // 3. Let options be OrdinaryObjectCreate(%Object.prototype%). // 4. For each row of Table 19, except the header row, in table order, do @@ -285,12 +292,12 @@ impl Segmenter { let options = ObjectInitializer::new(context) .property( js_string!("locale"), - js_string!(segmenter.locale.to_string()), + js_string!(locale_str), Attribute::all(), ) .property( js_string!("granularity"), - js_string!(segmenter.native.granularity().to_string()), + js_string!(granularity_str), Attribute::all(), ) .build(); diff --git a/core/engine/src/builtins/intl/segmenter/segments.rs b/core/engine/src/builtins/intl/segmenter/segments.rs index 248acc71318..857282132e7 100644 --- a/core/engine/src/builtins/intl/segmenter/segments.rs +++ b/core/engine/src/builtins/intl/segmenter/segments.rs @@ -56,14 +56,20 @@ impl Segments { fn containing(this: &JsValue, args: &[JsValue], context: &mut Context) -> JsResult { // 1. Let segments be the this value. // 2. Perform ? RequireInternalSlot(segments, [[SegmentsSegmenter]]). - let object = this.as_object(); + let object = this.as_object().filter(|o| o.is::()).ok_or_else(|| { + JsNativeError::typ() + .with_message("`containing` can only be called on a `Segments` object") + })?; + + // 6. Let n be ? ToIntegerOrInfinity(index). + let n_val = args + .get_or_undefined(0) + .to_integer_or_infinity(context)? + .as_integer(); + let segments = object - .as_ref() - .and_then(JsObject::downcast_ref::) - .ok_or_else(|| { - JsNativeError::typ() - .with_message("`containing` can only be called on a `Segments` object") - })?; + .downcast_ref::() + .expect("already checked that it is a Segments object"); // 3. Let segmenter be segments.[[SegmentsSegmenter]]. let segmenter = segments @@ -75,11 +81,7 @@ impl Segments { // 5. Let len be the length of string. let len = segments.string.len() as i64; - // 6. Let n be ? ToIntegerOrInfinity(index). - let Some(n) = args - .get_or_undefined(0) - .to_integer_or_infinity(context)? - .as_integer() + let Some(n) = n_val // 7. If n < 0 or n ≥ len, return undefined. .filter(|i| (0..len).contains(i)) .map(|n| n as usize) diff --git a/core/engine/src/builtins/iterable/async_from_sync_iterator.rs b/core/engine/src/builtins/iterable/async_from_sync_iterator.rs index d1c2bbd4e6d..64cee86d4aa 100644 --- a/core/engine/src/builtins/iterable/async_from_sync_iterator.rs +++ b/core/engine/src/builtins/iterable/async_from_sync_iterator.rs @@ -364,14 +364,17 @@ impl AsyncFromSyncIterator { let on_fulfilled = FunctionObjectBuilder::new( context.realm(), context.gc_collector(), - NativeFunction::from_copy_closure(move |_this, args, context| { - // a. Return CreateIterResultObject(value, done). - Ok(create_iter_result_object( - args.get_or_undefined(0).clone(), - done, - context, - )) - }), + NativeFunction::from_copy_closure( + context.gc_collector(), + move |_this, args, context| { + // a. Return CreateIterResultObject(value, done). + Ok(create_iter_result_object( + args.get_or_undefined(0).clone(), + done, + context, + )) + }, + ), ) .name(js_string!()) .length(1) @@ -397,6 +400,7 @@ impl AsyncFromSyncIterator { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, iter, context| { // i. Return ? IteratorClose(syncIteratorRecord, ThrowCompletion(error)). iter.close( diff --git a/core/engine/src/builtins/iterable/iterator_constructor.rs b/core/engine/src/builtins/iterable/iterator_constructor.rs index 9ede87f269f..617dc858b71 100644 --- a/core/engine/src/builtins/iterable/iterator_constructor.rs +++ b/core/engine/src/builtins/iterable/iterator_constructor.rs @@ -85,7 +85,7 @@ pub(crate) struct IteratorConstructor; impl IntrinsicObject for IteratorConstructor { fn init(realm: &Realm, mc: &boa_gc::MutationContext<'static, '_>) { let iterator_prototype = realm.intrinsics().constructors().iterator().prototype(); - let builder = BuiltInBuilder::from_standard_constructor::(realm, mc) + let mut builder = BuiltInBuilder::from_standard_constructor::(realm, mc) .inherits(Some(iterator_prototype.clone())) // Static methods .static_method(Self::from, js_string!("from"), 1) @@ -242,7 +242,10 @@ impl IteratorConstructor { // (implemented via IteratorHelperOp::Concat in execute_next) // 4-5. Let result be CreateIteratorFromClosure(closure, "Iterator Helper", ...) // with [[UnderlyingIterators]] set to a new empty List. - let helper = IteratorHelper::create(iterator_helper::Concat::new(iterables), context); + let helper = IteratorHelper::create( + iterator_helper::Concat::new(context.gc_collector(), iterables), + context, + ); // 6. Return result. Ok(helper.into()) @@ -303,7 +306,13 @@ impl IteratorConstructor { // 15. Let finishResults be a new Abstract Closure ... (handled in ZipIterator::create_zip_iterator) // 16. Return ? IteratorZip(iters, mode, padding, finishResults). let helper = IteratorHelper::create( - iterator_helper::Zip::new(iters, mode, padding, ZipResultKind::Array), + iterator_helper::Zip::new( + context.gc_collector(), + iters, + mode, + padding, + ZipResultKind::Array, + ), context, ); Ok(helper.into()) @@ -391,7 +400,13 @@ impl IteratorConstructor { // 15.b.c. Return obj. // All this is done within `Zip`. let helper = IteratorHelper::create( - iterator_helper::Zip::new(iters, mode, padding, ZipResultKind::Keyed(keys)), + iterator_helper::Zip::new( + context.gc_collector(), + iters, + mode, + padding, + ZipResultKind::Keyed(keys), + ), context, ); diff --git a/core/engine/src/builtins/iterable/iterator_helper/concat.rs b/core/engine/src/builtins/iterable/iterator_helper/concat.rs index cbda860728a..5526971b744 100644 --- a/core/engine/src/builtins/iterable/iterator_helper/concat.rs +++ b/core/engine/src/builtins/iterable/iterator_helper/concat.rs @@ -43,11 +43,14 @@ impl Concat { clippy::new_ret_no_self, reason = "slightly cleaner to have this be a `new` method" )] - pub(crate) fn new(iterables: VecDeque) -> NativeCoroutine { + pub(crate) fn new( + mc: &boa_gc::MutationContext<'_, '_>, + iterables: VecDeque, + ) -> NativeCoroutine { // 3. Let closure be a new Abstract Closure with no parameters that captures // iterables and performs the following steps when called: NativeCoroutine::from_copy_closure_with_captures( - // a. For each Record iterable of iterables, do + mc, // a. For each Record iterable of iterables, do |completion, state, context| { let st = state.take(); match &st { diff --git a/core/engine/src/builtins/iterable/iterator_helper/drop.rs b/core/engine/src/builtins/iterable/iterator_helper/drop.rs index ffc4326c6b8..a12a263b9f3 100644 --- a/core/engine/src/builtins/iterable/iterator_helper/drop.rs +++ b/core/engine/src/builtins/iterable/iterator_helper/drop.rs @@ -31,12 +31,16 @@ impl Drop { clippy::new_ret_no_self, reason = "slightly cleaner to have this be a `new` method" )] - pub(crate) fn new(iterated: IteratorRecord, limit: Option) -> NativeCoroutine { + pub(crate) fn new( + mc: &boa_gc::MutationContext<'_, '_>, + iterated: IteratorRecord, + limit: Option, + ) -> NativeCoroutine { // 10. Let closure be a new Abstract Closure with no parameters that // captures iterated and integerLimit and performs the following steps // when called: NativeCoroutine::from_copy_closure_with_captures( - // a. Let remaining be integerLimit. + mc, // a. Let remaining be integerLimit. // c. Repeat, |completion, state, context| { let st = state.take(); diff --git a/core/engine/src/builtins/iterable/iterator_helper/filter.rs b/core/engine/src/builtins/iterable/iterator_helper/filter.rs index 93e74bc9204..57d1aaf73ba 100644 --- a/core/engine/src/builtins/iterable/iterator_helper/filter.rs +++ b/core/engine/src/builtins/iterable/iterator_helper/filter.rs @@ -31,11 +31,15 @@ impl Filter { clippy::new_ret_no_self, reason = "slightly cleaner to have this be a `new` method" )] - pub(crate) fn new(iterated: IteratorRecord, predicate: JsFunction) -> NativeCoroutine { + pub(crate) fn new( + mc: &boa_gc::MutationContext<'_, '_>, + iterated: IteratorRecord, + predicate: JsFunction, + ) -> NativeCoroutine { // 6. Let closure be a new Abstract Closure with no parameters that captures // iterated and predicate and performs the following steps when called: NativeCoroutine::from_copy_closure_with_captures( - // a. Let counter be 0. + mc, // a. Let counter be 0. // b. Repeat, |completion, state, context| { let st = state.take(); diff --git a/core/engine/src/builtins/iterable/iterator_helper/flat_map.rs b/core/engine/src/builtins/iterable/iterator_helper/flat_map.rs index 96529387de9..f41cde0f8e8 100644 --- a/core/engine/src/builtins/iterable/iterator_helper/flat_map.rs +++ b/core/engine/src/builtins/iterable/iterator_helper/flat_map.rs @@ -39,11 +39,15 @@ impl FlatMap { clippy::new_ret_no_self, reason = "slightly cleaner to have this be a `new` method" )] - pub(crate) fn new(iterated: IteratorRecord, mapper: JsFunction) -> NativeCoroutine { + pub(crate) fn new( + mc: &boa_gc::MutationContext<'_, '_>, + iterated: IteratorRecord, + mapper: JsFunction, + ) -> NativeCoroutine { // 6. Let closure be a new Abstract Closure with no parameters that captures // iterated and mapper and performs the following steps when called: NativeCoroutine::from_copy_closure_with_captures( - // a. Let counter be 0. + mc, // a. Let counter be 0. // b. Repeat, |completion, state, context| { let st = state.take(); diff --git a/core/engine/src/builtins/iterable/iterator_helper/map.rs b/core/engine/src/builtins/iterable/iterator_helper/map.rs index 2621696a614..f0879407722 100644 --- a/core/engine/src/builtins/iterable/iterator_helper/map.rs +++ b/core/engine/src/builtins/iterable/iterator_helper/map.rs @@ -31,11 +31,15 @@ impl Map { clippy::new_ret_no_self, reason = "slightly cleaner to have this be a `new` method" )] - pub(crate) fn new(iterated: IteratorRecord, mapper: JsFunction) -> NativeCoroutine { + pub(crate) fn new( + mc: &boa_gc::MutationContext<'_, '_>, + iterated: IteratorRecord, + mapper: JsFunction, + ) -> NativeCoroutine { // 6. Let closure be a new Abstract Closure with no parameters that captures // iterated and mapper and performs the following steps when called: NativeCoroutine::from_copy_closure_with_captures( - // a. Let counter be 0. + mc, // a. Let counter be 0. // b. Repeat, |completion, state, context| { let st = state.take(); diff --git a/core/engine/src/builtins/iterable/iterator_helper/take.rs b/core/engine/src/builtins/iterable/iterator_helper/take.rs index 91e353bd4d4..b1feee3b533 100644 --- a/core/engine/src/builtins/iterable/iterator_helper/take.rs +++ b/core/engine/src/builtins/iterable/iterator_helper/take.rs @@ -29,12 +29,16 @@ impl Take { clippy::new_ret_no_self, reason = "slightly cleaner to have this be a `new` method" )] - pub(crate) fn new(iterated: IteratorRecord, limit: Option) -> NativeCoroutine { + pub(crate) fn new( + mc: &boa_gc::MutationContext<'_, '_>, + iterated: IteratorRecord, + limit: Option, + ) -> NativeCoroutine { // 10. Let closure be a new Abstract Closure with no parameters that // captures iterated and integerLimit and performs the following steps // when called: NativeCoroutine::from_copy_closure_with_captures( - // a. Let remaining be integerLimit. + mc, // a. Let remaining be integerLimit. // b. Repeat, |completion, state, context| { let st = state.take(); diff --git a/core/engine/src/builtins/iterable/iterator_helper/zip.rs b/core/engine/src/builtins/iterable/iterator_helper/zip.rs index 988575f5843..7f13d07502f 100644 --- a/core/engine/src/builtins/iterable/iterator_helper/zip.rs +++ b/core/engine/src/builtins/iterable/iterator_helper/zip.rs @@ -54,6 +54,7 @@ impl Zip { reason = "slightly cleaner to have this be a `new` method" )] pub(crate) fn new( + mc: &boa_gc::MutationContext<'_, '_>, iters: Vec, mode: ZipMode, padding: Vec, @@ -62,6 +63,7 @@ impl Zip { let iters = iters.into_iter().map(Some).collect(); NativeCoroutine::from_copy_closure_with_captures( + mc, |completion, state, context| { let st = state.take(); let (mut iters, mode, padding, result_kind) = match st { diff --git a/core/engine/src/builtins/iterable/iterator_prototype.rs b/core/engine/src/builtins/iterable/iterator_prototype.rs index f244ae1c6f3..abe398f4f73 100644 --- a/core/engine/src/builtins/iterable/iterator_prototype.rs +++ b/core/engine/src/builtins/iterable/iterator_prototype.rs @@ -235,7 +235,10 @@ impl Iterator { let iterated = get_iterator_direct(iterated.iterator(), context)?; // 6-8 are deferred to `IteratorHelper::create` and `Map::new`. - let result = IteratorHelper::create(iterator_helper::Map::new(iterated, mapper), context); + let result = IteratorHelper::create( + iterator_helper::Map::new(context.gc_collector(), iterated, mapper), + context, + ); // 9. Return result. Ok(result.into()) @@ -274,8 +277,10 @@ impl Iterator { let iterated = get_iterator_direct(iterated.iterator(), context)?; // 6-8 are deferred to `IteratorHelper::create` and `Filter::new`. - let result = - IteratorHelper::create(iterator_helper::Filter::new(iterated, predicate), context); + let result = IteratorHelper::create( + iterator_helper::Filter::new(context.gc_collector(), iterated, predicate), + context, + ); // 9. Return result. Ok(result.into()) @@ -335,8 +340,10 @@ impl Iterator { let iterated = get_iterator_direct(iterated.iterator(), context)?; // 10-12 are deferred to `IteratorHelper::create` and `Take::new`. - let result = - IteratorHelper::create(iterator_helper::Take::new(iterated, integer_limit), context); + let result = IteratorHelper::create( + iterator_helper::Take::new(context.gc_collector(), iterated, integer_limit), + context, + ); // 13. Return result. Ok(result.into()) @@ -395,8 +402,10 @@ impl Iterator { let iterated = get_iterator_direct(iterated.iterator(), context)?; // 10-12 are deferred to `IteratorHelper::create` and `Drop::new`. - let result = - IteratorHelper::create(iterator_helper::Drop::new(iterated, integer_limit), context); + let result = IteratorHelper::create( + iterator_helper::Drop::new(context.gc_collector(), iterated, integer_limit), + context, + ); // 13. Return result. Ok(result.into()) @@ -435,8 +444,10 @@ impl Iterator { let iterated = get_iterator_direct(iterated.iterator(), context)?; // 6-8 are deferred to `IteratorHelper::create` and `FlatMap::new`. - let helper = - IteratorHelper::create(iterator_helper::FlatMap::new(iterated, mapper), context); + let helper = IteratorHelper::create( + iterator_helper::FlatMap::new(context.gc_collector(), iterated, mapper), + context, + ); // 9. Return result. Ok(helper.into()) diff --git a/core/engine/src/builtins/iterable/mod.rs b/core/engine/src/builtins/iterable/mod.rs index 2b2962d24d0..eb816a6a4c3 100644 --- a/core/engine/src/builtins/iterable/mod.rs +++ b/core/engine/src/builtins/iterable/mod.rs @@ -89,13 +89,6 @@ pub struct IteratorPrototypes { wrap_for_valid_iterator: JsObject, } -impl Default for IteratorPrototypes { - fn default() -> Self { - // SAFETY: The global mutation context is used as a fallback during the context threading migration. - Self::uninit_in(&boa_gc::MutationContext::global()) - } -} - impl IteratorPrototypes { pub(crate) fn uninit_in(mc: &boa_gc::MutationContext<'static, '_>) -> Self { Self { @@ -113,7 +106,6 @@ impl IteratorPrototypes { wrap_for_valid_iterator: JsObject::with_null_proto(mc), } } - /// Returns the `ArrayIteratorPrototype` object. #[inline] #[must_use] diff --git a/core/engine/src/builtins/json/mod.rs b/core/engine/src/builtins/json/mod.rs index 4e162119896..2b57237d055 100644 --- a/core/engine/src/builtins/json/mod.rs +++ b/core/engine/src/builtins/json/mod.rs @@ -303,8 +303,8 @@ impl Json { false, false, context.interner_mut(), - gc, - in_with, + &gc, + false, spanned_source_text, SourcePath::Json, ); diff --git a/core/engine/src/builtins/map/map_iterator.rs b/core/engine/src/builtins/map/map_iterator.rs index 326f28a8567..5789abdd94b 100644 --- a/core/engine/src/builtins/map/map_iterator.rs +++ b/core/engine/src/builtins/map/map_iterator.rs @@ -106,41 +106,54 @@ impl MapIterator { /// /// [spec]: https://tc39.es/ecma262/#sec-%mapiteratorprototype%.next pub(crate) fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { - let object = this.as_object(); - let mut map_iterator = object - .as_ref() - .and_then(JsObject::downcast_mut::) + let object = this + .as_object() + .filter(|o| o.is::()) .ok_or_else(|| JsNativeError::typ().with_message("`this` is not a MapIterator"))?; - let item_kind = map_iterator.iteration_kind; + let (item_kind, element, iterated_map) = { + let mut map_iterator = object + .downcast_mut::() + .expect("already checked that it is a MapIterator"); - if let Some(obj) = map_iterator.iterated_map.take() { - let e = { - let mut entries = obj.0.borrow_mut(); - let entries = entries.data_mut(); - let len = entries.full_len(); - loop { - let element = entries - .get_index(map_iterator.next_index) - .map(|(v, k)| (v.clone(), k.clone())); - map_iterator.next_index += 1; - if element.is_some() || map_iterator.next_index >= len { - break element; - } - } - }; - if let Some((key, value)) = e { - let item = match item_kind { - PropertyNameKind::Key => Ok(create_iter_result_object(key, false, context)), - PropertyNameKind::Value => Ok(create_iter_result_object(value, false, context)), - PropertyNameKind::KeyAndValue => { - let result = Array::create_array_from_list([key, value], context); - Ok(create_iter_result_object(result.into(), false, context)) + let item_kind = map_iterator.iteration_kind; + + if let Some(obj) = map_iterator.iterated_map.take() { + let e = { + let mut entries = obj.0.borrow_mut(); + let entries = entries.data_mut(); + let len = entries.full_len(); + loop { + let element = entries + .get_index(map_iterator.next_index) + .map(|(v, k)| (v.clone(), k.clone())); + map_iterator.next_index += 1; + if element.is_some() || map_iterator.next_index >= len { + break element; + } } }; - map_iterator.iterated_map = Some(obj); - return item; + (item_kind, e, Some(obj)) + } else { + (item_kind, None, None) } + }; + + if let (Some((key, value)), Some(obj)) = (element, iterated_map) { + object + .downcast_mut::() + .expect("already checked") + .iterated_map = Some(obj); + + let item = match item_kind { + PropertyNameKind::Key => Ok(create_iter_result_object(key, false, context)), + PropertyNameKind::Value => Ok(create_iter_result_object(value, false, context)), + PropertyNameKind::KeyAndValue => { + let result = Array::create_array_from_list([key, value], context); + Ok(create_iter_result_object(result.into(), false, context)) + } + }; + return item; } Ok(create_iter_result_object( diff --git a/core/engine/src/builtins/object/for_in_iterator.rs b/core/engine/src/builtins/object/for_in_iterator.rs index dfeac3601e1..03fec1e6fee 100644 --- a/core/engine/src/builtins/object/for_in_iterator.rs +++ b/core/engine/src/builtins/object/for_in_iterator.rs @@ -86,16 +86,28 @@ impl ForInIterator { /// /// [spec]: https://tc39.es/ecma262/#sec-%foriniteratorprototype%.next pub(crate) fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { - let object = this.as_object(); - let mut iterator = object - .as_ref() - .and_then(|o| o.downcast_mut::()) + let object = this + .as_object() + .filter(|o| o.is::()) .ok_or_else(|| JsNativeError::typ().with_message("`this` is not a ForInIterator"))?; - let mut object = iterator.object.to_object(context)?; + + let mut current_object = { + let iterator = object.downcast_ref::().expect("already checked"); + iterator.object.clone() + }; + + let mut current_object_obj = current_object.to_object(context)?; loop { - if !iterator.object_was_visited { - let keys = object + let was_visited = { + let iterator = object.downcast_ref::().expect("checked"); + iterator.object_was_visited + }; + + if !was_visited { + let keys = current_object_obj .__own_property_keys__(&mut InternalMethodPropertyContext::new(context))?; + + let mut iterator = object.downcast_mut::().expect("checked"); for k in keys { match k { PropertyKey::String(ref k) => { @@ -109,23 +121,40 @@ impl ForInIterator { } iterator.object_was_visited = true; } - while let Some(r) = iterator.remaining_keys.pop_front() { - if !iterator.visited_keys.contains(&r) - && let Some(desc) = object.__get_own_property__( + + loop { + let r = { + let mut iterator = object.downcast_mut::().expect("checked"); + iterator.remaining_keys.pop_front() + }; + + let Some(r) = r else { break }; + + let already_visited = { + let iterator = object.downcast_ref::().expect("checked"); + iterator.visited_keys.contains(&r) + }; + + if !already_visited { + let desc = current_object_obj.__get_own_property__( &PropertyKey::from(r.clone()), &mut InternalMethodPropertyContext::new(context), - )? - { - iterator.visited_keys.insert(r.clone()); - if desc.expect_enumerable() { - return Ok(create_iter_result_object(JsValue::new(r), false, context)); + )?; + + if let Some(desc) = desc { + let mut iterator = object.downcast_mut::().expect("checked"); + iterator.visited_keys.insert(r.clone()); + if desc.expect_enumerable() { + return Ok(create_iter_result_object(JsValue::new(r), false, context)); + } } } } - let proto = object.prototype().clone(); + + let proto = current_object_obj.prototype().clone(); match proto { Some(o) => { - object = o; + current_object_obj = o; } _ => { return Ok(create_iter_result_object( @@ -135,7 +164,9 @@ impl ForInIterator { )); } } - iterator.object = JsValue::new(object.clone()); + + let mut iterator = object.downcast_mut::().expect("checked"); + iterator.object = JsValue::new(current_object_obj.clone()); iterator.object_was_visited = false; } } diff --git a/core/engine/src/builtins/object/mod.rs b/core/engine/src/builtins/object/mod.rs index 8872a4c2815..49dad929215 100644 --- a/core/engine/src/builtins/object/mod.rs +++ b/core/engine/src/builtins/object/mod.rs @@ -1309,6 +1309,7 @@ impl OrdinaryObject { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, obj, context| { let key = args.get_or_undefined(0); let value = args.get_or_undefined(1); diff --git a/core/engine/src/builtins/promise/mod.rs b/core/engine/src/builtins/promise/mod.rs index 6d7d2c1c76c..15a0f6c3ad8 100644 --- a/core/engine/src/builtins/promise/mod.rs +++ b/core/engine/src/builtins/promise/mod.rs @@ -254,6 +254,7 @@ impl PromiseCapability { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args: &[JsValue], captures, _| { let mut promise_capability = captures.borrow_mut(); // a. If promiseCapability.[[Resolve]] is not undefined, throw a TypeError exception. @@ -685,6 +686,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, captures, context| { // https://tc39.es/ecma262/#sec-promise.all-resolve-element-functions @@ -904,6 +906,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, captures, context| { // https://tc39.es/ecma262/#sec-promise.allsettled-resolve-element-functions @@ -998,6 +1001,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, captures, context| { // https://tc39.es/ecma262/#sec-promise.allsettled-reject-element-functions @@ -1286,6 +1290,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, captures, context| { // 1. If alreadyCalled.[[Value]] is true, return undefined. if captures.already_called.get() { @@ -1372,6 +1377,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, captures, context| { // 1. If alreadyCalled.[[Value]] is true, return undefined. if captures.already_called.get() { @@ -1595,6 +1601,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, captures, context| { // https://tc39.es/ecma262/#sec-promise.any-reject-element-functions @@ -2029,6 +2036,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, captures, context| { /// Capture object for the abstract `returnValue` closure. #[derive(Debug, Trace, Finalize)] @@ -2051,6 +2059,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, _args, captures, _context| { // 1. Return value. Ok(captures.value.clone()) @@ -2082,6 +2091,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, captures, context| { /// Capture object for the abstract `throwReason` closure. #[derive(Debug, Trace, Finalize)] @@ -2104,6 +2114,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, _args, captures, _context| { // 1. Return ThrowCompletion(reason). Err(JsError::from_opaque(captures.reason.clone())) @@ -2479,6 +2490,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, captures, context| { // https://tc39.es/ecma262/#sec-promise-resolve-functions @@ -2577,6 +2589,7 @@ impl Promise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, captures, context| { // https://tc39.es/ecma262/#sec-promise-reject-functions diff --git a/core/engine/src/builtins/proxy/mod.rs b/core/engine/src/builtins/proxy/mod.rs index 4b7c4d5e27c..046e15ff75b 100644 --- a/core/engine/src/builtins/proxy/mod.rs +++ b/core/engine/src/builtins/proxy/mod.rs @@ -197,6 +197,7 @@ impl Proxy { // 4. Set revoker.[[RevocableProxy]] to p. NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, _, revocable_proxy, _| { // a. Let F be the active function object. // b. Let p be F.[[RevocableProxy]]. @@ -547,6 +548,7 @@ pub(crate) fn proxy_exotic_get_own_property( extensible_target, result_desc.clone(), target_desc.clone(), + context.gc_collector(), ) { return Err(JsNativeError::typ() .with_message("Proxy trap returned unexpected property") @@ -662,6 +664,7 @@ pub(crate) fn proxy_exotic_define_own_property( extensible_target, desc.clone(), Some(target_desc.clone()), + context.gc_collector(), ) { return Err(JsNativeError::typ() .with_message("Proxy trap set property to unexpected value") diff --git a/core/engine/src/builtins/regexp/regexp_string_iterator.rs b/core/engine/src/builtins/regexp/regexp_string_iterator.rs index 0e72a55a759..9bcc1e9371c 100644 --- a/core/engine/src/builtins/regexp/regexp_string_iterator.rs +++ b/core/engine/src/builtins/regexp/regexp_string_iterator.rs @@ -115,14 +115,29 @@ impl RegExpStringIterator { /// /// [spec]: https://tc39.es/ecma262/#sec-%regexpstringiteratorprototype%.next pub(crate) fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { - let object = this.as_object(); - let mut iterator = object - .as_ref() - .and_then(JsObject::downcast_mut::) - .ok_or_else(|| { - JsNativeError::typ().with_message("`this` is not a RegExpStringIterator") - })?; - if iterator.completed { + // Extract all state we need in a scoped block to drop the RefMut before + // any context call. Holding a RefMut<'_, T> across context operations is a + // use-after-free because the GC can collect the backing object while the + // mutable borrow guard is live. + let object = this.as_object().filter(|o| o.is::()).ok_or_else(|| { + JsNativeError::typ().with_message("`this` is not a RegExpStringIterator") + })?; + + let (completed, matcher, string, global, unicode) = { + let iterator = object + .downcast_ref::() + .expect("already checked that it is a RegExpStringIterator"); + ( + iterator.completed, + iterator.matcher.clone(), + iterator.string.clone(), + iterator.global, + iterator.unicode, + ) + }; + // RefMut dropped here — safe to use context below. + + if completed { return Ok(create_iter_result_object( JsValue::undefined(), true, @@ -133,14 +148,18 @@ impl RegExpStringIterator { // TODO: This is the code that should be created as a closure in create_regexp_string_iterator. // i. Let match be ? RegExpExec(R, S). - let m = RegExp::abstract_exec(&iterator.matcher, iterator.string.clone(), context)?; + let m = RegExp::abstract_exec(&matcher, string.clone(), context)?; if let Some(m) = m { // iii. If global is false, then - if !iterator.global { + if !global { // 1. Perform ? Yield(match). // 2. Return undefined. - iterator.completed = true; + // Write back completed = true (no borrow held before this point). + object + .downcast_mut::() + .expect("already checked") + .completed = true; return Ok(create_iter_result_object(m.into(), false, context)); } @@ -150,26 +169,25 @@ impl RegExpStringIterator { // v. If matchStr is the empty String, then if m_str.is_empty() { // 1. Let thisIndex be ℝ(? ToLength(? Get(R, "lastIndex"))). - let this_index = iterator - .matcher + let this_index = matcher .get(js_string!("lastIndex"), context)? .to_length(context)?; // 2. Let nextIndex be ! AdvanceStringIndex(S, thisIndex, fullUnicode). - let next_index = - advance_string_index(&iterator.string, this_index, iterator.unicode); + let next_index = advance_string_index(&string, this_index, unicode); // 3. Perform ? Set(R, "lastIndex", 𝔽(nextIndex), true). - iterator - .matcher - .set(js_string!("lastIndex"), next_index, true, context)?; + matcher.set(js_string!("lastIndex"), next_index, true, context)?; } // vi. Perform ? Yield(match). Ok(create_iter_result_object(m.into(), false, context)) } else { // ii. If match is null, return undefined. - iterator.completed = true; + object + .downcast_mut::() + .expect("already checked") + .completed = true; Ok(create_iter_result_object( JsValue::undefined(), true, diff --git a/core/engine/src/builtins/set/set_iterator.rs b/core/engine/src/builtins/set/set_iterator.rs index 5872b9f0b94..792f0b68268 100644 --- a/core/engine/src/builtins/set/set_iterator.rs +++ b/core/engine/src/builtins/set/set_iterator.rs @@ -105,39 +105,51 @@ impl SetIterator { /// /// [spec]: https://tc39.es/ecma262/#sec-%setiteratorprototype%.next pub(crate) fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { - let object = this.as_object(); - let mut set_iterator = object - .as_ref() - .and_then(JsObject::downcast_mut::) - .ok_or_else(|| JsNativeError::typ().with_message("`this` is not an SetIterator"))?; + let object = this + .as_object() + .filter(|o| o.is::()) + .ok_or_else(|| JsNativeError::typ().with_message("`this` is not a SetIterator"))?; - let item_kind = set_iterator.iteration_kind; + let (item_kind, element, iterated_set) = { + let mut set_iterator = object + .downcast_mut::() + .expect("already checked that it is a SetIterator"); - if let Some(obj) = set_iterator.iterated_set.take() { - let e = { - let mut entries = obj.0.borrow_mut(); - let entries = entries.data_mut(); - let len = entries.full_len(); - loop { - let element = entries.get_index(set_iterator.next_index); - set_iterator.next_index += 1; - if element.is_some() || set_iterator.next_index >= len { - break element.cloned(); - } - } - }; - if let Some(element) = e { - let item = match item_kind { - PropertyNameKind::KeyAndValue => { - let result = - Array::create_array_from_list([element.clone(), element], context); - Ok(create_iter_result_object(result.into(), false, context)) + let item_kind = set_iterator.iteration_kind; + + if let Some(obj) = set_iterator.iterated_set.take() { + let e = { + let mut entries = obj.0.borrow_mut(); + let entries = entries.data_mut(); + let len = entries.full_len(); + loop { + let element = entries.get_index(set_iterator.next_index); + set_iterator.next_index += 1; + if element.is_some() || set_iterator.next_index >= len { + break element.cloned(); + } } - _ => Ok(create_iter_result_object(element, false, context)), }; - set_iterator.iterated_set = Some(obj); - return item; + (item_kind, e, Some(obj)) + } else { + (item_kind, None, None) } + }; + + if let (Some(element), Some(obj)) = (element, iterated_set) { + object + .downcast_mut::() + .expect("already checked") + .iterated_set = Some(obj); + + let item = match item_kind { + PropertyNameKind::KeyAndValue => { + let result = Array::create_array_from_list([element.clone(), element], context); + Ok(create_iter_result_object(result.into(), false, context)) + } + _ => Ok(create_iter_result_object(element, false, context)), + }; + return item; } Ok(create_iter_result_object( diff --git a/core/engine/src/builtins/string/string_iterator.rs b/core/engine/src/builtins/string/string_iterator.rs index bf15695adfa..7b8d7d540d2 100644 --- a/core/engine/src/builtins/string/string_iterator.rs +++ b/core/engine/src/builtins/string/string_iterator.rs @@ -69,35 +69,49 @@ impl StringIterator { /// `StringIterator.prototype.next( )` pub(crate) fn next(this: &JsValue, _: &[JsValue], context: &mut Context) -> JsResult { - let object = this.as_object(); - let mut string_iterator = object - .as_ref() - .and_then(JsObject::downcast_mut::) - .ok_or_else(|| JsNativeError::typ().with_message("`this` is not an ArrayIterator"))?; + let object = this + .as_object() + .filter(|o| o.is::()) + .ok_or_else(|| JsNativeError::typ().with_message("`this` is not a StringIterator"))?; - if string_iterator.string.is_empty() { + let (mut string, position) = { + let string_iterator = object + .downcast_ref::() + .expect("already checked that it is a StringIterator"); + (string_iterator.string.clone(), string_iterator.next_index) + }; + + if string.is_empty() { return Ok(create_iter_result_object( JsValue::undefined(), true, context, )); } - let native_string = &string_iterator.string; - let len = native_string.len(); - let position = string_iterator.next_index; + let len = string.len(); if position >= len { - string_iterator.string = js_string!(); + object + .downcast_mut::() + .expect("already checked") + .string = js_string!(); return Ok(create_iter_result_object( JsValue::undefined(), true, context, )); } - let code_point = native_string.code_point_at(position); - string_iterator.next_index += code_point.code_unit_count(); + + let code_point = string.code_point_at(position); + let next_index = position + code_point.code_unit_count(); + + object + .downcast_mut::() + .expect("already checked") + .next_index = next_index; + let result_string = crate::builtins::string::String::substring( - &string_iterator.string.clone().into(), - &[position.into(), string_iterator.next_index.into()], + &string.into(), + &[position.into(), next_index.into()], context, )?; Ok(create_iter_result_object(result_string, false, context)) diff --git a/core/engine/src/builtins/uri/mod.rs b/core/engine/src/builtins/uri/mod.rs index acf15c7d7fb..b55e351cd3d 100644 --- a/core/engine/src/builtins/uri/mod.rs +++ b/core/engine/src/builtins/uri/mod.rs @@ -47,13 +47,6 @@ pub struct UriFunctions { encode_uri_component: JsFunction, } -impl Default for UriFunctions { - fn default() -> Self { - // SAFETY: The global mutation context is used as a fallback during the context threading migration. - Self::uninit_in(&boa_gc::MutationContext::global()) - } -} - impl UriFunctions { pub(crate) fn uninit_in(mc: &boa_gc::MutationContext<'static, '_>) -> Self { Self { diff --git a/core/engine/src/bytecompiler/class.rs b/core/engine/src/bytecompiler/class.rs index 25f8563b08c..f48d0862deb 100644 --- a/core/engine/src/bytecompiler/class.rs +++ b/core/engine/src/bytecompiler/class.rs @@ -157,7 +157,7 @@ impl ByteCompiler<'_> { class.super_ref.is_some(), ); - let code = Gc::new(self.mc.0, compiler.finish()); + let code = boa_gc::allocate_rooted(self.mc.0, compiler.finish()); let index = self.push_function_to_constants(code); let class_register = self.register_allocator.alloc(); @@ -442,7 +442,7 @@ impl ByteCompiler<'_> { field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; - let code = Gc::new(self.mc.0, field_compiler.finish()); + let code = boa_gc::allocate_rooted(self.mc.0, field_compiler.finish()); let index = self.push_function_to_constants(code); let dst = self.register_allocator.alloc(); @@ -489,7 +489,7 @@ impl ByteCompiler<'_> { field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; - let code = Gc::new(self.mc.0, field_compiler.finish()); + let code = boa_gc::allocate_rooted(self.mc.0, field_compiler.finish()); let index = self.push_function_to_constants(code); let dst = self.register_allocator.alloc(); self.emit_get_function(&dst, index); @@ -546,7 +546,7 @@ impl ByteCompiler<'_> { field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; let code = field_compiler.finish(); - let code = Gc::new(self.mc.0, code); + let code = boa_gc::allocate_rooted(self.mc.0, code); static_elements.push(StaticElement::StaticField { code, @@ -591,7 +591,7 @@ impl ByteCompiler<'_> { field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; let code = field_compiler.finish(); - let code = Gc::new(self.mc.0, code); + let code = boa_gc::allocate_rooted(self.mc.0, code); static_elements.push(StaticElement::StaticField { code, @@ -635,7 +635,7 @@ impl ByteCompiler<'_> { ); } - let code = Gc::new(self.mc.0, compiler.finish()); + let code = boa_gc::allocate_rooted(self.mc.0, compiler.finish()); static_elements.push(StaticElement::StaticBlock(code)); } } diff --git a/core/engine/src/bytecompiler/function.rs b/core/engine/src/bytecompiler/function.rs index 371b8ab53fc..97321284d34 100644 --- a/core/engine/src/bytecompiler/function.rs +++ b/core/engine/src/bytecompiler/function.rs @@ -229,6 +229,6 @@ impl FunctionCompiler { let code = compiler.finish(); - Gc::new(mc, code) + boa_gc::allocate_rooted(mc, code) } } diff --git a/core/engine/src/bytecompiler/mod.rs b/core/engine/src/bytecompiler/mod.rs index b47bfe36291..672f32fd481 100644 --- a/core/engine/src/bytecompiler/mod.rs +++ b/core/engine/src/bytecompiler/mod.rs @@ -492,7 +492,7 @@ impl<'a> BorrowMut> for SourcePositionGuard<'_, 'a> { #[derive(Clone, Copy)] pub(crate) struct McWrapper<'ctx>(pub(crate) &'ctx boa_gc::MutationContext<'static, 'static>); -impl std::fmt::Debug for McWrapper<'_> { +impl<'ctx> std::fmt::Debug for McWrapper<'ctx> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_tuple("MutationContext").finish() } @@ -565,7 +565,7 @@ pub struct ByteCompiler<'ctx> { pub(crate) emitted_mapped_arguments_object_opcode: bool, pub(crate) interner: &'ctx mut Interner, - /// The `MutationContext` for GC allocations. + /// The MutationContext for GC allocations. pub(crate) mc: McWrapper<'ctx>, spanned_source_text: SpannedSourceText, diff --git a/core/engine/src/context/intrinsics.rs b/core/engine/src/context/intrinsics.rs index 0c967d67af3..7a182e34317 100644 --- a/core/engine/src/context/intrinsics.rs +++ b/core/engine/src/context/intrinsics.rs @@ -103,6 +103,8 @@ impl StandardConstructor { } } + /// Build a constructor with a defined prototype. + /// Return the prototype of the constructor object. /// /// This is the same as `Object.prototype`, `Array.prototype`, etc. diff --git a/core/engine/src/context/mod.rs b/core/engine/src/context/mod.rs index 2074930fe13..94ba0486d88 100644 --- a/core/engine/src/context/mod.rs +++ b/core/engine/src/context/mod.rs @@ -107,7 +107,9 @@ pub struct Context { pub(crate) kept_alive: Vec, - pub(crate) gc: boa_gc::GcContext, + pub gc: boa_gc::GcContext, + #[cfg(feature = "oscars_backend")] + global_scope: boa_gc::HandleScope, can_block: bool, @@ -1229,13 +1231,15 @@ impl ContextBuilder { } let gc = boa_gc::GcContext::new(); + #[cfg(feature = "oscars_backend")] + let global_scope = boa_gc::HandleScope::enter(); let mc = gc.gc_collector(); - let root_shape = RootShape::new(mc); + let root_shape = RootShape::new(&mc); let host_hooks = self.host_hooks.unwrap_or(Rc::new(DefaultHooks)); let clock = self.clock.unwrap_or_else(|| Rc::new(StdClock::new())); - let realm = Realm::create(host_hooks.as_ref(), &root_shape, mc)?; - let vm = Vm::new(realm, mc); + let realm = Realm::create(host_hooks.as_ref(), &root_shape, &mc)?; + let vm = Vm::new(realm, &mc); let module_loader: Rc = if let Some(loader) = self.module_loader { loader @@ -1285,6 +1289,8 @@ impl ContextBuilder { root_shape, parser_identifier: 0, gc, + #[cfg(feature = "oscars_backend")] + global_scope, can_block: self.can_block, data: HostDefined::default(), }; diff --git a/core/engine/src/environments/runtime/mod.rs b/core/engine/src/environments/runtime/mod.rs index 41f8b795672..ad4c7be8e1f 100644 --- a/core/engine/src/environments/runtime/mod.rs +++ b/core/engine/src/environments/runtime/mod.rs @@ -227,8 +227,8 @@ impl EnvironmentStack { let index = self.depth; self.push_env( - Environment::Declarative(Gc::new( - gc, + Environment::Declarative(boa_gc::allocate_rooted( + &gc, DeclarativeEnvironment::new( DeclarativeEnvironmentKind::Lexical(LexicalEnvironment::new(bindings_count)), poisoned, @@ -254,7 +254,7 @@ impl EnvironmentStack { let (poisoned, with) = self.compute_poisoned_with(global); self.push_env( - Environment::Declarative(Gc::new( + Environment::Declarative(boa_gc::allocate_rooted( gc, DeclarativeEnvironment::new( DeclarativeEnvironmentKind::Function(FunctionEnvironment::new( @@ -274,7 +274,7 @@ impl EnvironmentStack { pub(crate) fn push_module(&mut self, scope: Scope, gc: &boa_gc::MutationContext<'static, '_>) { let num_bindings = scope.num_bindings_non_local(); self.push_env( - Environment::Declarative(Gc::new( + Environment::Declarative(boa_gc::allocate_rooted( gc, DeclarativeEnvironment::new( DeclarativeEnvironmentKind::Module(ModuleEnvironment::new(num_bindings, scope)), @@ -430,7 +430,7 @@ impl EnvironmentStack { /// Push an environment onto the chain. fn push_env(&mut self, env: Environment, mc: &boa_gc::MutationContext<'static, '_>) { - self.tip = Some(Gc::new( + self.tip = Some(boa_gc::allocate_rooted( mc, EnvironmentNode { env, diff --git a/core/engine/src/interop/into_js_function_impls.rs b/core/engine/src/interop/into_js_function_impls.rs index 9a4cb19195f..51954f837f2 100644 --- a/core/engine/src/interop/into_js_function_impls.rs +++ b/core/engine/src/interop/into_js_function_impls.rs @@ -51,7 +51,7 @@ macro_rules! impl_into_js_function { unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { let s = RefCell::new(self); unsafe { - NativeFunction::from_closure(move |this, args, ctx| { + NativeFunction::from_closure(_context.gc_collector(), move |this, args, ctx| { let rest = args; $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; @@ -77,7 +77,7 @@ macro_rules! impl_into_js_function { unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { let s = RefCell::new(self); unsafe { - NativeFunction::from_closure(move |this, args, ctx| { + NativeFunction::from_closure(_context.gc_collector(), move |this, args, ctx| { let rest = args; $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; @@ -103,7 +103,7 @@ macro_rules! impl_into_js_function { unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { let s = RefCell::new(self); unsafe { - NativeFunction::from_closure(move |this, args, ctx| { + NativeFunction::from_closure(_context.gc_collector(), move |this, args, ctx| { let rest = args; $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; @@ -125,7 +125,7 @@ macro_rules! impl_into_js_function { unsafe fn into_js_function_unsafe(self, _context: &mut Context) -> NativeFunction { let s = RefCell::new(self); unsafe { - NativeFunction::from_closure(move |this, args, ctx| { + NativeFunction::from_closure(_context.gc_collector(), move |this, args, ctx| { let rest = args; $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; @@ -145,9 +145,9 @@ macro_rules! impl_into_js_function { T: Fn($($t,)*) -> R + 'static + Copy, { #[allow(unused_variables)] - fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { + fn into_js_function_copied(self, context: &mut Context) -> NativeFunction { let s = self; - NativeFunction::from_copy_closure(move |this, args, ctx| { + NativeFunction::from_copy_closure(context.gc_collector(), move |this, args, ctx| { let rest = args; $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; @@ -165,9 +165,9 @@ macro_rules! impl_into_js_function { T: Fn($($t,)* JsRest<'_>) -> R + 'static + Copy, { #[allow(unused_variables)] - fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { + fn into_js_function_copied(self, context: &mut Context) -> NativeFunction { let s = self; - NativeFunction::from_copy_closure(move |this, args, ctx| { + NativeFunction::from_copy_closure(context.gc_collector(), move |this, args, ctx| { let rest = args; $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; @@ -185,9 +185,9 @@ macro_rules! impl_into_js_function { T: Fn($($t,)* &mut Context) -> R + 'static + Copy, { #[allow(unused_variables)] - fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { + fn into_js_function_copied(self, context: &mut Context) -> NativeFunction { let s = self; - NativeFunction::from_copy_closure(move |this, args, ctx| { + NativeFunction::from_copy_closure(context.gc_collector(), move |this, args, ctx| { let rest = args; $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; @@ -205,9 +205,9 @@ macro_rules! impl_into_js_function { T: Fn($($t,)* JsRest<'_>, &mut Context) -> R + 'static + Copy, { #[allow(unused_variables)] - fn into_js_function_copied(self, _context: &mut Context) -> NativeFunction { + fn into_js_function_copied(self, context: &mut Context) -> NativeFunction { let s = self; - NativeFunction::from_copy_closure(move |this, args, ctx| { + NativeFunction::from_copy_closure(context.gc_collector(), move |this, args, ctx| { let rest = args; $( let ($id, rest) = $t::try_from_js_argument(this, rest, ctx)?; diff --git a/core/engine/src/module/mod.rs b/core/engine/src/module/mod.rs index 01c9d8f7a45..e1cb1f29dfa 100644 --- a/core/engine/src/module/mod.rs +++ b/core/engine/src/module/mod.rs @@ -330,13 +330,16 @@ impl Module { pub fn from_value_as_default(value: JsValue, context: &mut Context) -> Self { Module::synthetic( &[js_string!("default")], - SyntheticModuleInitializer::from_copy_closure_with_captures( - move |m, value, _ctx| { - m.set_export(&js_string!("default"), value.clone())?; - Ok(()) - }, - value, - ), + unsafe { + SyntheticModuleInitializer::from_closure_with_captures( + context.gc_collector(), + move |m, value, _ctx| { + m.set_export(&js_string!("default"), value.clone())?; + Ok(()) + }, + value, + ) + }, None, None, context, @@ -652,6 +655,7 @@ impl Module { .then( Some( NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, _, module, context| { module.link(context)?; Ok(JsValue::undefined()) @@ -667,6 +671,7 @@ impl Module { .then( Some( NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, _, module, context| Ok(module.evaluate(context)?.into()), self.clone(), ) @@ -782,17 +787,20 @@ impl + Clone> IntoJsModule fo Module::synthetic( exports.as_slice(), unsafe { - SyntheticModuleInitializer::from_closure(move |module, context| { - for (name, f) in names.iter().zip(fns.iter()) { - module.set_export( - name, - f.clone() - .to_js_function(context.realm(), context.gc_collector()) - .into(), - )?; - } - Ok(()) - }) + SyntheticModuleInitializer::from_closure( + context.gc_collector(), + move |module, context| { + for (name, f) in names.iter().zip(fns.iter()) { + module.set_export( + name, + f.clone() + .to_js_function(context.realm(), context.gc_collector()) + .into(), + )?; + } + Ok(()) + }, + ) }, None, None, diff --git a/core/engine/src/module/source.rs b/core/engine/src/module/source.rs index 956519f4faf..25127a48d49 100644 --- a/core/engine/src/module/source.rs +++ b/core/engine/src/module/source.rs @@ -1479,6 +1479,7 @@ impl SourceTextModule { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, _, module, context| { // a. Perform AsyncModuleExecutionFulfilled(module). async_module_execution_fulfilled(module, context)?; @@ -1496,6 +1497,7 @@ impl SourceTextModule { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, module, context| { let error = JsError::from_opaque(args.get_or_undefined(0).clone()); // a. Perform AsyncModuleExecutionRejected(module, error). @@ -1657,7 +1659,7 @@ impl SourceTextModule { self.code.has_tla, false, context.interner_mut(), - mc, + &mc, false, spanned_source_text, self.code.path.clone().into(), @@ -1838,8 +1840,7 @@ impl SourceTextModule { // 8. Let moduleContext be a new ECMAScript code execution context. let mut envs = EnvironmentStack::new(); - // SAFETY: TODO - Add safety comment for this block - envs.push_module(source.scope().clone(), context.gc_collector()); + envs.push_module(source.scope().clone(), unsafe { context.gc_collector() }); drop(status); // 9. Set the Function of moduleContext to null. diff --git a/core/engine/src/module/synthetic.rs b/core/engine/src/module/synthetic.rs index ca4dea2116e..201a004086a 100644 --- a/core/engine/src/module/synthetic.rs +++ b/core/engine/src/module/synthetic.rs @@ -65,22 +65,26 @@ impl std::fmt::Debug for SyntheticModuleInitializer { impl SyntheticModuleInitializer { /// Creates a `SyntheticModuleInitializer` from a [`Copy`] closure. - pub fn from_copy_closure(closure: F) -> Self + pub fn from_copy_closure(mc: &boa_gc::MutationContext<'_, '_>, closure: F) -> Self where F: Fn(&SyntheticModule, &mut Context) -> JsResult<()> + Copy + 'static, { // SAFETY: The `Copy` bound ensures there are no traceable types inside the closure. - unsafe { Self::from_closure(closure) } + unsafe { Self::from_closure(mc, closure) } } /// Creates a `SyntheticModuleInitializer` from a [`Copy`] closure and a list of traceable captures. - pub fn from_copy_closure_with_captures(closure: F, captures: T) -> Self + pub fn from_copy_closure_with_captures( + mc: &boa_gc::MutationContext<'_, '_>, + closure: F, + captures: T, + ) -> Self where F: Fn(&SyntheticModule, &T, &mut Context) -> JsResult<()> + Copy + 'static, T: Trace + 'static, { // SAFETY: The `Copy` bound ensures there are no traceable types inside the closure. - unsafe { Self::from_closure_with_captures(closure, captures) } + unsafe { Self::from_closure_with_captures(mc, closure, captures) } } /// Creates a new `SyntheticModuleInitializer` from a closure. @@ -91,13 +95,14 @@ impl SyntheticModuleInitializer { /// collector could cause an use after free, memory corruption or other kinds of **Undefined /// Behaviour**. See for a technical explanation /// on why that is the case. - pub unsafe fn from_closure(closure: F) -> Self + pub unsafe fn from_closure(mc: &boa_gc::MutationContext<'_, '_>, closure: F) -> Self where F: Fn(&SyntheticModule, &mut Context) -> JsResult<()> + 'static, { // SAFETY: The caller must ensure the invariants of the closure hold. unsafe { Self::from_closure_with_captures( + mc, move |module, (), context| closure(module, context), (), ) @@ -112,16 +117,19 @@ impl SyntheticModuleInitializer { /// collector could cause an use after free, memory corruption or other kinds of **Undefined /// Behaviour**. See for a technical explanation /// on why that is the case. - pub unsafe fn from_closure_with_captures(closure: F, captures: T) -> Self + pub unsafe fn from_closure_with_captures( + mc: &boa_gc::MutationContext<'_, '_>, + closure: F, + captures: T, + ) -> Self where F: Fn(&SyntheticModule, &T, &mut Context) -> JsResult<()> + 'static, T: Trace + 'static, { // Hopefully, this unsafe operation will be replaced by the `CoerceUnsized` API in the // future: https://github.com/rust-lang/rust/issues/18598 - let ptr = Gc::into_raw(Gc::new( - // SAFETY: The global mutation context is used as a fallback during the context threading migration. - &boa_gc::MutationContext::global(), + let ptr = Gc::into_raw(boa_gc::allocate_rooted( + mc, Callback { f: closure, captures, @@ -322,7 +330,7 @@ impl SyntheticModule { false, false, context.interner_mut(), - mc, + &mc, false, // A synthetic module does not contain `SourceText` SpannedSourceText::new_empty(), @@ -345,8 +353,7 @@ impl SyntheticModule { let cb = context.alloc(finished); let mut envs = EnvironmentStack::new(); - // SAFETY: The global mutation context is used as a fallback during the context threading migration. - envs.push_module(module_scope, &boa_gc::MutationContext::global()); + envs.push_module(module_scope, context.gc_collector()); for locator in exports { // b. Perform ! env.InitializeBinding(exportName, undefined). diff --git a/core/engine/src/native_function/continuation.rs b/core/engine/src/native_function/continuation.rs index de4e00e3c52..51ded311742 100644 --- a/core/engine/src/native_function/continuation.rs +++ b/core/engine/src/native_function/continuation.rs @@ -83,13 +83,17 @@ impl std::fmt::Debug for NativeCoroutine { impl NativeCoroutine { /// Creates a `NativeCoroutine` from a `Copy` closure and a list of traceable captures. - pub(crate) fn from_copy_closure_with_captures(closure: F, captures: T) -> Self + pub(crate) fn from_copy_closure_with_captures( + mc: &boa_gc::MutationContext<'_, '_>, + closure: F, + captures: T, + ) -> Self where F: Fn(CompletionRecord, &T, &mut Context) -> CoroutineState + Copy + 'static, T: Trace + 'static, { // SAFETY: The `Copy` bound ensures there are no traceable types inside the closure. - unsafe { Self::from_closure_with_captures(closure, captures) } + unsafe { Self::from_closure_with_captures(mc, closure, captures) } } /// Create a new `NativeCoroutine` from a closure and a list of traceable captures. @@ -100,15 +104,19 @@ impl NativeCoroutine { /// collector could cause an use after free, memory corruption or other kinds of **Undefined /// Behaviour**. See for a technical explanation /// on why that is the case. - pub(crate) unsafe fn from_closure_with_captures(closure: F, captures: T) -> Self + pub(crate) unsafe fn from_closure_with_captures( + mc: &boa_gc::MutationContext<'_, '_>, + closure: F, + captures: T, + ) -> Self where F: Fn(CompletionRecord, &T, &mut Context) -> CoroutineState + 'static, T: Trace + 'static, { // Hopefully, this unsafe operation will be replaced by the `CoerceUnsized` API in the // future: https://github.com/rust-lang/rust/issues/18598 - let ptr = Gc::into_raw(Gc::new( - &boa_gc::MutationContext::global(), + let ptr = Gc::into_raw(boa_gc::allocate_rooted( + mc, Coroutine { f: closure, captures, diff --git a/core/engine/src/native_function/mod.rs b/core/engine/src/native_function/mod.rs index d1adcce5169..4d01ec0846a 100644 --- a/core/engine/src/native_function/mod.rs +++ b/core/engine/src/native_function/mod.rs @@ -193,14 +193,15 @@ impl NativeFunction { /// let value = arg.to_u32(&mut context.borrow_mut())?; /// Ok(JsValue::from(value * 2)) /// } - /// NativeFunction::from_async_fn(test); + /// let mut context = Context::default(); + /// NativeFunction::from_async_fn(context.gc_collector(), test); /// ``` - pub fn from_async_fn(f: F) -> Self + pub fn from_async_fn(mc: &boa_gc::MutationContext<'_, '_>, f: F) -> Self where F: AsyncFn(&JsValue, &[JsValue], &RefCell<&mut Context>) -> JsResult + 'static, F: Copy, { - Self::from_copy_closure(move |this, args, context| { + Self::from_copy_closure(mc, move |this, args, context| { let (promise, resolvers) = JsPromise::new_pending(context); let this = this.clone(); let args = args.to_vec(); @@ -226,22 +227,26 @@ impl NativeFunction { } /// Creates a `NativeFunction` from a `Copy` closure. - pub fn from_copy_closure(closure: F) -> Self + pub fn from_copy_closure(mc: &boa_gc::MutationContext<'_, '_>, closure: F) -> Self where F: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult + Copy + 'static, { // SAFETY: The `Copy` bound ensures there are no traceable types inside the closure. - unsafe { Self::from_closure(closure) } + unsafe { Self::from_closure(mc, closure) } } /// Creates a `NativeFunction` from a `Copy` closure and a list of traceable captures. - pub fn from_copy_closure_with_captures(closure: F, captures: T) -> Self + pub fn from_copy_closure_with_captures( + mc: &boa_gc::MutationContext<'_, '_>, + closure: F, + captures: T, + ) -> Self where F: Fn(&JsValue, &[JsValue], &T, &mut Context) -> JsResult + Copy + 'static, T: Trace + 'static, { // SAFETY: The `Copy` bound ensures there are no traceable types inside the closure. - unsafe { Self::from_closure_with_captures(closure, captures) } + unsafe { Self::from_closure_with_captures(mc, closure, captures) } } /// Creates a new `NativeFunction` from a closure. @@ -252,13 +257,14 @@ impl NativeFunction { /// collector could cause an use after free, memory corruption or other kinds of **Undefined /// Behaviour**. See for a technical explanation /// on why that is the case. - pub unsafe fn from_closure(closure: F) -> Self + pub unsafe fn from_closure(mc: &boa_gc::MutationContext<'_, '_>, closure: F) -> Self where F: Fn(&JsValue, &[JsValue], &mut Context) -> JsResult + 'static, { // SAFETY: The caller must ensure the invariants of the closure hold. unsafe { Self::from_closure_with_captures( + mc, move |this, args, (), context| closure(this, args, context), (), ) @@ -273,15 +279,19 @@ impl NativeFunction { /// collector could cause an use after free, memory corruption or other kinds of **Undefined /// Behaviour**. See for a technical explanation /// on why that is the case. - pub unsafe fn from_closure_with_captures(closure: F, captures: T) -> Self + pub unsafe fn from_closure_with_captures( + mc: &boa_gc::MutationContext<'_, '_>, + closure: F, + captures: T, + ) -> Self where F: Fn(&JsValue, &[JsValue], &T, &mut Context) -> JsResult + 'static, T: Trace + 'static, { // Hopefully, this unsafe operation will be replaced by the `CoerceUnsized` API in the // future: https://github.com/rust-lang/rust/issues/18598 - let ptr = Gc::into_raw(Gc::new( - &boa_gc::MutationContext::global(), + let ptr = Gc::into_raw(boa_gc::allocate_rooted( + mc, Closure { f: closure, captures, diff --git a/core/engine/src/object/builtins/jspromise.rs b/core/engine/src/object/builtins/jspromise.rs index fef2ce9811f..5c3957ef189 100644 --- a/core/engine/src/object/builtins/jspromise.rs +++ b/core/engine/src/object/builtins/jspromise.rs @@ -1105,6 +1105,7 @@ impl JsPromise { let state = state.clone(); NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), move |_, args, state, _| { finish(state, Ok(args.get_or_undefined(0).clone())); Ok(JsValue::undefined()) @@ -1117,6 +1118,7 @@ impl JsPromise { let state = state.clone(); NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), move |_, args, state, _| { let err = JsError::from_opaque(args.get_or_undefined(0).clone()); finish(state, Err(err)); @@ -1245,6 +1247,7 @@ impl JsPromise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, captures, context| { // a. Let prevContext be the running execution context. // b. Suspend prevContext. @@ -1310,6 +1313,7 @@ impl JsPromise { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, captures, context| { // a. Let prevContext be the running execution context. // b. Suspend prevContext. diff --git a/core/engine/src/object/builtins/jstypedarray.rs b/core/engine/src/object/builtins/jstypedarray.rs index 817391d4ca0..4cc23bdfb69 100644 --- a/core/engine/src/object/builtins/jstypedarray.rs +++ b/core/engine/src/object/builtins/jstypedarray.rs @@ -687,6 +687,7 @@ impl JsTypedArray { /// context.realm(), /// context.gc_collector(), /// NativeFunction::from_copy_closure_with_captures( + /// context.gc_collector(), /// |_, args, captures, inner_context| { /// let element = args /// .first() diff --git a/core/engine/src/object/internal_methods/mod.rs b/core/engine/src/object/internal_methods/mod.rs index 80d264ffffc..2f0cba2bd25 100644 --- a/core/engine/src/object/internal_methods/mod.rs +++ b/core/engine/src/object/internal_methods/mod.rs @@ -1010,18 +1010,11 @@ pub(crate) fn is_compatible_property_descriptor( extensible: bool, desc: PropertyDescriptor, current: Option, + mc: &boa_gc::MutationContext<'_, '_>, ) -> bool { // 1. Return ValidateAndApplyPropertyDescriptor(undefined, undefined, Extensible, Desc, Current). let mut dummy_slot = Slot::new(); - let dummy_mc = boa_gc::MutationContext::global(); - validate_and_apply_property_descriptor( - None, - extensible, - desc, - current, - &mut dummy_slot, - &dummy_mc, - ) + validate_and_apply_property_descriptor(None, extensible, desc, current, &mut dummy_slot, mc) } /// Abstract operation `ValidateAndApplyPropertyDescriptor` diff --git a/core/engine/src/object/internal_methods/string.rs b/core/engine/src/object/internal_methods/string.rs index 11a60219bd3..57f7087a103 100644 --- a/core/engine/src/object/internal_methods/string.rs +++ b/core/engine/src/object/internal_methods/string.rs @@ -68,6 +68,7 @@ pub(crate) fn string_exotic_define_own_property( extensible, desc, Some(string_desc), + context.gc_collector(), )) } else { // 4. Return ! OrdinaryDefineOwnProperty(S, P, Desc). diff --git a/core/engine/src/object/jsobject.rs b/core/engine/src/object/jsobject.rs index 48d16a0265f..09939259424 100644 --- a/core/engine/src/object/jsobject.rs +++ b/core/engine/src/object/jsobject.rs @@ -70,13 +70,70 @@ impl Clone for JsObject { // implementation of `Debug` for `JsObject` could easily cause stack overflows, // so we have to force our users to debug the `JsObject` instead. #[allow(missing_debug_implementations)] -#[derive(Trace, Finalize)] +#[cfg_attr(not(feature = "oscars_backend"), derive(Trace, Finalize))] +/// Note: We must use `repr(C)` to ensure that `VTableObject` and +/// `VTableObject` have the exact same prefix layout, allowing safe pointer casting. +#[repr(C)] pub(crate) struct VTableObject { - #[unsafe_ignore_trace] + #[cfg_attr(not(feature = "oscars_backend"), unsafe_ignore_trace)] vtable: &'static InternalObjectMethods, + #[cfg(feature = "oscars_backend")] + trace_fn: fn(&VTableObject, &mut boa_gc::Tracer<'_>), + #[cfg(feature = "oscars_backend")] + finalize_fn: fn(&VTableObject), object: GcRefCell>, } +#[cfg(feature = "oscars_backend")] +unsafe impl Trace for VTableObject { + #[inline] + unsafe fn trace(&self, tracer: &mut boa_gc::Tracer<'_>) { + (self.trace_fn)( + unsafe { &*(self as *const _ as *const VTableObject) }, + tracer, + ); + } +} + +#[cfg(feature = "oscars_backend")] +impl Finalize for VTableObject { + fn finalize(&self) { + (self.finalize_fn)(unsafe { + &*(self as *const _ as *const VTableObject) + }); + } +} + +impl VTableObject { + pub(crate) fn new(object: Object, vtable: &'static InternalObjectMethods) -> Self { + #[cfg(feature = "oscars_backend")] + fn trace_fn( + this: &VTableObject, + tracer: &mut boa_gc::Tracer<'_>, + ) { + let this = unsafe { &*(this as *const _ as *const VTableObject) }; + unsafe { + boa_gc::Trace::trace(&this.object, tracer); + } + } + + #[cfg(feature = "oscars_backend")] + fn finalize_fn(this: &VTableObject) { + let this = unsafe { &*(this as *const _ as *const VTableObject) }; + boa_gc::Finalize::finalize(&this.object); + } + + Self { + object: GcRefCell::new(object), + vtable, + #[cfg(feature = "oscars_backend")] + trace_fn: trace_fn::, + #[cfg(feature = "oscars_backend")] + finalize_fn: finalize_fn::, + } + } +} + impl JsObject { /// Converts the `JsObject` into a raw pointer to its inner `GcBox`. #[cfg(not(feature = "jsvalue-enum"))] @@ -122,13 +179,7 @@ impl JsObject { object: Object, vtable: &'static InternalObjectMethods, ) -> Self { - let inner = Gc::new( - mc, - VTableObject { - object: GcRefCell::new(object), - vtable, - }, - ); + let inner = boa_gc::allocate_rooted(mc, VTableObject::new(object, vtable)); JsObject { inner }.upcast() } @@ -177,17 +228,17 @@ impl JsObject { data: T, ) -> Self { let internal_methods = data.internal_methods(); - let inner = Gc::new( + let inner = boa_gc::allocate_rooted( mc, - VTableObject { - object: GcRefCell::new(Object { + VTableObject::new( + Object { data: ObjectData::new(data), properties: PropertyMap::from_prototype_unique_shape(mc, prototype.into()), extensible: true, private_elements: ThinVec::new(), - }), - vtable: internal_methods, - }, + }, + internal_methods, + ), ); JsObject { inner }.upcast() @@ -201,10 +252,10 @@ impl JsObject { data: T, ) -> JsObject { let internal_methods = data.internal_methods(); - let inner = Gc::new( + let inner = boa_gc::allocate_rooted( mc, - VTableObject { - object: GcRefCell::new(Object { + VTableObject::new( + Object { data: ObjectData::new(data), properties: PropertyMap::from_prototype_with_shared_shape( mc, @@ -213,9 +264,9 @@ impl JsObject { ), extensible: true, private_elements: ThinVec::new(), - }), - vtable: internal_methods, - }, + }, + internal_methods, + ), ); JsObject { inner } @@ -1032,6 +1083,8 @@ impl JsObject { } pub(crate) fn from_inner(inner: Gc<'static, VTableObject>) -> Self { + #[cfg(feature = "oscars_backend")] + let _root = boa_gc::Local::new(inner.clone()); Self { inner } } @@ -1051,10 +1104,10 @@ impl JsObject { data: T, ) -> Self { let internal_methods = data.internal_methods(); - let inner = Gc::new( + let inner = boa_gc::allocate_rooted( mc, - VTableObject { - object: GcRefCell::new(Object { + VTableObject::new( + Object { data: ObjectData::new(data), properties: PropertyMap::from_prototype_with_shared_shape( mc, @@ -1063,9 +1116,9 @@ impl JsObject { ), extensible: true, private_elements: ThinVec::new(), - }), - vtable: internal_methods, - }, + }, + internal_methods, + ), ); Self { inner } @@ -1101,17 +1154,17 @@ impl JsObject { data: T, ) -> Self { let internal_methods = data.internal_methods(); - let inner = Gc::new( + let inner = boa_gc::allocate_rooted( mc, - VTableObject { - object: GcRefCell::new(Object { + VTableObject::new( + Object { data: ObjectData::new(data), properties: PropertyMap::from_prototype_unique_shape(mc, prototype.into()), extensible: true, private_elements: ThinVec::new(), - }), - vtable: internal_methods, - }, + }, + internal_methods, + ), ); Self { inner } diff --git a/core/engine/src/object/mod.rs b/core/engine/src/object/mod.rs index a61869f7f7c..5a1d0c49591 100644 --- a/core/engine/src/object/mod.rs +++ b/core/engine/src/object/mod.rs @@ -412,7 +412,6 @@ where } /// Builder for creating native function objects -#[expect(missing_debug_implementations)] pub struct FunctionObjectBuilder<'realm> { realm: &'realm Realm, mc: &'realm boa_gc::MutationContext<'static, 'realm>, diff --git a/core/engine/src/object/property_map.rs b/core/engine/src/object/property_map.rs index 4cabc278fdc..983f5b5622f 100644 --- a/core/engine/src/object/property_map.rs +++ b/core/engine/src/object/property_map.rs @@ -593,6 +593,7 @@ impl PropertyMap { } /// Insert the given property descriptor with the given key [`PropertyMap`]. + pub fn insert( &mut self, mc: &boa_gc::MutationContext<'static, '_>, diff --git a/core/engine/src/object/shape/mod.rs b/core/engine/src/object/shape/mod.rs index a26f75f8f48..68ed751628b 100644 --- a/core/engine/src/object/shape/mod.rs +++ b/core/engine/src/object/shape/mod.rs @@ -123,6 +123,10 @@ impl Shape { } } + /// Create an insert property transitions returning the new transitioned [`Shape`]. + /// + /// NOTE: This assumes that there is no property with the given key! + /// Create a change attribute property transitions returning [`ChangeTransition`] containing the new [`Shape`] /// and actions to be performed, using the given context. /// @@ -150,6 +154,11 @@ impl Shape { } } + /// Create a change attribute property transitions returning [`ChangeTransition`] containing the new [`Shape`] + /// and actions to be performed + /// + /// NOTE: This assumes that there already is a property with the given key! + /// Remove a property from the [`Shape`] returning the new transitioned [`Shape`] using the given context. /// /// NOTE: This assumes that there already is a property with the given key! @@ -170,6 +179,10 @@ impl Shape { } } + /// Remove a property from the [`Shape`] returning the new transitioned [`Shape`]. + /// + /// NOTE: This assumes that there already is a property with the given key! + /// Create a prototype transition returning the new transitioned [`Shape`] using the given context. pub(crate) fn change_prototype_transition( &self, @@ -189,7 +202,7 @@ impl Shape { } /// Create a prototype transition returning the new transitioned [`Shape`]. - /// + /// Get the [`JsPrototype`] of the [`Shape`]. #[must_use] pub fn prototype(&self) -> JsPrototype { diff --git a/core/engine/src/object/shape/root_shape.rs b/core/engine/src/object/shape/root_shape.rs index bcf8e308001..fb26c29b7cf 100644 --- a/core/engine/src/object/shape/root_shape.rs +++ b/core/engine/src/object/shape/root_shape.rs @@ -13,7 +13,6 @@ pub struct RootShape { impl RootShape { /// Create a new root shape using the given context. #[inline] - #[must_use] pub fn new(mc: &boa_gc::MutationContext<'static, '_>) -> Self { Self { shape: SharedShape::root(mc), diff --git a/core/engine/src/object/shape/shared_shape/forward_transition.rs b/core/engine/src/object/shape/shared_shape/forward_transition.rs index 126c161b847..ada106bb71a 100644 --- a/core/engine/src/object/shape/shared_shape/forward_transition.rs +++ b/core/engine/src/object/shape/shared_shape/forward_transition.rs @@ -72,6 +72,8 @@ impl ForwardTransition { properties.map.insert(key, WeakGc::new(mc, value)); } + /// Insert a property transition. + /// Insert a prototype transition using the given context. pub(super) fn insert_prototype( &self, @@ -89,6 +91,8 @@ impl ForwardTransition { prototypes.map.insert(key, WeakGc::new(mc, value)); } + /// Insert a prototype transition. + /// Get a property transition, return [`None`] otherwise. #[allow(clippy::cloned_instead_of_copied)] pub(super) fn get_property(&self, key: &TransitionKey) -> Option> { diff --git a/core/engine/src/object/shape/shared_shape/mod.rs b/core/engine/src/object/shape/shared_shape/mod.rs index 1b8bfd8f242..a790b04b502 100644 --- a/core/engine/src/object/shape/shared_shape/mod.rs +++ b/core/engine/src/object/shape/shared_shape/mod.rs @@ -175,10 +175,12 @@ impl SharedShape { /// Create a new [`SharedShape`] using the given context. fn new(mc: &boa_gc::MutationContext<'static, '_>, inner: Inner) -> Self { Self { - inner: Gc::new(mc, inner), + inner: boa_gc::allocate_rooted(mc, inner), } } + /// Create a new [`SharedShape`]. + /// Create a root [`SharedShape`] using the given context. #[must_use] pub(crate) fn root(mc: &boa_gc::MutationContext<'static, '_>) -> Self { @@ -197,6 +199,8 @@ impl SharedShape { ) } + /// Create a root [`SharedShape`]. + /// Create a [`SharedShape`] change prototype transition using the given context. pub(crate) fn change_prototype_transition( &self, @@ -227,6 +231,8 @@ impl SharedShape { new_shape } + /// Create a [`SharedShape`] change prototype transition. + /// Create a [`SharedShape`] insert property transition using the given context. pub(crate) fn insert_property_transition( &self, @@ -264,6 +270,10 @@ impl SharedShape { new_shape } + /// Create a [`SharedShape`] insert property transition. + + /// Create a [`SharedShape`] change prototype transition, returning [`ChangeTransition`]. + /// Create a [`SharedShape`] change prototype transition using the given context, returning [`ChangeTransition`]. pub(crate) fn change_attributes_transition( &self, @@ -457,6 +467,8 @@ impl SharedShape { base } + /// Remove a property from [`SharedShape`], returning the new [`SharedShape`]. + /// Do a property lookup, returns [`None`] if property not found. pub(crate) fn lookup(&self, key: &PropertyKey) -> Option { let property_count = self.property_count(); @@ -518,11 +530,11 @@ impl WeakSharedShape { /// Upgrade returns a [`SharedShape`] pointer for the internal value if the pointer is still live, /// or [`None`] if the value was already garbage collected. + #[allow(dead_code)] pub(crate) fn is_upgradable(&self) -> bool { self.inner.is_upgradable() } - pub(crate) fn new(mc: &boa_gc::MutationContext<'static, '_>, value: &SharedShape) -> Self { WeakSharedShape { inner: WeakGc::new(mc, &value.inner), diff --git a/core/engine/src/object/shape/shared_shape/template.rs b/core/engine/src/object/shape/shared_shape/template.rs index adba604623c..9adb3b28f63 100644 --- a/core/engine/src/object/shape/shared_shape/template.rs +++ b/core/engine/src/object/shape/shared_shape/template.rs @@ -37,6 +37,8 @@ impl ObjectTemplate { Self { shape } } + /// Create and [`ObjectTemplate`] with a prototype. + /// Check if the shape has a specific, prototype. pub(crate) fn has_prototype(&self, prototype: &JsObject) -> bool { self.shape.has_prototype(prototype) @@ -54,6 +56,10 @@ impl ObjectTemplate { self } + /// Set the prototype of the [`ObjectTemplate`]. + /// + /// This assumes that the prototype has not been set yet. + /// Returns the inner shape of the [`ObjectTemplate`]. pub(crate) const fn shape(&self) -> &SharedShape { &self.shape @@ -79,6 +85,11 @@ impl ObjectTemplate { self } + /// Add a data property to the [`ObjectTemplate`]. + /// + /// This assumes that the property with the given key was not previously set + /// and that it's a string or symbol. + /// Add a accessor property to the [`ObjectTemplate`]. /// /// This assumes that the property with the given key was not previously set @@ -121,6 +132,11 @@ impl ObjectTemplate { self } + /// Add a accessor property to the [`ObjectTemplate`]. + /// + /// This assumes that the property with the given key was not previously set + /// and that it's a string or symbol. + /// Create an object from the [`ObjectTemplate`] using the given context. pub(crate) fn create( &self, @@ -130,11 +146,13 @@ impl ObjectTemplate { ) -> JsObject { let internal_methods = data.internal_methods(); - let mut properties = - PropertyMap::new(self.shape.clone().into(), IndexedProperties::default()); + let mut properties = PropertyMap::new( + self.shape.clone().into(), + crate::object::IndexedProperties::default(), + ); properties.storage = storage; - let object = Object { + let mut object = Object { data: ObjectData::new(data), extensible: true, properties, @@ -144,6 +162,10 @@ impl ObjectTemplate { JsObject::from_object_and_vtable(mc, object, internal_methods) } + /// Create an object from the [`ObjectTemplate`] + /// + /// The storage must match the properties provided. + /// Create an object from the [`ObjectTemplate`] /// /// The storage must match the properties provided. It does not apply to diff --git a/core/engine/src/object/shape/shared_shape/tests.rs b/core/engine/src/object/shape/shared_shape/tests.rs index 6c1a2db16a8..d1bbca9f229 100644 --- a/core/engine/src/object/shape/shared_shape/tests.rs +++ b/core/engine/src/object/shape/shared_shape/tests.rs @@ -4,7 +4,7 @@ use super::{SharedShape, TransitionKey}; #[test] fn test_prune_property_on_counter_limit() { - let shape = SharedShape::root(&boa_gc::MutationContext::global()); + let shape = SharedShape::root(&unsafe { boa_gc::MutationContext::global() }); for i in 0..255 { assert_eq!( @@ -13,7 +13,7 @@ fn test_prune_property_on_counter_limit() { ); shape.insert_property_transition( - &boa_gc::MutationContext::global(), + &unsafe { boa_gc::MutationContext::global() }, TransitionKey { property_key: PropertyKey::Symbol(JsSymbol::new(None).unwrap()), attributes: SlotAttributes::all(), @@ -30,7 +30,7 @@ fn test_prune_property_on_counter_limit() { { shape.insert_property_transition( - &boa_gc::MutationContext::global(), + &unsafe { boa_gc::MutationContext::global() }, TransitionKey { property_key: PropertyKey::Symbol(JsSymbol::new(None).unwrap()), attributes: SlotAttributes::all(), @@ -45,7 +45,7 @@ fn test_prune_property_on_counter_limit() { { shape.insert_property_transition( - &boa_gc::MutationContext::global(), + &unsafe { boa_gc::MutationContext::global() }, TransitionKey { property_key: PropertyKey::Symbol(JsSymbol::new(None).unwrap()), attributes: SlotAttributes::all(), @@ -68,7 +68,7 @@ fn test_prune_property_on_counter_limit() { #[test] fn test_prune_prototype_on_counter_limit() { - let shape = SharedShape::root(&boa_gc::MutationContext::global()); + let shape = SharedShape::root(&unsafe { boa_gc::MutationContext::global() }); assert_eq!( shape.forward_transitions().prototype_transitions_count(), @@ -82,7 +82,7 @@ fn test_prune_prototype_on_counter_limit() { ); shape.change_prototype_transition( - &boa_gc::MutationContext::global(), + &unsafe { boa_gc::MutationContext::global() }, Some(JsObject::with_null_proto(&unsafe { boa_gc::MutationContext::global() })), @@ -98,7 +98,7 @@ fn test_prune_prototype_on_counter_limit() { { shape.change_prototype_transition( - &boa_gc::MutationContext::global(), + &unsafe { boa_gc::MutationContext::global() }, Some(JsObject::with_null_proto(&unsafe { boa_gc::MutationContext::global() })), @@ -112,7 +112,7 @@ fn test_prune_prototype_on_counter_limit() { { shape.change_prototype_transition( - &boa_gc::MutationContext::global(), + &unsafe { boa_gc::MutationContext::global() }, Some(JsObject::with_null_proto(&unsafe { boa_gc::MutationContext::global() })), diff --git a/core/engine/src/object/shape/unique_shape.rs b/core/engine/src/object/shape/unique_shape.rs index 77107d1cc5d..33d62b08234 100644 --- a/core/engine/src/object/shape/unique_shape.rs +++ b/core/engine/src/object/shape/unique_shape.rs @@ -41,7 +41,7 @@ impl UniqueShape { property_table: PropertyTableInner, ) -> Self { Self { - inner: Gc::new( + inner: boa_gc::allocate_rooted( mc, Inner { property_table: RefCell::new(property_table), @@ -51,6 +51,8 @@ impl UniqueShape { } } + /// Create a new [`UniqueShape`]. + pub(crate) fn override_internal( &self, property_table: PropertyTableInner, @@ -276,12 +278,13 @@ impl WeakUniqueShape { }) } - /// Checks if `WeakGc` is upgradeable + /// Upgrade returns a [`UniqueShape`] pointer for the internal value if the pointer is still live, + /// or [`None`] if the value was already garbage collected. + #[allow(dead_code)] pub(crate) fn is_upgradable(&self) -> bool { self.inner.is_upgradable() } - pub(crate) fn new(mc: &boa_gc::MutationContext<'static, '_>, value: &UniqueShape) -> Self { WeakUniqueShape { inner: WeakGc::new(mc, &value.inner), diff --git a/core/engine/src/realm.rs b/core/engine/src/realm.rs index 39d1a382b4d..2050c5a9052 100644 --- a/core/engine/src/realm.rs +++ b/core/engine/src/realm.rs @@ -90,11 +90,11 @@ impl Realm { let global_this = hooks .create_global_this(&intrinsics) .unwrap_or_else(|| global_object.clone()); - let environment = Gc::new(mc, DeclarativeEnvironment::global()); + let environment = boa_gc::allocate_rooted(mc, DeclarativeEnvironment::global()); let scope = Scope::new_global(); let realm = Self { - inner: Gc::new( + inner: boa_gc::allocate_rooted( mc, Inner { intrinsics, diff --git a/core/engine/src/script.rs b/core/engine/src/script.rs index 24961aa5533..373129572b9 100644 --- a/core/engine/src/script.rs +++ b/core/engine/src/script.rs @@ -147,10 +147,14 @@ impl Script { false, false, context.interner_mut(), - mc, + &mc, false, spanned_source_text, - self.inner.path.as_deref().map(Path::to_path_buf).into(), + self.inner + .path + .as_deref() + .map(std::path::Path::to_path_buf) + .into(), ); #[cfg(feature = "annex-b")] diff --git a/core/engine/src/value/inner/nan_boxed.rs b/core/engine/src/value/inner/nan_boxed.rs index 7cb0aa97b40..978e637eb46 100644 --- a/core/engine/src/value/inner/nan_boxed.rs +++ b/core/engine/src/value/inner/nan_boxed.rs @@ -1014,7 +1014,10 @@ fn bigint() { #[test] fn object() { - let object = JsObject::with_null_proto(&boa_gc::MutationContext::global()); + #[cfg(feature = "oscars_backend")] + let _scope = boa_gc::HandleScope::enter(); + + let object = JsObject::with_null_proto(&unsafe { boa_gc::MutationContext::global() }); let v = NanBoxedValue::object(object.clone()); assert_type!(v is object(object)); } diff --git a/core/engine/src/value/mod.rs b/core/engine/src/value/mod.rs index ea09d5785df..af5351f04fc 100644 --- a/core/engine/src/value/mod.rs +++ b/core/engine/src/value/mod.rs @@ -359,7 +359,7 @@ impl JsValue { /// use boa_engine::{Context, JsValue, NativeFunction}; /// /// let context = &mut Context::default(); - /// let native_fn = NativeFunction::from_copy_closure(|_, _, _| Ok(JsValue::undefined())); + /// let native_fn = NativeFunction::from_copy_closure(context.gc_collector(), |_, _, _| Ok(JsValue::undefined())); /// let js_value = JsValue::from(native_fn.to_js_function(context.realm(), context.gc_collector())); /// assert!(js_value.is_callable()); /// @@ -380,7 +380,7 @@ impl JsValue { /// use boa_engine::{Context, JsValue, NativeFunction}; /// /// let context = &mut Context::default(); - /// let native_fn = NativeFunction::from_copy_closure(|_, _, _| Ok(JsValue::undefined())); + /// let native_fn = NativeFunction::from_copy_closure(context.gc_collector(), |_, _, _| Ok(JsValue::undefined())); /// let js_value = JsValue::from(native_fn.to_js_function(context.realm(), context.gc_collector())); /// assert!(js_value.as_callable().is_some()); /// @@ -402,7 +402,7 @@ impl JsValue { /// use boa_engine::{Context, JsValue, NativeFunction}; /// /// let context = &mut Context::default(); - /// let native_fn = NativeFunction::from_copy_closure(|_, _, _| Ok(JsValue::undefined())); + /// let native_fn = NativeFunction::from_copy_closure(context.gc_collector(), |_, _, _| Ok(JsValue::undefined())); /// let js_value = JsValue::from(native_fn.to_js_function(context.realm(), context.gc_collector())); /// assert!(js_value.as_function().is_some()); /// diff --git a/core/engine/src/value/tests.rs b/core/engine/src/value/tests.rs index 3b6ab94780f..a1c607562bb 100644 --- a/core/engine/src/value/tests.rs +++ b/core/engine/src/value/tests.rs @@ -128,11 +128,20 @@ fn hash_rational() { #[test] fn hash_object() { - let object1 = JsValue::new(JsObject::with_null_proto(&boa_gc::MutationContext::global())); + #[cfg(feature = "oscars_backend")] + let _scope = boa_gc::HandleScope::enter(); + + #[allow(unused_unsafe)] + let object1 = JsValue::new(JsObject::with_null_proto(&unsafe { + boa_gc::MutationContext::global() + })); assert_eq!(object1, object1); assert_eq!(object1, object1.clone()); - let object2 = JsValue::new(JsObject::with_null_proto(&boa_gc::MutationContext::global())); + #[allow(unused_unsafe)] + let object2 = JsValue::new(JsObject::with_null_proto(&unsafe { + boa_gc::MutationContext::global() + })); assert_ne!(object1, object2); assert_eq!(hash_value(&object1), hash_value(&object1.clone())); diff --git a/core/engine/src/vm/mod.rs b/core/engine/src/vm/mod.rs index 08b6b34a91d..7055812233a 100644 --- a/core/engine/src/vm/mod.rs +++ b/core/engine/src/vm/mod.rs @@ -407,7 +407,7 @@ impl Vm { pub(crate) fn new(realm: Realm, mc: &boa_gc::MutationContext<'static, '_>) -> Self { let mut frames = Vec::with_capacity(16); frames.push(CallFrame::new( - Gc::new(mc, CodeBlock::new(JsString::default(), 0, true)), + boa_gc::allocate_rooted(mc, CodeBlock::new(JsString::default(), 0, true)), None, EnvironmentStack::new(), realm, diff --git a/core/engine/src/vm/opcode/await/mod.rs b/core/engine/src/vm/opcode/await/mod.rs index e6f58d6c6d1..b62ca12c262 100644 --- a/core/engine/src/vm/opcode/await/mod.rs +++ b/core/engine/src/vm/opcode/await/mod.rs @@ -63,6 +63,7 @@ impl Await { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, captures, context| { // a. Let prevContext be the running execution context. // b. Suspend prevContext. @@ -103,6 +104,7 @@ impl Await { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_this, args, captures, context| { // a. Let prevContext be the running execution context. // b. Suspend prevContext. diff --git a/core/engine/src/vm/opcode/call/mod.rs b/core/engine/src/vm/opcode/call/mod.rs index dcc48b63d57..aa1b42e3bb9 100644 --- a/core/engine/src/vm/opcode/call/mod.rs +++ b/core/engine/src/vm/opcode/call/mod.rs @@ -451,8 +451,9 @@ async fn load_dyn_import( let mc = context.borrow().gc_collector(); let on_rejected = FunctionObjectBuilder::new( context.borrow().realm(), - mc, + &mc, NativeFunction::from_copy_closure_with_captures( + &mc, |_, args, cap, context| { // a. Perform ! Call(promiseCapability.[[Reject]], undefined, « reason »). cap.reject() @@ -472,8 +473,9 @@ async fn load_dyn_import( let mc = context.borrow().gc_collector(); let link_evaluate = FunctionObjectBuilder::new( context.borrow().realm(), - mc, + &mc, NativeFunction::from_copy_closure_with_captures( + &mc, |_, _, (module, cap, on_rejected), context| { // a. Let link be Completion(module.Link()). // b. If link is an abrupt completion, then @@ -496,6 +498,7 @@ async fn load_dyn_import( context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, _, (module, cap), context| { // i. Let namespace be GetModuleNamespace(module). let namespace = module.namespace(context); diff --git a/core/engine/src/vm/opcode/generator/yield_stm.rs b/core/engine/src/vm/opcode/generator/yield_stm.rs index fc42314416d..d1171cc9369 100644 --- a/core/engine/src/vm/opcode/generator/yield_stm.rs +++ b/core/engine/src/vm/opcode/generator/yield_stm.rs @@ -91,15 +91,18 @@ impl AsyncGeneratorYield { return context.handle_error(err); } - let mut r#gen = async_generator_object.borrow_mut(); - - // 10. Let queue be generator.[[AsyncGeneratorQueue]]. - // 11. If queue is not empty, then - // a. NOTE: Execution continues without suspending the generator. - // b. Let toYield be the first element of queue. - if let Some(next) = r#gen.data().queue.front() { - // c. Let resumptionValue be Completion(toYield.[[Completion]]). - let resume_kind = match next.completion.clone() { + // 10. Let queue be generator.[[AsyncGeneratorQueue]] + // 11. If queue is not empty, resume without suspending. + let next_completion = async_generator_object + .borrow() + .data() + .queue + .front() + .map(|n| n.completion.clone()); + + if let Some(next) = next_completion { + // c. Let resumptionValue be Completion(toYield.[[Completion]]) + let resume_kind = match next { CompletionRecord::Normal(val) => { context.vm.stack.push(val); GeneratorResumeKind::Normal @@ -125,9 +128,8 @@ impl AsyncGeneratorYield { } // 12. Else, - // a. Set generator.[[AsyncGeneratorState]] to suspended-yield. - r#gen.data_mut().state = AsyncGeneratorState::SuspendedYield; + async_generator_object.borrow_mut().data_mut().state = AsyncGeneratorState::SuspendedYield; // b. Remove genContext from the execution context stack and restore the execution context // that is at the top of the execution context stack as the running execution context. diff --git a/core/engine/src/vm/tests.rs b/core/engine/src/vm/tests.rs index 137b7735c77..5b1aa16b243 100644 --- a/core/engine/src/vm/tests.rs +++ b/core/engine/src/vm/tests.rs @@ -52,7 +52,7 @@ fn position() { .register_global_callable( js_string!("check_stack"), 2, - NativeFunction::from_copy_closure(|_, _, context| { + NativeFunction::from_copy_closure(context.gc_collector(), |_, _, context| { let frame = context.stack_trace().collect::>(); assert_eq!(frame.len(), 4); diff --git a/core/gc/Cargo.toml b/core/gc/Cargo.toml index 030298d6d33..f9853078ea4 100644 --- a/core/gc/Cargo.toml +++ b/core/gc/Cargo.toml @@ -34,7 +34,7 @@ either = { workspace = true, optional = true } thin-vec = { workspace = true, optional = true } icu_locale_core = { workspace = true, optional = true } arrayvec = { workspace = true, optional = true } -oscars = { git = "https://github.com/boa-dev/oscars.git", branch = "main", features = ["mark_sweep_branded"], optional = true } +oscars = { git = "https://github.com/shruti2522/oscars.git", branch = "size_class", features = ["mark_sweep_branded"], optional = true } typeid = { workspace = true, optional = true } [lints] diff --git a/core/gc/src/context.rs b/core/gc/src/context.rs index 5250d0f2266..2e1d78c1418 100644 --- a/core/gc/src/context.rs +++ b/core/gc/src/context.rs @@ -12,32 +12,50 @@ impl Default for GcContext { } } +#[cfg(feature = "oscars_backend")] +thread_local! { + static COLLECTOR: &'static oscars::collectors::mark_sweep_branded::Collector = + Box::leak(Box::new(oscars::collectors::mark_sweep_branded::Collector::new())); + + static DUMMY: &'static MutationContext<'static, 'static> = COLLECTOR.with(|c| { + Box::leak(Box::new(unsafe { + MutationContext::from_collector_erased(c) + })) + }); + + static TRACKER: std::cell::RefCell>> = const { std::cell::RefCell::new(None) }; +} + #[cfg(feature = "oscars_backend")] impl GcContext { #[must_use] + /// # Panics + /// Panics if the `HandleScope` cannot be rooted. pub fn new() -> Self { + TRACKER.with(|tracker| { + if tracker.borrow().is_none() { + let mc = DUMMY.with(|dummy| *dummy); + let handle = Gc::new(mc, crate::scope_tracker::HandleScopeTracker); + let root = mc.root(handle).expect("Failed to root HandleScopeTracker"); + *tracker.borrow_mut() = Some(root); + } + }); Self } pub fn alloc(&self, value: T) -> Gc<'static, T> { - let mc = MutationContext::global(); - Gc::new(&mc, value) + let mc = self.gc_collector(); + Gc::new(mc, value) } #[must_use] pub fn gc_collector(&self) -> &'static MutationContext<'static, 'static> { - thread_local! { - static COLLECTOR: &'static oscars::collectors::mark_sweep_branded::Collector = - Box::leak(Box::new(oscars::collectors::mark_sweep_branded::Collector::new())); - - static DUMMY: &'static MutationContext<'static, 'static> = COLLECTOR.with(|c| { - Box::leak(Box::new(unsafe { - MutationContext::from_collector_erased(c) - })) - }); - } DUMMY.with(|dummy| *dummy) } + + pub fn force_collect(&self) { + COLLECTOR.with(|c| c.collect()); + } } #[cfg(not(feature = "oscars_backend"))] @@ -61,12 +79,13 @@ unsafe impl Send for SyncWrapperDefault {} #[cfg(not(feature = "oscars_backend"))] impl GcContext { #[must_use] + /// # Panics + /// Panics if the `HandleScope` cannot be rooted. pub fn new() -> Self { Self } pub fn alloc(&self, value: T) -> crate::Gc<'static, T> { - // SAFETY: The global mutation context is used as a fallback during the context threading migration. let mc = unsafe { crate::MutationContext::global() }; crate::Gc::new(&mc, value) } @@ -74,7 +93,6 @@ impl GcContext { #[must_use] pub fn gc_collector(&self) -> &'static crate::MutationContext<'static, 'static> { static DUMMY: SyncWrapperDefault = - // SAFETY: The global mutation context is used as a fallback during the context threading migration. SyncWrapperDefault(unsafe { crate::MutationContext::global() }); &DUMMY.0 } diff --git a/core/gc/src/lib.rs b/core/gc/src/lib.rs index 2296038dc87..4faa94a046d 100644 --- a/core/gc/src/lib.rs +++ b/core/gc/src/lib.rs @@ -30,8 +30,15 @@ mod pointers; mod trace; pub mod context; +#[cfg(feature = "oscars_backend")] +pub(crate) mod scope; +#[cfg(feature = "oscars_backend")] +pub(crate) mod scope_tracker; pub use context::GcContext; +#[cfg(feature = "oscars_backend")] +pub use scope::{HandleScope, Local}; + #[cfg(not(feature = "oscars_backend"))] pub(crate) mod internals; @@ -171,4 +178,26 @@ mod test; #[cfg(feature = "oscars_backend")] /// Forces a garbage collection -pub fn force_collect() {} +pub fn force_collect() { + let mc = MutationContext::global(); + mc.collect(); + GcContext::new().force_collect(); +} + +#[cfg(feature = "oscars_backend")] +pub fn allocate_rooted<'gc, T: Trace + Finalize + 'gc>( + mc: &MutationContext<'gc, '_>, + value: T, +) -> Gc<'gc, T> { + let gc = Gc::new(mc, value); + drop(Local::new(gc.clone())); + gc +} + +#[cfg(not(feature = "oscars_backend"))] +pub fn allocate_rooted<'gc, T: Trace + Finalize + 'gc>( + mc: &MutationContext<'gc, '_>, + value: T, +) -> Gc<'gc, T> { + Gc::new(mc, value) +} diff --git a/core/gc/src/pointers/gc.rs b/core/gc/src/pointers/gc.rs index 3ef43301f5f..fd604351eec 100644 --- a/core/gc/src/pointers/gc.rs +++ b/core/gc/src/pointers/gc.rs @@ -175,10 +175,15 @@ impl<'gc, T: Trace + ?Sized + 'static> Gc<'gc, T> { // Note: Allocator can cause Collector to run let inner_ptr = Allocator::alloc_gc(GcBox::new(value)); - Self { + let gc = Self { inner_ptr, marker: PhantomData, - } + }; + + #[cfg(feature = "oscars_backend")] + crate::Local::new(gc.clone()); + + gc } /// Constructs a new `Gc` while giving you a `WeakGc` to the allocation, to allow diff --git a/core/gc/src/pointers/weak_map.rs b/core/gc/src/pointers/weak_map.rs index 85f76a6a653..624b1e71130 100644 --- a/core/gc/src/pointers/weak_map.rs +++ b/core/gc/src/pointers/weak_map.rs @@ -65,7 +65,7 @@ impl WeakMap { { let ephemeron = self.get(key)?; ephemeron - .value(&unsafe { crate::MutationContext::global() }) + .value(&unsafe { crate::MutationContext::dummy() }) .map(|v| v.clone()) } } diff --git a/core/gc/src/scope.rs b/core/gc/src/scope.rs new file mode 100644 index 00000000000..c14755b8543 --- /dev/null +++ b/core/gc/src/scope.rs @@ -0,0 +1,114 @@ +use std::cell::RefCell; +use std::marker::PhantomData; +use std::ptr::NonNull; + +use crate::{Gc, Trace, Tracer}; + +/// A type-erased local root for tracing. +#[derive(Clone, Copy)] +pub(crate) struct ErasedRoot { + /// The erased `PoolPointer` (`NonNull<()>`) from a `Gc<'_, T>`. + ptr: NonNull<()>, + /// A function that casts the pointer back to `Gc<'_, T>` and marks it. + trace_fn: unsafe fn(NonNull<()>, &mut Tracer<'_>), +} + +impl ErasedRoot { + #[allow(clippy::needless_pass_by_value)] + fn new(gc: Gc<'_, T>) -> Self { + unsafe fn trace_gc(ptr: NonNull<()>, tracer: &mut Tracer<'_>) { + unsafe { + // Reconstruct the Gc pointer + let gc: Gc<'_, T> = std::mem::transmute(ptr); + tracer.mark(&gc); + } + } + + Self { + // Safe because Gc has exactly the same memory layout as NonNull. + ptr: unsafe { std::mem::transmute_copy(&gc) }, + trace_fn: trace_gc::, + } + } + + pub(crate) unsafe fn trace(&self, tracer: &mut Tracer<'_>) { + unsafe { (self.trace_fn)(self.ptr, tracer) }; + } +} + +thread_local! { + /// A stack of handle scopes for the current thread. + pub(crate) static SCOPE_STACK: RefCell>> = const { RefCell::new(Vec::new()) }; +} + +/// A scope for tracking local handles. +#[derive(Debug)] +pub struct HandleScope { + _marker: PhantomData<*mut ()>, // Not Send or Sync +} + +impl HandleScope { + /// Enter a new handle scope. + #[must_use] + pub fn enter() -> Self { + SCOPE_STACK.with(|stack| stack.borrow_mut().push(Vec::new())); + Self { + _marker: PhantomData, + } + } +} + +impl Drop for HandleScope { + fn drop(&mut self) { + SCOPE_STACK.with(|stack| { + stack + .borrow_mut() + .pop() + .expect("HandleScope popped without being pushed"); + }); + } +} + +/// A local handle to a GC-managed value, scoped to the current `HandleScope`. +#[derive(Debug)] +pub struct Local<'gc, T: Trace> { + inner: Gc<'gc, T>, +} + +impl<'gc, T: Trace + 'gc> Local<'gc, T> { + /// Create a new local handle from a GC pointer. + #[must_use] + /// # Panics + /// Panics if there is no active `HandleScope`. + pub fn new(gc: Gc<'gc, T>) -> Self { + SCOPE_STACK.with(|stack| { + let mut stack = stack.borrow_mut(); + if let Some(top) = stack.last_mut() { + top.push(ErasedRoot::new(gc.clone())); + } else { + panic!("Cannot create Local without an active HandleScope"); + } + }); + + Self { inner: gc } + } + + #[must_use] + pub fn into_inner(self) -> Gc<'gc, T> { + self.inner + } +} + +impl<'gc, T: Trace + 'gc> Clone for Local<'gc, T> { + fn clone(&self) -> Self { + Self::new(self.inner.clone()) + } +} + +impl<'gc, T: Trace + 'gc> std::ops::Deref for Local<'gc, T> { + type Target = Gc<'gc, T>; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} diff --git a/core/gc/src/scope_tracker.rs b/core/gc/src/scope_tracker.rs new file mode 100644 index 00000000000..98a95dc4087 --- /dev/null +++ b/core/gc/src/scope_tracker.rs @@ -0,0 +1,22 @@ +use crate::scope::SCOPE_STACK; +use crate::{Finalize, Trace, Tracer}; + +pub(crate) struct HandleScopeTracker; + +impl Finalize for HandleScopeTracker { + fn finalize(&self) {} +} + +unsafe impl Trace for HandleScopeTracker { + unsafe fn trace(&self, tracer: &mut Tracer<'_>) { + SCOPE_STACK.with(|stack| { + for scope in stack.borrow().iter() { + for root in scope { + unsafe { + root.trace(tracer); + } + } + } + }); + } +} diff --git a/core/macros/src/module.rs b/core/macros/src/module.rs index beb9c5cef59..046bc4fe022 100644 --- a/core/macros/src/module.rs +++ b/core/macros/src/module.rs @@ -272,7 +272,7 @@ fn module_impl_impl(_args: ModuleArguments, mut mod_: ItemMod) -> SpannedResult< boa_engine::Module::synthetic( &[ #module_exports ], boa_engine::module::SyntheticModuleInitializer::from_copy_closure( - |m, context| { + context.gc_collector(), |m, context| { #module_fn Ok(()) } diff --git a/core/runtime/src/console/mod.rs b/core/runtime/src/console/mod.rs index 3aec9447708..e3513ac0acd 100644 --- a/core/runtime/src/console/mod.rs +++ b/core/runtime/src/console/mod.rs @@ -96,7 +96,7 @@ pub trait Logger: Trace { /// Returning an error will throw an exception in JavaScript. fn table(&self, data: TableData, state: &ConsoleState, context: &mut Context) -> JsResult<()> { let mut table = Table::new(); - table.load_preset(comfy_table::presets::UTF8_FULL); + table.load_style(comfy_table::presets::UTF8_FULL); table.set_content_arrangement(comfy_table::ContentArrangement::Dynamic); table.set_header(&data.col_names); @@ -339,25 +339,27 @@ impl Console { L: Logger + 'static, { fn console_method( + mc: &boa_gc::MutationContext<'_, '_>, f: fn(&JsValue, &[JsValue], &Console, &L, &mut Context) -> JsResult, state: Rc>, logger: Rc, ) -> NativeFunction { // SAFETY: `Console` doesn't contain types that need tracing. unsafe { - NativeFunction::from_closure(move |this, args, context| { + NativeFunction::from_closure(mc, move |this, args, context| { f(this, args, &state.borrow(), &logger, context) }) } } fn console_method_mut( + mc: &boa_gc::MutationContext<'_, '_>, f: fn(&JsValue, &[JsValue], &mut Console, &L, &mut Context) -> JsResult, state: Rc>, logger: Rc, ) -> NativeFunction { // SAFETY: `Console` doesn't contain types that need tracing. unsafe { - NativeFunction::from_closure(move |this, args, context| { + NativeFunction::from_closure(mc, move |this, args, context| { f(this, args, &mut state.borrow_mut(), &logger, context) }) } @@ -366,6 +368,116 @@ impl Console { let state = Rc::new(RefCell::new(Self::default())); let logger = Rc::new(logger); + let assert_fn = console_method( + context.gc_collector(), + Self::assert, + state.clone(), + logger.clone(), + ); + let clear_fn = console_method_mut( + context.gc_collector(), + Self::clear, + state.clone(), + logger.clone(), + ); + let debug_fn = console_method( + context.gc_collector(), + Self::debug, + state.clone(), + logger.clone(), + ); + let error_fn = console_method( + context.gc_collector(), + Self::error, + state.clone(), + logger.clone(), + ); + let info_fn = console_method( + context.gc_collector(), + Self::info, + state.clone(), + logger.clone(), + ); + let log_fn = console_method( + context.gc_collector(), + Self::log, + state.clone(), + logger.clone(), + ); + let trace_fn = console_method( + context.gc_collector(), + Self::trace, + state.clone(), + logger.clone(), + ); + let warn_fn = console_method( + context.gc_collector(), + Self::warn, + state.clone(), + logger.clone(), + ); + let count_fn = console_method_mut( + context.gc_collector(), + Self::count, + state.clone(), + logger.clone(), + ); + let count_reset_fn = console_method_mut( + context.gc_collector(), + Self::count_reset, + state.clone(), + logger.clone(), + ); + let group_fn = console_method_mut( + context.gc_collector(), + Self::group, + state.clone(), + logger.clone(), + ); + let group_collapsed_fn = console_method_mut( + context.gc_collector(), + Self::group_collapsed, + state.clone(), + logger.clone(), + ); + let group_end_fn = console_method_mut( + context.gc_collector(), + Self::group_end, + state.clone(), + logger.clone(), + ); + let time_fn = console_method_mut( + context.gc_collector(), + Self::time, + state.clone(), + logger.clone(), + ); + let time_log_fn = console_method( + context.gc_collector(), + Self::time_log, + state.clone(), + logger.clone(), + ); + let time_end_fn = console_method_mut( + context.gc_collector(), + Self::time_end, + state.clone(), + logger.clone(), + ); + let dir_fn = console_method( + context.gc_collector(), + Self::dir, + state.clone(), + logger.clone(), + ); + let dirxml_fn = console_method( + context.gc_collector(), + Self::dir, + state.clone(), + logger.clone(), + ); + let table_fn = console_method(context.gc_collector(), Self::table, state, logger.clone()); + ObjectInitializer::with_native_data_and_proto( Self::default(), JsObject::with_object_proto(context.gc_collector(), context.realm().intrinsics()), @@ -376,101 +488,25 @@ impl Console { Self::NAME, Attribute::CONFIGURABLE, ) - .function( - console_method(Self::assert, state.clone(), logger.clone()), - js_string!("assert"), - 0, - ) - .function( - console_method_mut(Self::clear, state.clone(), logger.clone()), - js_string!("clear"), - 0, - ) - .function( - console_method(Self::debug, state.clone(), logger.clone()), - js_string!("debug"), - 0, - ) - .function( - console_method(Self::error, state.clone(), logger.clone()), - js_string!("error"), - 0, - ) - .function( - console_method(Self::info, state.clone(), logger.clone()), - js_string!("info"), - 0, - ) - .function( - console_method(Self::log, state.clone(), logger.clone()), - js_string!("log"), - 0, - ) - .function( - console_method(Self::trace, state.clone(), logger.clone()), - js_string!("trace"), - 0, - ) - .function( - console_method(Self::warn, state.clone(), logger.clone()), - js_string!("warn"), - 0, - ) - .function( - console_method_mut(Self::count, state.clone(), logger.clone()), - js_string!("count"), - 0, - ) - .function( - console_method_mut(Self::count_reset, state.clone(), logger.clone()), - js_string!("countReset"), - 0, - ) - .function( - console_method_mut(Self::group, state.clone(), logger.clone()), - js_string!("group"), - 0, - ) - .function( - console_method_mut(Self::group_collapsed, state.clone(), logger.clone()), - js_string!("groupCollapsed"), - 0, - ) - .function( - console_method_mut(Self::group_end, state.clone(), logger.clone()), - js_string!("groupEnd"), - 0, - ) - .function( - console_method_mut(Self::time, state.clone(), logger.clone()), - js_string!("time"), - 0, - ) - .function( - console_method(Self::time_log, state.clone(), logger.clone()), - js_string!("timeLog"), - 0, - ) - .function( - console_method_mut(Self::time_end, state.clone(), logger.clone()), - js_string!("timeEnd"), - 0, - ) - .function( - console_method(Self::dir, state.clone(), logger.clone()), - js_string!("dir"), - 0, - ) - .function( - console_method(Self::dir, state.clone(), logger.clone()), - js_string!("dirxml"), - 0, - ) - .function( - console_method(Self::table, state, logger.clone()), - js_string!("table"), - 0, - ) + .function(assert_fn, js_string!("assert"), 0) + .function(clear_fn, js_string!("clear"), 0) + .function(debug_fn, js_string!("debug"), 0) + .function(error_fn, js_string!("error"), 0) + .function(info_fn, js_string!("info"), 0) + .function(log_fn, js_string!("log"), 0) + .function(trace_fn, js_string!("trace"), 0) + .function(warn_fn, js_string!("warn"), 0) + .function(count_fn, js_string!("count"), 0) + .function(count_reset_fn, js_string!("countReset"), 0) + .function(group_fn, js_string!("group"), 0) + .function(group_collapsed_fn, js_string!("groupCollapsed"), 0) + .function(group_end_fn, js_string!("groupEnd"), 0) + .function(time_fn, js_string!("time"), 0) + .function(time_log_fn, js_string!("timeLog"), 0) + .function(time_end_fn, js_string!("timeEnd"), 0) + .function(dir_fn, js_string!("dir"), 0) + .function(dirxml_fn, js_string!("dirxml"), 0) + .function(table_fn, js_string!("table"), 0) .build() } diff --git a/core/runtime/src/fetch/headers_iterator.rs b/core/runtime/src/fetch/headers_iterator.rs index 44e669ab0a5..6be8da9de08 100644 --- a/core/runtime/src/fetch/headers_iterator.rs +++ b/core/runtime/src/fetch/headers_iterator.rs @@ -116,8 +116,7 @@ impl HeadersIterator { .ok_or_else(|| boa_engine::js_error!(Error: "Headers Iterator not registered"))? .prototype(); - let headers_iterator = - JsObject::from_proto_and_data(&boa_gc::MutationContext::global(), proto, iter); + let headers_iterator = JsObject::from_proto_and_data(context.gc_collector(), proto, iter); Ok(headers_iterator.into()) } } diff --git a/core/runtime/src/process/mod.rs b/core/runtime/src/process/mod.rs index a9f04743aab..48b39757297 100644 --- a/core/runtime/src/process/mod.rs +++ b/core/runtime/src/process/mod.rs @@ -69,12 +69,13 @@ impl Process { P: ProcessProvider + 'static, { fn process_method( + mc: &boa_gc::MutationContext<'_, '_>, f: fn(&JsValue, &[JsValue], &P, &mut Context) -> JsResult, provider: Rc

, ) -> NativeFunction { // SAFETY: `Process` doesn't contain types that need tracing. unsafe { - NativeFunction::from_closure(move |this, args, context| { + NativeFunction::from_closure(mc, move |this, args, context| { f(this, args, &provider, context) }) } @@ -87,6 +88,8 @@ impl Process { env.set(key, JsValue::from(value), false, context)?; } + let gc_collector = context.gc_collector(); + Ok(ObjectInitializer::new(context) .property( JsSymbol::to_string_tag(), @@ -100,6 +103,7 @@ impl Process { ) .function( process_method( + gc_collector, |_, _, provider, _| provider.cwd().map(JsValue::from), provider.clone(), ), diff --git a/core/runtime/src/test262.rs b/core/runtime/src/test262.rs index ca172a06a5c..9dac56e496e 100644 --- a/core/runtime/src/test262.rs +++ b/core/runtime/src/test262.rs @@ -127,7 +127,7 @@ pub fn register_js262(handles: WorkerHandles, console: bool, context: &mut Conte js262 .create_data_property_or_throw( js_string!("IsHTMLDDA"), - JsObject::from_proto_and_data(&boa_gc::MutationContext::global(), None, IsHTMLDDA), + JsObject::from_proto_and_data(context.gc_collector(), None, IsHTMLDDA), context, ) .expect("the IsHTMLDDA property must be definable"); @@ -231,7 +231,7 @@ fn agent_obj(handles: WorkerHandles, console: bool, context: &mut Context) -> Js let start = unsafe { let bus = bus.clone(); - NativeFunction::from_closure(move |_, args, context| { + NativeFunction::from_closure(context.gc_collector(), move |_, args, context| { let script = args .get_or_undefined(0) .to_string(context)? @@ -270,7 +270,7 @@ fn agent_obj(handles: WorkerHandles, console: bool, context: &mut Context) -> Js let broadcast = unsafe { // should technically also have a second numeric argument, but the test262 never uses it. - NativeFunction::from_closure(move |_, args, _| { + NativeFunction::from_closure(context.gc_collector(), move |_, args, _| { let buffer = args.get_or_undefined(0).as_object().ok_or_else(|| { JsNativeError::typ().with_message("argument was not a shared array") })?; @@ -286,7 +286,7 @@ fn agent_obj(handles: WorkerHandles, console: bool, context: &mut Context) -> Js }; let get_report = unsafe { - NativeFunction::from_closure(move |_, _, _| { + NativeFunction::from_closure(context.gc_collector(), move |_, _, _| { let Ok(msg) = reports_rx.try_recv() else { return Ok(JsValue::null()); }; @@ -317,7 +317,7 @@ fn register_js262_worker( let rx = RefCell::new(rx); let receive_broadcast = unsafe { // should technically also have a second numeric argument, but the test262 never uses it. - NativeFunction::from_closure(move |_, args, context| { + NativeFunction::from_closure(context.gc_collector(), move |_, args, context| { let array = rx.borrow_mut().recv().map_err(|err| { JsNativeError::typ().with_message(format!("failed to receive buffer: {err}")) })?; @@ -333,7 +333,7 @@ fn register_js262_worker( }; let report = unsafe { - NativeFunction::from_closure(move |_, args, context| { + NativeFunction::from_closure(context.gc_collector(), move |_, args, context| { let string = args.get_or_undefined(0).to_string(context)?.to_vec(); tx.send(string) .map_err(|e| JsNativeError::typ().with_message(e.to_string()))?; diff --git a/core/string/Cargo.toml b/core/string/Cargo.toml index 89ee9a52791..7b35a45f455 100644 --- a/core/string/Cargo.toml +++ b/core/string/Cargo.toml @@ -12,7 +12,7 @@ repository.workspace = true rust-version.workspace = true [dependencies] -oscars = { git = "https://github.com/boa-dev/oscars.git", branch = "main", features = ["mark_sweep_branded"], optional = true } +oscars = { git = "https://github.com/shruti2522/oscars.git", branch = "size_class", features = ["mark_sweep_branded"], optional = true } itoa.workspace = true rustc-hash = { workspace = true, features = ["std"] } ryu-js.workspace = true diff --git a/core/string/src/builder.rs b/core/string/src/builder.rs index 96220f51b69..843c27861e2 100644 --- a/core/string/src/builder.rs +++ b/core/string/src/builder.rs @@ -771,12 +771,23 @@ impl<'seg, 'ref_str: 'seg> CommonJsStringBuilder<'seg> { let mut builder = Latin1JsStringBuilder::new(); for seg in &self.segments { match seg { - Segment::String(s) => { - builder.extend_from_slice(s.as_str().as_latin1()?); + Segment::String(s) => + { + #[allow(clippy::question_mark)] + if let Some(data) = s.as_str().as_latin1() { + builder.extend_from_slice(data); + } else { + return None; + } } - Segment::Str(s) => { - let data = s.as_latin1()?; - builder.extend_from_slice(data); + Segment::Str(s) => + { + #[allow(clippy::question_mark)] + if let Some(data) = s.as_latin1() { + builder.extend_from_slice(data); + } else { + return None; + } } Segment::Latin1(b) => { if *b <= 0x7f { diff --git a/core/wintertc/src/console/mod.rs b/core/wintertc/src/console/mod.rs index 3ff148f8878..8ae96c85f85 100644 --- a/core/wintertc/src/console/mod.rs +++ b/core/wintertc/src/console/mod.rs @@ -364,9 +364,10 @@ impl Console { ) -> NativeFunction { // SAFETY: `Console` doesn't contain types that need tracing. unsafe { - NativeFunction::from_closure(move |this, args, context| { - f(this, args, &state.borrow(), &logger, context) - }) + NativeFunction::from_closure( + &boa_gc::MutationContext::global(), + move |this, args, context| f(this, args, &state.borrow(), &logger, context), + ) } } fn console_method_mut( @@ -376,9 +377,12 @@ impl Console { ) -> NativeFunction { // SAFETY: `Console` doesn't contain types that need tracing. unsafe { - NativeFunction::from_closure(move |this, args, context| { - f(this, args, &mut state.borrow_mut(), &logger, context) - }) + NativeFunction::from_closure( + &boa_gc::MutationContext::global(), + move |this, args, context| { + f(this, args, &mut state.borrow_mut(), &logger, context) + }, + ) } } diff --git a/core/wintertc/src/console/tests.rs b/core/wintertc/src/console/tests.rs index 2c2e0c2116e..e24470acf84 100644 --- a/core/wintertc/src/console/tests.rs +++ b/core/wintertc/src/console/tests.rs @@ -136,6 +136,29 @@ impl Logger for RecordingLogger { fn error(&self, msg: String, state: &ConsoleState, context: &mut Context) -> JsResult<()> { self.log(msg, state, context) } + + fn table( + &self, + data: crate::console::TableData, + state: &ConsoleState, + context: &mut Context, + ) -> JsResult<()> { + let mut table = comfy_table::Table::new(); + table.load_style(comfy_table::presets::UTF8_FULL); + // Do not use Dynamic arrangement in tests to avoid wrapping based on pseudo-TTY width. + table.set_header(&data.col_names); + + for row in &data.rows { + let cells: Vec = data + .col_names + .iter() + .map(|name| comfy_table::Cell::new(row.get(name).cloned().unwrap_or_default())) + .collect(); + table.add_row(cells); + } + + self.log(table.to_string(), state, context) + } } /// Harness methods to be used in JS tests. diff --git a/examples/src/bin/closures.rs b/examples/src/bin/closures.rs index ffeda1681bc..24d92645b4d 100644 --- a/examples/src/bin/closures.rs +++ b/examples/src/bin/closures.rs @@ -24,7 +24,7 @@ fn main() -> Result<(), JsError> { .register_global_callable( JsString::from("closure"), 0, - NativeFunction::from_copy_closure(move |_, _, _| { + NativeFunction::from_copy_closure(context.gc_collector(), move |_, _, _| { println!("Called `closure`"); // `variable` is captured from the main function. println!("variable = {variable}"); @@ -72,6 +72,7 @@ fn main() -> Result<(), JsError> { context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, _, captures, context| { let mut captures = captures.borrow_mut(); let BigStruct { greeting, object } = &mut *captures; @@ -80,15 +81,16 @@ fn main() -> Result<(), JsError> { let name = object.get(js_string!("name"), context)?; // We create a new message from our captured variable. + let greeting_ref: &JsString = greeting; let message = js_string!( &js_string!("message from `"), &name.to_string(context)?, &js_string!("`: "), - &*greeting + greeting_ref ); // We can also mutate the moved data inside the closure. - captures.greeting = js_string!(&*greeting, &js_string!(" Hello!")); + captures.greeting = js_string!(greeting_ref, &js_string!(" Hello!")); println!("{}", message.to_std_string_escaped()); println!(); @@ -149,7 +151,7 @@ fn main() -> Result<(), JsError> { // Note that it is required to use `unsafe` code, since the compiler cannot verify that the // types captured by the closure are not traceable. unsafe { - NativeFunction::from_closure(move |_, _, context| { + NativeFunction::from_closure(context.gc_collector(), move |_, _, context| { println!("Called `enumerate`"); // `index` is captured from the main function. println!("index = {}", index.get()); diff --git a/examples/src/bin/jstypedarray.rs b/examples/src/bin/jstypedarray.rs index c6829bb2d92..92987af7c2f 100644 --- a/examples/src/bin/jstypedarray.rs +++ b/examples/src/bin/jstypedarray.rs @@ -95,12 +95,13 @@ fn main() -> JsResult<()> { // forEach let array = JsUint8Array::from_iter(vec![1, 2, 3, 4, 5], context)?; - let num_to_modify = Gc::new(&boa_gc::MutationContext::global(), GcRefCell::new(0u8)); + let num_to_modify = Gc::new(context.gc_collector(), GcRefCell::new(0u8)); let js_function = FunctionObjectBuilder::new( context.realm(), context.gc_collector(), NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, args, captures, inner_context| { let element = args .first() diff --git a/examples/src/bin/modules.rs b/examples/src/bin/modules.rs index d513c6f3ccb..aefc3f2f947 100644 --- a/examples/src/bin/modules.rs +++ b/examples/src/bin/modules.rs @@ -55,6 +55,7 @@ fn main() -> Result<(), Box> { .then( Some( NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), |_, _, module, context| { // After loading, link all modules by resolving the imports // and exports on the full module graph, initializing module @@ -74,6 +75,7 @@ fn main() -> Result<(), Box> { .then( Some( NativeFunction::from_copy_closure_with_captures( + context.gc_collector(), // Finally, evaluate the root module. // This returns a `JsPromise` since a module could have // top-level await statements, which defers module execution to the diff --git a/examples/src/bin/smol_event_loop.rs b/examples/src/bin/smol_event_loop.rs index 504c39761d1..b54bf9ade1a 100644 --- a/examples/src/bin/smol_event_loop.rs +++ b/examples/src/bin/smol_event_loop.rs @@ -201,7 +201,7 @@ fn add_runtime(context: &mut Context) { .register_global_builtin_callable( js_string!("delay"), 1, - NativeFunction::from_async_fn(delay), + NativeFunction::from_async_fn(context.gc_collector(), delay), ) .expect("the delay builtin shouldn't exist"); diff --git a/examples/src/bin/synthetic.rs b/examples/src/bin/synthetic.rs index c892c9b35de..e87fdec0679 100644 --- a/examples/src/bin/synthetic.rs +++ b/examples/src/bin/synthetic.rs @@ -167,6 +167,7 @@ fn create_operations_module(context: &mut Context) -> Module { // The initializer is evaluated every time a module imports this synthetic module, // so we avoid creating duplicate objects by capturing and cloning them instead. SyntheticModuleInitializer::from_copy_closure_with_captures( + context.gc_collector(), |module, fns, _| { println!("Running initializer!"); module.set_export(&js_string!("sum"), fns.0.clone().into())?; diff --git a/examples/src/bin/tokio_event_loop.rs b/examples/src/bin/tokio_event_loop.rs index d82a1ed31af..2261cd52e58 100644 --- a/examples/src/bin/tokio_event_loop.rs +++ b/examples/src/bin/tokio_event_loop.rs @@ -208,7 +208,7 @@ fn add_runtime(context: &mut Context) { .register_global_builtin_callable( js_string!("delay"), 1, - NativeFunction::from_async_fn(delay), + NativeFunction::from_async_fn(context.gc_collector(), delay), ) .expect("the delay builtin shouldn't exist"); diff --git a/ffi/wasm/Cargo.toml b/ffi/wasm/Cargo.toml index 55b997db512..ec51c1b2d94 100644 --- a/ffi/wasm/Cargo.toml +++ b/ffi/wasm/Cargo.toml @@ -27,6 +27,7 @@ default = [ "boa_engine/intl_bundled", "boa_engine/temporal", "boa_engine/xsum", + "boa_engine/oscars_backend", ] [lib] diff --git a/tests/macros/tests/class.rs b/tests/macros/tests/class.rs index 4f6c201e218..e31a931edc3 100644 --- a/tests/macros/tests/class.rs +++ b/tests/macros/tests/class.rs @@ -46,7 +46,7 @@ impl Animal { #[boa(method)] #[boa(length = 11)] fn method(context: &mut Context) -> JsObject { - let obj = JsObject::with_null_proto(&boa_gc::MutationContext::global()); + let obj = JsObject::with_null_proto(&unsafe { boa_gc::MutationContext::global() }); obj.set(js_string!("key"), 43, false, context).unwrap(); obj } diff --git a/tests/macros/tests/gcd_callback.rs b/tests/macros/tests/gcd_callback.rs index 796d211066f..1a4bfe9dbe1 100644 --- a/tests/macros/tests/gcd_callback.rs +++ b/tests/macros/tests/gcd_callback.rs @@ -19,7 +19,10 @@ fn gcd_callback() { // Create the engine. let context = &mut Context::default(); - let result = Gc::new(&boa_gc::MutationContext::global(), AtomicUsize::new(0)); + let result = Gc::new( + &unsafe { boa_gc::MutationContext::global() }, + AtomicUsize::new(0), + ); context.insert_data(result.clone()); // Load the JavaScript code. diff --git a/tests/tester/src/exec/mod.rs b/tests/tester/src/exec/mod.rs index 4fbe43a7a9d..44e47688b58 100644 --- a/tests/tester/src/exec/mod.rs +++ b/tests/tester/src/exec/mod.rs @@ -515,6 +515,9 @@ impl Test { }, ); + // Force GC collection to prevent memory exhaustion when running tens of thousands of tests. + boa_engine::gc::force_collect(); + self.create_result(result, result_text, strict, verbosity) } @@ -628,7 +631,7 @@ fn register_print_fn(context: &mut Context, async_result: AsyncResult) { context.gc_collector(), // SAFETY: `AsyncResult` has only non-traceable captures, making this safe. unsafe { - NativeFunction::from_closure(move |_, args, context| { + NativeFunction::from_closure(context.gc_collector(), move |_, args, context| { let message = args .get_or_undefined(0) .to_string(context)?