Commit 6e281ec3 authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

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

With recent spec changes, table.copy of length 0 does not trap anymore,
and we copy backwards whenever src < dst.

R=binji@chromium.org

Change-Id: I48e2b65083565631abc41bf4fdf4971f80fdf440
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1706471
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: 's avatarBen Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62797}
parent 0c919c45
...@@ -1832,6 +1832,8 @@ bool WasmInstanceObject::CopyTableEntries(Isolate* isolate, ...@@ -1832,6 +1832,8 @@ bool WasmInstanceObject::CopyTableEntries(Isolate* isolate,
uint32_t table_src_index, uint32_t table_src_index,
uint32_t dst, uint32_t src, uint32_t dst, uint32_t src,
uint32_t count) { uint32_t count) {
// Copying 0 elements is a no-op.
if (count == 0) return true;
CHECK_LT(table_dst_index, instance->tables().length()); CHECK_LT(table_dst_index, instance->tables().length());
CHECK_LT(table_src_index, instance->tables().length()); CHECK_LT(table_src_index, instance->tables().length());
auto table_dst = handle( auto table_dst = handle(
...@@ -1840,7 +1842,7 @@ bool WasmInstanceObject::CopyTableEntries(Isolate* isolate, ...@@ -1840,7 +1842,7 @@ bool WasmInstanceObject::CopyTableEntries(Isolate* isolate,
WasmTableObject::cast(instance->tables().get(table_src_index)), isolate); WasmTableObject::cast(instance->tables().get(table_src_index)), isolate);
uint32_t max_dst = static_cast<uint32_t>(table_dst->entries().length()); uint32_t max_dst = static_cast<uint32_t>(table_dst->entries().length());
uint32_t max_src = static_cast<uint32_t>(table_src->entries().length()); uint32_t max_src = static_cast<uint32_t>(table_src->entries().length());
bool copy_backward = src < dst && dst - src < count; bool copy_backward = src < dst;
bool ok = ClampToBounds(dst, &count, max_dst); bool ok = ClampToBounds(dst, &count, max_dst);
// Use & instead of && so the clamp is not short-circuited. // Use & instead of && so the clamp is not short-circuited.
ok &= ClampToBounds(src, &count, max_src); ok &= ClampToBounds(src, &count, max_src);
......
...@@ -776,34 +776,23 @@ void TestTableCopyOobWrites(ExecutionTier execution_tier, int table_dst, ...@@ -776,34 +776,23 @@ void TestTableCopyOobWrites(ExecutionTier execution_tier, int table_dst,
CheckTable(isolate, table, f0, f1, f2, f3, f4); CheckTable(isolate, table, f0, f1, f2, f3, f4);
// Non-overlapping, src < dst. // Non-overlapping, src < dst. Because of src < dst, we copy backwards.
// Therefore the first access already traps, and the table is not changed.
r.CheckCallViaJS(0xDEADBEEF, 3, 0, 3); r.CheckCallViaJS(0xDEADBEEF, 3, 0, 3);
CheckTable(isolate, table, f0, f1, f2, f0, f1); CheckTable(isolate, table, f0, f1, f2, f3, f4);
// Non-overlapping, dst < src. // Non-overlapping, dst < src.
r.CheckCallViaJS(0xDEADBEEF, 0, 4, 2); r.CheckCallViaJS(0xDEADBEEF, 0, 4, 2);
if (table_dst == table_src) { CheckTable(isolate, table, f4, f1, f2, f3, f4);
CheckTable(isolate, table, f1, f1, f2, f0, f1);
} else {
CheckTable(isolate, table, f4, f1, f2, f0, f1);
}
// Overlapping, src < dst. This is required to copy backward, but the first // Overlapping, src < dst. This is required to copy backward, but the first
// access will be out-of-bounds, so nothing changes. // access will be out-of-bounds, so nothing changes.
r.CheckCallViaJS(0xDEADBEEF, 3, 0, 99); r.CheckCallViaJS(0xDEADBEEF, 3, 0, 99);
if (table_dst == table_src) { CheckTable(isolate, table, f4, f1, f2, f3, f4);
CheckTable(isolate, table, f1, f1, f2, f0, f1);
} else {
CheckTable(isolate, table, f4, f1, f2, f0, f1);
}
// Overlapping, dst < src. // Overlapping, dst < src.
r.CheckCallViaJS(0xDEADBEEF, 0, 1, 99); r.CheckCallViaJS(0xDEADBEEF, 0, 1, 99);
if (table_dst == table_src) { CheckTable(isolate, table, f1, f2, f3, f4, f4);
CheckTable(isolate, table, f1, f2, f0, f1, f1);
} else {
CheckTable(isolate, table, f1, f2, f3, f4, f1);
}
} }
WASM_EXEC_TEST(TableCopyOobWritesFrom0To0) { WASM_EXEC_TEST(TableCopyOobWritesFrom0To0) {
...@@ -848,8 +837,8 @@ void TestTableCopyOob1(ExecutionTier execution_tier, int table_dst, ...@@ -848,8 +837,8 @@ void TestTableCopyOob1(ExecutionTier execution_tier, int table_dst,
{ {
const uint32_t big = 1000000; const uint32_t big = 1000000;
r.CheckCallViaJS(0xDEADBEEF, big, 0, 0); r.CheckCallViaJS(0, big, 0, 0);
r.CheckCallViaJS(0xDEADBEEF, 0, big, 0); r.CheckCallViaJS(0, 0, big, 0);
} }
for (uint32_t big = 4294967295; big > 1000; big >>= 1) { for (uint32_t big = 4294967295; big > 1000; big >>= 1) {
......
...@@ -50,10 +50,11 @@ resetTable(); ...@@ -50,10 +50,11 @@ resetTable();
instance.exports.copy(3, 0, 2); instance.exports.copy(3, 0, 2);
assertTable([1000, 1001, 1002, 1000, 1001]); assertTable([1000, 1001, 1002, 1000, 1001]);
// Non-overlapping, src < dst. // Non-overlapping, src < dst. Because of src < dst, we copy backwards.
// Therefore the first access already traps, and the table is not changed.
resetTable(); resetTable();
assertTraps(kTrapTableOutOfBounds, () => instance.exports.copy(3, 0, 3)); assertTraps(kTrapTableOutOfBounds, () => instance.exports.copy(3, 0, 3));
assertTable([1000, 1001, 1002, 1000, 1001]); assertTable([1000, 1001, 1002, 1003, 1004]);
// Non-overlapping, dst < src. // Non-overlapping, dst < src.
resetTable(); resetTable();
......
...@@ -17,7 +17,6 @@ ...@@ -17,7 +17,6 @@
# TODO(ahaas): Incorporate recent changes to the bulk-memory-operations # TODO(ahaas): Incorporate recent changes to the bulk-memory-operations
# proposal. # proposal.
'tests/proposals/bulk-memory-operations/elem': [FAIL], 'tests/proposals/bulk-memory-operations/elem': [FAIL],
'tests/proposals/bulk-memory-operations/table_copy': [FAIL],
'tests/proposals/bulk-memory-operations/memory_fill': [FAIL], 'tests/proposals/bulk-memory-operations/memory_fill': [FAIL],
'tests/proposals/bulk-memory-operations/bulk': [FAIL], 'tests/proposals/bulk-memory-operations/bulk': [FAIL],
'tests/proposals/bulk-memory-operations/data': [FAIL], 'tests/proposals/bulk-memory-operations/data': [FAIL],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment