[wasm-gc] Skip array.copy if length == 0

manoskouk · V8 LUCI CQ · commit 2ae6cda1ee8a · 2021-09-30T13:23:11.000Z
Bug: v8:7748 Change-Id: Id6adc39af6818f5a37307f26cfe40de11a0ce3c2 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3195872 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Manos Koukoutos <manoskouk@chromium.org> Cr-Commit-Position: refs/heads/main@{#77169}
diff --git a/src/builtins/wasm.tq b/src/builtins/wasm.tq
@@ -364,6 +364,7 @@ builtin WasmArrayCopyWithChecks(
       srcIndex + length > srcArray.length || srcIndex + length < srcIndex) {
     tail ThrowWasmTrapArrayOutOfBounds();
   }
+  if (length == 0) return Undefined;
   tail runtime::WasmArrayCopy(
       LoadContextFromFrame(), dstArray, SmiFromUint32(dstIndex), srcArray,
       SmiFromUint32(srcIndex), SmiFromUint32(length));
diff --git a/src/compiler/wasm-compiler.cc b/src/compiler/wasm-compiler.cc
@@ -6009,6 +6009,11 @@ void WasmGraphBuilder::ArrayCopy(Node* dst_array, Node* dst_index,
   BoundsCheckArrayCopy(dst_array, dst_index, length, position);
   BoundsCheckArrayCopy(src_array, src_index, length, position);
 
+  auto skip = gasm_->MakeLabel();
+
+  gasm_->GotoIf(gasm_->WordEqual(length, Int32Constant(0)), &skip,
+                BranchHint::kFalse);
+
   Node* function =
       gasm_->ExternalConstant(ExternalReference::wasm_array_copy());
   MachineType arg_types[]{
@@ -6018,6 +6023,8 @@ void WasmGraphBuilder::ArrayCopy(Node* dst_array, Node* dst_index,
   MachineSignature sig(0, 6, arg_types);
   BuildCCall(&sig, function, GetInstance(), dst_array, dst_index, src_array,
              src_index, length);
+  gasm_->Goto(&skip);
+  gasm_->Bind(&skip);
 }
 
 // 1 bit V8 Smi tag, 31 bits V8 Smi shift, 1 bit i31ref high-bit truncation.
diff --git a/src/runtime/runtime-wasm.cc b/src/runtime/runtime-wasm.cc
@@ -640,7 +640,7 @@ inline void* ArrayElementAddress(Handle<WasmArray> array, uint32_t index,
 }
 }  // namespace
 
-// Assumes copy ranges are in-bounds.
+// Assumes copy ranges are in-bounds and copy length > 0.
 RUNTIME_FUNCTION(Runtime_WasmArrayCopy) {
   ClearThreadInWasmScope flag_scope(isolate);
   HandleScope scope(isolate);
@@ -650,6 +650,7 @@ RUNTIME_FUNCTION(Runtime_WasmArrayCopy) {
   CONVERT_ARG_HANDLE_CHECKED(WasmArray, src_array, 2);
   CONVERT_UINT32_ARG_CHECKED(src_index, 3);
   CONVERT_UINT32_ARG_CHECKED(length, 4);
+  DCHECK_GT(length, 0);
   bool overlapping_ranges =
       dst_array->ptr() == src_array->ptr() &&
       (dst_index < src_index ? dst_index + length > src_index
diff --git a/src/wasm/wasm-external-refs.cc b/src/wasm/wasm-external-refs.cc
@@ -548,6 +548,7 @@ inline void* ArrayElementAddress(WasmArray array, uint32_t index,
 void array_copy_wrapper(Address raw_instance, Address raw_dst_array,
                         uint32_t dst_index, Address raw_src_array,
                         uint32_t src_index, uint32_t length) {
+  DCHECK_GT(length, 0);
   ThreadNotInWasmScope thread_not_in_wasm_scope;
   DisallowGarbageCollection no_gc;
   WasmArray dst_array = WasmArray::cast(Object(raw_dst_array));
diff --git a/src/wasm/wasm-external-refs.h b/src/wasm/wasm-external-refs.h
@@ -111,6 +111,7 @@ int32_t memory_copy_wrapper(Address data);
 // zero-extend the result in the return register.
 int32_t memory_fill_wrapper(Address data);
 
+// Assumes copy ranges are in-bounds and length > 0.
 void array_copy_wrapper(Address raw_instance, Address raw_dst_array,
                         uint32_t dst_index, Address raw_src_array,
                         uint32_t src_index, uint32_t length);
diff --git a/test/cctest/wasm/test-gc.cc b/test/cctest/wasm/test-gc.cc
@@ -1111,6 +1111,19 @@ WASM_COMPILED_EXEC_TEST(WasmArrayCopy) {
                        WASM_I32V(5)),
        kExprEnd});
 
+  const byte kZeroLength = tester.DefineFunction(
+      tester.sigs.i_v(), {optref(arrayref_index), optref(arrayref_index)},
+      {WASM_LOCAL_SET(
+           0, WASM_ARRAY_NEW_DEFAULT_WITH_RTT(arrayref_index, WASM_I32V(10),
+                                              WASM_RTT_CANON(arrayref_index))),
+       WASM_LOCAL_SET(
+           1, WASM_ARRAY_NEW_DEFAULT_WITH_RTT(arrayref_index, WASM_I32V(10),
+                                              WASM_RTT_CANON(arrayref_index))),
+       WASM_ARRAY_COPY(arrayref_index, arrayref_index, WASM_LOCAL_GET(1),
+                       WASM_I32V(6), WASM_LOCAL_GET(0), WASM_I32V(3),
+                       WASM_I32V(0)),
+       WASM_I32V(0), kExprEnd});
+
   tester.CompileModule();
 
   tester.CheckResult(kCopyI32, 0, 5);
@@ -1153,6 +1166,7 @@ WASM_COMPILED_EXEC_TEST(WasmArrayCopy) {
 
   tester.CheckHasThrown(kOobSource);
   tester.CheckHasThrown(kOobDestination);
+  tester.CheckResult(kZeroLength, 0);  // Does not throw.
 }
 
 WASM_COMPILED_EXEC_TEST(NewDefault) {

Original file line number	Diff line number	Diff line change
`@@ -364,6 +364,7 @@ builtin WasmArrayCopyWithChecks(`
`364`	`364`	`srcIndex + length > srcArray.length \|\| srcIndex + length < srcIndex) {`
`365`	`365`	`tail ThrowWasmTrapArrayOutOfBounds();`
`366`	`366`	`}`
	`367`	`+ if (length == 0) return Undefined;`
`367`	`368`	`tail runtime::WasmArrayCopy(`
`368`	`369`	`LoadContextFromFrame(), dstArray, SmiFromUint32(dstIndex), srcArray,`
`369`	`370`	`SmiFromUint32(srcIndex), SmiFromUint32(length));`