[wasm-gc] Support storing to primitive WasmObject fields in runtime

StoreICs use slow handler for now.

Bug: v8:11804
Change-Id: I008fc9a3639f649b63881f759078e664b16e25e3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2985403Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75395}

src/objects/lookup.cc

@@ -17,6 +17,7 @@
 #include "src/objects/heap-number-inl.h"
 #include "src/objects/map-updater.h"
 #include "src/objects/ordered-hash-table.h"
+#include "src/objects/property-details.h"
 #include "src/objects/struct-inl.h"
 
 #if V8_ENABLE_WEBASSEMBLY
@@ -330,6 +331,9 @@ void LookupIterator::InternalUpdateProtector(Isolate* isolate,
 void LookupIterator::PrepareForDataProperty(Handle<Object> value) {
   DCHECK(state_ == DATA || state_ == ACCESSOR);
   DCHECK(HolderIsReceiverOrHiddenPrototype());
+#if V8_ENABLE_WEBASSEMBLY
+  DCHECK(!receiver_->IsWasmObject(isolate_));
+#endif  // V8_ENABLE_WEBASSEMBLY
 
   Handle<JSReceiver> holder = GetHolder<JSReceiver>();
   // We are not interested in tracking constness of a JSProxy's direct
@@ -457,6 +461,9 @@ void LookupIterator::ReconfigureDataProperty(Handle<Object> value,
   DCHECK(HolderIsReceiverOrHiddenPrototype());
 
   Handle<JSReceiver> holder = GetHolder<JSReceiver>();
+#if V8_ENABLE_WEBASSEMBLY
+  if (V8_UNLIKELY(holder->IsWasmObject())) UNREACHABLE();
+#endif  // V8_ENABLE_WEBASSEMBLY
 
   // Property details can never change for private properties.
   if (holder->IsJSProxy(isolate_)) {
@@ -1042,12 +1049,19 @@ Handle<Object> LookupIterator::GetDataValue(
 void LookupIterator::WriteDataValue(Handle<Object> value,
                                     bool initializing_store) {
   DCHECK_EQ(DATA, state_);
+#if V8_ENABLE_WEBASSEMBLY
+  // WriteDataValueToWasmObject() must be used instead for writing to
+  // WasmObjects.
+  DCHECK(!holder_->IsWasmObject(isolate_));
+#endif  // V8_ENABLE_WEBASSEMBLY
+
   Handle<JSReceiver> holder = GetHolder<JSReceiver>();
   if (IsElement(*holder)) {
     Handle<JSObject> object = Handle<JSObject>::cast(holder);
     ElementsAccessor* accessor = object->GetElementsAccessor(isolate_);
     accessor->Set(object, number_, *value);
   } else if (holder->HasFastProperties(isolate_)) {
+    DCHECK(holder->IsJSObject(isolate_));
     if (property_details_.location() == kField) {
       // Check that in case of VariableMode::kConst field the existing value is
       // equal to |value|.
@@ -1090,6 +1104,42 @@ void LookupIterator::WriteDataValue(Handle<Object> value,
   }
 }
 
+#if V8_ENABLE_WEBASSEMBLY
+
+wasm::ValueType LookupIterator::wasm_value_type() const {
+  DCHECK(has_property_);
+  DCHECK(holder_->IsWasmObject(isolate_));
+  if (holder_->IsWasmStruct(isolate_)) {
+    wasm::StructType* wasm_struct_type = WasmStruct::cast(*holder_).type();
+    return wasm_struct_type->field(property_details_.field_index());
+
+  } else {
+    DCHECK(holder_->IsWasmArray(isolate_));
+    wasm::ArrayType* wasm_array_type = WasmArray::cast(*holder_).type();
+    return wasm_array_type->element_type();
+  }
+}
+
+void LookupIterator::WriteDataValueToWasmObject(Handle<Object> value) {
+  DCHECK_EQ(DATA, state_);
+  DCHECK(holder_->IsWasmObject(isolate_));
+  Handle<JSReceiver> holder = GetHolder<JSReceiver>();
+
+  if (IsElement(*holder)) {
+    // TODO(ishell): consider supporting indexed access to WasmStruct fields.
+    // TODO(v8:11804): implement stores to WasmArrays.
+    UNIMPLEMENTED();
+  } else {
+    // WasmArrays don't have writable properties.
+    DCHECK(holder->IsWasmStruct());
+    Handle<WasmStruct> holder = GetHolder<WasmStruct>();
+    WasmStruct::SetField(isolate_, holder, property_details_.field_index(),
+                         value);
+  }
+}
+
+#endif  // V8_ENABLE_WEBASSEMBLY
+
 template <bool is_element>
 bool LookupIterator::SkipInterceptor(JSObject holder) {
   InterceptorInfo info = GetInterceptor<is_element>(holder);
@@ -1214,17 +1264,19 @@ LookupIterator::State LookupIterator::LookupInRegularHolder(
 
   if (is_element && IsElement(holder)) {
 #if V8_ENABLE_WEBASSEMBLY
-    if (V8_UNLIKELY(holder.IsWasmObject())) {
+    if (V8_UNLIKELY(holder.IsWasmObject(isolate_))) {
       // TODO(ishell): consider supporting indexed access to WasmStruct fields.
-      if (holder.IsWasmArray()) {
+      if (holder.IsWasmArray(isolate_)) {
         WasmArray wasm_array = WasmArray::cast(holder);
         number_ = index_ < wasm_array.length() ? InternalIndex(index_)
                                                : InternalIndex::NotFound();
-        property_details_ =
-            PropertyDetails(kData, SEALED, PropertyCellType::kNoCell);
+        wasm::ArrayType* wasm_array_type = wasm_array.type();
+        property_details_ = PropertyDetails(
+            kData, wasm_array_type->mutability() ? SEALED : FROZEN,
+            PropertyCellType::kNoCell);
 
       } else {
-        DCHECK(holder.IsWasmStruct());
+        DCHECK(holder.IsWasmStruct(isolate_));
         DCHECK(number_.is_not_found());
       }
     } else  // NOLINT(readability/braces)

src/objects/lookup.h

@@ -13,6 +13,10 @@
 #include "src/objects/map.h"
 #include "src/objects/objects.h"
 
+#if V8_ENABLE_WEBASSEMBLY
+#include "src/wasm/value-type.h"
+#endif  // V8_ENABLE_WEBASSEMBLY
+
 namespace v8 {
 namespace internal {
 
@@ -188,6 +192,13 @@ class V8_EXPORT_PRIVATE LookupIterator final {
   static inline void UpdateProtector(Isolate* isolate, Handle<Object> receiver,
                                      Handle<Name> name);
 
+#if V8_ENABLE_WEBASSEMBLY
+  // Fetches type of WasmStruct's field or WasmArray's elements, it
+  // is used for preparing the value for storing into WasmObjects.
+  wasm::ValueType wasm_value_type() const;
+  void WriteDataValueToWasmObject(Handle<Object> value);
+#endif  // V8_ENABLE_WEBASSEMBLY
+
   // Lookup a 'cached' private property for an accessor.
   // If not found returns false and leaves the LookupIterator unmodified.
   bool TryLookupCachedProperty(Handle<AccessorPair> accessor);

src/objects/objects.cc

@@ -2741,9 +2741,10 @@ Maybe<bool> Object::RedefineIncompatibleProperty(
 }
 
 Maybe<bool> Object::SetDataProperty(LookupIterator* it, Handle<Object> value) {
-  DCHECK_IMPLIES(it->GetReceiver()->IsJSProxy(),
-                 it->GetName()->IsPrivateName());
-  DCHECK_IMPLIES(!it->IsElement() && it->GetName()->IsPrivateName(),
+  Isolate* isolate = it->isolate();
+  DCHECK_IMPLIES(it->GetReceiver()->IsJSProxy(isolate),
+                 it->GetName()->IsPrivateName(isolate));
+  DCHECK_IMPLIES(!it->IsElement() && it->GetName()->IsPrivateName(isolate),
                  it->state() == LookupIterator::DATA);
   Handle<JSReceiver> receiver = Handle<JSReceiver>::cast(it->GetReceiver());
 
@@ -2753,13 +2754,13 @@ Maybe<bool> Object::SetDataProperty(LookupIterator* it, Handle<Object> value) {
   Handle<Object> to_assign = value;
   // Convert the incoming value to a number for storing into typed arrays.
   // TODO(v8:11111): Support RAB / GSAB.
-  if (it->IsElement() && receiver->IsJSObject() &&
-      JSObject::cast(*receiver).HasTypedArrayElements()) {
+  if (it->IsElement() && receiver->IsJSObject(isolate) &&
+      JSObject::cast(*receiver).HasTypedArrayElements(isolate)) {
     ElementsKind elements_kind = JSObject::cast(*receiver).GetElementsKind();
     if (elements_kind == BIGINT64_ELEMENTS ||
         elements_kind == BIGUINT64_ELEMENTS) {
-      ASSIGN_RETURN_ON_EXCEPTION_VALUE(it->isolate(), to_assign,
-                                       BigInt::FromObject(it->isolate(), value),
+      ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, to_assign,
+                                       BigInt::FromObject(isolate, value),
                                        Nothing<bool>());
       // We have to recheck the length. However, it can only change if the
       // underlying buffer was detached, so just check that.
@@ -2767,9 +2768,9 @@ Maybe<bool> Object::SetDataProperty(LookupIterator* it, Handle<Object> value) {
         return Just(true);
         // TODO(neis): According to the spec, this should throw a TypeError.
       }
-    } else if (!value->IsNumber() && !value->IsUndefined(it->isolate())) {
-      ASSIGN_RETURN_ON_EXCEPTION_VALUE(it->isolate(), to_assign,
-                                       Object::ToNumber(it->isolate(), value),
+    } else if (!value->IsNumber() && !value->IsUndefined(isolate)) {
+      ASSIGN_RETURN_ON_EXCEPTION_VALUE(isolate, to_assign,
+                                       Object::ToNumber(isolate, value),
                                        Nothing<bool>());
       // We have to recheck the length. However, it can only change if the
       // underlying buffer was detached, so just check that.
@@ -2780,16 +2781,32 @@ Maybe<bool> Object::SetDataProperty(LookupIterator* it, Handle<Object> value) {
     }
   }
 
-  // Possibly migrate to the most up-to-date map that will be able to store
-  // |value| under it->name().
-  it->PrepareForDataProperty(to_assign);
+#if V8_ENABLE_WEBASSEMBLY
+  if (receiver->IsWasmObject(isolate)) {
+    // Prepares given value for being stored into a field of given Wasm type
+    // or throw if the value can't be stored into the field.
+    ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+        isolate, to_assign,
+        WasmObject::ToWasmValue(isolate, it->wasm_value_type(), to_assign),
+        Nothing<bool>());
+
+    // Store prepared value.
+    it->WriteDataValueToWasmObject(to_assign);
 
-  // Write the property value.
-  it->WriteDataValue(to_assign, false);
+  } else  // NOLINT(readability/braces)
+#endif    // V8_ENABLE_WEBASSEMBLY
+  {
+    // Possibly migrate to the most up-to-date map that will be able to store
+    // |value| under it->name().
+    it->PrepareForDataProperty(to_assign);
+
+    // Write the property value.
+    it->WriteDataValue(to_assign, false);
+  }
 
 #if VERIFY_HEAP
   if (FLAG_verify_heap) {
-    receiver->HeapObjectVerify(it->isolate());
+    receiver->HeapObjectVerify(isolate);
   }
 #endif
   return Just(true);

src/wasm/wasm-objects-inl.h

@@ -9,6 +9,8 @@
 #ifndef V8_WASM_WASM_OBJECTS_INL_H_
 #define V8_WASM_WASM_OBJECTS_INL_H_
 
+#include <type_traits>
+
 #include "src/base/memory.h"
 #include "src/heap/heap-write-barrier-inl.h"
 #include "src/objects/contexts-inl.h"
@@ -479,6 +481,120 @@ Handle<Object> WasmObject::ReadValueAt(Isolate* isolate, Handle<HeapObject> obj,
   }
 }
 
+// static
+MaybeHandle<Object> WasmObject::ToWasmValue(Isolate* isolate,
+                                            wasm::ValueType type,
+                                            Handle<Object> value) {
+  switch (type.kind()) {
+    case wasm::kI8:
+    case wasm::kI16:
+    case wasm::kI32:
+    case wasm::kF32:
+    case wasm::kF64:
+      return Object::ToNumber(isolate, value);
+
+    case wasm::kI64:
+      return BigInt::FromObject(isolate, value);
+
+    case wasm::kRef:
+    case wasm::kOptRef: {
+      // TODO(v8:11804): implement ref type check
+      UNREACHABLE();
+    }
+
+    case wasm::kS128:
+      // TODO(v8:11804): implement
+      UNREACHABLE();
+
+    case wasm::kRtt:
+    case wasm::kRttWithDepth:
+      // Rtt values are not supposed to be made available to JavaScript side.
+      UNREACHABLE();
+
+    case wasm::kVoid:
+    case wasm::kBottom:
+      UNREACHABLE();
+  }
+}
+
+// Conversions from Numeric objects.
+// static
+template <typename ElementType>
+ElementType WasmObject::FromNumber(Object value) {
+  // The value must already be prepared for storing to numeric fields.
+  DCHECK(value.IsNumber());
+  if (value.IsSmi()) {
+    return static_cast<ElementType>(Smi::ToInt(value));
+
+  } else if (value.IsHeapNumber()) {
+    double double_value = HeapNumber::cast(value).value();
+    if (std::is_same<ElementType, double>::value ||
+        std::is_same<ElementType, float>::value) {
+      return static_cast<ElementType>(double_value);
+    } else {
+      CHECK(std::is_integral<ElementType>::value);
+      return static_cast<ElementType>(DoubleToInt32(double_value));
+    }
+  }
+  UNREACHABLE();
+}
+
+// static
+void WasmObject::WriteValueAt(Isolate* isolate, Handle<HeapObject> obj,
+                              wasm::ValueType type, uint32_t offset,
+                              Handle<Object> value) {
+  Address field_address = obj->GetFieldAddress(offset);
+  switch (type.kind()) {
+    case wasm::kI8: {
+      auto scalar_value = FromNumber<int8_t>(*value);
+      base::Memory<int8_t>(field_address) = scalar_value;
+      break;
+    }
+    case wasm::kI16: {
+      auto scalar_value = FromNumber<int16_t>(*value);
+      base::Memory<int16_t>(field_address) = scalar_value;
+      break;
+    }
+    case wasm::kI32: {
+      auto scalar_value = FromNumber<int32_t>(*value);
+      base::Memory<int32_t>(field_address) = scalar_value;
+      break;
+    }
+    case wasm::kI64: {
+      int64_t scalar_value = BigInt::cast(*value).AsInt64();
+      base::WriteUnalignedValue<int64_t>(field_address, scalar_value);
+      break;
+    }
+    case wasm::kF32: {
+      auto scalar_value = FromNumber<float>(*value);
+      base::Memory<float>(field_address) = scalar_value;
+      break;
+    }
+    case wasm::kF64: {
+      auto scalar_value = FromNumber<double>(*value);
+      base::WriteUnalignedValue<double>(field_address, scalar_value);
+      break;
+    }
+    case wasm::kRef:
+    case wasm::kOptRef:
+      // TODO(v8:11804): implement
+      UNREACHABLE();
+
+    case wasm::kS128:
+      // TODO(v8:11804): implement
+      UNREACHABLE();
+
+    case wasm::kRtt:
+    case wasm::kRttWithDepth:
+      // Rtt values are not supposed to be made available to JavaScript side.
+      UNREACHABLE();
+
+    case wasm::kVoid:
+    case wasm::kBottom:
+      UNREACHABLE();
+  }
+}
+
 wasm::StructType* WasmStruct::type(Map map) {
   WasmTypeInfo type_info = map.wasm_type_info();
   return reinterpret_cast<wasm::StructType*>(type_info.foreign_address());
@@ -529,6 +645,16 @@ Handle<Object> WasmStruct::GetField(Isolate* isolate, Handle<WasmStruct> obj,
   return ReadValueAt(isolate, obj, field_type, offset);
 }
 
+// static
+void WasmStruct::SetField(Isolate* isolate, Handle<WasmStruct> obj,
+                          uint32_t field_index, Handle<Object> value) {
+  wasm::StructType* type = obj->type();
+  CHECK_LT(field_index, type->field_count());
+  wasm::ValueType field_type = type->field(field_index);
+  int offset = WasmStruct::kHeaderSize + type->field_offset(field_index);
+  WriteValueAt(isolate, obj, field_type, offset, value);
+}
+
 wasm::ArrayType* WasmArray::type(Map map) {
   DCHECK_EQ(WASM_ARRAY_TYPE, map.instance_type());
   WasmTypeInfo type_info = map.wasm_type_info();

src/wasm/wasm-objects.h

@@ -944,6 +944,10 @@ class WasmObject : public JSReceiver {
   DECL_CAST(WasmObject)
   DECL_VERIFIER(WasmObject)
 
+  // Prepares given value for being stored into a field of given Wasm type.
+  V8_WARN_UNUSED_RESULT static inline MaybeHandle<Object> ToWasmValue(
+      Isolate* isolate, wasm::ValueType type, Handle<Object> value);
+
  protected:
   // Returns boxed value of the object's field/element with given type and
   // offset.
@@ -952,6 +956,14 @@ class WasmObject : public JSReceiver {
                                            wasm::ValueType type,
                                            uint32_t offset);
 
+  static inline void WriteValueAt(Isolate* isolate, Handle<HeapObject> obj,
+                                  wasm::ValueType type, uint32_t offset,
+                                  Handle<Object> value);
+
+ private:
+  template <typename ElementType>
+  static ElementType FromNumber(Object value);
+
   OBJECT_CONSTRUCTORS(WasmObject, JSReceiver);
 };
 
@@ -976,6 +988,9 @@ class WasmStruct : public TorqueGeneratedWasmStruct<WasmStruct, WasmObject> {
                                         Handle<WasmStruct> obj,
                                         uint32_t field_index);
 
+  static inline void SetField(Isolate* isolate, Handle<WasmStruct> obj,
+                              uint32_t field_index, Handle<Object> value);
+
   DECL_CAST(WasmStruct)
   DECL_PRINTER(WasmStruct)
 

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

@@ -76,12 +76,30 @@ function SimpleStructInterop(field_type, value_generator,
   const { new_struct, get_field, set_field } =
       createSimpleStruct(field_type, value1, value2);
 
-  function f(o) {
+  function f(o, value) {
     for (let i = 0; i < kIterationsCountForICProgression; i++) {
-      let v = o.$field0;
+      let store_IC_exception;
+      try { o.$field0 = value; } catch (e) { store_IC_exception = e; }
+
+      let set_field_exception;
+      try { set_field(o, value); } catch (e) { set_field_exception = e; };
+
+      // set_field() and store IC should throw the same error at the same time.
+      assertEquals(set_field_exception, store_IC_exception);
+      if (set_field_exception != undefined) continue;
+
       let expected = get_field(o);
+
+      let v = o.$field0;
       assertEquals(expected, v);
 
+      // "Clear" the field value.
+      set_field(o, value1);
+      assertEquals(value1, get_field(o));
+
+      o.$field0 = value;
+      assertEquals(expected, get_field(o));
+
       v = o[i];
       assertEquals(undefined, v);
     }
@@ -93,108 +111,70 @@ function SimpleStructInterop(field_type, value_generator,
 
   for (const value of value_generator()) {
     print("value: " + value);
-    set_field(o, value);
-    f(o);
+    f(o, value);
   }
   gc();
 }
 
-(function TestSimpleStructsInteropI8() {
-  SimpleStructInterop(kWasmI8, function*() {
-    const max = 0x7f;
-    const min = -max - 1;
-    yield min;
+function MakeValueGenerator(max, ValueConstructor = Number) {
+  return function*() {
+    const max_safe_integer_float = 2**23 - 1;
+
+    yield -max;
+    yield -max - ValueConstructor(1);
     yield max;
-    yield 0;
+    yield 0.0;
+    yield -0.0;
+    yield 0n;
 
     for (let i = 0; i < 10; i++) {
-      yield Math.floor((Math.random() - 0.5) * max);
+      yield ValueConstructor(Math.floor((Math.random() - 0.5) * Number(max)));
     }
-  });
+
+    // Try some non-trivial values.
+    yield 153;
+    yield 77n;
+    yield ValueConstructor(Number.MAX_SAFE_INTEGER);
+    yield ValueConstructor(Number.MIN_SAFE_INTEGER);
+    yield ValueConstructor(max_safe_integer_float);
+
+    yield { foo:17 };
+    yield "boom";
+    yield { valueOf() { return 42; } };
+    yield { valueOf() { return 15142n; } };
+    yield { valueOf() { return BigInt(max); } };
+    yield { toString() { return "" + max; } };
+    yield { toString() { return "" + (-max); } };
+    yield { toString() { return "5421351n"; } };
+  };
+}
+
+(function TestSimpleStructsInteropI8() {
+  const max = 0x7f;
+  SimpleStructInterop(kWasmI8, MakeValueGenerator(max));
 })();
 
 (function TestSimpleStructsInteropI16() {
-  SimpleStructInterop(kWasmI16, function*() {
-    const max = 0x7fff;
-    const min = -max - 1;
-    yield min;
-    yield max;
-    yield 0;
-
-    for (let i = 0; i < 10; i++) {
-      yield Math.floor((Math.random() - 0.5) * max);
-    }
-  });
+  const max = 0x7fff;
+  SimpleStructInterop(kWasmI16, MakeValueGenerator(max));
 })();
 
 (function TestSimpleStructsInteropI32() {
-  SimpleStructInterop(kWasmI32, function*() {
-    const max = 0x7fffffff;
-    const min = -max - 1;
-    yield min;
-    yield max;
-    yield 0;
-
-    for (let i = 0; i < 10; i++) {
-      yield Math.floor((Math.random() - 0.5) * max);
-    }
-  });
+  const max = 0x7fffffff;
+  SimpleStructInterop(kWasmI32, MakeValueGenerator(max));
 })();
 
 (function TestSimpleStructsInteropI64() {
-  SimpleStructInterop(kWasmI64, function*() {
-    const max = 0x7fffffffffffffn;
-    const min = -max - 1n;
-    yield min;
-    yield max;
-    yield 0n;
-
-    for (let i = 0; i < 10; i++) {
-      yield BigInt(Math.floor((Math.random() - 0.5) * Number(max)));
-    }
-  }, 42n, 153n);
+  const max = 0x7fffffffffffffn;
+  SimpleStructInterop(kWasmI64, MakeValueGenerator(max, BigInt), 42n, 153n);
 })();
 
 (function TestSimpleStructsInteropF32() {
-  SimpleStructInterop(kWasmF32, function*() {
-    const max_safe_integer = 2**23 - 1;
-    const max = 3.4028234664e+38;
-    yield -max;
-    yield max;
-    yield max_safe_integer;
-    yield -max_safe_integer;
-    yield 0.0;
-    yield -0.0;
-
-    for (let i = 0; i < 10; i++) {
-      yield Math.floor((Math.random() - 0.5) * max);
-    }
-  });
+  const max = 3.402823466e+38;
+  SimpleStructInterop(kWasmF32, MakeValueGenerator(max));
 })();
 
 (function TestSimpleStructsInteropF64() {
-  SimpleStructInterop(kWasmF64, function*() {
-    const max = 1.7976931348623157e+308
-    yield -max;
-    yield max;
-    yield Number.MAX_SAFE_INTEGER;;
-    yield Number.MIN_SAFE_INTEGER;
-    yield 0.0;
-    yield -0.0;
-
-    for (let i = 0; i < 10; i++) {
-      yield Math.floor((Math.random() - 0.5) * max);
-    }
-  });
-})();
-
-(function TestSimpleStructsInteropRef() {
-  SimpleStructInterop(kWasmAnyRef, function*() {
-    yield "foo";
-    yield null;
-    yield {x:1};
-    yield {x:1.4, y:2.3};
-    yield Number(13);
-    yield this;
-  }, null, undefined);
+  const max = 1.7976931348623157e+308;
+  SimpleStructInterop(kWasmF64, MakeValueGenerator(max));
 })();