Skip to content

Commit

Permalink
Bug 1337723 - wasm: Warn about executable memory allocation failure. …
Browse files Browse the repository at this point in the history
…r=lth

This commit causes ModuleGenerator to report a warning when
we fail to allocate executable memory for a ModuleSegment. This will
get reported in the JS console in addition to the OOM error.

The one tricky change is that entry points to module compilation
needed to be reworked so that they could report warnings even if
an error or OOM was encountered.

Differential Revision: https://phabricator.services.mozilla.com/D125230
  • Loading branch information
eqrion committed Sep 21, 2021
1 parent 2fc4dda commit faddec7
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 18 deletions.
3 changes: 2 additions & 1 deletion js/src/wasm/AsmJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2175,7 +2175,8 @@ class MOZ_STACK_CLASS ModuleValidator : public ModuleValidatorShared {
return nullptr;
}

ModuleGenerator mg(*args, &moduleEnv_, &compilerEnv_, nullptr, nullptr);
ModuleGenerator mg(*args, &moduleEnv_, &compilerEnv_, nullptr, nullptr,
nullptr);
if (!mg.init(asmJSMetadata_.get())) {
return nullptr;
}
Expand Down
9 changes: 6 additions & 3 deletions js/src/wasm/WasmCompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ SharedModule wasm::CompileBuffer(const CompileArgs& args,
CompilerEnvironment compilerEnv(args);
compilerEnv.computeParameters(d);

ModuleGenerator mg(args, &moduleEnv, &compilerEnv, nullptr, error);
ModuleGenerator mg(args, &moduleEnv, &compilerEnv, nullptr, error, warnings);
if (!mg.init(nullptr)) {
return nullptr;
}
Expand Down Expand Up @@ -728,7 +728,8 @@ void wasm::CompileTier2(const CompileArgs& args, const Bytes& bytecode,
optimizedBackend, DebugEnabled::False);
compilerEnv.computeParameters(d);

ModuleGenerator mg(args, &moduleEnv, &compilerEnv, cancelled, &error);
ModuleGenerator mg(args, &moduleEnv, &compilerEnv, cancelled, &error,
nullptr);
if (!mg.init(nullptr)) {
return;
}
Expand All @@ -747,6 +748,7 @@ void wasm::CompileTier2(const CompileArgs& args, const Bytes& bytecode,

// The caller doesn't care about success or failure; only that compilation
// is inactive, so there is no success to return here.
// TODO: report warnings to the console
}

class StreamingDecoder {
Expand Down Expand Up @@ -850,7 +852,8 @@ SharedModule wasm::CompileStreaming(
MOZ_RELEASE_ASSERT(d.done());
}

ModuleGenerator mg(args, &moduleEnv, &compilerEnv, &cancelled, error);
ModuleGenerator mg(args, &moduleEnv, &compilerEnv, &cancelled, error,
warnings);
if (!mg.init(nullptr)) {
return nullptr;
}
Expand Down
22 changes: 21 additions & 1 deletion js/src/wasm/WasmGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

#include "jit/Assembler.h"
#include "jit/JitOptions.h"
#include "js/Printf.h"
#include "util/Memory.h"
#include "util/Text.h"
#include "vm/HelperThreads.h"
Expand Down Expand Up @@ -83,9 +84,11 @@ ModuleGenerator::ModuleGenerator(const CompileArgs& args,
ModuleEnvironment* moduleEnv,
CompilerEnvironment* compilerEnv,
const Atomic<bool>* cancelled,
UniqueChars* error)
UniqueChars* error,
UniqueCharsVector* warnings)
: compileArgs_(&args),
error_(error),
warnings_(warnings),
cancelled_(cancelled),
moduleEnv_(moduleEnv),
compilerEnv_(compilerEnv),
Expand Down Expand Up @@ -1080,6 +1083,7 @@ UniqueCodeTier ModuleGenerator::finishCodeTier() {
UniqueModuleSegment segment =
ModuleSegment::create(tier(), masm_, *linkData_);
if (!segment) {
warnf("failed to allocate executable memory for module");
return nullptr;
}

Expand Down Expand Up @@ -1293,6 +1297,22 @@ bool ModuleGenerator::finishTier2(const Module& module) {
return module.finishTier2(*linkData_, std::move(codeTier));
}

void ModuleGenerator::warnf(const char* msg, ...) {
if (!warnings_) {
return;
}

va_list ap;
va_start(ap, msg);
UniqueChars str(JS_vsmprintf(msg, ap));
va_end(ap);
if (!str) {
return;
}

(void)warnings_->append(std::move(str));
}

size_t CompiledCode::sizeOfExcludingThis(
mozilla::MallocSizeOf mallocSizeOf) const {
size_t trapSitesSize = 0;
Expand Down
7 changes: 6 additions & 1 deletion js/src/wasm/WasmGenerator.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#ifndef wasm_generator_h
#define wasm_generator_h

#include "mozilla/Attributes.h"
#include "mozilla/MemoryReporting.h"

#include "jit/MacroAssembler.h"
Expand Down Expand Up @@ -189,6 +190,7 @@ class MOZ_STACK_CLASS ModuleGenerator {
// Constant parameters
SharedCompileArgs const compileArgs_;
UniqueChars* const error_;
UniqueCharsVector* const warnings_;
const Atomic<bool>* const cancelled_;
ModuleEnvironment* const moduleEnv_;
CompilerEnvironment* const compilerEnv_;
Expand Down Expand Up @@ -245,10 +247,13 @@ class MOZ_STACK_CLASS ModuleGenerator {
CompileMode mode() const { return compilerEnv_->mode(); }
bool debugEnabled() const { return compilerEnv_->debugEnabled(); }

void warnf(const char* msg, ...) MOZ_FORMAT_PRINTF(2, 3);

public:
ModuleGenerator(const CompileArgs& args, ModuleEnvironment* moduleEnv,
CompilerEnvironment* compilerEnv,
const Atomic<bool>* cancelled, UniqueChars* error);
const Atomic<bool>* cancelled, UniqueChars* error,
UniqueCharsVector* warnings);
~ModuleGenerator();
[[nodiscard]] bool init(Metadata* maybeAsmJSMetadata = nullptr);

Expand Down
3 changes: 2 additions & 1 deletion js/src/wasm/WasmIntrinsic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ bool wasm::CompileIntrinsicModule(JSContext* cx,

// Compile the module functions
UniqueChars error;
ModuleGenerator mg(*compileArgs, &moduleEnv, &compilerEnv, nullptr, &error);
ModuleGenerator mg(*compileArgs, &moduleEnv, &compilerEnv, nullptr, &error,
nullptr);
if (!mg.init(nullptr)) {
ReportOutOfMemory(cx);
return false;
Expand Down
23 changes: 12 additions & 11 deletions js/src/wasm/WasmJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1758,6 +1758,10 @@ bool WasmModuleObject::construct(JSContext* cx, unsigned argc, Value* vp) {
UniqueCharsVector warnings;
SharedModule module =
CompileBuffer(*compileArgs, *bytecode, &error, &warnings, nullptr);

if (!ReportCompileWarnings(cx, warnings)) {
return false;
}
if (!module) {
if (error) {
JS_ReportErrorNumberUTF8(cx, GetErrorMessage, nullptr,
Expand All @@ -1768,10 +1772,6 @@ bool WasmModuleObject::construct(JSContext* cx, unsigned argc, Value* vp) {
return false;
}

if (!ReportCompileWarnings(cx, warnings)) {
return false;
}

RootedObject proto(
cx, GetWasmConstructorPrototype(cx, callArgs, JSProto_WasmModule));
if (!proto) {
Expand Down Expand Up @@ -4216,7 +4216,8 @@ JSFunction* WasmFunctionCreate(JSContext* cx, HandleFunction func,
return nullptr;
}

ModuleGenerator mg(*compileArgs, &moduleEnv, &compilerEnv, nullptr, nullptr);
ModuleGenerator mg(*compileArgs, &moduleEnv, &compilerEnv, nullptr, nullptr,
nullptr);
if (!mg.init(nullptr)) {
return nullptr;
}
Expand Down Expand Up @@ -4560,12 +4561,12 @@ struct CompileBufferTask : PromiseHelperTask {
}

bool resolve(JSContext* cx, Handle<PromiseObject*> promise) override {
if (!module) {
return Reject(cx, *compileArgs, promise, error);
}
if (!ReportCompileWarnings(cx, warnings)) {
return false;
}
if (!module) {
return Reject(cx, *compileArgs, promise, error);
}
if (instantiate) {
return AsyncInstantiate(cx, *module, importObj, Ret::Pair, promise);
}
Expand Down Expand Up @@ -5019,11 +5020,11 @@ class CompileStreamTask : public PromiseHelperTask, public JS::StreamConsumer {
bool resolve(JSContext* cx, Handle<PromiseObject*> promise) override {
MOZ_ASSERT(streamState_.lock() == Closed);

if (!ReportCompileWarnings(cx, warnings_)) {
return false;
}
if (module_) {
MOZ_ASSERT(!streamFailed_ && !streamError_ && !compileError_);
if (!ReportCompileWarnings(cx, warnings_)) {
return false;
}
if (instantiate_) {
return AsyncInstantiate(cx, *module_, importObj_, Ret::Pair, promise);
}
Expand Down

0 comments on commit faddec7

Please sign in to comment.