[wasm][bulk-memory] Adjust memory.copy to recent spec changes

CC=binji@chromium.org
R=mstarzinger@chromium.org

Change-Id: If613032af81f5cba152d1e4e45017eb13082ec76
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1706481
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62781}

src/compiler/wasm-compiler.cc

@@ -4778,16 +4778,19 @@ Node* WasmGraphBuilder::DataDrop(uint32_t data_segment_index,
 Node* WasmGraphBuilder::MemoryCopy(Node* dst, Node* src, Node* size,
                                    wasm::WasmCodePosition position) {
   auto m = mcgraph()->machine();
-  // The data must be copied backward if the regions overlap and src < dst. The
-  // regions overlap if {src + size > dst && dst + size > src}. Since we already
-  // test that {src < dst}, we know that {dst + size > src}, so this simplifies
-  // to just {src + size > dst}. That sum can overflow, but if we subtract
-  // {size} from both sides of the inequality we get the equivalent test
-  // {size > dst - src}.
-  Node* copy_backward = graph()->NewNode(
-      m->Word32And(), graph()->NewNode(m->Uint32LessThan(), src, dst),
-      graph()->NewNode(m->Uint32LessThan(),
-                       graph()->NewNode(m->Int32Sub(), dst, src), size));
+  auto common = mcgraph()->common();
+  // If size == 0, then memory.copy is a no-op.
+  Node* size_null_check =
+      graph()->NewNode(m->Word32Equal(), size, mcgraph()->Int32Constant(0));
+  Node* size_null_branch = graph()->NewNode(common->Branch(BranchHint::kFalse),
+                                            size_null_check, Control());
+
+  Node* size_null_etrue = Effect();
+  Node* size_null_if_false =
+      graph()->NewNode(common->IfFalse(), size_null_branch);
+  SetControl(size_null_if_false);
+  // The data must be copied backward if src < dst.
+  Node* copy_backward = graph()->NewNode(m->Uint32LessThan(), src, dst);
 
   Node* dst_fail = BoundsCheckMemRange(&dst, &size, position);
 
@@ -4807,9 +4810,16 @@ Node* WasmGraphBuilder::MemoryCopy(Node* dst, Node* src, Node* size,
                              MachineType::Uint32()};
   MachineSignature sig(0, 3, sig_types);
   BuildCCall(&sig, function, dst, src, size);
-  return TrapIfTrue(wasm::kTrapMemOutOfBounds,
-                    graph()->NewNode(m->Word32Or(), dst_fail, src_fail),
-                    position);
+  TrapIfTrue(wasm::kTrapMemOutOfBounds,
+             graph()->NewNode(m->Word32Or(), dst_fail, src_fail), position);
+  Node* size_null_if_true =
+      graph()->NewNode(common->IfTrue(), size_null_branch);
+
+  Node* merge = SetControl(
+      graph()->NewNode(common->Merge(2), size_null_if_true, Control()));
+  SetEffect(
+      graph()->NewNode(common->EffectPhi(2), size_null_etrue, Effect(), merge));
+  return merge;
 }
 
 Node* WasmGraphBuilder::MemoryFill(Node* dst, Node* value, Node* size,

src/wasm/wasm-interpreter.cc

@@ -1812,11 +1812,15 @@ class ThreadImpl {
       }
       case kExprMemoryCopy: {
         MemoryCopyImmediate<Decoder::kNoValidate> imm(decoder, code->at(pc));
+        *len += imm.length;
         auto size = Pop().to<uint32_t>();
         auto src = Pop().to<uint32_t>();
         auto dst = Pop().to<uint32_t>();
+        if (size == 0) {
+          return true;
+        }
         Address dst_addr;
-        bool copy_backward = src < dst && dst - src < size;
+        bool copy_backward = src < dst;
         bool ok = BoundsCheckMemRange(dst, &size, &dst_addr);
         // Trap without copying any bytes if we are copying backward and the
         // copy is partially out-of-bounds. We only need to check that the dst
@@ -1829,7 +1833,6 @@ class ThreadImpl {
           memory_copy_wrapper(dst_addr, src_addr, size);
         }
         if (!ok) DoTrap(kTrapMemOutOfBounds, pc);
-        *len += imm.length;
         return ok;
       }
       case kExprMemoryFill: {

test/cctest/wasm/test-run-wasm-bulk-memory.cc

@@ -206,9 +206,10 @@ WASM_EXEC_TEST(MemoryCopyOutOfBoundsData) {
 
   const uint32_t last_5_bytes = kWasmPageSize - 5;
 
-  // Write all values up to the out-of-bounds access.
+  // Copy with source < destination. Copy would happen backwards,
+  // but the first byte to copy is out-of-bounds, so no data should be written.
   CHECK_EQ(0xDEADBEEF, r.Call(last_5_bytes, 0, 6));
-  CheckMemoryEquals(r.builder(), last_5_bytes, {11, 22, 33, 44, 55});
+  CheckMemoryEquals(r.builder(), last_5_bytes, {0, 0, 0, 0, 0});
 
   // Copy overlapping with destination < source. Copy will happen forwards, up
   // to the out-of-bounds access.
@@ -247,9 +248,9 @@ WASM_EXEC_TEST(MemoryCopyOutOfBounds) {
   CHECK_EQ(0xDEADBEEF, r.Call(1000, 0, kWasmPageSize));
   CHECK_EQ(0xDEADBEEF, r.Call(kWasmPageSize, 0, 1));
 
-  // Copy 0 out-of-bounds fails.
-  CHECK_EQ(0xDEADBEEF, r.Call(kWasmPageSize + 1, 0, 0));
-  CHECK_EQ(0xDEADBEEF, r.Call(0, kWasmPageSize + 1, 0));
+  // Copy 0 out-of-bounds always succeeds.
+  CHECK_EQ(0, r.Call(kWasmPageSize + 1, 0, 0));
+  CHECK_EQ(0, r.Call(0, kWasmPageSize + 1, 0));
 
   // Make sure bounds aren't checked with 32-bit wrapping.
   CHECK_EQ(0xDEADBEEF, r.Call(1, 1, 0xFFFFFFFF));

test/wasm-spec-tests/wasm-spec-tests.status

@@ -17,7 +17,6 @@
   # TODO(ahaas): Incorporate recent changes to the bulk-memory-operations
   # proposal.
   'tests/proposals/bulk-memory-operations/elem': [FAIL],
-  'tests/proposals/bulk-memory-operations/memory_copy': [FAIL],
   'tests/proposals/bulk-memory-operations/table_copy': [FAIL],
   'tests/proposals/bulk-memory-operations/memory_init': [FAIL],
   'tests/proposals/bulk-memory-operations/memory_fill': [FAIL],