Skip to content

Commit

Permalink
Bug 1757733 - wasm: Don't report warnings from AsmJS compilation, whi…
Browse files Browse the repository at this point in the history
…ch may be off-main-thread. r=yury

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
  • Loading branch information
eqrion committed Mar 16, 2022
1 parent f43adbd commit 3c6f4ab
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 16 deletions.
2 changes: 1 addition & 1 deletion js/src/fuzz-tests/testWasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 5 additions & 1 deletion js/src/wasm/AsmJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2154,9 +2154,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;
}

Expand Down
39 changes: 32 additions & 7 deletions js/src/wasm/WasmCompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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.
*
Expand Down
19 changes: 15 additions & 4 deletions js/src/wasm/WasmCompileArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)),
Expand Down
2 changes: 1 addition & 1 deletion js/src/wasm/WasmIntrinsic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions js/src/wasm/WasmJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,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);
}

// ============================================================================
Expand Down Expand Up @@ -4283,7 +4283,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;
}
Expand Down

0 comments on commit 3c6f4ab

Please sign in to comment.