Prevent recursing in __call__ and __construct__ internal methods

boa-dev · Oct 9, 2023 · a73be22 · a73be22
1 parent f6ca5c1
commit a73be22
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 53 deletions.
diff --git 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<bool> {
+) -> JsResult<CallValue> {
     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<bool> {
+) -> JsResult<CallValue> {
     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
@@ -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<bool> {
+) -> JsResult<CallValue> {
     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<bool> {
+) -> JsResult<CallValue> {
     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<bool> {
+) -> JsResult<CallValue> {
     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<bool> {
+) -> JsResult<CallValue> {
     // 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
@@ -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<bool> {
-        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<bool> {
-        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<bool>,
     pub(crate) __delete__: fn(&JsObject, &PropertyKey, &mut Context<'_>) -> JsResult<bool>,
     pub(crate) __own_property_keys__: fn(&JsObject, &mut Context<'_>) -> JsResult<Vec<PropertyKey>>,
-    pub(crate) __call__: fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult<bool>,
+    pub(crate) __call__:
+        fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult<CallValue>,
     pub(crate) __construct__:
-        fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult<bool>,
+        fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult<CallValue>,
+}
+
+pub(crate) enum CallValue {
+    Ready,
+    Pending {
+        func: fn(&JsObject, arguments_count: usize, &mut Context<'_>) -> JsResult<CallValue>,
+        object: JsObject,
+        argument_count: usize,
+    },
+    Complete,
+}
+
+impl CallValue {
+    pub(crate) fn resolve(mut self, context: &mut Context<'_>) -> JsResult<bool> {
+        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<bool> {
+) -> JsResult<CallValue> {
     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<bool> {
+) -> JsResult<CallValue> {
     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
@@ -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<bool> {
+) -> JsResult<CallValue> {
     // 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<bool> {
+) -> JsResult<CallValue> {
     // 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
@@ -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
@@ -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
@@ -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)
     }
 }