[wasm] redirect wasm calls to js functions through a GCed table

With this patch, rather than embedding the JSReceiver address directly
in the WasmToJS wrappers, we put that in a fixed array with global handle
scope and instead embed the location of the handle and the index in the
wrapper. This ensures that the wrapper doesn't need to be patched if the
GC kicks in. This is needed to get the WASM code off the GCed heap.

R=mtrofin@chromium.org

Bug: 
Change-Id: Ie5a77a78cdecec51b04f702c63b8e4285e6a2d8d
Reviewed-on: https://chromium-review.googlesource.com/581682
Commit-Queue: Aseem Garg <aseemgarg@chromium.org>
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46884}

src/compiler/wasm-compiler.cc

@@ -2754,8 +2754,9 @@ int WasmGraphBuilder::AddParameterNodes(Node** args, int pos, int param_count,
   return pos;
 }
 
-void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target,
-                                            wasm::FunctionSig* sig) {
+bool WasmGraphBuilder::BuildWasmToJSWrapper(
+    Handle<JSReceiver> target, wasm::FunctionSig* sig,
+    Handle<FixedArray> global_js_imports_table, int index) {
   DCHECK(target->IsCallable());
 
   int wasm_count = static_cast<int>(sig->parameter_count());
@@ -2777,7 +2778,7 @@ void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target,
     // We don't need to return a value here, as the runtime call will not return
     // anyway (the c entry stub will trigger stack unwinding).
     ReturnVoid();
-    return;
+    return false;
   }
 
   Node** args = Buffer(wasm_count + 7);
@@ -2786,11 +2787,31 @@ void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target,
 
   BuildModifyThreadInWasmFlag(false);
 
+  // We add the target function to a table and look it up during runtime. This
+  // ensures that if the GC kicks in, it doesn't need to patch the code for the
+  // JS function.
+  // js_imports_table is fixed array with global handle scope whose lifetime is
+  // tied to the instance.
+  // TODO(aseemgarg): explore using per-import global handle instead of a table
+  Node* js_table = jsgraph()->IntPtrConstant(
+      reinterpret_cast<intptr_t>(global_js_imports_table.location()));
+  Node* load_table = graph()->NewNode(
+      jsgraph()->machine()->Load(LoadRepresentation::TaggedPointer()), js_table,
+      jsgraph()->IntPtrConstant(0), *effect_, *control_);
+  *effect_ = load_table;
+  int offset =
+      global_js_imports_table->OffsetOfElementAt(index) - kHeapObjectTag;
+  Node* offset_node = jsgraph()->Int32Constant(offset);
+  Node* target_address = graph()->NewNode(
+      jsgraph()->machine()->Load(LoadRepresentation::TaggedPointer()),
+      load_table, offset_node, *effect_, *control_);
+  *effect_ = target_address;
+
   if (target->IsJSFunction()) {
     Handle<JSFunction> function = Handle<JSFunction>::cast(target);
     if (function->shared()->internal_formal_parameter_count() == wasm_count) {
       int pos = 0;
-      args[pos++] = jsgraph()->Constant(target);  // target callable.
+      args[pos++] = target_address;  // target callable.
       // Receiver.
       if (is_sloppy(function->shared()->language_mode()) &&
           !function->shared()->native()) {
@@ -2822,7 +2843,7 @@ void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target,
     int pos = 0;
     Callable callable = CodeFactory::Call(isolate);
     args[pos++] = jsgraph()->HeapConstant(callable.code());
-    args[pos++] = jsgraph()->Constant(target);           // target callable
+    args[pos++] = target_address;                        // target callable
     args[pos++] = jsgraph()->Int32Constant(wasm_count);  // argument count
     args[pos++] = jsgraph()->Constant(
         handle(isolate->heap()->undefined_value(), isolate));  // receiver
@@ -2857,6 +2878,7 @@ void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target,
                   : FromJS(call, HeapConstant(isolate->native_context()),
                            sig->GetReturn());
   Return(val);
+  return true;
 }
 
 void WasmGraphBuilder::BuildWasmInterpreterEntry(
@@ -3846,11 +3868,10 @@ Handle<Code> CompileJSToWasmWrapper(Isolate* isolate,
   return code;
 }
 
-Handle<Code> CompileWasmToJSWrapper(Isolate* isolate, Handle<JSReceiver> target,
-                                    wasm::FunctionSig* sig, uint32_t index,
-                                    Handle<String> module_name,
-                                    MaybeHandle<String> import_name,
-                                    wasm::ModuleOrigin origin) {
+Handle<Code> CompileWasmToJSWrapper(
+    Isolate* isolate, Handle<JSReceiver> target, wasm::FunctionSig* sig,
+    uint32_t index, Handle<String> module_name, MaybeHandle<String> import_name,
+    wasm::ModuleOrigin origin, Handle<FixedArray> global_js_imports_table) {
   //----------------------------------------------------------------------------
   // Create the Graph
   //----------------------------------------------------------------------------
@@ -3872,7 +3893,10 @@ Handle<Code> CompileWasmToJSWrapper(Isolate* isolate, Handle<JSReceiver> target,
                            source_position_table);
   builder.set_control_ptr(&control);
   builder.set_effect_ptr(&effect);
-  builder.BuildWasmToJSWrapper(target, sig);
+  if (builder.BuildWasmToJSWrapper(target, sig, global_js_imports_table,
+                                   index)) {
+    global_js_imports_table->set(index, *target);
+  }
 
   Handle<Code> code = Handle<Code>::null();
   {
@@ -3906,6 +3930,18 @@ Handle<Code> CompileWasmToJSWrapper(Isolate* isolate, Handle<JSReceiver> target,
     CompilationInfo info(func_name, isolate, &zone, flags);
     code = Pipeline::GenerateCodeForTesting(&info, incoming, &graph, nullptr,
                                             source_position_table);
+    if (FLAG_wasm_interpret_all) {
+      Handle<FixedArray> deopt_data =
+          isolate->factory()->NewFixedArray(2, TENURED);
+      intptr_t loc =
+          reinterpret_cast<intptr_t>(global_js_imports_table.location());
+      Handle<Object> loc_handle =
+          isolate->factory()->NewHeapNumberFromBits(loc);
+      deopt_data->set(0, *loc_handle);
+      Handle<Object> index_handle = isolate->factory()->NewNumberFromInt(index);
+      deopt_data->set(1, *index_handle);
+      code->set_deoptimization_data(*deopt_data);
+    }
 #ifdef ENABLE_DISASSEMBLER
     if (FLAG_print_opt_code && !code.is_null()) {
       OFStream os(stdout);

src/compiler/wasm-compiler.h

@@ -107,11 +107,15 @@ class WasmCompilationUnit final {
 };
 
 // Wraps a JS function, producing a code object that can be called from wasm.
+// The global_js_imports_table is a global handle to a fixed array of target
+// JSReceiver with the lifetime tied to the module. We store it's location (non
+// GCable) in the generated code so that it can reside outside of GCed heap.
 Handle<Code> CompileWasmToJSWrapper(Isolate* isolate, Handle<JSReceiver> target,
                                     wasm::FunctionSig* sig, uint32_t index,
                                     Handle<String> module_name,
                                     MaybeHandle<String> import_name,
-                                    wasm::ModuleOrigin origin);
+                                    wasm::ModuleOrigin origin,
+                                    Handle<FixedArray> global_js_imports_table);
 
 // Wraps a given wasm code object, producing a code object.
 Handle<Code> CompileJSToWasmWrapper(Isolate* isolate,
@@ -218,7 +222,9 @@ class WasmGraphBuilder {
                      wasm::WasmCodePosition position);
 
   void BuildJSToWasmWrapper(Handle<Code> wasm_code, wasm::FunctionSig* sig);
-  void BuildWasmToJSWrapper(Handle<JSReceiver> target, wasm::FunctionSig* sig);
+  bool BuildWasmToJSWrapper(Handle<JSReceiver> target, wasm::FunctionSig* sig,
+                            Handle<FixedArray> global_js_imports_table,
+                            int index);
   void BuildWasmInterpreterEntry(uint32_t func_index, wasm::FunctionSig* sig,
                                  Handle<WasmInstanceObject> instance);
 

src/wasm/module-compiler.cc

@@ -514,7 +514,8 @@ using WasmInstanceMap =
 Handle<Code> UnwrapOrCompileImportWrapper(
     Isolate* isolate, int index, FunctionSig* sig, Handle<JSReceiver> target,
     Handle<String> module_name, MaybeHandle<String> import_name,
-    ModuleOrigin origin, WasmInstanceMap* imported_instances) {
+    ModuleOrigin origin, WasmInstanceMap* imported_instances,
+    Handle<FixedArray> js_imports_table) {
   WasmFunction* other_func = GetWasmFunctionForImportWrapper(isolate, target);
   if (other_func) {
     if (!sig->Equals(other_func->sig)) return Handle<Code>::null();
@@ -529,7 +530,8 @@ Handle<Code> UnwrapOrCompileImportWrapper(
   // No wasm function or being debugged. Compile a new wrapper for the new
   // signature.
   return compiler::CompileWasmToJSWrapper(isolate, target, sig, index,
-                                          module_name, import_name, origin);
+                                          module_name, import_name, origin,
+                                          js_imports_table);
 }
 
 double MonotonicallyIncreasingTimeInMs() {
@@ -1265,6 +1267,13 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,
                                     Handle<WasmInstanceObject> instance) {
   int num_imported_functions = 0;
   int num_imported_tables = 0;
+  Handle<FixedArray> func_table = isolate_->factory()->NewFixedArray(
+      static_cast<int>(module_->import_table.size()), TENURED);
+  Handle<FixedArray> js_imports_table =
+      isolate_->global_handles()->Create(*func_table);
+  Handle<Foreign> js_imports_foreign = isolate_->factory()->NewForeign(
+      reinterpret_cast<Address>(js_imports_table.location()), TENURED);
+  instance->set_js_imports_table(*js_imports_foreign);
   WasmInstanceMap imported_wasm_instances(isolate_->heap());
   for (int index = 0; index < static_cast<int>(module_->import_table.size());
        ++index) {
@@ -1300,7 +1309,7 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,
         Handle<Code> import_wrapper = UnwrapOrCompileImportWrapper(
             isolate_, index, module_->functions[import.index].sig,
             Handle<JSReceiver>::cast(value), module_name, import_name,
-            module_->origin(), &imported_wasm_instances);
+            module_->origin(), &imported_wasm_instances, js_imports_table);
         if (import_wrapper.is_null()) {
           ReportLinkError("imported function does not match the expected type",
                           index, module_name, import_name);

src/wasm/wasm-interpreter.cc

@@ -634,23 +634,24 @@ const char* OpcodeName(uint32_t val) {
 Handle<HeapObject> UnwrapWasmToJSWrapper(Isolate* isolate,
                                          Handle<Code> js_wrapper) {
   DCHECK_EQ(Code::WASM_TO_JS_FUNCTION, js_wrapper->kind());
-  int mask = RelocInfo::ModeMask(RelocInfo::EMBEDDED_OBJECT);
-  for (RelocIterator it(*js_wrapper, mask); !it.done(); it.next()) {
-    HeapObject* obj = it.rinfo()->target_object();
-    if (!obj->IsCallable()) continue;
-#ifdef DEBUG
-    // There should only be this one reference to a callable object.
-    for (it.next(); !it.done(); it.next()) {
-      HeapObject* other = it.rinfo()->target_object();
-      DCHECK(!other->IsCallable());
-    }
-#endif
-    return handle(obj, isolate);
+  Handle<FixedArray> deopt_data(js_wrapper->deoptimization_data(), isolate);
+  DCHECK_EQ(2, deopt_data->length());
+  intptr_t js_imports_table_loc = static_cast<intptr_t>(
+      HeapNumber::cast(deopt_data->get(0))->value_as_bits());
+  Handle<FixedArray> js_imports_table(
+      reinterpret_cast<FixedArray**>(js_imports_table_loc));
+  int index = 0;
+  CHECK(deopt_data->get(1)->ToInt32(&index));
+  DCHECK_GT(js_imports_table->length(), index);
+  Handle<Object> obj(js_imports_table->get(index), isolate);
+  if (obj->IsCallable()) {
+    return Handle<HeapObject>::cast(obj);
+  } else {
+    // If we did not find a callable object, this is not from wasm and obj must
+    // be undefined.
+    DCHECK(obj->IsUndefined(isolate));
+    return Handle<HeapObject>::null();
   }
-  // If we did not find a callable object, then there must be a reference to
-  // the WasmThrowTypeError runtime function.
-  // TODO(clemensh): Check that this is the case.
-  return Handle<HeapObject>::null();
 }
 
 class SideTable;

src/wasm/wasm-module.cc

@@ -179,6 +179,9 @@ static void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
     TRACE_CHAIN(wasm_module->compiled_module());
     TRACE("}\n");
   }
+  Foreign* js_imports_foreign = owner->js_imports_table();
+  GlobalHandles::Destroy(
+      reinterpret_cast<Object**>(js_imports_foreign->foreign_address()));
   compiled_module->reset_weak_owning_instance();
   GlobalHandles::Destroy(reinterpret_cast<Object**>(p));
   TRACE("}\n");
@@ -399,7 +402,6 @@ void wasm::UpdateDispatchTables(Isolate* isolate,
   }
 }
 
-
 void wasm::TableSet(ErrorThrower* thrower, Isolate* isolate,
                     Handle<WasmTableObject> table, int32_t index,
                     Handle<JSFunction> function) {

src/wasm/wasm-objects.h

@@ -155,6 +155,7 @@ class WasmInstanceObject : public JSObject {
   DECL_OPTIONAL_ACCESSORS(debug_info, WasmDebugInfo)
   // FixedArray of all instances whose code was imported
   DECL_OPTIONAL_ACCESSORS(directly_called_instances, FixedArray)
+  DECL_ACCESSORS(js_imports_table, Foreign)
 
   enum {  // --
     kCompiledModuleIndex,
@@ -163,6 +164,7 @@ class WasmInstanceObject : public JSObject {
     kGlobalsBufferIndex,
     kDebugInfoIndex,
     kDirectlyCalledInstancesIndex,
+    kJsImportsTableIndex,
     kFieldCount
   };
 
@@ -173,6 +175,7 @@ class WasmInstanceObject : public JSObject {
   DEF_OFFSET(GlobalsBuffer)
   DEF_OFFSET(DebugInfo)
   DEF_OFFSET(DirectlyCalledInstances)
+  DEF_OFFSET(JsImportsTable)
 
   WasmModuleObject* module_object();
   V8_EXPORT_PRIVATE wasm::WasmModule* module();
@@ -688,6 +691,7 @@ ACCESSORS(WasmInstanceObject, globals_buffer, JSArrayBuffer,
 ACCESSORS(WasmInstanceObject, debug_info, WasmDebugInfo, kDebugInfoOffset)
 ACCESSORS(WasmInstanceObject, directly_called_instances, FixedArray,
           kDirectlyCalledInstancesOffset)
+ACCESSORS(WasmInstanceObject, js_imports_table, Foreign, kJsImportsTableOffset)
 
 // WasmSharedModuleData
 ACCESSORS(WasmSharedModuleData, module_bytes, SeqOneByteString,

test/cctest/wasm/test-run-wasm-js.cc

@@ -46,7 +46,8 @@ class PredictableInputValues {
   }
 };
 
-uint32_t AddJSSelector(TestingModule* module, FunctionSig* sig, int which) {
+uint32_t AddJSSelector(TestingModule* module, FunctionSig* sig, int which,
+                       Handle<FixedArray> js_imports_table) {
   const int kMaxParams = 11;
   static const char* formals[kMaxParams] = {"",
                                             "a",
@@ -67,7 +68,7 @@ uint32_t AddJSSelector(TestingModule* module, FunctionSig* sig, int which) {
   SNPrintF(source, "(function(%s) { return %c; })",
            formals[sig->parameter_count()], param);
 
-  return module->AddJsFunction(sig, source.start());
+  return module->AddJsFunction(sig, source.start(), js_imports_table);
 }
 
 void EXPECT_CALL(double expected, Handle<JSFunction> jsfunc,
@@ -136,8 +137,10 @@ TEST(Run_I32Popcount_jswrapped) {
 TEST(Run_CallJS_Add_jswrapped) {
   WasmRunner<int, int> r(kExecuteCompiled);
   TestSignatures sigs;
-  uint32_t js_index =
-      r.module().AddJsFunction(sigs.i_i(), "(function(a) { return a + 99; })");
+  Handle<FixedArray> js_imports_table =
+      r.main_isolate()->factory()->NewFixedArray(2, TENURED);
+  uint32_t js_index = r.module().AddJsFunction(
+      sigs.i_i(), "(function(a) { return a + 99; })", js_imports_table);
   BUILD(r, WASM_CALL_FUNCTION(js_index, WASM_GET_LOCAL(0)));
 
   Handle<JSFunction> jsfunc = r.module().WrapCode(r.function()->func_index);
@@ -158,7 +161,10 @@ void RunJSSelectTest(int which) {
     FunctionSig sig(1, num_params, types);
 
     WasmRunner<void> r(kExecuteCompiled);
-    uint32_t js_index = AddJSSelector(&r.module(), &sig, which);
+    Handle<FixedArray> js_imports_table =
+        scope.isolate()->factory()->NewFixedArray(2, TENURED);
+    uint32_t js_index =
+        AddJSSelector(&r.module(), &sig, which, js_imports_table);
     WasmFunctionCompiler& t = r.NewFunction(&sig);
 
     {
@@ -416,7 +422,9 @@ void RunJSSelectAlignTest(int num_args, int num_params) {
   // Call different select JS functions.
   for (int which = 0; which < num_params; which++) {
     WasmRunner<void> r(kExecuteCompiled);
-    uint32_t js_index = AddJSSelector(&r.module(), &sig, which);
+    Handle<FixedArray> js_imports_table = factory->NewFixedArray(2, TENURED);
+    uint32_t js_index =
+        AddJSSelector(&r.module(), &sig, which, js_imports_table);
     CHECK_EQ(predicted_js_index, js_index);
     WasmFunctionCompiler& t = r.NewFunction(&sig);
     t.Build(&code[0], &code[end]);

test/cctest/wasm/test-wasm-stack.cc

@@ -79,9 +79,12 @@ TEST(CollectDetailedWasmStack_ExplicitThrowFromJs) {
   WasmRunner<void> r(kExecuteCompiled);
   TestSignatures sigs;
 
+  Handle<FixedArray> js_imports_table =
+      r.main_isolate()->factory()->NewFixedArray(2, TENURED);
   uint32_t js_throwing_index = r.module().AddJsFunction(
       sigs.v_v(),
-      "(function js() {\n function a() {\n throw new Error(); };\n a(); })");
+      "(function js() {\n function a() {\n throw new Error(); };\n a(); })",
+      js_imports_table);
 
   // Add a nop such that we don't always get position 1.
   BUILD(r, WASM_NOP, WASM_CALL_FUNCTION0(js_throwing_index));

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

@@ -209,13 +209,14 @@ class TestingModule : public ModuleEnv {
     return index;
   }
 
-  uint32_t AddJsFunction(FunctionSig* sig, const char* source) {
+  uint32_t AddJsFunction(FunctionSig* sig, const char* source,
+                         Handle<FixedArray> js_imports_table) {
     Handle<JSFunction> jsfunc = Handle<JSFunction>::cast(v8::Utils::OpenHandle(
         *v8::Local<v8::Function>::Cast(CompileRun(source))));
     uint32_t index = AddFunction(sig, Handle<Code>::null(), nullptr);
     Handle<Code> code = CompileWasmToJSWrapper(
         isolate_, jsfunc, sig, index, Handle<String>::null(),
-        Handle<String>::null(), module->origin());
+        Handle<String>::null(), module->origin(), js_imports_table);
     instance->function_code[index] = code;
     return index;
   }