From c14db6ab12a63c6bd2fe82ddb1631c88a8508779 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 3 Oct 2023 08:18:10 +0200 Subject: [PATCH] LibJS: Make Executable ref-counted and let instruction iterator co-own it This ensures that the instruction stream pointed at by the instruction iterator remains valid as long as the iterator exists. --- .../Libraries/LibJS/Bytecode/Executable.cpp | 25 ++++++++++++++++ .../Libraries/LibJS/Bytecode/Executable.h | 16 +++++++++- .../Libraries/LibJS/Bytecode/Generator.cpp | 30 ++++++++----------- Userland/Libraries/LibJS/Bytecode/Generator.h | 2 +- .../Libraries/LibJS/Bytecode/Instruction.h | 4 ++- .../Libraries/LibJS/Bytecode/Interpreter.cpp | 2 +- .../Libraries/LibJS/Bytecode/Interpreter.h | 2 +- Userland/Libraries/LibJS/Forward.h | 2 +- .../LibJS/Runtime/ECMAScriptFunctionObject.h | 4 +-- 9 files changed, 61 insertions(+), 26 deletions(-) diff --git a/Userland/Libraries/LibJS/Bytecode/Executable.cpp b/Userland/Libraries/LibJS/Bytecode/Executable.cpp index 722388e51b..568e8f787a 100644 --- a/Userland/Libraries/LibJS/Bytecode/Executable.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Executable.cpp @@ -5,9 +5,34 @@ */ #include +#include namespace JS::Bytecode { +Executable::Executable( + NonnullOwnPtr identifier_table, + NonnullOwnPtr string_table, + NonnullOwnPtr regex_table, + NonnullRefPtr source_code, + size_t number_of_property_lookup_caches, + size_t number_of_global_variable_caches, + size_t number_of_registers, + Vector> basic_blocks, + bool is_strict_mode) + : basic_blocks(move(basic_blocks)) + , string_table(move(string_table)) + , identifier_table(move(identifier_table)) + , regex_table(move(regex_table)) + , source_code(move(source_code)) + , number_of_registers(number_of_registers) + , is_strict_mode(is_strict_mode) +{ + property_lookup_caches.resize(number_of_property_lookup_caches); + global_variable_caches.resize(number_of_global_variable_caches); +} + +Executable::~Executable() = default; + void Executable::dump() const { dbgln("\033[33;1mJS::Bytecode::Executable\033[0m ({})", name); diff --git a/Userland/Libraries/LibJS/Bytecode/Executable.h b/Userland/Libraries/LibJS/Bytecode/Executable.h index aeda600d57..6e020e19d0 100644 --- a/Userland/Libraries/LibJS/Bytecode/Executable.h +++ b/Userland/Libraries/LibJS/Bytecode/Executable.h @@ -31,7 +31,21 @@ struct SourceRecord { u32 source_end_offset {}; }; -struct Executable { +class Executable final : public RefCounted { +public: + Executable( + NonnullOwnPtr, + NonnullOwnPtr, + NonnullOwnPtr, + NonnullRefPtr, + size_t number_of_property_lookup_caches, + size_t number_of_global_variable_caches, + size_t number_of_registers, + Vector>, + bool is_strict_mode); + + ~Executable(); + DeprecatedFlyString name; Vector property_lookup_caches; Vector global_variable_caches; diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index 1b68bd5dfb..6448c0f4c0 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -20,7 +20,7 @@ Generator::Generator() { } -CodeGenerationErrorOr> Generator::generate(ASTNode const& node, FunctionKind enclosing_function_kind) +CodeGenerationErrorOr> Generator::generate(ASTNode const& node, FunctionKind enclosing_function_kind) { Generator generator; generator.switch_to_basic_block(generator.make_block()); @@ -56,24 +56,18 @@ CodeGenerationErrorOr> Generator::generate(ASTNode con else if (is(node)) is_strict_mode = static_cast(node).is_strict_mode(); - Vector property_lookup_caches; - property_lookup_caches.resize(generator.m_next_property_lookup_cache); + auto executable = adopt_ref(*new Executable( + move(generator.m_identifier_table), + move(generator.m_string_table), + move(generator.m_regex_table), + node.source_code(), + generator.m_next_property_lookup_cache, + generator.m_next_global_variable_cache, + generator.m_next_register, + move(generator.m_root_basic_blocks), + is_strict_mode)); - Vector global_variable_caches; - global_variable_caches.resize(generator.m_next_global_variable_cache); - - return adopt_own(*new Executable { - .name = {}, - .property_lookup_caches = move(property_lookup_caches), - .global_variable_caches = move(global_variable_caches), - .basic_blocks = move(generator.m_root_basic_blocks), - .string_table = move(generator.m_string_table), - .identifier_table = move(generator.m_identifier_table), - .regex_table = move(generator.m_regex_table), - .source_code = node.source_code(), - .number_of_registers = generator.m_next_register, - .is_strict_mode = is_strict_mode, - }); + return executable; } void Generator::grow(size_t additional_size) diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h index 62b245f982..9590063d2a 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.h +++ b/Userland/Libraries/LibJS/Bytecode/Generator.h @@ -30,7 +30,7 @@ public: Function, Block, }; - static CodeGenerationErrorOr> generate(ASTNode const&, FunctionKind = FunctionKind::Normal); + static CodeGenerationErrorOr> generate(ASTNode const&, FunctionKind = FunctionKind::Normal); Register allocate_register(); diff --git a/Userland/Libraries/LibJS/Bytecode/Instruction.h b/Userland/Libraries/LibJS/Bytecode/Instruction.h index 212e5c8b05..9ed16122ac 100644 --- a/Userland/Libraries/LibJS/Bytecode/Instruction.h +++ b/Userland/Libraries/LibJS/Bytecode/Instruction.h @@ -178,13 +178,15 @@ public: UnrealizedSourceRange source_range() const; RefPtr source_code() const; + Executable const* executable() const { return m_executable; } + private: Instruction const& dereference() const { return *reinterpret_cast(m_ptr); } u8 const* m_begin { nullptr }; u8 const* m_end { nullptr }; u8 const* m_ptr { nullptr }; - Executable const* m_executable { nullptr }; + RefPtr m_executable; }; } diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index b100662002..28e8ad572b 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -407,7 +407,7 @@ void Interpreter::leave_unwind_context() unwind_contexts().take_last(); } -ThrowCompletionOr> compile(VM& vm, ASTNode const& node, FunctionKind kind, DeprecatedFlyString const& name) +ThrowCompletionOr> compile(VM& vm, ASTNode const& node, FunctionKind kind, DeprecatedFlyString const& name) { auto executable_result = Bytecode::Generator::generate(node, kind); if (executable_result.is_error()) diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.h b/Userland/Libraries/LibJS/Bytecode/Interpreter.h index 448939857a..fa911d8230 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.h +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.h @@ -110,6 +110,6 @@ private: extern bool g_dump_bytecode; -ThrowCompletionOr> compile(VM&, ASTNode const& no, JS::FunctionKind kind, DeprecatedFlyString const& name); +ThrowCompletionOr> compile(VM&, ASTNode const& no, JS::FunctionKind kind, DeprecatedFlyString const& name); } diff --git a/Userland/Libraries/LibJS/Forward.h b/Userland/Libraries/LibJS/Forward.h index d8a0aff15c..8074d1a79f 100644 --- a/Userland/Libraries/LibJS/Forward.h +++ b/Userland/Libraries/LibJS/Forward.h @@ -302,7 +302,7 @@ class MarkedVector; namespace Bytecode { class BasicBlock; -struct Executable; +class Executable; class Generator; class Instruction; class Interpreter; diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h index 2f01f1add6..78ade0b336 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h @@ -111,8 +111,8 @@ private: ThrowCompletionOr function_declaration_instantiation(); DeprecatedFlyString m_name; - OwnPtr m_bytecode_executable; - Vector> m_default_parameter_bytecode_executables; + RefPtr m_bytecode_executable; + Vector> m_default_parameter_bytecode_executables; i32 m_function_length { 0 }; Vector m_local_variables_names;