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

The spec wasn't clear (or I misunderstood). As per (WebAssembly/bulk-memory-operations#11), zero-count table operations are also out of bounds. [email protected] [email protected] BUG=v8:7747 Change-Id: Iac689b93a040eb6eb06975bc2ba0facb85d24756 Reviewed-on: https://chromium-review.googlesource.com/c/1436022 Reviewed-by: Michael Starzinger <[email protected]> Commit-Queue: Ben Titzer <[email protected]> Cr-Commit-Position: refs/heads/master@{#59121}
brn · Jan 28, 2019 · 3a638a5 · 3a638a5
1 parent a1efb41
commit 3a638a5
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 13 deletions.
diff --git a/src/wasm/module-instantiate.cc b/src/wasm/module-instantiate.cc
@@ -1461,7 +1461,6 @@ bool LoadElemSegmentImpl(Isolate* isolate, Handle<WasmInstanceObject> instance,
                          JSToWasmWrapperCache* js_to_wasm_cache,
                          const WasmElemSegment& elem_segment, uint32_t dst,
                          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(src, count, elem_segment.entries.size())) return false;
 

diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc
@@ -1439,7 +1439,6 @@ bool WasmInstanceObject::CopyTableEntries(Isolate* isolate,
                                           uint32_t table_index, uint32_t dst,
                                           uint32_t src, uint32_t count) {
   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();
   if (!IsInBounds(dst, count, max)) return false;
   if (!IsInBounds(src, count, max)) return false;

diff --git a/test/mjsunit/wasm/table-copy.js b/test/mjsunit/wasm/table-copy.js
@@ -30,9 +30,6 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
     copy(0, i, kTableSize - i);
     copy(i, 0, kTableSize - i);
   }
-  let big = 1000000;
-  copy(big, 0, 0); // nop
-  copy(0, big, 0); // nop
 })();
 
 function addFunction(builder, k) {
@@ -176,6 +173,13 @@ function assertCall(call, ...elems) {
   assertThrows(() => copy(1, 0, 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) {
     assertThrows(() => copy(big, 0, 1));
     assertThrows(() => copy(0, big, 1));
@@ -187,6 +191,7 @@ function assertCall(call, ...elems) {
     assertThrows(() => copy(0, big, 1));
     assertThrows(() => copy(0, 0, big));
   }
+
 })();
 
 (function TestTableCopyShared() {

diff --git a/test/mjsunit/wasm/table-init.js b/test/mjsunit/wasm/table-init.js
@@ -57,14 +57,6 @@ function assertTable(obj, ...elems) {
 
   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.
   x.init0(0, 0, 1);
   assertTable(x.table, x.f0, null, null, null, null);
@@ -109,6 +101,11 @@ function assertTable(obj, ...elems) {
   let x = instance.exports;
 
   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, 1, 5));
   assertThrows(() => x.init0(0, 2, 4));