From be055a30e197ce44514354224ffe6199efc4d794 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Sat, 29 Jul 2023 23:22:36 +0200 Subject: [PATCH] Refactor environment, exception handling and jumping in VM (#3059) * Refactor environment, exception handling, generators and jumping in VM * Add helper for JumpIfNotResumeKind opcode * Better handler display for CodeBlock * Update documentaion * Factor exception handling from throw opcodes * Simplify try compilation * Only push try statement jump controll info if needed * Add helper functions checks in async and generator * Run prettier * Remove redundant check for end of bytecode. We always emit a `Return` opcode at the end of a function, so this should never be reached. * Fix doc link * Implement stack_count calculation of handlers * Fix typo * Rename `LoopContinue` to `IncrementLoopIteration` * Fix typo * Remove #[allow(unused)] from Handler field --- boa_engine/src/builtins/eval/mod.rs | 5 +- boa_engine/src/builtins/generator/mod.rs | 3 +- boa_engine/src/builtins/json/mod.rs | 5 +- boa_engine/src/bytecompiler/class.rs | 15 +- .../declaration/declaration_pattern.rs | 42 +- boa_engine/src/bytecompiler/declarations.rs | 1 + boa_engine/src/bytecompiler/env.rs | 2 + boa_engine/src/bytecompiler/expression/mod.rs | 25 +- boa_engine/src/bytecompiler/function.rs | 14 +- boa_engine/src/bytecompiler/jump_control.rs | 368 +++++++++++------- boa_engine/src/bytecompiler/mod.rs | 83 ++-- .../src/bytecompiler/statement/break.rs | 85 ++-- .../src/bytecompiler/statement/continue.rs | 135 ++----- .../src/bytecompiler/statement/labelled.rs | 9 +- boa_engine/src/bytecompiler/statement/loop.rs | 60 +-- boa_engine/src/bytecompiler/statement/mod.rs | 37 +- .../src/bytecompiler/statement/switch.rs | 5 - boa_engine/src/bytecompiler/statement/try.rs | 129 ++++-- boa_engine/src/bytecompiler/utils.rs | 49 ++- boa_engine/src/module/source.rs | 5 +- boa_engine/src/script.rs | 7 +- boa_engine/src/vm/call_frame/abrupt_record.rs | 89 ----- boa_engine/src/vm/call_frame/env_stack.rs | 201 ---------- boa_engine/src/vm/call_frame/mod.rs | 80 ++-- boa_engine/src/vm/code_block.rs | 153 ++++++-- boa_engine/src/vm/flowgraph/mod.rs | 240 ++++-------- boa_engine/src/vm/mod.rs | 94 +++-- boa_engine/src/vm/opcode/await_stm/mod.rs | 3 +- .../src/vm/opcode/control_flow/break.rs | 54 --- .../src/vm/opcode/control_flow/continue.rs | 67 ---- .../src/vm/opcode/control_flow/finally.rs | 148 ------- .../src/vm/opcode/control_flow/labelled.rs | 55 --- boa_engine/src/vm/opcode/control_flow/mod.rs | 10 - .../src/vm/opcode/control_flow/return.rs | 36 +- .../src/vm/opcode/control_flow/throw.rs | 215 +++------- boa_engine/src/vm/opcode/control_flow/try.rs | 68 ---- boa_engine/src/vm/opcode/generator/mod.rs | 81 ++-- .../src/vm/opcode/generator/yield_stm.rs | 7 +- .../src/vm/opcode/iteration/iterator.rs | 68 +++- .../src/vm/opcode/iteration/loop_ops.rs | 117 +----- boa_engine/src/vm/opcode/jump/mod.rs | 39 +- boa_engine/src/vm/opcode/mod.rs | 172 ++++---- boa_engine/src/vm/opcode/pop/mod.rs | 1 - boa_engine/src/vm/opcode/push/environment.rs | 2 - docs/boa_object.md | 29 +- docs/vm.md | 27 +- 46 files changed, 1223 insertions(+), 1917 deletions(-) delete mode 100644 boa_engine/src/vm/call_frame/abrupt_record.rs delete mode 100644 boa_engine/src/vm/call_frame/env_stack.rs delete mode 100644 boa_engine/src/vm/opcode/control_flow/break.rs delete mode 100644 boa_engine/src/vm/opcode/control_flow/continue.rs delete mode 100644 boa_engine/src/vm/opcode/control_flow/finally.rs delete mode 100644 boa_engine/src/vm/opcode/control_flow/labelled.rs delete mode 100644 boa_engine/src/vm/opcode/control_flow/try.rs diff --git a/boa_engine/src/builtins/eval/mod.rs b/boa_engine/src/builtins/eval/mod.rs index 9d7c4946fb..ab3e25b1b8 100644 --- a/boa_engine/src/builtins/eval/mod.rs +++ b/boa_engine/src/builtins/eval/mod.rs @@ -253,7 +253,10 @@ impl Eval { context.vm.environments.extend_outer_function_environment(); } - context.vm.push_frame(CallFrame::new(code_block)); + let env_fp = context.vm.environments.len() as u32; + context + .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(); diff --git a/boa_engine/src/builtins/generator/mod.rs b/boa_engine/src/builtins/generator/mod.rs index f2fc866555..cfca602d79 100644 --- a/boa_engine/src/builtins/generator/mod.rs +++ b/boa_engine/src/builtins/generator/mod.rs @@ -108,10 +108,11 @@ impl GeneratorContext { context .vm .push_frame(self.call_frame.take().expect("should have a call frame")); - context.vm.frame_mut().generator_resume_kind = resume_kind; + if let Some(value) = value { context.vm.push(value); } + context.vm.push(resume_kind); let result = context.run(); diff --git a/boa_engine/src/builtins/json/mod.rs b/boa_engine/src/builtins/json/mod.rs index 20b80fef10..67fd9a41bc 100644 --- a/boa_engine/src/builtins/json/mod.rs +++ b/boa_engine/src/builtins/json/mod.rs @@ -128,7 +128,10 @@ impl Json { Gc::new(compiler.finish()) }; - context.vm.push_frame(CallFrame::new(code_block)); + let env_fp = context.vm.environments.len() as u32; + context + .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(); diff --git a/boa_engine/src/bytecompiler/class.rs b/boa_engine/src/bytecompiler/class.rs index 65d5a53b48..81ee0bf846 100644 --- a/boa_engine/src/bytecompiler/class.rs +++ b/boa_engine/src/bytecompiler/class.rs @@ -69,14 +69,12 @@ impl ByteCompiler<'_, '_> { compiler.pop_compile_environment(); compiler.code_block_flags |= CodeBlockFlags::IS_CLASS_CONSTRUCTOR; } + compiler.emit_opcode(Opcode::SetReturnValue); if class.name().is_some() && class.has_binding_identifier() { compiler.pop_compile_environment(); } - compiler.emit_opcode(Opcode::SetReturnValue); - compiler.emit_opcode(Opcode::Return); - let code = Gc::new(compiler.finish()); let index = self.functions.len() as u32; self.functions.push(code); @@ -268,12 +266,11 @@ impl ByteCompiler<'_, '_> { } else { field_compiler.emit_opcode(Opcode::PushUndefined); } + field_compiler.emit_opcode(Opcode::SetReturnValue); + field_compiler.pop_compile_environment(); field_compiler.pop_compile_environment(); - field_compiler.emit_opcode(Opcode::SetReturnValue); - field_compiler.emit_opcode(Opcode::Return); - field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; let code = field_compiler.finish(); @@ -306,7 +303,6 @@ impl ByteCompiler<'_, '_> { field_compiler.pop_compile_environment(); field_compiler.emit_opcode(Opcode::SetReturnValue); - field_compiler.emit_opcode(Opcode::Return); field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; @@ -346,12 +342,11 @@ impl ByteCompiler<'_, '_> { } else { field_compiler.emit_opcode(Opcode::PushUndefined); } + field_compiler.emit_opcode(Opcode::SetReturnValue); + field_compiler.pop_compile_environment(); field_compiler.pop_compile_environment(); - field_compiler.emit_opcode(Opcode::SetReturnValue); - field_compiler.emit_opcode(Opcode::Return); - field_compiler.code_block_flags |= CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER; let code = field_compiler.finish(); diff --git a/boa_engine/src/bytecompiler/declaration/declaration_pattern.rs b/boa_engine/src/bytecompiler/declaration/declaration_pattern.rs index 9801c63bd3..17a9bd4c12 100644 --- a/boa_engine/src/bytecompiler/declaration/declaration_pattern.rs +++ b/boa_engine/src/bytecompiler/declaration/declaration_pattern.rs @@ -180,11 +180,39 @@ impl ByteCompiler<'_, '_> { self.emit_opcode(Opcode::ValueNotNullOrUndefined); self.emit_opcode(Opcode::GetIterator); + // TODO: maybe, refactor this to be more condensed. + let handler_index = self.push_handler(); for element in pattern.bindings() { self.compile_array_pattern_element(element, def); } + self.emit_opcode(Opcode::PushFalse); + + let exit = self.jump(); + self.patch_handler(handler_index); + self.emit_opcode(Opcode::Exception); + self.emit_opcode(Opcode::PushTrue); + self.patch_jump(exit); + + 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); } } } @@ -198,15 +226,15 @@ impl ByteCompiler<'_, '_> { match element { // ArrayBindingPattern : [ Elision ] Elision => { - self.emit_opcode(Opcode::IteratorNext); + self.emit_opcode(Opcode::IteratorNextWithoutPop); } // SingleNameBinding : BindingIdentifier Initializer[opt] SingleName { ident, default_init, } => { - self.emit_opcode(Opcode::IteratorNext); - self.emit_opcode(Opcode::IteratorValue); + self.emit_opcode(Opcode::IteratorNextWithoutPop); + self.emit_opcode(Opcode::IteratorValueWithoutPop); if let Some(init) = default_init { let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined); self.compile_expr(init, true); @@ -216,8 +244,8 @@ impl ByteCompiler<'_, '_> { } PropertyAccess { access } => { self.access_set(Access::Property { access }, false, |compiler, _level| { - compiler.emit_opcode(Opcode::IteratorNext); - compiler.emit_opcode(Opcode::IteratorValue); + compiler.emit_opcode(Opcode::IteratorNextWithoutPop); + compiler.emit_opcode(Opcode::IteratorValueWithoutPop); }); } // BindingElement : BindingPattern Initializer[opt] @@ -225,8 +253,8 @@ impl ByteCompiler<'_, '_> { pattern, default_init, } => { - self.emit_opcode(Opcode::IteratorNext); - self.emit_opcode(Opcode::IteratorValue); + self.emit_opcode(Opcode::IteratorNextWithoutPop); + self.emit_opcode(Opcode::IteratorValueWithoutPop); if let Some(init) = default_init { let skip = self.emit_opcode_with_operand(Opcode::JumpIfNotUndefined); diff --git a/boa_engine/src/bytecompiler/declarations.rs b/boa_engine/src/bytecompiler/declarations.rs index 08ba3d6794..67dc318058 100644 --- a/boa_engine/src/bytecompiler/declarations.rs +++ b/boa_engine/src/bytecompiler/declarations.rs @@ -1015,6 +1015,7 @@ impl ByteCompiler<'_, '_> { // Don't need to use `AsyncGeneratorYield` since // we just want to stop the execution of the generator. self.emit_opcode(Opcode::GeneratorYield); + self.emit_opcode(Opcode::Pop); } // 27. If hasParameterExpressions is false, then diff --git a/boa_engine/src/bytecompiler/env.rs b/boa_engine/src/bytecompiler/env.rs index fb3be0b9e7..c817b5b794 100644 --- a/boa_engine/src/bytecompiler/env.rs +++ b/boa_engine/src/bytecompiler/env.rs @@ -7,6 +7,7 @@ use boa_ast::expression::Identifier; impl ByteCompiler<'_, '_> { /// Push either a new declarative or function environment on the compile time environment stack. pub(crate) fn push_compile_environment(&mut self, function_scope: bool) { + self.current_open_environments_count += 1; self.current_environment = Rc::new(CompileTimeEnvironment::new( self.current_environment.clone(), function_scope, @@ -16,6 +17,7 @@ impl ByteCompiler<'_, '_> { /// Pops the top compile time environment and returns its index in the compile time environments array. #[track_caller] pub(crate) fn pop_compile_environment(&mut self) -> u32 { + self.current_open_environments_count -= 1; let index = self.compile_environments.len() as u32; self.compile_environments .push(self.current_environment.clone()); diff --git a/boa_engine/src/bytecompiler/expression/mod.rs b/boa_engine/src/bytecompiler/expression/mod.rs index ef118a5cfa..34d072c8eb 100644 --- a/boa_engine/src/bytecompiler/expression/mod.rs +++ b/boa_engine/src/bytecompiler/expression/mod.rs @@ -7,7 +7,7 @@ mod update; use super::{Access, Callable, NodeKind}; use crate::{ bytecompiler::{ByteCompiler, Literal}, - vm::Opcode, + vm::{GeneratorResumeKind, Opcode}, }; use boa_ast::{ expression::{ @@ -153,31 +153,41 @@ impl ByteCompiler<'_, '_> { } } Expression::Yield(r#yield) => { + // stack: if let Some(expr) = r#yield.target() { self.compile_expr(expr, true); } else { self.emit_opcode(Opcode::PushUndefined); } + // stack: value + if r#yield.delegate() { - if self.in_async_generator { + if self.in_async() { self.emit_opcode(Opcode::GetAsyncIterator); } else { self.emit_opcode(Opcode::GetIterator); } + // stack: self.emit_opcode(Opcode::PushUndefined); + + // stack: undefined + self.emit_push_integer(GeneratorResumeKind::Normal as i32); + + // stack: resume_kind, undefined let start_address = self.next_opcode_location(); let (throw_method_undefined, return_method_undefined) = self.emit_opcode_with_two_operands(Opcode::GeneratorDelegateNext); - if self.in_async_generator { + if self.in_async() { + self.emit_opcode(Opcode::Pop); self.emit_opcode(Opcode::Await); } let (return_gen, exit) = self.emit_opcode_with_two_operands(Opcode::GeneratorDelegateResume); - if self.in_async_generator { + if self.in_async() { self.emit_opcode(Opcode::IteratorValue); self.async_generator_yield(); } else { @@ -188,16 +198,17 @@ impl ByteCompiler<'_, '_> { self.patch_jump(return_gen); self.patch_jump(return_method_undefined); - if self.in_async_generator { + if self.in_async() { self.emit_opcode(Opcode::Await); + self.emit_opcode(Opcode::Pop); } self.close_active_iterators(); self.emit_opcode(Opcode::SetReturnValue); - self.emit_opcode(Opcode::GeneratorResumeReturn); + self.r#return(); self.patch_jump(throw_method_undefined); - self.iterator_close(self.in_async_generator); + self.iterator_close(self.in_async()); self.emit_opcode(Opcode::Throw); self.patch_jump(exit); diff --git a/boa_engine/src/bytecompiler/function.rs b/boa_engine/src/bytecompiler/function.rs index f55b7ff4fb..9ceffde007 100644 --- a/boa_engine/src/bytecompiler/function.rs +++ b/boa_engine/src/bytecompiler/function.rs @@ -4,7 +4,7 @@ use crate::{ builtins::function::ThisMode, bytecompiler::ByteCompiler, environments::CompileTimeEnvironment, - vm::{CodeBlock, CodeBlockFlags, Opcode}, + vm::{CodeBlock, CodeBlockFlags}, Context, }; use boa_ast::function::{FormalParameterList, FunctionBody}; @@ -99,7 +99,8 @@ impl FunctionCompiler { let mut compiler = ByteCompiler::new(self.name, self.strict, false, outer_env, context); compiler.length = length; - compiler.in_async_generator = self.generator && self.r#async; + compiler.in_async = self.r#async; + compiler.in_generator = self.generator; if self.arrow { compiler.this_mode = ThisMode::Lexical; @@ -151,15 +152,6 @@ impl FunctionCompiler { compiler.params = parameters.clone(); - if compiler - .bytecode - .last() - .filter(|last| **last == Opcode::Return as u8) - .is_none() - { - compiler.emit_opcode(Opcode::Return); - } - Gc::new(compiler.finish()) } } diff --git a/boa_engine/src/bytecompiler/jump_control.rs b/boa_engine/src/bytecompiler/jump_control.rs index 0c86fbd0ba..d531eab6ec 100644 --- a/boa_engine/src/bytecompiler/jump_control.rs +++ b/boa_engine/src/bytecompiler/jump_control.rs @@ -9,38 +9,157 @@ //! [try spec]: https://tc39.es/ecma262/#sec-try-statement //! [labelled spec]: https://tc39.es/ecma262/#sec-labelled-statements -use crate::bytecompiler::{ByteCompiler, Label}; +use crate::{ + bytecompiler::{ByteCompiler, Label}, + vm::{Handler, Opcode}, +}; use bitflags::bitflags; use boa_interner::Sym; +/// An actions to be performed for the local control flow. +#[derive(Debug, Clone, Copy)] +pub(crate) enum JumpRecordAction { + /// Places a [`Opcode::Jump`], transfers to a specified [`JumpControlInfo`] to be handled when it gets poped. + Transfer { + /// [`JumpControlInfo`] index to be transferred. + index: u32, + }, + + /// Places [`Opcode::PopEnvironment`] opcodes, `count` times. + PopEnvironments { count: u32 }, + + /// Closes the an iterator. + CloseIterator { r#async: bool }, + + /// Handles finally, this needs to be done if we are in the try or catch section of a try statement that + /// has a finally block. + /// + /// It places push integer value [`Opcode`] as well as [`Opcode::PushFalse`], which means don't [`ReThrow`](Opcode::ReThrow). + /// + /// The integer is an index used to jump. See [`Opcode::JumpTable`]. This is needed because the following code: + /// + /// ```JavaScript + /// do { + /// try { + /// if (cond) { + /// continue; + /// } + /// + /// break; + /// } finally { + /// // Must execute the finally, even if `continue` is executed or `break` is executed. + /// } + /// } while (true) + /// ``` + /// + /// Both `continue` and `break` must go through the finally, but the `continue` goes to the beginning of the loop, + /// and the `break` goes to the end of the loop, this is solved by having a jump table (See [`Opcode::JumpTable`]) + /// at the end of finally (It is constructed in [`ByteCompiler::pop_try_with_finally_control_info()`]). + HandleFinally { + /// Jump table index. + index: u32, + }, +} + +/// Local Control flow type. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum JumpRecordKind { + Break, + Continue, + Return, +} + +/// This represents a local control flow handling. See [`JumpRecordKind`] for types. +#[derive(Debug, Clone)] +pub(crate) struct JumpRecord { + kind: JumpRecordKind, + label: Label, + actions: Vec, +} + +impl JumpRecord { + pub(crate) const fn new(kind: JumpRecordKind, actions: Vec) -> Self { + Self { + kind, + label: ByteCompiler::DUMMY_LABEL, + actions, + } + } + + /// Performs the [`JumpRecordAction`]s. + pub(crate) fn perform_actions( + mut self, + start_address: u32, + compiler: &mut ByteCompiler<'_, '_>, + ) { + while let Some(action) = self.actions.pop() { + match action { + JumpRecordAction::Transfer { index } => { + self.label = compiler.jump(); + compiler.jump_info[index as usize].jumps.push(self); + + // Don't continue actions, let the delegate jump control info handle it! + return; + } + JumpRecordAction::PopEnvironments { count } => { + for _ in 0..count { + compiler.emit_opcode(Opcode::PopEnvironment); + } + } + JumpRecordAction::HandleFinally { index: value } => { + // Note: +1 because 0 is reserved for default entry in jump table (for fallthrough). + let index = value as i32 + 1; + compiler.emit_push_integer(index); + compiler.emit_opcode(Opcode::PushFalse); + } + JumpRecordAction::CloseIterator { r#async } => { + compiler.iterator_close(r#async); + } + } + } + + // If there are no actions left, finalize the jump record. + match self.kind { + JumpRecordKind::Break => compiler.patch_jump(self.label), + JumpRecordKind::Continue => compiler.patch_jump_with_target(self.label, start_address), + JumpRecordKind::Return => compiler.emit_opcode(Opcode::Return), + } + } +} + /// Boa's `ByteCompiler` jump information tracking struct. #[derive(Debug, Clone)] pub(crate) struct JumpControlInfo { label: Option, start_address: u32, - flags: JumpControlInfoFlags, - set_jumps: Vec