Skip to content

Commit

Permalink
Bug 1782585 - wasm: Generalize validation to multiple memories. r=jse…
Browse files Browse the repository at this point in the history
…ward

This commit switches ModuleEnvironment to hold a vector of memories, instead
of a single memory. Callers that assumed there were only one memory now
specify that they want 'memory 0'. We still only allow one memory using
a dynamic check when decoding memory section, and the Instance state
is not updated, so this is purely a data structure change.

Depends on D170977

Differential Revision: https://phabricator.services.mozilla.com/D170978
  • Loading branch information
eqrion committed Jul 7, 2023
1 parent fd40403 commit 3a48376
Show file tree
Hide file tree
Showing 20 changed files with 190 additions and 162 deletions.
2 changes: 1 addition & 1 deletion js/src/jit/MacroAssembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3551,7 +3551,7 @@ WasmMacroAssembler::WasmMacroAssembler(TempAllocator& alloc,
SetStackPointer64(sp);
#endif
setWasmMaxOffsetGuardLimit(
wasm::GetMaxOffsetGuardLimit(env.hugeMemoryEnabled()));
wasm::GetMaxOffsetGuardLimit(env.hugeMemoryEnabled(0)));
if (!limitedSize) {
setUnlimitedBuffer();
}
Expand Down
22 changes: 13 additions & 9 deletions js/src/wasm/AsmJS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2106,15 +2106,17 @@ class MOZ_STACK_CLASS ModuleValidator : public ModuleValidatorShared {
}

SharedModule finish() {
MOZ_ASSERT(!moduleEnv_.usesMemory());
MOZ_ASSERT(moduleEnv_.numMemories() == 0);
if (memory_.usage != MemoryUsage::None) {
Limits limits;
limits.shared = memory_.usage == MemoryUsage::Shared ? Shareable::True
: Shareable::False;
limits.initial = memory_.minPages();
limits.maximum = Nothing();
limits.indexType = IndexType::I32;
moduleEnv_.memory = Some(MemoryDesc(limits));
if (!moduleEnv_.memories.append(MemoryDesc(limits))) {
return nullptr;
}
}
MOZ_ASSERT(moduleEnv_.funcs.empty());
if (!moduleEnv_.funcs.resize(funcImportMap_.count() + funcDefs_.length())) {
Expand Down Expand Up @@ -6803,7 +6805,7 @@ static bool CheckBuffer(JSContext* cx, const AsmJSMetadata& metadata,
}
JSObject* bufferObj = &bufferVal.toObject();

if (metadata.usesSharedMemory()) {
if (metadata.memories[0].isShared()) {
if (!bufferObj->is<SharedArrayBufferObject>()) {
return LinkFail(
cx, "shared views can only be constructed onto SharedArrayBuffer");
Expand Down Expand Up @@ -6843,8 +6845,9 @@ static bool CheckBuffer(JSContext* cx, const AsmJSMetadata& metadata,
// This check is sufficient without considering the size of the loaded datum
// because heap loads and stores start on an aligned boundary and the heap
// byteLength has larger alignment.
uint64_t minMemoryLength =
metadata.usesMemory() ? metadata.memory->initialLength32() : 0;
uint64_t minMemoryLength = metadata.memories.length() != 0
? metadata.memories[0].initialLength32()
: 0;
MOZ_ASSERT((minMemoryLength - 1) <= INT32_MAX);
if (memoryLength < minMemoryLength) {
UniqueChars msg(JS_smprintf("ArrayBuffer byteLength of 0x%" PRIx64
Expand Down Expand Up @@ -6950,15 +6953,16 @@ static bool TryInstantiate(JSContext* cx, CallArgs args, const Module& module,

Rooted<ImportValues> imports(cx);

if (module.metadata().usesMemory()) {
if (module.metadata().memories.length() != 0) {
MOZ_ASSERT(module.metadata().memories.length() == 1);
RootedArrayBufferObject buffer(cx);
if (!CheckBuffer(cx, metadata, bufferVal, &buffer)) {
return false;
}

imports.get().memory =
WasmMemoryObject::create(cx, buffer, /* isHuge= */ false, nullptr);
if (!imports.get().memory) {
Rooted<WasmMemoryObject*> memory(
cx, WasmMemoryObject::create(cx, buffer, /* isHuge= */ false, nullptr));
if (!memory || !imports.get().memories.append(memory)) {
return false;
}
}
Expand Down
12 changes: 8 additions & 4 deletions js/src/wasm/WasmBCClass-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@ const FuncType& BaseCompiler::funcType() const {
return *moduleEnv_.funcs[func_.index].type;
}

bool BaseCompiler::usesMemory() const { return moduleEnv_.usesMemory(); }
bool BaseCompiler::usesMemory() const {
return moduleEnv_.memories.length() > 0;
}

bool BaseCompiler::usesSharedMemory() const {
return moduleEnv_.usesSharedMemory();
return moduleEnv_.usesSharedMemory(0);
}

const Local& BaseCompiler::localFromSlot(uint32_t slot, MIRType type) {
Expand All @@ -45,11 +47,13 @@ BytecodeOffset BaseCompiler::bytecodeOffset() const {
}

bool BaseCompiler::isMem32() const {
return moduleEnv_.memory->indexType() == IndexType::I32;
MOZ_ASSERT(moduleEnv_.memories.length() == 1);
return moduleEnv_.memories[0].indexType() == IndexType::I32;
}

bool BaseCompiler::isMem64() const {
return moduleEnv_.memory->indexType() == IndexType::I64;
MOZ_ASSERT(moduleEnv_.memories.length() == 1);
return moduleEnv_.memories[0].indexType() == IndexType::I64;
}

} // namespace wasm
Expand Down
22 changes: 11 additions & 11 deletions js/src/wasm/WasmBCMemory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void BaseCompiler::bceCheckLocal(MemoryAccessDesc* access, AccessCheck* check,
}

uint32_t offsetGuardLimit =
GetMaxOffsetGuardLimit(moduleEnv_.hugeMemoryEnabled());
GetMaxOffsetGuardLimit(moduleEnv_.hugeMemoryEnabled(0));

if ((bceSafe_ & (BCESet(1) << local)) &&
access->offset64() < offsetGuardLimit) {
Expand Down Expand Up @@ -131,10 +131,10 @@ RegI32 BaseCompiler::popConstMemoryAccess<RegI32>(MemoryAccessDesc* access,
uint32_t addr = addrTemp;

uint32_t offsetGuardLimit =
GetMaxOffsetGuardLimit(moduleEnv_.hugeMemoryEnabled());
GetMaxOffsetGuardLimit(moduleEnv_.hugeMemoryEnabled(0));

uint64_t ea = uint64_t(addr) + uint64_t(access->offset());
uint64_t limit = moduleEnv_.memory->initialLength32() + offsetGuardLimit;
uint64_t limit = moduleEnv_.memories[0].initialLength32() + offsetGuardLimit;

check->omitBoundsCheck = ea < limit;
check->omitAlignmentCheck = (ea & (access->byteSize() - 1)) == 0;
Expand All @@ -160,11 +160,11 @@ RegI64 BaseCompiler::popConstMemoryAccess<RegI64>(MemoryAccessDesc* access,
uint64_t addr = addrTemp;

uint32_t offsetGuardLimit =
GetMaxOffsetGuardLimit(moduleEnv_.hugeMemoryEnabled());
GetMaxOffsetGuardLimit(moduleEnv_.hugeMemoryEnabled(0));

uint64_t ea = addr + access->offset64();
bool overflow = ea < addr;
uint64_t limit = moduleEnv_.memory->initialLength64() + offsetGuardLimit;
uint64_t limit = moduleEnv_.memories[0].initialLength64() + offsetGuardLimit;

if (!overflow) {
check->omitBoundsCheck = ea < limit;
Expand Down Expand Up @@ -351,7 +351,7 @@ void BaseCompiler::prepareMemoryAccess(MemoryAccessDesc* access,
AccessCheck* check, RegPtr instance,
RegIndexType ptr) {
uint32_t offsetGuardLimit =
GetMaxOffsetGuardLimit(moduleEnv_.hugeMemoryEnabled());
GetMaxOffsetGuardLimit(moduleEnv_.hugeMemoryEnabled(0));

// Fold offset if necessary for further computations.
if (access->offset64() >= offsetGuardLimit ||
Expand Down Expand Up @@ -379,7 +379,7 @@ void BaseCompiler::prepareMemoryAccess(MemoryAccessDesc* access,

// Ensure no instance if we don't need it.

if (moduleEnv_.hugeMemoryEnabled()) {
if (moduleEnv_.hugeMemoryEnabled(0)) {
// We have HeapReg and no bounds checking and need load neither
// memoryBase nor boundsCheckLimit from instance.
MOZ_ASSERT_IF(check->omitBoundsCheck, instance.isInvalid());
Expand All @@ -391,14 +391,14 @@ void BaseCompiler::prepareMemoryAccess(MemoryAccessDesc* access,

// Bounds check if required.

if (!moduleEnv_.hugeMemoryEnabled() && !check->omitBoundsCheck) {
if (!moduleEnv_.hugeMemoryEnabled(0) && !check->omitBoundsCheck) {
Label ok;
#ifdef JS_64BIT
// The checking depends on how many bits are in the pointer and how many
// bits are in the bound.
static_assert(0x100000000 % PageSize == 0);
if (!moduleEnv_.memory->boundsCheckLimitIs32Bits() &&
MaxMemoryPages(moduleEnv_.memory->indexType()) >=
if (!moduleEnv_.memories[0].boundsCheckLimitIs32Bits() &&
MaxMemoryPages(moduleEnv_.memories[0].indexType()) >=
Pages(0x100000000 / PageSize)) {
boundsCheck4GBOrLargerAccess(instance, ptr, &ok);
} else {
Expand Down Expand Up @@ -432,7 +432,7 @@ bool BaseCompiler::needInstanceForAccess(const AccessCheck& check) {
// Platform requires instance for memory base.
return true;
#else
return !moduleEnv_.hugeMemoryEnabled() && !check.omitBoundsCheck;
return !moduleEnv_.hugeMemoryEnabled(0) && !check.omitBoundsCheck;
#endif
}

Expand Down
2 changes: 1 addition & 1 deletion js/src/wasm/WasmBaselineCompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10896,7 +10896,7 @@ BaseCompiler::~BaseCompiler() {

bool BaseCompiler::init() {
// We may lift this restriction in the future.
MOZ_ASSERT_IF(usesMemory() && isMem64(), !moduleEnv_.hugeMemoryEnabled());
MOZ_ASSERT_IF(usesMemory() && isMem64(), !moduleEnv_.hugeMemoryEnabled(0));
// asm.js is not supported in baseline
MOZ_ASSERT(!moduleEnv_.isAsmJS());
// Only asm.js modules have call site line numbers
Expand Down
9 changes: 2 additions & 7 deletions js/src/wasm/WasmCode.h
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,6 @@ using FuncImportVector = Vector<FuncImport, 0, SystemAllocPolicy>;

struct MetadataCacheablePod {
ModuleKind kind;
Maybe<MemoryDesc> memory;
uint32_t instanceDataLength;
Maybe<uint32_t> startFuncIndex;
Maybe<uint32_t> nameCustomSectionIndex;
Expand All @@ -368,7 +367,7 @@ struct MetadataCacheablePod {
uint32_t tagsOffsetStart;
uint32_t padding;

WASM_CHECK_CACHEABLE_POD(kind, memory, instanceDataLength, startFuncIndex,
WASM_CHECK_CACHEABLE_POD(kind, instanceDataLength, startFuncIndex,
nameCustomSectionIndex, filenameIsURL,
omitsBoundsChecks, typeDefsOffsetStart,
tablesOffsetStart, tagsOffsetStart)
Expand All @@ -392,6 +391,7 @@ using ModuleHash = uint8_t[8];

struct Metadata : public ShareableBase<Metadata>, public MetadataCacheablePod {
SharedTypeContext types;
MemoryDescVector memories;
GlobalDescVector globals;
TableDescVector tables;
TagDescVector tags;
Expand All @@ -417,11 +417,6 @@ struct Metadata : public ShareableBase<Metadata>, public MetadataCacheablePod {
MetadataCacheablePod& pod() { return *this; }
const MetadataCacheablePod& pod() const { return *this; }

bool usesMemory() const { return memory.isSome(); }
bool usesSharedMemory() const {
return memory.isSome() && memory->isShared();
}

const FuncType& getFuncImportType(const FuncImport& funcImport) const {
return types->type(funcImport.typeIndex()).funcType();
}
Expand Down
4 changes: 2 additions & 2 deletions js/src/wasm/WasmGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1049,15 +1049,15 @@ SharedMetadata ModuleGenerator::finishMetadata(const Bytes& bytecode) {

// Copy over data from the ModuleEnvironment.

metadata_->memory = moduleEnv_->memory;
metadata_->startFuncIndex = moduleEnv_->startFuncIndex;
metadata_->memories = std::move(moduleEnv_->memories);
metadata_->tables = std::move(moduleEnv_->tables);
metadata_->globals = std::move(moduleEnv_->globals);
metadata_->tags = std::move(moduleEnv_->tags);
metadata_->nameCustomSectionIndex = moduleEnv_->nameCustomSectionIndex;
metadata_->moduleName = moduleEnv_->moduleName;
metadata_->funcNames = std::move(moduleEnv_->funcNames);
metadata_->omitsBoundsChecks = moduleEnv_->hugeMemoryEnabled();
metadata_->omitsBoundsChecks = moduleEnv_->hugeMemoryEnabled(0);

// Copy over additional debug information.

Expand Down
5 changes: 3 additions & 2 deletions js/src/wasm/WasmInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1929,9 +1929,10 @@ bool Instance::memoryAccessInGuardRegion(const uint8_t* addr,
unsigned numBytes) const {
MOZ_ASSERT(numBytes > 0);

if (!metadata().usesMemory()) {
if (metadata().memories.length() != 0) {
return false;
}
MOZ_RELEASE_ASSERT(metadata().memories.length() == 1);

uint8_t* base = memoryBase().unwrap(/* comparison */);
if (addr < base) {
Expand Down Expand Up @@ -2103,7 +2104,7 @@ uintptr_t Instance::traceFrame(JSTracer* trc, const wasm::WasmFrameIter& wfi,
WasmMemoryObject* Instance::memory() const { return memory_; }

SharedMem<uint8_t*> Instance::memoryBase() const {
MOZ_ASSERT(metadata().usesMemory());
MOZ_ASSERT(metadata().memories.length() > 0);
MOZ_ASSERT(memoryBase_ == memory_->buffer().dataPointerEither());
return memory_->buffer().dataPointerEither();
}
Expand Down
6 changes: 5 additions & 1 deletion js/src/wasm/WasmIntrinsic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,11 @@ bool wasm::CompileIntrinsicModule(JSContext* cx,
ReportOutOfMemory(cx);
return false;
}
moduleEnv.memory = Some(MemoryDesc(Limits(0, Nothing(), sharedMemory)));
if (!moduleEnv.memories.append(
MemoryDesc(Limits(0, Nothing(), sharedMemory)))) {
ReportOutOfMemory(cx);
return false;
}

// Add (type (func (params ...))) for each intrinsic. The function types will
// be deduplicated by the runtime
Expand Down
Loading

0 comments on commit 3a48376

Please sign in to comment.