[wasm-gc] Fix struct/array visiting

The old "gc-safe" implementation to get the off-heap type information
wasn't quite as gc-safe as it needs to be.
Due to parallel compaction, we shouldn't check for forwarding pointers;
instead we should rely on the old location of the Foreign, but make sure
not to look at its Map (which might be a forwarding pointer).

Bug: v8:12185
Change-Id: I4570b00a5300a0d7ed8c042fa21d355373e0e691
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3260513
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77707}

src/heap/factory.cc

@@ -1445,11 +1445,15 @@ Handle<Foreign> Factory::NewForeign(Address addr) {
 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.
-  // The subtypes list, however, is expected to grow (and hence be replaced),
-  // so we don't pretenure it.
+  // We pretenure WasmTypeInfo objects for two reasons:
+  // (1) They are referenced by Maps, which are assumed to be long-lived,
+  //     so pretenuring the WTI is a bit more efficient.
+  // (2) The object visitors need to read the WasmTypeInfo to find tagged
+  //     fields in Wasm structs; in the middle of a GC cycle that's only
+  //     safe to do if the WTI is in old space.
+  // The supertypes list is constant after initialization, so we pretenure
+  // that too. The subtypes list, however, is expected to grow (and hence be
+  // replaced), so we don't pretenure it.
   Handle<ArrayList> subtypes = ArrayList::New(isolate(), 0);
   Handle<FixedArray> supertypes;
   if (opt_parent.is_null()) {

src/wasm/wasm-objects-inl.h

@@ -548,10 +548,10 @@ wasm::StructType* WasmStruct::type(Map map) {
 wasm::StructType* WasmStruct::GcSafeType(Map map) {
   DCHECK_EQ(WASM_STRUCT_TYPE, map.instance_type());
   HeapObject raw = HeapObject::cast(map.constructor_or_back_pointer());
-  MapWord map_word = raw.map_word(kRelaxedLoad);
-  HeapObject forwarded =
-      map_word.IsForwardingAddress() ? map_word.ToForwardingAddress() : raw;
-  Foreign foreign = Foreign::cast(forwarded);
+  // The {Foreign} might be in the middle of being moved, which is why we
+  // can't read its map for a checked cast. But we can rely on its payload
+  // being intact in the old location.
+  Foreign foreign = Foreign::unchecked_cast(raw);
   return reinterpret_cast<wasm::StructType*>(foreign.foreign_address());
 }
 
@@ -624,10 +624,10 @@ wasm::ArrayType* WasmArray::type(Map map) {
 wasm::ArrayType* WasmArray::GcSafeType(Map map) {
   DCHECK_EQ(WASM_ARRAY_TYPE, map.instance_type());
   HeapObject raw = HeapObject::cast(map.constructor_or_back_pointer());
-  MapWord map_word = raw.map_word(kRelaxedLoad);
-  HeapObject forwarded =
-      map_word.IsForwardingAddress() ? map_word.ToForwardingAddress() : raw;
-  Foreign foreign = Foreign::cast(forwarded);
+  // The {Foreign} might be in the middle of being moved, which is why we
+  // can't read its map for a checked cast. But we can rely on its payload
+  // being intact in the old location.
+  Foreign foreign = Foreign::unchecked_cast(raw);
   return reinterpret_cast<wasm::ArrayType*>(foreign.foreign_address());
 }
 

test/mjsunit/mjsunit.status

@@ -1035,9 +1035,6 @@
   # BUG(v8:12013) Skipped until we remove flakes on NumFuzz.
   'compiler/concurrent-inlining-1': [SKIP],
   'compiler/inlined-call-polymorphic': [SKIP],
-
-  # BUG(v8:12185).
-  'wasm/wasm-struct-js-interop': [SKIP],
 }],  # 'deopt_fuzzer'
 
 ##############################################################################