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

This is revert of commit aee1e1fb with the fix for A1 and N6 jetstream failure.

R=bradnelson@chromium.org,mtrofin@chromium.org,clemensh@chromium.org
Bug: chromium:750828

Change-Id: Id38896af51315f76a0667ace32c77a2ba7287eec
Reviewed-on: https://chromium-review.googlesource.com/607092
Commit-Queue: Mircea Trofin <mtrofin@chromium.org>
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Reviewed-by: Brad Nelson <bradnelson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47910}

src/compiler/wasm-compiler.cc

@@ -2705,7 +2705,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());
@@ -2727,7 +2729,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);
@@ -2736,11 +2738,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()) {
@@ -2772,7 +2794,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
@@ -2807,6 +2829,7 @@ void WasmGraphBuilder::BuildWasmToJSWrapper(Handle<JSReceiver> target) {
                   : FromJS(call, HeapConstant(isolate->native_context()),
                            sig_->GetReturn());
   Return(val);
+  return true;
 }
 
 namespace {
@@ -3916,11 +3939,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
   //----------------------------------------------------------------------------
@@ -3942,7 +3964,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();
   {
@@ -3976,6 +4000,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

@@ -139,11 +139,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,
@@ -262,7 +266,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);
   void BuildCWasmEntry();

src/wasm/module-compiler.cc

@@ -493,7 +493,8 @@ using WasmInstanceMap =
 Handle<Code> UnwrapExportOrCompileImportWrapper(
     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 = GetWasmFunctionForExport(isolate, target);
   if (other_func) {
     if (!sig->Equals(other_func->sig)) return Handle<Code>::null();
@@ -508,7 +509,8 @@ Handle<Code> UnwrapExportOrCompileImportWrapper(
   // 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() {
@@ -1303,6 +1305,15 @@ 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);
+  GlobalHandles::MakeWeak(
+      reinterpret_cast<Object**>(js_imports_table.location()),
+      js_imports_table.location(), &FunctionTableFinalizer,
+      v8::WeakCallbackType::kFinalizer);
+  instance->set_js_imports_table(*func_table);
   WasmInstanceMap imported_wasm_instances(isolate_->heap());
   for (int index = 0; index < static_cast<int>(module_->import_table.size());
        ++index) {
@@ -1338,7 +1349,7 @@ int InstanceBuilder::ProcessImports(Handle<FixedArray> code_table,
         Handle<Code> import_code = UnwrapExportOrCompileImportWrapper(
             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_code.is_null()) {
           ReportLinkError("imported function does not match the expected type",
                           index, module_name, import_name);

src/wasm/wasm-interpreter.cc

@@ -667,23 +667,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-objects-inl.h

@@ -58,6 +58,8 @@ OPTIONAL_ACCESSORS(WasmInstanceObject, signature_tables, FixedArray,
                    kSignatureTablesOffset)
 ACCESSORS(WasmInstanceObject, directly_called_instances, FixedArray,
           kDirectlyCalledInstancesOffset)
+ACCESSORS(WasmInstanceObject, js_imports_table, FixedArray,
+          kJsImportsTableOffset)
 
 // WasmSharedModuleData
 ACCESSORS(WasmSharedModuleData, module_bytes, SeqOneByteString,

src/wasm/wasm-objects.h

@@ -155,6 +155,7 @@ class WasmInstanceObject : public JSObject {
 
   // FixedArray of all instances whose code was imported
   DECL_OPTIONAL_ACCESSORS(directly_called_instances, FixedArray)
+  DECL_ACCESSORS(js_imports_table, FixedArray)
 
   enum {  // --
     kCompiledModuleIndex,
@@ -166,6 +167,7 @@ class WasmInstanceObject : public JSObject {
     kFunctionTablesIndex,
     kSignatureTablesIndex,
     kDirectlyCalledInstancesIndex,
+    kJsImportsTableIndex,
     kFieldCount
   };
 
@@ -179,6 +181,7 @@ class WasmInstanceObject : public JSObject {
   DEF_OFFSET(FunctionTables)
   DEF_OFFSET(SignatureTables)
   DEF_OFFSET(DirectlyCalledInstances)
+  DEF_OFFSET(JsImportsTable)
 
   WasmModuleObject* module_object();
   V8_EXPORT_PRIVATE wasm::WasmModule* module();

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

@@ -46,7 +46,7 @@ class PredictableInputValues {
 };
 
 uint32_t AddJSSelector(TestingModuleBuilder* builder, FunctionSig* sig,
-                       int which) {
+                       int which, Handle<FixedArray> js_imports_table) {
   const int kMaxParams = 11;
   static const char* formals[kMaxParams] = {"",
                                             "a",
@@ -67,7 +67,7 @@ uint32_t AddJSSelector(TestingModuleBuilder* builder, FunctionSig* sig,
   SNPrintF(source, "(function(%s) { return %c; })",
            formals[sig->parameter_count()], param);
 
-  return builder->AddJsFunction(sig, source.start());
+  return builder->AddJsFunction(sig, source.start(), js_imports_table);
 }
 
 void EXPECT_CALL(double expected, Handle<JSFunction> jsfunc,
@@ -136,8 +136,10 @@ TEST(Run_I32Popcount_jswrapped) {
 TEST(Run_CallJS_Add_jswrapped) {
   WasmRunner<int, int> r(kExecuteCompiled);
   TestSignatures sigs;
-  uint32_t js_index =
-      r.builder().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.builder().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.builder().WrapCode(r.function()->func_index);
@@ -158,7 +160,11 @@ void RunJSSelectTest(int which) {
     FunctionSig sig(1, num_params, types);
 
     WasmRunner<void> r(kExecuteCompiled);
-    uint32_t js_index = AddJSSelector(&r.builder(), &sig, which);
+    Handle<FixedArray> js_imports_table =
+        scope.isolate()->factory()->NewFixedArray(2, TENURED);
+    uint32_t js_index =
+        AddJSSelector(&r.builder(), &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.builder(), &sig, which);
+    Handle<FixedArray> js_imports_table = factory->NewFixedArray(2, TENURED);
+    uint32_t js_index =
+        AddJSSelector(&r.builder(), &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

@@ -101,9 +101,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.builder().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.cc

@@ -81,14 +81,14 @@ uint32_t TestingModuleBuilder::AddFunction(FunctionSig* sig, Handle<Code> code,
   return index;
 }
 
-uint32_t TestingModuleBuilder::AddJsFunction(FunctionSig* sig,
-                                             const char* source) {
+uint32_t TestingModuleBuilder::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 = compiler::CompileWasmToJSWrapper(
       isolate_, jsfunc, sig, index, Handle<String>::null(),
-      Handle<String>::null(), test_module_.origin());
+      Handle<String>::null(), test_module_.origin(), js_imports_table);
   function_code_[index] = code;
   return index;
 }

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

@@ -159,7 +159,8 @@ class TestingModuleBuilder {
 
   uint32_t AddFunction(FunctionSig* sig, Handle<Code> code, const char* name);
 
-  uint32_t AddJsFunction(FunctionSig* sig, const char* source);
+  uint32_t AddJsFunction(FunctionSig* sig, const char* source,
+                         Handle<FixedArray> js_imports_table);
 
   Handle<JSFunction> WrapCode(uint32_t index);