[wasm] Avoid transition from unhandlified to handlified

The ImportedFunctionEntry and IndirectFunctionTableEntry stored handles
internally, but were created from raw pointers. This is not allowed.
The two options to fix this are to either handlify the whole interface,
or do the opposite and use raw pointers everywhere. Since no current
user depends on a handlified interface, and both objects are being used
in performance critical code, this CL unhandlifies the interface and
adds a DisallowHeapAllocation scope to enforce that no GC happens while
any ImportedFunctionEntry or IndirectFunctionTableEntry is alive.

R=mstarzinger@chromium.org
CC=titzer@chromium.org

Change-Id: I098c2abcdd28c4b117272ac3ea0358ff2e56b36c
Reviewed-on: https://chromium-review.googlesource.com/1005075
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52531}

src/wasm/module-compiler.cc

@@ -347,14 +347,15 @@ MaybeHandle<WasmInstanceObject> InstantiateToInstanceObject(
 // Utilizes a reverse mapping to prevent O(n^2) behavior.
 class IndirectPatcher {
  public:
-  void Patch(Handle<WasmInstanceObject> caller_instance,
-             Handle<WasmInstanceObject> target_instance, int func_index,
+  void Patch(WasmInstanceObject* caller_instance,
+             WasmInstanceObject* target_instance, int func_index,
              Address old_target, const WasmCode* new_code) {
+    DisallowHeapAllocation no_gc;
     TRACE_LAZY(
         "IndirectPatcher::Patch(caller=%p, target=%p, func_index=%i, "
         "old_target=%p, "
         "new_code=%p)\n",
-        *caller_instance, *target_instance, func_index, old_target, new_code);
+        caller_instance, target_instance, func_index, old_target, new_code);
     if (mapping_.size() == 0 || misses_ >= kMaxMisses) {
       BuildMapping(caller_instance);
     }
@@ -368,7 +369,7 @@ class IndirectPatcher {
       if (index < 0) {
         // Imported function entry.
         int i = -1 - index;
-        auto entry = caller_instance->imported_function_entry_at(i);
+        ImportedFunctionEntry entry(caller_instance, i);
         if (entry.target() == old_target) {
           DCHECK_EQ(
               func_index,
@@ -379,7 +380,7 @@ class IndirectPatcher {
       } else {
         // Indirect function table entry.
         int i = index;
-        auto entry = caller_instance->indirect_function_table_entry_at(i);
+        IndirectFunctionTableEntry entry(caller_instance, i);
         if (entry.target() == old_target) {
           DCHECK_EQ(
               func_index,
@@ -393,23 +394,23 @@ class IndirectPatcher {
   }
 
  private:
-  void BuildMapping(Handle<WasmInstanceObject> caller_instance) {
+  void BuildMapping(WasmInstanceObject* caller_instance) {
     mapping_.clear();
     misses_ = 0;
-    TRACE_LAZY("BuildMapping for (caller=%p)...\n", *caller_instance);
+    TRACE_LAZY("BuildMapping for (caller=%p)...\n", caller_instance);
     Isolate* isolate = caller_instance->GetIsolate();
     WasmCodeManager* code_manager = isolate->wasm_engine()->code_manager();
     uint32_t num_imported_functions =
         caller_instance->module()->num_imported_functions;
     // Process the imported function entries.
     for (unsigned i = 0; i < num_imported_functions; i++) {
-      auto entry = caller_instance->imported_function_entry_at(i);
+      ImportedFunctionEntry entry(caller_instance, i);
       WasmCode* code = code_manager->GetCodeFromStartAddress(entry.target());
       if (code->kind() != WasmCode::kLazyStub) continue;
       TRACE_LAZY(" +import[%u] -> #%d (%p)\n", i, code->index(),
                  code->instructions().start());
       DCHECK(!entry.is_js_receiver_entry());
-      Handle<WasmInstanceObject> target_instance(entry.instance(), isolate);
+      WasmInstanceObject* target_instance = entry.instance();
       WasmCode* new_code =
           target_instance->compiled_module()->GetNativeModule()->GetCode(
               code->index());
@@ -425,13 +426,13 @@ class IndirectPatcher {
     // Process the indirect function table entries.
     size_t ift_size = caller_instance->indirect_function_table_size();
     for (unsigned i = 0; i < ift_size; i++) {
-      auto entry = caller_instance->indirect_function_table_entry_at(i);
+      IndirectFunctionTableEntry entry(caller_instance, i);
       if (entry.target() == nullptr) continue;  // null IFT entry
       WasmCode* code = code_manager->GetCodeFromStartAddress(entry.target());
       if (code->kind() != WasmCode::kLazyStub) continue;
       TRACE_LAZY(" +indirect[%u] -> #%d (lazy:%p)\n", i, code->index(),
                  code->instructions().start());
-      Handle<WasmInstanceObject> target_instance(entry.instance(), isolate);
+      WasmInstanceObject* target_instance = entry.instance();
       WasmCode* new_code =
           target_instance->compiled_module()->GetNativeModule()->GetCode(
               code->index());
@@ -827,7 +828,7 @@ Address CompileLazy(Isolate* isolate,
                                    caller_instance->managed_indirect_patcher())
                                    ->get();
     Address old_target = lazy_stub->instructions().start();
-    patcher->Patch(caller_instance, target_instance, target_func_index,
+    patcher->Patch(*caller_instance, *target_instance, target_func_index,
                    old_target, result);
   }
 
@@ -1935,9 +1936,9 @@ int InstanceBuilder::ProcessImports(Handle<WasmInstanceObject> instance) {
             return -1;
           }
           // The import reference is the instance object itself.
-          auto entry = instance->imported_function_entry_at(func_index);
           auto wasm_code = imported_function->GetWasmCode();
-          entry.set(imported_instance, wasm_code);
+          ImportedFunctionEntry(*instance, func_index)
+              .set(*imported_instance, wasm_code);
           native_module->SetCode(func_index, wasm_code);
         } else {
           // The imported function is a callable.
@@ -1950,8 +1951,8 @@ int InstanceBuilder::ProcessImports(Handle<WasmInstanceObject> instance) {
 
           WasmCode* wasm_code = native_module->AddCodeCopy(
               wrapper_code, wasm::WasmCode::kWasmToJsWrapper, func_index);
-          auto entry = instance->imported_function_entry_at(func_index);
-          entry.set(js_receiver, wasm_code);
+          ImportedFunctionEntry(*instance, func_index)
+              .set(*js_receiver, wasm_code);
         }
         num_imported_functions++;
         break;
@@ -2026,9 +2027,9 @@ int InstanceBuilder::ProcessImports(Handle<WasmInstanceObject> instance) {
           FunctionSig* sig = imported_instance->module()
                                  ->functions[exported_code->index()]
                                  .sig;
-          auto entry = instance->indirect_function_table_entry_at(i);
-          entry.set(module_->signature_map.Find(sig), imported_instance,
-                    exported_code);
+          IndirectFunctionTableEntry(*instance, i)
+              .set(module_->signature_map.Find(sig), *imported_instance,
+                   exported_code);
         }
         num_imported_tables++;
         break;
@@ -2398,8 +2399,8 @@ void InstanceBuilder::LoadTableSegments(Handle<WasmInstanceObject> instance) {
         wasm::WasmCode* wasm_code =
             native_module->GetIndirectlyCallableCode(func_index);
 
-        auto entry = instance->indirect_function_table_entry_at(table_index);
-        entry.set(sig_id, instance, wasm_code);
+        IndirectFunctionTableEntry(*instance, table_index)
+            .set(sig_id, *instance, wasm_code);
 
         if (!table_instance.table_object.is_null()) {
           // Update the table object's other dispatch tables.

src/wasm/wasm-interpreter.cc

@@ -2357,11 +2357,15 @@ class ThreadImpl {
     HandleScope handle_scope(isolate);
 
     DCHECK_GT(module()->num_imported_functions, function_index);
-    auto entry = instance_object_->imported_function_entry_at(function_index);
-    Handle<WasmInstanceObject> instance(entry.instance(), isolate);
+    Handle<WasmInstanceObject> instance;
+    WasmCode* code;
+    {
+      ImportedFunctionEntry entry(*instance_object_, function_index);
+      instance = handle(entry.instance(), isolate);
+      code = isolate->wasm_engine()->code_manager()->GetCodeFromStartAddress(
+          entry.target());
+    }
     FunctionSig* sig = codemap()->module()->functions[function_index].sig;
-    auto code = isolate->wasm_engine()->code_manager()->GetCodeFromStartAddress(
-        entry.target());
     return CallExternalWasmFunction(isolate, instance, code, sig);
   }
 
@@ -2399,22 +2403,25 @@ class ThreadImpl {
     if (entry_index >= instance_object_->indirect_function_table_size()) {
       return {ExternalCallResult::INVALID_FUNC};
     }
-    auto entry =
-        instance_object_->indirect_function_table_entry_at(entry_index);
 
-    // Signature check.
-    if (entry.sig_id() != static_cast<int32_t>(expected_sig_id)) {
-      return {ExternalCallResult::SIGNATURE_MISMATCH};
+    WasmCode* code;
+    Handle<WasmInstanceObject> instance;
+    {
+      IndirectFunctionTableEntry entry(*instance_object_, entry_index);
+      instance = handle(entry.instance(), isolate);
+      code = isolate->wasm_engine()->code_manager()->GetCodeFromStartAddress(
+          entry.target());
+
+      // Signature check.
+      if (entry.sig_id() != static_cast<int32_t>(expected_sig_id)) {
+        return {ExternalCallResult::SIGNATURE_MISMATCH};
+      }
     }
 
     // Call either an internal or external WASM function.
     HandleScope scope(isolate);
     FunctionSig* signature = module()->signatures[sig_index];
-    Handle<WasmInstanceObject> instance(entry.instance(), isolate);
 
-    // Lookup code object from entry address.
-    auto code = isolate->wasm_engine()->code_manager()->GetCodeFromStartAddress(
-        entry.target());
     if (code->kind() == wasm::WasmCode::kFunction) {
       if (!instance_object_.is_identical_to(instance)) {
         // Cross instance call.

src/wasm/wasm-objects-inl.h

@@ -143,6 +143,20 @@ inline bool WasmInstanceObject::has_indirect_function_table() {
   return indirect_function_table_sig_ids() != nullptr;
 }
 
+IndirectFunctionTableEntry::IndirectFunctionTableEntry(
+    WasmInstanceObject* instance, int index)
+    : instance_(instance), index_(index) {
+  DCHECK_GE(index, 0);
+  DCHECK_LT(index, instance->indirect_function_table_size());
+}
+
+ImportedFunctionEntry::ImportedFunctionEntry(WasmInstanceObject* instance,
+                                             int index)
+    : instance_(instance), index_(index) {
+  DCHECK_GE(index, 0);
+  DCHECK_LT(index, instance->module()->num_imported_functions);
+}
+
 // WasmSharedModuleData
 ACCESSORS(WasmSharedModuleData, module_wrapper, Object, kModuleWrapperOffset)
 ACCESSORS(WasmSharedModuleData, module_bytes, SeqOneByteString,

src/wasm/wasm-objects.cc

@@ -110,9 +110,7 @@ class WasmInstanceNativeAllocations {
 
     instance->set_indirect_function_table_instances(*new_instances);
     for (size_t j = old_size; j < new_size; j++) {
-      auto entry =
-          instance->indirect_function_table_entry_at(static_cast<int>(j));
-      entry.clear();
+      IndirectFunctionTableEntry(*instance, static_cast<int>(j)).clear();
     }
   }
   uint32_t* indirect_function_table_sig_ids_ = nullptr;
@@ -390,8 +388,8 @@ void WasmTableObject::UpdateDispatchTables(
     WasmInstanceObject* to_instance = WasmInstanceObject::cast(
         dispatch_tables->get(i + kDispatchTableInstanceOffset));
     auto sig_id = to_instance->module()->signature_map.Find(sig);
-    auto entry = to_instance->indirect_function_table_entry_at(table_index);
-    entry.set(sig_id, from_instance, wasm_code);
+    IndirectFunctionTableEntry(to_instance, table_index)
+        .set(sig_id, *from_instance, wasm_code);
   }
 }
 
@@ -405,8 +403,7 @@ void WasmTableObject::ClearDispatchTables(Handle<WasmTableObject> table,
     WasmInstanceObject* target_instance = WasmInstanceObject::cast(
         dispatch_tables->get(i + kDispatchTableInstanceOffset));
     DCHECK_LT(index, target_instance->indirect_function_table_size());
-    auto entry = target_instance->indirect_function_table_entry_at(index);
-    entry.clear();
+    IndirectFunctionTableEntry(target_instance, index).clear();
   }
 }
 
@@ -640,16 +637,15 @@ void IndirectFunctionTableEntry::clear() {
       index_, instance_->GetIsolate()->heap()->undefined_value());
 }
 
-void IndirectFunctionTableEntry::set(int sig_id,
-                                     Handle<WasmInstanceObject> instance,
+void IndirectFunctionTableEntry::set(int sig_id, WasmInstanceObject* instance,
                                      const wasm::WasmCode* wasm_code) {
   TRACE_IFT("IFT entry %p[%d] = {sig_id=%d, instance=%p, target=%p}\n",
-            *instance_, index_, sig_id, *instance,
+            instance_, index_, sig_id, instance,
             wasm_code->instructions().start());
   instance_->indirect_function_table_sig_ids()[index_] = sig_id;
   instance_->indirect_function_table_targets()[index_] =
       wasm_code->instructions().start();
-  instance_->indirect_function_table_instances()->set(index_, *instance);
+  instance_->indirect_function_table_instances()->set(index_, instance);
 }
 
 WasmInstanceObject* IndirectFunctionTableEntry::instance() {
@@ -665,22 +661,22 @@ Address IndirectFunctionTableEntry::target() {
   return instance_->indirect_function_table_targets()[index_];
 }
 
-void ImportedFunctionEntry::set(Handle<JSReceiver> callable,
+void ImportedFunctionEntry::set(JSReceiver* callable,
                                 const wasm::WasmCode* wasm_to_js_wrapper) {
-  TRACE_IFT("Import callable %p[%d] = {callable=%p, target=%p}\n", *instance_,
-            index_, *callable, wasm_to_js_wrapper->instructions().start());
+  TRACE_IFT("Import callable %p[%d] = {callable=%p, target=%p}\n", instance_,
+            index_, callable, wasm_to_js_wrapper->instructions().start());
   DCHECK_EQ(wasm::WasmCode::kWasmToJsWrapper, wasm_to_js_wrapper->kind());
-  instance_->imported_function_instances()->set(index_, *instance_);
-  instance_->imported_function_callables()->set(index_, *callable);
+  instance_->imported_function_instances()->set(index_, instance_);
+  instance_->imported_function_callables()->set(index_, callable);
   instance_->imported_function_targets()[index_] =
       wasm_to_js_wrapper->instructions().start();
 }
 
-void ImportedFunctionEntry::set(Handle<WasmInstanceObject> instance,
+void ImportedFunctionEntry::set(WasmInstanceObject* instance,
                                 const wasm::WasmCode* wasm_code) {
-  TRACE_IFT("Import WASM %p[%d] = {instance=%p, target=%p}\n", *instance_,
-            index_, *instance, wasm_code->instructions().start());
-  instance_->imported_function_instances()->set(index_, *instance);
+  TRACE_IFT("Import WASM %p[%d] = {instance=%p, target=%p}\n", instance_,
+            index_, instance, wasm_code->instructions().start());
+  instance_->imported_function_instances()->set(index_, instance);
   instance_->imported_function_callables()->set(
       index_, instance_->GetHeap()->undefined_value());
   instance_->imported_function_targets()[index_] =
@@ -718,23 +714,6 @@ bool WasmInstanceObject::EnsureIndirectFunctionTableWithMinimumSize(
   return true;
 }
 
-IndirectFunctionTableEntry WasmInstanceObject::indirect_function_table_entry_at(
-    int i) {
-  DCHECK_GE(i, 0);
-  DCHECK_LT(i, indirect_function_table_size());
-  Handle<WasmInstanceObject> handle(this, GetIsolate());
-  IndirectFunctionTableEntry entry(handle, i);
-  return entry;
-}
-
-ImportedFunctionEntry WasmInstanceObject::imported_function_entry_at(int i) {
-  DCHECK_GE(i, 0);
-  DCHECK_LT(i, compiled_module()->shared()->module()->num_imported_functions);
-  Handle<WasmInstanceObject> handle(this, GetIsolate());
-  ImportedFunctionEntry entry(handle, i);
-  return entry;
-}
-
 void WasmInstanceObject::SetRawMemory(byte* mem_start, size_t mem_size) {
   DCHECK_LE(mem_size, wasm::kV8MaxWasmMemoryPages * wasm::kWasmPageSize);
   uint64_t mem_size64 = mem_size;

src/wasm/wasm-objects.h

@@ -48,8 +48,10 @@ class WasmInstanceObject;
 // - target   = entrypoint to wasm code for the function, or wasm-to-js wrapper
 class IndirectFunctionTableEntry {
  public:
+  inline IndirectFunctionTableEntry(WasmInstanceObject*, int index);
+
   void clear();
-  void set(int sig_id, Handle<WasmInstanceObject> instance,
+  void set(int sig_id, WasmInstanceObject* instance,
            const wasm::WasmCode* wasm_code);
 
   WasmInstanceObject* instance();
@@ -57,12 +59,11 @@ class IndirectFunctionTableEntry {
   Address target();
 
  private:
-  // These entries are only constructed by the WasmInstanceObject.
-  friend class WasmInstanceObject;
-  IndirectFunctionTableEntry(Handle<WasmInstanceObject> instance, int index)
-      : instance_(instance), index_(index) {}
-  Handle<WasmInstanceObject> instance_;
-  int index_;
+#ifdef DEBUG
+  DisallowHeapAllocation no_gc;
+#endif
+  WasmInstanceObject* const instance_;
+  int const index_;
 };
 
 // An entry for an imported function.
@@ -78,11 +79,12 @@ class IndirectFunctionTableEntry {
 //      - target   = entrypoint to wasm code of the function
 class ImportedFunctionEntry {
  public:
+  inline ImportedFunctionEntry(WasmInstanceObject*, int index);
+
   // Initialize this entry as a {JSReceiver} call.
-  void set(Handle<JSReceiver> callable,
-           const wasm::WasmCode* wasm_to_js_wrapper);
+  void set(JSReceiver* callable, const wasm::WasmCode* wasm_to_js_wrapper);
   // Initialize this entry as a WASM to WASM call.
-  void set(Handle<WasmInstanceObject> target_instance,
+  void set(WasmInstanceObject* target_instance,
            const wasm::WasmCode* wasm_function);
 
   WasmInstanceObject* instance();
@@ -91,12 +93,11 @@ class ImportedFunctionEntry {
   bool is_js_receiver_entry();
 
  private:
-  // These entries are only constructed by the WasmInstanceObject.
-  friend class WasmInstanceObject;
-  ImportedFunctionEntry(Handle<WasmInstanceObject> instance, int index)
-      : instance_(instance), index_(index) {}
-  Handle<WasmInstanceObject> instance_;
-  int index_;
+#ifdef DEBUG
+  DisallowHeapAllocation no_gc;
+#endif
+  WasmInstanceObject* const instance_;
+  int const index_;
 };
 
 // Representation of a WebAssembly.Module JavaScript-level object.
@@ -312,9 +313,6 @@ class WasmInstanceObject : public JSObject {
   static bool EnsureIndirectFunctionTableWithMinimumSize(
       Handle<WasmInstanceObject> instance, size_t minimum_size);
 
-  IndirectFunctionTableEntry indirect_function_table_entry_at(int index);
-  ImportedFunctionEntry imported_function_entry_at(int index);
-
   bool has_indirect_function_table();
 
   void SetRawMemory(byte* mem_start, size_t mem_size);

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

@@ -50,9 +50,8 @@ TestingModuleBuilder::TestingModuleBuilder(
     auto wasm_to_js_wrapper = native_module_->AddCodeCopy(
         code, wasm::WasmCode::kWasmToJsWrapper, maybe_import_index);
 
-    auto entry =
-        instance_object()->imported_function_entry_at(maybe_import_index);
-    entry.set(maybe_import->js_function, wasm_to_js_wrapper);
+    ImportedFunctionEntry(*instance_object_, maybe_import_index)
+        .set(*maybe_import->js_function, wasm_to_js_wrapper);
   }
 
   if (mode == kExecuteInterpreter) {
@@ -169,8 +168,8 @@ void TestingModuleBuilder::PopulateIndirectFunctionTable() {
       WasmFunction& function = test_module_.functions[table.values[j]];
       int sig_id = test_module_.signature_map.Find(function.sig);
       auto wasm_code = native_module_->GetCode(function.func_index);
-      auto entry = instance->indirect_function_table_entry_at(j);
-      entry.set(sig_id, instance, wasm_code);
+      IndirectFunctionTableEntry(*instance, j)
+          .set(sig_id, *instance, wasm_code);
     }
   }
 }