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

This reverts commit 25f03308.

Reason for revert: Fix the cause of bot failure and reland

Original change's description:
> Revert "[wasm] redirect wasm calls to js functions through a GCed table"
> 
> This reverts commit eb65f35e.
> 
> Reason for revert: Broke jetstream benchmark on android.
> 
> BUG=chromium:750828
> 
> Original change's description:
> > [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}
> 
> TBR=mtrofin@chromium.org,aseemgarg@google.com,aseemgarg@chromium.org,clemensh@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Change-Id: I26f49ee0a1fe73cc5d8852ded87b56638be39ebf
> Reviewed-on: https://chromium-review.googlesource.com/596268
> Commit-Queue: Aseem Garg <aseemgarg@chromium.org>
> Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#47059}

R=mtrofin@chromium.org,aseemgarg@google.com,aseemgarg@chromium.org,clemensh@chromium.org,sullivan@chromium.org

Change-Id: I29ef35f6e612a706d9f571da3e7beb1da8b5052b
Bug: chromium:750828
Reviewed-on: https://chromium-review.googlesource.com/597010
Commit-Queue: Aseem Garg <aseemgarg@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47177}

src/compiler/wasm-compiler.cc

@@ -2751,7 +2751,9 @@ int WasmGraphBuilder::AddParameterNodes(Node** args, int pos, int param_count,
   return pos;
 }
 
-void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target) {
+bool WasmGraphBuilder::BuildWasmToJSWrapper(
+    Handle<JSReceiver> target, Handle<FixedArray> global_js_imports_table,
+    int index) {
   DCHECK(target->IsCallable());
 
   int wasm_count = static_cast<int>(sig_->parameter_count());
@@ -2773,7 +2775,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);
@@ -2782,11 +2784,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()) {
@@ -2818,7 +2840,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
@@ -2853,6 +2875,7 @@ void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target) {
                   : FromJS(call, HeapConstant(isolate->native_context()),
                            sig_->GetReturn());
   Return(val);
+  return true;
 }
 
 namespace {
@@ -3858,11 +3881,10 @@ Handle<Code> CompileJSToWasmWrapper(Isolate* isolate, wasm::WasmModule* module,
   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
   //----------------------------------------------------------------------------
@@ -3884,7 +3906,9 @@ Handle<Code> CompileWasmToJSWrapper(Isolate* isolate, Handle<JSReceiver> target,
                            source_position_table);
   builder.set_control_ptr(&control);
   builder.set_effect_ptr(&effect);
-  builder.BuildWasmToJSWrapper(target);
+  if (builder.BuildWasmToJSWrapper(target, global_js_imports_table, index)) {
+    global_js_imports_table->set(index, *target);
+  }
 
   Handle<Code> code = Handle<Code>::null();
   {
@@ -3918,6 +3942,15 @@ 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);
+    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, wasm::WasmModule* module,
@@ -218,7 +222,9 @@ class WasmGraphBuilder {
                      wasm::WasmCodePosition position);
 
   void BuildJSToWasmWrapper(Handle<Code> wasm_code);
-  void BuildWasmToJSWrapper(Handle<JSReceiver> target);
+  bool BuildWasmToJSWrapper(Handle<JSReceiver> target,
+                            Handle<FixedArray> global_js_imports_table,
+                            int index);
   void BuildWasmInterpreterEntry(uint32_t func_index,
                                  Handle<WasmInstanceObject> instance);
 

src/wasm/module-compiler.cc

@@ -506,7 +506,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();
@@ -521,7 +522,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() {
@@ -1253,6 +1255,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) {
@@ -1288,7 +1297,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 an illegal JS import 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");
@@ -402,7 +405,6 @@ void wasm::UpdateDispatchTables(Isolate* isolate,
   }
 }
 
-
 void wasm::TableSet(ErrorThrower* thrower, Isolate* isolate,
                     Handle<WasmTableObject> table, int64_t index,
                     Handle<JSFunction> function) {

src/wasm/wasm-objects.h

@@ -152,6 +152,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,
@@ -161,6 +162,7 @@ class WasmInstanceObject : public JSObject {
     kGlobalsBufferIndex,
     kDebugInfoIndex,
     kDirectlyCalledInstancesIndex,
+    kJsImportsTableIndex,
     kFieldCount
   };
 
@@ -172,6 +174,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();
@@ -704,6 +707,7 @@ OPTIONAL_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;
   }