[wasm-gc] Some cleanup and documentation

Changes:
- Add dedicated exception for call_ref invoking a WasmJSFunction.
- Small restructuring of read_value_type.
- Change HeapType::kLastSentinel to point to the last valid type,
  update is_valid().
- Remove redundant DCHECK from ValueType constructors.
- Rename a few section-related macros in module-decoder-unittest.cc,
  add a small test.
- Rename "Simd128" -> "s128" in error message.
- Write some documentation, mostly in value-type.h and wasm-subtyping.h.

Bug: v8:7748
Change-Id: I4fc4826fbdeac50e21ef524787c2024d7aa1b3b2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2424139
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70118}

src/builtins/base.tq

@@ -330,6 +330,7 @@ extern enum MessageTemplate {
   kWasmTrapNullDereference,
   kWasmTrapIllegalCast,
   kWasmTrapArrayOutOfBounds,
+  kWasmTrapWasmJSFunction,
   kWeakRefsRegisterTargetAndHoldingsMustNotBeSame,
   kWeakRefsRegisterTargetMustBeObject,
   kWeakRefsUnregisterTokenMustBeObject,

src/builtins/wasm.tq

@@ -424,4 +424,8 @@ builtin ThrowWasmTrapIllegalCast(): JSAny {
 builtin ThrowWasmTrapArrayOutOfBounds(): JSAny {
   tail WasmTrap(SmiConstant(MessageTemplate::kWasmTrapArrayOutOfBounds));
 }
+
+builtin ThrowWasmTrapWasmJSFunction(): JSAny {
+  tail WasmTrap(SmiConstant(MessageTemplate::kWasmTrapWasmJSFunction));
+}
 }

src/common/globals.h

@@ -1627,6 +1627,7 @@ enum class LoadSensitivity {
   V(TrapRethrowNull)               \
   V(TrapNullDereference)           \
   V(TrapIllegalCast)               \
+  V(TrapWasmJSFunction)            \
   V(TrapArrayOutOfBounds)
 
 enum KeyedAccessLoadMode {

src/common/message-template.h

@@ -563,6 +563,7 @@ namespace internal {
   T(WasmTrapNullDereference, "dereferencing a null pointer")                   \
   T(WasmTrapIllegalCast, "illegal cast")                                       \
   T(WasmTrapArrayOutOfBounds, "array element access out of bounds")            \
+  T(WasmTrapWasmJSFunction, "cannot call WebAssembly.Function with call_ref")  \
   T(WasmExceptionError, "wasm exception")                                      \
   /* Asm.js validation related */                                              \
   T(AsmJsInvalid, "Invalid asm.js: %")                                         \

src/compiler/wasm-compiler.cc

@@ -3081,8 +3081,9 @@ Node* WasmGraphBuilder::BuildCallRef(uint32_t sig_index, Vector<Node*> args,
     // Call to a WasmJSFunction.
     // The call target is the wasm-to-js wrapper code.
     gasm_->Bind(&js_label);
-    // TODO(7748): Implement.
-    TrapIfTrue(wasm::kTrapUnreachable, gasm_->Int32Constant(1), position);
+    // TODO(9495): Implement when the interaction with the type reflection
+    // proposal is clear.
+    TrapIfTrue(wasm::kTrapWasmJSFunction, gasm_->Int32Constant(1), position);
     gasm_->Goto(&end_label, args[0], RefNull() /* Dummy value */);
   }
 

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

@@ -268,23 +268,24 @@ ValueType read_value_type(Decoder* decoder, const byte* pc,
       return heap_type.is_bottom() ? kWasmBottom
                                    : ValueType::Rtt(heap_type, depth);
     }
-    case kLocalS128:
+    case kLocalS128: {
       if (!VALIDATE(enabled.has_simd())) {
         decoder->error(pc,
-                       "invalid value type 'Simd128', enable with "
+                       "invalid value type 's128', enable with "
                        "--experimental-wasm-simd");
         return kWasmBottom;
       }
       return kWasmS128;
+    }
+    // Although these codes are included in ValueTypeCode, they technically
+    // do not correspond to value types and are only used in specific
+    // contexts. The caller of this function is responsible for handling them.
     case kLocalVoid:
     case kLocalI8:
     case kLocalI16:
-      // Although these types are included in ValueType, they are technically
-      // not value types and are only used in specific contexts. The caller of
-      // this function is responsible to check for them separately.
-      break;
+      return kWasmBottom;
   }
-  // Malformed modules specifying invalid types can get here.
+  // Anything that doesn't match an enumeration value is an invalid type code.
   return kWasmBottom;
 }
 }  // namespace value_type_reader

src/wasm/value-type.h

@@ -43,6 +43,12 @@ class Simd128;
   V(OptRef, kSystemPointerSizeLog2, OptRef, TaggedPointer, 'n', "ref null") \
   V(Bottom, -1, Void, None, '*', "<bot>")
 
+// Represents a WebAssembly heap type, as per the typed-funcref and gc
+// proposals.
+// The underlying Representation enumeration encodes heap types as follows:
+// a number t < kV8MaxWasmTypes represents the type defined in the module at
+// index t. Numbers directly beyond that represent the generic heap types. The
+// next number represents the bottom heap type (internal use).
 class HeapType {
  public:
   enum Representation : uint32_t {
@@ -58,7 +64,7 @@ class HeapType {
   // 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 = kBottom;
+  static constexpr Representation kLastSentinel = kI31;
 
   static constexpr HeapType from_code(uint8_t code) {
     switch (code) {
@@ -129,6 +135,7 @@ class HeapType {
     }
   }
 
+  // Returns the code that represents this heap type in the wasm binary format.
   constexpr int32_t code() const {
     // kLocal* codes represent the first byte of the LEB128 encoding. To get the
     // int32 represented by a code, we need to sign-extend it from 7 to 32 bits.
@@ -152,13 +159,17 @@ class HeapType {
  private:
   friend class ValueType;
   Representation representation_;
-  constexpr bool is_valid() const {
-    return !is_bottom() && representation_ <= kLastSentinel;
-  }
+  constexpr bool is_valid() const { return representation_ <= kLastSentinel; }
 };
 
 enum Nullability : bool { kNonNullable, kNullable };
 
+// A ValueType is encoded by three components: A Kind, a heap representation
+// (for reference types), and an inheritance depth (for rtts only). Those are
+// encoded into 32 bits using base::BitField. The underlying Kind enumeration
+// includes four elements which do not strictly correspond to value types: the
+// two packed types i8 and i16, the type of void blocks (stmt), and a bottom
+// value (for internal use).
 class ValueType {
  public:
   enum Kind : uint8_t {
@@ -205,8 +216,7 @@ class ValueType {
     return ValueType(KindField::encode(kind));
   }
   static constexpr ValueType Ref(uint32_t heap_type, Nullability nullability) {
-    CONSTEXPR_DCHECK(heap_type != HeapType::kBottom &&
-                     HeapType(heap_type).is_valid());
+    CONSTEXPR_DCHECK(HeapType(heap_type).is_valid());
     return ValueType(
         KindField::encode(nullability == kNullable ? kOptRef : kRef) |
         HeapTypeField::encode(heap_type));
@@ -217,8 +227,7 @@ class ValueType {
 
   static constexpr ValueType Rtt(uint32_t heap_type,
                                  uint8_t inheritance_depth) {
-    CONSTEXPR_DCHECK(heap_type != HeapType::kBottom &&
-                     HeapType(heap_type).is_valid());
+    CONSTEXPR_DCHECK(HeapType(heap_type).is_valid());
     return ValueType(KindField::encode(kRtt) |
                      HeapTypeField::encode(heap_type) |
                      DepthField::encode(inheritance_depth));
@@ -228,6 +237,7 @@ class ValueType {
     return Rtt(heap_type.representation(), inheritance_depth);
   }
 
+  // Useful when deserializing a type stored in a runtime object.
   static constexpr ValueType FromRawBitField(uint32_t bit_field) {
     return ValueType(bit_field);
   }
@@ -250,6 +260,7 @@ class ValueType {
     return heap_type().ref_index();
   }
 
+  // Useful when serializing this type to store it into a runtime object.
   constexpr uint32_t raw_bit_field() const { return bit_field_; }
 
   static constexpr size_t bit_field_offset() {
@@ -305,6 +316,12 @@ class ValueType {
     return machine_type().representation();
   }
 
+  // Returns the first byte of this type's representation in the wasm binary
+  // format.
+  // For compatibility with the reftypes and exception-handling proposals, this
+  // function prioritizes shorthand encodings
+  // (e.g., Ref(HeapType::kFunc, kNullable).value_type_code will return
+  // kLocalFuncref and not kLocalOptRef).
   constexpr ValueTypeCode value_type_code() const {
     CONSTEXPR_DCHECK(kind() != kBottom);
     switch (kind()) {
@@ -339,6 +356,8 @@ class ValueType {
     }
   }
 
+  // Returns true iff the heap type is needed to encode this type in the wasm
+  // binary format, taking into account available type shorthands.
   constexpr bool encoding_needs_heap_type() const {
     return (kind() == kRef && heap_representation() != HeapType::kI31) ||
            kind() == kRtt ||
@@ -455,6 +474,7 @@ inline std::ostream& operator<<(std::ostream& oss, ValueType type) {
   return oss << type.name();
 }
 
+// Precomputed primitive types.
 constexpr ValueType kWasmI32 = ValueType::Primitive(ValueType::kI32);
 constexpr ValueType kWasmI64 = ValueType::Primitive(ValueType::kI64);
 constexpr ValueType kWasmF32 = ValueType::Primitive(ValueType::kF32);

src/wasm/wasm-subtyping.h

@@ -17,15 +17,42 @@ V8_NOINLINE V8_EXPORT_PRIVATE bool IsSubtypeOfImpl(
     ValueType subtype, ValueType supertype, const WasmModule* sub_module,
     const WasmModule* super_module);
 
+// Checks if type1, defined in module1, is equivalent with type2, defined in
+// module2.
+// Type equivalence (~) is described by the following rules (structural
+// equivalence):
+// - Two numeric types are equivalent if they are equal.
+// - optref(ht1) ~ optref(ht2) iff ht1 ~ ht2.
+// - ref(ht1) ~ ref(ht2) iff ht1 ~ ht2.
+// - rtt(d1, ht1) ~ rtt(d2, ht2) iff (d1 = d2 and ht1 ~ ht2).
+// For heap types, the following rules hold:
+// - Two generic heap types are equivalent iff they are equal.
+// - Two structs are equivalent iff they contain the same number of fields and
+// these are pairwise equivalent.
+// - Two functions are equivalent iff they contain the same number of parameters
+// and returns and these are pairwise equivalent.
+// - Two arrays are equivalent iff their underlying types are equivalent.
 V8_NOINLINE bool EquivalentTypes(ValueType type1, ValueType type2,
                                  const WasmModule* module1,
                                  const WasmModule* module2);
 
-// The subtyping between value types is described by the following rules:
-// - All types are a supertype of bottom.
-// - All reference types, except funcref, are subtypes of eqref.
+// Checks if subtype, defined in module1, is a subtype of supertype, defined in
+// module2.
+// Subtyping between value types is described by the following rules
+// (structural subtyping):
+// - numeric types are subtype-related iff they are equal.
 // - optref(ht1) <: optref(ht2) iff ht1 <: ht2.
 // - ref(ht1) <: ref/optref(ht2) iff ht1 <: ht2.
+// - rtt1 <: rtt2 iff rtt1 ~ rtt2.
+// For heap types, the following subtyping rules hold:
+// - Each generic heap type is a subtype of itself.
+// - All functions are subtypes of func.
+// - i31, structs and arrays are subtypes of eq.
+// - Struct subtyping: Subtype must have at least as many fields as supertype,
+//   covariance for immutable fields, equivalence for mutable fields.
+// - Array subtyping (mutable only) is the equivalence relation.
+// - Function subtyping is the equivalence relation (note: this rule might
+//   change in the future to include type variance).
 V8_INLINE bool IsSubtypeOf(ValueType subtype, ValueType supertype,
                            const WasmModule* sub_module,
                            const WasmModule* super_module) {
@@ -33,6 +60,7 @@ V8_INLINE bool IsSubtypeOf(ValueType subtype, ValueType supertype,
   return IsSubtypeOfImpl(subtype, supertype, sub_module, super_module);
 }
 
+// Checks if 'subtype' is a subtype of 'supertype' (both defined in module).
 V8_INLINE bool IsSubtypeOf(ValueType subtype, ValueType supertype,
                            const WasmModule* module) {
   // If the types are trivially identical, exit early.
@@ -40,6 +68,10 @@ V8_INLINE bool IsSubtypeOf(ValueType subtype, ValueType supertype,
   return IsSubtypeOfImpl(subtype, supertype, module, module);
 }
 
+// Returns the weakest type that is a subtype of both a and b
+// (which is currently always one of a, b, or kWasmBottom).
+// TODO(manoskouk): Update this once we have settled on a type system for
+// reference types.
 ValueType CommonSubtype(ValueType a, ValueType b, const WasmModule* module);
 
 }  // namespace wasm