[wasm] fix js-api Memory and Table constructor

Fix and re-enable tests for WebAssembly's memory/constructor and
table/constructor js-api.

It introduces the '[EnforceRange] unsigned long' algorithm used
to validate initial and maximum properties.

The initial property is now required, by the switch to the Web IDL
specification. Most of the input validations errors are now considered
TypeError instead of RangeError.

The WasmTableObject and WasmMemoryObject APIs use more consistently uint32_t
to ensure integer range and remove the need for bounds checks.

Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Bug: v8:8319
Change-Id: Iedd3ee6484ef688a5e96f93006eb6ca66d805a48
Reviewed-on: https://chromium-review.googlesource.com/c/1354043
Commit-Queue: Adam Klein <adamk@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58138}

src/wasm/wasm-js.cc

@@ -409,6 +409,39 @@ class AsyncInstantiateCompileResultResolver
 
 }  // namespace
 
+// 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) {
+  double double_number;
+
+  if (!v->NumberValue(context).To(&double_number)) {
+    thrower->TypeError("Property '%s' must be convertible to a number",
+                       argument_name->ToCString().get());
+    return false;
+  }
+  if (!std::isfinite(double_number)) {
+    thrower->TypeError("Property '%s' must be convertible to a valid number",
+                       argument_name->ToCString().get());
+    return false;
+  }
+  if (double_number < 0) {
+    thrower->TypeError("Property '%s' must be non-negative",
+                       argument_name->ToCString().get());
+    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());
+    return false;
+  }
+
+  *res = static_cast<uint32_t>(double_number);
+  return true;
+}
+
 // WebAssembly.compile(bytes) -> Promise
 void WebAssemblyCompile(const v8::FunctionCallbackInfo<v8::Value>& args) {
   v8::Isolate* isolate = args.GetIsolate();
@@ -847,30 +880,74 @@ void WebAssemblyInstantiate(const v8::FunctionCallbackInfo<v8::Value>& args) {
 }
 
 bool GetIntegerProperty(v8::Isolate* isolate, ErrorThrower* thrower,
-                        Local<Context> context, Local<v8::Object> object,
-                        Local<String> property, int64_t* result,
+                        Local<Context> context, v8::Local<v8::Value> value,
+                        i::Handle<i::String> property_name, int64_t* result,
                         int64_t lower_bound, uint64_t upper_bound) {
-  v8::MaybeLocal<v8::Value> maybe = object->Get(context, property);
-  v8::Local<v8::Value> value;
-  if (maybe.ToLocal(&value)) {
-    int64_t number;
-    if (!value->IntegerValue(context).To(&number)) return false;
+  uint32_t number;
+  if (!EnforceUint32(property_name, value, context, thrower, &number)) {
+    return false;
+  }
   if (number < lower_bound) {
-      thrower->RangeError("Property value %" PRId64
+    thrower->RangeError("Property '%s': value %" PRIu32
                         " is below the lower bound %" PRIx64,
-                          number, lower_bound);
+                        property_name->ToCString().get(), number, lower_bound);
     return false;
   }
-    if (number > static_cast<int64_t>(upper_bound)) {
-      thrower->RangeError("Property value %" PRId64
+  if (number > upper_bound) {
+    thrower->RangeError("Property '%s': value %" PRIu32
                         " is above the upper bound %" PRIu64,
-                          number, upper_bound);
+                        property_name->ToCString().get(), number, upper_bound);
     return false;
   }
-    *result = static_cast<int>(number);
+
+  *result = static_cast<int64_t>(number);
   return true;
+}
+
+bool GetRequiredIntegerProperty(v8::Isolate* isolate, ErrorThrower* thrower,
+                                Local<Context> context,
+                                Local<v8::Object> object,
+                                Local<String> property, int64_t* result,
+                                int64_t lower_bound, uint64_t upper_bound) {
+  v8::Local<v8::Value> value;
+  if (!object->Get(context, property).ToLocal(&value)) {
+    return false;
+  }
+
+  i::Handle<i::String> property_name = v8::Utils::OpenHandle(*property);
+
+  // Web IDL: dictionary presence
+  // https://heycam.github.io/webidl/#dfn-present
+  if (value->IsUndefined()) {
+    thrower->TypeError("Property '%s' is required",
+                       property_name->ToCString().get());
+    return false;
   }
+
+  return GetIntegerProperty(isolate, thrower, context, value, property_name,
+                            result, lower_bound, upper_bound);
+}
+
+bool GetOptionalIntegerProperty(v8::Isolate* isolate, ErrorThrower* thrower,
+                                Local<Context> context,
+                                Local<v8::Object> object,
+                                Local<String> property, int64_t* result,
+                                int64_t lower_bound, uint64_t upper_bound) {
+  v8::Local<v8::Value> value;
+  if (!object->Get(context, property).ToLocal(&value)) {
     return false;
+  }
+
+  // Web IDL: dictionary presence
+  // https://heycam.github.io/webidl/#dfn-present
+  if (value->IsUndefined()) {
+    return true;
+  }
+
+  i::Handle<i::String> property_name = v8::Utils::OpenHandle(*property);
+
+  return GetIntegerProperty(isolate, thrower, context, value, property_name,
+                            result, lower_bound, upper_bound);
 }
 
 // new WebAssembly.Table(args) -> WebAssembly.Table
@@ -904,27 +981,23 @@ void WebAssemblyTable(const v8::FunctionCallbackInfo<v8::Value>& args) {
   }
   // The descriptor's 'initial'.
   int64_t initial = 0;
-  if (!GetIntegerProperty(isolate, &thrower, context, descriptor,
+  if (!GetRequiredIntegerProperty(isolate, &thrower, context, descriptor,
                                   v8_str(isolate, "initial"), &initial, 0,
                                   i::FLAG_wasm_max_table_size)) {
     return;
   }
   // The descriptor's 'maximum'.
   int64_t maximum = -1;
-  Local<String> maximum_key = v8_str(isolate, "maximum");
-  Maybe<bool> has_maximum = descriptor->Has(context, maximum_key);
-
-  if (!has_maximum.IsNothing() && has_maximum.FromJust()) {
-    if (!GetIntegerProperty(isolate, &thrower, context, descriptor, maximum_key,
-                            &maximum, initial,
+  if (!GetOptionalIntegerProperty(isolate, &thrower, context, descriptor,
+                                  v8_str(isolate, "maximum"), &maximum, initial,
                                   i::wasm::kSpecMaxWasmTableSize)) {
     return;
   }
-  }
 
   i::Handle<i::FixedArray> fixed_array;
-  i::Handle<i::JSObject> table_obj = i::WasmTableObject::New(
-      i_isolate, static_cast<uint32_t>(initial), maximum, &fixed_array);
+  i::Handle<i::JSObject> table_obj =
+      i::WasmTableObject::New(i_isolate, static_cast<uint32_t>(initial),
+                              static_cast<uint32_t>(maximum), &fixed_array);
   v8::ReturnValue<v8::Value> return_value = args.GetReturnValue();
   return_value.Set(Utils::ToLocal(table_obj));
 }
@@ -946,23 +1019,18 @@ void WebAssemblyMemory(const v8::FunctionCallbackInfo<v8::Value>& args) {
   Local<v8::Object> descriptor = Local<Object>::Cast(args[0]);
   // The descriptor's 'initial'.
   int64_t initial = 0;
-  if (!GetIntegerProperty(isolate, &thrower, context, descriptor,
+  if (!GetRequiredIntegerProperty(isolate, &thrower, context, descriptor,
                                   v8_str(isolate, "initial"), &initial, 0,
                                   i::wasm::max_mem_pages())) {
     return;
   }
   // The descriptor's 'maximum'.
   int64_t maximum = -1;
-  Local<String> maximum_key = v8_str(isolate, "maximum");
-  Maybe<bool> has_maximum = descriptor->Has(context, maximum_key);
-
-  if (!has_maximum.IsNothing() && has_maximum.FromJust()) {
-    if (!GetIntegerProperty(isolate, &thrower, context, descriptor, maximum_key,
-                            &maximum, initial,
+  if (!GetOptionalIntegerProperty(isolate, &thrower, context, descriptor,
+                                  v8_str(isolate, "maximum"), &maximum, initial,
                                   i::wasm::kSpecMaxWasmMemoryPages)) {
     return;
   }
-  }
 
   bool is_shared_memory = false;
   auto enabled_features = i::wasm::WasmFeaturesFromIsolate(i_isolate);
@@ -1003,7 +1071,7 @@ void WebAssemblyMemory(const v8::FunctionCallbackInfo<v8::Value>& args) {
     }
   }
   i::Handle<i::JSObject> memory_obj = i::WasmMemoryObject::New(
-      i_isolate, buffer, static_cast<int32_t>(maximum));
+      i_isolate, buffer, static_cast<uint32_t>(maximum));
   args.GetReturnValue().Set(Utils::ToLocal(memory_obj));
 }
 

src/wasm/wasm-objects.cc

@@ -748,7 +748,7 @@ bool WasmModuleObject::GetPositionInfo(uint32_t position,
 }
 
 Handle<WasmTableObject> WasmTableObject::New(Isolate* isolate, uint32_t initial,
-                                             int64_t maximum,
+                                             uint32_t maximum,
                                              Handle<FixedArray>* js_functions) {
   Handle<JSFunction> table_ctor(
       isolate->native_context()->wasm_table_constructor(), isolate);
@@ -761,8 +761,7 @@ Handle<WasmTableObject> WasmTableObject::New(Isolate* isolate, uint32_t initial,
     (*js_functions)->set(i, null);
   }
   table_obj->set_functions(**js_functions);
-  DCHECK_EQ(maximum, static_cast<int>(maximum));
-  Handle<Object> max = isolate->factory()->NewNumber(maximum);
+  Handle<Object> max = isolate->factory()->NewNumberFromUint(maximum);
   table_obj->set_maximum_length(*max);
 
   table_obj->set_dispatch_tables(ReadOnlyRoots(isolate).empty_fixed_array());
@@ -964,7 +963,7 @@ void SetInstanceMemory(Handle<WasmInstanceObject> instance,
 
 Handle<WasmMemoryObject> WasmMemoryObject::New(
     Isolate* isolate, MaybeHandle<JSArrayBuffer> maybe_buffer,
-    int32_t maximum) {
+    uint32_t maximum) {
   // TODO(kschimpf): Do we need to add an argument that defines the
   // style of memory the user prefers (with/without trap handling), so
   // that the memory will match the style of the compiled wasm module.

src/wasm/wasm-objects.h

@@ -271,7 +271,7 @@ class WasmTableObject : public JSObject {
   void Grow(Isolate* isolate, uint32_t count);
 
   static Handle<WasmTableObject> New(Isolate* isolate, uint32_t initial,
-                                     int64_t maximum,
+                                     uint32_t maximum,
                                      Handle<FixedArray>* js_functions);
   static void AddDispatchTable(Isolate* isolate, Handle<WasmTableObject> table,
                                Handle<WasmInstanceObject> instance,
@@ -322,7 +322,7 @@ class WasmMemoryObject : public JSObject {
   bool has_full_guard_region(Isolate* isolate);
 
   V8_EXPORT_PRIVATE static Handle<WasmMemoryObject> New(
-      Isolate* isolate, MaybeHandle<JSArrayBuffer> buffer, int32_t maximum);
+      Isolate* isolate, MaybeHandle<JSArrayBuffer> buffer, uint32_t maximum);
 
   static int32_t Grow(Isolate*, Handle<WasmMemoryObject>, uint32_t pages);
 

test/mjsunit/regress/wasm/regress-680938.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-var v17 = {};
-var v32 = {};
+var v17 = 42;
+var v32 = { initial: 1 };
 v39 = new WebAssembly.Memory(v32);
 v49 = v39.grow(v17);

test/mjsunit/regress/wasm/regress-736584.js

@@ -7,7 +7,7 @@
 load('test/mjsunit/wasm/wasm-constants.js');
 load('test/mjsunit/wasm/wasm-module-builder.js');
 
-let mem = new WebAssembly.Memory({});
+let mem = new WebAssembly.Memory({initial: 0});
 let builder = new WasmModuleBuilder();
 builder.addImportedMemory("mod", "imported_mem");
 builder.addFunction('mem_size', kSig_i_v)

test/mjsunit/regress/wasm/regress-816226.js

@@ -2,4 +2,4 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-(new Int8Array((new WebAssembly.Memory({})).buffer)).buffer;
+(new Int8Array((new WebAssembly.Memory({initial: 0})).buffer)).buffer;

test/mjsunit/wasm/js-api.js

@@ -435,9 +435,9 @@ assertErrorMessage(
     () => new Memory({initial: {valueOf() { throw new Error('here') }}}), Error,
     'here');
 assertErrorMessage(
-    () => new Memory({initial: -1}), RangeError, /bad Memory initial size/);
+    () => new Memory({initial: -1}), TypeError, /bad Memory initial size/);
 assertErrorMessage(
-    () => new Memory({initial: Math.pow(2, 32)}), RangeError,
+    () => new Memory({initial: Math.pow(2, 32)}), TypeError,
     /bad Memory initial size/);
 assertErrorMessage(
     () => new Memory({initial: 1, maximum: Math.pow(2, 32) / Math.pow(2, 14)}),
@@ -446,7 +446,7 @@ assertErrorMessage(
     () => new Memory({initial: 2, maximum: 1}), RangeError,
     /bad Memory maximum size/);
 assertErrorMessage(
-    () => new Memory({maximum: -1}), RangeError, /bad Memory maximum size/);
+    () => new Memory({maximum: -1}), TypeError, /bad Memory maximum size/);
 assertTrue(new Memory({initial: 1}) instanceof Memory);
 assertEq(new Memory({initial: 1.5}).buffer.byteLength, kPageSize);
 
@@ -583,17 +583,17 @@ assertErrorMessage(
         {initial: {valueOf() { throw new Error('here') }}, element: 'anyfunc'}),
     Error, 'here');
 assertErrorMessage(
-    () => new Table({initial: -1, element: 'anyfunc'}), RangeError,
+    () => new Table({initial: -1, element: 'anyfunc'}), TypeError,
     /bad Table initial size/);
 assertErrorMessage(
-    () => new Table({initial: Math.pow(2, 32), element: 'anyfunc'}), RangeError,
+    () => new Table({initial: Math.pow(2, 32), element: 'anyfunc'}), TypeError,
     /bad Table initial size/);
 assertErrorMessage(
     () => new Table({initial: 2, maximum: 1, element: 'anyfunc'}), RangeError,
     /bad Table maximum size/);
 assertErrorMessage(
     () => new Table({initial: 2, maximum: Math.pow(2, 32), element: 'anyfunc'}),
-    RangeError, /bad Table maximum size/);
+    TypeError, /bad Table maximum size/);
 assertTrue(new Table({initial: 1, element: 'anyfunc'}) instanceof Table);
 assertTrue(new Table({initial: 1.5, element: 'anyfunc'}) instanceof Table);
 assertTrue(
@@ -824,7 +824,7 @@ function assertInstantiateError(args, err, msg) {
     // TODO assertTrue(Boolean(error.message.match(msg)));
   });
 }
-var scratch_memory = new WebAssembly.Memory(new ArrayBuffer(10));
+var scratch_memory = new WebAssembly.Memory({ initial: 0 });
 assertInstantiateError([], TypeError, /requires more than 0 arguments/);
 assertInstantiateError(
     [undefined], TypeError, /first argument must be a BufferSource/);

test/mjsunit/wasm/memory.js

@@ -29,11 +29,11 @@ function assertMemoryIsValid(memory) {
   assertThrows(() => new WebAssembly.Memory(1), TypeError);
   assertThrows(() => new WebAssembly.Memory(""), TypeError);
 
-  assertThrows(() => new WebAssembly.Memory({initial: -1}), RangeError);
-  assertThrows(() => new WebAssembly.Memory({initial: outOfUint32RangeValue}), RangeError);
+  assertThrows(() => new WebAssembly.Memory({initial: -1}), TypeError);
+  assertThrows(() => new WebAssembly.Memory({initial: outOfUint32RangeValue}), TypeError);
 
-  assertThrows(() => new WebAssembly.Memory({initial: 10, maximum: -1}), RangeError);
-  assertThrows(() => new WebAssembly.Memory({initial: 10, maximum: outOfUint32RangeValue}), RangeError);
+  assertThrows(() => new WebAssembly.Memory({initial: 10, maximum: -1}), TypeError);
+  assertThrows(() => new WebAssembly.Memory({initial: 10, maximum: outOfUint32RangeValue}), TypeError);
   assertThrows(() => new WebAssembly.Memory({initial: 10, maximum: 9}), RangeError);
 
   let memory = new WebAssembly.Memory({initial: 1});
@@ -45,12 +45,6 @@ function assertMemoryIsValid(memory) {
   assertMemoryIsValid(memory);
 })();
 
-(function TestInitialIsUndefined() {
-  // New memory with initial = undefined, which means initial = 0.
-  let memory = new WebAssembly.Memory({initial: undefined});
-  assertMemoryIsValid(memory);
-})();
-
 (function TestMaximumIsUndefined() {
   // New memory with maximum = undefined, which means maximum = 0.
   let memory = new WebAssembly.Memory({initial: 0, maximum: undefined});
@@ -74,7 +68,7 @@ function assertMemoryIsValid(memory) {
   assertMemoryIsValid(memory);
 })();
 
-(function TestMaximumDoesHasProperty() {
+(function TestMaximumDoesNotHasProperty() {
   var hasPropertyWasCalled = false;
   var desc = {initial: 10};
   var proxy = new Proxy({maximum: 16}, {
@@ -83,7 +77,7 @@ function assertMemoryIsValid(memory) {
   Object.setPrototypeOf(desc, proxy);
   let memory = new WebAssembly.Memory(desc);
   assertMemoryIsValid(memory);
-  assertTrue(hasPropertyWasCalled);
+  assertFalse(hasPropertyWasCalled);
 })();
 
 (function TestBuffer() {

test/mjsunit/wasm/table.js

@@ -46,14 +46,14 @@ function assertTableIsValid(table, length) {
   assertThrows(() => new WebAssembly.Table({element: "any", initial: 10}), TypeError);
 
   assertThrows(() => new WebAssembly.Table(
-    {element: "anyfunc", initial: -1}), RangeError);
+    {element: "anyfunc", initial: -1}), TypeError);
   assertThrows(() => new WebAssembly.Table(
-    {element: "anyfunc", initial: outOfUint32RangeValue}), RangeError);
+    {element: "anyfunc", initial: outOfUint32RangeValue}), TypeError);
 
   assertThrows(() => new WebAssembly.Table(
-    {element: "anyfunc", initial: 10, maximum: -1}), RangeError);
+    {element: "anyfunc", initial: 10, maximum: -1}), TypeError);
   assertThrows(() => new WebAssembly.Table(
-    {element: "anyfunc", initial: 10, maximum: outOfUint32RangeValue}), RangeError);
+    {element: "anyfunc", initial: 10, maximum: outOfUint32RangeValue}), TypeError);
   assertThrows(() => new WebAssembly.Table(
     {element: "anyfunc", initial: 10, maximum: 9}), RangeError);
 
@@ -75,31 +75,25 @@ function assertTableIsValid(table, length) {
   assertEquals(null, table.get(0));
   assertEquals(undefined, table[0]);
 
-  table = new WebAssembly.Table({element: "anyfunc", initial: undefined});
+  table = new WebAssembly.Table({element: "anyfunc", initial: 0, maximum: 10});
   assertTableIsValid(table, 0);
 
-  table = new WebAssembly.Table({element: "anyfunc"});
+  table = new WebAssembly.Table({element: "anyfunc", initial: 0,  maximum: "10"});
   assertTableIsValid(table, 0);
 
-  table = new WebAssembly.Table({element: "anyfunc", maximum: 10});
-  assertTableIsValid(table, 0);
-
-  table = new WebAssembly.Table({element: "anyfunc", maximum: "10"});
-  assertTableIsValid(table, 0);
-
-  table = new WebAssembly.Table({element: "anyfunc", maximum: {valueOf() { return "10" }}});
+  table = new WebAssembly.Table({element: "anyfunc", initial: 0, maximum: {valueOf() { return "10" }}});
   assertTableIsValid(table, 0);
 
   table = new WebAssembly.Table({element: "anyfunc", initial: 0, maximum: undefined});
   assertTableIsValid(table, 0);
 
-  table = new WebAssembly.Table({element: "anyfunc", maximum: kMaxUint31});
+  table = new WebAssembly.Table({element: "anyfunc", initial: 0, maximum: kMaxUint31});
   assertTableIsValid(table, 0);
 
-  table = new WebAssembly.Table({element: "anyfunc", maximum: kMaxUint32});
+  table = new WebAssembly.Table({element: "anyfunc", initial: 0, maximum: kMaxUint32});
   assertTableIsValid(table, 0);
 
-  table = new WebAssembly.Table({element: "anyfunc", maximum: kV8MaxWasmTableSize + 1});
+  table = new WebAssembly.Table({element: "anyfunc", initial: 0, maximum: kV8MaxWasmTableSize + 1});
   assertTableIsValid(table, 0);
 })();
 

test/wasm-js/wasm-js.status

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