Fix panics on empty return values (#3018)

boa_ast/src/operations.rs

@@ -2098,3 +2098,61 @@ impl<'ast> Visitor<'ast> for AnnexBFunctionDeclarationNamesVisitor<'_> {
         self.visit(node.statement())
     }
 }
+
+/// Returns `true` if the given statement returns a value.
+#[must_use]
+pub fn returns_value<'a, N>(node: &'a N) -> bool
+where
+    &'a N: Into<NodeRef<'a>>,
+{
+    ReturnsValueVisitor.visit(node.into()).is_break()
+}
+
+/// The [`Visitor`] used for [`returns_value`].
+#[derive(Debug)]
+struct ReturnsValueVisitor;
+
+impl<'ast> Visitor<'ast> for ReturnsValueVisitor {
+    type BreakTy = ();
+
+    fn visit_block(&mut self, node: &'ast crate::statement::Block) -> ControlFlow<Self::BreakTy> {
+        for statement in node.statement_list().statements() {
+            match statement {
+                StatementListItem::Declaration(_) => {}
+                StatementListItem::Statement(node) => try_break!(self.visit(node)),
+            }
+        }
+        ControlFlow::Continue(())
+    }
+
+    fn visit_statement(&mut self, node: &'ast Statement) -> ControlFlow<Self::BreakTy> {
+        match node {
+            Statement::Empty | Statement::Var(_) => {}
+            Statement::Block(node) => try_break!(self.visit(node)),
+            Statement::Labelled(node) => try_break!(self.visit(node)),
+            _ => return ControlFlow::Break(()),
+        }
+        ControlFlow::Continue(())
+    }
+
+    fn visit_case(&mut self, node: &'ast crate::statement::Case) -> ControlFlow<Self::BreakTy> {
+        for statement in node.body().statements() {
+            match statement {
+                StatementListItem::Declaration(_) => {}
+                StatementListItem::Statement(node) => try_break!(self.visit(node)),
+            }
+        }
+        ControlFlow::Continue(())
+    }
+
+    fn visit_labelled(
+        &mut self,
+        node: &'ast crate::statement::Labelled,
+    ) -> ControlFlow<Self::BreakTy> {
+        match node.item() {
+            LabelledItem::Statement(node) => try_break!(self.visit(node)),
+            LabelledItem::Function(_) => {}
+        }
+        ControlFlow::Continue(())
+    }
+}

boa_ast/src/statement/mod.rs

@@ -162,17 +162,6 @@ impl Statement {
             _ => false,
         }
     }
-
-    /// Returns `true` if the statement returns a value.
-    #[inline]
-    #[must_use]
-    pub const fn returns_value(&self) -> bool {
-        match self {
-            Self::Block(block) if block.statement_list().statements().is_empty() => false,
-            Self::Empty | Self::Var(_) | Self::Break(_) | Self::Continue(_) => false,
-            _ => true,
-        }
-    }
 }
 
 impl ToIndentedString for Statement {

boa_engine/src/bytecompiler/mod.rs

@@ -31,6 +31,7 @@ use boa_ast::{
         ArrowFunction, AsyncArrowFunction, AsyncFunction, AsyncGenerator, Class,
         FormalParameterList, Function, FunctionBody, Generator, PrivateName,
     },
+    operations::returns_value,
     pattern::Pattern,
     Declaration, Expression, Statement, StatementList, StatementListItem,
 };
@@ -734,34 +735,34 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> {
     /// Compile a [`StatementList`].
     pub fn compile_statement_list(&mut self, list: &StatementList, use_expr: bool, block: bool) {
         if use_expr {
-            let list_until_loop_exit: Vec<_> = list
-                .statements()
-                .iter()
-                .take_while(|item| {
-                    !matches!(
-                        item,
-                        StatementListItem::Statement(Statement::Break(_) | Statement::Continue(_))
-                    )
-                })
-                .collect();
-            let expr_index = list_until_loop_exit
-                .iter()
-                .rev()
-                .skip_while(|item| {
-                    matches!(
-                        item,
-                        &&StatementListItem::Statement(Statement::Empty | Statement::Var(_))
-                            | &&StatementListItem::Declaration(_)
-                    )
-                })
-                .count();
+            let mut has_returns_value = false;
+            let mut use_expr_index = 0;
+            let mut first_return_is_abrupt = false;
+            for (i, statement) in list.statements().iter().enumerate() {
+                match statement {
+                    StatementListItem::Statement(Statement::Break(_) | Statement::Continue(_)) => {
+                        if !has_returns_value {
+                            first_return_is_abrupt = true;
+                        }
+                        break;
+                    }
+                    StatementListItem::Statement(Statement::Empty | Statement::Var(_))
+                    | StatementListItem::Declaration(_) => {}
+                    StatementListItem::Statement(Statement::Block(block))
+                        if !returns_value(block) => {}
+                    StatementListItem::Statement(_) => {
+                        has_returns_value = true;
+                        use_expr_index = i;
+                    }
+                }
+            }
 
-            if expr_index == 0 && !list.statements().is_empty() {
+            if first_return_is_abrupt {
                 self.emit_opcode(Opcode::PushUndefined);
             }
 
             for (i, item) in list.statements().iter().enumerate() {
-                self.compile_stmt_list_item(item, i + 1 == expr_index, block);
+                self.compile_stmt_list_item(item, i == use_expr_index, block);
             }
         } else {
             for item in list.statements() {

boa_engine/src/bytecompiler/statement/break.rs

@@ -1,5 +1,5 @@
 use crate::{
-    bytecompiler::{ByteCompiler, Label},
+    bytecompiler::{ByteCompiler, JumpControlInfo},
     vm::Opcode,
 };
 use boa_ast::statement::Break;
@@ -29,10 +29,11 @@ impl ByteCompiler<'_, '_> {
             let (break_label, target_jump_label) = self.emit_opcode_with_two_operands(opcode);
 
             if let Some(node_label) = node.label() {
-                self.search_jump_info_label(target_jump_label, node_label);
+                let info = self.jump_info_label(node_label);
+                info.push_break_label(target_jump_label);
 
                 if !has_finally_or_is_finally {
-                    self.search_jump_info_label(break_label, node_label);
+                    info.push_break_label(break_label);
                     return;
                 }
             } else {
@@ -55,29 +56,32 @@ impl ByteCompiler<'_, '_> {
         // Emit the break opcode -> (Label, Label)
         let (break_label, target_label) = self.emit_opcode_with_two_operands(opcode);
         if let Some(label) = node.label() {
-            self.search_jump_info_label(break_label, label);
-            self.search_jump_info_label(target_label, label);
+            let info = self.jump_info_label(label);
+            info.push_break_label(break_label);
+            info.push_break_label(target_label);
             return;
         }
 
-        let info = self
-            .jump_info
-            .last_mut()
-            .expect("unlabeled break must be inside loop or switch");
-
+        let info = self.jump_info_non_labelled();
         info.push_break_label(break_label);
         info.push_break_label(target_label);
     }
 
-    fn search_jump_info_label(&mut self, address: Label, node_label: Sym) {
-        let mut found = false;
+    fn jump_info_non_labelled(&mut self) -> &mut JumpControlInfo {
+        for info in self.jump_info.iter_mut().rev() {
+            if !info.is_labelled() {
+                return info;
+            }
+        }
+        panic!("Jump info without label must exist");
+    }
+
+    fn jump_info_label(&mut self, label: Sym) -> &mut JumpControlInfo {
         for info in self.jump_info.iter_mut().rev() {
-            if info.label() == Some(node_label) {
-                info.push_break_label(address);
-                found = true;
-                break;
+            if info.label() == Some(label) {
+                return info;
             }
         }
-        assert!(found, "Cannot use the undeclared label");
+        panic!("Jump info with label must exist");
     }
 }

boa_engine/src/bytecompiler/statement/if.rs

@@ -1,12 +1,12 @@
 use crate::{bytecompiler::ByteCompiler, vm::Opcode};
-use boa_ast::statement::If;
+use boa_ast::{operations::returns_value, statement::If};
 
 impl ByteCompiler<'_, '_> {
     pub(crate) fn compile_if(&mut self, node: &If, use_expr: bool) {
         self.compile_expr(node.cond(), true);
         let jelse = self.jump_if_false();
 
-        if !node.body().returns_value() {
+        if !returns_value(node.body()) {
             self.emit_opcode(Opcode::PushUndefined);
         }
         self.compile_stmt(node.body(), true);
@@ -18,7 +18,7 @@ impl ByteCompiler<'_, '_> {
                 self.emit_opcode(Opcode::PushUndefined);
             }
             Some(else_body) => {
-                if !else_body.returns_value() {
+                if !returns_value(else_body) {
                     self.emit_opcode(Opcode::PushUndefined);
                 }
                 self.compile_stmt(else_body, true);

boa_engine/src/bytecompiler/statement/loop.rs

@@ -1,6 +1,6 @@
 use boa_ast::{
     declaration::Binding,
-    operations::bound_names,
+    operations::{bound_names, returns_value},
     statement::{
         iteration::{ForLoopInitializer, IterableLoopInitializer},
         DoWhileLoop, ForInLoop, ForLoop, ForOfLoop, WhileLoop,
@@ -95,7 +95,7 @@ impl ByteCompiler<'_, '_> {
         }
         let exit = self.jump_if_false();
 
-        if !for_loop.body().returns_value() {
+        if !returns_value(for_loop.body()) {
             self.emit_opcode(Opcode::PushUndefined);
         }
         self.compile_stmt(for_loop.body(), true);
@@ -229,7 +229,7 @@ impl ByteCompiler<'_, '_> {
             }
         }
 
-        if !for_in_loop.body().returns_value() {
+        if !returns_value(for_in_loop.body()) {
             self.emit_opcode(Opcode::PushUndefined);
         }
         self.compile_stmt(for_in_loop.body(), true);
@@ -375,7 +375,7 @@ impl ByteCompiler<'_, '_> {
             }
         }
 
-        if !for_of_loop.body().returns_value() {
+        if !returns_value(for_of_loop.body()) {
             self.emit_opcode(Opcode::PushUndefined);
         }
         self.compile_stmt(for_of_loop.body(), true);
@@ -416,7 +416,7 @@ impl ByteCompiler<'_, '_> {
         self.compile_expr(while_loop.condition(), true);
         let exit = self.jump_if_false();
 
-        if !while_loop.body().returns_value() {
+        if !returns_value(while_loop.body()) {
             self.emit_opcode(Opcode::PushUndefined);
         }
         self.compile_stmt(while_loop.body(), true);
@@ -454,7 +454,7 @@ impl ByteCompiler<'_, '_> {
 
         self.patch_jump(initial_label);
 
-        if !do_while_loop.body().returns_value() {
+        if !returns_value(do_while_loop.body()) {
             self.emit_opcode(Opcode::PushUndefined);
         }
         self.compile_stmt(do_while_loop.body(), true);

boa_engine/src/bytecompiler/statement/switch.rs

@@ -1,5 +1,5 @@
 use crate::{bytecompiler::ByteCompiler, vm::Opcode};
-use boa_ast::statement::Switch;
+use boa_ast::{operations::returns_value, statement::Switch};
 
 impl ByteCompiler<'_, '_> {
     /// Compile a [`Switch`] `boa_ast` node
@@ -44,7 +44,7 @@ impl ByteCompiler<'_, '_> {
             };
             self.patch_jump(label);
             self.compile_statement_list(case.body(), true, true);
-            if !case.body().statements().is_empty() {
+            if returns_value(case) {
                 self.emit_opcode(Opcode::LoopUpdateReturnValue);
             }
         }

boa_engine/src/bytecompiler/statement/try.rs

@@ -4,7 +4,7 @@ use crate::{
 };
 use boa_ast::{
     declaration::Binding,
-    operations::bound_names,
+    operations::{bound_names, returns_value},
     statement::{Catch, Finally, Try},
 };
 
@@ -21,7 +21,7 @@ impl ByteCompiler<'_, '_> {
         self.push_try_control_info(t.finally().is_some(), try_start);
 
         self.compile_block(t.block(), true);
-        if t.block().statement_list().statements().is_empty() {
+        if !returns_value(t.block()) {
             self.emit_opcode(Opcode::PushUndefined);
         }
         if !use_expr {
@@ -80,7 +80,7 @@ impl ByteCompiler<'_, '_> {
         }
 
         self.compile_block(catch.block(), true);
-        if catch.block().statement_list().statements().is_empty() {
+        if !returns_value(catch.block()) {
             self.emit_opcode(Opcode::PushUndefined);
         }
         if !use_expr {
@@ -103,7 +103,7 @@ impl ByteCompiler<'_, '_> {
 
     pub(crate) fn compile_finally_stmt(&mut self, finally: &Finally, finally_end_label: Label) {
         self.compile_block(finally.block(), true);
-        if !finally.block().statement_list().statements().is_empty() {
+        if returns_value(finally.block()) {
             self.emit_opcode(Opcode::Pop);
         }
 

boa_engine/src/bytecompiler/statement/with.rs

@@ -1,5 +1,5 @@
 use crate::{bytecompiler::ByteCompiler, vm::Opcode};
-use boa_ast::statement::With;
+use boa_ast::{operations::returns_value, statement::With};
 
 impl ByteCompiler<'_, '_> {
     /// Compile a [`With`] `boa_ast` node
@@ -8,7 +8,7 @@ impl ByteCompiler<'_, '_> {
         self.push_compile_environment(false);
         self.emit_opcode(Opcode::PushObjectEnvironment);
 
-        if !with.statement().returns_value() {
+        if !returns_value(with.statement()) {
             self.emit_opcode(Opcode::PushUndefined);
         }
         self.compile_stmt(with.statement(), true);

boa_engine/src/vm/tests.rs

@@ -324,3 +324,36 @@ fn arguments_object_constructor_valid_index() {
         "object",
     )]);
 }
+
+#[test]
+fn empty_return_values() {
+    run_test_actions([
+        TestAction::run(indoc! {r#"do {{}} while (false);"#}),
+        TestAction::run(indoc! {r#"do try {{}} catch {} while (false);"#}),
+        TestAction::run(indoc! {r#"do {} while (false);"#}),
+        TestAction::run(indoc! {r#"do try {{}{}} catch {} while (false);"#}),
+        TestAction::run(indoc! {r#"do {{}{}} while (false);"#}),
+        TestAction::run(indoc! {r#"do {;{}} while (false);"#}),
+        TestAction::run(indoc! {r#"do {e: {}} while (false);"#}),
+        TestAction::run(indoc! {r#"do {e: ;} while (false);"#}),
+        TestAction::run(indoc! {r#"do { break } while (false);"#}),
+        TestAction::run(indoc! {r#"while (true) a: break"#}),
+        TestAction::run(indoc! {r#"while (true) a: {"a"; break};"#}),
+        TestAction::run(indoc! {r#"do {"a";{}} while (false);"#}),
+        TestAction::run(indoc! {r#"
+            switch (false) {
+                default: {}
+            }
+        "#}),
+        TestAction::run(indoc! {r#"
+            switch (false) {
+                default: {}{}
+            }
+        "#}),
+        TestAction::run(indoc! {r#"
+            switch (false) {
+                default: ;{}{}
+            }
+        "#}),
+    ]);
+}