[wasm] fix js-api table/grow

Fix WebAssembly's table/grow js-api. The argument is a unsigned long,
this change refactors most of arithmetic and bounds checks type from
int64 to uint32_t, according to the spec.

Bug: v8:8319
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Change-Id: Ia29121c930d7fb930668e54a5a769dae25234f2c
Reviewed-on: https://chromium-review.googlesource.com/c/1351006
Commit-Queue: Sven Sauleau <ssauleau@igalia.com>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58936}

src/wasm/wasm-js.cc

@@ -4,6 +4,8 @@
 
 #include "src/wasm/wasm-js.h"
 
+#include <string>
+
 #include "src/api-inl.h"
 #include "src/api-natives.h"
 #include "src/assert-scope.h"
@@ -438,40 +440,46 @@ class AsyncInstantiateCompileResultResolver
 
 constexpr char AsyncInstantiateCompileResultResolver::kGlobalPromiseHandle[];
 constexpr char AsyncInstantiateCompileResultResolver::kGlobalImportsHandle[];
-}  // namespace
+
+std::string ToString(const char* name) { return std::string(name); }
+
+std::string ToString(const i::Handle<i::String> name) {
+  return std::string("Property '") + name->ToCString().get() + "'";
+}
 
 // Web IDL: '[EnforceRange] unsigned long'
 // Previously called ToNonWrappingUint32 in the draft WebAssembly JS spec.
 // https://heycam.github.io/webidl/#EnforceRange
-bool EnforceUint32(i::Handle<i::String> argument_name, Local<v8::Value> v,
-                   Local<Context> context, ErrorThrower* thrower,
-                   uint32_t* res) {
+template <typename T>
+bool EnforceUint32(T argument_name, Local<v8::Value> v, Local<Context> context,
+                   ErrorThrower* thrower, uint32_t* res) {
   double double_number;
 
   if (!v->NumberValue(context).To(&double_number)) {
-    thrower->TypeError("Property '%s' must be convertible to a number",
-                       argument_name->ToCString().get());
+    thrower->TypeError("%s must be convertible to a number",
+                       ToString(argument_name).c_str());
     return false;
   }
   if (!std::isfinite(double_number)) {
-    thrower->TypeError("Property '%s' must be convertible to a valid number",
-                       argument_name->ToCString().get());
+    thrower->TypeError("%s must be convertible to a valid number",
+                       ToString(argument_name).c_str());
     return false;
   }
   if (double_number < 0) {
-    thrower->TypeError("Property '%s' must be non-negative",
-                       argument_name->ToCString().get());
+    thrower->TypeError("%s must be non-negative",
+                       ToString(argument_name).c_str());
     return false;
   }
   if (double_number > std::numeric_limits<uint32_t>::max()) {
-    thrower->TypeError("Property '%s' must be in the unsigned long range",
-                       argument_name->ToCString().get());
+    thrower->TypeError("%s must be in the unsigned long range",
+                       ToString(argument_name).c_str());
     return false;
   }
 
   *res = static_cast<uint32_t>(double_number);
   return true;
 }
+}  // namespace
 
 // WebAssembly.compile(bytes) -> Promise
 void WebAssemblyCompile(const v8::FunctionCallbackInfo<v8::Value>& args) {
@@ -1301,31 +1309,39 @@ void WebAssemblyTableGrow(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Local<Context> context = isolate->GetCurrentContext();
   EXTRACT_THIS(receiver, WasmTableObject);
 
-  int64_t grow_by = 0;
-  if (!args[0]->IntegerValue(context).To(&grow_by)) return;
+  uint32_t grow_by;
+  if (!EnforceUint32("Argument 0", args[0], context, &thrower, &grow_by)) {
+    return;
+  }
+
   i::Handle<i::FixedArray> old_array(receiver->functions(), i_isolate);
-  int old_size = old_array->length();
+  uint32_t old_size = static_cast<uint32_t>(old_array->length());
 
-  int64_t max_size64 = receiver->maximum_length()->Number();
-  if (max_size64 < 0 || max_size64 > i::FLAG_wasm_max_table_size) {
+  uint64_t max_size64 = receiver->maximum_length()->Number();
+  if (max_size64 > i::FLAG_wasm_max_table_size) {
     max_size64 = i::FLAG_wasm_max_table_size;
   }
 
-  if (grow_by < 0 || grow_by > max_size64 - old_size) {
-    thrower.RangeError(grow_by < 0 ? "trying to shrink table"
-                                   : "maximum table size exceeded");
+  DCHECK_LE(max_size64, std::numeric_limits<uint32_t>::max());
+
+  uint64_t new_size64 =
+      static_cast<uint64_t>(old_size) + static_cast<uint64_t>(grow_by);
+  if (new_size64 > max_size64) {
+    thrower.RangeError("maximum table size exceeded");
     return;
   }
-
-  int new_size = static_cast<int>(old_size + grow_by);
-  receiver->Grow(i_isolate, static_cast<uint32_t>(new_size - old_size));
+  uint32_t new_size = static_cast<uint32_t>(new_size64);
 
   if (new_size != old_size) {
+    receiver->Grow(i_isolate, new_size - old_size);
+
     i::Handle<i::FixedArray> new_array =
         i_isolate->factory()->NewFixedArray(new_size);
-    for (int i = 0; i < old_size; ++i) new_array->set(i, old_array->get(i));
+    for (uint32_t i = 0; i < old_size; ++i) {
+      new_array->set(i, old_array->get(i));
+    }
     i::Object null = i::ReadOnlyRoots(i_isolate).null_value();
-    for (int i = old_size; i < new_size; ++i) new_array->set(i, null);
+    for (uint32_t i = old_size; i < new_size; ++i) new_array->set(i, null);
     receiver->set_functions(*new_array);
   }
 
@@ -1343,15 +1359,19 @@ void WebAssemblyTableGet(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Local<Context> context = isolate->GetCurrentContext();
   EXTRACT_THIS(receiver, WasmTableObject);
   i::Handle<i::FixedArray> array(receiver->functions(), i_isolate);
-  int64_t i = 0;
-  if (!args[0]->IntegerValue(context).To(&i)) return;
+
+  uint32_t index;
+  if (!EnforceUint32("Argument 0", args[0], context, &thrower, &index)) {
+    return;
+  }
+
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
-  if (i < 0 || i >= array->length()) {
-    thrower.RangeError("index out of bounds");
+  if (index >= static_cast<uint32_t>(array->length())) {
+    thrower.RangeError("Index out of bounds");
     return;
   }
 
-  i::Handle<i::Object> value(array->get(static_cast<int>(i)), i_isolate);
+  i::Handle<i::Object> value(array->get(static_cast<int>(index)), i_isolate);
   return_value.Set(Utils::ToLocal(value));
 }
 

test/mjsunit/regress/regress-crbug-772056.js

@@ -14,4 +14,4 @@ let table = new WebAssembly.Table({element: "anyfunc",
   initial: 1, maximum:1000000});
 let instance = new WebAssembly.Instance(module, {x: {table:table}});
 
-assertThrows(() => table.grow(Infinity), RangeError);
+assertThrows(() => table.grow(Infinity), TypeError);

test/mjsunit/wasm/js-api.js

@@ -651,7 +651,8 @@ assertErrorMessage(
     () => get.call(), TypeError, /called on incompatible undefined/);
 assertErrorMessage(
     () => get.call({}), TypeError, /called on incompatible Object/);
-assertEq(get.call(tbl1), null);
+assertErrorMessage(
+    () => get.call(tbl1), TypeError, /Argument 0 must be convertible to a valid number/);
 assertEq(get.call(tbl1, 0), null);
 assertEq(get.call(tbl1, 0, Infinity), null);
 assertEq(get.call(tbl1, 1), null);
@@ -659,9 +660,9 @@ assertEq(get.call(tbl1, 1.5), null);
 assertErrorMessage(() => get.call(tbl1, 2), RangeError, /bad Table get index/);
 assertErrorMessage(
     () => get.call(tbl1, 2.5), RangeError, /bad Table get index/);
-assertErrorMessage(() => get.call(tbl1, -1), RangeError, /bad Table get index/);
+assertErrorMessage(() => get.call(tbl1, -1), TypeError, /bad Table get index/);
 assertErrorMessage(
-    () => get.call(tbl1, Math.pow(2, 33)), RangeError, /bad Table get index/);
+    () => get.call(tbl1, Math.pow(2, 33)), TypeError, /bad Table get index/);
 assertErrorMessage(
     () => get.call(tbl1, {valueOf() { throw new Error('hi') }}), Error, 'hi');
 
@@ -729,27 +730,26 @@ assertErrorMessage(
 assertErrorMessage(
     () => tblGrow.call({}), TypeError, /called on incompatible Object/);
 assertErrorMessage(
-    () => tblGrow.call(tbl1, -1), RangeError, /bad Table grow delta/);
+    () => tblGrow.call(tbl1, -1), TypeError, /bad Table grow delta/);
 assertErrorMessage(
-    () => tblGrow.call(tbl1, Math.pow(2, 32)), RangeError,
+    () => tblGrow.call(tbl1, Math.pow(2, 32)), TypeError,
     /bad Table grow delta/);
 var tbl = new Table({element: 'anyfunc', initial: 1, maximum: 2});
 assertEq(tbl.length, 1);
 assertErrorMessage(
-    () => tbl.grow(Infinity), RangeError, /failed to grow table/);
+    () => tbl.grow(Infinity), TypeError, /failed to grow table/);
 assertErrorMessage(
-    () => tbl.grow(-Infinity), RangeError, /failed to grow table/);
+    () => tbl.grow(-Infinity), TypeError, /failed to grow table/);
 assertEq(tbl.grow(0), 1);
 assertEq(tbl.length, 1);
 assertEq(tbl.grow(1, 4), 1);
 assertEq(tbl.length, 2);
-assertEq(tbl.grow(), 2);
 assertEq(tbl.length, 2);
 assertErrorMessage(() => tbl.grow(1), Error, /failed to grow table/);
 assertErrorMessage(
-    () => tbl.grow(Infinity), RangeError, /failed to grow table/);
+    () => tbl.grow(Infinity), TypeError, /failed to grow table/);
 assertErrorMessage(
-    () => tbl.grow(-Infinity), RangeError, /failed to grow table/);
+    () => tbl.grow(-Infinity), TypeError, /failed to grow table/);
 
 // 'WebAssembly.validate' function
 assertErrorMessage(() => WebAssembly.validate(), TypeError);

test/mjsunit/wasm/table.js

@@ -141,10 +141,13 @@ function assertTableIsValid(table, length) {
     assertEquals(null, table.get(i));
     assertEquals(null, table.get(String(i)));
   }
-  for (let key of [0.4, "", NaN, {}, [], () => {}]) {
+  for (let key of [0.4, "", []]) {
     assertEquals(null, table.get(key));
   }
-  for (let key of [-1, table.length, table.length * 10]) {
+  for (let key of [-1, NaN, {}, () => {}]) {
+    assertThrows(() => table.get(key), TypeError);
+  }
+  for (let key of [table.length, table.length * 10]) {
     assertThrows(() => table.get(key), RangeError);
   }
   assertThrows(() => table.get(Symbol()), TypeError);
@@ -214,7 +217,12 @@ function assertTableIsValid(table, length) {
       assertSame(f, table[i]);
     }
 
-    for (let key of [0.4, "", NaN, {}, [], () => {}]) {
+    for (let key of [NaN, {}, () => {}]) {
+      assertSame(f, table[key] = f);
+      assertSame(f, table[key]);
+      assertThrows(() => table.get(key), TypeError);
+    }
+    for (let key of [0.4, "", []]) {
       assertSame(f, table[key] = f);
       assertSame(f, table[key]);
       assertSame(null, table.get(key));
@@ -246,7 +254,7 @@ function assertTableIsValid(table, length) {
   check(table);
   table.grow(10);
   check(table);
-  assertThrows(() => table.grow(-10), RangeError);
+  assertThrows(() => table.grow(-10), TypeError);
 
   table = new WebAssembly.Table({element: "anyfunc", initial: 20, maximum: 25});
   init(table);
@@ -258,7 +266,7 @@ function assertTableIsValid(table, length) {
   table.grow(0);
   check(table);
   assertThrows(() => table.grow(1), RangeError);
-  assertThrows(() => table.grow(-10), RangeError);
+  assertThrows(() => table.grow(-10), TypeError);
 
   assertThrows(() => WebAssembly.Table.prototype.grow.call([], 0), TypeError);
 

test/wasm-js/wasm-js.status

@@ -6,7 +6,6 @@
 [ALWAYS, {
   # https://bugs.chromium.org/p/v8/issues/detail?id=8319
   'memory/grow': [FAIL],
-  'table/grow': [FAIL],
   'table/get-set': [FAIL],
   'module/customSections': [FAIL],
 }], # ALWAYS