[wasm] Register and release protected instructions only once

We currently have a system where the protected instructions are
unregistered when the last instance dies, and registered again on the
next instantiation. This is triggered by {WasmCompiledModule::Reset}.
Since the reference to the {NativeModule} will move to the
{WasmModuleObject}, and this object stays alive even if the last
instance dies, this will become hard to maintain.
It will also make it harder to share wasm code across isolates.
This CL refactors this to register trap handler data once when the code
is added to the {NativeModule}, and releases it if the code dies.

R=mstarzinger@chromium.org
CC=​eholk@chromium.org

Bug: v8:5277
Change-Id: I3f1b336095230b255f3849c271b37b62f2b96cd6
Reviewed-on: https://chromium-review.googlesource.com/1103567
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53791}

src/wasm/module-compiler.cc

@@ -569,9 +569,6 @@ const wasm::WasmCode* LazyCompileFunction(
       compilation_time != 0 ? static_cast<int>(func_size / compilation_time)
                             : 0);
 
-  if (trap_handler::IsTrapHandlerEnabled()) {
-    wasm_code->RegisterTrapHandlerData();
-  }
   return wasm_code;
 }
 
@@ -1819,13 +1816,6 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
   FlushICache(native_module);
   FlushICache(handle(module_object_->export_wrappers(), isolate_));
 
-  //--------------------------------------------------------------------------
-  // Unpack and notify signal handler of protected instructions.
-  //--------------------------------------------------------------------------
-  if (use_trap_handler()) {
-    native_module->UnpackAndRegisterProtectedInstructions();
-  }
-
   //--------------------------------------------------------------------------
   // Insert the compiled module into the weak list of compiled modules.
   //--------------------------------------------------------------------------

src/wasm/wasm-code-manager.cc

@@ -139,8 +139,8 @@ void WasmCode::set_trap_handler_index(size_t value) {
 }
 
 void WasmCode::RegisterTrapHandlerData() {
+  DCHECK(!HasTrapHandlerIndex());
   if (kind() != wasm::WasmCode::kFunction) return;
-  if (HasTrapHandlerIndex()) return;
 
   Address base = instruction_start();
 
@@ -156,8 +156,6 @@ void WasmCode::RegisterTrapHandlerData() {
 
 bool WasmCode::HasTrapHandlerIndex() const { return trap_handler_index_ >= 0; }
 
-void WasmCode::ResetTrapHandlerIndex() { trap_handler_index_ = -1; }
-
 bool WasmCode::ShouldBeLogged(Isolate* isolate) {
   return isolate->logger()->is_listening_to_code_events() ||
          isolate->is_profiling();
@@ -563,8 +561,6 @@ WasmCode* NativeModule::AddCode(
       safepoint_table_offset, handler_table_offset,
       std::move(protected_instructions), tier, WasmCode::kNoFlushICache);
 
-  set_code(index, ret);
-
   // Apply the relocation delta by iterating over the RelocInfo.
   AllowDeferredHandleDereference embedding_raw_address;
   intptr_t delta = ret->instructions().start() - desc.buffer;
@@ -595,6 +591,11 @@ WasmCode* NativeModule::AddCode(
     }
   }
 
+  set_code(index, ret);
+  if (use_trap_handler_) {
+    ret->RegisterTrapHandlerData();
+  }
+
   // Flush the i-cache here instead of in AddOwnedCode, to include the changes
   // made while iterating over the RelocInfo above.
   Assembler::FlushICache(ret->instructions().start(),
@@ -805,24 +806,6 @@ WasmCode* NativeModule::CloneCode(const WasmCode* original_code,
   return ret;
 }
 
-void NativeModule::UnpackAndRegisterProtectedInstructions() {
-  for (WasmCode* wasm_code : code_table()) {
-    if (wasm_code == nullptr) continue;
-    wasm_code->RegisterTrapHandlerData();
-  }
-}
-
-void NativeModule::ReleaseProtectedInstructions() {
-  for (WasmCode* wasm_code : code_table()) {
-    if (wasm_code == nullptr || !wasm_code->HasTrapHandlerIndex()) continue;
-    CHECK_LT(wasm_code->trap_handler_index(),
-             static_cast<size_t>(std::numeric_limits<int>::max()));
-    trap_handler::ReleaseHandlerData(
-        static_cast<int>(wasm_code->trap_handler_index()));
-    wasm_code->ResetTrapHandlerIndex();
-  }
-}
-
 void NativeModule::DisableTrapHandler() {
   // Switch {use_trap_handler_} from true to false.
   DCHECK(use_trap_handler_);

src/wasm/wasm-code-manager.h

@@ -149,10 +149,6 @@ class V8_EXPORT_PRIVATE WasmCode final {
     return *protected_instructions_.get();
   }
 
-  // Register protected instruction information with the trap handler. Sets
-  // trap_handler_index.
-  void RegisterTrapHandlerData();
-
   void Validate() const;
   void Print(Isolate* isolate) const;
   void Disassemble(const char* name, Isolate* isolate, std::ostream& os,
@@ -206,7 +202,10 @@ class V8_EXPORT_PRIVATE WasmCode final {
   size_t trap_handler_index() const;
   void set_trap_handler_index(size_t);
   bool HasTrapHandlerIndex() const;
-  void ResetTrapHandlerIndex();
+
+  // Register protected instruction information with the trap handler. Sets
+  // trap_handler_index.
+  void RegisterTrapHandlerData();
 
   Vector<byte> instructions_;
   std::unique_ptr<const byte[]> reloc_info_;
@@ -283,11 +282,6 @@ class V8_EXPORT_PRIVATE NativeModule final {
     return code;
   }
 
-  // Register/release the protected instructions in all code objects with the
-  // global trap handler for this process.
-  void UnpackAndRegisterProtectedInstructions();
-  void ReleaseProtectedInstructions();
-
   // Transition this module from code relying on trap handlers (i.e. without
   // explicit memory bounds checks) to code that does not require trap handlers
   // (i.e. code with explicit bounds checks).

src/wasm/wasm-objects.cc

@@ -1551,14 +1551,6 @@ void WasmCompiledModule::Reset(Isolate* isolate,
   DisallowHeapAllocation no_gc;
   compiled_module->reset_prev_instance();
   compiled_module->reset_next_instance();
-  wasm::NativeModule* native_module = compiled_module->GetNativeModule();
-  if (native_module == nullptr) return;
-  native_module->SetExecutable(false);
-
-  TRACE("Resetting %zu\n", native_module->instance_id);
-  if (native_module->use_trap_handler()) {
-    native_module->ReleaseProtectedInstructions();
-  }
 }
 
 void WasmCompiledModule::PrintInstancesChain() {

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

@@ -435,9 +435,6 @@ void WasmFunctionCompiler::Build(const byte* start, const byte* end) {
     wasm_code->LogCode(isolate());
   }
   CHECK(!thrower.error());
-  if (trap_handler::IsTrapHandlerEnabled()) {
-    native_module->UnpackAndRegisterProtectedInstructions();
-  }
 }
 
 WasmFunctionCompiler::WasmFunctionCompiler(Zone* zone, FunctionSig* sig,