Use main stack for calling ordinary functions (#3185)

* Use main stack for calling

- Move `return_value` to stack

* Move return_value to VM

* Apply review

boa_engine/src/builtins/eval/mod.rs

@@ -258,6 +258,7 @@ impl Eval {
             .vm
             .push_frame(CallFrame::new(code_block).with_env_fp(env_fp));
         context.realm().resize_global_env();
+
         let record = context.run();
         context.vm.pop_frame();
 

boa_engine/src/builtins/generator/mod.rs

@@ -85,13 +85,18 @@ impl GeneratorContext {
 
     /// Creates a new `GeneratorContext` from the current `Context` state.
     pub(crate) fn from_current(context: &mut Context<'_>) -> Self {
-        Self {
+        let fp = context.vm.frame().fp as usize;
+        let this = Self {
             environments: context.vm.environments.clone(),
             call_frame: Some(context.vm.frame().clone()),
-            stack: context.vm.stack.clone(),
+            stack: context.vm.stack[fp..].to_vec(),
             active_function: context.vm.active_function.clone(),
             realm: context.realm().clone(),
-        }
+        };
+
+        context.vm.stack.truncate(fp);
+
+        this
     }
 
     /// Resumes execution with `GeneratorContext` as the current execution context.
@@ -109,6 +114,8 @@ impl GeneratorContext {
             .vm
             .push_frame(self.call_frame.take().expect("should have a call frame"));
 
+        context.vm.frame_mut().fp = 0;
+
         if let Some(value) = value {
             context.vm.push(value);
         }

boa_engine/src/bytecompiler/declaration/declaration_pattern.rs

@@ -191,30 +191,26 @@ impl ByteCompiler<'_, '_> {
                     self.compile_array_pattern_element(element, def);
                 }
 
+                let no_exception_thrown = self.jump();
                 self.patch_handler(handler_index);
                 self.emit_opcode(Opcode::MaybeException);
 
                 // stack: hasPending, exception?
 
                 self.current_stack_value_count += 2;
-
                 let iterator_close_handler = self.push_handler();
                 self.iterator_close(false);
-
-                let exit = self.jump();
                 self.patch_handler(iterator_close_handler);
                 self.current_stack_value_count -= 2;
-                {
-                    let jump = self.jump_if_false();
-                    self.emit_opcode(Opcode::Throw);
-                    self.patch_jump(jump);
-                }
-                self.emit_opcode(Opcode::ReThrow);
-                self.patch_jump(exit);
 
                 let jump = self.jump_if_false();
                 self.emit_opcode(Opcode::Throw);
                 self.patch_jump(jump);
+                self.emit_opcode(Opcode::ReThrow);
+
+                self.patch_jump(no_exception_thrown);
+
+                self.iterator_close(false);
             }
         }
     }

boa_engine/src/bytecompiler/expression/mod.rs

@@ -204,8 +204,7 @@ impl ByteCompiler<'_, '_> {
                     }
                     self.close_active_iterators();
 
-                    self.emit_opcode(Opcode::SetReturnValue);
-                    self.r#return();
+                    self.r#return(true);
 
                     self.patch_jump(throw_method_undefined);
                     self.iterator_close(self.in_async());

boa_engine/src/bytecompiler/function.rs

@@ -159,6 +159,7 @@ impl FunctionCompiler {
         // Note: We do handle exceptions thrown by generator body in `AsyncGeneratorStart`.
         if compiler.in_generator() {
             assert!(compiler.async_handler.is_none());
+
             if compiler.in_async() {
                 // Patched in `ByteCompiler::finish()`.
                 compiler.async_handler = Some(compiler.push_handler());

boa_engine/src/bytecompiler/jump_control.rs

@@ -66,7 +66,7 @@ pub(crate) enum JumpRecordAction {
 pub(crate) enum JumpRecordKind {
     Break,
     Continue,
-    Return,
+    Return { return_value_on_stack: bool },
 }
 
 /// This represents a local control flow handling. See [`JumpRecordKind`] for types.
@@ -122,7 +122,13 @@ impl JumpRecord {
         match self.kind {
             JumpRecordKind::Break => compiler.patch_jump(self.label),
             JumpRecordKind::Continue => compiler.patch_jump_with_target(self.label, start_address),
-            JumpRecordKind::Return => {
+            JumpRecordKind::Return {
+                return_value_on_stack,
+            } => {
+                if return_value_on_stack {
+                    compiler.emit_opcode(Opcode::SetReturnValue);
+                }
+
                 match (compiler.in_async(), compiler.in_generator()) {
                     // Taken from:
                     //  - 27.6.3.2 AsyncGeneratorStart ( generator, generatorBody ): https://tc39.es/ecma262/#sec-asyncgeneratorstart
@@ -137,6 +143,7 @@ impl JumpRecord {
                     (true, false) => compiler.emit_opcode(Opcode::CompletePromiseCapability),
                     (_, _) => {}
                 }
+
                 compiler.emit_opcode(Opcode::Return);
             }
         }

boa_engine/src/bytecompiler/mod.rs

@@ -315,6 +315,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
             params: FormalParameterList::default(),
             compile_environments: Vec::default(),
             current_open_environments_count: 0,
+
             current_stack_value_count: 0,
             code_block_flags,
             handlers: ThinVec::default(),
@@ -1496,7 +1497,7 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
         if let Some(async_handler) = self.async_handler {
             self.patch_handler(async_handler);
         }
-        self.r#return();
+        self.r#return(false);
 
         let name = self
             .context

boa_engine/src/bytecompiler/statement/mod.rs

@@ -72,9 +72,8 @@ impl ByteCompiler<'_, '_> {
                 } else {
                     self.emit_opcode(Opcode::PushUndefined);
                 }
-                self.emit_opcode(Opcode::SetReturnValue);
 
-                self.r#return();
+                self.r#return(true);
             }
             Statement::Try(t) => self.compile_try(t, use_expr),
             Statement::Expression(expr) => {
@@ -88,10 +87,16 @@ impl ByteCompiler<'_, '_> {
         }
     }
 
-    pub(crate) fn r#return(&mut self) {
+    pub(crate) fn r#return(&mut self, return_value_on_stack: bool) {
         let actions = self.return_jump_record_actions();
 
-        JumpRecord::new(JumpRecordKind::Return, actions).perform_actions(Self::DUMMY_ADDRESS, self);
+        JumpRecord::new(
+            JumpRecordKind::Return {
+                return_value_on_stack,
+            },
+            actions,
+        )
+        .perform_actions(Self::DUMMY_ADDRESS, self);
     }
 
     fn return_jump_record_actions(&self) -> Vec<JumpRecordAction> {

boa_engine/src/vm/call_frame/mod.rs

@@ -36,11 +36,6 @@ pub struct CallFrame {
 
     /// How many iterations a loop has done.
     pub(crate) loop_iteration_count: u64,
-
-    /// The value that is returned from the function.
-    //
-    // TODO(HalidOdat): Remove this and put into the stack, maybe before frame pointer.
-    pub(crate) return_value: JsValue,
 }
 
 /// ---- `CallFrame` public API ----
@@ -68,7 +63,6 @@ impl CallFrame {
             iterators: ThinVec::new(),
             binding_stack: Vec::new(),
             loop_iteration_count: 0,
-            return_value: JsValue::undefined(),
         }
     }
 

boa_engine/src/vm/code_block.rs

@@ -1235,22 +1235,7 @@ impl JsObject {
         }
 
         let argument_count = args.len();
-
-        // Push function arguments to the stack.
-        let mut args = if code.params.as_ref().len() > args.len() {
-            let mut v = args.to_vec();
-            v.extend(vec![
-                JsValue::Undefined;
-                code.params.as_ref().len() - args.len()
-            ]);
-            v
-        } else {
-            args.to_vec()
-        };
-        args.reverse();
-        let mut stack = args;
-
-        std::mem::swap(&mut context.vm.stack, &mut stack);
+        let parameters_count = code.params.as_ref().len();
 
         let frame = CallFrame::new(code)
             .with_argument_count(argument_count as u32)
@@ -1260,14 +1245,19 @@ impl JsObject {
 
         context.vm.push_frame(frame);
 
+        // Push function arguments to the stack.
+        for _ in argument_count..parameters_count {
+            context.vm.push(JsValue::undefined());
+        }
+        context.vm.stack.extend(args.iter().rev().cloned());
+
         let result = context
             .run()
             .consume()
             .map_err(|err| err.inject_realm(context.realm().clone()));
 
         context.vm.pop_frame().expect("frame must exist");
         std::mem::swap(&mut environments, &mut context.vm.environments);
-        std::mem::swap(&mut context.vm.stack, &mut stack);
         std::mem::swap(&mut context.vm.active_runnable, &mut script_or_module);
 
         result
@@ -1442,22 +1432,7 @@ impl JsObject {
                 }
 
                 let argument_count = args.len();
-
-                // Push function arguments to the stack.
-                let args = if code.params.as_ref().len() > args.len() {
-                    let mut v = args.to_vec();
-                    v.extend(vec![
-                        JsValue::Undefined;
-                        code.params.as_ref().len() - args.len()
-                    ]);
-                    v
-                } else {
-                    args.to_vec()
-                };
-
-                for arg in args.iter().rev() {
-                    context.vm.push(arg.clone());
-                }
+                let parameters_count = code.params.as_ref().len();
 
                 let has_binding_identifier = code.has_binding_identifier();
 
@@ -1469,6 +1444,12 @@ impl JsObject {
                         .with_env_fp(environments_len as u32),
                 );
 
+                // Push function arguments to the stack.
+                for _ in argument_count..parameters_count {
+                    context.vm.push(JsValue::undefined());
+                }
+                context.vm.stack.extend(args.iter().rev().cloned());
+
                 let record = context.run();
 
                 context.vm.pop_frame();

boa_engine/src/vm/mod.rs

@@ -53,6 +53,7 @@ mod tests;
 pub struct Vm {
     pub(crate) frames: Vec<CallFrame>,
     pub(crate) stack: Vec<JsValue>,
+    pub(crate) return_value: JsValue,
 
     /// When an error is thrown, the pending exception is set.
     ///
@@ -94,6 +95,7 @@ impl Vm {
         Self {
             frames: Vec::with_capacity(16),
             stack: Vec::with_capacity(1024),
+            return_value: JsValue::undefined(),
             environments: EnvironmentStack::new(global),
             pending_exception: None,
             runtime_limits: RuntimeLimits::default(),
@@ -181,6 +183,14 @@ impl Vm {
 
         true
     }
+
+    pub(crate) fn get_return_value(&self) -> JsValue {
+        self.return_value.clone()
+    }
+
+    pub(crate) fn set_return_value(&mut self, value: JsValue) {
+        self.return_value = value;
+    }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -329,7 +339,7 @@ impl Context<'_> {
                 Ok(CompletionType::Normal) => {}
                 Ok(CompletionType::Return) => {
                     self.vm.stack.truncate(self.vm.frame().fp as usize);
-                    let execution_result = self.vm.frame_mut().return_value.clone();
+                    let execution_result = std::mem::take(&mut self.vm.return_value);
                     return CompletionRecord::Normal(execution_result);
                 }
                 Ok(CompletionType::Throw) => {

boa_engine/src/vm/opcode/await/mod.rs

@@ -189,14 +189,16 @@ impl Operation for CompletePromiseCapability {
                 .call(&JsValue::undefined(), &[error.to_opaque(context)], context)
                 .expect("cannot fail per spec");
         } else {
-            let return_value = context.vm.frame().return_value.clone();
+            let return_value = context.vm.get_return_value();
             promise_capability
                 .resolve()
                 .call(&JsValue::undefined(), &[return_value], context)
                 .expect("cannot fail per spec");
         };
 
-        context.vm.frame_mut().return_value = promise_capability.promise().clone().into();
+        context
+            .vm
+            .set_return_value(promise_capability.promise().clone().into());
 
         Ok(CompletionType::Normal)
     }

boa_engine/src/vm/opcode/control_flow/return.rs

@@ -31,7 +31,7 @@ impl Operation for GetReturnValue {
     const INSTRUCTION: &'static str = "INST - GetReturnValue";
 
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
-        let value = context.vm.frame().return_value.clone();
+        let value = context.vm.get_return_value();
         context.vm.push(value);
         Ok(CompletionType::Normal)
     }
@@ -50,7 +50,7 @@ impl Operation for SetReturnValue {
 
     fn execute(context: &mut Context<'_>) -> JsResult<CompletionType> {
         let value = context.vm.pop();
-        context.vm.frame_mut().return_value = value;
+        context.vm.set_return_value(value);
         Ok(CompletionType::Normal)
     }
 }

boa_engine/src/vm/opcode/generator/mod.rs

@@ -16,7 +16,7 @@ use crate::{
         opcode::{Operation, ReThrow},
         CallFrame, CompletionType,
     },
-    Context, JsError, JsObject, JsResult,
+    Context, JsError, JsObject, JsResult, JsValue,
 };
 
 pub(crate) use yield_stm::*;
@@ -41,7 +41,14 @@ impl Operation for Generator {
         let pc = context.vm.frame().pc;
         let mut dummy_call_frame = CallFrame::new(code_block);
         dummy_call_frame.pc = pc;
-        let call_frame = std::mem::replace(context.vm.frame_mut(), dummy_call_frame);
+        let mut call_frame = std::mem::replace(context.vm.frame_mut(), dummy_call_frame);
+
+        let fp = call_frame.fp as usize;
+
+        let stack = context.vm.stack[fp..].to_vec();
+        context.vm.stack.truncate(fp);
+
+        call_frame.fp = 0;
 
         let this_function_object = context
             .vm
@@ -69,7 +76,6 @@ impl Operation for Generator {
             &mut context.vm.environments,
             EnvironmentStack::new(global_environement),
         );
-        let stack = std::mem::take(&mut context.vm.stack);
 
         let data = if r#async {
             ObjectData::async_generator(AsyncGenerator {
@@ -154,7 +160,8 @@ impl Operation for AsyncGeneratorClose {
             .expect("must have item in queue");
         drop(generator_object_mut);
 
-        let return_value = std::mem::take(&mut context.vm.frame_mut().return_value);
+        let return_value = context.vm.get_return_value();
+        context.vm.set_return_value(JsValue::undefined());
 
         if let Some(error) = context.vm.pending_exception.take() {
             AsyncGenerator::complete_step(&next, Err(error), true, None, context);