[wasm] Do memory.copy bounds check in C++ code

In the existing implementation we first did a bounds check in generated
code, and then called a simple C++ function to do the actual copying.
With this CL now we pass the WasmInstanceObject to the C++ function in
addition to the memory.copy parameters. Thereby we can do the bounds
check in C++, which is much easier, less error prone, and which also
speeds up code generation and reduces code size. Performance should not
be worse, because we were already doing the call to C++ anyways.

R=clemensb@chromium.org

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

src/codegen/external-reference.cc

@@ -289,6 +289,7 @@ FUNCTION_REFERENCE(wasm_word32_rol, wasm::word32_rol_wrapper)
 FUNCTION_REFERENCE(wasm_word32_ror, wasm::word32_ror_wrapper)
 FUNCTION_REFERENCE(wasm_word64_rol, wasm::word64_rol_wrapper)
 FUNCTION_REFERENCE(wasm_word64_ror, wasm::word64_ror_wrapper)
+FUNCTION_REFERENCE(wasm_memory_init, wasm::memory_init_wrapper)
 FUNCTION_REFERENCE(wasm_memory_copy, wasm::memory_copy_wrapper)
 FUNCTION_REFERENCE(wasm_memory_fill, wasm::memory_fill_wrapper)
 

src/codegen/external-reference.h

@@ -200,6 +200,7 @@ class StatsCounter;
   V(wasm_word64_ror, "wasm::word64_ror")                                      \
   V(wasm_word64_ctz, "wasm::word64_ctz")                                      \
   V(wasm_word64_popcnt, "wasm::word64_popcnt")                                \
+  V(wasm_memory_init, "wasm::memory_init")                                    \
   V(wasm_memory_copy, "wasm::memory_copy")                                    \
   V(wasm_memory_fill, "wasm::memory_fill")                                    \
   V(call_enqueue_microtask_function, "MicrotaskQueue::CallEnqueueMicrotask")  \

src/compiler/wasm-compiler.cc

@@ -4931,7 +4931,7 @@ Node* WasmGraphBuilder::MemoryInit(uint32_t data_segment_index, Node* dst,
   }
 
   Node* function = graph()->NewNode(mcgraph()->common()->ExternalConstant(
-      ExternalReference::wasm_memory_copy()));
+      ExternalReference::wasm_memory_init()));
 
   Node* stack_slot =
       StoreArgsInStackSlot({{MachineType::PointerRepresentation(), dst},
@@ -4981,22 +4981,19 @@ Node* WasmGraphBuilder::StoreArgsInStackSlot(
 
 Node* WasmGraphBuilder::MemoryCopy(Node* dst, Node* src, Node* size,
                                    wasm::WasmCodePosition position) {
-  Node* dst_fail = BoundsCheckMemRange(&dst, &size, position);
-  TrapIfTrue(wasm::kTrapMemOutOfBounds, dst_fail, position);
-  Node* src_fail = BoundsCheckMemRange(&src, &size, position);
-  TrapIfTrue(wasm::kTrapMemOutOfBounds, src_fail, position);
-
   Node* function = graph()->NewNode(mcgraph()->common()->ExternalConstant(
       ExternalReference::wasm_memory_copy()));
 
-  Node* stack_slot =
-      StoreArgsInStackSlot({{MachineType::PointerRepresentation(), dst},
-                            {MachineType::PointerRepresentation(), src},
-                            {MachineRepresentation::kWord32, size}});
+  Node* stack_slot = StoreArgsInStackSlot(
+      {{MachineType::PointerRepresentation(), instance_node_.get()},
+       {MachineRepresentation::kWord32, dst},
+       {MachineRepresentation::kWord32, src},
+       {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::MemoryFill(Node* dst, Node* value, Node* size,

src/wasm/module-instantiate.cc

@@ -669,11 +669,8 @@ void InstanceBuilder::LoadDataSegments(Handle<WasmInstanceObject> instance) {
       }
       // No need to copy empty segments.
       if (size == 0) continue;
-      Address dest_addr =
-          reinterpret_cast<Address>(instance->memory_start()) + dest_offset;
-      Address src_addr = reinterpret_cast<Address>(wire_bytes.begin()) +
-                         segment.source.offset();
-      memory_copy(dest_addr, src_addr, size);
+      std::memcpy(instance->memory_start() + dest_offset,
+                  wire_bytes.begin() + segment.source.offset(), size);
     } else {
       DCHECK(segment.active);
       // Segments of size == 0 are just nops.

src/wasm/wasm-external-refs.cc

@@ -11,7 +11,9 @@
 
 #include "src/base/bits.h"
 #include "src/base/ieee754.h"
+#include "src/common/assert-scope.h"
 #include "src/utils/memcopy.h"
+#include "src/wasm/wasm-objects-inl.h"
 
 #if defined(ADDRESS_SANITIZER) || defined(MEMORY_SANITIZER) || \
     defined(THREAD_SANITIZER) || defined(LEAK_SANITIZER) ||    \
@@ -323,41 +325,35 @@ void float64_pow_wrapper(Address data) {
   WriteUnalignedValue<double>(data, base::ieee754::pow(x, y));
 }
 
-// Asan on Windows triggers exceptions in this function to allocate
-// shadow memory lazily. When this function is called from WebAssembly,
-// these exceptions would be handled by the trap handler before they get
-// handled by Asan, and thereby confuse the thread-in-wasm flag.
-// Therefore we disable ASAN for this function. Alternatively we could
-// reset the thread-in-wasm flag before calling this function. However,
-// as this is only a problem with Asan on Windows, we did not consider
-// it worth the overhead.
-DISABLE_ASAN void memory_copy(Address dst, Address src, uint32_t size) {
-  // Use explicit forward and backward copy to match the required semantics for
-  // the memory.copy instruction. It is assumed that the caller of this
-  // function has already performed bounds checks, so {src + size} and
-  // {dst + size} should not overflow.
-  DCHECK(src + size >= src && dst + size >= dst);
-  uint8_t* dst8 = reinterpret_cast<uint8_t*>(dst);
-  uint8_t* src8 = reinterpret_cast<uint8_t*>(src);
-  if (src < dst && src + size > dst && dst + size > src) {
-    dst8 += size - 1;
-    src8 += size - 1;
-    for (; size > 0; size--) {
-      *dst8-- = *src8--;
-    }
-  } else {
-    for (; size > 0; size--) {
-      *dst8++ = *src8++;
-    }
-  }
-}
-
-void memory_copy_wrapper(Address data) {
+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));
-  memory_copy(dst, src, size);
+  std::memmove(reinterpret_cast<byte*>(dst), reinterpret_cast<byte*>(src),
+               size);
+}
+
+bool memory_copy_wrapper(Address data) {
+  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);
+  size_t size = ReadUnalignedValue<uint32_t>(data + offset);
+
+  size_t mem_size = instance.memory_size();
+  if (!base::IsInBounds(dst, size, mem_size)) return false;
+  if (!base::IsInBounds(src, size, mem_size)) return false;
+
+  byte* memory_start = instance.memory_start();
+  // Use std::memmove, because the ranges can overlap.
+  std::memmove(memory_start + dst, memory_start + src, size);
+  return true;
 }
 
 // Asan on Windows triggers exceptions in this function that confuse the

src/wasm/wasm-external-refs.h

@@ -71,9 +71,9 @@ V8_EXPORT_PRIVATE void word64_ror_wrapper(Address data);
 
 V8_EXPORT_PRIVATE void float64_pow_wrapper(Address data);
 
-void memory_copy_wrapper(Address data);
+void memory_init_wrapper(Address data);
 
-void memory_copy(Address dst, Address src, uint32_t size);
+bool memory_copy_wrapper(Address data);
 
 void memory_fill_wrapper(Address dst, uint32_t value, uint32_t size);
 

src/wasm/wasm-interpreter.cc

@@ -1824,7 +1824,8 @@ class ThreadImpl {
         Address src_addr =
             instance_object_->data_segment_starts()[imm.data_segment_index] +
             src;
-        memory_copy(dst_addr, src_addr, size);
+        std::memmove(reinterpret_cast<void*>(dst_addr),
+                     reinterpret_cast<void*>(src_addr), size);
         return true;
       }
       case kExprDataDrop: {
@@ -1850,7 +1851,8 @@ class ThreadImpl {
           return false;
         }
 
-        memory_copy(dst_addr, src_addr, size);
+        std::memmove(reinterpret_cast<void*>(dst_addr),
+                     reinterpret_cast<void*>(src_addr), size);
         return true;
       }
       case kExprMemoryFill: {