From d906255cbb0e4484e4eaaeb1a2c7138653d042bc Mon Sep 17 00:00:00 2001 From: Diego <96022404+dzfrias@users.noreply.github.com> Date: Fri, 31 May 2024 17:14:49 -0700 Subject: [PATCH] LibWasm: Improve table support Implements `table.get`, `table.set`, `elem.drop`, `table.size`, and `table.grow`. Also fixes a few issues when generating ref-related spectests. Also changes the `TableInstance` type to use `Vector` instead of `Vector>`, because the ability to be null is already encoded in the `Reference` type. --- Meta/generate-libwasm-spec-test.py | 9 ++- Tests/LibWasm/test-wasm.cpp | 10 ++- .../AbstractMachine/AbstractMachine.cpp | 12 +--- .../LibWasm/AbstractMachine/AbstractMachine.h | 15 +++-- .../AbstractMachine/BytecodeInterpreter.cpp | 61 ++++++++++++++++--- .../Libraries/LibWeb/WebAssembly/Table.cpp | 4 +- 6 files changed, 82 insertions(+), 29 deletions(-) diff --git a/Meta/generate-libwasm-spec-test.py b/Meta/generate-libwasm-spec-test.py index 3f4bbecf72c..a42c6d86536 100644 --- a/Meta/generate-libwasm-spec-test.py +++ b/Meta/generate-libwasm-spec-test.py @@ -62,6 +62,9 @@ def parse_typed_value(ast): 'i64.const': 'i64', 'f32.const': 'float', 'f64.const': 'double', + 'ref.null': 'null', + 'ref.extern': 'i32', + 'ref.func': 'i32', 'v128.const': 'bigint', } @@ -331,9 +334,9 @@ def generate(ast): "function": { "module": module, "name": name, - "args": list(parse_typed_value(x) for x in entry[1][arg + 2:]) + "args": [parse_typed_value(entry[2])] if len(entry) == 3 else [] }, - "result": parse_typed_value(entry[2]) if len(entry) == 3 + arg else None + "result": None }) elif len(entry) > 1 and entry[0][0] == 'register': if len(entry) == 3: @@ -370,6 +373,8 @@ def genarg(spec): x = spec['value'] if spec['type'] == 'bigint': return f"0x{x}n" + if spec['type'] == 'null': + return 'null' if spec['type'] in ('i32', 'i64'): if x.startswith('0x'): diff --git a/Tests/LibWasm/test-wasm.cpp b/Tests/LibWasm/test-wasm.cpp index fc3d67821b5..3ac3d862913 100644 --- a/Tests/LibWasm/test-wasm.cpp +++ b/Tests/LibWasm/test-wasm.cpp @@ -242,10 +242,18 @@ JS_DEFINE_NATIVE_FUNCTION(WebAssemblyModule::wasm_invoke) break; } case Wasm::ValueType::Kind::FunctionReference: + if (argument.is_null()) { + arguments.append(Wasm::Value(Wasm::Reference { Wasm::Reference::Null { Wasm::ValueType(Wasm::ValueType::Kind::FunctionReference) } })); + break; + } arguments.append(Wasm::Value(Wasm::Reference { Wasm::Reference::Func { static_cast(double_value) } })); break; case Wasm::ValueType::Kind::ExternReference: - arguments.append(Wasm::Value(Wasm::Reference { Wasm::Reference::Func { static_cast(double_value) } })); + if (argument.is_null()) { + arguments.append(Wasm::Value(Wasm::Reference { Wasm::Reference::Null { Wasm::ValueType(Wasm::ValueType::Kind::ExternReference) } })); + break; + } + arguments.append(Wasm::Value(Wasm::Reference { Wasm::Reference::Extern { static_cast(double_value) } })); break; case Wasm::ValueType::Kind::NullFunctionReference: arguments.append(Wasm::Value(Wasm::Reference { Wasm::Reference::Null { Wasm::ValueType(Wasm::ValueType::Kind::FunctionReference) } })); diff --git a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp index c8c292b88d7..7d0cdab3057 100644 --- a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp +++ b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp @@ -34,7 +34,7 @@ Optional Store::allocate(HostFunction&& function) Optional Store::allocate(TableType const& type) { TableAddress address { m_tables.size() }; - Vector> elements; + Vector elements; elements.resize(type.limits().min()); m_tables.empend(TableInstance { type, move(elements) }); return address; @@ -252,10 +252,6 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vector(); if (!active_ptr) continue; - if (active_ptr->index.value() != 0) { - instantiation_result = InstantiationError { "Non-zero table referenced by active element segment" }; - return IterationDecision::Break; - } Configuration config { m_store }; if (m_should_limit_instruction_count) config.enable_instruction_count_limit(); @@ -275,11 +271,7 @@ InstantiationResult AbstractMachine::instantiate(Module const& module, Vectorindex.value()]); if (current_index >= main_module_instance.elements().size()) { instantiation_result = InstantiationError { "Invalid element referenced by active element segment" }; return IterationDecision::Break; diff --git a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h index 262dbd86b03..e88172e5e11 100644 --- a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h +++ b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h @@ -61,6 +61,10 @@ public: : m_ref(move(ref)) { } + explicit Reference() + : m_ref(Reference::Null { ValueType(ValueType::Kind::FunctionReference) }) + { + } auto& ref() const { return m_ref; } @@ -370,7 +374,7 @@ using FunctionInstance = Variant; class TableInstance { public: - explicit TableInstance(TableType const& type, Vector> elements) + explicit TableInstance(TableType const& type, Vector elements) : m_elements(move(elements)) , m_type(type) { @@ -380,15 +384,18 @@ public: auto& elements() { return m_elements; } auto& type() const { return m_type; } - bool grow(size_t size_to_grow, Reference const& fill_value) + bool grow(u32 size_to_grow, Reference const& fill_value) { if (size_to_grow == 0) return true; - auto new_size = m_elements.size() + size_to_grow; + size_t new_size = m_elements.size() + size_to_grow; if (auto max = m_type.limits().max(); max.has_value()) { if (max.value() < new_size) return false; } + if (new_size >= NumericLimits::max()) { + return false; + } auto previous_size = m_elements.size(); if (m_elements.try_resize(new_size).is_error()) return false; @@ -398,7 +405,7 @@ public: } private: - Vector> m_elements; + Vector m_elements; TableType m_type; }; diff --git a/Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp b/Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp index 61e335b7a97..513f3b94e12 100644 --- a/Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp +++ b/Userland/Libraries/LibWasm/AbstractMachine/BytecodeInterpreter.cpp @@ -665,9 +665,8 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi TRAP_IF_NOT(index.value() >= 0); TRAP_IF_NOT(static_cast(index.value()) < table_instance->elements().size()); auto element = table_instance->elements()[index.value()]; - TRAP_IF_NOT(element.has_value()); - TRAP_IF_NOT(element->ref().has()); - auto address = element->ref().get().address; + TRAP_IF_NOT(element.ref().has()); + auto address = element.ref().get().address; dbgln_if(WASM_TRACE_DEBUG, "call_indirect({} -> {})", index.value(), address.value()); call_address(configuration, address); return; @@ -859,9 +858,55 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi *configuration.store().get(data_address) = DataInstance({}); return; } - case Instructions::table_get.value(): - case Instructions::table_set.value(): - goto unimplemented; + case Instructions::elem_drop.value(): { + auto elem_index = instruction.arguments().get(); + auto address = configuration.frame().module().elements()[elem_index.value()]; + auto elem = configuration.store().get(address); + *configuration.store().get(address) = ElementInstance(elem->type(), {}); + return; + } + case Instructions::table_set.value(): { + auto ref = *configuration.stack().pop().get().to(); + auto index = (size_t)(*configuration.stack().pop().get().to()); + auto table_index = instruction.arguments().get(); + auto address = configuration.frame().module().tables()[table_index.value()]; + auto table = configuration.store().get(address); + TRAP_IF_NOT(index < table->elements().size()); + table->elements()[index] = ref; + return; + } + case Instructions::table_get.value(): { + auto index = (size_t)(*configuration.stack().pop().get().to()); + auto table_index = instruction.arguments().get(); + auto address = configuration.frame().module().tables()[table_index.value()]; + auto table = configuration.store().get(address); + TRAP_IF_NOT(index < table->elements().size()); + auto ref = table->elements()[index]; + configuration.stack().push(Value(ref)); + return; + } + case Instructions::table_grow.value(): { + auto size = *configuration.stack().pop().get().to(); + auto fill_value = *configuration.stack().pop().get().to(); + auto table_index = instruction.arguments().get(); + auto address = configuration.frame().module().tables()[table_index.value()]; + auto table = configuration.store().get(address); + auto previous_size = table->elements().size(); + auto did_grow = table->grow(size, fill_value); + if (!did_grow) { + configuration.stack().push(Value((i32)-1)); + } else { + configuration.stack().push(Value((i32)previous_size)); + } + return; + } + case Instructions::table_size.value(): { + auto table_index = instruction.arguments().get(); + auto address = configuration.frame().module().tables()[table_index.value()]; + auto table = configuration.store().get(address); + configuration.stack().push(Value((i32)table->elements().size())); + return; + } case Instructions::ref_null.value(): { auto type = instruction.arguments().get(); configuration.stack().push(Value(Reference(Reference::Null { type }))); @@ -1503,13 +1548,9 @@ void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPoi case Instructions::f64x2_convert_low_i32x4_s.value(): case Instructions::f64x2_convert_low_i32x4_u.value(): case Instructions::table_init.value(): - case Instructions::elem_drop.value(): case Instructions::table_copy.value(): - case Instructions::table_grow.value(): - case Instructions::table_size.value(): case Instructions::table_fill.value(): default: - unimplemented:; dbgln_if(WASM_TRACE_DEBUG, "Instruction '{}' not implemented", instruction_name(instruction.opcode())); m_trap = Trap { ByteString::formatted("Unimplemented instruction {}", instruction_name(instruction.opcode())) }; return; diff --git a/Userland/Libraries/LibWeb/WebAssembly/Table.cpp b/Userland/Libraries/LibWeb/WebAssembly/Table.cpp index fda0605581b..4c4d3d3582e 100644 --- a/Userland/Libraries/LibWeb/WebAssembly/Table.cpp +++ b/Userland/Libraries/LibWeb/WebAssembly/Table.cpp @@ -106,10 +106,10 @@ WebIDL::ExceptionOr Table::get(u32 index) const return vm.throw_completion("Table element index out of range"sv); auto& ref = table->elements()[index]; - if (!ref.has_value()) + if (!ref.ref().has()) return JS::js_undefined(); - Wasm::Value wasm_value { ref.value() }; + Wasm::Value wasm_value { ref }; return Detail::to_js_value(vm, wasm_value); }