[wasm] Fix some bugs in mut global implementation

* If the mutability of the global object doesn't match the module, then
  it should throw a LinkError.
* There was a missing `return` when importing a Number as a mutable
  global.
* All globals were being exported as immutable.
* Attempting to set the value of an immutable global should throw a
  TypeError.
* The length of the setter function should be 1.

Bug: v8:7625
Change-Id: I08d6a428506a18db15eecadf4cbcee89e0658924
Reviewed-on: https://chromium-review.googlesource.com/1031626Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Ben Smith <binji@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52865}

src/wasm/module-compiler.cc

@@ -2359,6 +2359,12 @@ int InstanceBuilder::ProcessImports(Handle<WasmInstanceObject> instance) {
                   module_name, import_name);
               return -1;
             }
+            if (global_object->is_mutable() != global.mutability) {
+              ReportLinkError(
+                  "imported global does not match the expected mutability",
+                  index, module_name, import_name);
+              return -1;
+            }
             if (global.mutability) {
               Handle<JSArrayBuffer> buffer(global_object->array_buffer(),
                                            isolate_);
@@ -2378,6 +2384,7 @@ int InstanceBuilder::ProcessImports(Handle<WasmInstanceObject> instance) {
               ReportLinkError(
                   "imported mutable global must be a WebAssembly.Global object",
                   index, module_name, import_name);
+              return -1;
             }
             WriteGlobalValue(global, value->Number());
           } else {
@@ -2608,14 +2615,13 @@ void InstanceBuilder::ProcessExports(
       case kExternalGlobal: {
         WasmGlobal& global = module_->globals[exp.index];
         if (FLAG_experimental_wasm_mut_global) {
-          const bool is_mutable = false;
           Handle<JSArrayBuffer> globals_buffer(instance->globals_buffer(),
                                                isolate_);
           // Since the global's array buffer is always provided, allocation
           // should never fail.
           Handle<WasmGlobalObject> global_obj =
               WasmGlobalObject::New(isolate_, globals_buffer, global.type,
-                                    global.offset, is_mutable)
+                                    global.offset, global.mutability)
                   .ToHandleChecked();
           desc.set_value(global_obj);
         } else {

src/wasm/wasm-js.cc

@@ -1033,6 +1033,11 @@ void WebAssemblyGlobalSetValue(
   ScheduledErrorThrower thrower(i_isolate, "set WebAssembly.Global.value");
   EXTRACT_THIS(receiver, WasmGlobalObject);
 
+  if (!receiver->is_mutable()) {
+    thrower.TypeError("Can't set the value of an immutable global.");
+    return;
+  }
+
   switch (receiver->type()) {
     case i::wasm::kWasmI32: {
       int32_t i32_value = 0;
@@ -1125,6 +1130,7 @@ void InstallGetterSetter(Isolate* isolate, Handle<JSObject> object,
       CreateFunc(isolate, GetterName(isolate, name), getter);
   Handle<JSFunction> setter_func =
       CreateFunc(isolate, SetterName(isolate, name), setter);
+  setter_func->shared()->set_length(1);
 
   v8::PropertyAttribute attributes =
       static_cast<v8::PropertyAttribute>(v8::DontEnum);

test/mjsunit/wasm/export-mutable-global.js

@@ -21,13 +21,14 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
     let global_builder = builder.addGlobal(type, false).exportAs(name);
     if (value) global_builder.init = value;
   }
-  var module = builder.instantiate();
+  var instance = builder.instantiate();
 
   for (let [type, name, value] of globals) {
-    let obj = module.exports[name];
+    let obj = instance.exports[name];
     assertEquals("object", typeof obj, name);
     assertTrue(obj instanceof WebAssembly.Global, name);
     assertEquals(value || 0, obj.value, name);
+    assertThrows(() => obj.value = 0);
   }
 })();
 
@@ -67,19 +68,19 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
       .addBody([kExprGetLocal, 0, kExprSetGlobal, index])
       .exportFunc();
   }
-  var module = builder.instantiate();
+  var instance = builder.instantiate();
 
   for (let [type, name, value] of globals) {
-    let obj = module.exports[name];
+    let obj = instance.exports[name];
 
     assertEquals(value || 0, obj.value, name);
 
-    // Changing the exported global should change the module's global.
+    // Changing the exported global should change the instance's global.
     obj.value = 1001;
-    assertEquals(1001, module.exports['get ' + name](), name);
+    assertEquals(1001, instance.exports['get ' + name](), name);
 
-    // Changing the module's global should change the exported global.
-    module.exports['set ' + name](112358);
+    // Changing the instance's global should change the exported global.
+    instance.exports['set ' + name](112358);
     assertEquals(112358, obj.value, name);
   }
 })();

test/mjsunit/wasm/import-mutable-global.js

@@ -23,13 +23,30 @@ load("test/mjsunit/wasm/wasm-module-builder.js");
   let global = new WebAssembly.Global({type: 'f32'}, 1);
   let builder = new WasmModuleBuilder();
   builder.addImportedGlobal("mod", "g", kWasmI32);
-  builder.addFunction("main", kSig_i_v)
-    .addBody([kExprGetGlobal, 0])
-    .exportAs("main");
 
   assertThrows(() => builder.instantiate({mod: {g: global}}));
 })();
 
+(function TestMutableMismatch() {
+  let global = new WebAssembly.Global({type: 'f64'}, 1);
+  let builder = new WasmModuleBuilder();
+  builder.addImportedGlobal("mod", "g", kWasmF64, true);
+
+  assertThrows(() => builder.instantiate({mod: {g: global}}));
+
+  global = new WebAssembly.Global({type: 'i32', mutable: true}, 1);
+  builder = new WasmModuleBuilder();
+  builder.addImportedGlobal("mod", "g", kWasmI32);
+
+  assertThrows(() => builder.instantiate({mod: {g: global}}));
+})();
+
+(function TestImportMutableAsNumber() {
+  let builder = new WasmModuleBuilder();
+  builder.addImportedGlobal("mod", "g", kWasmF32, true);
+  assertThrows(() => builder.instantiate({mod: {g: 1234}}));
+})();
+
 (function TestImportI64AsNumber() {
   let builder = new WasmModuleBuilder();
   builder.addImportedGlobal("mod", "g", kWasmI64);

test/mjsunit/wasm/mutable-globals.js

@@ -214,16 +214,24 @@ const f64_values = [
   NaN
 ];
 
-function GlobalI32(value) {
-  return new WebAssembly.Global({type: 'i32'}, value);
+function GlobalI32(value, mutable = false) {
+  return new WebAssembly.Global({type: 'i32', mutable}, value);
 }
 
-function GlobalF32(value) {
-  return new WebAssembly.Global({type: 'f32'}, value);
+function GlobalI64(mutable = false) {
+  let builder = new WasmModuleBuilder();
+  builder.addGlobal(kWasm64, mutable).exportAs('i64');
+  let module = new WebAssembly.Module(builder.toBuffer());
+  let instance = new WebAssembly.Instance(module);
+  return instance.exports.i64;
 }
 
-function GlobalF64(value) {
-  return new WebAssembly.Global({type: 'f64'}, value);
+function GlobalF32(value, mutable = false) {
+  return new WebAssembly.Global({type: 'f32', mutable}, value);
+}
+
+function GlobalF64(value, mutable = false) {
+  return new WebAssembly.Global({type: 'f64', mutable}, value);
 }
 
 (function TestDefaultValue() {
@@ -233,6 +241,8 @@ function GlobalF64(value) {
 })();
 
 (function TestValueOf() {
+  assertTrue(WebAssembly.Global.prototype.valueOf instanceof Function);
+  assertSame(0, WebAssembly.Global.prototype.valueOf.length);
 
   for (let u32_value of u32_values) {
     let i32_value = u32_value | 0;
@@ -241,6 +251,8 @@ function GlobalF64(value) {
     assertSame(i32_value, GlobalI32(i32_value).valueOf());
   }
 
+  assertThrows(() => GlobalI64().valueOf());
+
   for (let f32_value of f32_values) {
     assertSame(Math.fround(f32_value), GlobalF32(f32_value).valueOf());
   }
@@ -248,10 +260,14 @@ function GlobalF64(value) {
   for (let f64_value of f64_values) {
     assertSame(f64_value, GlobalF64(f64_value).valueOf());
   }
-
 })();
 
 (function TestGetValue() {
+  let getter =
+      Object.getOwnPropertyDescriptor(WebAssembly.Global.prototype, 'value')
+          .get;
+  assertTrue(getter instanceof Function);
+  assertSame(0, getter.length);
 
   for (let u32_value of u32_values) {
     let i32_value = u32_value | 0;
@@ -259,6 +275,8 @@ function GlobalF64(value) {
     assertSame(i32_value, GlobalI32(i32_value).value);
   }
 
+  assertThrows(() => GlobalI64().value);
+
   for (let f32_value of f32_values) {
     assertSame(Math.fround(f32_value), GlobalF32(f32_value).value);
   }
@@ -266,15 +284,26 @@ function GlobalF64(value) {
   for (let f64_value of f64_values) {
     assertSame(f64_value, GlobalF64(f64_value).value);
   }
+})();
 
+(function TestSetValueImmutable() {
+  assertThrows(() => GlobalI32().value = 0);
+  assertThrows(() => GlobalI64().value = 0);
+  assertThrows(() => GlobalF32().value = 0);
+  assertThrows(() => GlobalF64().value = 0);
 })();
 
 (function TestSetValue() {
+  let setter =
+      Object.getOwnPropertyDescriptor(WebAssembly.Global.prototype, 'value')
+          .set;
+  assertTrue(setter instanceof Function);
+  assertSame(1, setter.length);
 
   for (let u32_value of u32_values) {
     let i32_value = u32_value | 0;
 
-    let global = GlobalI32();
+    let global = GlobalI32(0, true);
     global.value = u32_value;
     assertSame(i32_value, global.value);
 
@@ -282,16 +311,17 @@ function GlobalF64(value) {
     assertSame(i32_value, global.value);
   }
 
+  assertThrows(() => GlobalI64(true).value = 0);
+
   for (let f32_value of f32_values) {
-    let global = GlobalF32();
+    let global = GlobalF32(0, true);
     global.value = f32_value;
     assertSame(Math.fround(f32_value), global.value);
   }
 
   for (let f64_value of f64_values) {
-    let global = GlobalF64();
+    let global = GlobalF64(0, true);
     global.value = f64_value;
     assertSame(f64_value, global.value);
   }
-
 })();