From efb106069bb434e96aac3391fd2d81ca68007000 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Mon, 10 May 2021 15:40:49 +0430 Subject: [PATCH] LibWasm: Decouple ModuleInstance from the AbstractMachine This fixes a FIXME and will allow linking only select modules together, instead of linking every instantiated module into a big mess of exported entities :P --- Tests/LibWasm/test-wasm.cpp | 20 ++++-- .../AbstractMachine/AbstractMachine.cpp | 69 ++++++++++--------- .../LibWasm/AbstractMachine/AbstractMachine.h | 8 +-- Userland/Utilities/wasm.cpp | 5 +- 4 files changed, 55 insertions(+), 47 deletions(-) diff --git a/Tests/LibWasm/test-wasm.cpp b/Tests/LibWasm/test-wasm.cpp index ac2eea0b06..14fff623f2 100644 --- a/Tests/LibWasm/test-wasm.cpp +++ b/Tests/LibWasm/test-wasm.cpp @@ -31,18 +31,23 @@ class WebAssemblyModule final : public JS::Object { JS_OBJECT(WebAssemblyModule, JS::Object); public: - // FIXME: This should only contain an instantiated module, not the entire abstract machine! explicit WebAssemblyModule(JS::Object& prototype) : JS::Object(prototype) { } + static Wasm::AbstractMachine& machine() { return m_machine; } + Wasm::Module& module() { return *m_module; } + Wasm::ModuleInstance& module_instance() { return *m_module_instance; } + static WebAssemblyModule* create(JS::GlobalObject& global_object, Wasm::Module module) { auto instance = global_object.heap().allocate(global_object, *global_object.object_prototype()); instance->m_module = move(module); - if (auto result = instance->m_machine.instantiate(*instance->m_module, {}); result.is_error()) + if (auto result = machine().instantiate(*instance->m_module, {}); result.is_error()) global_object.vm().throw_exception(global_object, result.release_error().error); + else + instance->m_module_instance = result.release_value(); return instance; } void initialize(JS::GlobalObject&) override; @@ -53,10 +58,13 @@ private: JS_DECLARE_NATIVE_FUNCTION(get_export); JS_DECLARE_NATIVE_FUNCTION(wasm_invoke); - Wasm::AbstractMachine m_machine; + static Wasm::AbstractMachine m_machine; Optional m_module; + Optional m_module_instance; }; +Wasm::AbstractMachine WebAssemblyModule::m_machine; + TESTJS_GLOBAL_FUNCTION(parse_webassembly_module, parseWebAssemblyModule) { auto object = vm.argument(0).to_object(global_object); @@ -123,7 +131,7 @@ JS_DEFINE_NATIVE_FUNCTION(WebAssemblyModule::get_export) return {}; } auto instance = static_cast(object); - for (auto& entry : instance->m_machine.module_instance().exports()) { + for (auto& entry : instance->module_instance().exports()) { if (entry.name() == name) { auto& value = entry.value(); if (auto ptr = value.get_pointer()) @@ -151,7 +159,7 @@ JS_DEFINE_NATIVE_FUNCTION(WebAssemblyModule::wasm_invoke) } auto instance = static_cast(object); Wasm::FunctionAddress function_address { address }; - auto function_instance = instance->m_machine.store().get(function_address); + auto function_instance = WebAssemblyModule::machine().store().get(function_address); if (!function_instance) { vm.throw_exception(global_object, "Invalid function address"); return {}; @@ -194,7 +202,7 @@ JS_DEFINE_NATIVE_FUNCTION(WebAssemblyModule::wasm_invoke) } } - auto result = instance->m_machine.invoke(function_address, arguments); + auto result = WebAssemblyModule::machine().invoke(function_address, arguments); if (result.is_trap()) { vm.throw_exception(global_object, "Execution trapped"); return {}; diff --git a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp index 2ad7ea7c83..9383927c9a 100644 --- a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp +++ b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp @@ -85,10 +85,11 @@ GlobalInstance* Store::get(GlobalAddress address) InstantiationResult AbstractMachine::instantiate(const Module& module, Vector externs) { + ModuleInstance main_module_instance; Optional instantiation_result; module.for_each_section_of_type([&](const TypeSection& section) { - m_module_instance.types() = section.types(); + main_module_instance.types() = section.types(); }); // FIXME: Validate stuff @@ -121,8 +122,8 @@ InstantiationResult AbstractMachine::instantiate(const Module& module, Vector([&](const ElementSection&) { @@ -136,7 +137,7 @@ InstantiationResult AbstractMachine::instantiate(const Module& module, Vector( - m_module_instance, + main_module_instance, Vector {}, data.offset, 1); @@ -150,11 +151,11 @@ InstantiationResult AbstractMachine::instantiate(const Module& module, Vectoris_error()) return; - if (m_module_instance.memories().size() <= data.index.value()) { - instantiation_result = InstantiationError { String::formatted("Data segment referenced out-of-bounds memory ({}) of max {} entries", data.index.value(), m_module_instance.memories().size()) }; + if (main_module_instance.memories().size() <= data.index.value()) { + instantiation_result = InstantiationError { String::formatted("Data segment referenced out-of-bounds memory ({}) of max {} entries", data.index.value(), main_module_instance.memories().size()) }; return; } - auto address = m_module_instance.memories()[data.index.value()]; + auto address = main_module_instance.memories()[data.index.value()]; if (auto instance = m_store.get(address)) { if (instance->type().limits().max().value_or(data.init.size() + offset + 1) <= data.init.size() + offset) { instantiation_result = InstantiationError { String::formatted("Data segment attempted to write to out-of-bounds memory ({}) of max {} bytes", data.init.size() + offset, instance->type().limits().max().value()) }; @@ -171,7 +172,7 @@ InstantiationResult AbstractMachine::instantiate(const Module& module, Vector([&](const StartSection& section) { - auto& functions = m_module_instance.functions(); + auto& functions = main_module_instance.functions(); auto index = section.function().index(); if (functions.size() <= index.value()) { instantiation_result = InstantiationError { String::formatted("Start section function referenced invalid index {} of max {} entries", index.value(), functions.size()) }; @@ -180,34 +181,34 @@ InstantiationResult AbstractMachine::instantiate(const Module& module, Vector& externs, Vector& global_values) +Optional AbstractMachine::allocate_all(const Module& module, ModuleInstance& module_instance, Vector& externs, Vector& global_values) { - Optional result; + Optional result; for (auto& entry : externs) { entry.visit( - [&](const FunctionAddress& address) { m_module_instance.functions().append(address); }, - [&](const TableAddress& address) { m_module_instance.tables().append(address); }, - [&](const MemoryAddress& address) { m_module_instance.memories().append(address); }, - [&](const GlobalAddress& address) { m_module_instance.globals().append(address); }); + [&](const FunctionAddress& address) { module_instance.functions().append(address); }, + [&](const TableAddress& address) { module_instance.tables().append(address); }, + [&](const MemoryAddress& address) { module_instance.memories().append(address); }, + [&](const GlobalAddress& address) { module_instance.globals().append(address); }); } // FIXME: What if this fails? for (auto& func : module.functions()) { - auto address = m_store.allocate(m_module_instance, func); + auto address = m_store.allocate(module_instance, func); VERIFY(address.has_value()); - m_module_instance.functions().append(*address); + module_instance.functions().append(*address); } module.for_each_section_of_type([&](const TableSection& section) { for (auto& table : section.tables()) { auto table_address = m_store.allocate(table.type()); VERIFY(table_address.has_value()); - m_module_instance.tables().append(*table_address); + module_instance.tables().append(*table_address); } }); @@ -215,7 +216,7 @@ InstantiationResult AbstractMachine::allocate_all(const Module& module, Vector address { Empty {} }; entry.description().visit( [&](const FunctionIndex& index) { - if (m_module_instance.functions().size() > index.value()) - address = FunctionAddress { m_module_instance.functions()[index.value()] }; + if (module_instance.functions().size() > index.value()) + address = FunctionAddress { module_instance.functions()[index.value()] }; else - dbgln("Failed to export '{}', the exported address ({}) was out of bounds (min: 0, max: {})", entry.name(), index.value(), m_module_instance.functions().size()); + dbgln("Failed to export '{}', the exported address ({}) was out of bounds (min: 0, max: {})", entry.name(), index.value(), module_instance.functions().size()); }, [&](const TableIndex& index) { - if (m_module_instance.tables().size() > index.value()) - address = TableAddress { m_module_instance.tables()[index.value()] }; + if (module_instance.tables().size() > index.value()) + address = TableAddress { module_instance.tables()[index.value()] }; else - dbgln("Failed to export '{}', the exported address ({}) was out of bounds (min: 0, max: {})", entry.name(), index.value(), m_module_instance.tables().size()); + dbgln("Failed to export '{}', the exported address ({}) was out of bounds (min: 0, max: {})", entry.name(), index.value(), module_instance.tables().size()); }, [&](const MemoryIndex& index) { - if (m_module_instance.memories().size() > index.value()) - address = MemoryAddress { m_module_instance.memories()[index.value()] }; + if (module_instance.memories().size() > index.value()) + address = MemoryAddress { module_instance.memories()[index.value()] }; else - dbgln("Failed to export '{}', the exported address ({}) was out of bounds (min: 0, max: {})", entry.name(), index.value(), m_module_instance.memories().size()); + dbgln("Failed to export '{}', the exported address ({}) was out of bounds (min: 0, max: {})", entry.name(), index.value(), module_instance.memories().size()); }, [&](const GlobalIndex& index) { - if (m_module_instance.globals().size() > index.value()) - address = GlobalAddress { m_module_instance.globals()[index.value()] }; + if (module_instance.globals().size() > index.value()) + address = GlobalAddress { module_instance.globals()[index.value()] }; else - dbgln("Failed to export '{}', the exported address ({}) was out of bounds (min: 0, max: {})", entry.name(), index.value(), m_module_instance.globals().size()); + dbgln("Failed to export '{}', the exported address ({}) was out of bounds (min: 0, max: {})", entry.name(), index.value(), module_instance.globals().size()); }); if (address.has()) { @@ -263,14 +264,14 @@ InstantiationResult AbstractMachine::allocate_all(const Module& module, Vector(), }); } }); - return result.value_or({}); + return result; } Result AbstractMachine::invoke(FunctionAddress address, Vector arguments) diff --git a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h index 94f5e29ce9..1f06b76f79 100644 --- a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h +++ b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h @@ -15,7 +15,6 @@ namespace Wasm { struct InstantiationError { String error { "Unknown error" }; }; -using InstantiationResult = Result; TYPEDEF_DISTINCT_NUMERIC_GENERAL(u64, true, true, false, false, false, true, FunctionAddress); TYPEDEF_DISTINCT_NUMERIC_GENERAL(u64, true, true, false, false, false, true, ExternAddress); @@ -419,6 +418,8 @@ private: Vector m_data; }; +using InstantiationResult = AK::Result; + class AbstractMachine { public: explicit AbstractMachine() = default; @@ -427,14 +428,11 @@ public: InstantiationResult instantiate(const Module&, Vector); Result invoke(FunctionAddress, Vector); - auto& module_instance() const { return m_module_instance; } - auto& module_instance() { return m_module_instance; } auto& store() const { return m_store; } auto& store() { return m_store; } private: - InstantiationResult allocate_all(const Module&, Vector&, Vector& global_values); - ModuleInstance m_module_instance; + Optional allocate_all(const Module&, ModuleInstance&, Vector&, Vector& global_values); Store m_store; }; diff --git a/Userland/Utilities/wasm.cpp b/Userland/Utilities/wasm.cpp index fa3d910cc9..eed9f3ccd0 100644 --- a/Userland/Utilities/wasm.cpp +++ b/Userland/Utilities/wasm.cpp @@ -70,6 +70,7 @@ int main(int argc, char* argv[]) warnln("Module instantiation failed: {}", result.error().error); return 1; } + auto module_instance = result.release_value(); auto stream = Core::OutputFileStream::standard_output(); auto print_func = [&](const auto& address) { @@ -90,7 +91,7 @@ int main(int argc, char* argv[]) }; if (print) { // Now, let's dump the functions! - for (auto& address : machine.module_instance().functions()) { + for (auto& address : module_instance.functions()) { print_func(address); } } @@ -98,7 +99,7 @@ int main(int argc, char* argv[]) if (!exported_function_to_execute.is_empty()) { Optional run_address; Vector values; - for (auto& entry : machine.module_instance().exports()) { + for (auto& entry : module_instance.exports()) { if (entry.name() == exported_function_to_execute) { if (auto addr = entry.value().get_pointer()) run_address = *addr;