Bug 1791992 - wasm: Fix OOM handling in CompileIntrinsicModule. r=yury

Differential Revision: https://phabricator.services.mozilla.com/D157933
dothq · Sep 23, 2022 · 39d4c14 · 39d4c14
1 parent a6cb0e0
commit 39d4c14
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 10 deletions.
diff --git a/js/src/jit-test/tests/wasm/intrinsics/oom-test.js b/js/src/jit-test/tests/wasm/intrinsics/oom-test.js
@@ -0,0 +1,6 @@
+// |jit-test| skip-if: !('oomTest' in this)
+
+oomTest(() => {
+    const module = wasmIntrinsicI8VecMul();
+    WebAssembly.Module.imports(module);
+});
diff --git a/js/src/wasm/WasmCompile.cpp b/js/src/wasm/WasmCompile.cpp
@@ -151,7 +151,8 @@ SharedCompileArgs CompileArgs::build(JSContext* cx,
 
 SharedCompileArgs CompileArgs::buildAndReport(JSContext* cx,
                                               ScriptedCaller&& scriptedCaller,
-                                              const FeatureOptions& options) {
+                                              const FeatureOptions& options,
+                                              bool reportOOM) {
   CompileArgsError error;
   SharedCompileArgs args =
       CompileArgs::build(cx, std::move(scriptedCaller), options, &error);
@@ -168,7 +169,11 @@ SharedCompileArgs CompileArgs::buildAndReport(JSContext* cx,
       break;
     }
     case CompileArgsError::OutOfMemory: {
-      // Intentionally do not report the OOM, as callers expect this behavior
+      // Most callers are required to return 'false' without reporting an OOM,
+      // so we make reporting it optional here.
+      if (reportOOM) {
+        ReportOutOfMemory(cx);
+      }
       break;
     }
   }

diff --git a/js/src/wasm/WasmCompileArgs.h b/js/src/wasm/WasmCompileArgs.h
@@ -142,19 +142,22 @@ struct CompileArgs : ShareableBase<CompileArgs> {
 
   // CompileArgs has three constructors:
   //
-  // - two through a factory function `build`, which checks that flags are
-  // consistent with each other, and optionally reports any errors.
+  // - two through factory functions `build`/`buildAndReport`, which checks
+  //   that flags are consistent with each other, and optionally reports any
+  //   errors.
   // - one that gives complete access to underlying fields.
   //
-  // You should use the first one in general, unless you have a very good
-  // reason (i.e. no JSContext around and you know which flags have been used).
+  // You should use the factory functions in general, unless you have a very
+  // good reason (i.e. no JSContext around and you know which flags have been
+  // used).
 
   static SharedCompileArgs build(JSContext* cx, ScriptedCaller&& scriptedCaller,
                                  const FeatureOptions& options,
                                  CompileArgsError* error);
   static SharedCompileArgs buildAndReport(JSContext* cx,
                                           ScriptedCaller&& scriptedCaller,
-                                          const FeatureOptions& options);
+                                          const FeatureOptions& options,
+                                          bool reportOOM = false);
 
   explicit CompileArgs(ScriptedCaller&& scriptedCaller)
       : scriptedCaller(std::move(scriptedCaller)),

diff --git a/js/src/wasm/WasmIntrinsic.cpp b/js/src/wasm/WasmIntrinsic.cpp
@@ -98,8 +98,8 @@ bool wasm::CompileIntrinsicModule(JSContext* cx,
   featureOptions.intrinsics = true;
 
   // Initialize the compiler environment, choosing the best tier possible
-  SharedCompileArgs compileArgs =
-      CompileArgs::buildAndReport(cx, ScriptedCaller(), featureOptions);
+  SharedCompileArgs compileArgs = CompileArgs::buildAndReport(
+      cx, ScriptedCaller(), featureOptions, /* reportOOM */ true);
   if (!compileArgs) {
     return false;
   }
@@ -115,6 +115,7 @@ bool wasm::CompileIntrinsicModule(JSContext* cx,
   CacheableName emptyString;
   CacheableName memoryString;
   if (!CacheableName::fromUTF8Chars("memory", &memoryString)) {
+    ReportOutOfMemory(cx);
     return false;
   }
   if (!moduleEnv.imports.append(Import(std::move(emptyString),
@@ -127,6 +128,7 @@ bool wasm::CompileIntrinsicModule(JSContext* cx,
 
   // Initialize the type section
   if (!moduleEnv.initTypes(ids.size())) {
+    ReportOutOfMemory(cx);
     return false;
   }
 
@@ -214,8 +216,14 @@ bool wasm::CompileIntrinsicModule(JSContext* cx,
     return false;
   }
 
-  // Finish the module
+  // Create a dummy bytecode vector, that will not be used
   SharedBytes bytecode = js_new<ShareableBytes>();
+  if (!bytecode) {
+    ReportOutOfMemory(cx);
+    return false;
+  }
+
+  // Finish the module
   SharedModule module = mg.finishModule(*bytecode, nullptr);
   if (!module) {
     ReportOutOfMemory(cx);

diff --git a/js/src/wasm/WasmJS.cpp b/js/src/wasm/WasmJS.cpp
@@ -1098,6 +1098,9 @@ static JSObject* MemoryTypeToObject(JSContext* cx, bool shared,
 #  ifdef ENABLE_WASM_MEMORY64
   RootedString it(
       cx, JS_NewStringCopyZ(cx, indexType == IndexType::I32 ? "i32" : "i64"));
+  if (!it) {
+    return nullptr;
+  }
   if (!props.append(
           IdValuePair(NameToId(cx->names().index), StringValue(it)))) {
     ReportOutOfMemory(cx);