Skip to content

Commit

Permalink
Refactor call frame access to avoid panic checks (#3888)
Browse files Browse the repository at this point in the history
  • Loading branch information
raskad authored Jul 3, 2024
1 parent bb2d028 commit 961d7b4
Show file tree
Hide file tree
Showing 14 changed files with 125 additions and 114 deletions.
5 changes: 3 additions & 2 deletions core/engine/src/builtins/generator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,9 @@ impl GeneratorContext {
let rp = frame.rp;
context.vm.push_frame(frame);

context.vm.frame_mut().rp = rp;
context.vm.frame_mut().set_exit_early(true);
let frame = context.vm.frame_mut();
frame.rp = rp;
frame.set_exit_early(true);

if let Some(value) = value {
context.vm.push(value);
Expand Down
10 changes: 5 additions & 5 deletions core/engine/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,10 @@ impl Context {
// 1. If the execution context stack is empty, return null.
// 2. Let ec be the topmost execution context on the execution context stack whose ScriptOrModule component is not null.
// 3. If no such execution context exists, return null. Otherwise, return ec's ScriptOrModule.
if let Some(active_runnable) = &self.vm.frame.active_runnable {
return Some(active_runnable.clone());
}

self.vm
.frames
.iter()
Expand All @@ -846,11 +850,7 @@ impl Context {
return self.vm.native_active_function.clone();
}

if let Some(frame) = self.vm.frames.last() {
return frame.function(&self.vm);
}

None
self.vm.frame.function(&self.vm)
}
}

Expand Down
4 changes: 1 addition & 3 deletions core/engine/src/module/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1772,9 +1772,7 @@ impl SourceTextModule {

context
.vm
.frames
.last()
.expect("there should be a frame")
.frame
.set_promise_capability(&mut context.vm.stack, capability);

// 9. If module.[[HasTLA]] is false, then
Expand Down
12 changes: 10 additions & 2 deletions core/engine/src/object/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,11 @@ impl JsObject {
return Ok(context.vm.pop());
}

context.vm.frames[frame_index].set_exit_early(true);
if frame_index + 1 == context.vm.frames.len() {
context.vm.frame.set_exit_early(true);
} else {
context.vm.frames[frame_index + 1].set_exit_early(true);
}

let result = context.run().consume();

Expand Down Expand Up @@ -461,7 +465,11 @@ impl JsObject {
.clone());
}

context.vm.frames[frame_index].set_exit_early(true);
if frame_index + 1 == context.vm.frames.len() {
context.vm.frame.set_exit_early(true);
} else {
context.vm.frames[frame_index + 1].set_exit_early(true);
}

let result = context.run().consume();

Expand Down
127 changes: 73 additions & 54 deletions core/engine/src/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ use crate::{
Context, JsError, JsNativeError, JsObject, JsResult, JsValue, Module,
};

use boa_gc::{custom_trace, Finalize, Trace};
use boa_gc::{custom_trace, Finalize, Gc, Trace};
use boa_profiler::Profiler;
use boa_string::JsString;
use std::{future::Future, mem::size_of, ops::ControlFlow, pin::Pin, task};

#[cfg(feature = "trace")]
Expand Down Expand Up @@ -52,7 +53,18 @@ mod tests;
/// Virtual Machine.
#[derive(Debug)]
pub struct Vm {
/// The current call frame.
///
/// Whenever a new frame is pushed, it will be swaped into this field.
/// Then the old frame will get pushed to the [`Self::frames`] stack.
/// Whenever the current frame gets poped, the last frame on the [`Self::frames`] stack will be swaped into this field.
///
/// By default this is a dummy frame that gets pushed to [`Self::frames`] when the first real frame is pushed.
pub(crate) frame: CallFrame,

/// The stack for call frames.
pub(crate) frames: Vec<CallFrame>,

pub(crate) stack: Vec<JsValue>,
pub(crate) return_value: JsValue,

Expand Down Expand Up @@ -100,6 +112,12 @@ impl Vm {
pub(crate) fn new(realm: Realm) -> Self {
Self {
frames: Vec::with_capacity(16),
frame: CallFrame::new(
Gc::new(CodeBlock::new(JsString::default(), 0, true)),
None,
EnvironmentStack::new(realm.environment().clone()),
realm.clone(),
),
stack: Vec::with_capacity(1024),
return_value: JsValue::undefined(),
environments: EnvironmentStack::new(realm.environment().clone()),
Expand Down Expand Up @@ -132,29 +150,22 @@ impl Vm {

#[track_caller]
pub(crate) fn read<T: Readable>(&mut self) -> T {
let value = self.frame().code_block.read::<T>(self.frame().pc as usize);
self.frame_mut().pc += size_of::<T>() as u32;
let frame = self.frame_mut();
let value = frame.code_block.read::<T>(frame.pc as usize);
frame.pc += size_of::<T>() as u32;
value
}

/// Retrieves the VM frame
///
/// # Panics
///
/// If there is no frame, then this will panic.
/// Retrieves the VM frame.
#[track_caller]
pub(crate) fn frame(&self) -> &CallFrame {
self.frames.last().expect("no frame found")
&self.frame
}

/// Retrieves the VM frame mutably
///
/// # Panics
///
/// If there is no frame, then this will panic.
/// Retrieves the VM frame mutably.
#[track_caller]
pub(crate) fn frame_mut(&mut self) -> &mut CallFrame {
self.frames.last_mut().expect("no frame found")
&mut self.frame
}

pub(crate) fn push_frame(&mut self, mut frame: CallFrame) {
Expand All @@ -177,9 +188,12 @@ impl Vm {
// Keep carrying the last active runnable in case the current callframe
// yields.
if frame.active_runnable.is_none() {
frame.active_runnable = self.frames.last().and_then(|fr| fr.active_runnable.clone());
frame
.active_runnable
.clone_from(&self.frame.active_runnable);
}

std::mem::swap(&mut self.frame, &mut frame);
self.frames.push(frame);
}

Expand All @@ -196,13 +210,14 @@ impl Vm {
}

pub(crate) fn pop_frame(&mut self) -> Option<CallFrame> {
let mut frame = self.frames.pop();
if let Some(frame) = &mut frame {
if let Some(mut frame) = self.frames.pop() {
std::mem::swap(&mut self.frame, &mut frame);
std::mem::swap(&mut self.environments, &mut frame.environments);
std::mem::swap(&mut self.realm, &mut frame.realm);
Some(frame)
} else {
None
}

frame
}

/// Handles an exception thrown at position `pc`.
Expand Down Expand Up @@ -271,16 +286,17 @@ impl Context {
const NUMBER_OF_COLUMNS: usize = 4;

pub(crate) fn trace_call_frame(&self) {
let msg = if self.vm.frames.last().is_some() {
let frame = self.vm.frame();
let msg = if self.vm.frames.is_empty() {
" VM Start ".to_string()
} else {
format!(
" Call Frame -- {} ",
self.vm.frame().code_block().name().to_std_string_escaped()
frame.code_block().name().to_std_string_escaped()
)
} else {
" VM Start ".to_string()
};

println!("{}", self.vm.frame().code_block);
println!("{}", frame.code_block);
println!(
"{msg:-^width$}",
width = Self::COLUMN_WIDTH * Self::NUMBER_OF_COLUMNS - 10
Expand All @@ -300,16 +316,13 @@ impl Context {
where
F: FnOnce(Opcode, &mut Context) -> JsResult<CompletionType>,
{
let bytecodes = &self.vm.frame().code_block.bytecode;
let pc = self.vm.frame().pc as usize;
let frame = self.vm.frame();
let bytecodes = &frame.code_block.bytecode;
let pc = frame.pc as usize;
let (_, varying_operand_kind, instruction) = InstructionIterator::with_pc(bytecodes, pc)
.next()
.expect("There should be an instruction left");
let operands = self
.vm
.frame()
.code_block
.instruction_operands(&instruction);
let operands = frame.code_block.instruction_operands(&instruction);

let opcode = instruction.opcode();
match opcode {
Expand All @@ -332,12 +345,11 @@ impl Context {
let result = self.execute_instruction(f);
let duration = instant.elapsed();

let fp = self
.vm
.frames
.last()
.map(CallFrame::fp)
.map(|fp| fp as usize);
let fp = if self.vm.frames.is_empty() {
None
} else {
Some(self.vm.frame.fp() as usize)
};

let stack = {
let mut stack = String::from("[ ");
Expand Down Expand Up @@ -434,14 +446,16 @@ impl Context {
if !err.is_catchable() {
let mut fp = self.vm.stack.len();
let mut env_fp = self.vm.environments.len();
while let Some(frame) = self.vm.frames.last() {
if frame.exit_early() {
loop {
if self.vm.frame.exit_early() {
break;
}

fp = frame.fp() as usize;
env_fp = frame.env_fp as usize;
self.vm.pop_frame();
fp = self.vm.frame.fp() as usize;
env_fp = self.vm.frame.env_fp as usize;
if self.vm.pop_frame().is_none() {
break;
}
}
self.vm.environments.truncate(env_fp);
self.vm.stack.truncate(fp);
Expand All @@ -466,21 +480,24 @@ impl Context {
match result {
CompletionType::Normal => {}
CompletionType::Return => {
let fp = self.vm.frame().fp() as usize;
let frame = self.vm.frame();
let fp = frame.fp() as usize;
let exit_early = frame.exit_early();
self.vm.stack.truncate(fp);

let result = self.vm.take_return_value();
if self.vm.frame().exit_early() {
if exit_early {
return ControlFlow::Break(CompletionRecord::Normal(result));
}

self.vm.push(result);
self.vm.pop_frame();
}
CompletionType::Throw => {
let mut fp = self.vm.frame().fp();
let mut env_fp = self.vm.frame().env_fp;
if self.vm.frame().exit_early() {
let frame = self.vm.frame();
let mut fp = frame.fp();
let mut env_fp = frame.env_fp;
if frame.exit_early() {
self.vm.environments.truncate(env_fp as usize);
self.vm.stack.truncate(fp as usize);
return ControlFlow::Break(CompletionRecord::Throw(
Expand All @@ -493,11 +510,11 @@ impl Context {

self.vm.pop_frame();

while let Some(frame) = self.vm.frames.last_mut() {
fp = frame.fp();
env_fp = frame.env_fp;
let pc = frame.pc;
let exit_early = frame.exit_early();
loop {
fp = self.vm.frame.fp();
env_fp = self.vm.frame.env_fp;
let pc = self.vm.frame.pc;
let exit_early = self.vm.frame.exit_early();

if self.vm.handle_exception_at(pc) {
return ControlFlow::Continue(());
Expand All @@ -512,7 +529,9 @@ impl Context {
));
}

self.vm.pop_frame();
if self.vm.pop_frame().is_none() {
break;
}
}
self.vm.environments.truncate(env_fp as usize);
self.vm.stack.truncate(fp as usize);
Expand Down
9 changes: 4 additions & 5 deletions core/engine/src/vm/opcode/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,13 @@ impl Operation for CreateMappedArgumentsObject {
const COST: u8 = 8;

fn execute(context: &mut Context) -> JsResult<CompletionType> {
let function_object = context
.vm
.frame()
let frame = context.vm.frame();
let function_object = frame
.function(&context.vm)
.clone()
.expect("there should be a function object");
let code = context.vm.frame().code_block().clone();
let args = context.vm.frame().arguments(&context.vm).to_vec();
let code = frame.code_block().clone();
let args = frame.arguments(&context.vm).to_vec();

let env = context.vm.environments.current_ref();
let arguments = MappedArguments::new(
Expand Down
4 changes: 1 addition & 3 deletions core/engine/src/vm/opcode/await/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@ impl Operation for CreatePromiseCapability {

context
.vm
.frames
.last()
.expect("there should be a frame")
.frame
.set_promise_capability(&mut context.vm.stack, Some(&promise_capability));
Ok(CompletionType::Normal)
}
Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/vm/opcode/control_flow/return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ impl Operation for CheckReturn {
const COST: u8 = 3;

fn execute(context: &mut Context) -> JsResult<CompletionType> {
if !context.vm.frame().construct() {
let frame = context.vm.frame();
if !frame.construct() {
return Ok(CompletionType::Normal);
}
let frame = context.vm.frame();
let this = frame.this(&context.vm);
let result = context.vm.take_return_value();

Expand Down
10 changes: 4 additions & 6 deletions core/engine/src/vm/opcode/define/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,11 @@ pub(crate) struct DefInitVar;
impl DefInitVar {
fn operation(context: &mut Context, index: usize) -> JsResult<CompletionType> {
let value = context.vm.pop();
let mut binding_locator = context.vm.frame().code_block.bindings[index].clone();
let frame = context.vm.frame();
let strict = frame.code_block.strict();
let mut binding_locator = frame.code_block.bindings[index].clone();
context.find_runtime_binding(&mut binding_locator)?;
context.set_binding(
&binding_locator,
value,
context.vm.frame().code_block.strict(),
)?;
context.set_binding(&binding_locator, value, strict)?;

Ok(CompletionType::Normal)
}
Expand Down
Loading

0 comments on commit 961d7b4

Please sign in to comment.