github
/
boa
mirror of https://github.com/boa-dev/boa.git
1
0
Fork 0
Code Issues Releases Wiki Activity
Browse Source

Fix try/catch/finally related bugs and add tests (#1901) 

<!---
Thank you for contributing to Boa! Please fill out the template below, and remove or add any
information as you feel neccesary.
--->

This Pull Request fixes some bugs related to try blocks:

- Fixes a panic when a finally block contained variable declarations. (Thanks to @VTCAKAVSMoACE for the report!)
- Fixes a bug where try blocks in the last position of a statement list didn't return its inner last value as the result of the evaluation.
- Add tests for both cases and two other common cases.
- Extract and cleanup some code.
pull/1895/head
jedel1043 3 years ago
parent
2a6ea9dad6
commit
9f9e36c910
3 changed files with 121 additions and 56 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 99
      boa_engine/src/bytecompiler.rs
  2. 6
      boa_engine/src/context/mod.rs
  3. 72
      boa_engine/src/vm/tests.rs

99
boa_engine/src/bytecompiler.rs
Unescape View File

@ -7,7 +7,7 @@ use crate::{
iteration::IterableLoopInitializer,
object::{MethodDefinition, PropertyDefinition, PropertyName},
template::TemplateElement,
Declaration, GetConstField, GetField, StatementList,
Declaration, GetConstField, GetField,
},
op::{AssignOp, BinOp, BitOp, CompOp, LogOp, NumOp, UnaryOp},
Const, Node,
@ -532,14 +532,12 @@ impl<'b> ByteCompiler<'b> {
}
#[inline]
pub fn compile_statement_list(&mut self, list: &StatementList, use_expr: bool) -> JsResult<()> {
for (i, node) in list.items().iter().enumerate() {
if i + 1 == list.items().len() {
self.compile_stmt(node, use_expr)?;
break;
pub fn compile_statement_list(&mut self, list: &[Node], use_expr: bool) -> JsResult<()> {
if let Some((last, items)) = list.split_last() {
for node in items {
self.compile_stmt(node, false)?;
}
self.compile_stmt(node, false)?;
self.compile_stmt(last, use_expr)?;
}
Ok(())
}
@ -1203,7 +1201,7 @@ impl<'b> ByteCompiler<'b> {
let jelse = self.jump_if_false();
if !matches!(node.body(), Node::Block(_)) {
self.create_declarations(node.body())?;
self.create_decls_from_stmt(node.body())?;
}
self.compile_stmt(node.body(), false)?;
@ -1216,7 +1214,7 @@ impl<'b> ByteCompiler<'b> {
let exit = self.jump();
self.patch_jump(jelse);
if !matches!(else_body, Node::Block(_)) {
self.create_declarations(else_body)?;
self.create_decls_from_stmt(else_body)?;
}
self.compile_stmt(else_body, false)?;
self.patch_jump(exit);
@ -1228,7 +1226,7 @@ impl<'b> ByteCompiler<'b> {
let push_env = self.jump_with_custom_opcode(Opcode::PushDeclarativeEnvironment);
if let Some(init) = for_loop.init() {
self.create_declarations(init)?;
self.create_decls_from_stmt(init)?;
self.compile_stmt(init, false)?;
}
@ -1253,7 +1251,7 @@ impl<'b> ByteCompiler<'b> {
let exit = self.jump_if_false();
if !matches!(for_loop.body(), Node::Block(_)) {
self.create_declarations(for_loop.body())?;
self.create_decls_from_stmt(for_loop.body())?;
}
self.compile_stmt(for_loop.body(), false)?;
@ -1580,12 +1578,8 @@ impl<'b> ByteCompiler<'b> {
Node::Block(block) => {
self.context.push_compile_time_environment(false);
let push_env = self.jump_with_custom_opcode(Opcode::PushDeclarativeEnvironment);
for node in block.items() {
self.create_declarations(node)?;
}
for node in block.items() {
self.compile_stmt(node, use_expr)?;
}
self.create_declarations(block.items())?;
self.compile_statement_list(block.items(), use_expr)?;
let num_bindings = self.context.pop_compile_time_environment().num_bindings();
self.patch_jump_with_target(push_env, num_bindings as u32);
self.emit_opcode(Opcode::PopEnvironment);
@ -1598,9 +1592,7 @@ impl<'b> ByteCompiler<'b> {
self.context.push_compile_time_environment(false);
let push_env = self.jump_with_custom_opcode(Opcode::PushDeclarativeEnvironment);
for case in switch.cases() {
for node in case.body().items() {
self.create_declarations(node)?;
}
self.create_declarations(case.body().items())?;
}
self.emit_opcode(Opcode::LoopStart);
@ -1618,17 +1610,13 @@ impl<'b> ByteCompiler<'b> {
for (label, case) in labels.into_iter().zip(switch.cases()) {
self.patch_jump(label);
self.compile_statement_list(case.body(), false)?;
self.compile_statement_list(case.body().items(), false)?;
}
self.patch_jump(exit);
if let Some(body) = switch.default() {
for node in body {
self.create_declarations(node)?;
}
for node in body {
self.compile_stmt(node, false)?;
}
self.create_declarations(body)?;
self.compile_statement_list(body, false)?;
}
self.pop_switch_control_info();
@ -1653,12 +1641,10 @@ impl<'b> ByteCompiler<'b> {
self.emit(Opcode::TryStart, &[Self::DUMMY_ADDRESS, 0]);
self.context.push_compile_time_environment(false);
let push_env = self.jump_with_custom_opcode(Opcode::PushDeclarativeEnvironment);
for node in t.block().items() {
self.create_declarations(node)?;
}
for node in t.block().items() {
self.compile_stmt(node, false)?;
}
self.create_declarations(t.block().items())?;
self.compile_statement_list(t.block().items(), use_expr)?;
let num_bindings = self.context.pop_compile_time_environment().num_bindings();
self.patch_jump_with_target(push_env, num_bindings as u32);
self.emit_opcode(Opcode::PopEnvironment);
@ -1693,12 +1679,10 @@ impl<'b> ByteCompiler<'b> {
} else {
self.emit_opcode(Opcode::Pop);
}
for node in catch.block().items() {
self.create_declarations(node)?;
}
for node in catch.block().items() {
self.compile_stmt(node, use_expr)?;
}
self.create_declarations(catch.block().items())?;
self.compile_statement_list(catch.block().items(), use_expr)?;
let num_bindings = self.context.pop_compile_time_environment().num_bindings();
self.patch_jump_with_target(push_env, num_bindings as u32);
self.emit_opcode(Opcode::PopEnvironment);
@ -1725,9 +1709,16 @@ impl<'b> ByteCompiler<'b> {
finally_start_address,
);
for node in finally.items() {
self.compile_stmt(node, false)?;
}
self.context.push_compile_time_environment(false);
let push_env = self.jump_with_custom_opcode(Opcode::PushDeclarativeEnvironment);
self.create_declarations(finally.items())?;
self.compile_statement_list(finally.items(), false)?;
let num_bindings = self.context.pop_compile_time_environment().num_bindings();
self.patch_jump_with_target(push_env, num_bindings as u32);
self.emit_opcode(Opcode::PopEnvironment);
self.emit_opcode(Opcode::FinallyEnd);
self.pop_try_control_info(Some(finally_start_address));
} else {
@ -1879,11 +1870,8 @@ impl<'b> ByteCompiler<'b> {
compiler.emit_opcode(Opcode::Yield);
}
for node in body.items() {
compiler.create_declarations(node)?;
}
compiler.compile_statement_list(body, false)?;
compiler.create_declarations(body.items())?;
compiler.compile_statement_list(body.items(), false)?;
if let Some(env_label) = env_label {
let num_bindings = compiler
@ -2160,7 +2148,14 @@ impl<'b> ByteCompiler<'b> {
Ok(())
}
pub(crate) fn create_declarations(&mut self, node: &Node) -> JsResult<bool> {
pub(crate) fn create_declarations(&mut self, stmt_list: &[Node]) -> JsResult<()> {
for node in stmt_list {
self.create_decls_from_stmt(node)?;
}
Ok(())
}
pub(crate) fn create_decls_from_stmt(&mut self, node: &Node) -> JsResult<bool> {
let mut has_identifier_argument = false;
match node {
@ -2257,17 +2252,17 @@ impl<'b> ByteCompiler<'b> {
}
Node::DoWhileLoop(do_while_loop) => {
if !matches!(do_while_loop.body(), Node::Block(_)) {
self.create_declarations(do_while_loop.body())?;
self.create_decls_from_stmt(do_while_loop.body())?;
}
}
Node::ForInLoop(for_in_loop) => {
if !matches!(for_in_loop.body(), Node::Block(_)) {
self.create_declarations(for_in_loop.body())?;
self.create_decls_from_stmt(for_in_loop.body())?;
}
}
Node::ForOfLoop(for_of_loop) => {
if !matches!(for_of_loop.body(), Node::Block(_)) {
self.create_declarations(for_of_loop.body())?;
self.create_decls_from_stmt(for_of_loop.body())?;
}
}
_ => {}

6
boa_engine/src/context/mod.rs
Unescape View File

@ -671,10 +671,8 @@ impl Context {
pub fn compile(&mut self, statement_list: &StatementList) -> JsResult<Gc<CodeBlock>> {
let _timer = Profiler::global().start_event("Compilation", "Main");
let mut compiler = ByteCompiler::new(Sym::MAIN, statement_list.strict(), self);
for node in statement_list.items() {
compiler.create_declarations(node)?;
}
compiler.compile_statement_list(statement_list, true)?;
compiler.create_declarations(statement_list.items())?;
compiler.compile_statement_list(statement_list.items(), true)?;
Ok(Gc::new(compiler.finish()))
}

72
boa_engine/src/vm/tests.rs
Unescape View File

@ -65,3 +65,75 @@ fn multiple_catches() {
Ok(JsValue::Undefined)
);
}
#[test]
fn use_last_expr_try_block() {
let source = r#"
try {
19;
7.5;
"Hello!";
} catch (y) {
14;
"Bye!"
}
"#;
assert_eq!(
Context::default().eval(source.as_bytes()),
Ok(JsValue::from("Hello!"))
);
}
#[test]
fn use_last_expr_catch_block() {
let source = r#"
try {
throw Error("generic error");
19;
7.5;
} catch (y) {
14;
"Hello!";
}
"#;
assert_eq!(
Context::default().eval(source.as_bytes()),
Ok(JsValue::from("Hello!"))
);
}
#[test]
fn no_use_last_expr_finally_block() {
let source = r#"
try {
} catch (y) {
} finally {
"Unused";
}
"#;
assert_eq!(
Context::default().eval(source.as_bytes()),
Ok(JsValue::undefined())
);
}
#[test]
fn finally_block_binding_env() {
let source = r#"
let buf = "Hey hey"
try {
} catch (y) {
} finally {
let x = " people";
buf += x;
}
buf
"#;
assert_eq!(
Context::default().eval(source.as_bytes()),
Ok(JsValue::from("Hey hey people"))
);
}

Write Preview
Loading…
Cancel
Save