Skip to content

Commit

Permalink
Review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
nekevss committed Feb 3, 2023
1 parent 0833494 commit d6f3f44
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 171 deletions.
21 changes: 12 additions & 9 deletions boa_engine/src/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,9 @@ impl Context<'_> {
self.vm.frame_mut().env_stack.pop();
}

for _ in 0..env_to_pop {
self.realm.environments.pop();
}
let env_truncation_len =
self.realm.environments.len().saturating_sub(env_to_pop);
self.realm.environments.truncate(env_truncation_len);

if target_address == catch_target {
self.vm.frame_mut().pc = catch_target as usize;
Expand Down Expand Up @@ -378,13 +378,16 @@ impl Context<'_> {
self.vm.frame_mut().env_stack.pop();
}

for _ in 0..env_to_pop {
self.realm.environments.pop();
}
let env_truncation_len =
self.realm.environments.len().saturating_sub(env_to_pop);
self.realm.environments.truncate(env_truncation_len);

for _ in 0..self.vm.frame().pop_on_return {
self.vm.pop();
}
let previous_stack_size = self
.vm
.stack
.len()
.saturating_sub(self.vm.frame().pop_on_return);
self.vm.stack.truncate(previous_stack_size);
self.vm.frame_mut().pop_on_return = 0;

let record = AbruptCompletionRecord::default().with_throw_flag();
Expand Down
37 changes: 13 additions & 24 deletions boa_engine/src/vm/opcode/control_flow/break.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,23 @@ impl Operation for Break {

// 1. Iterate through Env stack looking for exit address.
let mut envs_to_pop = 0;
for _ in 0..context.vm.frame().env_stack.len() {
let env_entry = context
.vm
.frame_mut()
.env_stack
.last()
.expect("EnvStackEntry must exist");

if (jump_address <= env_entry.exit_address())
while let Some(env_entry) = context.vm.frame().env_stack.last() {
if (jump_address == env_entry.exit_address())
|| (env_entry.is_finally_env() && jump_address == env_entry.start_address())
{
break;
}

if jump_address <= env_entry.exit_address() {
// Checks for the break if we have jumped from inside of a finally block
if jump_address == env_entry.exit_address() {
break;
}
envs_to_pop += env_entry.env_num();
context.vm.frame_mut().env_stack.pop();
}

for _ in 0..envs_to_pop {
context.realm.environments.pop();
}
let env_truncation_len = context.realm.environments.len().saturating_sub(envs_to_pop);
context.realm.environments.truncate(env_truncation_len);

// 2. Register target address in AbruptCompletionRecord.
let new_record = AbruptCompletionRecord::default()
Expand Down Expand Up @@ -77,31 +70,27 @@ impl Operation for Continue {

// 1. Iterate through Env stack looking for exit address.
let mut envs_to_pop = 0;
for _ in 0..context.vm.frame().env_stack.len() {
let env_entry = context
.vm
.frame_mut()
.env_stack
.last()
.expect("EnvStackEntry must exist");

while let Some(env_entry) = context.vm.frame_mut().env_stack.last() {
// We check two conditions here where continue actually jumps to a higher address.
// 1. When we have reached a finally env that matches the jump address we are moving to.
// 2. When there is no finally, and we have reached the continue location.
if (env_entry.is_finally_env() && jump_address == env_entry.start_address())
|| (jump_address == target_address && jump_address == env_entry.start_address())
{
break;
}

envs_to_pop += env_entry.env_num();
// The below check determines whether we have continued from inside of a finally block.
if jump_address > target_address && jump_address == env_entry.exit_address() {
context.vm.frame_mut().env_stack.pop();
break;
}
context.vm.frame_mut().env_stack.pop();
}

for _ in 0..envs_to_pop {
context.realm.environments.pop();
}
let env_truncation_len = context.realm.environments.len().saturating_sub(envs_to_pop);
context.realm.environments.truncate(env_truncation_len);

// 2. Register target address in AbruptCompletionRecord.
let new_record = AbruptCompletionRecord::default()
Expand Down
43 changes: 18 additions & 25 deletions boa_engine/src/vm/opcode/control_flow/catch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,16 @@ impl Operation for CatchEnd {
fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
context.vm.frame_mut().try_catch.pop();
let mut envs_to_pop = 0_usize;
for _ in 1..context.vm.frame().env_stack.len() {
let env_entry = context
.vm
.frame_mut()
.env_stack
.pop()
.expect("this must exist");

while let Some(env_entry) = context.vm.frame_mut().env_stack.pop() {
envs_to_pop += env_entry.env_num();

if env_entry.is_catch_env() {
break;
}
}

for _ in 0..envs_to_pop {
context.realm.environments.pop();
}
let env_truncation_len = context.realm.environments.len().saturating_sub(envs_to_pop);
context.realm.environments.truncate(env_truncation_len);

context.vm.frame_mut().finally_return = FinallyReturn::None;
Ok(ShouldExit::False)
Expand All @@ -79,25 +72,25 @@ impl Operation for CatchEnd2 {
const INSTRUCTION: &'static str = "INST - CatchEnd2";

fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
let frame = context.vm.frame_mut();
if frame
if let Some(catch_entry) = context
.vm
.frame()
.env_stack
.last()
.expect("Env stack entry must exist")
.is_catch_env()
.filter(|entry| entry.is_catch_env())
{
let catch_entry = frame
.env_stack
.pop()
.expect("must exist as catch env entry");

for _ in 0..catch_entry.env_num() {
context.realm.environments.pop();
}
let env_truncation_len = context
.realm
.environments
.len()
.saturating_sub(catch_entry.env_num());
context.realm.environments.truncate(env_truncation_len);

context.vm.frame_mut().env_stack.pop();
}

if frame.finally_return == FinallyReturn::Err {
frame.finally_return = FinallyReturn::None;
if context.vm.frame_mut().finally_return == FinallyReturn::Err {
context.vm.frame_mut().finally_return = FinallyReturn::None;
}
Ok(ShouldExit::False)
}
Expand Down
81 changes: 29 additions & 52 deletions boa_engine/src/vm/opcode/control_flow/finally.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,35 +60,23 @@ impl Operation for FinallyEnd {
if next_finally < record.target() {
context.vm.frame_mut().pc = next_finally as usize;

for _ in 0..context.vm.frame().env_stack.len() {
let env_entry = context
.vm
.frame_mut()
.env_stack
.last()
.expect("Environment stack entries must exist");

while let Some(env_entry) = context.vm.frame().env_stack.last() {
if next_finally <= env_entry.exit_address() {
break;
}

envs_to_pop += env_entry.env_num();
context.vm.frame_mut().env_stack.pop();
}
} else if record.is_break() && context.vm.frame().pc < record.target() as usize
{
// handle the continuation of an abrupt break.
context.vm.frame_mut().pc = record.target() as usize;
for _ in 0..context.vm.frame().env_stack.len() {
let env_entry = context
.vm
.frame_mut()
.env_stack
.last()
.expect("Environment stack entries must exist");

while let Some(env_entry) = context.vm.frame().env_stack.last() {
if record.target() == env_entry.exit_address() {
break;
}

envs_to_pop += env_entry.env_num();
context.vm.frame_mut().env_stack.pop();
}
Expand All @@ -99,14 +87,7 @@ impl Operation for FinallyEnd {
{
// Handle the continuation of an abrupt continue
context.vm.frame_mut().pc = record.target() as usize;
for _ in 0..context.vm.frame().env_stack.len() {
let env_entry = context
.vm
.frame_mut()
.env_stack
.last()
.expect("EnvStackEntry must exist");

while let Some(env_entry) = context.vm.frame().env_stack.last() {
if env_entry.start_address() == record.target() {
break;
}
Expand All @@ -117,9 +98,9 @@ impl Operation for FinallyEnd {
context.vm.frame_mut().abrupt_completion = None;
}

for _ in 0..envs_to_pop {
context.realm.environments.pop();
}
let env_truncation_len =
context.realm.environments.len().saturating_sub(envs_to_pop);
context.realm.environments.truncate(env_truncation_len);
} else {
context.vm.frame_mut().env_stack.pop();
}
Expand All @@ -133,32 +114,19 @@ impl Operation for FinallyEnd {
if next_finally < record.target() {
context.vm.frame_mut().pc = next_finally as usize;

for _ in 0..context.vm.frame().env_stack.len() {
let env_entry = context
.vm
.frame_mut()
.env_stack
.last()
.expect("Environment stack entries must exist");

while let Some(env_entry) = context.vm.frame().env_stack.last() {
if next_finally <= env_entry.exit_address() {
break;
}

envs_to_pop += env_entry.env_num();
context.vm.frame_mut().env_stack.pop();
}
} else if record.is_throw_with_target()
&& context.vm.frame().pc < record.target() as usize
{
context.vm.frame_mut().pc = record.target() as usize;
for _ in 0..context.vm.frame().env_stack.len() {
let env_entry = context
.vm
.frame_mut()
.env_stack
.pop()
.expect("EnvStackEntry must exist");

while let Some(env_entry) = context.vm.frame_mut().env_stack.pop() {
envs_to_pop += env_entry.env_num();
if env_entry.start_address() == record.target() {
break;
Expand All @@ -173,15 +141,20 @@ impl Operation for FinallyEnd {
.pop()
.expect("Popping current finally stack.");

for _ in 0..current_stack.env_num() {
context.realm.environments.pop();
}
let env_truncation_len = context
.realm
.environments
.len()
.saturating_sub(current_stack.env_num());
context.realm.environments.truncate(env_truncation_len);

return Err(JsError::from_opaque(context.vm.pop()));
}

for _ in 0..envs_to_pop {
context.realm.environments.pop();
}
let env_truncation_len =
context.realm.environments.len().saturating_sub(envs_to_pop);
context.realm.environments.truncate(env_truncation_len);

return Ok(ShouldExit::False);
}
let current_stack = context
Expand All @@ -191,9 +164,13 @@ impl Operation for FinallyEnd {
.pop()
.expect("Popping current finally stack.");

for _ in 0..current_stack.env_num() {
context.realm.environments.pop();
}
let env_truncation_len = context
.realm
.environments
.len()
.saturating_sub(current_stack.env_num());
context.realm.environments.truncate(env_truncation_len);

Err(JsError::from_opaque(context.vm.pop()))
}
}
Expand Down
20 changes: 7 additions & 13 deletions boa_engine/src/vm/opcode/control_flow/labelled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,20 @@ impl Operation for LabelledStart {

fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
let start = context.vm.frame().pc as u32 - 1;
let finally = context.vm.read::<u32>();
let end = context.vm.read::<u32>();
context
.vm
.frame_mut()
.env_stack
.push(EnvStackEntry::new(start, finally).with_labelled_flag());
.push(EnvStackEntry::new(start, end).with_labelled_flag());
Ok(ShouldExit::False)
}
}

/// `LabelledEnd` implements the Opcode Operation for `Opcode::LabelledEnd`
///
/// Operation:
/// - Clean up enviroments at the end of labelled block.
/// - Clean up environments at the end of labelled block.
#[derive(Debug, Clone, Copy)]
pub(crate) struct LabelledEnd;

Expand All @@ -39,23 +39,17 @@ impl Operation for LabelledEnd {

fn execute(context: &mut Context<'_>) -> JsResult<ShouldExit> {
let mut envs_to_pop = 0_usize;
for _ in 1..context.vm.frame().env_stack.len() {
let env_entry = context
.vm
.frame_mut()
.env_stack
.pop()
.expect("this must exist");
while let Some(env_entry) = context.vm.frame_mut().env_stack.pop() {
envs_to_pop += env_entry.env_num();

if env_entry.is_labelled_env() {
break;
}
}

for _ in 0..envs_to_pop {
context.realm.environments.pop();
}
let env_truncation_len = context.realm.environments.len().saturating_sub(envs_to_pop);
context.realm.environments.truncate(env_truncation_len);

Ok(ShouldExit::False)
}
}
Loading

0 comments on commit d6f3f44

Please sign in to comment.