Reland "[wasm] Do memory.init bounds check in C++ code"

The return value of {memory_init_wrapper} was defined as {bool} in
the original CL. When compiled with clang, the full return register is
written when {true} or {false} is returned. With msvc, however, the
return value is written as a single byte, without zero-extension. In
generated code, the full return register is used and therefore stale
bytes in the return register caused problems.

With this CL the return value is changed to {uint32_t}. This enforces
zero-extension of the return value and thereby fixes the issue.

R=clemensb@chromium.org

Bug: v8:10281
Change-Id: I1446e51d88a35def56bd39a8336baa81543497bf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2096627
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66670}

src/compiler/wasm-compiler.cc

@@ -4892,55 +4892,21 @@ Node* WasmGraphBuilder::MemoryInit(uint32_t data_segment_index, Node* dst,
   // validation.
   DCHECK_LT(data_segment_index, env_->module->num_declared_data_segments);
 
-  Node* dst_fail = BoundsCheckMemRange(&dst, &size, position);
-  TrapIfTrue(wasm::kTrapMemOutOfBounds, dst_fail, position);
-
-  Node* seg_index = Uint32Constant(data_segment_index);
-  auto m = mcgraph()->machine();
-
-  {
-    // Load segment size from WasmInstanceObject::data_segment_sizes.
-    Node* seg_size_array =
-        LOAD_INSTANCE_FIELD(DataSegmentSizes, MachineType::Pointer());
-    STATIC_ASSERT(wasm::kV8MaxWasmDataSegments <= kMaxUInt32 >> 2);
-    Node* scaled_index = Uint32ToUintptr(
-        graph()->NewNode(m->Word32Shl(), seg_index, Int32Constant(2)));
-    Node* seg_size = SetEffect(graph()->NewNode(m->Load(MachineType::Uint32()),
-                                                seg_size_array, scaled_index,
-                                                effect(), control()));
-
-    // Bounds check the src index against the segment size.
-    Node* src_fail = BoundsCheckRange(src, &size, seg_size, position);
-    TrapIfTrue(wasm::kTrapMemOutOfBounds, src_fail, position);
-  }
-
-  {
-    // Load segment's base pointer from WasmInstanceObject::data_segment_starts.
-    Node* seg_start_array =
-        LOAD_INSTANCE_FIELD(DataSegmentStarts, MachineType::Pointer());
-    STATIC_ASSERT(wasm::kV8MaxWasmDataSegments <=
-                  kMaxUInt32 / kSystemPointerSize);
-    Node* scaled_index = Uint32ToUintptr(graph()->NewNode(
-        m->Word32Shl(), seg_index, Int32Constant(kSystemPointerSizeLog2)));
-    Node* seg_start = SetEffect(
-        graph()->NewNode(m->Load(MachineType::Pointer()), seg_start_array,
-                         scaled_index, effect(), control()));
-
-    // Convert src index to pointer.
-    src = graph()->NewNode(m->IntAdd(), seg_start, Uint32ToUintptr(src));
-  }
-
   Node* function = graph()->NewNode(mcgraph()->common()->ExternalConstant(
       ExternalReference::wasm_memory_init()));
 
-  Node* stack_slot =
-      StoreArgsInStackSlot({{MachineType::PointerRepresentation(), dst},
-                            {MachineType::PointerRepresentation(), src},
+  Node* stack_slot = StoreArgsInStackSlot(
+      {{MachineType::PointerRepresentation(), instance_node_.get()},
+       {MachineRepresentation::kWord32, dst},
+       {MachineRepresentation::kWord32, src},
+       {MachineRepresentation::kWord32,
+        gasm_->Uint32Constant(data_segment_index)},
        {MachineRepresentation::kWord32, size}});
 
-  MachineType sig_types[] = {MachineType::Pointer()};
-  MachineSignature sig(0, 1, sig_types);
-  return SetEffect(BuildCCall(&sig, function, stack_slot));
+  MachineType sig_types[] = {MachineType::Bool(), MachineType::Pointer()};
+  MachineSignature sig(1, 1, sig_types);
+  Node* call = SetEffect(BuildCCall(&sig, function, stack_slot));
+  return TrapIfFalse(wasm::kTrapMemOutOfBounds, call, position);
 }
 
 Node* WasmGraphBuilder::DataDrop(uint32_t data_segment_index,

src/wasm/wasm-external-refs.cc

@@ -325,13 +325,33 @@ void float64_pow_wrapper(Address data) {
   WriteUnalignedValue<double>(data, base::ieee754::pow(x, y));
 }
 
-void memory_init_wrapper(Address data) {
-  Address dst = ReadUnalignedValue<Address>(data);
-  Address src = ReadUnalignedValue<Address>(data + sizeof(dst));
-  uint32_t size =
-      ReadUnalignedValue<uint32_t>(data + sizeof(dst) + sizeof(src));
-  std::memmove(reinterpret_cast<byte*>(dst), reinterpret_cast<byte*>(src),
-               size);
+int32_t memory_init_wrapper(Address data) {
+  constexpr int32_t kSuccess = 1;
+  constexpr int32_t kOutOfBounds = 0;
+  DisallowHeapAllocation disallow_heap_allocation;
+  size_t offset = 0;
+  Object raw_instance = ReadUnalignedValue<Object>(data);
+  WasmInstanceObject instance = WasmInstanceObject::cast(raw_instance);
+  offset += sizeof(Object);
+  size_t dst = ReadUnalignedValue<uint32_t>(data + offset);
+  offset += sizeof(uint32_t);
+  size_t src = ReadUnalignedValue<uint32_t>(data + offset);
+  offset += sizeof(uint32_t);
+  uint32_t seg_index = ReadUnalignedValue<uint32_t>(data + offset);
+  offset += sizeof(uint32_t);
+  size_t size = ReadUnalignedValue<uint32_t>(data + offset);
+
+  size_t mem_size = instance.memory_size();
+  if (!base::IsInBounds(dst, size, mem_size)) return kOutOfBounds;
+
+  size_t seg_size = instance.data_segment_sizes()[seg_index];
+  if (!base::IsInBounds(src, size, seg_size)) return kOutOfBounds;
+
+  byte* mem_start = instance.memory_start();
+  byte* seg_start =
+      reinterpret_cast<byte*>(instance.data_segment_starts()[seg_index]);
+  std::memcpy(mem_start + dst, seg_start + src, size);
+  return kSuccess;
 }
 
 int32_t memory_copy_wrapper(Address data) {

src/wasm/wasm-external-refs.h

@@ -71,7 +71,9 @@ V8_EXPORT_PRIVATE void word64_ror_wrapper(Address data);
 
 V8_EXPORT_PRIVATE void float64_pow_wrapper(Address data);
 
-void memory_init_wrapper(Address data);
+// The return type is {int32_t} instead of {bool} to enforce the compiler to
+// zero-extend the result in the return register.
+int32_t memory_init_wrapper(Address data);
 
 // The return type is {int32_t} instead of {bool} to enforce the compiler to
 // zero-extend the result in the return register.