[wasm] Various small cleanups/fixes

Changes:
- Fix a bug in objects-printer where array elements were not treated as
  tagged pointers.
- Fix a few TODOs, mainly in the wasm interpreter.
- Improve documentation, small refactorings.

Change-Id: I1d70ad454b3a0693b9b784b17395434d81d01b61
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3383136Reviewed-by: Nikolaos Papaspyrou <nikolaos@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78656}

src/compiler/wasm-compiler.cc

@@ -3065,7 +3065,7 @@ Node* WasmGraphBuilder::BuildImportCall(const wasm::FunctionSig* sig,
       func_index_intptr, gasm_->IntPtrConstant(kSystemPointerSize));
   Node* imported_targets =
       LOAD_INSTANCE_FIELD(ImportedFunctionTargets, MachineType::Pointer());
-  Node* target_node = gasm_->LoadImmutableFromObject(
+  Node* target_node = gasm_->LoadImmutable(
       MachineType::Pointer(), imported_targets, func_index_times_pointersize);
   args[0] = target_node;
 

src/diagnostics/objects-printer.cc

@@ -1805,9 +1805,16 @@ void WasmStruct::WasmStructPrint(std::ostream& os) {
       case wasm::kRef:
       case wasm::kOptRef:
       case wasm::kRtt:
-      case wasm::kRttWithDepth:
-        os << Brief(base::ReadUnalignedValue<Object>(field_address));
+      case wasm::kRttWithDepth: {
+        Tagged_t raw = base::ReadUnalignedValue<Tagged_t>(field_address);
+#if V8_COMPRESS_POINTERS
+        Address obj = DecompressTaggedPointer(address(), raw);
+#else
+        Address obj = raw;
+#endif
+        os << Brief(Object(obj));
         break;
+      }
       case wasm::kS128:
         os << "UNIMPLEMENTED";  // TODO(7748): Implement.
         break;
@@ -1844,17 +1851,24 @@ void WasmArray::WasmArrayPrint(std::ostream& os) {
                               true);
       break;
     case wasm::kI8:
+      PrintTypedArrayElements(os, reinterpret_cast<int8_t*>(data_ptr), len,
+                              true);
+      break;
     case wasm::kI16:
+      PrintTypedArrayElements(os, reinterpret_cast<int16_t*>(data_ptr), len,
+                              true);
+      break;
     case wasm::kS128:
     case wasm::kRef:
     case wasm::kOptRef:
     case wasm::kRtt:
     case wasm::kRttWithDepth:
-    case wasm::kBottom:
-    case wasm::kVoid:
       os << "\n   Printing elements of this type is unimplemented, sorry";
       // TODO(7748): Implement.
       break;
+    case wasm::kBottom:
+    case wasm::kVoid:
+      UNREACHABLE();
   }
   os << "\n";
 }

src/wasm/function-body-decoder-impl.h

@@ -1273,6 +1273,8 @@ class WasmDecoder : public Decoder {
   }
 
   bool Validate(const byte* pc, GlobalIndexImmediate<validate>& imm) {
+    // We compare with the current size of the globals vector. This is important
+    // if we are decoding a constant expression in the global section.
     if (!VALIDATE(imm.index < module_->globals.size())) {
       DecodeError(pc, "Invalid global index: %u", imm.index);
       return false;
@@ -2506,9 +2508,16 @@ class WasmFullDecoder : public WasmDecoder<validate, decoding_mode> {
             Append("T");
             break;
           case kControlIfElse:
+            Append("E");
+            break;
           case kControlTryCatch:
+            Append("C");
+            break;
           case kControlTryCatchAll:
-          case kControlLet:  // TODO(7748): Implement
+            Append("A");
+            break;
+          case kControlLet:
+            Append("D");
             break;
         }
         if (c.start_merge.arity) Append("%u-", c.start_merge.arity);

src/wasm/init-expr-interface.h

@@ -63,7 +63,7 @@ class InitExprInterface {
 #define UNREACHABLE_INTERFACE_FUNCTION(name, ...) \
   V8_INLINE void name(FullDecoder* decoder, ##__VA_ARGS__) { UNREACHABLE(); }
   INTERFACE_NON_CONSTANT_FUNCTIONS(UNREACHABLE_INTERFACE_FUNCTION)
-#undef EMPTY_INTERFACE_FUNCTION
+#undef UNREACHABLE_INTERFACE_FUNCTION
 
 #define DECLARE_INTERFACE_FUNCTION(name, ...) \
   void name(FullDecoder* decoder, ##__VA_ARGS__);

src/wasm/module-decoder.cc

@@ -859,6 +859,8 @@ class ModuleDecoderImpl : public Decoder {
   void DecodeGlobalSection() {
     uint32_t globals_count = consume_count("globals count", kV8MaxWasmGlobals);
     uint32_t imported_globals = static_cast<uint32_t>(module_->globals.size());
+    // It is important to not resize the globals vector from the beginning,
+    // because we use its current size when decoding the initializer.
     module_->globals.reserve(imported_globals + globals_count);
     for (uint32_t i = 0; ok() && i < globals_count; ++i) {
       TRACE("DecodeGlobal[%d] module+%d\n", i, static_cast<int>(pc_ - start_));
@@ -1928,10 +1930,8 @@ class ModuleDecoderImpl : public Decoder {
     ValueType* fields = zone->NewArray<ValueType>(field_count);
     bool* mutabilities = zone->NewArray<bool>(field_count);
     for (uint32_t i = 0; ok() && i < field_count; ++i) {
-      ValueType field = consume_storage_type();
-      fields[i] = field;
-      bool mutability = consume_mutability();
-      mutabilities[i] = mutability;
+      fields[i] = consume_storage_type();
+      mutabilities[i] = consume_mutability();
     }
     if (failed()) return nullptr;
     uint32_t* offsets = zone->NewArray<uint32_t>(field_count);
@@ -1939,10 +1939,10 @@ class ModuleDecoderImpl : public Decoder {
   }
 
   const ArrayType* consume_array(Zone* zone) {
-    ValueType field = consume_storage_type();
-    if (failed()) return nullptr;
+    ValueType element_type = consume_storage_type();
     bool mutability = consume_mutability();
-    return zone->New<ArrayType>(field, mutability);
+    if (failed()) return nullptr;
+    return zone->New<ArrayType>(element_type, mutability);
   }
 
   // Consume the attribute field of an exception.

src/wasm/module-instantiate.cc

@@ -2077,16 +2077,12 @@ void InstanceBuilder::LoadTableSegments(Handle<WasmInstanceObject> instance) {
                    instance->tables().get(elem_segment.table_index)),
                isolate_),
         table_index, segment_index, dst, src, count);
-    // Set the active segments to being already dropped, since memory.init on
-    // a dropped passive segment and an active segment have the same
-    // behavior.
+    // Set the active segments to being already dropped, since table.init on
+    // a dropped passive segment and an active segment have the same behavior.
     instance->dropped_elem_segments()[segment_index] = 1;
     if (!success) {
       thrower_->RuntimeError("table initializer is out of bounds");
-      // Break out instead of returning; we don't want to continue to
-      // initialize any further element segments, but still need to add
-      // dispatch tables below.
-      break;
+      return;
     }
   }
 }

src/wasm/value-type.h

@@ -69,13 +69,9 @@ class HeapType {
     kArray,                   // shorthand: g
     kAny,                     // shorthand: a
     // This value is used to represent failures in the parsing of heap types and
-    // does not correspond to a wasm heap type.
+    // does not correspond to a wasm heap type. It has to be last in this list.
     kBottom
   };
-  // Internal use only; defined in the public section to make it easy to
-  // check that they are defined correctly:
-  static constexpr Representation kFirstSentinel = kFunc;
-  static constexpr Representation kLastSentinel = kAny;
 
   static constexpr HeapType from_code(uint8_t code) {
     switch (code) {
@@ -181,8 +177,14 @@ class HeapType {
 
  private:
   friend class ValueType;
-  Representation representation_;
+
   constexpr bool is_valid() const { return representation_ <= kLastSentinel; }
+
+  static constexpr Representation kFirstSentinel =
+      static_cast<Representation>(kV8MaxWasmTypes);
+  static constexpr Representation kLastSentinel =
+      static_cast<Representation>(kBottom - 1);
+  Representation representation_;
 };
 
 enum Nullability : bool { kNonNullable, kNullable };

src/wasm/wasm-opcodes.h

@@ -661,6 +661,10 @@ bool V8_EXPORT_PRIVATE IsJSCompatibleSignature(const FunctionSig* sig,
   V(I64AtomicCompareExchange16U, 0xfe4d, l_ill) \
   V(I64AtomicCompareExchange32U, 0xfe4e, l_ill)
 
+#define FOREACH_ATOMIC_0_OPERAND_OPCODE(V)                      \
+  /* AtomicFence does not target a particular linear memory. */ \
+  V(AtomicFence, 0xfe03, v_v)
+
 #define FOREACH_GC_OPCODE(V)                                         \
   V(StructNewWithRtt, 0xfb01, _)                                     \
   V(StructNewDefaultWithRtt, 0xfb02, _)                              \
@@ -713,10 +717,6 @@ bool V8_EXPORT_PRIVATE IsJSCompatibleSignature(const FunctionSig* sig,
   V(BrOnNonI31, 0xfb65, _)                                           \
   V(BrOnNonArray, 0xfb67, _) /* not standardized - V8 experimental */
 
-#define FOREACH_ATOMIC_0_OPERAND_OPCODE(V)                      \
-  /* AtomicFence does not target a particular linear memory. */ \
-  V(AtomicFence, 0xfe03, v_v)
-
 // All opcodes.
 #define FOREACH_OPCODE(V)            \
   FOREACH_CONTROL_OPCODE(V)          \

test/cctest/wasm/test-gc.cc

@@ -240,67 +240,82 @@ ValueType optref(uint32_t type_index) {
 WASM_COMPILED_EXEC_TEST(WasmBasicStruct) {
   WasmGCTester tester(execution_tier);
 
-  const byte type_index =
+  const byte kStructIndex =
       tester.DefineStruct({F(kWasmI32, true), F(kWasmI32, true)});
-  const byte empty_struct_index = tester.DefineStruct({});
-  ValueType kRefType = ref(type_index);
-  ValueType kEmptyStructType = ref(empty_struct_index);
-  ValueType kOptRefType = optref(type_index);
-  FunctionSig sig_q_v(1, 0, &kRefType);
-  FunctionSig sig_qe_v(1, 0, &kEmptyStructType);
+  const byte kEmptyStructIndex = tester.DefineStruct({});
+  const byte kComplexStructIndex = tester.DefineStruct(
+      {F(kWasmI32, false), F(optref(kStructIndex), false), F(kWasmS128, false),
+       F(ValueType::Rtt(kStructIndex), false)});
+  auto sig_n_v = FixedSizeSignature<ValueType>::Returns(optref(kStructIndex));
+  auto sig_r_v = FixedSizeSignature<ValueType>::Returns(ref(kEmptyStructIndex));
+  auto sig_r_v_2 =
+      FixedSizeSignature<ValueType>::Returns(ref(kComplexStructIndex));
 
   // Test struct.new and struct.get.
   const byte kGet1 = tester.DefineFunction(
       tester.sigs.i_v(), {},
       {WASM_STRUCT_GET(
-           type_index, 0,
-           WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(42), WASM_I32V(64),
-                                    WASM_RTT_CANON(type_index))),
+           kStructIndex, 0,
+           WASM_STRUCT_NEW_WITH_RTT(kStructIndex, WASM_I32V(42), WASM_I32V(64),
+                                    WASM_RTT_CANON(kStructIndex))),
        kExprEnd});
 
   // Test struct.new and struct.get.
   const byte kGet2 = tester.DefineFunction(
       tester.sigs.i_v(), {},
       {WASM_STRUCT_GET(
-           type_index, 1,
-           WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(42), WASM_I32V(64),
-                                    WASM_RTT_CANON(type_index))),
+           kStructIndex, 1,
+           WASM_STRUCT_NEW_WITH_RTT(kStructIndex, WASM_I32V(42), WASM_I32V(64),
+                                    WASM_RTT_CANON(kStructIndex))),
        kExprEnd});
 
   // Test struct.new, returning struct reference.
   const byte kGetStruct = tester.DefineFunction(
-      &sig_q_v, {},
-      {WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(42), WASM_I32V(64),
-                                WASM_RTT_CANON(type_index)),
+      &sig_n_v, {},
+      {WASM_STRUCT_NEW_WITH_RTT(kStructIndex, WASM_I32V(42), WASM_I32V(64),
+                                WASM_RTT_CANON(kStructIndex)),
        kExprEnd});
 
   const byte kGetStructNominal = tester.DefineFunction(
-      &sig_q_v, {},
-      {WASM_STRUCT_NEW_DEFAULT(type_index), WASM_DROP,
-       WASM_STRUCT_NEW(type_index, WASM_I32V(42), WASM_I32V(64)), kExprEnd});
+      &sig_n_v, {},
+      {WASM_STRUCT_NEW_DEFAULT(kStructIndex), WASM_DROP,
+       WASM_STRUCT_NEW(kStructIndex, WASM_I32V(42), WASM_I32V(64)), kExprEnd});
 
   // Test struct.new, returning reference to an empty struct.
   const byte kGetEmptyStruct = tester.DefineFunction(
-      &sig_qe_v, {},
-      {WASM_STRUCT_NEW_WITH_RTT(empty_struct_index,
-                                WASM_RTT_CANON(empty_struct_index)),
+      &sig_r_v, {},
+      {WASM_STRUCT_NEW_WITH_RTT(kEmptyStructIndex,
+                                WASM_RTT_CANON(kEmptyStructIndex)),
        kExprEnd});
 
   // Test struct.set, struct refs types in locals.
   const byte j_local_index = 0;
   const byte j_field_index = 0;
   const byte kSet = tester.DefineFunction(
-      tester.sigs.i_v(), {kOptRefType},
+      tester.sigs.i_v(), {optref(kStructIndex)},
       {WASM_LOCAL_SET(
            j_local_index,
-           WASM_STRUCT_NEW_WITH_RTT(type_index, WASM_I32V(42), WASM_I32V(64),
-                                    WASM_RTT_CANON(type_index))),
-       WASM_STRUCT_SET(type_index, j_field_index, WASM_LOCAL_GET(j_local_index),
-                       WASM_I32V(-99)),
-       WASM_STRUCT_GET(type_index, j_field_index,
+           WASM_STRUCT_NEW_WITH_RTT(kStructIndex, WASM_I32V(42), WASM_I32V(64),
+                                    WASM_RTT_CANON(kStructIndex))),
+       WASM_STRUCT_SET(kStructIndex, j_field_index,
+                       WASM_LOCAL_GET(j_local_index), WASM_I32V(-99)),
+       WASM_STRUCT_GET(kStructIndex, j_field_index,
                        WASM_LOCAL_GET(j_local_index)),
        kExprEnd});
 
+  const byte kSimdConstant[16] = {0, 1, 2,  3,  4,  5,  6,  7,
+                                  8, 9, 10, 11, 12, 13, 14, 15};
+
+  const byte kComplexStructProducer = tester.DefineFunction(
+      &sig_r_v_2, {},
+      {WASM_STRUCT_NEW_WITH_RTT(kComplexStructIndex, WASM_I32V(42),
+                                WASM_STRUCT_NEW_DEFAULT_WITH_RTT(
+                                    kStructIndex, WASM_RTT_CANON(kStructIndex)),
+                                WASM_SIMD_CONSTANT(kSimdConstant),
+                                WASM_RTT_CANON(kStructIndex),
+                                WASM_RTT_CANON(kComplexStructIndex)),
+       kExprEnd});
+
   tester.CompileModule();
 
   tester.CheckResult(kGet1, 42);
@@ -313,6 +328,9 @@ WASM_COMPILED_EXEC_TEST(WasmBasicStruct) {
             .ToHandleChecked()
             ->IsWasmStruct());
   tester.CheckResult(kSet, -99);
+  CHECK(tester.GetResultObject(kComplexStructProducer)
+            .ToHandleChecked()
+            ->IsWasmStruct());
 }
 
 // Test struct.get, ref.as_non_null and ref-typed globals.

test/common/wasm/wasm-interpreter.cc

@@ -1440,7 +1440,7 @@ class WasmInterpreterInternals {
           val = WasmValue(isolate_->factory()->null_value(), p);
           break;
         }
-        case kRef:  // TODO(7748): Implement.
+        case kRef:
         case kRtt:
         case kRttWithDepth:
         case kVoid:
@@ -3164,29 +3164,11 @@ class WasmInterpreterInternals {
           break;
         }
         case kRef:
-        case kOptRef: {
-          switch (sig->GetParam(i).heap_representation()) {
-            case HeapType::kExtern:
-            case HeapType::kFunc:
-            case HeapType::kEq:
-            case HeapType::kData:
-            case HeapType::kArray:
-            case HeapType::kI31:
-            case HeapType::kAny: {
-              Handle<Object> ref = value.to_ref();
-              encoded_values->set(encoded_index++, *ref);
-              break;
-            }
-            case HeapType::kBottom:
-              UNREACHABLE();
-            default:
-              // TODO(7748): Implement these.
-              UNIMPLEMENTED();
-          }
-          break;
-        }
-        case kRtt:  // TODO(7748): Implement.
+        case kOptRef:
+        case kRtt:
         case kRttWithDepth:
+          encoded_values->set(encoded_index++, *value.to_ref());
+          break;
         case kI8:
         case kI16:
         case kVoid:
@@ -3270,28 +3252,13 @@ class WasmInterpreterInternals {
           break;
         }
         case kRef:
-        case kOptRef: {
-          switch (sig->GetParam(i).heap_representation()) {
-            case HeapType::kExtern:
-            case HeapType::kFunc:
-            case HeapType::kEq:
-            case HeapType::kData:
-            case HeapType::kArray:
-            case HeapType::kI31:
-            case HeapType::kAny: {
-              Handle<Object> ref(encoded_values->get(encoded_index++),
-                                 isolate_);
-              value = WasmValue(ref, sig->GetParam(i));
-              break;
-            }
-            default:
-              // TODO(7748): Implement these.
-              UNIMPLEMENTED();
-          }
+        case kOptRef:
+        case kRtt:
+        case kRttWithDepth: {
+          Handle<Object> ref(encoded_values->get(encoded_index++), isolate_);
+          value = WasmValue(ref, sig->GetParam(i));
           break;
         }
-        case kRtt:  // TODO(7748): Implement.
-        case kRttWithDepth:
         case kI8:
         case kI16:
         case kVoid:
@@ -3662,7 +3629,9 @@ class WasmInterpreterInternals {
             FOREACH_WASMVALUE_CTYPES(CASE_TYPE)
 #undef CASE_TYPE
             case kRef:
-            case kOptRef: {
+            case kOptRef:
+            case kRtt:
+            case kRttWithDepth: {
               // TODO(7748): Type checks or DCHECKs for ref types?
               HandleScope handle_scope(isolate_);  // Avoid leaking handles.
               Handle<FixedArray> global_buffer;    // The buffer of the global.
@@ -3674,8 +3643,6 @@ class WasmInterpreterInternals {
               global_buffer->set(global_index, *ref);
               break;
             }
-            case kRtt:  // TODO(7748): Implement.
-            case kRttWithDepth:
             case kI8:
             case kI16:
             case kVoid:
@@ -4073,25 +4040,18 @@ class WasmInterpreterInternals {
         case kVoid:
           PrintF("void");
           break;
-        case kRef:
-        case kOptRef: {
-          if (val.type().is_reference_to(HeapType::kExtern)) {
-            Handle<Object> ref = val.to_ref();
-            if (ref->IsNull()) {
-              PrintF("ref:null");
-            } else {
-              PrintF("ref:0x%" V8PRIxPTR, ref->ptr());
-            }
-          } else {
-            // TODO(7748): Implement this properly.
-            PrintF("ref/ref null");
+        case kOptRef:
+          if (val.to_ref()->IsNull()) {
+            PrintF("ref:null");
+            break;
           }
+          V8_FALLTHROUGH;
+        case kRef:
+          PrintF("ref:0x%" V8PRIxPTR, val.to_ref()->ptr());
           break;
-        }
         case kRtt:
         case kRttWithDepth:
-          // TODO(7748): Implement properly.
-          PrintF("rtt");
+          PrintF("rtt:0x%" V8PRIxPTR, val.to_ref()->ptr());
           break;
         case kI8:
         case kI16: