Reland "[wasm] Unify deoptimization data"

This is a reland of 236298ac.

Original change's description:
> [wasm] Unify deoptimization data
>
> Add methods to add deoptimization data and use them from all the places
> where we currently add them manually. Also add them to wasm-to-wasm
> wrappers compiled on table set, which was missing before, leading to
> the referenced bug.
>
> R=ahaas@chromium.org
>
> Bug: chromium:779292
> Change-Id: Ib9132d9faeb1092c46e22dd8196d201ce5c0942f
> Reviewed-on: https://chromium-review.googlesource.com/774838
> Reviewed-by: Andreas Haas <ahaas@chromium.org>
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#49452}

Bug: chromium:779292
Change-Id: I8219305fc894c50904db57e51245733f6613dcd3
Reviewed-on: https://chromium-review.googlesource.com/778159Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49508}

src/compiler/wasm-compiler.cc

@@ -4277,6 +4277,8 @@ Handle<Code> CompileJSToWasmWrapper(Isolate* isolate, wasm::WasmModule* module,
   return code;
 }
 
+namespace {
+
 void ValidateImportWrapperReferencesImmovables(Handle<Code> wrapper) {
 #if !DEBUG
   return;
@@ -4315,6 +4317,36 @@ void ValidateImportWrapperReferencesImmovables(Handle<Code> wrapper) {
   }
 }
 
+}  // namespace
+
+void AttachWasmFunctionInfo(Isolate* isolate, Handle<Code> code,
+                            MaybeHandle<WeakCell> weak_instance,
+                            int func_index) {
+  DCHECK(weak_instance.is_null() ||
+         weak_instance.ToHandleChecked()->value()->IsWasmInstanceObject());
+  Handle<FixedArray> deopt_data = isolate->factory()->NewFixedArray(2, TENURED);
+  if (!weak_instance.is_null()) {
+    // TODO(wasm): Introduce constants for the indexes in wasm deopt data.
+    deopt_data->set(0, *weak_instance.ToHandleChecked());
+  }
+  deopt_data->set(1, Smi::FromInt(func_index));
+
+  // TODO(6792): No longer needed once WebAssembly code is off heap.
+  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
+
+  code->set_deoptimization_data(*deopt_data);
+}
+
+void AttachWasmFunctionInfo(Isolate* isolate, Handle<Code> code,
+                            MaybeHandle<WasmInstanceObject> instance,
+                            int func_index) {
+  MaybeHandle<WeakCell> weak_instance;
+  if (!instance.is_null()) {
+    weak_instance = isolate->factory()->NewWeakCell(instance.ToHandleChecked());
+  }
+  AttachWasmFunctionInfo(isolate, code, weak_instance, func_index);
+}
+
 Handle<Code> CompileWasmToJSWrapper(
     Isolate* isolate, Handle<JSReceiver> target, wasm::FunctionSig* sig,
     uint32_t index, wasm::ModuleOrigin origin,

src/compiler/wasm-compiler.h

@@ -183,6 +183,21 @@ Handle<Code> CompileWasmInterpreterEntry(Isolate* isolate, uint32_t func_index,
                                          wasm::FunctionSig* sig,
                                          Handle<WasmInstanceObject> instance);
 
+// Attach function information in the form of deoptimization data to the given
+// code object. This information will be used for generating stack traces,
+// calling imported functions in the interpreter, knowing which function to
+// compile in a lazy compile stub, and more. The deopt data will be a newly
+// allocated FixedArray of length 2, where the first element is a WeakCell
+// containing the WasmInstanceObject, and the second element is the function
+// index.
+// If calling this method repeatedly for the same instance, pass a WeakCell
+// directly in order to avoid creating many cells pointing to the same instance.
+void AttachWasmFunctionInfo(Isolate*, Handle<Code>,
+                            MaybeHandle<WeakCell> weak_instance,
+                            int func_index);
+void AttachWasmFunctionInfo(Isolate*, Handle<Code>,
+                            MaybeHandle<WasmInstanceObject>, int func_index);
+
 enum CWasmEntryParameters {
   kCodeObject,
   kArgumentsBuffer,

src/wasm/module-compiler.cc

@@ -218,6 +218,8 @@ class ModuleCompiler {
   Handle<Code> centry_stub_;
 };
 
+namespace {
+
 class JSToWasmWrapperCache {
  public:
   void SetContextAddress(Address context_address) {
@@ -391,7 +393,7 @@ class InstanceBuilder {
 };
 
 // TODO(titzer): move to wasm-objects.cc
-static void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
+void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
   DisallowHeapAllocation no_gc;
   JSObject** p = reinterpret_cast<JSObject**>(data.GetParameter());
   WasmInstanceObject* owner = reinterpret_cast<WasmInstanceObject*>(*p);
@@ -465,6 +467,8 @@ static void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
   TRACE("}\n");
 }
 
+}  // namespace
+
 bool SyncValidate(Isolate* isolate, const ModuleWireBytes& bytes) {
   if (bytes.start() == nullptr || bytes.length() == 0) return false;
   ModuleResult result = SyncDecodeWasmModule(isolate, bytes.start(),
@@ -749,15 +753,7 @@ void LazyCompilationOrchestrator::CompileFunction(
   CHECK(!thrower.error());
   Handle<Code> code = maybe_code.ToHandleChecked();
 
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
-
-  Handle<FixedArray> deopt_data = isolate->factory()->NewFixedArray(2, TENURED);
-  Handle<WeakCell> weak_instance = isolate->factory()->NewWeakCell(instance);
-  // TODO(wasm): Introduce constants for the indexes in wasm deopt data.
-  deopt_data->set(0, *weak_instance);
-  deopt_data->set(1, Smi::FromInt(func_index));
-  code->set_deoptimization_data(*deopt_data);
+  compiler::AttachWasmFunctionInfo(isolate, code, instance, func_index);
 
   DCHECK_EQ(Builtins::kWasmCompileLazy,
             Code::cast(compiled_module->code_table()->get(func_index))
@@ -1263,21 +1259,11 @@ Handle<Code> EnsureExportedLazyDeoptData(Isolate* isolate,
   //   #1: func_index
   // might be extended later for table exports (see
   // EnsureTableExportLazyDeoptData).
-  Handle<FixedArray> deopt_data(code->deoptimization_data());
-  DCHECK_EQ(0, deopt_data->length() % 2);
-  if (deopt_data->length() == 0) {
+  DCHECK_EQ(0, code->deoptimization_data()->length() % 2);
+  if (code->deoptimization_data()->length() == 0) {
     code = isolate->factory()->CopyCode(code);
     code_table->set(func_index, *code);
-    deopt_data = isolate->factory()->NewFixedArray(2, TENURED);
-    // TODO(6792): No longer needed once WebAssembly code is off heap.
-    CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
-    code->set_deoptimization_data(*deopt_data);
-    if (!instance.is_null()) {
-      Handle<WeakCell> weak_instance =
-          isolate->factory()->NewWeakCell(instance);
-      deopt_data->set(0, *weak_instance);
-    }
-    deopt_data->set(1, Smi::FromInt(func_index));
+    compiler::AttachWasmFunctionInfo(isolate, code, instance, func_index);
   }
   DCHECK_IMPLIES(!instance.is_null(),
                  WeakCell::cast(code->deoptimization_data()->get(0))->value() ==
@@ -1361,18 +1347,11 @@ Handle<Code> MakeWasmToWasmWrapper(
   Handle<Code> wrapper_code = compiler::CompileWasmToWasmWrapper(
       isolate, wasm_code, *sig, imported_function->function_index(),
       new_wasm_context_address);
-  // TODO(6792): No longer needed once WebAssembly code is off heap.
-  CodeSpaceMemoryModificationScope modification_scope(isolate->heap());
   // Set the deoptimization data for the WasmToWasm wrapper. This is
   // needed by the interpreter to find the imported instance for
   // a cross-instance call.
-  Factory* factory = isolate->factory();
-  Handle<WeakCell> weak_link = factory->NewWeakCell(imported_instance);
-  Handle<FixedArray> deopt_data = factory->NewFixedArray(2, TENURED);
-  deopt_data->set(0, *weak_link);
-  auto function_index = Smi::FromInt(imported_function->function_index());
-  deopt_data->set(1, function_index);
-  wrapper_code->set_deoptimization_data(*deopt_data);
+  compiler::AttachWasmFunctionInfo(isolate, wrapper_code, imported_instance,
+                                   imported_function->function_index());
   return wrapper_code;
 }
 
@@ -1684,15 +1663,8 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
             // unique copy to attach updated deoptimization data.
             if (orig_code->deoptimization_data()->length() > 0) {
               Handle<Code> code = factory->CopyCode(orig_code);
-              Handle<FixedArray> deopt_data =
-                  factory->NewFixedArray(2, TENURED);
-              deopt_data->set(1, Smi::FromInt(i));
-              // TODO(6792): No longer needed once WebAssembly code is off heap.
-              {
-                CodeSpaceMemoryModificationScope modification_scope(
-                    isolate_->heap());
-                code->set_deoptimization_data(*deopt_data);
-              }
+              compiler::AttachWasmFunctionInfo(isolate_, code,
+                                               Handle<WasmInstanceObject>(), i);
               code_table->set(i, *code);
             }
             break;
@@ -1870,12 +1842,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
        i < num_functions; ++i) {
     Handle<Code> code = handle(Code::cast(code_table->get(i)), isolate_);
     if (code->kind() == Code::WASM_FUNCTION) {
-      Handle<FixedArray> deopt_data = factory->NewFixedArray(2, TENURED);
-      deopt_data->set(0, *weak_link);
-      deopt_data->set(1, Smi::FromInt(i));
-      // TODO(6792): No longer needed once WebAssembly code is off heap.
-      CodeSpaceMemoryModificationScope modification_scope(isolate_->heap());
-      code->set_deoptimization_data(*deopt_data);
+      compiler::AttachWasmFunctionInfo(isolate_, code, weak_link, i);
       continue;
     }
     DCHECK_EQ(Builtins::kWasmCompileLazy, code->builtin_index());

src/wasm/wasm-objects.cc

@@ -298,11 +298,14 @@ void WasmTableObject::Set(Isolate* isolate, Handle<WasmTableObject> table,
     value = function;
     // TODO(titzer): Make JSToWasm wrappers just call the WASM to WASM wrapper,
     // and then we can just reuse the WASM to WASM wrapper.
-    Address new_context_address = reinterpret_cast<Address>(
-        exported_function->instance()->wasm_context()->get());
+    Handle<WasmInstanceObject> instance(exported_function->instance(), isolate);
+    int func_index = exported_function->function_index();
+    Address new_context_address =
+        reinterpret_cast<Address>(instance->wasm_context()->get());
     code = compiler::CompileWasmToWasmWrapper(
         isolate, exported_function->GetWasmCode(), wasm_function->sig,
-        exported_function->function_index(), new_context_address);
+        func_index, new_context_address);
+    compiler::AttachWasmFunctionInfo(isolate, code, instance, func_index);
   }
 
   UpdateDispatchTables(isolate, dispatch_tables, index, wasm_function, code);

test/mjsunit/wasm/interpreter.js

@@ -449,3 +449,21 @@ function checkStack(stack, expected_lines) {
   const instance = builder2.instantiate({imp: {func: exp}});
   assertEquals(23, instance.exports.main());
 })();
+
+(function testTableCall() {
+  print(arguments.callee.name);
+  const builder1 = new WasmModuleBuilder();
+  builder1.addFunction('func', kSig_v_v).addBody([]).exportFunc();
+  const instance1 = builder1.instantiate();
+  const table = new WebAssembly.Table({element: 'anyfunc', initial: 2});
+
+  const builder2 = new WasmModuleBuilder()
+  builder2.addImportedTable('m', 'table');
+  const sig = builder2.addType(kSig_v_v);
+  builder2.addFunction('call_func', kSig_v_v)
+      .addBody([kExprI32Const, 0, kExprCallIndirect, sig, kTableZero])
+      .exportFunc();
+  const instance2 = builder2.instantiate({m: {table: table}});
+  table.set(0, instance1.exports.func);
+  instance2.exports.call_func();
+})();