LibWasm: Make Interpreter a virtual interface

This allows multiply different kinds of interpreters to be used by the
runtime; currently a BytecodeInterpreter and a
DebuggerBytecodeInterpreter is provided.

Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.cpp

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include "Interpreter.h"
 #include <LibWasm/AbstractMachine/AbstractMachine.h>
 #include <LibWasm/AbstractMachine/Configuration.h>
 #include <LibWasm/Types.h>
@@ -105,18 +106,18 @@ InstantiationResult AbstractMachine::instantiate(const Module& module, Vector<Ex
             auxiliary_instance.globals().append(*ptr);
     }
 
+    BytecodeInterpreter interpreter;
+
     module.for_each_section_of_type<GlobalSection>([&](auto& global_section) {
         for (auto& entry : global_section.entries()) {
             Configuration config { m_store };
-            config.pre_interpret_hook = &pre_interpret_hook;
-            config.post_interpret_hook = &post_interpret_hook;
             config.set_frame(Frame {
                 auxiliary_instance,
                 Vector<Value> {},
                 entry.expression(),
                 1,
             });
-            auto result = config.execute();
+            auto result = config.execute(interpreter);
             // What if this traps?
             if (result.is_trap())
                 instantiation_result = InstantiationError { "Global value construction trapped" };
@@ -140,15 +141,13 @@ InstantiationResult AbstractMachine::instantiate(const Module& module, Vector<Ex
             segment.value().visit(
                 [&](const DataSection::Data::Active& data) {
                     Configuration config { m_store };
-                    config.pre_interpret_hook = &pre_interpret_hook;
-                    config.post_interpret_hook = &post_interpret_hook;
                     config.set_frame(Frame {
                         main_module_instance,
                         Vector<Value> {},
                         data.offset,
                         1,
                     });
-                    auto result = config.execute();
+                    auto result = config.execute(interpreter);
                     size_t offset = 0;
                     result.values().first().value().visit(
                         [&](const auto& value) { offset = value; },
@@ -284,11 +283,15 @@ Optional<InstantiationError> AbstractMachine::allocate_all(const Module& module,
 }
 
 Result AbstractMachine::invoke(FunctionAddress address, Vector<Value> arguments)
+{
+    BytecodeInterpreter interpreter;
+    return invoke(interpreter, address, move(arguments));
+}
+
+Result AbstractMachine::invoke(Interpreter& interpreter, FunctionAddress address, Vector<Value> arguments)
 {
     Configuration configuration { m_store };
-    configuration.pre_interpret_hook = &pre_interpret_hook;
+    return configuration.call(interpreter, address, move(arguments));
-    configuration.post_interpret_hook = &post_interpret_hook;
-    return configuration.call(address, move(arguments));
 }
 
 void Linker::link(const ModuleInstance& instance)

Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h

@@ -441,13 +441,11 @@ public:
     // Load and instantiate a module, and link it into this interpreter.
     InstantiationResult instantiate(const Module&, Vector<ExternValue>);
     Result invoke(FunctionAddress, Vector<Value>);
+    Result invoke(Interpreter&, FunctionAddress, Vector<Value>);
 
     auto& store() const { return m_store; }
     auto& store() { return m_store; }
 
-    Function<bool(Configuration&, InstructionPointer&, const Instruction&)> pre_interpret_hook;
-    Function<bool(Configuration&, InstructionPointer&, const Instruction&, const Interpreter&)> post_interpret_hook;
-
 private:
     Optional<InstantiationError> allocate_all(const Module&, ModuleInstance&, Vector<ExternValue>&, Vector<Value>& global_values);
     Store m_store;

Userland/Libraries/LibWasm/AbstractMachine/Configuration.cpp

@@ -32,7 +32,7 @@ void Configuration::unwind(Badge<CallFrameHandle>, const CallFrameHandle& frame_
     VERIFY(m_stack.size() == frame_handle.stack_size);
 }
 
-Result Configuration::call(FunctionAddress address, Vector<Value> arguments)
+Result Configuration::call(Interpreter& interpreter, FunctionAddress address, Vector<Value> arguments)
 {
     auto* function = m_store.get(address);
     if (!function)
@@ -52,7 +52,7 @@ Result Configuration::call(FunctionAddress address, Vector<Value> arguments)
             wasm_function->type().results().size(),
         });
         m_ip = 0;
-        return execute();
+        return execute(interpreter);
     }
 
     // It better be a host function, else something is really wrong.
@@ -60,12 +60,8 @@ Result Configuration::call(FunctionAddress address, Vector<Value> arguments)
     return host_function.function()(*this, arguments);
 }
 
-Result Configuration::execute()
+Result Configuration::execute(Interpreter& interpreter)
 {
-    Interpreter interpreter;
-    interpreter.pre_interpret_hook = pre_interpret_hook;
-    interpreter.post_interpret_hook = post_interpret_hook;
-
     interpreter.interpret(*this);
     if (interpreter.did_trap())
         return Trap {};

Userland/Libraries/LibWasm/AbstractMachine/Configuration.h

@@ -58,14 +58,11 @@ public:
     };
 
     void unwind(Badge<CallFrameHandle>, const CallFrameHandle&);
-    Result call(FunctionAddress, Vector<Value> arguments);
+    Result call(Interpreter&, FunctionAddress, Vector<Value> arguments);
-    Result execute();
+    Result execute(Interpreter&);
 
     void dump_stack();
 
-    Function<bool(Configuration&, InstructionPointer&, const Instruction&)>* pre_interpret_hook { nullptr };
-    Function<bool(Configuration&, InstructionPointer&, const Instruction&, const Interpreter&)>* post_interpret_hook { nullptr };
-
 private:
     Store& m_store;
     size_t m_current_frame_index { 0 };

Userland/Libraries/LibWasm/AbstractMachine/Interpreter.cpp

@@ -29,7 +29,7 @@ namespace Wasm {
         }                                                                                      \
     } while (false)
 
-void Interpreter::interpret(Configuration& configuration)
+void BytecodeInterpreter::interpret(Configuration& configuration)
 {
     auto& instructions = configuration.frame().expression().instructions();
     auto max_ip_value = InstructionPointer { instructions.size() };
@@ -46,7 +46,7 @@ void Interpreter::interpret(Configuration& configuration)
     }
 }
 
-void Interpreter::branch_to_label(Configuration& configuration, LabelIndex index)
+void BytecodeInterpreter::branch_to_label(Configuration& configuration, LabelIndex index)
 {
     dbgln_if(WASM_TRACE_DEBUG, "Branch to label with index {}...", index.value());
     auto label = configuration.nth_label(index.value());
@@ -71,7 +71,7 @@ void Interpreter::branch_to_label(Configuration& configuration, LabelIndex index
     configuration.ip() = label->continuation();
 }
 
-ReadonlyBytes Interpreter::load_from_memory(Configuration& configuration, const Instruction& instruction, size_t size)
+ReadonlyBytes BytecodeInterpreter::load_from_memory(Configuration& configuration, const Instruction& instruction, size_t size)
 {
     auto& address = configuration.frame().module().memories().first();
     auto memory = configuration.store().get(address);
@@ -95,7 +95,7 @@ ReadonlyBytes Interpreter::load_from_memory(Configuration& configuration, const
     return memory->data().bytes().slice(instance_address, size);
 }
 
-void Interpreter::store_to_memory(Configuration& configuration, const Instruction& instruction, ReadonlyBytes data)
+void BytecodeInterpreter::store_to_memory(Configuration& configuration, const Instruction& instruction, ReadonlyBytes data)
 {
     auto& address = configuration.frame().module().memories().first();
     auto memory = configuration.store().get(address);
@@ -113,7 +113,7 @@ void Interpreter::store_to_memory(Configuration& configuration, const Instructio
     data.copy_to(memory->data().bytes().slice(instance_address, data.size()));
 }
 
-void Interpreter::call_address(Configuration& configuration, FunctionAddress address)
+void BytecodeInterpreter::call_address(Configuration& configuration, FunctionAddress address)
 {
     auto instance = configuration.store().get(address);
     TRAP_IF_NOT(instance);
@@ -129,7 +129,7 @@ void Interpreter::call_address(Configuration& configuration, FunctionAddress add
     Result result { Trap {} };
     {
         Configuration::CallFrameHandle handle { configuration };
-        result = configuration.call(address, move(args));
+        result = configuration.call(*this, address, move(args));
     }
 
     if (result.is_trap()) {
@@ -215,7 +215,7 @@ void Interpreter::call_address(Configuration& configuration, FunctionAddress add
     } while (false)
 
 template<typename T>
-T Interpreter::read_value(ReadonlyBytes data)
+T BytecodeInterpreter::read_value(ReadonlyBytes data)
 {
     T value;
     InputMemoryStream stream { data };
@@ -226,7 +226,7 @@ T Interpreter::read_value(ReadonlyBytes data)
 }
 
 template<>
-float Interpreter::read_value<float>(ReadonlyBytes data)
+float BytecodeInterpreter::read_value<float>(ReadonlyBytes data)
 {
     InputMemoryStream stream { data };
     LittleEndian<u32> raw_value;
@@ -237,7 +237,7 @@ float Interpreter::read_value<float>(ReadonlyBytes data)
 }
 
 template<>
-double Interpreter::read_value<double>(ReadonlyBytes data)
+double BytecodeInterpreter::read_value<double>(ReadonlyBytes data)
 {
     InputMemoryStream stream { data };
     LittleEndian<u64> raw_value;
@@ -282,7 +282,7 @@ struct ConvertToRaw<double> {
 };
 
 template<typename V, typename T>
-MakeSigned<T> Interpreter::checked_signed_truncate(V value)
+MakeSigned<T> BytecodeInterpreter::checked_signed_truncate(V value)
 {
     if (isnan(value) || isinf(value)) { // "undefined", let's just trap.
         m_do_trap = true;
@@ -305,7 +305,7 @@ MakeSigned<T> Interpreter::checked_signed_truncate(V value)
 }
 
 template<typename V, typename T>
-MakeUnsigned<T> Interpreter::checked_unsigned_truncate(V value)
+MakeUnsigned<T> BytecodeInterpreter::checked_unsigned_truncate(V value)
 {
     if (isnan(value) || isinf(value)) { // "undefined", let's just trap.
         m_do_trap = true;
@@ -326,7 +326,7 @@ MakeUnsigned<T> Interpreter::checked_unsigned_truncate(V value)
     return true;
 }
 
-Vector<Value> Interpreter::pop_values(Configuration& configuration, size_t count)
+Vector<Value> BytecodeInterpreter::pop_values(Configuration& configuration, size_t count)
 {
     Vector<Value> results;
     for (size_t i = 0; i < count; ++i) {
@@ -339,28 +339,10 @@ Vector<Value> Interpreter::pop_values(Configuration& configuration, size_t count
     return results;
 }
 
-void Interpreter::interpret(Configuration& configuration, InstructionPointer& ip, const Instruction& instruction)
+void BytecodeInterpreter::interpret(Configuration& configuration, InstructionPointer& ip, const Instruction& instruction)
 {
     dbgln_if(WASM_TRACE_DEBUG, "Executing instruction {} at ip {}", instruction_name(instruction.opcode()), ip.value());
 
-    if (pre_interpret_hook && *pre_interpret_hook) {
-        auto result = pre_interpret_hook->operator()(configuration, ip, instruction);
-        if (!result) {
-            m_do_trap = true;
-            return;
-        }
-    }
-
-    ScopeGuard guard { [&] {
-        if (post_interpret_hook && *post_interpret_hook) {
-            auto result = post_interpret_hook->operator()(configuration, ip, instruction, *this);
-            if (!result) {
-                m_do_trap = true;
-                return;
-            }
-        }
-    } };
-
     switch (instruction.opcode().value()) {
     case Instructions::unreachable.value():
         m_do_trap = true;
@@ -912,4 +894,28 @@ void Interpreter::interpret(Configuration& configuration, InstructionPointer& ip
         return;
     }
 }
+
+void DebuggerBytecodeInterpreter::interpret(Configuration& configuration, InstructionPointer& ip, const Instruction& instruction)
+{
+    if (pre_interpret_hook) {
+        auto result = pre_interpret_hook(configuration, ip, instruction);
+        if (!result) {
+            m_do_trap = true;
+            return;
+        }
+    }
+
+    ScopeGuard guard { [&] {
+        if (post_interpret_hook) {
+            auto result = post_interpret_hook(configuration, ip, instruction, *this);
+            if (!result) {
+                m_do_trap = true;
+                return;
+            }
+        }
+    } };
+
+    BytecodeInterpreter::interpret(configuration, ip, instruction);
+}
+
 }

Userland/Libraries/LibWasm/AbstractMachine/Interpreter.h

@@ -11,15 +11,20 @@
 namespace Wasm {
 
 struct Interpreter {
-    void interpret(Configuration&);
+    virtual ~Interpreter() = default;
-    bool did_trap() const { return m_do_trap; }
+    virtual void interpret(Configuration&) = 0;
-    void clear_trap() { m_do_trap = false; }
+    virtual bool did_trap() const = 0;
+    virtual void clear_trap() = 0;
+};
 
-    Function<bool(Configuration&, InstructionPointer&, const Instruction&)>* pre_interpret_hook { nullptr };
+struct BytecodeInterpreter : public Interpreter {
-    Function<bool(Configuration&, InstructionPointer&, const Instruction&, const Interpreter&)>* post_interpret_hook { nullptr };
+    virtual void interpret(Configuration&) override;
+    virtual ~BytecodeInterpreter() override = default;
+    virtual bool did_trap() const override { return m_do_trap; }
+    virtual void clear_trap() override { m_do_trap = false; }
 
-private:
+protected:
-    void interpret(Configuration&, InstructionPointer&, const Instruction&);
+    virtual void interpret(Configuration&, InstructionPointer&, const Instruction&);
     void branch_to_label(Configuration&, LabelIndex);
     ReadonlyBytes load_from_memory(Configuration&, const Instruction&, size_t);
     void store_to_memory(Configuration&, const Instruction&, ReadonlyBytes data);
@@ -44,4 +49,14 @@ private:
     bool m_do_trap { false };
 };
 
+struct DebuggerBytecodeInterpreter : public BytecodeInterpreter {
+    virtual ~DebuggerBytecodeInterpreter() override = default;
+
+    Function<bool(Configuration&, InstructionPointer&, const Instruction&)> pre_interpret_hook;
+    Function<bool(Configuration&, InstructionPointer&, const Instruction&, const Interpreter&)> post_interpret_hook;
+
+private:
+    virtual void interpret(Configuration&, InstructionPointer&, const Instruction&) override;
+};
+
 }

Userland/Utilities/wasm.cpp

@@ -20,6 +20,7 @@ static auto g_stdout = Core::OutputFileStream::standard_error();
 static Wasm::Printer g_printer { g_stdout };
 static bool g_continue { false };
 static void (*old_signal)(int);
+static Wasm::DebuggerBytecodeInterpreter g_interpreter;
 
 static void print_buffer(ReadonlyBytes buffer, int split)
 {
@@ -190,7 +191,11 @@ static bool pre_interpret_hook(Wasm::Configuration& config, Wasm::InstructionPoi
             for (auto& param : type.parameters())
                 values.append(Wasm::Value { param, values_to_push.take_last() });
 
-            auto result = config.call(*address, move(values));
+            Wasm::Result result { Wasm::Trap {} };
+            {
+                Wasm::Configuration::CallFrameHandle handle { config };
+                result = config.call(g_interpreter, *address, move(values));
+            }
             if (result.is_trap())
                 warnln("Execution trapped!");
             if (!result.values().is_empty())
@@ -328,8 +333,8 @@ int main(int argc, char* argv[])
         Core::EventLoop main_loop;
         if (debug) {
             g_line_editor = Line::Editor::construct();
-            machine.pre_interpret_hook = pre_interpret_hook;
+            g_interpreter.pre_interpret_hook = pre_interpret_hook;
-            machine.post_interpret_hook = post_interpret_hook;
+            g_interpreter.post_interpret_hook = post_interpret_hook;
         }
         // First, resolve the linked modules
         NonnullOwnPtrVector<Wasm::ModuleInstance> linked_instances;
@@ -433,7 +438,7 @@ int main(int argc, char* argv[])
                 outln();
             }
 
-            auto result = machine.invoke(run_address.value(), move(values));
+            auto result = machine.invoke(g_interpreter, run_address.value(), move(values));
 
             if (debug) {
                 Wasm::Configuration config { machine.store() };