[wasm-gc] Fix lifetime of off-heap type information...

...while on-heap objects are referring to it. This is accomplished
by storing a reference to its associated WasmInstanceObject on every
WasmTypeInfo object.
Details: https://bit.ly/2UxD4hW

Fixed: v8:11953
Change-Id: Ifb6f976142356021393d41c50717d210d525d521
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3043959
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75863}

src/diagnostics/objects-printer.cc

@@ -1858,6 +1858,7 @@ void WasmTypeInfo::WasmTypeInfoPrint(std::ostream& os) {
   os << "\n - type address: " << reinterpret_cast<void*>(foreign_address());
   os << "\n - supertypes: " << Brief(supertypes());
   os << "\n - subtypes: " << Brief(subtypes());
+  os << "\n - instance: " << Brief(instance());
   os << "\n";
 }
 

src/heap/factory.cc

@@ -1466,9 +1466,9 @@ Handle<Foreign> Factory::NewForeign(Address addr) {
 }
 
 #if V8_ENABLE_WEBASSEMBLY
-Handle<WasmTypeInfo> Factory::NewWasmTypeInfo(Address type_address,
-                                              Handle<Map> opt_parent,
-                                              int instance_size_bytes) {
+Handle<WasmTypeInfo> Factory::NewWasmTypeInfo(
+    Address type_address, Handle<Map> opt_parent, int instance_size_bytes,
+    Handle<WasmInstanceObject> instance) {
   // We pretenure WasmTypeInfo objects because they are refererenced by Maps,
   // which are assumed to be long-lived. The supertypes list is constant
   // after initialization, so we pretenure that too.
@@ -1493,6 +1493,7 @@ Handle<WasmTypeInfo> Factory::NewWasmTypeInfo(Address type_address,
   result.set_supertypes(*supertypes, SKIP_WRITE_BARRIER);
   result.set_subtypes(*subtypes);
   result.set_instance_size(instance_size_bytes);
+  result.set_instance(*instance);
   return handle(result, isolate());
 }
 

src/heap/factory.h

@@ -561,7 +561,8 @@ class V8_EXPORT_PRIVATE Factory : public FactoryBase<Factory> {
 #if V8_ENABLE_WEBASSEMBLY
   Handle<WasmTypeInfo> NewWasmTypeInfo(Address type_address,
                                        Handle<Map> opt_parent,
-                                       int instance_size_bytes);
+                                       int instance_size_bytes,
+                                       Handle<WasmInstanceObject> instance);
   Handle<WasmCapiFunctionData> NewWasmCapiFunctionData(
       Address call_target, Handle<Foreign> embedder_data,
       Handle<Code> wrapper_code,

src/objects/objects-body-descriptors-inl.h

@@ -617,6 +617,7 @@ class WasmTypeInfo::BodyDescriptor final : public BodyDescriptorBase {
                                                         v);
     IteratePointer(obj, kSupertypesOffset, v);
     IteratePointer(obj, kSubtypesOffset, v);
+    IteratePointer(obj, kInstanceOffset, v);
   }
 
   static inline int SizeOf(Map map, HeapObject object) { return kSize; }
@@ -719,6 +720,8 @@ class WasmArray::BodyDescriptor final : public BodyDescriptorBase {
   template <typename ObjectVisitor>
   static inline void IterateBody(Map map, HeapObject obj, int object_size,
                                  ObjectVisitor* v) {
+    // The type is safe to use because it's kept alive by the {map}'s
+    // WasmTypeInfo.
     if (!WasmArray::GcSafeType(map)->element_type().is_reference()) return;
     IteratePointers(obj, WasmArray::kHeaderSize, object_size, v);
   }
@@ -741,6 +744,8 @@ class WasmStruct::BodyDescriptor final : public BodyDescriptorBase {
   static inline void IterateBody(Map map, HeapObject obj, int object_size,
                                  ObjectVisitor* v) {
     WasmStruct wasm_struct = WasmStruct::cast(obj);
+    // The {type} is safe to use because it's kept alive by the {map}'s
+    // WasmTypeInfo.
     wasm::StructType* type = WasmStruct::GcSafeType(map);
     for (uint32_t i = 0; i < type->field_count(); i++) {
       if (!type->field(i).is_reference()) continue;

src/wasm/module-instantiate.cc

@@ -139,7 +139,8 @@ Handle<DescriptorArray> CreateArrayDescriptorArray(
 
 // TODO(jkummerow): Move these elsewhere.
 Handle<Map> CreateStructMap(Isolate* isolate, const WasmModule* module,
-                            int struct_index, Handle<Map> opt_rtt_parent) {
+                            int struct_index, Handle<Map> opt_rtt_parent,
+                            Handle<WasmInstanceObject> instance) {
   const wasm::StructType* type = module->struct_type(struct_index);
   const int inobject_properties = 0;
   // We have to use the variable size sentinel because the instance size
@@ -150,7 +151,8 @@ Handle<Map> CreateStructMap(Isolate* isolate, const WasmModule* module,
   // TODO(jkummerow): If NO_ELEMENTS were supported, we could use that here.
   const ElementsKind elements_kind = TERMINAL_FAST_ELEMENTS_KIND;
   Handle<WasmTypeInfo> type_info = isolate->factory()->NewWasmTypeInfo(
-      reinterpret_cast<Address>(type), opt_rtt_parent, real_instance_size);
+      reinterpret_cast<Address>(type), opt_rtt_parent, real_instance_size,
+      instance);
   Handle<DescriptorArray> descriptors =
       CreateStructDescriptorArray(isolate, type);
   Handle<Map> map = isolate->factory()->NewMap(
@@ -163,7 +165,8 @@ Handle<Map> CreateStructMap(Isolate* isolate, const WasmModule* module,
 }
 
 Handle<Map> CreateArrayMap(Isolate* isolate, const WasmModule* module,
-                           int array_index, Handle<Map> opt_rtt_parent) {
+                           int array_index, Handle<Map> opt_rtt_parent,
+                           Handle<WasmInstanceObject> instance) {
   const wasm::ArrayType* type = module->array_type(array_index);
   const int inobject_properties = 0;
   const int instance_size = kVariableSizeSentinel;
@@ -172,7 +175,8 @@ Handle<Map> CreateArrayMap(Isolate* isolate, const WasmModule* module,
   const InstanceType instance_type = WASM_ARRAY_TYPE;
   const ElementsKind elements_kind = TERMINAL_FAST_ELEMENTS_KIND;
   Handle<WasmTypeInfo> type_info = isolate->factory()->NewWasmTypeInfo(
-      reinterpret_cast<Address>(type), opt_rtt_parent, cached_instance_size);
+      reinterpret_cast<Address>(type), opt_rtt_parent, cached_instance_size,
+      instance);
   // TODO(ishell): get canonical descriptor array for WasmArrays from roots.
   Handle<DescriptorArray> descriptors =
       CreateArrayDescriptorArray(isolate, type);
@@ -242,10 +246,10 @@ Handle<Map> AllocateSubRtt(Isolate* isolate,
   // Allocate a fresh RTT otherwise.
   Handle<Map> rtt;
   if (module->has_struct(type)) {
-    rtt = wasm::CreateStructMap(isolate, module, type, parent);
+    rtt = wasm::CreateStructMap(isolate, module, type, parent, instance);
   } else {
     DCHECK(module->has_array(type));
-    rtt = wasm::CreateArrayMap(isolate, module, type, parent);
+    rtt = wasm::CreateArrayMap(isolate, module, type, parent, instance);
   }
 
   if (mode == WasmRttSubMode::kCanonicalize) {
@@ -658,10 +662,12 @@ MaybeHandle<WasmInstanceObject> InstanceBuilder::Build() {
       Handle<Map> map;
       switch (module_->type_kinds[map_index]) {
         case kWasmStructTypeCode:
-          map = CreateStructMap(isolate_, module_, map_index, Handle<Map>());
+          map = CreateStructMap(isolate_, module_, map_index, Handle<Map>(),
+                                instance);
           break;
         case kWasmArrayTypeCode:
-          map = CreateArrayMap(isolate_, module_, map_index, Handle<Map>());
+          map = CreateArrayMap(isolate_, module_, map_index, Handle<Map>(),
+                               instance);
           break;
         case kWasmFunctionTypeCode:
           // TODO(7748): Think about canonicalizing rtts to make them work for

src/wasm/wasm-objects.h

@@ -943,9 +943,11 @@ class WasmArray : public TorqueGeneratedWasmArray<WasmArray, WasmObject> {
 namespace wasm {
 
 Handle<Map> CreateStructMap(Isolate* isolate, const WasmModule* module,
-                            int struct_index, MaybeHandle<Map> rtt_parent);
+                            int struct_index, MaybeHandle<Map> rtt_parent,
+                            Handle<WasmInstanceObject> instance);
 Handle<Map> CreateArrayMap(Isolate* isolate, const WasmModule* module,
-                           int array_index, MaybeHandle<Map> rtt_parent);
+                           int array_index, MaybeHandle<Map> rtt_parent,
+                           Handle<WasmInstanceObject> instance);
 Handle<Map> AllocateSubRtt(Isolate* isolate,
                            Handle<WasmInstanceObject> instance, uint32_t type,
                            Handle<Map> parent, WasmRttSubMode mode);

src/wasm/wasm-objects.tq

@@ -130,6 +130,15 @@ extern class WasmTypeInfo extends Foreign {
   subtypes: ArrayList;
   // In bytes, used for struct allocation.
   instance_size: Smi;
+  // We must make sure that the StructType/ArrayType, which is allocated in
+  // the WasmModule's "signature_zone", stays around as long as there are
+  // HeapObjects referring to it. Short term, we simply keep a reference to
+  // the instance, which in turn keeps the entire WasmModule alive.
+  // TODO(jkummerow): Possible optimization: manage the "signature_zone"'s
+  // lifetime separately by having WasmModule refer to it via std::shared_ptr,
+  // and introduce a new link from here to just that zone using a Managed<...>.
+  // Details: https://bit.ly/2UxD4hW
+  instance: WasmInstanceObject;
 }
 
 // WasmObject corresponds to data ref types which are WasmStruct and WasmArray.

test/mjsunit/wasm/wasm-array-js-interop.js

@@ -9,9 +9,6 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
 
 const kIterationsCountForICProgression = 20;
 
-// TODO(ishell): remove once leaked maps could keep NativeModule alive.
-let instances = [];
-
 function createArray_i() {
   let builder = new WasmModuleBuilder();
 
@@ -49,7 +46,6 @@ function createArray_i() {
     .exportAs("array_set");
 
   let instance = builder.instantiate();
-  instances.push(instance);
   let new_array = instance.exports.new_array;
   let array_get = instance.exports.array_get;
   let array_set = instance.exports.array_set;

test/mjsunit/wasm/wasm-struct-js-interop.js

@@ -9,9 +9,6 @@ d8.file.execute("test/mjsunit/wasm/wasm-module-builder.js");
 
 const kIterationsCountForICProgression = 20;
 
-// TODO(ishell): remove once leaked maps could keep NativeModule alive.
-let instances = [];
-
 function createSimpleStruct(field_type, value1, value2) {
   const builder = new WasmModuleBuilder();
 
@@ -52,7 +49,6 @@ function createSimpleStruct(field_type, value1, value2) {
     .exportAs("set_field");
 
   let instance = builder.instantiate();
-  instances.push(instance);
   let new_struct = instance.exports.new_struct;
   let get_field = instance.exports.get_field;
   let set_field = instance.exports.set_field;