Ver código fonte

LibJS: Cleanup unwind state when transferring control out of a finalizer

This does two things:
* Clear exceptions when transferring control out of a finalizer
  Otherwise they would resurface at the end of the next finalizer
  (see test the new test case), or at the end of a function
* Pop one scheduled jump when transferring control out of a finalizer
  This removes one old FIXME
Hendiadyoin1 1 ano atrás
pai
commit
ada5027163

+ 4 - 0
Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp

@@ -2443,7 +2443,11 @@ Bytecode::CodeGenerationErrorOr<Optional<Bytecode::Operand>> TryStatement::gener
         auto& finalizer_block = generator.make_block();
         generator.switch_to_basic_block(finalizer_block);
         generator.emit<Bytecode::Op::LeaveUnwindContext>();
+
+        generator.start_boundary(Bytecode::Generator::BlockBoundaryType::LeaveFinally);
         (void)TRY(m_finalizer->generate_bytecode(generator));
+        generator.end_boundary(Bytecode::Generator::BlockBoundaryType::LeaveFinally);
+
         if (!generator.is_current_block_terminated()) {
             next_block = &generator.make_block();
             auto next_target = Bytecode::Label { *next_block };

+ 4 - 1
Userland/Libraries/LibJS/Bytecode/Generator.cpp

@@ -532,7 +532,10 @@ void Generator::generate_scoped_jump(JumpType type)
             switch_to_basic_block(block);
             last_was_finally = true;
             break;
-        };
+        }
+        case LeaveFinally:
+            emit<Op::LeaveFinally>();
+            break;
         }
     }
     VERIFY_NOT_REACHED();

+ 4 - 0
Userland/Libraries/LibJS/Bytecode/Generator.h

@@ -209,6 +209,7 @@ public:
         Continue,
         Unwind,
         ReturnToFinally,
+        LeaveFinally,
         LeaveLexicalEnvironment,
     };
     template<typename OpType>
@@ -232,6 +233,9 @@ public:
                 break;
             case ReturnToFinally:
                 return;
+            case LeaveFinally:
+                emit<Bytecode::Op::LeaveFinally>();
+                break;
             };
         }
     }

+ 3 - 2
Userland/Libraries/LibJS/Bytecode/Instruction.h

@@ -39,8 +39,8 @@
     O(Div)                             \
     O(Dump)                            \
     O(End)                             \
-    O(EnterUnwindContext)              \
     O(EnterObjectEnvironment)          \
+    O(EnterUnwindContext)              \
     O(Exp)                             \
     O(GetById)                         \
     O(GetByIdWithThis)                 \
@@ -48,10 +48,10 @@
     O(GetByValueWithThis)              \
     O(GetCalleeAndThisFromEnvironment) \
     O(GetIterator)                     \
-    O(GetObjectFromIteratorRecord)     \
     O(GetMethod)                       \
     O(GetNewTarget)                    \
     O(GetNextMethodFromIteratorRecord) \
+    O(GetObjectFromIteratorRecord)     \
     O(GetImportMeta)                   \
     O(GetObjectPropertyIterator)       \
     O(GetPrivateById)                  \
@@ -71,6 +71,7 @@
     O(JumpIf)                          \
     O(JumpNullish)                     \
     O(JumpUndefined)                   \
+    O(LeaveFinally)                    \
     O(LeaveLexicalEnvironment)         \
     O(LeaveUnwindContext)              \
     O(LeftShift)                       \

+ 17 - 3
Userland/Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -368,9 +368,6 @@ void Interpreter::run_bytecode()
                 auto& running_execution_context = vm().running_execution_context();
                 auto const* old_scheduled_jump = running_execution_context.previously_scheduled_jumps.take_last();
                 if (m_scheduled_jump) {
-                    // FIXME: If we `break` or `continue` in the finally, we need to clear
-                    //        this field
-                    //        Same goes for popping an old_scheduled_jump form the stack
                     m_current_block = exchange(m_scheduled_jump, nullptr);
                 } else {
                     m_current_block = &static_cast<Op::ContinuePendingUnwind const&>(instruction).resume_target().block();
@@ -537,6 +534,12 @@ void Interpreter::restore_scheduled_jump()
     m_scheduled_jump = call_frame().previously_scheduled_jumps.take_last();
 }
 
+void Interpreter::leave_finally()
+{
+    reg(Register::exception()) = {};
+    m_scheduled_jump = call_frame().previously_scheduled_jumps.take_last();
+}
+
 void Interpreter::enter_object_environment(Object& object)
 {
     auto& running_execution_context = vm().running_execution_context();
@@ -1020,6 +1023,12 @@ ThrowCompletionOr<void> Catch::execute_impl(Bytecode::Interpreter& interpreter)
     return {};
 }
 
+ThrowCompletionOr<void> LeaveFinally::execute_impl(Bytecode::Interpreter& interpreter) const
+{
+    interpreter.leave_finally();
+    return {};
+}
+
 ThrowCompletionOr<void> RestoreScheduledJump::execute_impl(Bytecode::Interpreter& interpreter) const
 {
     interpreter.restore_scheduled_jump();
@@ -2237,6 +2246,11 @@ ByteString Catch::to_byte_string_impl(Bytecode::Executable const& executable) co
         format_operand("dst"sv, m_dst, executable));
 }
 
+ByteString LeaveFinally::to_byte_string_impl(Bytecode::Executable const&) const
+{
+    return ByteString::formatted("LeaveFinally");
+}
+
 ByteString RestoreScheduledJump::to_byte_string_impl(Bytecode::Executable const&) const
 {
     return ByteString::formatted("RestoreScheduledJump");

+ 1 - 0
Userland/Libraries/LibJS/Bytecode/Interpreter.h

@@ -69,6 +69,7 @@ public:
     void leave_unwind_context();
     void catch_exception(Operand dst);
     void restore_scheduled_jump();
+    void leave_finally();
 
     void enter_object_environment(Object&);
 

+ 11 - 0
Userland/Libraries/LibJS/Bytecode/Op.h

@@ -452,6 +452,17 @@ private:
     Operand m_dst;
 };
 
+class LeaveFinally final : public Instruction {
+public:
+    explicit LeaveFinally()
+        : Instruction(Type::LeaveFinally, sizeof(*this))
+    {
+    }
+
+    ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
+    ByteString to_byte_string_impl(Bytecode::Executable const&) const;
+};
+
 class RestoreScheduledJump final : public Instruction {
 public:
     explicit RestoreScheduledJump()

+ 21 - 0
Userland/Libraries/LibJS/Tests/try-catch-finally-nested.js

@@ -118,3 +118,24 @@ test("Nested try/finally/catch with exception in inner context ", () => {
     }
     expect(success).toBe(2);
 });
+
+test("Nested try/catch/finally with exception in inner most finally inside loop", () => {
+    success = 0;
+    try {
+        try {
+            do {
+                try {
+                    throw 1;
+                } finally {
+                    break;
+                }
+                expect.fail();
+            } while (expect.fail());
+        } catch (e) {
+            expect.fail();
+        }
+    } finally {
+        success = 1;
+    }
+    expect(success).toBe(1);
+});