[wasm-gc] Implement array.get/array.set

With bounds checks, null checks, and a test case.

Bug: v8:7748
Change-Id: I9e7d68ecd883bd0279f22d11c1dc73cc8716a4cb
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2192659
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67714}

src/builtins/base.tq

@@ -324,6 +324,7 @@ extern enum MessageTemplate {
   kWasmTrapRethrowNullRef,
   kWasmTrapNullDereference,
   kWasmTrapIllegalCast,
+  kWasmTrapArrayOutOfBounds,
   ...
 }
 

src/builtins/wasm.tq

@@ -268,4 +268,8 @@ namespace wasm {
   builtin ThrowWasmTrapIllegalCast(): JSAny {
     tail WasmTrap(SmiConstant(MessageTemplate::kWasmTrapIllegalCast));
   }
+
+  builtin ThrowWasmTrapArrayOutOfBounds(): JSAny {
+    tail WasmTrap(SmiConstant(MessageTemplate::kWasmTrapArrayOutOfBounds));
+  }
 }

src/common/globals.h

@@ -1595,7 +1595,8 @@ enum class LoadSensitivity {
   V(TrapBrOnExnNullRef)            \
   V(TrapRethrowNullRef)            \
   V(TrapNullDereference)           \
-  V(TrapIllegalCast)
+  V(TrapIllegalCast)               \
+  V(TrapArrayOutOfBounds)
 
 enum KeyedAccessLoadMode {
   STANDARD_LOAD,

src/common/message-template.h

@@ -555,6 +555,7 @@ namespace internal {
   T(WasmTrapRethrowNullRef, "rethrowing nullref value")                        \
   T(WasmTrapNullDereference, "dereferencing a null pointer")                   \
   T(WasmTrapIllegalCast, "illegal cast")                                       \
+  T(WasmTrapArrayOutOfBounds, "array element access out of bounds")            \
   T(WasmExceptionError, "wasm exception")                                      \
   /* Asm.js validation related */                                              \
   T(AsmJsInvalid, "Invalid asm.js: %")                                         \

src/compiler/wasm-compiler.cc

@@ -5177,6 +5177,50 @@ Node* WasmGraphBuilder::StructSet(Node* struct_object,
                                    struct_type, field_index, field_value);
 }
 
+Node* ArrayElementOffset(GraphAssembler* gasm, Node* index,
+                         wasm::ValueType element_type) {
+  return gasm->Int32Add(
+      gasm->Int32Constant(WasmArray::kHeaderSize - kHeapObjectTag),
+      gasm->Int32Mul(index,
+                     gasm->Int32Constant(element_type.element_size_bytes())));
+}
+
+void WasmGraphBuilder::BoundsCheck(Node* array, Node* index,
+                                   wasm::WasmCodePosition position) {
+  Node* length = gasm_->Load(
+      MachineType::Uint32(), array,
+      gasm_->Int32Constant(WasmArray::kLengthOffset - kHeapObjectTag));
+  TrapIfFalse(wasm::kTrapArrayOutOfBounds, gasm_->Uint32LessThan(index, length),
+              position);
+}
+
+Node* WasmGraphBuilder::ArrayGet(Node* array_object,
+                                 const wasm::ArrayType* type, Node* index,
+                                 wasm::WasmCodePosition position) {
+  TrapIfTrue(wasm::kTrapNullDereference,
+             gasm_->WordEqual(array_object, RefNull()), position);
+  BoundsCheck(array_object, index, position);
+  MachineType machine_type = MachineType::TypeForRepresentation(
+      type->element_type().machine_representation());
+  Node* offset = ArrayElementOffset(gasm_.get(), index, type->element_type());
+  return gasm_->Load(machine_type, array_object, offset);
+}
+
+Node* WasmGraphBuilder::ArraySet(Node* array_object,
+                                 const wasm::ArrayType* type, Node* index,
+                                 Node* value, wasm::WasmCodePosition position) {
+  TrapIfTrue(wasm::kTrapNullDereference,
+             gasm_->WordEqual(array_object, RefNull()), position);
+  BoundsCheck(array_object, index, position);
+  WriteBarrierKind write_barrier = type->element_type().IsReferenceType()
+                                       ? kPointerWriteBarrier
+                                       : kNoWriteBarrier;
+  StoreRepresentation rep(type->element_type().machine_representation(),
+                          write_barrier);
+  Node* offset = ArrayElementOffset(gasm_.get(), index, type->element_type());
+  return gasm_->Store(rep, array_object, offset, value);
+}
+
 class WasmDecorator final : public GraphDecorator {
  public:
   explicit WasmDecorator(NodeOriginTable* origins, wasm::Decoder* decoder)

src/compiler/wasm-compiler.h

@@ -385,6 +385,11 @@ class WasmGraphBuilder {
                   wasm::WasmCodePosition position);
   Node* ArrayNew(uint32_t array_index, const wasm::ArrayType* type,
                  Node* length, Node* initial_value);
+  void BoundsCheck(Node* array, Node* index, wasm::WasmCodePosition position);
+  Node* ArrayGet(Node* array_object, const wasm::ArrayType* type, Node* index,
+                 wasm::WasmCodePosition position);
+  Node* ArraySet(Node* array_object, const wasm::ArrayType* type, Node* index,
+                 Node* value, wasm::WasmCodePosition position);
 
   bool has_simd() const { return has_simd_; }
 

src/wasm/baseline/liftoff-compiler.cc

@@ -3379,6 +3379,18 @@ class LiftoffCompiler {
     // TODO(7748): Implement.
     unsupported(decoder, kGC, "array.new");
   }
+  void ArrayGet(FullDecoder* decoder, const Value& array_obj,
+                const ArrayIndexImmediate<validate>& imm, const Value& index,
+                Value* result) {
+    // TODO(7748): Implement.
+    unsupported(decoder, kGC, "array.get");
+  }
+  void ArraySet(FullDecoder* decoder, const Value& array_obj,
+                const ArrayIndexImmediate<validate>& imm, const Value& index,
+                const Value& value) {
+    // TODO(7748): Implement.
+    unsupported(decoder, kGC, "array.set");
+  }
 
   void PassThrough(FullDecoder* decoder, const Value& from, Value* to) {
     // TODO(7748): Implement.

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

@@ -919,6 +919,12 @@ enum class LoadTransformationKind : uint8_t {
     const FieldIndexImmediate<validate>& field, const Value& field_value)     \
   F(ArrayNew, const ArrayIndexImmediate<validate>& imm, const Value& length,  \
     const Value& initial_value, Value* result)                                \
+  F(ArrayGet, const Value& array_obj,                                         \
+    const ArrayIndexImmediate<validate>& imm, const Value& index,             \
+    Value* result)                                                            \
+  F(ArraySet, const Value& array_obj,                                         \
+    const ArrayIndexImmediate<validate>& imm, const Value& index,             \
+    const Value& value)                                                       \
   F(PassThrough, const Value& from, Value* to)
 
 // Generic Wasm bytecode decoder with utilities for decoding immediates,
@@ -3072,10 +3078,28 @@ class WasmFullDecoder : public WasmDecoder<validate> {
                                     value);
         break;
       }
-      case kExprArrayGet:
-        UNIMPLEMENTED();  // TODO(7748): Implement.
+      case kExprArrayGet: {
+        ArrayIndexImmediate<validate> imm(this, this->pc_ + len);
+        len += imm.length;
+        if (!this->Validate(this->pc_ + len, imm)) break;
+        auto index = Pop(0, kWasmI32);
+        auto array_obj = Pop(0, ValueType(ValueType::kOptRef, imm.index));
+        auto* value = Push(imm.array_type->element_type());
+        // TODO(7748): Optimize this when array_obj is non-nullable ref.
+        CALL_INTERFACE_IF_REACHABLE(ArrayGet, array_obj, imm, index, value);
         break;
-      case kExprArraySet:
+      }
+      case kExprArraySet: {
+        ArrayIndexImmediate<validate> imm(this, this->pc_ + len);
+        len += imm.length;
+        if (!this->Validate(this->pc_ + len, imm)) break;
+        auto value = Pop(0, imm.array_type->element_type());
+        auto index = Pop(0, kWasmI32);
+        auto array_obj = Pop(0, ValueType(ValueType::kOptRef, imm.index));
+        // TODO(7748): Optimize this when array_obj is non-nullable ref.
+        CALL_INTERFACE_IF_REACHABLE(ArraySet, array_obj, imm, index, value);
+        break;
+      }
         UNIMPLEMENTED();  // TODO(7748): Implement.
         break;
       case kExprArrayLen:

src/wasm/graph-builder-interface.cc

@@ -658,6 +658,20 @@ class WasmGraphBuildingInterface {
                          initial_value.node);
   }
 
+  void ArrayGet(FullDecoder* decoder, const Value& array_obj,
+                const ArrayIndexImmediate<validate>& imm, const Value& index,
+                Value* result) {
+    result->node = BUILD(ArrayGet, array_obj.node, imm.array_type, index.node,
+                         decoder->position());
+  }
+
+  void ArraySet(FullDecoder* decoder, const Value& array_obj,
+                const ArrayIndexImmediate<validate>& imm, const Value& index,
+                const Value& value) {
+    BUILD(ArraySet, array_obj.node, imm.array_type, index.node, value.node,
+          decoder->position());
+  }
+
   void PassThrough(FullDecoder* decoder, const Value& from, Value* to) {
     to->node = from.node;
   }

test/cctest/wasm/test-gc.cc

@@ -185,6 +185,21 @@ WASM_EXEC_TEST(BasicArray) {
   int32_t type_index = builder->AddArrayType(&type);
   ValueType kRefTypes[] = {ValueType(ValueType::kRef, type_index)};
   FunctionSig sig_q_v(1, 0, kRefTypes);
+  ValueType kOptRefType = ValueType(ValueType::kOptRef, type_index);
+
+  WasmFunctionBuilder* f = builder->AddFunction(sigs.i_i());
+  uint32_t local_index = f->AddLocal(kOptRefType);
+  f->builder()->AddExport(CStrVector("f"), f);
+  // f: a = [12, 12, 12]; a[1] = 42; return a[arg0]
+  byte f_code[] = {
+      WASM_SET_LOCAL(local_index,
+                     WASM_ARRAY_NEW(type_index, WASM_I32V(12), WASM_I32V(3))),
+      WASM_ARRAY_SET(type_index, WASM_GET_LOCAL(local_index), WASM_I32V(1),
+                     WASM_I32V(42)),
+      WASM_ARRAY_GET(type_index, WASM_GET_LOCAL(local_index),
+                     WASM_GET_LOCAL(0)),
+      kExprEnd};
+  f->EmitCode(f_code, sizeof(f_code));
 
   WasmFunctionBuilder* h = builder->AddFunction(&sig_q_v);
   h->builder()->AddExport(CStrVector("h"), h);
@@ -205,12 +220,38 @@ WASM_EXEC_TEST(BasicArray) {
           isolate, &thrower, ModuleWireBytes(buffer.begin(), buffer.end()))
           .ToHandleChecked();
 
+  Handle<Object> argv[] = {handle(Smi::FromInt(0), isolate)};
+  CHECK_EQ(12, testing::CallWasmFunctionForTesting(isolate, instance, &thrower,
+                                                   "f", 1, argv));
+  argv[0] = handle(Smi::FromInt(1), isolate);
+  CHECK_EQ(42, testing::CallWasmFunctionForTesting(isolate, instance, &thrower,
+                                                   "f", 1, argv));
+  argv[0] = handle(Smi::FromInt(2), isolate);
+  CHECK_EQ(12, testing::CallWasmFunctionForTesting(isolate, instance, &thrower,
+                                                   "f", 1, argv));
+  Handle<Object> undefined = isolate->factory()->undefined_value();
+  {
+    Handle<WasmExportedFunction> f_export =
+        testing::GetExportedFunction(isolate, instance, "f").ToHandleChecked();
+    TryCatch try_catch(reinterpret_cast<v8::Isolate*>(isolate));
+    argv[0] = handle(Smi::FromInt(3), isolate);
+    MaybeHandle<Object> no_result =
+        Execution::Call(isolate, f_export, undefined, 1, argv);
+    CHECK(no_result.is_null());
+    CHECK(try_catch.HasCaught());
+    isolate->clear_pending_exception();
+    argv[0] = handle(Smi::FromInt(-1), isolate);
+    no_result = Execution::Call(isolate, f_export, undefined, 1, argv);
+    CHECK(no_result.is_null());
+    CHECK(try_catch.HasCaught());
+    isolate->clear_pending_exception();
+  }
+
   // TODO(7748): This uses the JavaScript interface to retrieve the plain
   // WasmArray. Once the JS interaction story is settled, this may well
   // need to be changed.
   Handle<WasmExportedFunction> h_export =
       testing::GetExportedFunction(isolate, instance, "h").ToHandleChecked();
-  Handle<Object> undefined = isolate->factory()->undefined_value();
   Handle<Object> ref_result =
       Execution::Call(isolate, h_export, undefined, 0, nullptr)
           .ToHandleChecked();

test/common/wasm/wasm-macro-gen.h

@@ -426,11 +426,11 @@ inline WasmOpcode LoadStoreOpcodeOf(MachineType type, bool store) {
 #define WASM_GC_OP(op) kGCPrefix, static_cast<byte>(op)
 #define WASM_STRUCT_NEW(index, ...) \
   __VA_ARGS__, WASM_GC_OP(kExprStructNew), static_cast<byte>(index)
-#define WASM_STRUCT_GET(typeidx, fieldidx, ...)                        \
-  __VA_ARGS__, WASM_GC_OP(kExprStructGet), static_cast<byte>(typeidx), \
+#define WASM_STRUCT_GET(typeidx, fieldidx, struct_obj)                \
+  struct_obj, WASM_GC_OP(kExprStructGet), static_cast<byte>(typeidx), \
       static_cast<byte>(fieldidx)
-#define WASM_STRUCT_SET(typeidx, fieldidx, ...)                        \
-  __VA_ARGS__, WASM_GC_OP(kExprStructSet), static_cast<byte>(typeidx), \
+#define WASM_STRUCT_SET(typeidx, fieldidx, struct_obj, value)                \
+  struct_obj, value, WASM_GC_OP(kExprStructSet), static_cast<byte>(typeidx), \
       static_cast<byte>(fieldidx)
 #define WASM_REF_NULL kExprRefNull
 #define WASM_REF_FUNC(val) kExprRefFunc, val
@@ -439,6 +439,10 @@ inline WasmOpcode LoadStoreOpcodeOf(MachineType type, bool store) {
 
 #define WASM_ARRAY_NEW(index, default_value, length) \
   default_value, length, WASM_GC_OP(kExprArrayNew), static_cast<byte>(index)
+#define WASM_ARRAY_GET(typeidx, array, index) \
+  array, index, WASM_GC_OP(kExprArrayGet), static_cast<byte>(typeidx)
+#define WASM_ARRAY_SET(typeidx, array, index, value) \
+  array, index, value, WASM_GC_OP(kExprArraySet), static_cast<byte>(typeidx)
 
 #define WASM_BR_ON_NULL(depth, ref_object) \
   ref_object, kExprBrOnNull, static_cast<byte>(depth)