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

This reverts commit c475e704.

Reason for revert: Fails on MSVC: https://ci.chromium.org/p/v8/builders/ci/V8%20Win64%20-%20msvc/12805

Original change's description:
> [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}

TBR=ahaas@chromium.org,clemensb@chromium.org

Change-Id: Ic2491f635a292e004f6c95498a045ba102138dc5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:10281
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2096623
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66658}

src/codegen/external-reference.cc

@@ -289,7 +289,6 @@ 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,7 +200,6 @@ 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_init()));
+      ExternalReference::wasm_memory_copy()));
 
   Node* stack_slot =
       StoreArgsInStackSlot({{MachineType::PointerRepresentation(), dst},
@@ -4981,19 +4981,22 @@ 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(), instance_node_.get()},
-       {MachineRepresentation::kWord32, dst},
-       {MachineRepresentation::kWord32, src},
-       {MachineRepresentation::kWord32, size}});
+  Node* stack_slot =
+      StoreArgsInStackSlot({{MachineType::PointerRepresentation(), dst},
+                            {MachineType::PointerRepresentation(), src},
+                            {MachineRepresentation::kWord32, size}});
 
-  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);
+  MachineType sig_types[] = {MachineType::Pointer()};
+  MachineSignature sig(0, 1, sig_types);
+  return SetEffect(BuildCCall(&sig, function, stack_slot));
 }
 
 Node* WasmGraphBuilder::MemoryFill(Node* dst, Node* value, Node* size,

src/wasm/module-instantiate.cc

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

src/wasm/wasm-external-refs.cc

@@ -11,9 +11,7 @@
 
 #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) ||    \
@@ -325,35 +323,41 @@ void float64_pow_wrapper(Address data) {
   WriteUnalignedValue<double>(data, base::ieee754::pow(x, y));
 }
 
-void memory_init_wrapper(Address data) {
+// 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) {
   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);
-}
-
-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;
+  memory_copy(dst, src, size);
 }
 
 // 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_init_wrapper(Address data);
+void memory_copy_wrapper(Address data);
 
-bool memory_copy_wrapper(Address data);
+void memory_copy(Address dst, Address src, uint32_t size);
 
 void memory_fill_wrapper(Address dst, uint32_t value, uint32_t size);
 

src/wasm/wasm-interpreter.cc

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