From a73be2224583e930a330f42e23068cf50fefa3a4 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Mon, 9 Oct 2023 16:27:24 +0200 Subject: [PATCH] Prevent recursing in __call__ and __construct__ internal methods --- .../object/internal_methods/bound_function.rs | 22 ++++--- .../src/object/internal_methods/function.rs | 21 +++---- boa_engine/src/object/internal_methods/mod.rs | 60 +++++++++++++------ .../src/object/internal_methods/proxy.rs | 28 ++++++--- boa_engine/src/object/operations.rs | 4 +- boa_engine/src/vm/opcode/call/mod.rs | 9 +-- boa_engine/src/vm/opcode/new/mod.rs | 6 +- 7 files changed, 97 insertions(+), 53 deletions(-) diff --git a/boa_engine/src/object/internal_methods/bound_function.rs b/boa_engine/src/object/internal_methods/bound_function.rs index 47cd2c527ad..ccc7abf34f2 100644 --- a/boa_engine/src/object/internal_methods/bound_function.rs +++ b/boa_engine/src/object/internal_methods/bound_function.rs @@ -1,6 +1,6 @@ use crate::{object::JsObject, Context, JsResult, JsValue}; -use super::{InternalObjectMethods, ORDINARY_INTERNAL_METHODS}; +use super::{CallValue, InternalObjectMethods, ORDINARY_INTERNAL_METHODS}; /// Definitions of the internal object methods for function objects. /// @@ -27,12 +27,12 @@ pub(crate) static BOUND_CONSTRUCTOR_EXOTIC_INTERNAL_METHODS: InternalObjectMetho /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-bound-function-exotic-objects-call-thisargument-argumentslist -#[track_caller] +#[allow(clippy::unnecessary_wraps)] fn bound_function_exotic_call( obj: &JsObject, arguments_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { let obj = obj.borrow(); let bound_function = obj .as_bound_function() @@ -55,7 +55,11 @@ fn bound_function_exotic_call( context.vm.stack.splice(at..at, bound_args.iter().cloned()); // 5. Return ? Call(target, boundThis, args). - target.__call__(bound_args.len() + arguments_count, context) + Ok(CallValue::Pending { + func: target.vtable().__call__, + object: target.clone(), + argument_count: bound_args.len() + arguments_count, + }) } /// Internal method `[[Construct]]` for Bound Function Exotic Objects @@ -64,12 +68,12 @@ fn bound_function_exotic_call( /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-bound-function-exotic-objects-construct-argumentslist-newtarget -#[track_caller] +#[allow(clippy::unnecessary_wraps)] fn bound_function_exotic_construct( function_object: &JsObject, arguments_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { let new_target = context.vm.pop(); debug_assert!(new_target.is_object(), "new.target should be an object"); @@ -102,5 +106,9 @@ fn bound_function_exotic_construct( // 6. Return ? Construct(target, args, newTarget). context.vm.push(new_target); - target.__construct__(bound_args.len() + arguments_count, context) + Ok(CallValue::Pending { + func: target.vtable().__construct__, + object: target.clone(), + argument_count: bound_args.len() + arguments_count, + }) } diff --git a/boa_engine/src/object/internal_methods/function.rs b/boa_engine/src/object/internal_methods/function.rs index 4ce97782347..763831dae26 100644 --- a/boa_engine/src/object/internal_methods/function.rs +++ b/boa_engine/src/object/internal_methods/function.rs @@ -10,7 +10,7 @@ use crate::{ Context, JsNativeError, JsResult, JsValue, }; -use super::get_prototype_from_constructor; +use super::{get_prototype_from_constructor, CallValue}; /// Definitions of the internal object methods for function objects. /// @@ -40,7 +40,7 @@ pub(crate) fn function_call( function_object: &JsObject, arguments_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { context.check_runtime_limits()?; let object = function_object.borrow(); @@ -154,8 +154,7 @@ pub(crate) fn function_call( .put_lexical_value(env_index, 0, arguments_obj.into()); } - // The call is pending, not complete. - Ok(false) + Ok(CallValue::Ready) } /// Construct an instance of this object with the specified arguments. @@ -168,7 +167,7 @@ fn function_construct( this_function_object: &JsObject, argument_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { context.check_runtime_limits()?; let object = this_function_object.borrow(); @@ -302,7 +301,7 @@ fn function_construct( .stack .insert(at - 1, this.map(JsValue::new).unwrap_or_default()); - Ok(false) + Ok(CallValue::Ready) } /// Definitions of the internal object methods for native function objects. @@ -334,7 +333,7 @@ pub(crate) fn native_function_call( obj: &JsObject, arguments_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { let args = context.pop_n_arguments(arguments_count); let _func = context.vm.pop(); let this = context.vm.pop(); @@ -374,8 +373,7 @@ pub(crate) fn native_function_call( context.vm.push(result?); - // The function has been evaluated, it's complete. - Ok(true) + Ok(CallValue::Complete) } /// Construct an instance of this object with the specified arguments. @@ -388,7 +386,7 @@ fn native_function_construct( obj: &JsObject, arguments_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { // We technically don't need this since native functions don't push any new frames to the // vm, but we'll eventually have to combine the native stack with the vm stack. context.check_runtime_limits()?; @@ -446,6 +444,5 @@ fn native_function_construct( context.vm.push(result?); - // The function has been evaluated, it's complete. - Ok(true) + Ok(CallValue::Complete) } diff --git a/boa_engine/src/object/internal_methods/mod.rs b/boa_engine/src/object/internal_methods/mod.rs index 6e54a165e02..be7e37a6f5c 100644 --- a/boa_engine/src/object/internal_methods/mod.rs +++ b/boa_engine/src/object/internal_methods/mod.rs @@ -217,13 +217,12 @@ impl JsObject { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-ecmascript-function-objects-call-thisargument-argumentslist - pub(crate) fn __call__( - &self, - arguments_count: usize, - context: &mut Context<'_>, - ) -> JsResult { - let _timer = Profiler::global().start_event("Object::__call__", "object"); - (self.vtable().__call__)(self, arguments_count, context) + pub(crate) fn __call__(&self, argument_count: usize) -> CallValue { + CallValue::Pending { + func: self.vtable().__call__, + object: self.clone(), + argument_count, + } } /// Internal method `[[Construct]]` @@ -234,13 +233,12 @@ impl JsObject { /// - [ECMAScript reference][spec] /// /// [spec]: https://tc39.es/ecma262/#sec-ecmascript-function-objects-construct-argumentslist-newtarget - pub(crate) fn __construct__( - &self, - arguments_count: usize, - context: &mut Context<'_>, - ) -> JsResult { - let _timer = Profiler::global().start_event("Object::__construct__", "object"); - (self.vtable().__construct__)(self, arguments_count, context) + pub(crate) fn __construct__(&self, argument_count: usize) -> CallValue { + CallValue::Pending { + func: self.vtable().__construct__, + object: self.clone(), + argument_count, + } } } @@ -295,9 +293,35 @@ pub(crate) struct InternalObjectMethods { fn(&JsObject, PropertyKey, JsValue, JsValue, &mut Context<'_>) -> JsResult, pub(crate) __delete__: fn(&JsObject, &PropertyKey, &mut Context<'_>) -> JsResult, pub(crate) __own_property_keys__: fn(&JsObject, &mut Context<'_>) -> JsResult>, - pub(crate) __call__: fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult, + pub(crate) __call__: + fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult, pub(crate) __construct__: - fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult, + fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult, +} + +pub(crate) enum CallValue { + Ready, + Pending { + func: fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult, + object: JsObject, + argument_count: usize, + }, + Complete, +} + +impl CallValue { + pub(crate) fn resolve(mut self, context: &mut Context<'_>) -> JsResult { + while let Self::Pending { + func, + object, + argument_count, + } = self + { + self = func(&object, argument_count, context)?; + } + + Ok(matches!(self, Self::Complete)) + } } /// Abstract operation `OrdinaryGetPrototypeOf`. @@ -911,7 +935,7 @@ pub(crate) fn non_existant_call( _obj: &JsObject, _argument_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { Err(JsNativeError::typ() .with_message("not a callable function") .with_realm(context.realm().clone()) @@ -922,7 +946,7 @@ pub(crate) fn non_existant_construct( _obj: &JsObject, _arguments_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { Err(JsNativeError::typ() .with_message("not a constructor") .with_realm(context.realm().clone()) diff --git a/boa_engine/src/object/internal_methods/proxy.rs b/boa_engine/src/object/internal_methods/proxy.rs index 06f852b78a8..3d57f698d03 100644 --- a/boa_engine/src/object/internal_methods/proxy.rs +++ b/boa_engine/src/object/internal_methods/proxy.rs @@ -9,7 +9,7 @@ use crate::{ }; use rustc_hash::FxHashSet; -use super::ORDINARY_INTERNAL_METHODS; +use super::{CallValue, ORDINARY_INTERNAL_METHODS}; /// Definitions of the internal object methods for array exotic objects. /// @@ -924,7 +924,7 @@ fn proxy_exotic_call( obj: &JsObject, arguments_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { // 1. Let handler be O.[[ProxyHandler]]. // 2. If handler is null, throw a TypeError exception. // 3. Assert: Type(handler) is Object. @@ -939,7 +939,11 @@ fn proxy_exotic_call( let Some(trap) = handler.get_method(utf16!("apply"), context)? else { // 6. If trap is undefined, then // a. Return ? Call(target, thisArgument, argumentsList). - return target.__call__(arguments_count, context); + return Ok(CallValue::Pending { + func: target.vtable().__call__, + object: target, + argument_count: arguments_count, + }); }; let args = context.pop_n_arguments(arguments_count); @@ -957,7 +961,11 @@ fn proxy_exotic_call( context.vm.push(target); context.vm.push(this); context.vm.push(arg_array); - trap.__call__(3, context) + Ok(CallValue::Pending { + func: trap.vtable().__call__, + object: trap, + argument_count: 3, + }) } /// `[[Construct]] ( argumentsList, newTarget )` @@ -970,7 +978,7 @@ fn proxy_exotic_construct( obj: &JsObject, arguments_count: usize, context: &mut Context<'_>, -) -> JsResult { +) -> JsResult { // 1. Let handler be O.[[ProxyHandler]]. // 2. If handler is null, throw a TypeError exception. // 3. Assert: Type(handler) is Object. @@ -988,7 +996,11 @@ fn proxy_exotic_construct( let Some(trap) = handler.get_method(utf16!("construct"), context)? else { // 7. If trap is undefined, then // a. Return ? Construct(target, argumentsList, newTarget). - return target.__construct__(arguments_count, context); + return Ok(CallValue::Pending { + func: target.vtable().__construct__, + object: target, + argument_count: arguments_count, + }); }; let new_target = context.vm.pop(); @@ -1001,7 +1013,7 @@ fn proxy_exotic_construct( // 9. Let newObj be ? Call(trap, handler, « target, argArray, newTarget »). let new_obj = trap.call( &handler.into(), - &[target.clone().into(), arg_array.into(), new_target], + &[target.into(), arg_array.into(), new_target], context, )?; @@ -1012,5 +1024,5 @@ fn proxy_exotic_construct( // 11. Return newObj. context.vm.push(new_obj); - Ok(true) + Ok(CallValue::Complete) } diff --git a/boa_engine/src/object/operations.rs b/boa_engine/src/object/operations.rs index fe1eb6c5e96..25e48a4874f 100644 --- a/boa_engine/src/object/operations.rs +++ b/boa_engine/src/object/operations.rs @@ -330,7 +330,7 @@ impl JsObject { // 3. Return ? F.[[Call]](V, argumentsList). let frame_index = context.vm.frames.len(); - let is_complete = self.__call__(arguments_count, context)?; + let is_complete = self.__call__(arguments_count).resolve(context)?; if is_complete { return Ok(context.vm.pop()); @@ -379,7 +379,7 @@ impl JsObject { // 2. If argumentsList is not present, set argumentsList to a new empty List. // 3. Return ? F.[[Construct]](argumentsList, newTarget). let frame_index = context.vm.frames.len(); - let is_complete = self.__construct__(arguments_count, context)?; + let is_complete = self.__construct__(arguments_count).resolve(context)?; if is_complete { let result = context.vm.pop(); diff --git a/boa_engine/src/vm/opcode/call/mod.rs b/boa_engine/src/vm/opcode/call/mod.rs index 92938524465..fe2c0cd7551 100644 --- a/boa_engine/src/vm/opcode/call/mod.rs +++ b/boa_engine/src/vm/opcode/call/mod.rs @@ -43,7 +43,7 @@ impl CallEval { return Ok(CompletionType::Normal); } - object.clone().__call__(argument_count, context)?; + object.__call__(argument_count).resolve(context)?; Ok(CompletionType::Normal) } } @@ -123,7 +123,8 @@ impl Operation for CallEvalSpread { let argument_count = arguments.len(); context.push_n_arguments(&arguments); - object.__call__(argument_count, context)?; + + object.__call__(argument_count).resolve(context)?; Ok(CompletionType::Normal) } } @@ -146,7 +147,7 @@ impl Call { .into()); }; - object.clone().__call__(argument_count, context)?; + object.__call__(argument_count).resolve(context)?; Ok(CompletionType::Normal) } } @@ -203,7 +204,7 @@ impl Operation for CallSpread { .into()); }; - object.clone().__call__(argument_count, context)?; + object.__call__(argument_count).resolve(context)?; Ok(CompletionType::Normal) } } diff --git a/boa_engine/src/vm/opcode/new/mod.rs b/boa_engine/src/vm/opcode/new/mod.rs index 7071942f861..783a2a141f8 100644 --- a/boa_engine/src/vm/opcode/new/mod.rs +++ b/boa_engine/src/vm/opcode/new/mod.rs @@ -22,7 +22,8 @@ impl New { .clone(); context.vm.push(cons.clone()); // Push new.target - cons.__construct__(argument_count, context)?; + + cons.__construct__(argument_count).resolve(context)?; Ok(CompletionType::Normal) } } @@ -82,7 +83,8 @@ impl Operation for NewSpread { context.vm.push(func); context.push_n_arguments(&arguments); context.vm.push(cons.clone()); // Push new.target - cons.__construct__(argument_count, context)?; + + cons.__construct__(argument_count).resolve(context)?; Ok(CompletionType::Normal) } }