From 652db85ad03e2106bcc5eded4c78914a6ad298bc Mon Sep 17 00:00:00 2001 From: raskad <32105367+raskad@users.noreply.github.com> Date: Thu, 4 Aug 2022 00:49:06 +0200 Subject: [PATCH] Fix spread arguments in function calls --- boa_engine/src/bytecompiler.rs | 62 +++++++++----- boa_engine/src/object/property_map.rs | 9 ++ boa_engine/src/vm/code_block.rs | 8 +- boa_engine/src/vm/mod.rs | 113 ++++++++++++-------------- boa_engine/src/vm/opcode.rs | 48 +++++------ 5 files changed, 132 insertions(+), 108 deletions(-) diff --git a/boa_engine/src/bytecompiler.rs b/boa_engine/src/bytecompiler.rs index c5c34f33f72..31bce479ef0 100644 --- a/boa_engine/src/bytecompiler.rs +++ b/boa_engine/src/bytecompiler.rs @@ -1227,15 +1227,30 @@ impl<'b> ByteCompiler<'b> { } Node::ClassExpr(class) => self.class(class, true)?, Node::SuperCall(super_call) => { - for arg in super_call.args() { - self.compile_expr(arg, true)?; + let contains_spread = super_call + .args() + .iter() + .any(|arg| matches!(arg, Node::Spread(_))); + + if contains_spread { + self.emit_opcode(Opcode::PushNewArray); + for arg in super_call.args() { + self.compile_expr(arg, true)?; + if let Node::Spread(_) = arg { + self.emit_opcode(Opcode::InitIterator); + self.emit_opcode(Opcode::PushIteratorToArray); + } else { + self.emit_opcode(Opcode::PushValueToArray); + } + } + } else { + for arg in super_call.args() { + self.compile_expr(arg, true)?; + } } - let last_is_rest_parameter = - matches!(super_call.args().last(), Some(Node::Spread(_))); - - if last_is_rest_parameter { - self.emit(Opcode::SuperCallWithRest, &[super_call.args().len() as u32]); + if contains_spread { + self.emit_opcode(Opcode::SuperCallSpread); } else { self.emit(Opcode::SuperCall, &[super_call.args().len() as u32]); } @@ -2243,24 +2258,31 @@ impl<'b> ByteCompiler<'b> { } } - for arg in call.args().iter() { - self.compile_expr(arg, true)?; - } + let contains_spread = call.args().iter().any(|arg| matches!(arg, Node::Spread(_))); - let last_is_rest_parameter = matches!(call.args().last(), Some(Node::Spread(_))); + if contains_spread { + self.emit_opcode(Opcode::PushNewArray); + for arg in call.args() { + self.compile_expr(arg, true)?; + if let Node::Spread(_) = arg { + self.emit_opcode(Opcode::InitIterator); + self.emit_opcode(Opcode::PushIteratorToArray); + } else { + self.emit_opcode(Opcode::PushValueToArray); + } + } + } else { + for arg in call.args() { + self.compile_expr(arg, true)?; + } + } match kind { - CallKind::CallEval if last_is_rest_parameter => { - self.emit(Opcode::CallEvalWithRest, &[call.args().len() as u32]); - } + CallKind::CallEval if contains_spread => self.emit_opcode(Opcode::CallEvalSpread), CallKind::CallEval => self.emit(Opcode::CallEval, &[call.args().len() as u32]), - CallKind::Call if last_is_rest_parameter => { - self.emit(Opcode::CallWithRest, &[call.args().len() as u32]); - } + CallKind::Call if contains_spread => self.emit_opcode(Opcode::CallSpread), CallKind::Call => self.emit(Opcode::Call, &[call.args().len() as u32]), - CallKind::New if last_is_rest_parameter => { - self.emit(Opcode::NewWithRest, &[call.args().len() as u32]); - } + CallKind::New if contains_spread => self.emit_opcode(Opcode::NewSpread), CallKind::New => self.emit(Opcode::New, &[call.args().len() as u32]), } diff --git a/boa_engine/src/object/property_map.rs b/boa_engine/src/object/property_map.rs index c83eeb21462..c06bb1e4125 100644 --- a/boa_engine/src/object/property_map.rs +++ b/boa_engine/src/object/property_map.rs @@ -272,6 +272,15 @@ impl PropertyMap { self.indexed_properties = IndexedProperties::Dense(properties); } + /// Returns the vec of dense indexed properties if they exist. + pub(crate) fn dense_indexed_properties(&self) -> Option<&Vec> { + if let IndexedProperties::Dense(properties) = &self.indexed_properties { + Some(properties) + } else { + None + } + } + /// An iterator visiting all key-value pairs in arbitrary order. The iterator element type is `(PropertyKey, &'a Property)`. /// /// This iterator does not recurse down the prototype chain. diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 9d89e8fb8fa..044b8a3f0d5 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -198,13 +198,9 @@ impl CodeBlock { | Opcode::LogicalOr | Opcode::Coalesce | Opcode::CallEval - | Opcode::CallEvalWithRest | Opcode::Call - | Opcode::CallWithRest | Opcode::New - | Opcode::NewWithRest | Opcode::SuperCall - | Opcode::SuperCallWithRest | Opcode::ForInLoopInitIterator | Opcode::ForInLoopNext | Opcode::ConcatToString @@ -367,6 +363,10 @@ impl CodeBlock { | Opcode::PushClassField | Opcode::SuperCallDerived | Opcode::Await + | Opcode::CallEvalSpread + | Opcode::CallSpread + | Opcode::NewSpread + | Opcode::SuperCallSpread | Opcode::Nop => String::new(), } } diff --git a/boa_engine/src/vm/mod.rs b/boa_engine/src/vm/mod.rs index 625471c6ebd..0163f14d385 100644 --- a/boa_engine/src/vm/mod.rs +++ b/boa_engine/src/vm/mod.rs @@ -1560,21 +1560,18 @@ impl Context { } self.vm.push(result); } - Opcode::SuperCallWithRest => { - let argument_count = self.vm.read::(); - let rest_argument = self.vm.pop(); - let mut arguments = Vec::with_capacity(argument_count as usize); - for _ in 0..(argument_count - 1) { - arguments.push(self.vm.pop()); - } - arguments.reverse(); - - let iterator_record = rest_argument.get_iterator(self, None, None)?; - let mut rest_arguments = Vec::new(); - while let Some(next) = iterator_record.step(self)? { - rest_arguments.push(next.value(self)?); - } - arguments.append(&mut rest_arguments); + Opcode::SuperCallSpread => { + // Get the arguments that are stored as an array object on the stack. + let arguments_array = self.vm.pop(); + let arguments_array_object = arguments_array + .as_object() + .expect("arguments array in call spread function must be an object"); + let arguments = arguments_array_object + .borrow() + .properties() + .dense_indexed_properties() + .expect("arguments array in call spread function must be dense") + .clone(); let (new_target, active_function) = { let this_env = self @@ -1735,27 +1732,26 @@ impl Context { self.vm.push(result); } } - Opcode::CallEvalWithRest => { + Opcode::CallEvalSpread => { if self.vm.stack_size_limit <= self.vm.stack.len() { return self.throw_range_error("Maximum call stack size exceeded"); } - let argument_count = self.vm.read::(); - let rest_argument = self.vm.pop(); - let mut arguments = Vec::with_capacity(argument_count as usize); - for _ in 0..(argument_count - 1) { - arguments.push(self.vm.pop()); - } - arguments.reverse(); + + // Get the arguments that are stored as an array object on the stack. + let arguments_array = self.vm.pop(); + let arguments_array_object = arguments_array + .as_object() + .expect("arguments array in call spread function must be an object"); + let arguments = arguments_array_object + .borrow() + .properties() + .dense_indexed_properties() + .expect("arguments array in call spread function must be dense") + .clone(); + let func = self.vm.pop(); let this = self.vm.pop(); - let iterator_record = rest_argument.get_iterator(self, None, None)?; - let mut rest_arguments = Vec::new(); - while let Some(next) = iterator_record.step(self)? { - rest_arguments.push(next.value(self)?); - } - arguments.append(&mut rest_arguments); - let object = match func { JsValue::Object(ref object) if object.is_callable() => object.clone(), _ => return self.throw_type_error("not a callable function"), @@ -1802,27 +1798,26 @@ impl Context { self.vm.push(result); } - Opcode::CallWithRest => { + Opcode::CallSpread => { if self.vm.stack_size_limit <= self.vm.stack.len() { return self.throw_range_error("Maximum call stack size exceeded"); } - let argument_count = self.vm.read::(); - let rest_argument = self.vm.pop(); - let mut arguments = Vec::with_capacity(argument_count as usize); - for _ in 0..(argument_count - 1) { - arguments.push(self.vm.pop()); - } - arguments.reverse(); + + // Get the arguments that are stored as an array object on the stack. + let arguments_array = self.vm.pop(); + let arguments_array_object = arguments_array + .as_object() + .expect("arguments array in call spread function must be an object"); + let arguments = arguments_array_object + .borrow() + .properties() + .dense_indexed_properties() + .expect("arguments array in call spread function must be dense") + .clone(); + let func = self.vm.pop(); let this = self.vm.pop(); - let iterator_record = rest_argument.get_iterator(self, None, None)?; - let mut rest_arguments = Vec::new(); - while let Some(next) = iterator_record.step(self)? { - rest_arguments.push(next.value(self)?); - } - arguments.append(&mut rest_arguments); - let object = match func { JsValue::Object(ref object) if object.is_callable() => object.clone(), _ => return self.throw_type_error("not a callable function"), @@ -1851,25 +1846,23 @@ impl Context { self.vm.push(result); } - Opcode::NewWithRest => { + Opcode::NewSpread => { if self.vm.stack_size_limit <= self.vm.stack.len() { return self.throw_range_error("Maximum call stack size exceeded"); } - let argument_count = self.vm.read::(); - let rest_argument = self.vm.pop(); - let mut arguments = Vec::with_capacity(argument_count as usize); - for _ in 0..(argument_count - 1) { - arguments.push(self.vm.pop()); - } - arguments.reverse(); - let func = self.vm.pop(); + // Get the arguments that are stored as an array object on the stack. + let arguments_array = self.vm.pop(); + let arguments_array_object = arguments_array + .as_object() + .expect("arguments array in call spread function must be an object"); + let arguments = arguments_array_object + .borrow() + .properties() + .dense_indexed_properties() + .expect("arguments array in call spread function must be dense") + .clone(); - let iterator_record = rest_argument.get_iterator(self, None, None)?; - let mut rest_arguments = Vec::new(); - while let Some(next) = iterator_record.step(self)? { - rest_arguments.push(next.value(self)?); - } - arguments.append(&mut rest_arguments); + let func = self.vm.pop(); let result = func .as_constructor() diff --git a/boa_engine/src/vm/opcode.rs b/boa_engine/src/vm/opcode.rs index 72303c93590..2c594bb61e5 100644 --- a/boa_engine/src/vm/opcode.rs +++ b/boa_engine/src/vm/opcode.rs @@ -877,12 +877,12 @@ pub enum Opcode { /// Stack: argument_1, ... argument_n **=>** SuperCall, - /// Execute the `super()` method where the last argument is a rest parameter. + /// Execute the `super()` method where the arguments contain spreads. /// - /// Operands: argument_count: `u32` + /// Operands: /// - /// Stack: argument_1, ... argument_n **=>** - SuperCallWithRest, + /// Stack: arguments_array **=>** + SuperCallSpread, /// Execute the `super()` method when no constructor of the class is defined. /// @@ -934,12 +934,12 @@ pub enum Opcode { /// Stack: func, this, argument_1, ... argument_n **=>** result CallEval, - /// Call a function named "eval" where the last argument is a rest parameter. + /// Call a function named "eval" where the arguments contain spreads. /// - /// Operands: argument_count: `u32` + /// Operands: /// - /// Stack: func, this, argument_1, ... argument_n **=>** result - CallEvalWithRest, + /// Stack: arguments_array, func, this **=>** result + CallEvalSpread, /// Call a function. /// @@ -948,12 +948,12 @@ pub enum Opcode { /// Stack: func, this, argument_1, ... argument_n **=>** result Call, - /// Call a function where the last argument is a rest parameter. + /// Call a function where the arguments contain spreads. /// - /// Operands: argument_count: `u32` + /// Operands: /// - /// Stack: func, this, argument_1, ... argument_n **=>** result - CallWithRest, + /// Stack: arguments_array, func, this **=>** result + CallSpread, /// Call construct on a function. /// @@ -962,12 +962,12 @@ pub enum Opcode { /// Stack: func, argument_1, ... argument_n **=>** result New, - /// Call construct on a function where the last argument is a rest parameter. + /// Call construct on a function where the arguments contain spreads. /// - /// Operands: argument_count: `u32` + /// Operands: /// - /// Stack: func, argument_1, ... argument_n **=>** result - NewWithRest, + /// Stack: arguments_array, func **=>** result + NewSpread, /// Return from a function. /// @@ -1278,7 +1278,7 @@ impl Opcode { Self::This => "This", Self::Super => "Super", Self::SuperCall => "SuperCall", - Self::SuperCallWithRest => "SuperCallWithRest", + Self::SuperCallSpread => "SuperCallWithRest", Self::SuperCallDerived => "SuperCallDerived", Self::Case => "Case", Self::Default => "Default", @@ -1286,11 +1286,11 @@ impl Opcode { Self::GetFunctionAsync => "GetFunctionAsync", Self::GetGenerator => "GetGenerator", Self::CallEval => "CallEval", - Self::CallEvalWithRest => "CallEvalWithRest", + Self::CallEvalSpread => "CallEvalSpread", Self::Call => "Call", - Self::CallWithRest => "CallWithRest", + Self::CallSpread => "CallSpread", Self::New => "New", - Self::NewWithRest => "NewWithRest", + Self::NewSpread => "NewSpread", Self::Return => "Return", Self::PushDeclarativeEnvironment => "PushDeclarativeEnvironment", Self::PushFunctionEnvironment => "PushFunctionEnvironment", @@ -1418,7 +1418,7 @@ impl Opcode { Self::This => "INST - This", Self::Super => "INST - Super", Self::SuperCall => "INST - SuperCall", - Self::SuperCallWithRest => "INST - SuperCallWithRest", + Self::SuperCallSpread => "INST - SuperCallWithRest", Self::SuperCallDerived => "INST - SuperCallDerived", Self::Case => "INST - Case", Self::Default => "INST - Default", @@ -1426,11 +1426,11 @@ impl Opcode { Self::GetFunctionAsync => "INST - GetFunctionAsync", Self::GetGenerator => "INST - GetGenerator", Self::CallEval => "INST - CallEval", - Self::CallEvalWithRest => "INST - CallEvalWithRest", + Self::CallEvalSpread => "INST - CallEvalSpread", Self::Call => "INST - Call", - Self::CallWithRest => "INST - CallWithRest", + Self::CallSpread => "INST - CallSpread", Self::New => "INST - New", - Self::NewWithRest => "INST - NewWithRest", + Self::NewSpread => "INST - NewSpread", Self::Return => "INST - Return", Self::PushDeclarativeEnvironment => "INST - PushDeclarativeEnvironment", Self::PushFunctionEnvironment => "INST - PushFunctionEnvironment",