[wasm] 0-count is out-of-bounds for table.*

The spec wasn't clear (or I misunderstood). As per
(https://github.com/WebAssembly/bulk-memory-operations/issues/11),
zero-count table operations are also out of bounds.

R=mstarzinger@chromium.org
CC=binji@chromium.org
BUG=v8:7747

Change-Id: Iac689b93a040eb6eb06975bc2ba0facb85d24756
Reviewed-on: https://chromium-review.googlesource.com/c/1436022
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59121}
This commit is contained in:
Ben L. Titzer 2019-01-28 11:46:03 +01:00 committed by Commit Bot
parent a1efb4134e
commit 3a638a57cf
4 changed files with 13 additions and 13 deletions

View File

@ -1461,7 +1461,6 @@ bool LoadElemSegmentImpl(Isolate* isolate, Handle<WasmInstanceObject> instance,
JSToWasmWrapperCache* js_to_wasm_cache, JSToWasmWrapperCache* js_to_wasm_cache,
const WasmElemSegment& elem_segment, uint32_t dst, const WasmElemSegment& elem_segment, uint32_t dst,
uint32_t src, size_t count) { uint32_t src, size_t count) {
if (count == 0) return true; // nothing to do.
if (!IsInBounds(dst, count, table_instance.table_size)) return false; if (!IsInBounds(dst, count, table_instance.table_size)) return false;
if (!IsInBounds(src, count, elem_segment.entries.size())) return false; if (!IsInBounds(src, count, elem_segment.entries.size())) return false;

View File

@ -1439,7 +1439,6 @@ bool WasmInstanceObject::CopyTableEntries(Isolate* isolate,
uint32_t table_index, uint32_t dst, uint32_t table_index, uint32_t dst,
uint32_t src, uint32_t count) { uint32_t src, uint32_t count) {
CHECK_EQ(0, table_index); // TODO(titzer): multiple tables in TableCopy CHECK_EQ(0, table_index); // TODO(titzer): multiple tables in TableCopy
if (count == 0) return true; // no-op
auto max = instance->indirect_function_table_size(); auto max = instance->indirect_function_table_size();
if (!IsInBounds(dst, count, max)) return false; if (!IsInBounds(dst, count, max)) return false;
if (!IsInBounds(src, count, max)) return false; if (!IsInBounds(src, count, max)) return false;

View File

@ -30,9 +30,6 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
copy(0, i, kTableSize - i); copy(0, i, kTableSize - i);
copy(i, 0, kTableSize - i); copy(i, 0, kTableSize - i);
} }
let big = 1000000;
copy(big, 0, 0); // nop
copy(0, big, 0); // nop
})(); })();
function addFunction(builder, k) { function addFunction(builder, k) {
@ -176,6 +173,13 @@ function assertCall(call, ...elems) {
assertThrows(() => copy(1, 0, kTableSize)); assertThrows(() => copy(1, 0, kTableSize));
assertThrows(() => copy(0, 1, kTableSize)); assertThrows(() => copy(0, 1, kTableSize));
{
let big = 1000000;
assertThrows(() => copy(big, 0, 0));
assertThrows(() => copy(0, big, 0));
}
for (let big = 4294967295; big > 1000; big >>>= 1) { for (let big = 4294967295; big > 1000; big >>>= 1) {
assertThrows(() => copy(big, 0, 1)); assertThrows(() => copy(big, 0, 1));
assertThrows(() => copy(0, big, 1)); assertThrows(() => copy(0, big, 1));
@ -187,6 +191,7 @@ function assertCall(call, ...elems) {
assertThrows(() => copy(0, big, 1)); assertThrows(() => copy(0, big, 1));
assertThrows(() => copy(0, 0, big)); assertThrows(() => copy(0, 0, big));
} }
})(); })();
(function TestTableCopyShared() { (function TestTableCopyShared() {

View File

@ -57,14 +57,6 @@ function assertTable(obj, ...elems) {
assertTable(x.table, null, null, null, null, null); assertTable(x.table, null, null, null, null, null);
// 0-count is not oob.
x.init0(0, 0, 0);
assertTable(x.table, null, null, null, null, null);
x.init0(kTableSize+1, 0, 0);
assertTable(x.table, null, null, null, null, null);
x.init0(0, kTableSize+1, 0);
assertTable(x.table, null, null, null, null, null);
// test actual writes. // test actual writes.
x.init0(0, 0, 1); x.init0(0, 0, 1);
assertTable(x.table, x.f0, null, null, null, null); assertTable(x.table, x.f0, null, null, null, null);
@ -109,6 +101,11 @@ function assertTable(obj, ...elems) {
let x = instance.exports; let x = instance.exports;
assertTable(x.table, null, null, null, null, null); assertTable(x.table, null, null, null, null, null);
// 0-count is oob.
assertThrows(() => x.init0(kTableSize+1, 0, 0));
assertThrows(() => x.init0(0, kTableSize+1, 0));
assertThrows(() => x.init0(0, 0, 6)); assertThrows(() => x.init0(0, 0, 6));
assertThrows(() => x.init0(0, 1, 5)); assertThrows(() => x.init0(0, 1, 5));
assertThrows(() => x.init0(0, 2, 4)); assertThrows(() => x.init0(0, 2, 4));