[wasm] Construct WasmExportedFunctions for table elements lazily

We have to create WasmExportedFunction objects for any WebAssembly
function which may escape a WebAssembly instance. Up until now we
created these WasmExportedFunction objects eagerly during instantiation
time: for any exported function, and any element in an exported table we
create such an object.

With the anyref proposal, the table.get instruction can allow any
function in a table to escape its instance. Therefore we would have to
create a WasmExportedFunction object for any function which is put into
a table.

With this CL we create WasmExportedFunctions for table entries lazily.
We initialize tables with placeholders consisting of the instance and
the function index. If we encounter a placeholder in table.get, we
create the WasmExportedFunction for the expected function to return it.

R=mstarzinger@chromium.org
CC=titzer@chromium.org

Bug: v8:7581
Change-Id: I4f32bd7433285d0b04a22c0fb70b736bac55b3f1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1505575Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60115}

src/wasm/module-instantiate.cc

@@ -879,26 +879,41 @@ bool InstanceBuilder::ProcessImportedTable(Handle<WasmInstanceObject> instance,
   // Initialize the dispatch table with the (foreign) JS functions
   // that are already in the table.
   for (int i = 0; i < imported_table_size; ++i) {
+    // TODO(ahaas): Extract this code here into a function on WasmTableObject.
     Handle<Object> val(table_instance.js_functions->get(i), isolate_);
-    // TODO(mtrofin): this is the same logic as WasmTableObject::Set:
-    // insert in the local table a wrapper from the other module, and add
-    // a reference to the owning instance of the other module.
-    if (!val->IsJSFunction()) continue;
-    if (!WasmExportedFunction::IsWasmExportedFunction(*val)) {
+    Handle<WasmInstanceObject> target_instance;
+    int function_index;
+    FunctionSig* sig;
+    if (val->IsNull(isolate_)) {
+      continue;
+    } else if (WasmExportedFunction::IsWasmExportedFunction(*val)) {
+      auto target_func = Handle<WasmExportedFunction>::cast(val);
+      target_instance = handle(target_func->instance(), isolate_);
+      sig = target_func->sig();
+      function_index = target_func->function_index();
+    } else if (val->IsTuple2()) {
+      // {val} can be a {Tuple2} if no WasmExportedFunction has been
+      // constructed for the function yet, but the function exists.
+      auto tuple = Handle<Tuple2>::cast(val);
+      target_instance =
+          handle(WasmInstanceObject::cast(tuple->value1()), isolate_);
+      function_index = Smi::cast(tuple->value2()).value();
+      sig = target_instance->module_object()
+                ->module()
+                ->functions[function_index]
+                .sig;
+    } else {
       thrower_->LinkError("table import %d[%d] is not a wasm function",
                           import_index, i);
       return false;
     }
-    auto target_func = Handle<WasmExportedFunction>::cast(val);
-    Handle<WasmInstanceObject> target_instance =
-        handle(target_func->instance(), isolate_);
+
     // Look up the signature's canonical id. If there is no canonical
     // id, then the signature does not appear at all in this module,
     // so putting {-1} in the table will cause checks to always fail.
-    FunctionSig* sig = target_func->sig();
     IndirectFunctionTableEntry(instance, i)
         .Set(module_->signature_map.Find(*sig), target_instance,
-             target_func->function_index());
+             function_index);
   }
   return true;
 }
@@ -1485,6 +1500,33 @@ void InstanceBuilder::InitializeTables(Handle<WasmInstanceObject> instance) {
   }
 }
 
+namespace {
+Handle<WasmExportedFunction> CreateWasmExportedFunctionForAsm(
+    Isolate* isolate, Handle<WasmInstanceObject> instance,
+    const WasmModule* module, uint32_t func_index,
+    JSToWasmWrapperCache* js_to_wasm_cache) {
+  const WasmFunction* function = &module->functions[func_index];
+  DCHECK_EQ(module->origin, kAsmJsOrigin);
+
+  Handle<Code> wrapper_code = js_to_wasm_cache->GetOrCompileJSToWasmWrapper(
+      isolate, function->sig, function->imported);
+  auto module_object =
+      Handle<WasmModuleObject>(instance->module_object(), isolate);
+  WireBytesRef func_name_ref = module->LookupFunctionName(
+      ModuleWireBytes(module_object->native_module()->wire_bytes()),
+      func_index);
+  auto func_name = WasmModuleObject::ExtractUtf8StringFromModuleBytes(
+                       isolate, module_object, func_name_ref)
+                       .ToHandleChecked();
+  auto wasm_exported_function = WasmExportedFunction::New(
+      isolate, instance, func_name, func_index,
+      static_cast<int>(function->sig->parameter_count()), wrapper_code);
+  WasmInstanceObject::SetWasmExportedFunction(isolate, instance, func_index,
+                                              wasm_exported_function);
+  return wasm_exported_function;
+}
+}  // namespace
+
 bool LoadElemSegmentImpl(Isolate* isolate, Handle<WasmInstanceObject> instance,
                          const TableInstance& table_instance,
                          JSToWasmWrapperCache* js_to_wasm_cache,
@@ -1524,35 +1566,28 @@ bool LoadElemSegmentImpl(Isolate* isolate, Handle<WasmInstanceObject> instance,
           WasmInstanceObject::GetWasmExportedFunction(isolate, instance,
                                                       func_index);
       if (wasm_exported_function.is_null()) {
-        // No JSFunction entry yet exists for this function. Create one.
-        // TODO(titzer): We compile JS->wasm wrappers for functions are
-        // not exported but are in an exported table. This should be done
-        // at module compile time and cached instead.
-
-        Handle<Code> wrapper_code =
-            js_to_wasm_cache->GetOrCompileJSToWasmWrapper(
-                isolate, function->sig, function->imported);
-        MaybeHandle<String> func_name;
+        // No JSFunction entry yet exists for this function. Create a {Tuple2}
+        // holding the information to lazily allocate one (or eagerly allocate
+        // one for asm.js code).
         if (module->origin == kAsmJsOrigin) {
-          // For modules arising from asm.js, honor the names section.
-          auto module_object =
-              Handle<WasmModuleObject>(instance->module_object(), isolate);
-          WireBytesRef func_name_ref = module->LookupFunctionName(
-              ModuleWireBytes(module_object->native_module()->wire_bytes()),
-              func_index);
-          func_name = WasmModuleObject::ExtractUtf8StringFromModuleBytes(
-                          isolate, module_object, func_name_ref)
-                          .ToHandleChecked();
+          Handle<WasmExportedFunction> function =
+              CreateWasmExportedFunctionForAsm(isolate, instance, module,
+                                               func_index, js_to_wasm_cache);
+          table_instance.js_functions->set(entry_index, *function);
+        } else {
+          // TODO(ahaas): Handle all this Tuple2 stuff in a method of
+          // {WasmTableObject}.
+          // Put (instance, func_index) as a placeholder into the table_index.
+          // The {WasmExportedFunction} will be created lazily.
+          Handle<Tuple2> tuple = isolate->factory()->NewTuple2(
+              instance, Handle<Smi>(Smi::FromInt(func_index), isolate),
+              NOT_TENURED);
+          table_instance.js_functions->set(entry_index, *tuple);
         }
-        wasm_exported_function = WasmExportedFunction::New(
-            isolate, instance, func_name, func_index,
-            static_cast<int>(function->sig->parameter_count()), wrapper_code);
-        WasmInstanceObject::SetWasmExportedFunction(
-            isolate, instance, func_index,
-            wasm_exported_function.ToHandleChecked());
+      } else {
+        table_instance.js_functions->set(
+            entry_index, *wasm_exported_function.ToHandleChecked());
       }
-      table_instance.js_functions->set(
-          entry_index, *wasm_exported_function.ToHandleChecked());
       // UpdateDispatchTables() updates all other dispatch tables, since
       // we have not yet added the dispatch table we are currently building.
       WasmTableObject::UpdateDispatchTables(

src/wasm/wasm-js.cc

@@ -1373,21 +1373,25 @@ void WebAssemblyTableGet(const v8::FunctionCallbackInfo<v8::Value>& args) {
   ScheduledErrorThrower thrower(i_isolate, "WebAssembly.Table.get()");
   Local<Context> context = isolate->GetCurrentContext();
   EXTRACT_THIS(receiver, WasmTableObject);
-  i::Handle<i::FixedArray> array(receiver->elements(), i_isolate);
 
   uint32_t index;
   if (!EnforceUint32("Argument 0", args[0], context, &thrower, &index)) {
     return;
   }
 
-  v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
-  if (index >= static_cast<uint32_t>(array->length())) {
-    thrower.RangeError("Index out of bounds");
+  i::MaybeHandle<i::Object> maybe_result =
+      i::WasmTableObject::Get(i_isolate, receiver, index);
+
+  if (maybe_result.is_null()) {
+    // No result was produced, which means that an exception should have been
+    // thrown. We can just return in that case, the ScheduledErrorThrower will
+    // take care of transforming the exception into a scheduled exception.
+    CHECK(i_isolate->has_pending_exception());
     return;
   }
 
-  i::Handle<i::Object> value(array->get(static_cast<int>(index)), i_isolate);
-  return_value.Set(Utils::ToLocal(value));
+  v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
+  return_value.Set(Utils::ToLocal(maybe_result.ToHandleChecked()));
 }
 
 // WebAssembly.Table.set(num, JSFunction)

src/wasm/wasm-objects.cc

@@ -870,6 +870,61 @@ void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
   array->set(table_index, *function);
 }
 
+MaybeHandle<Object> WasmTableObject::Get(Isolate* isolate,
+                                         Handle<WasmTableObject> table,
+                                         int table_index) {
+  Handle<FixedArray> elements(table->elements(), isolate);
+  if (table_index >= elements->length()) {
+    isolate->Throw(*isolate->factory()->NewRangeError(
+        MessageTemplate::kWasmTrapFuncInvalid));
+    return MaybeHandle<Object>();
+  }
+  Handle<Object> element(elements->get(table_index), isolate);
+  if (WasmExportedFunction::IsWasmExportedFunction(*element)) {
+    return element;
+  }
+
+  if (element->IsNull(isolate)) {
+    return element;
+  }
+
+  // {element} is not a valid entry in the table. It has to be a placeholder
+  // for lazy initialization.
+  Handle<Tuple2> tuple = Handle<Tuple2>::cast(element);
+  auto instance = handle(WasmInstanceObject::cast(tuple->value1()), isolate);
+  int function_index = Smi::cast(tuple->value2()).value();
+
+  // Check if we already compiled a wrapper for the function but did not store
+  // it in the table slot yet.
+  MaybeHandle<Object> maybe_element =
+      WasmInstanceObject::GetWasmExportedFunction(isolate, instance,
+                                                  function_index);
+  if (maybe_element.ToHandle(&element)) {
+    elements->set(table_index, *element);
+    return element;
+  }
+
+  const WasmModule* module = instance->module_object()->module();
+  const WasmFunction& function = module->functions[function_index];
+  // Exported functions got their wrapper compiled during instantiation.
+  CHECK(!function.exported);
+  Handle<Code> wrapper_code =
+      compiler::CompileJSToWasmWrapper(isolate, function.sig, function.imported)
+          .ToHandleChecked();
+
+  MaybeHandle<String> function_name = WasmModuleObject::GetFunctionNameOrNull(
+      isolate, handle(instance->module_object(), isolate), function_index);
+
+  Handle<WasmExportedFunction> result = WasmExportedFunction::New(
+      isolate, instance, function_name, function_index,
+      static_cast<int>(function.sig->parameter_count()), wrapper_code);
+
+  elements->set(table_index, *result);
+  WasmInstanceObject::SetWasmExportedFunction(isolate, instance, function_index,
+                                              result);
+  return result;
+}
+
 void WasmTableObject::UpdateDispatchTables(
     Isolate* isolate, Handle<WasmTableObject> table, int table_index,
     wasm::FunctionSig* sig, Handle<WasmInstanceObject> target_instance,

src/wasm/wasm-objects.h

@@ -282,6 +282,10 @@ class WasmTableObject : public JSObject {
   static void Set(Isolate* isolate, Handle<WasmTableObject> table,
                   uint32_t index, Handle<JSFunction> function);
 
+  static MaybeHandle<Object> Get(Isolate* isolate,
+                                 Handle<WasmTableObject> table,
+                                 int table_index);
+
   static void UpdateDispatchTables(Isolate* isolate,
                                    Handle<WasmTableObject> table,
                                    int table_index, wasm::FunctionSig* sig,

test/mjsunit/wasm/js-api.js

@@ -641,9 +641,9 @@ assertEq(get.call(tbl1, 0), null);
 assertEq(get.call(tbl1, 0, Infinity), null);
 assertEq(get.call(tbl1, 1), null);
 assertEq(get.call(tbl1, 1.5), null);
-assertThrows(() => get.call(tbl1, 2), RangeError, /Index out of bounds/);
+assertThrows(() => get.call(tbl1, 2), RangeError, /invalid index into function table/);
 assertThrows(
-    () => get.call(tbl1, 2.5), RangeError, /Index out of bounds/);
+    () => get.call(tbl1, 2.5), RangeError, /invalid index into function table/);
 assertThrows(() => get.call(tbl1, -1), TypeError, /must be non-negative/);
 assertThrows(
     () => get.call(tbl1, Math.pow(2, 33)), TypeError,

test/mjsunit/wasm/table-get.js

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+(function testTableGetNonExportedFunction() {
+  print(arguments.callee.name);
+  const builder = new WasmModuleBuilder();
+  const table = builder.addTable(kWasmAnyFunc, 20).exportAs("table");
+  const f1 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 11]);
+  const f2 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 22]);
+  const offset = 3;
+  builder.addElementSegment(offset, false, [f1.index, f2.index], false);
+
+  const instance = builder.instantiate();
+
+  const table_function2 = instance.exports.table.get(offset + 1);
+  assertEquals(22, table_function2());
+})();
+
+(function testTableGetExportedFunction() {
+  print(arguments.callee.name);
+  const builder = new WasmModuleBuilder();
+  const table = builder.addTable(kWasmAnyFunc, 20).exportAs("table");
+  const f1 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 11]);
+  const f2 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 22])
+                    .exportAs("f2");
+  const offset = 3;
+  builder.addElementSegment(offset, false, [f1.index, f2.index], false);
+
+  const instance = builder.instantiate();
+
+  const table_function2 = instance.exports.table.get(offset + 1);
+  assertEquals(22, table_function2());
+})();
+
+(function testTableGetOverlappingSegments() {
+  print(arguments.callee.name);
+  const builder = new WasmModuleBuilder();
+  const table = builder.addTable(kWasmAnyFunc, 20).exportAs("table");
+  const f1 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 11]);
+  const f2 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 22]);
+  const offset = 3;
+  builder.addElementSegment(offset, false, [f1.index, f2.index], false);
+  builder.addElementSegment(offset + 1, false, [f1.index, f2.index], false);
+
+  const instance = builder.instantiate();
+
+  const table_function1 = instance.exports.table.get(offset + 1);
+  assertEquals(11, table_function1());
+})();
+
+(function testTableGetUniqueWrapperExportedFunction() {
+  print(arguments.callee.name);
+  const builder = new WasmModuleBuilder();
+  const table = builder.addTable(kWasmAnyFunc, 20).exportAs("table");
+  const f1 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 11]).exportAs("f1");
+  const f2 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 22]);
+  const offset = 3;
+  builder.addElementSegment(offset, false, [f1.index, f1.index, f1.index], false);
+
+  const instance = builder.instantiate();
+
+  assertEquals(undefined, instance.exports.f1.tag);
+  const my_tag = { hello: 15 };
+  instance.exports.f1.tag = my_tag;
+
+  assertSame(my_tag, instance.exports.table.get(offset).tag);
+  assertSame(my_tag, instance.exports.table.get(offset + 1).tag);
+  assertSame(my_tag, instance.exports.table.get(offset + 2).tag);
+})();
+
+(function testTableGetUniqueWrapperNonExportedFunction() {
+  print(arguments.callee.name);
+  const builder = new WasmModuleBuilder();
+  const table = builder.addTable(kWasmAnyFunc, 20).exportAs("table");
+  const f1 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 11]);
+  const f2 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 22]);
+  const offset = 3;
+  builder.addElementSegment(offset, false, [f1.index, f1.index, f1.index], false);
+
+  const instance = builder.instantiate();
+
+  assertEquals(undefined, instance.exports.table.get(offset).tag);
+  const my_tag = { hello: 15 };
+  instance.exports.table.get(offset).tag = my_tag;
+
+  assertSame(my_tag, instance.exports.table.get(offset + 1).tag);
+  assertSame(my_tag, instance.exports.table.get(offset + 2).tag);
+})();
+
+(function testTableGetEmptyValue() {
+  print(arguments.callee.name);
+  const builder = new WasmModuleBuilder();
+  const table = builder.addTable(kWasmAnyFunc, 20).exportAs("table");
+  const f1 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 11]);
+  const f2 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 22]);
+  const offset = 3;
+  builder.addElementSegment(offset, false, [f1.index, f1.index, f1.index], false);
+
+  const instance = builder.instantiate();
+  assertEquals(null, instance.exports.table.get(offset - 1));
+})();
+
+(function testTableGetOOB() {
+  print(arguments.callee.name);
+  const builder = new WasmModuleBuilder();
+  const size = 20;
+  const table = builder.addTable(kWasmAnyFunc, size).exportAs("table");
+  const f1 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 11]);
+  const f2 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 22]);
+  const offset = 3;
+  builder.addElementSegment(offset, false, [f1.index, f1.index, f1.index], false);
+
+  const instance = builder.instantiate();
+  assertThrows(() => instance.exports.table.get(size + 3), RangeError);
+})();
+
+(function testTableGetImportedFunction() {
+  print(arguments.callee.name);
+  const builder = new WasmModuleBuilder();
+  const size = 20;
+  const table = builder.addTable(kWasmAnyFunc, size).exportAs("table");
+  const import1 = builder.addImport("q", "fun", kSig_i_ii);
+  const f1 = builder.addFunction('f', kSig_i_v).addBody([kExprI32Const, 11]);
+  const offset = 3;
+  builder.addElementSegment(offset, false, [f1.index, import1], false);
+
+  const instance = builder.instantiate({q: {fun: () => 33}});
+  assertEquals(33, instance.exports.table.get(offset + 1)());
+})();