[tbb-commits] [tor-browser] 223/311: Bug 1757733 - wasm: Don't report warnings from AsmJS compilation, which may be off-main-thread. r=yury a=dmeehan

gitolite role git at cupani.torproject.org
Tue Apr 26 15:30:23 UTC 2022


This is an automated email from the git hooks/post-receive script.

pierov pushed a commit to branch geckoview-99.0.1-11.0-1
in repository tor-browser.

commit ca062c9e8d8f95d792bbf7f058572593d71192ef
Author: Ryan Hunt <rhunt at eqrion.net>
AuthorDate: Wed Mar 16 19:52:45 2022 +0000

    Bug 1757733 - wasm: Don't report warnings from AsmJS compilation, which may be off-main-thread. r=yury a=dmeehan
    
    AsmJS compilation may be off the main thread, so we cannot report warnings
    to JSContext. wasm::Log may do this if the right pref is on. CompileArgs::
    build() uses wasm::Log. AsmJS uses CompileArgs::build(). This commit adds
    a separate version of CompileArgs::build() which will not log or report
    errors. AsmJS then asserts that only an OOM may be possible here, as we
    should ensure a wasm compiler is available before compiling.
    
    Differential Revision: https://phabricator.services.mozilla.com/D141007
---
 js/src/fuzz-tests/testWasm.cpp |  2 +-
 js/src/wasm/AsmJS.cpp          |  6 +++++-
 js/src/wasm/WasmCompile.cpp    | 39 ++++++++++++++++++++++++++++++++-------
 js/src/wasm/WasmCompileArgs.h  | 19 +++++++++++++++----
 js/src/wasm/WasmIntrinsic.cpp  |  2 +-
 js/src/wasm/WasmJS.cpp         |  4 ++--
 6 files changed, 56 insertions(+), 16 deletions(-)

diff --git a/js/src/fuzz-tests/testWasm.cpp b/js/src/fuzz-tests/testWasm.cpp
index d86c79d2d5675..3bd268c85ee22 100644
--- a/js/src/fuzz-tests/testWasm.cpp
+++ b/js/src/fuzz-tests/testWasm.cpp
@@ -255,7 +255,7 @@ static int testWasmFuzz(const uint8_t* buf, size_t size) {
     ScriptedCaller scriptedCaller;
     FeatureOptions options;
     SharedCompileArgs compileArgs =
-        CompileArgs::build(gCx, std::move(scriptedCaller), options);
+        CompileArgs::buildAndReport(gCx, std::move(scriptedCaller), options);
     if (!compileArgs) {
       return 0;
     }
diff --git a/js/src/wasm/AsmJS.cpp b/js/src/wasm/AsmJS.cpp
index 910a5adcb016e..41935c517f069 100644
--- a/js/src/wasm/AsmJS.cpp
+++ b/js/src/wasm/AsmJS.cpp
@@ -2153,9 +2153,13 @@ class MOZ_STACK_CLASS ModuleValidator : public ModuleValidatorShared {
 
     // The default options are fine for asm.js
     FeatureOptions options;
+    CompileArgsError error;
     SharedCompileArgs args =
-        CompileArgs::build(cx_, std::move(scriptedCaller), options);
+        CompileArgs::build(cx_, std::move(scriptedCaller), options, &error);
     if (!args) {
+      // EstablishPreconditions will ensure that a compiler is available by
+      // this point
+      MOZ_RELEASE_ASSERT(error == CompileArgsError::OutOfMemory);
       return nullptr;
     }
 
diff --git a/js/src/wasm/WasmCompile.cpp b/js/src/wasm/WasmCompile.cpp
index 1ae629929844b..cfa42b39749fd 100644
--- a/js/src/wasm/WasmCompile.cpp
+++ b/js/src/wasm/WasmCompile.cpp
@@ -107,7 +107,8 @@ FeatureArgs FeatureArgs::build(JSContext* cx, const FeatureOptions& options) {
 
 SharedCompileArgs CompileArgs::build(JSContext* cx,
                                      ScriptedCaller&& scriptedCaller,
-                                     const FeatureOptions& options) {
+                                     const FeatureOptions& options,
+                                     CompileArgsError* error) {
   bool baseline = BaselineAvailable(cx);
   bool ion = IonAvailable(cx);
   bool cranelift = CraneliftAvailable(cx);
@@ -128,7 +129,7 @@ SharedCompileArgs CompileArgs::build(JSContext* cx,
   // when we're fuzzing we allow inconsistent switches and the check may thus
   // fail.  Let it go to a run-time error instead of crashing.
   if (debug && (ion || cranelift)) {
-    JS_ReportErrorASCII(cx, "no WebAssembly compiler available");
+    *error = CompileArgsError::NoCompiler;
     return nullptr;
   }
 
@@ -140,12 +141,13 @@ SharedCompileArgs CompileArgs::build(JSContext* cx,
   }
 
   if (!(baseline || ion || cranelift)) {
-    JS_ReportErrorASCII(cx, "no WebAssembly compiler available");
+    *error = CompileArgsError::NoCompiler;
     return nullptr;
   }
 
   CompileArgs* target = cx->new_<CompileArgs>(std::move(scriptedCaller));
   if (!target) {
+    *error = CompileArgsError::OutOfMemory;
     return nullptr;
   }
 
@@ -156,13 +158,36 @@ SharedCompileArgs CompileArgs::build(JSContext* cx,
   target->forceTiering = forceTiering;
   target->features = FeatureArgs::build(cx, options);
 
-  Log(cx, "available wasm compilers: tier1=%s tier2=%s",
-      baseline ? "baseline" : "none",
-      ion ? "ion" : (cranelift ? "cranelift" : "none"));
-
   return target;
 }
 
+SharedCompileArgs CompileArgs::buildAndReport(JSContext* cx,
+                                              ScriptedCaller&& scriptedCaller,
+                                              const FeatureOptions& options) {
+  CompileArgsError error;
+  SharedCompileArgs args =
+      CompileArgs::build(cx, std::move(scriptedCaller), options, &error);
+  if (args) {
+    Log(cx, "available wasm compilers: tier1=%s tier2=%s",
+        args->baselineEnabled ? "baseline" : "none",
+        args->ionEnabled ? "ion"
+                         : (args->craneliftEnabled ? "cranelift" : "none"));
+    return args;
+  }
+
+  switch (error) {
+    case CompileArgsError::NoCompiler: {
+      JS_ReportErrorASCII(cx, "no WebAssembly compiler available");
+      break;
+    }
+    case CompileArgsError::OutOfMemory: {
+      // Intentionally do not report the OOM, as callers expect this behavior
+      break;
+    }
+  }
+  return nullptr;
+}
+
 /*
  * [SMDOC] Tiered wasm compilation.
  *
diff --git a/js/src/wasm/WasmCompileArgs.h b/js/src/wasm/WasmCompileArgs.h
index a45660e43cc71..9d408ed3f285f 100644
--- a/js/src/wasm/WasmCompileArgs.h
+++ b/js/src/wasm/WasmCompileArgs.h
@@ -118,6 +118,13 @@ struct ScriptedCaller {
   ScriptedCaller() : filenameIsURL(false), line(0) {}
 };
 
+// Describes the reasons we cannot compute compile args
+
+enum class CompileArgsError {
+  OutOfMemory,
+  NoCompiler,
+};
+
 // Describes all the parameters that control wasm compilation.
 
 struct CompileArgs;
@@ -136,17 +143,21 @@ struct CompileArgs : ShareableBase<CompileArgs> {
 
   FeatureArgs features;
 
-  // CompileArgs has two constructors:
+  // CompileArgs has three constructors:
   //
-  // - one through a factory function `build`, which checks that flags are
-  // consistent with each other.
+  // - two through a factory function `build`, 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).
 
   static SharedCompileArgs build(JSContext* cx, ScriptedCaller&& scriptedCaller,
-                                 const FeatureOptions& options);
+                                 const FeatureOptions& options,
+                                 CompileArgsError* error);
+  static SharedCompileArgs buildAndReport(JSContext* cx,
+                                          ScriptedCaller&& scriptedCaller,
+                                          const FeatureOptions& options);
 
   explicit CompileArgs(ScriptedCaller&& scriptedCaller)
       : scriptedCaller(std::move(scriptedCaller)),
diff --git a/js/src/wasm/WasmIntrinsic.cpp b/js/src/wasm/WasmIntrinsic.cpp
index b0c596139a7f7..d4ebb2345dbe4 100644
--- a/js/src/wasm/WasmIntrinsic.cpp
+++ b/js/src/wasm/WasmIntrinsic.cpp
@@ -99,7 +99,7 @@ bool wasm::CompileIntrinsicModule(JSContext* cx,
 
   // Initialize the compiler environment, choosing the best tier possible
   SharedCompileArgs compileArgs =
-      CompileArgs::build(cx, ScriptedCaller(), featureOptions);
+      CompileArgs::buildAndReport(cx, ScriptedCaller(), featureOptions);
   if (!compileArgs) {
     return false;
   }
diff --git a/js/src/wasm/WasmJS.cpp b/js/src/wasm/WasmJS.cpp
index 3b7a2a5d9413a..7ac9f92de8842 100644
--- a/js/src/wasm/WasmJS.cpp
+++ b/js/src/wasm/WasmJS.cpp
@@ -702,7 +702,7 @@ static SharedCompileArgs InitCompileArgs(JSContext* cx,
   if (!ParseCompileOptions(cx, maybeOptions, &options)) {
     return nullptr;
   }
-  return CompileArgs::build(cx, std::move(scriptedCaller), options);
+  return CompileArgs::buildAndReport(cx, std::move(scriptedCaller), options);
 }
 
 // ============================================================================
@@ -4286,7 +4286,7 @@ JSFunction* WasmFunctionCreate(JSContext* cx, HandleFunction func,
   FeatureOptions options;
   ScriptedCaller scriptedCaller;
   SharedCompileArgs compileArgs =
-      CompileArgs::build(cx, std::move(scriptedCaller), options);
+      CompileArgs::buildAndReport(cx, std::move(scriptedCaller), options);
   if (!compileArgs) {
     return nullptr;
   }

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the tbb-commits mailing list