Bug 1736544 - Guard isMem32 when we don't yet know if there's a memory. r=jseward
authorLars T Hansen <lhansen@mozilla.com>
Wed, 20 Oct 2021 06:40:20 +0000
changeset 596441 3f44570721461fa1812ff57a62d066d0b90ca0e1
parent 596440 0665a84c7045a8057c594ee400c950e3613ee093
child 596442 320b7d0b5790d05f5994336b247d88015433be2a
push id38898
push userimoraru@mozilla.com
push dateThu, 21 Oct 2021 03:32:00 +0000
treeherdermozilla-central@811f11b91f11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjseward
bugs1736544
milestone95.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1736544 - Guard isMem32 when we don't yet know if there's a memory. r=jseward The isMem32 predicate assumes that there's a memory present (and this is the right thing), but sometimes we must call isMem32 before we've checked for the presence of a memory, this is a consequence of the structure of the compiler. Specifically, we must determine the appropriate signature for a bulk memory instance call before reading the opcode, and the signature depends on the memory type, yet it's the opcode reading that checks for the presence of the memory and throws if one is not present. Introduce a new predicate for these situations and use it as appropriate. It is safe to assume mem32 if a memory is not present, as the presence of a memory will be checked properly subsequently. The isMem64 predicate is not affected as it is not used in these situations. Only Ion is affected, baseline already has the necessary guard in all the required situations. The missing test cases were an oversight - I introduced tests like these for the memory instructions previously, but I forgot wait and notify. Differential Revision: https://phabricator.services.mozilla.com/D128869
js/src/jit-test/tests/wasm/atomic.js
js/src/wasm/WasmIonCompile.cpp
--- a/js/src/jit-test/tests/wasm/atomic.js
+++ b/js/src/jit-test/tests/wasm/atomic.js
@@ -529,8 +529,34 @@ assertErrorMessage(() => wasmEvalText(`(
   (func (export "main")
     i32.const 1
     i32.const 2816
     i32.atomic.rmw16.xchg_u align=2
     i32.load16_s offset=83 align=1
     drop
   )
 )`).exports.main(), RuntimeError, unaligned);
+
+// Make sure we can handle wait and notify without memory
+
+var nomem = /(can't touch memory without memory)|(unknown memory)/;
+
+assertErrorMessage(() => new WebAssembly.Module(wasmTextToBinary(`
+(module
+  (func (result i32)
+    (atomic.notify (i32.const 0) (i32.const 1))))`)),
+                   WebAssembly.CompileError,
+                   nomem);
+
+assertErrorMessage(() => new WebAssembly.Module(wasmTextToBinary(`
+(module
+  (func (result i32)
+    (i32.atomic.wait (i32.const 0) (i32.const 1) (i64.const -1))))`)),
+                   WebAssembly.CompileError,
+                   nomem);
+
+assertErrorMessage(() => new WebAssembly.Module(wasmTextToBinary(`
+(module
+  (func (result i32)
+    (i64.atomic.wait (i32.const 0) (i64.const 1) (i64.const -1))))`)),
+                   WebAssembly.CompileError,
+                   nomem);
+
--- a/js/src/wasm/WasmIonCompile.cpp
+++ b/js/src/wasm/WasmIonCompile.cpp
@@ -1071,16 +1071,24 @@ class FunctionCompiler {
     }
     return false;
   }
 
  public:
   bool isMem32() { return moduleEnv_.memory->indexType() == IndexType::I32; }
   bool isMem64() { return moduleEnv_.memory->indexType() == IndexType::I64; }
 
+  // Sometimes, we need to determine the memory type before the opcode reader
+  // that will reject a memory opcode in the presence of no-memory gets a chance
+  // to do so. This predicate is safe.
+  bool isNoMemOrMem32() {
+    return !moduleEnv_.usesMemory() ||
+           moduleEnv_.memory->indexType() == IndexType::I32;
+  }
+
   // Add the offset into the pointer to yield the EA; trap on overflow.
   MDefinition* computeEffectiveAddress(MDefinition* base,
                                        MemoryAccessDesc* access) {
     if (inDeadCode()) {
       return nullptr;
     }
     if (!access->offset()) {
       return base;
@@ -3464,18 +3472,17 @@ static bool EmitBinaryMathBuiltinCall(Fu
   f.iter().setResult(def);
   return true;
 }
 
 static bool EmitMemoryGrow(FunctionCompiler& f) {
   uint32_t lineOrBytecode = f.readCallSiteLineOrBytecode();
 
   const SymbolicAddressSignature& callee =
-      !f.moduleEnv().usesMemory() || f.isMem32() ? SASigMemoryGrowM32
-                                                 : SASigMemoryGrowM64;
+      f.isNoMemOrMem32() ? SASigMemoryGrowM32 : SASigMemoryGrowM64;
   CallCompileState args;
   if (!f.passInstance(callee.argTypes[0], &args)) {
     return false;
   }
 
   MDefinition* delta;
   if (!f.iter().readMemoryGrow(&delta)) {
     return false;
@@ -3495,18 +3502,17 @@ static bool EmitMemoryGrow(FunctionCompi
   f.iter().setResult(ret);
   return true;
 }
 
 static bool EmitMemorySize(FunctionCompiler& f) {
   uint32_t lineOrBytecode = f.readCallSiteLineOrBytecode();
 
   const SymbolicAddressSignature& callee =
-      !f.moduleEnv().usesMemory() || f.isMem32() ? SASigMemorySizeM32
-                                                 : SASigMemorySizeM64;
+      f.isNoMemOrMem32() ? SASigMemorySizeM32 : SASigMemorySizeM64;
   CallCompileState args;
 
   if (!f.iter().readMemorySize()) {
     return false;
   }
 
   if (!f.passInstance(callee.argTypes[0], &args)) {
     return false;
@@ -3598,18 +3604,19 @@ static bool EmitAtomicStore(FunctionComp
 
 static bool EmitWait(FunctionCompiler& f, ValType type, uint32_t byteSize) {
   MOZ_ASSERT(type == ValType::I32 || type == ValType::I64);
   MOZ_ASSERT(SizeOf(type) == byteSize);
 
   uint32_t lineOrBytecode = f.readCallSiteLineOrBytecode();
 
   const SymbolicAddressSignature& callee =
-      f.isMem32() ? (type == ValType::I32 ? SASigWaitI32M32 : SASigWaitI64M32)
-                  : (type == ValType::I32 ? SASigWaitI32M64 : SASigWaitI64M64);
+      f.isNoMemOrMem32()
+          ? (type == ValType::I32 ? SASigWaitI32M32 : SASigWaitI64M32)
+          : (type == ValType::I32 ? SASigWaitI32M64 : SASigWaitI64M64);
   CallCompileState args;
   if (!f.passInstance(callee.argTypes[0], &args)) {
     return false;
   }
 
   LinearMemoryAddress<MDefinition*> addr;
   MDefinition* expected;
   MDefinition* timeout;
@@ -3658,17 +3665,17 @@ static bool EmitFence(FunctionCompiler& 
   f.fence();
   return true;
 }
 
 static bool EmitWake(FunctionCompiler& f) {
   uint32_t lineOrBytecode = f.readCallSiteLineOrBytecode();
 
   const SymbolicAddressSignature& callee =
-      f.isMem32() ? SASigWakeM32 : SASigWakeM64;
+      f.isNoMemOrMem32() ? SASigWakeM32 : SASigWakeM64;
   CallCompileState args;
   if (!f.passInstance(callee.argTypes[0], &args)) {
     return false;
   }
 
   LinearMemoryAddress<MDefinition*> addr;
   MDefinition* count;
   if (!f.iter().readWake(&addr, &count)) {