[wasm] Refactorings to improve naming

- Rename WasmCode::owner() to WasmCode::native_module() and
- Make {shared} field of WasmCompiledModule no-longer const, since
  it had a setter masquerading under the
  {OnWasmModuleDeserialization()}.
- Refactor and simplify the flow of "owner" in module-compiler.cc

R=mstarzinger@chromium.org

Change-Id: If9ee371124678fbbc845fc4e93279bf14f8f7ce8
Reviewed-on: https://chromium-review.googlesource.com/964263Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51959}

src/frames.cc

@@ -1316,8 +1316,8 @@ int FrameSummary::WasmCompiledFrameSummary::GetWasmSourcePosition(
   int position = 0;
   // Subtract one because the current PC is one instruction after the call site.
   offset--;
-  Handle<ByteArray> source_position_table(
-      ByteArray::cast(code->owner()->compiled_module()->source_positions()->get(
+  Handle<ByteArray> source_position_table(ByteArray::cast(
+      code->native_module()->compiled_module()->source_positions()->get(
           code->index())));
   for (SourcePositionTableIterator iterator(source_position_table);
        !iterator.done() && iterator.code_offset() <= offset;

src/wasm/module-compiler.cc

@@ -462,9 +462,10 @@ Address CompileLazy(Isolate* isolate) {
   DCHECK_EQ(wasm::WasmCode::kLazyStub, lazy_stub_or_copy->kind());
   if (!lazy_stub_or_copy->IsAnonymous()) {
     // Then it's an indirect call or via JS->wasm wrapper.
-    instance =
-        handle(lazy_stub_or_copy->owner()->compiled_module()->owning_instance(),
-               isolate);
+    instance = handle(lazy_stub_or_copy->native_module()
+                          ->compiled_module()
+                          ->owning_instance(),
+                      isolate);
     func_index_to_compile = Just(lazy_stub_or_copy->index());
     exp_deopt_data_entry =
         handle(instance->compiled_module()->lazy_compile_data()->get(
@@ -491,9 +492,10 @@ Address CompileLazy(Isolate* isolate) {
       // Then this is a direct call (otherwise we would have attached the
       // instance via deopt data to the lazy compile stub). Just use the
       // instance of the caller.
-      instance = handle(
-          wasm_caller_code->owner()->compiled_module()->owning_instance(),
-          isolate);
+      instance = handle(wasm_caller_code->native_module()
+                            ->compiled_module()
+                            ->owning_instance(),
+                        isolate);
     }
   }
 
@@ -686,7 +688,7 @@ const WasmCode* WasmExtractWasmToWasmCallee(const WasmCodeManager* code_manager,
   do {                                                                         \
     TRACE_LAZY("Patching wasm-to-wasm wrapper.\n");                            \
     DCHECK_EQ(WasmCode::kWasmToWasmWrapper, wasm_to_wasm->kind());             \
-    NativeModuleModificationScope scope(wasm_to_wasm->owner());                \
+    NativeModuleModificationScope scope(wasm_to_wasm->native_module());        \
     RelocIterator it(wasm_to_wasm->instructions(), wasm_to_wasm->reloc_info(), \
                      wasm_to_wasm->constant_pool(),                            \
                      RelocInfo::ModeMask(RelocInfo::JS_TO_WASM_CALL));         \
@@ -779,7 +781,7 @@ const wasm::WasmCode* LazyCompilationOrchestrator::CompileDirectCall(
   {
     DisallowHeapAllocation no_gc;
     Handle<WasmCompiledModule> caller_module(
-        wasm_caller->owner()->compiled_module(), isolate);
+        wasm_caller->native_module()->compiled_module(), isolate);
     SeqOneByteString* module_bytes = caller_module->shared()->module_bytes();
     uint32_t caller_func_index = wasm_caller->index();
     SourcePositionTableIterator source_pos_iterator(
@@ -1468,7 +1470,7 @@ MaybeHandle<WasmModuleObject> ModuleCompiler::CompileToModuleObjectInternal(
   Handle<WasmCompiledModule> compiled_module =
       NewCompiledModule(isolate_, shared->module(), export_wrappers, env.get());
   native_module_ = compiled_module->GetNativeModule();
-  compiled_module->OnWasmModuleDecodingComplete(shared);
+  compiled_module->set_shared(*shared);
   if (lazy_compile) {
     Handle<FixedArray> lazy_compile_data = isolate_->factory()->NewFixedArray(
         static_cast<int>(module_->functions.size()), TENURED);
@@ -1564,37 +1566,21 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
   // Reuse the compiled module (if no owner), otherwise clone.
   //--------------------------------------------------------------------------
   Handle<FixedArray> wrapper_table;
-  MaybeHandle<WasmInstanceObject> owner;
-  // native_module is the one we're building now, old_module
-  // is the one we clone from. They point to the same place if
-  // we don't need to clone.
   wasm::NativeModule* native_module = nullptr;
-  wasm::NativeModule* old_module = nullptr;
+  // Root the old instance, if any, in case later allocation causes GC,
+  // to prevent the finalizer running for the old instance.
+  MaybeHandle<WasmInstanceObject> old_instance;
 
   TRACE("Starting new module instantiation\n");
   {
-    // Root the owner, if any, before doing any allocations, which
-    // may trigger GC.
-    // Both owner and original template need to be in sync. Even
-    // after we lose the original template handle, the code
-    // objects we copied from it have data relative to the
-    // instance - such as globals addresses.
-    Handle<WasmCompiledModule> original;
-    {
-      DisallowHeapAllocation no_gc;
-      original = handle(module_object_->compiled_module());
-      if (original->has_weak_owning_instance()) {
-        owner = handle(WasmInstanceObject::cast(
-            original->weak_owning_instance()->value()));
-      }
-    }
-    DCHECK(!original.is_null());
+    Handle<WasmCompiledModule> original =
+        handle(module_object_->compiled_module());
     if (original->has_weak_owning_instance()) {
+      old_instance = handle(original->owning_instance());
       // Clone, but don't insert yet the clone in the instances chain.
-      // We do that last. Since we are holding on to the owner instance,
+      // We do that last. Since we are holding on to the old instance,
       // the owner + original state used for cloning and patching
       // won't be mutated by possible finalizer runs.
-      DCHECK(!owner.is_null());
       TRACE("Cloning from %zu\n", original->GetNativeModule()->instance_id);
       compiled_module_ = WasmCompiledModule::Clone(isolate_, original);
       native_module = compiled_module_->GetNativeModule();
@@ -1608,11 +1594,10 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
       RecordStats(native_module, counters());
       RecordStats(wrapper_table, counters());
     } else {
-      // There was no owner, so we can reuse the original.
+      // No instance owned the original compiled module.
       compiled_module_ = original;
       wrapper_table = handle(compiled_module_->export_wrappers(), isolate_);
-      old_module = compiled_module_->GetNativeModule();
-      native_module = old_module;
+      native_module = compiled_module_->GetNativeModule();
       TRACE("Reusing existing instance %zu\n",
             compiled_module_->GetNativeModule()->instance_id);
     }
@@ -1801,7 +1786,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
   //--------------------------------------------------------------------------
   {
     Handle<WeakCell> link_to_owning_instance = factory->NewWeakCell(instance);
-    if (!owner.is_null()) {
+    if (!old_instance.is_null()) {
       // Publish the new instance to the instances chain.
       DisallowHeapAllocation no_gc;
       compiled_module_->InsertInChain(*module_object_);
@@ -3175,7 +3160,7 @@ class AsyncCompileJob::FinishCompile : public CompileStep {
         WasmSharedModuleData::New(job_->isolate_, module_wrapper,
                                   Handle<SeqOneByteString>::cast(module_bytes),
                                   script, asm_js_offset_table);
-    job_->compiled_module_->OnWasmModuleDecodingComplete(shared);
+    job_->compiled_module_->set_shared(*shared);
     script->set_wasm_compiled_module(*job_->compiled_module_);
 
     // Finish the wasm script now and make it public to the debugger.

src/wasm/wasm-code-manager.cc

@@ -215,7 +215,7 @@ void WasmCode::Disassemble(const char* name, Isolate* isolate,
   // Anonymous functions don't have source positions.
   if (!IsAnonymous()) {
     Object* source_positions_or_undef =
-        owner_->compiled_module()->source_positions()->get(index());
+        native_module_->compiled_module()->source_positions()->get(index());
     if (!source_positions_or_undef->IsUndefined(isolate)) {
       os << "Source positions:\n pc offset  position\n";
       for (SourcePositionTableIterator it(

src/wasm/wasm-code-manager.h

@@ -111,7 +111,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
   // trampolines.
   bool IsAnonymous() const { return index_.IsNothing(); }
   Kind kind() const { return kind_; }
-  NativeModule* owner() const { return owner_; }
+  NativeModule* native_module() const { return native_module_; }
   Tier tier() const { return tier_; }
   Address constant_pool() const;
   size_t constant_pool_offset() const { return constant_pool_offset_; }
@@ -146,7 +146,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
 
   WasmCode(Vector<byte> instructions,
            std::unique_ptr<const byte[]>&& reloc_info, size_t reloc_size,
-           NativeModule* owner, Maybe<uint32_t> index, Kind kind,
+           NativeModule* native_module, Maybe<uint32_t> index, Kind kind,
            size_t constant_pool_offset, uint32_t stack_slots,
            size_t safepoint_table_offset, size_t handler_table_offset,
            std::shared_ptr<ProtectedInstructions> protected_instructions,
@@ -154,7 +154,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
       : instructions_(instructions),
         reloc_info_(std::move(reloc_info)),
         reloc_size_(reloc_size),
-        owner_(owner),
+        native_module_(native_module),
         index_(index),
         kind_(kind),
         constant_pool_offset_(constant_pool_offset),
@@ -170,7 +170,7 @@ class V8_EXPORT_PRIVATE WasmCode final {
   Vector<byte> instructions_;
   std::unique_ptr<const byte[]> reloc_info_;
   size_t reloc_size_ = 0;
-  NativeModule* owner_ = nullptr;
+  NativeModule* native_module_ = nullptr;
   Maybe<uint32_t> index_;
   Kind kind_;
   size_t constant_pool_offset_ = 0;

src/wasm/wasm-code-specialization.cc

@@ -180,7 +180,7 @@ bool CodeSpecialization::ApplyToWasmCode(wasm::WasmCode* code,
   base::Optional<PatchDirectCallsHelper> patch_direct_calls_helper;
   bool changed = false;
 
-  NativeModule* native_module = code->owner();
+  NativeModule* native_module = code->native_module();
 
   RelocIterator it(code->instructions(), code->reloc_info(),
                    code->constant_pool(), reloc_mode);

src/wasm/wasm-interpreter.cc

@@ -641,7 +641,7 @@ Handle<HeapObject> UnwrapWasmToJSWrapper(Isolate* isolate,
   Handle<FixedArray> js_imports_table;
   int index = 0;
   DCHECK_EQ(wasm::WasmCode::kWasmToJsWrapper, wasm_code->kind());
-  js_imports_table = Handle<FixedArray>(wasm_code->owner()
+  js_imports_table = Handle<FixedArray>(wasm_code->native_module()
                                             ->compiled_module()
                                             ->owning_instance()
                                             ->js_imports_table());
@@ -2464,7 +2464,7 @@ class ThreadImpl {
     static_assert(compiler::CWasmEntryParameters::kNumParameters == 3,
                   "code below needs adaption");
     Handle<Object> args[compiler::CWasmEntryParameters::kNumParameters];
-    WasmContext* context = code->owner()
+    WasmContext* context = code->native_module()
                                ->compiled_module()
                                ->owning_instance()
                                ->wasm_context()
@@ -2522,7 +2522,7 @@ class ThreadImpl {
     DCHECK(AllowHeapAllocation::IsAllowed());
 
     if (code->kind() == wasm::WasmCode::kFunction) {
-      if (code->owner()->compiled_module()->owning_instance() !=
+      if (code->native_module()->compiled_module()->owning_instance() !=
           codemap()->instance()) {
         return CallExternalWasmFunction(isolate, code, signature);
       }

src/wasm/wasm-objects.cc

@@ -643,7 +643,7 @@ WasmInstanceObject* WasmInstanceObject::GetOwningInstance(
   Object* weak_link = nullptr;
   DCHECK(code->kind() == wasm::WasmCode::kFunction ||
          code->kind() == wasm::WasmCode::kInterpreterStub);
-  weak_link = code->owner()->compiled_module()->weak_owning_instance();
+  weak_link = code->native_module()->compiled_module()->weak_owning_instance();
   DCHECK(weak_link->IsWeakCell());
   WeakCell* cell = WeakCell::cast(weak_link);
   if (cell->cleared()) return nullptr;
@@ -688,11 +688,11 @@ namespace {
 void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
   DisallowHeapAllocation no_gc;
   JSObject** p = reinterpret_cast<JSObject**>(data.GetParameter());
-  WasmInstanceObject* owner = reinterpret_cast<WasmInstanceObject*>(*p);
+  WasmInstanceObject* instance = reinterpret_cast<WasmInstanceObject*>(*p);
   Isolate* isolate = reinterpret_cast<Isolate*>(data.GetIsolate());
   // If a link to shared memory instances exists, update the list of memory
   // instances before the instance is destroyed.
-  WasmCompiledModule* compiled_module = owner->compiled_module();
+  WasmCompiledModule* compiled_module = instance->compiled_module();
   wasm::NativeModule* native_module = compiled_module->GetNativeModule();
   if (native_module) {
     TRACE("Finalizing %zu {\n", native_module->instance_id);
@@ -707,10 +707,9 @@ void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
   // Weak references to this instance won't be cleared until
   // the next GC cycle, so we need to manually break some links (such as
   // the weak references from {WasmMemoryObject::instances}.
-  if (owner->has_memory_object()) {
-    Handle<WasmMemoryObject> memory(owner->memory_object(), isolate);
-    Handle<WasmInstanceObject> instance(owner, isolate);
-    WasmMemoryObject::RemoveInstance(isolate, memory, instance);
+  if (instance->has_memory_object()) {
+    WasmMemoryObject::RemoveInstance(isolate, handle(instance->memory_object()),
+                                     handle(instance));
   }
 
   // weak_wasm_module may have been cleared, meaning the module object
@@ -735,11 +734,11 @@ void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
 
   void* invalid =
       reinterpret_cast<void*>(static_cast<uintptr_t>(kHeapObjectTag));
-  if (owner->indirect_function_table() &&
-      owner->indirect_function_table() != invalid) {
+  if (instance->indirect_function_table() &&
+      instance->indirect_function_table() != invalid) {
     // The indirect function table is C++ memory and needs to be explicitly
     // freed.
-    free(owner->indirect_function_table());
+    free(instance->indirect_function_table());
   }
 
   compiled_module->RemoveFromChain();
@@ -1448,11 +1447,6 @@ void WasmCompiledModule::RemoveFromChain() {
   }
 }
 
-void WasmCompiledModule::OnWasmModuleDecodingComplete(
-    Handle<WasmSharedModuleData> shared) {
-  set_shared(*shared);
-}
-
 void WasmCompiledModule::ReinitializeAfterDeserialization(
     Isolate* isolate, Handle<WasmCompiledModule> compiled_module) {
   // Reset, but don't delete any global handles, because their owning instance

src/wasm/wasm-objects.h

@@ -495,7 +495,7 @@ class WasmCompiledModule : public Struct {
   // for deserialization, and if they are serializable.
   // By default, instance values go to WasmInstanceObject, however, if
   // we embed the generated code with a value, then we track that value here.
-  WCM_CONST_OBJECT(WasmSharedModuleData, shared)
+  WCM_OBJECT(WasmSharedModuleData, shared)
   WCM_WEAK_LINK(Context, native_context)
   WCM_CONST_OBJECT(FixedArray, export_wrappers)
   WCM_OBJECT(FixedArray, weak_exported_functions)
@@ -523,7 +523,6 @@ class WasmCompiledModule : public Struct {
   wasm::NativeModule* GetNativeModule() const;
   void InsertInChain(WasmModuleObject*);
   void RemoveFromChain();
-  void OnWasmModuleDecodingComplete(Handle<WasmSharedModuleData>);
 
   DECL_ACCESSORS(raw_next_instance, Object);
   DECL_ACCESSORS(raw_prev_instance, Object);

src/wasm/wasm-serialization.cc

@@ -662,7 +662,7 @@ MaybeHandle<WasmCompiledModule> DeserializeNativeModule(
       WasmCompiledModule::New(isolate, shared->module(), export_wrappers,
                               std::vector<wasm::GlobalHandleAddress>(),
                               trap_handler::IsTrapHandlerEnabled());
-  compiled_module->OnWasmModuleDecodingComplete(shared);
+  compiled_module->set_shared(*shared);
   script->set_wasm_compiled_module(*compiled_module);
   NativeModuleDeserializer deserializer(isolate,
                                         compiled_module->GetNativeModule());

test/cctest/wasm/test-c-wasm-entry.cc

@@ -62,7 +62,7 @@ class CWasmEntryArgTester {
     Handle<Object> buffer_obj(reinterpret_cast<Object*>(arg_buffer.data()),
                               isolate_);
     CHECK(!buffer_obj->IsHeapObject());
-    WasmContext* wasm_context = wasm_code_->owner()
+    WasmContext* wasm_context = wasm_code_->native_module()
                                     ->compiled_module()
                                     ->owning_instance()
                                     ->wasm_context()
@@ -74,7 +74,7 @@ class CWasmEntryArgTester {
     static_assert(
         arraysize(call_args) == compiler::CWasmEntryParameters::kNumParameters,
         "adapt this test");
-    wasm_code_->owner()->SetExecutable(true);
+    wasm_code_->native_module()->SetExecutable(true);
     MaybeHandle<Object> return_obj = Execution::Call(
         isolate_, c_wasm_entry_fn_, receiver, arraysize(call_args), call_args);
     CHECK(!return_obj.is_null());

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

@@ -232,7 +232,7 @@ Handle<WasmInstanceObject> TestingModuleBuilder::InitInstanceObject() {
   Handle<WasmCompiledModule> compiled_module = WasmCompiledModule::New(
       isolate_, test_module_ptr_, export_wrappers, function_tables_,
       trap_handler::IsTrapHandlerEnabled());
-  compiled_module->OnWasmModuleDecodingComplete(shared_module_data);
+  compiled_module->set_shared(*shared_module_data);
   // This method is called when we initialize TestEnvironment. We don't
   // have a memory yet, so we won't create it here. We'll update the
   // interpreter when we get a memory. We do have globals, though.

test/unittests/wasm/wasm-code-manager-unittest.cc

@@ -308,11 +308,11 @@ TEST_F(WasmCodeManagerTest, Lookup) {
     NativeModulePtr nm1 = AllocModule(&manager, 1 * page(), style);
     NativeModulePtr nm2 = AllocModule(&manager, 1 * page(), style);
     WasmCode* code1_0 = AddCode(nm1.get(), 0, kCodeAlignment);
-    CHECK_EQ(nm1.get(), code1_0->owner());
+    CHECK_EQ(nm1.get(), code1_0->native_module());
     WasmCode* code1_1 = AddCode(nm1.get(), 1, kCodeAlignment);
     WasmCode* code2_0 = AddCode(nm2.get(), 0, kCodeAlignment);
     WasmCode* code2_1 = AddCode(nm2.get(), 1, kCodeAlignment);
-    CHECK_EQ(nm2.get(), code2_1->owner());
+    CHECK_EQ(nm2.get(), code2_1->native_module());
 
     CHECK_EQ(0, code1_0->index());
     CHECK_EQ(1, code1_1->index());
@@ -352,11 +352,11 @@ TEST_F(WasmCodeManagerTest, MultiManagerLookup) {
     NativeModulePtr nm2 = AllocModule(&manager2, 1 * page(), style);
 
     WasmCode* code1_0 = AddCode(nm1.get(), 0, kCodeAlignment);
-    CHECK_EQ(nm1.get(), code1_0->owner());
+    CHECK_EQ(nm1.get(), code1_0->native_module());
     WasmCode* code1_1 = AddCode(nm1.get(), 1, kCodeAlignment);
     WasmCode* code2_0 = AddCode(nm2.get(), 0, kCodeAlignment);
     WasmCode* code2_1 = AddCode(nm2.get(), 1, kCodeAlignment);
-    CHECK_EQ(nm2.get(), code2_1->owner());
+    CHECK_EQ(nm2.get(), code2_1->native_module());
 
     CHECK_EQ(0, code1_0->index());
     CHECK_EQ(1, code1_1->index());