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

This reverts commit 649b1e70e7.

Reason for revert: A1 Jetstream bots are still failing.

Original change's description:
> [wasm] Reland "[wasm] redirect wasm calls to js functions through a GCed table"
>
> This reverts commit 25f03308a7.
>
> 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 eb65f35e96.
> >
> > 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}

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

Bug: chromium:750828
Change-Id: I04b12c0eb0705ad809822a7d7461423be77d942a
Reviewed-on: https://chromium-review.googlesource.com/606867
Commit-Queue: Aseem Garg <aseemgarg@chromium.org>
Reviewed-by: Aseem Garg <aseemgarg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47231}

src/compiler/wasm-compiler.cc

@@ -2750,9 +2750,7 @@ int WasmGraphBuilder::AddParameterNodes(Node** args, int pos, int param_count,
   return pos;
 }
 
-bool WasmGraphBuilder::BuildWasmToJSWrapper(
+void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target) {
-    Handle<JSReceiver> target, Handle<FixedArray> global_js_imports_table,
-    int index) {
   DCHECK(target->IsCallable());
 
   int wasm_count = static_cast<int>(sig_->parameter_count());
@@ -2774,7 +2772,7 @@ bool WasmGraphBuilder::BuildWasmToJSWrapper(
     // 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 false;
+    return;
   }
 
   Node** args = Buffer(wasm_count + 7);
@@ -2783,31 +2781,11 @@ bool WasmGraphBuilder::BuildWasmToJSWrapper(
 
   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++] = target_address;  // target callable.
+      args[pos++] = jsgraph()->Constant(target);  // target callable.
       // Receiver.
       if (is_sloppy(function->shared()->language_mode()) &&
           !function->shared()->native()) {
@@ -2839,7 +2817,7 @@ bool WasmGraphBuilder::BuildWasmToJSWrapper(
     int pos = 0;
     Callable callable = CodeFactory::Call(isolate);
     args[pos++] = jsgraph()->HeapConstant(callable.code());
-    args[pos++] = target_address;                        // target callable
+    args[pos++] = jsgraph()->Constant(target);           // target callable
     args[pos++] = jsgraph()->Int32Constant(wasm_count);  // argument count
     args[pos++] = jsgraph()->Constant(
         handle(isolate->heap()->undefined_value(), isolate));  // receiver
@@ -2874,7 +2852,6 @@ bool WasmGraphBuilder::BuildWasmToJSWrapper(
                   : FromJS(call, HeapConstant(isolate->native_context()),
                            sig_->GetReturn());
   Return(val);
-  return true;
 }
 
 namespace {
@@ -3957,10 +3934,11 @@ Handle<Code> CompileJSToWasmWrapper(Isolate* isolate, wasm::WasmModule* module,
   return code;
 }
 
-Handle<Code> CompileWasmToJSWrapper(
+Handle<Code> CompileWasmToJSWrapper(Isolate* isolate, Handle<JSReceiver> target,
-    Isolate* isolate, Handle<JSReceiver> target, wasm::FunctionSig* sig,
+                                    wasm::FunctionSig* sig, uint32_t index,
-    uint32_t index, Handle<String> module_name, MaybeHandle<String> import_name,
+                                    Handle<String> module_name,
-    wasm::ModuleOrigin origin, Handle<FixedArray> global_js_imports_table) {
+                                    MaybeHandle<String> import_name,
+                                    wasm::ModuleOrigin origin) {
   //----------------------------------------------------------------------------
   // Create the Graph
   //----------------------------------------------------------------------------
@@ -3982,9 +3960,7 @@ Handle<Code> CompileWasmToJSWrapper(
                            source_position_table);
   builder.set_control_ptr(&control);
   builder.set_effect_ptr(&effect);
-  if (builder.BuildWasmToJSWrapper(target, global_js_imports_table, index)) {
+  builder.BuildWasmToJSWrapper(target);
-    global_js_imports_table->set(index, *target);
-  }
 
   Handle<Code> code = Handle<Code>::null();
   {
@@ -4018,15 +3994,6 @@ Handle<Code> CompileWasmToJSWrapper(
     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

@@ -103,15 +103,11 @@ 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,
@@ -230,9 +226,7 @@ class WasmGraphBuilder {
                      wasm::WasmCodePosition position);
 
   void BuildJSToWasmWrapper(Handle<Code> wasm_code);
-  bool BuildWasmToJSWrapper(Handle<JSReceiver> target,
+  void BuildWasmToJSWrapper(Handle<JSReceiver> target);
-                            Handle<FixedArray> global_js_imports_table,
-                            int index);
   void BuildWasmInterpreterEntry(uint32_t func_index,
                                  Handle<WasmInstanceObject> instance);
   void BuildCWasmEntry();

src/wasm/module-compiler.cc

@@ -497,8 +497,7 @@ 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();
@@ -513,8 +512,7 @@ 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() {
@@ -1260,13 +1258,6 @@ 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) {
@@ -1302,7 +1293,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, js_imports_table);
+            module_->origin(), &imported_wasm_instances);
         if (import_wrapper.is_null()) {
           ReportLinkError("imported function does not match the expected type",
                           index, module_name, import_name);

src/wasm/wasm-interpreter.cc

@@ -667,24 +667,23 @@ 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());
-  Handle<FixedArray> deopt_data(js_wrapper->deoptimization_data(), isolate);
+  int mask = RelocInfo::ModeMask(RelocInfo::EMBEDDED_OBJECT);
-  DCHECK_EQ(2, deopt_data->length());
+  for (RelocIterator it(*js_wrapper, mask); !it.done(); it.next()) {
-  intptr_t js_imports_table_loc = static_cast<intptr_t>(
+    HeapObject* obj = it.rinfo()->target_object();
-      HeapNumber::cast(deopt_data->get(0))->value_as_bits());
+    if (!obj->IsCallable()) continue;
-  Handle<FixedArray> js_imports_table(
+#ifdef DEBUG
-      reinterpret_cast<FixedArray**>(js_imports_table_loc));
+    // There should only be this one reference to a callable object.
-  int index = 0;
+    for (it.next(); !it.done(); it.next()) {
-  CHECK(deopt_data->get(1)->ToInt32(&index));
+      HeapObject* other = it.rinfo()->target_object();
-  DCHECK_GT(js_imports_table->length(), index);
+      DCHECK(!other->IsCallable());
-  Handle<Object> obj(js_imports_table->get(index), isolate);
+    }
-  if (obj->IsCallable()) {
+#endif
-    return Handle<HeapObject>::cast(obj);
+    return handle(obj, isolate);
-  } 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,9 +179,6 @@ 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");
@@ -436,6 +433,7 @@ 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,7 +152,6 @@ 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,
@@ -162,7 +161,6 @@ class WasmInstanceObject : public JSObject {
     kGlobalsBufferIndex,
     kDebugInfoIndex,
     kDirectlyCalledInstancesIndex,
-    kJsImportsTableIndex,
     kFieldCount
   };
 
@@ -174,7 +172,6 @@ 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();
@@ -715,7 +712,6 @@ 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,8 +46,7 @@ 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",
@@ -68,7 +67,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(), js_imports_table);
+  return module->AddJsFunction(sig, source.start());
 }
 
 void EXPECT_CALL(double expected, Handle<JSFunction> jsfunc,
@@ -137,10 +136,8 @@ TEST(Run_I32Popcount_jswrapped) {
 TEST(Run_CallJS_Add_jswrapped) {
   WasmRunner<int, int> r(kExecuteCompiled);
   TestSignatures sigs;
-  Handle<FixedArray> js_imports_table =
+  uint32_t js_index =
-      r.main_isolate()->factory()->NewFixedArray(2, TENURED);
+      r.module().AddJsFunction(sigs.i_i(), "(function(a) { return a + 99; })");
-  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);
@@ -161,10 +158,7 @@ void RunJSSelectTest(int which) {
     FunctionSig sig(1, num_params, types);
 
     WasmRunner<void> r(kExecuteCompiled);
-    Handle<FixedArray> js_imports_table =
+    uint32_t js_index = AddJSSelector(&r.module(), &sig, which);
-        scope.isolate()->factory()->NewFixedArray(2, TENURED);
-    uint32_t js_index =
-        AddJSSelector(&r.module(), &sig, which, js_imports_table);
     WasmFunctionCompiler& t = r.NewFunction(&sig);
 
     {
@@ -422,9 +416,7 @@ 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);
-    Handle<FixedArray> js_imports_table = factory->NewFixedArray(2, TENURED);
+    uint32_t js_index = AddJSSelector(&r.module(), &sig, which);
-    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,12 +79,9 @@ 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

@@ -222,14 +222,13 @@ 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(), js_imports_table);
+        Handle<String>::null(), module()->origin());
     function_code_[index] = code;
     return index;
   }