[wasm] [interpreter] Implement indirect function calls

This CL adds support for indirect function calls to the interpreter. It
can indirectly call other wasm function in the same instance, which are
then executed in the interpreter, or call imported functions.

Implementing this required some refactoring:
- The wasm interpreter now unwraps import wrappers on demand, instead
  of unwrapping all of them on instantiation and storing a vector of
  handles. This also avoids the DeferredHandleScope completely, instead
  we just store two global handles in the code map.
- The interpreter gets the code table, function tables and signature
  tables directly from the attached wasm instance object. This ensures
  that the interpreter sees all updates to tables that might have been
  performed by external code.
- There is now common functionality for calling a code object. This is
  used for direct calls to imported functions and for all indirect
  calls. As these code objects can also be wasm functions which should
  be executed in the interpreter itself, I introduce a struct to hold
  the outcome of calling the code object, or a pointer to
  InterpreterCode to be called in the interpreter.

R=ahaas@chromium.org
BUG=v8:5822

Change-Id: I20fb2ea007e79e5fcff9afb4b1ca31739ebcb83f
Reviewed-on: https://chromium-review.googlesource.com/458417
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44059}

src/wasm/wasm-debug.cc

@@ -24,39 +24,16 @@ using namespace v8::internal::wasm;
 
 namespace {
 
-// Unwrap a wasm to js wrapper, return the callable heap object.
-// Only call this method for proper wrappers that do not throw a type error.
-HeapObject* UnwrapJSWrapper(Code* js_wrapper) {
-  int mask = RelocInfo::ModeMask(RelocInfo::EMBEDDED_OBJECT);
-  for (RelocIterator it(js_wrapper, mask);; it.next()) {
-    DCHECK(!it.done());
-    HeapObject* obj = it.rinfo()->target_object();
-    if (!obj->IsCallable()) continue;
-#ifdef DEBUG
-    // There should only be this reference to a callable object.
-    for (it.next(); !it.done(); it.next()) {
-      HeapObject* other = it.rinfo()->target_object();
-      DCHECK(!other->IsCallable());
-    }
-#endif
-    return obj;
-  }
-  UNREACHABLE();
-  return nullptr;
-}
-
 // Forward declaration.
 class InterpreterHandle;
 InterpreterHandle* GetInterpreterHandle(WasmDebugInfo* debug_info);
 
 class InterpreterHandle {
-  AccountingAllocator allocator_;
   WasmInstance instance_;
   WasmInterpreter interpreter_;
   Isolate* isolate_;
   StepAction next_step_action_ = StepNone;
   int last_step_stack_depth_ = 0;
-  DeferredHandles* import_handles_ = nullptr;
   std::unordered_map<Address, uint32_t> activations_;
 
   uint32_t StartActivation(Address frame_pointer) {
@@ -93,10 +70,20 @@ class InterpreterHandle {
   // WasmInterpreter has to be allocated in place, since it is not movable.
   InterpreterHandle(Isolate* isolate, WasmDebugInfo* debug_info)
       : instance_(debug_info->wasm_instance()->compiled_module()->module()),
-        interpreter_(GetBytesEnv(&instance_, debug_info), &allocator_),
+        interpreter_(isolate, GetBytesEnv(&instance_, debug_info)),
         isolate_(isolate) {
-    if (debug_info->wasm_instance()->has_memory_buffer()) {
-      JSArrayBuffer* mem_buffer = debug_info->wasm_instance()->memory_buffer();
+    DisallowHeapAllocation no_gc;
+
+    WasmInstanceObject* instance = debug_info->wasm_instance();
+
+    // Store a global handle to the wasm instance in the interpreter.
+    interpreter_.SetInstanceObject(instance);
+
+    // Set memory start pointer and size.
+    // TODO(wasm): Update this on grow_memory in both directions (compiled ->
+    // interpreter, interpreter -> compiles).
+    if (instance->has_memory_buffer()) {
+      JSArrayBuffer* mem_buffer = instance->memory_buffer();
       instance_.mem_start =
           reinterpret_cast<byte*>(mem_buffer->backing_store());
       CHECK(mem_buffer->byte_length()->ToUint32(&instance_.mem_size));
@@ -105,37 +92,16 @@ class InterpreterHandle {
       instance_.mem_start = nullptr;
       instance_.mem_size = 0;
     }
-    if (debug_info->wasm_instance()->has_globals_buffer()) {
-      instance_.globals_start = reinterpret_cast<byte*>(
-          debug_info->wasm_instance()->globals_buffer()->backing_store());
-    }
-    if (instance_.module->num_imported_functions > 0) {
-      int num_imported_functions =
-          static_cast<int>(instance_.module->num_imported_functions);
-      Handle<WasmCompiledModule> compiled_module(
-          debug_info->wasm_instance()->compiled_module(), isolate);
-      Handle<FixedArray> code_table = compiled_module->code_table();
-      DeferredHandleScope deferred_scope(isolate_);
-      instance_.context =
-          handle(compiled_module->ptr_to_native_context(), isolate_);
-      for (int i = 0; i < num_imported_functions; ++i) {
-        Code* code = Code::cast(code_table->get(i));
-        Handle<HeapObject> called_obj;
-        if (code->kind() == Code::WASM_FUNCTION) {
-          called_obj = handle(code, isolate_);
-        } else if (IsJSCompatibleSignature(
-                       instance_.module->functions[i].sig)) {
-          called_obj = handle(UnwrapJSWrapper(code), isolate_);
-        }
-        interpreter_.AddImportedFunction(called_obj);
-      }
-      import_handles_ = deferred_scope.Detach();
+
+    // Set pointer to globals storage.
+    if (instance->has_globals_buffer()) {
+      instance_.globals_start =
+          reinterpret_cast<byte*>(instance->globals_buffer()->backing_store());
     }
   }
 
   ~InterpreterHandle() {
     DCHECK_EQ(0, activations_.size());
-    delete import_handles_;  // might be nullptr, which is ok.
   }
 
   static ModuleBytesEnv GetBytesEnv(WasmInstance* instance,

src/wasm/wasm-interpreter.h

@@ -14,6 +14,8 @@ class AccountingAllocator;
 }
 
 namespace internal {
+class WasmInstanceObject;
+
 namespace wasm {
 
 // forward declarations.
@@ -185,7 +187,7 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
     uint32_t ActivationFrameBase(uint32_t activation_id);
   };
 
-  WasmInterpreter(const ModuleBytesEnv& env, AccountingAllocator* allocator);
+  WasmInterpreter(Isolate* isolate, const ModuleBytesEnv& env);
   ~WasmInterpreter();
 
   //==========================================================================
@@ -204,10 +206,12 @@ class V8_EXPORT_PRIVATE WasmInterpreter {
   // Enable or disable tracing for {function}. Return the previous state.
   bool SetTracing(const WasmFunction* function, bool enabled);
 
-  // Add an imported function.
-  // We store the passed Handle internally, so the caller must ensure that it
-  // stays valid at least as long as the WasmInterpreter.
-  void AddImportedFunction(Handle<HeapObject>);
+  // Set the associated wasm instance object.
+  // If the instance object has been set, some tables stored inside it are used
+  // instead of the tables stored in the WasmModule struct. This allows to call
+  // back and forth between the interpreter and outside code (JS or wasm
+  // compiled) without repeatedly copying information.
+  void SetInstanceObject(WasmInstanceObject*);
 
   //==========================================================================
   // Thread iteration and inspection.

test/cctest/wasm/wasm-run-utils.h

@@ -80,12 +80,12 @@ class TestingModule : public ModuleEnv {
         instance_(&module_),
         isolate_(CcTest::InitIsolateOnce()),
         global_offset(0),
-        interpreter_(mode == kExecuteInterpreted
-                         ? new WasmInterpreter(
-                               ModuleBytesEnv(&module_, &instance_,
-                                              Vector<const byte>::empty()),
-                               zone->allocator())
-                         : nullptr) {
+        interpreter_(
+            mode == kExecuteInterpreted
+                ? new WasmInterpreter(
+                      isolate_, ModuleBytesEnv(&module_, &instance_,
+                                               Vector<const byte>::empty()))
+                : nullptr) {
     WasmJs::Install(isolate_);
     instance->module = &module_;
     instance->globals_start = global_data;

test/common/wasm/wasm-module-runner.cc

@@ -139,7 +139,7 @@ int32_t InterpretWasmModule(Isolate* isolate, ErrorThrower* thrower,
   instance.globals_start = nullptr;
 
   ModuleBytesEnv env(module, &instance, wire_bytes);
-  WasmInterpreter interpreter(env, isolate->allocator());
+  WasmInterpreter interpreter(isolate, env);
 
   WasmInterpreter::Thread* thread = interpreter.GetThread(0);
   thread->Reset();

test/mjsunit/wasm/interpreter.js

@@ -225,3 +225,87 @@ function checkStack(stack, expected_lines) {
     }
   }
 })();
+
+(function testIndirectImports() {
+  var builder = new WasmModuleBuilder();
+
+  var sig_i_ii = builder.addType(kSig_i_ii);
+  var sig_i_i = builder.addType(kSig_i_i);
+  var mul = builder.addImport('q', 'mul', sig_i_ii);
+  var add = builder.addFunction('add', sig_i_ii).addBody([
+    kExprGetLocal, 0, kExprGetLocal, 1, kExprI32Add
+  ]);
+  var mismatch =
+      builder.addFunction('sig_mismatch', sig_i_i).addBody([kExprGetLocal, 0]);
+  var main = builder.addFunction('main', kSig_i_iii)
+                 .addBody([
+                   // Call indirect #0 with args <#1, #2>.
+                   kExprGetLocal, 1, kExprGetLocal, 2, kExprGetLocal, 0,
+                   kExprCallIndirect, sig_i_ii, kTableZero
+                 ])
+                 .exportFunc();
+  builder.appendToTable([mul, add.index, mismatch.index, main.index]);
+
+  var instance = builder.instantiate({q: {mul: (a, b) => a * b}});
+
+  // Call mul.
+  assertEquals(-6, instance.exports.main(0, -2, 3));
+  // Call add.
+  assertEquals(99, instance.exports.main(1, 22, 77));
+  // main and sig_mismatch have another signature.
+  assertTraps(kTrapFuncSigMismatch, () => instance.exports.main(2, 12, 33));
+  assertTraps(kTrapFuncSigMismatch, () => instance.exports.main(3, 12, 33));
+  // Function index 4 does not exist.
+  assertTraps(kTrapFuncInvalid, () => instance.exports.main(4, 12, 33));
+})();
+
+(function testIllegalImports() {
+  var builder = new WasmModuleBuilder();
+
+  var sig_l_v = builder.addType(kSig_l_v);
+  var imp = builder.addImport('q', 'imp', sig_l_v);
+  var direct = builder.addFunction('direct', kSig_l_v)
+                   .addBody([kExprCallFunction, imp])
+                   .exportFunc();
+  var indirect = builder.addFunction('indirect', kSig_l_v).addBody([
+    kExprI32Const, 0, kExprCallIndirect, sig_l_v, kTableZero
+  ]);
+  var main =
+      builder.addFunction('main', kSig_v_i)
+          .addBody([
+            // Call indirect #0 with arg #0, drop result.
+            kExprGetLocal, 0, kExprCallIndirect, sig_l_v, kTableZero, kExprDrop
+          ])
+          .exportFunc();
+  builder.appendToTable([imp, direct.index, indirect.index]);
+
+  var instance = builder.instantiate({q: {imp: () => 1}});
+
+  // Calling imported functions with i64 in signature should fail.
+  try {
+    // Via direct call.
+    instance.exports.main(1);
+  } catch (e) {
+    if (!(e instanceof TypeError)) throw e;
+    checkStack(stripPath(e.stack), [
+      'TypeError: invalid type',                                // -
+      '    at direct (<WASM>[1]+1)',                            // -
+      '    at main (<WASM>[3]+3)',                              // -
+      /^    at testIllegalImports \(interpreter.js:\d+:22\)$/,  // -
+      /^    at interpreter.js:\d+:3$/
+    ]);
+  }
+  try {
+    // Via indirect call.
+    instance.exports.main(2);
+  } catch (e) {
+    if (!(e instanceof TypeError)) throw e;
+    checkStack(stripPath(e.stack), [
+      'TypeError: invalid type',                                // -
+      '    at indirect (<WASM>[2]+1)',                          // -
+      '    at main (<WASM>[3]+3)',                              // -
+      /^    at testIllegalImports \(interpreter.js:\d+:22\)$/,  // -
+      /^    at interpreter.js:\d+:3$/
+    ]);
+  }
+})();