LibJS: Save and restore exceptions on yields in finalizers

Also removes a bunch of related old FIXMEs.

Author: Hendiadyoin1

Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp

@@ -1782,6 +1782,12 @@ static void generate_yield(Bytecode::Generator& generator,
 
 Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> YieldExpression::generate_bytecode(Bytecode::Generator& generator, [[maybe_unused]] Optional<Bytecode::Operand> preferred_dst) const
 {
+    // Note: We need to catch any scheduled exceptions and reschedule them on re-entry
+    //       as the act of yielding would otherwise clear them out
+    //       This only applies when we are in a finalizer
+    bool is_in_finalizer = generator.is_in_finalizer();
+    Optional<Bytecode::Register> saved_exception;
+
     Bytecode::Generator::SourceLocationScope scope(generator, *this);
     VERIFY(generator.is_in_generator_function());
 
@@ -1890,6 +1896,11 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> YieldExpression::ge
             auto current_value = Bytecode::Operand(generator.allocate_register());
             generator.emit_iterator_value(current_value, inner_result);
 
+            if (is_in_finalizer) {
+                saved_exception = generator.allocate_register();
+                generator.emit<Bytecode::Op::Mov>(Bytecode::Operand(*saved_exception), Bytecode::Operand(Bytecode::Register::exception()));
+            }
+
             generate_yield(generator,
                 Bytecode::Label { continuation_block },
                 current_value,
@@ -2077,6 +2088,10 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> YieldExpression::ge
         generate_yield(generator, Bytecode::Label { continuation_block }, received, received_completion, received_completion_type, received_completion_value, type_identifier, value_identifier, AwaitBeforeYield::No);
 
         generator.switch_to_basic_block(continuation_block);
+
+        if (is_in_finalizer)
+            generator.emit<Bytecode::Op::Mov>(Bytecode::Operand(Bytecode::Register::exception()), Bytecode::Operand(*saved_exception));
+
         generator.emit<Bytecode::Op::Mov>(received_completion, Bytecode::Operand(Bytecode::Register(0)));
         get_received_completion_type_and_value(generator, received_completion, received_completion_type, received_completion_value, type_identifier, value_identifier);
         generator.emit<Bytecode::Op::Jump>(Bytecode::Label { loop_block });
@@ -2092,8 +2107,18 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> YieldExpression::ge
         argument = generator.add_constant(js_undefined());
 
     auto& continuation_block = generator.make_block();
+
+    if (is_in_finalizer) {
+        saved_exception = generator.allocate_register();
+        generator.emit<Bytecode::Op::Mov>(Bytecode::Operand(*saved_exception), Bytecode::Operand(Bytecode::Register::exception()));
+    }
+
     generate_yield(generator, Bytecode::Label { continuation_block }, *argument, received_completion, received_completion_type, received_completion_value, type_identifier, value_identifier, AwaitBeforeYield::Yes);
     generator.switch_to_basic_block(continuation_block);
+
+    if (is_in_finalizer)
+        generator.emit<Bytecode::Op::Mov>(Bytecode::Operand(Bytecode::Register::exception()), Bytecode::Operand(*saved_exception));
+
     generator.emit<Bytecode::Op::Mov>(received_completion, Bytecode::Operand(Bytecode::Register(0)));
     get_received_completion_type_and_value(generator, received_completion, received_completion_type, received_completion_value, type_identifier, value_identifier);
 
@@ -2190,9 +2215,6 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> IfStatement::genera
 Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> ContinueStatement::generate_bytecode(Bytecode::Generator& generator, [[maybe_unused]] Optional<Bytecode::Operand> preferred_dst) const
 {
     Bytecode::Generator::SourceLocationScope scope(generator, *this);
-    // FIXME: Handle finally blocks in a graceful manner
-    //        We need to execute the finally block, but tell it to resume
-    //        execution at the designated block
     if (!m_target_label.has_value()) {
         generator.generate_continue();
         return Optional<Bytecode::Operand> {};

Userland/Libraries/LibJS/Bytecode/Generator.h

@@ -238,6 +238,8 @@ class Generator {
         }
     }
 
+    bool is_in_finalizer() const { return m_boundaries.contains_slow(BlockBoundaryType::LeaveFinally); }
+
     void generate_break();
     void generate_break(DeprecatedFlyString const& break_label);
 

Userland/Libraries/LibJS/Bytecode/Op.h

@@ -1595,13 +1595,6 @@ class ScheduleJump final : public Instruction {
 public:
     // Note: We use this instruction to tell the next `finally` block to
     //       continue execution with a specific break/continue target;
-    // FIXME: We currently don't clear the interpreter internal flag, when we change
-    //        the control-flow (`break`, `continue`) in a finally-block,
-    // FIXME: .NET on x86_64 uses a call to the finally instead, which could make this
-    //        easier, at the cost of making control-flow changes (`break`, `continue`, `return`)
-    //        in the finally-block more difficult, but as stated above, those
-    //        aren't handled 100% correctly at the moment anyway
-    //        It might be worth investigating a similar mechanism
     constexpr static bool IsTerminator = true;
 
     ScheduleJump(Label target)

Userland/Libraries/LibJS/Tests/try-return-finally.js

@@ -24,3 +24,17 @@ test("return from outer finally with nested unwind contexts", () => {
 
     expect(value).toBe(2);
 });
+
+test("restore exception after generator yield in finally", () => {
+    let generator = (function* () {
+        try {
+            throw new Error("foo");
+        } finally {
+            yield 42;
+        }
+    })();
+
+    expect(generator.next().value).toBe(42);
+    expect(() => generator.next()).toThrowWithMessage(Error, "foo");
+    expect(generator.next().done).toBe(true);
+});