[wasm] Maintain link from Instance to Module.

This moves the link from a {WasmInstanceObject} to its corresponding
{WasmModuleObject} into the right place and also makes it strong. This
ensures that an instance always keeps the underlying module alive and
hence removes the situation of an "orphaned instance".

R=clemensh@chromium.org

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

src/objects-debug.cc

@@ -1516,7 +1516,6 @@ void WasmCompiledModule::WasmCompiledModuleVerify() {
   VerifyObjectField(kNextInstanceOffset);
   VerifyObjectField(kPrevInstanceOffset);
   VerifyObjectField(kOwningInstanceOffset);
-  VerifyObjectField(kWasmModuleOffset);
   VerifyObjectField(kNativeModuleOffset);
 }
 

src/runtime/runtime-test.cc

@@ -932,14 +932,6 @@ RUNTIME_FUNCTION(Runtime_ValidateWasmModuleState) {
   return isolate->heap()->ToBoolean(true);
 }
 
-RUNTIME_FUNCTION(Runtime_ValidateWasmOrphanedInstance) {
-  HandleScope shs(isolate);
-  DCHECK_EQ(1, args.length());
-  CONVERT_ARG_HANDLE_CHECKED(WasmInstanceObject, instance, 0);
-  WasmInstanceObject::ValidateOrphanedInstanceForTesting(isolate, instance);
-  return isolate->heap()->ToBoolean(true);
-}
-
 RUNTIME_FUNCTION(Runtime_HeapObjectVerify) {
   HandleScope shs(isolate);
   DCHECK_EQ(1, args.length());

src/runtime/runtime.h

@@ -596,7 +596,6 @@ namespace internal {
   F(UnblockConcurrentRecompilation, 0, 1)     \
   F(ValidateWasmInstancesChain, 2, 1)         \
   F(ValidateWasmModuleState, 1, 1)            \
-  F(ValidateWasmOrphanedInstance, 1, 1)       \
   F(WasmNumInterpretedCalls, 1, 1)            \
   F(WasmTraceMemory, 1, 1)
 

src/wasm/module-compiler.cc

@@ -1664,7 +1664,7 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
   //--------------------------------------------------------------------------
   CodeSpecialization code_specialization;
   Handle<WasmInstanceObject> instance =
-      WasmInstanceObject::New(isolate_, compiled_module_);
+      WasmInstanceObject::New(isolate_, module_object_, compiled_module_);
   Handle<WeakCell> weak_instance = factory->NewWeakCell(instance);
   Handle<WeakCell> old_weak_instance(original->weak_owning_instance(),
                                      isolate_);

src/wasm/wasm-objects-inl.h

@@ -127,6 +127,8 @@ PRIMITIVE_ACCESSORS(WasmInstanceObject, indirect_function_table_targets,
 
 ACCESSORS(WasmInstanceObject, compiled_module, WasmCompiledModule,
           kCompiledModuleOffset)
+ACCESSORS(WasmInstanceObject, module_object, WasmModuleObject,
+          kModuleObjectOffset)
 ACCESSORS(WasmInstanceObject, exports_object, JSObject, kExportsObjectOffset)
 ACCESSORS(WasmInstanceObject, native_context, Context, kNativeContextOffset)
 OPTIONAL_ACCESSORS(WasmInstanceObject, memory_object, WasmMemoryObject,
@@ -224,7 +226,6 @@ WCM_OBJECT(FixedArray, export_wrappers, kExportWrappersOffset)
 WCM_OBJECT(WasmCompiledModule, next_instance, kNextInstanceOffset)
 WCM_OBJECT(WasmCompiledModule, prev_instance, kPrevInstanceOffset)
 WCM_WEAK_LINK(WasmInstanceObject, owning_instance, kOwningInstanceOffset)
-WCM_WEAK_LINK(WasmModuleObject, wasm_module, kWasmModuleOffset)
 WCM_OBJECT(Foreign, native_module, kNativeModuleOffset)
 ACCESSORS(WasmCompiledModule, raw_next_instance, Object, kNextInstanceOffset);
 ACCESSORS(WasmCompiledModule, raw_prev_instance, Object, kPrevInstanceOffset);

src/wasm/wasm-objects.cc

@@ -267,9 +267,6 @@ Handle<WasmModuleObject> WasmModuleObject::New(
   auto module_object = Handle<WasmModuleObject>::cast(
       isolate->factory()->NewJSObject(module_cons));
   module_object->set_compiled_module(*compiled_module);
-  Handle<WeakCell> link_to_module =
-      isolate->factory()->NewWeakCell(module_object);
-  compiled_module->set_weak_wasm_module(*link_to_module);
 
   compiled_module->LogWasmCodes(isolate);
   return module_object;
@@ -279,8 +276,6 @@ void WasmModuleObject::ValidateStateForTesting(
     Isolate* isolate, Handle<WasmModuleObject> module_obj) {
   DisallowHeapAllocation no_gc;
   WasmCompiledModule* compiled_module = module_obj->compiled_module();
-  CHECK(compiled_module->has_weak_wasm_module());
-  CHECK_EQ(compiled_module->weak_wasm_module()->value(), *module_obj);
   CHECK(!compiled_module->has_prev_instance());
   CHECK(!compiled_module->has_next_instance());
   CHECK(!compiled_module->has_instance());
@@ -739,7 +734,8 @@ Handle<WasmDebugInfo> WasmInstanceObject::GetOrCreateDebugInfo(
 }
 
 Handle<WasmInstanceObject> WasmInstanceObject::New(
-    Isolate* isolate, Handle<WasmCompiledModule> compiled_module) {
+    Isolate* isolate, Handle<WasmModuleObject> module_object,
+    Handle<WasmCompiledModule> compiled_module) {
   Handle<JSFunction> instance_cons(
       isolate->native_context()->wasm_instance_constructor());
   Handle<JSObject> instance_object =
@@ -773,6 +769,7 @@ Handle<WasmInstanceObject> WasmInstanceObject::New(
   instance->set_indirect_function_table_targets(nullptr);
   instance->set_compiled_module(*compiled_module);
   instance->set_native_context(*isolate->native_context());
+  instance->set_module_object(*module_object);
 
   return instance;
 }
@@ -782,15 +779,12 @@ void WasmInstanceObject::ValidateInstancesChainForTesting(
   CHECK_GE(instance_count, 0);
   DisallowHeapAllocation no_gc;
   WasmCompiledModule* compiled_module = module_obj->compiled_module();
-  CHECK_EQ(JSObject::cast(compiled_module->weak_wasm_module()->value()),
-           *module_obj);
   Object* prev = nullptr;
   int found_instances = compiled_module->has_instance() ? 1 : 0;
   WasmCompiledModule* current_instance = compiled_module;
   while (current_instance->has_next_instance()) {
     CHECK((prev == nullptr && !current_instance->has_prev_instance()) ||
           current_instance->prev_instance() == prev);
-    CHECK_EQ(current_instance->weak_wasm_module()->value(), *module_obj);
     CHECK(current_instance->weak_owning_instance()
               ->value()
               ->IsWasmInstanceObject());
@@ -803,14 +797,6 @@ void WasmInstanceObject::ValidateInstancesChainForTesting(
   CHECK_EQ(found_instances, instance_count);
 }
 
-void WasmInstanceObject::ValidateOrphanedInstanceForTesting(
-    Isolate* isolate, Handle<WasmInstanceObject> instance) {
-  DisallowHeapAllocation no_gc;
-  WasmCompiledModule* compiled_module = instance->compiled_module();
-  CHECK(compiled_module->has_weak_wasm_module());
-  CHECK(compiled_module->weak_wasm_module()->cleared());
-}
-
 namespace {
 void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
   DisallowHeapAllocation no_gc;
@@ -826,7 +812,6 @@ void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
   } else {
     TRACE("Finalized already cleaned up compiled module\n");
   }
-  WeakCell* weak_wasm_module = compiled_module->weak_wasm_module();
 
   // Since the order of finalizers is not guaranteed, it can be the case
   // that {instance->compiled_module()->module()}, which is a
@@ -839,23 +824,18 @@ void InstanceFinalizer(const v8::WeakCallbackInfo<void>& data) {
                                      handle(instance));
   }
 
-  // weak_wasm_module may have been cleared, meaning the module object
-  // was GC-ed. We still want to maintain the links between instances, to
-  // release the WasmCompiledModule corresponding to the WasmModuleInstance
-  // being finalized here.
-  WasmModuleObject* wasm_module = nullptr;
-  if (!weak_wasm_module->cleared()) {
-    wasm_module = WasmModuleObject::cast(weak_wasm_module->value());
-    WasmCompiledModule* current_template = wasm_module->compiled_module();
-
-    DCHECK(!current_template->has_prev_instance());
-    if (current_template == compiled_module) {
-      if (!compiled_module->has_next_instance()) {
-        WasmCompiledModule::Reset(isolate, compiled_module);
-      } else {
-        WasmModuleObject::cast(wasm_module)
-            ->set_compiled_module(compiled_module->next_instance());
-      }
+  // We want to maintain a link from the {WasmModuleObject} to the first link
+  // within the linked {WasmInstanceObject} list, even if the last instance is
+  // finalized. This allows us to clone new {WasmCompiledModule} objects during
+  // instantiation without having to regenerate the compiled module.
+  WasmModuleObject* module_object = instance->module_object();
+  WasmCompiledModule* current_template = module_object->compiled_module();
+  DCHECK(!current_template->has_prev_instance());
+  if (current_template == compiled_module) {
+    if (!compiled_module->has_next_instance()) {
+      WasmCompiledModule::Reset(isolate, compiled_module);
+    } else {
+      module_object->set_compiled_module(compiled_module->next_instance());
     }
   }
 
@@ -1381,7 +1361,6 @@ Handle<WasmCompiledModule> WasmCompiledModule::Clone(
       isolate->factory()->NewStruct(WASM_COMPILED_MODULE_TYPE, TENURED));
   ret->set_shared(module->shared());
   ret->set_export_wrappers(module->export_wrappers());
-  ret->set_weak_wasm_module(module->weak_wasm_module());
   ret->set_weak_owning_instance(isolate->heap()->empty_weak_cell());
   ret->set_native_module(module->native_module());
 
@@ -1480,7 +1459,6 @@ void WasmCompiledModule::InsertInChain(WasmModuleObject* module) {
   WasmCompiledModule* original = module->compiled_module();
   set_next_instance(original);
   original->set_prev_instance(this);
-  set_weak_wasm_module(original->weak_wasm_module());
 }
 
 void WasmCompiledModule::RemoveFromChain() {

src/wasm/wasm-objects.h

@@ -262,6 +262,7 @@ class WasmInstanceObject : public JSObject {
   DECL_CAST(WasmInstanceObject)
 
   DECL_ACCESSORS(compiled_module, WasmCompiledModule)
+  DECL_ACCESSORS(module_object, WasmModuleObject)
   DECL_ACCESSORS(exports_object, JSObject)
   DECL_ACCESSORS(native_context, Context)
   DECL_OPTIONAL_ACCESSORS(memory_object, WasmMemoryObject)
@@ -287,6 +288,7 @@ class WasmInstanceObject : public JSObject {
 // Layout description.
 #define WASM_INSTANCE_OBJECT_FIELDS(V)                                  \
   V(kCompiledModuleOffset, kPointerSize)                                \
+  V(kModuleObjectOffset, kPointerSize)                                  \
   V(kExportsObjectOffset, kPointerSize)                                 \
   V(kNativeContextOffset, kPointerSize)                                 \
   V(kMemoryObjectOffset, kPointerSize)                                  \
@@ -330,15 +332,13 @@ class WasmInstanceObject : public JSObject {
   // If no debug info exists yet, it is created automatically.
   static Handle<WasmDebugInfo> GetOrCreateDebugInfo(Handle<WasmInstanceObject>);
 
-  static Handle<WasmInstanceObject> New(Isolate*, Handle<WasmCompiledModule>);
+  static Handle<WasmInstanceObject> New(Isolate*, Handle<WasmModuleObject>,
+                                        Handle<WasmCompiledModule>);
 
   static void ValidateInstancesChainForTesting(
       Isolate* isolate, Handle<WasmModuleObject> module_obj,
       int instance_count);
 
-  static void ValidateOrphanedInstanceForTesting(
-      Isolate* isolate, Handle<WasmInstanceObject> instance);
-
   static void InstallFinalizer(Isolate* isolate,
                                Handle<WasmInstanceObject> instance);
 
@@ -499,7 +499,6 @@ class WasmCompiledModule : public Struct {
   V(kNextInstanceOffset, kPointerSize)          \
   V(kPrevInstanceOffset, kPointerSize)          \
   V(kOwningInstanceOffset, kPointerSize)        \
-  V(kWasmModuleOffset, kPointerSize)            \
   V(kNativeModuleOffset, kPointerSize)          \
   V(kSize, 0)
 
@@ -536,7 +535,6 @@ class WasmCompiledModule : public Struct {
   WCM_CONST_OBJECT(WasmCompiledModule, next_instance)
   WCM_CONST_OBJECT(WasmCompiledModule, prev_instance)
   WCM_WEAK_LINK(WasmInstanceObject, owning_instance)
-  WCM_WEAK_LINK(WasmModuleObject, wasm_module)
   WCM_OBJECT(Foreign, native_module)
 
  public:

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

@@ -226,6 +226,8 @@ Handle<WasmInstanceObject> TestingModuleBuilder::InitInstanceObject() {
       WasmCompiledModule::New(isolate_, test_module_ptr_, export_wrappers, env);
   compiled_module->set_shared(*shared_module_data);
   compiled_module->GetNativeModule()->SetSharedModuleData(shared_module_data);
+  Handle<WasmModuleObject> module_object =
+      WasmModuleObject::New(isolate_, compiled_module);
   // 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.
@@ -233,7 +235,8 @@ Handle<WasmInstanceObject> TestingModuleBuilder::InitInstanceObject() {
 
   DCHECK(compiled_module->IsWasmCompiledModule());
   script->set_wasm_compiled_module(*compiled_module);
-  auto instance = WasmInstanceObject::New(isolate_, compiled_module);
+  auto instance =
+      WasmInstanceObject::New(isolate_, module_object, compiled_module);
   instance->set_globals_start(globals_data_);
   Handle<WeakCell> weak_instance = isolate()->factory()->NewWeakCell(instance);
   compiled_module->set_weak_owning_instance(*weak_instance);

test/mjsunit/wasm/compiled-module-management.js

@@ -82,7 +82,6 @@ print("After gc module state");
   module = null;
 })();
 
+// Note that the first GC will clear the module, the second the instance.
 gc();
-// the first GC will clear the module, the second the instance.
 gc();
-%ValidateWasmOrphanedInstance(instance4);