From e96ff8f3808fc2d9519e5290175441a0b2d0219c Mon Sep 17 00:00:00 2001 From: Ryan Hunt Date: Fri, 30 Aug 2019 03:17:29 +0000 Subject: [PATCH] Bug 1518210 - Wasm: Conditionally compile bounds checks based on wasm::IsHugeMemoryEnabled. r=lth This commit allows us to conditionally compile bounds checks based on runtime support for huge memory. New flags to CompileArgs and CompilerEnvironment are added for whether we are using huge memory or not, and computed based on the global flag. Differential Revision: https://phabricator.services.mozilla.com/D41868 --HG-- extra : moz-landing-system : lando --- js/src/jit/CodeGenerator.cpp | 4 ---- js/src/jit/Lowering.cpp | 4 ---- js/src/jit/WasmBCE.cpp | 5 ----- js/src/wasm/AsmJS.cpp | 2 +- js/src/wasm/WasmBaselineCompile.cpp | 20 ++++++++------------ js/src/wasm/WasmCode.cpp | 9 +++++++-- js/src/wasm/WasmCode.h | 1 + js/src/wasm/WasmCompile.cpp | 13 ++++++++++--- js/src/wasm/WasmCompile.h | 4 +++- js/src/wasm/WasmCraneliftCompile.cpp | 26 ++++++++++++-------------- js/src/wasm/WasmGenerator.cpp | 1 + js/src/wasm/WasmIonCompile.cpp | 4 +--- js/src/wasm/WasmJS.cpp | 5 +++++ js/src/wasm/WasmValidate.cpp | 7 ++++--- js/src/wasm/WasmValidate.h | 10 +++++++++- 15 files changed, 62 insertions(+), 53 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 5bf49787b002..44597cf7a6b5 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -13286,9 +13286,6 @@ void CodeGenerator::visitWasmTrap(LWasmTrap* lir) { } void CodeGenerator::visitWasmBoundsCheck(LWasmBoundsCheck* ins) { -#ifdef WASM_HUGE_MEMORY - MOZ_CRASH("No wasm bounds check for huge memory"); -#else const MWasmBoundsCheck* mir = ins->mir(); Register ptr = ToRegister(ins->ptr()); Register boundsCheckLimit = ToRegister(ins->boundsCheckLimit()); @@ -13296,7 +13293,6 @@ void CodeGenerator::visitWasmBoundsCheck(LWasmBoundsCheck* ins) { masm.wasmBoundsCheck(Assembler::Below, ptr, boundsCheckLimit, &ok); masm.wasmTrap(wasm::Trap::OutOfBounds, mir->bytecodeOffset()); masm.bind(&ok); -#endif } void CodeGenerator::visitWasmAlignmentCheck(LWasmAlignmentCheck* ins) { diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index d0ac60909bdb..54e03d7cb214 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -4206,9 +4206,6 @@ void LIRGenerator::visitWasmLoadTls(MWasmLoadTls* ins) { } void LIRGenerator::visitWasmBoundsCheck(MWasmBoundsCheck* ins) { -#ifdef WASM_HUGE_MEMORY - MOZ_CRASH("No bounds checking on huge memory"); -#else MOZ_ASSERT(!ins->isRedundant()); MDefinition* index = ins->index(); @@ -4226,7 +4223,6 @@ void LIRGenerator::visitWasmBoundsCheck(MWasmBoundsCheck* ins) { useRegisterAtStart(index), useRegisterAtStart(boundsCheckLimit)); add(lir, ins); } -#endif } void LIRGenerator::visitWasmAlignmentCheck(MWasmAlignmentCheck* ins) { diff --git a/js/src/jit/WasmBCE.cpp b/js/src/jit/WasmBCE.cpp index 70465a249102..713e9775b697 100644 --- a/js/src/jit/WasmBCE.cpp +++ b/js/src/jit/WasmBCE.cpp @@ -48,11 +48,6 @@ bool jit::EliminateBoundsChecks(MIRGenerator* mir, MIRGraph& graph) { // The payload of the MConstant will be Double if the constant // result is above 2^31-1, but we don't care about that for BCE. -#ifndef WASM_HUGE_MEMORY - MOZ_ASSERT(wasm::MaxMemoryAccessSize < wasm::GuardSize, - "Guard page handles partial out-of-bounds"); -#endif - if (addr->isConstant() && addr->toConstant()->type() == MIRType::Int32 && uint32_t(addr->toConstant()->toInt32()) < diff --git a/js/src/wasm/AsmJS.cpp b/js/src/wasm/AsmJS.cpp index 90c5c3aa5186..b4606a89fc36 100644 --- a/js/src/wasm/AsmJS.cpp +++ b/js/src/wasm/AsmJS.cpp @@ -1355,7 +1355,7 @@ class MOZ_STACK_CLASS JS_HAZ_ROOTED ModuleValidatorShared { arrayViews_(cx), compilerEnv_(CompileMode::Once, Tier::Optimized, OptimizedBackend::Ion, DebugEnabled::False, /* ref types */ false, - /* gc types */ false), + /* gc types */ false, /* huge memory */ false), env_(&compilerEnv_, Shareable::False, ModuleKind::AsmJS) { compilerEnv_.computeParameters(/* gc types */ false); env_.minMemoryLength = RoundUpToNextValidAsmJSHeapLength(0); diff --git a/js/src/wasm/WasmBaselineCompile.cpp b/js/src/wasm/WasmBaselineCompile.cpp index ccbe283c96f7..b40b47012bca 100644 --- a/js/src/wasm/WasmBaselineCompile.cpp +++ b/js/src/wasm/WasmBaselineCompile.cpp @@ -5292,11 +5292,11 @@ class BaseCompiler final : public BaseCompilerInterface { // Ensure no tls if we don't need it. -#ifdef WASM_HUGE_MEMORY - // We have HeapReg and no bounds checking and need load neither - // memoryBase nor boundsCheckLimit from tls. - MOZ_ASSERT_IF(check->omitBoundsCheck, tls.isInvalid()); -#endif + if (env_.hugeMemoryEnabled()) { + // We have HeapReg and no bounds checking and need load neither + // memoryBase nor boundsCheckLimit from tls. + MOZ_ASSERT_IF(check->omitBoundsCheck, tls.isInvalid()); + } #ifdef JS_CODEGEN_ARM // We have HeapReg on ARM and don't need to load the memoryBase from tls. MOZ_ASSERT_IF(check->omitBoundsCheck, tls.isInvalid()); @@ -5304,8 +5304,7 @@ class BaseCompiler final : public BaseCompilerInterface { // Bounds check if required. -#ifndef WASM_HUGE_MEMORY - if (!check->omitBoundsCheck) { + if (!env_.hugeMemoryEnabled() && !check->omitBoundsCheck) { Label ok; masm.wasmBoundsCheck(Assembler::Below, ptr, Address(tls, offsetof(TlsData, boundsCheckLimit)), @@ -5313,7 +5312,6 @@ class BaseCompiler final : public BaseCompilerInterface { masm.wasmTrap(Trap::OutOfBounds, bytecodeOffset()); masm.bind(&ok); } -#endif } #if defined(JS_CODEGEN_X64) || defined(JS_CODEGEN_ARM) || \ @@ -5369,12 +5367,10 @@ class BaseCompiler final : public BaseCompilerInterface { MOZ_MUST_USE bool needTlsForAccess(const AccessCheck& check) { #if defined(JS_CODEGEN_X86) + // x86 requires Tls for memory base return true; -#elif defined(JS_CODEGEN_ARM) || defined(JS_CODEGEN_MIPS32) || \ - defined(JS_CODEGEN_MIPS64) || !defined(WASM_HUGE_MEMORY) - return !check.omitBoundsCheck; #else - return false; + return !env_.hugeMemoryEnabled() && !check.omitBoundsCheck; #endif } diff --git a/js/src/wasm/WasmCode.cpp b/js/src/wasm/WasmCode.cpp index efc0f0455fac..5c357438cb61 100644 --- a/js/src/wasm/WasmCode.cpp +++ b/js/src/wasm/WasmCode.cpp @@ -891,7 +891,8 @@ size_t Metadata::serializedSize() const { return sizeof(pod()) + SerializedVectorSize(funcTypeIds) + SerializedPodVectorSize(globals) + SerializedPodVectorSize(tables) + sizeof(moduleName) + SerializedPodVectorSize(funcNames) + - filename.serializedSize() + sourceMapURL.serializedSize(); + filename.serializedSize() + sourceMapURL.serializedSize() + + sizeof(uint8_t); } uint8_t* Metadata::serialize(uint8_t* cursor) const { @@ -905,10 +906,12 @@ uint8_t* Metadata::serialize(uint8_t* cursor) const { cursor = SerializePodVector(cursor, funcNames); cursor = filename.serialize(cursor); cursor = sourceMapURL.serialize(cursor); + cursor = WriteScalar(cursor, uint8_t(omitsBoundsChecks)); return cursor; } /* static */ const uint8_t* Metadata::deserialize(const uint8_t* cursor) { + uint8_t scalarOmitsBoundsChecks = 0; (cursor = ReadBytes(cursor, &pod(), sizeof(pod()))) && (cursor = DeserializeVector(cursor, &funcTypeIds)) && (cursor = DeserializePodVector(cursor, &globals)) && @@ -916,10 +919,12 @@ uint8_t* Metadata::serialize(uint8_t* cursor) const { (cursor = ReadBytes(cursor, &moduleName, sizeof(moduleName))) && (cursor = DeserializePodVector(cursor, &funcNames)) && (cursor = filename.deserialize(cursor)) && - (cursor = sourceMapURL.deserialize(cursor)); + (cursor = sourceMapURL.deserialize(cursor)) && + (cursor = ReadScalar(cursor, &scalarOmitsBoundsChecks)); debugEnabled = false; debugFuncArgTypes.clear(); debugFuncReturnTypes.clear(); + omitsBoundsChecks = !!scalarOmitsBoundsChecks; return cursor; } diff --git a/js/src/wasm/WasmCode.h b/js/src/wasm/WasmCode.h index 56580c6a07c5..455de66f8d65 100644 --- a/js/src/wasm/WasmCode.h +++ b/js/src/wasm/WasmCode.h @@ -339,6 +339,7 @@ struct Metadata : public ShareableBase, public MetadataCacheablePod { TableDescVector tables; CacheableChars filename; CacheableChars sourceMapURL; + bool omitsBoundsChecks; // namePayload points at the name section's CustomSection::payload so that // the Names (which are use payload-relative offsets) can be used diff --git a/js/src/wasm/WasmCompile.cpp b/js/src/wasm/WasmCompile.cpp index b079bc8733bb..f8b1c3d2086b 100644 --- a/js/src/wasm/WasmCompile.cpp +++ b/js/src/wasm/WasmCompile.cpp @@ -28,6 +28,7 @@ #include "wasm/WasmGenerator.h" #include "wasm/WasmIonCompile.h" #include "wasm/WasmOpIter.h" +#include "wasm/WasmProcess.h" #include "wasm/WasmSignalHandlers.h" #include "wasm/WasmValidate.h" @@ -141,6 +142,7 @@ SharedCompileArgs CompileArgs::build(JSContext* cx, target->sharedMemoryEnabled = sharedMemory; target->forceTiering = forceTiering; target->gcEnabled = gc; + target->hugeMemory = wasm::IsHugeMemoryEnabled(); return target; } @@ -436,14 +438,16 @@ CompilerEnvironment::CompilerEnvironment(CompileMode mode, Tier tier, OptimizedBackend optimizedBackend, DebugEnabled debugEnabled, bool refTypesConfigured, - bool gcTypesConfigured) + bool gcTypesConfigured, + bool hugeMemory) : state_(InitialWithModeTierDebug), mode_(mode), tier_(tier), optimizedBackend_(optimizedBackend), debug_(debugEnabled), refTypes_(refTypesConfigured), - gcTypes_(gcTypesConfigured) {} + gcTypes_(gcTypesConfigured), + hugeMemory_(hugeMemory) {} void CompilerEnvironment::computeParameters(bool gcFeatureOptIn) { MOZ_ASSERT(state_ == InitialWithModeTierDebug); @@ -468,6 +472,7 @@ void CompilerEnvironment::computeParameters(Decoder& d, bool gcFeatureOptIn) { bool debugEnabled = args_->debugEnabled; bool craneliftEnabled = args_->craneliftEnabled; bool forceTiering = args_->forceTiering; + bool hugeMemory = args_->hugeMemory; bool hasSecondTier = ionEnabled || craneliftEnabled; MOZ_ASSERT_IF(gcEnabled || debugEnabled, baselineEnabled); @@ -498,6 +503,7 @@ void CompilerEnvironment::computeParameters(Decoder& d, bool gcFeatureOptIn) { debug_ = debugEnabled ? DebugEnabled::True : DebugEnabled::False; gcTypes_ = gcEnabled; refTypes_ = !craneliftEnabled; + hugeMemory_ = hugeMemory; state_ = Computed; } @@ -604,7 +610,8 @@ void wasm::CompileTier2(const CompileArgs& args, const Bytes& bytecode, CompilerEnvironment compilerEnv(CompileMode::Tier2, Tier::Optimized, optimizedBackend, DebugEnabled::False, - refTypesConfigured, gcTypesConfigured); + refTypesConfigured, gcTypesConfigured, + args.hugeMemory); ModuleEnvironment env(&compilerEnv, args.sharedMemoryEnabled ? Shareable::True diff --git a/js/src/wasm/WasmCompile.h b/js/src/wasm/WasmCompile.h index 71722ff6bea0..10011c60207a 100644 --- a/js/src/wasm/WasmCompile.h +++ b/js/src/wasm/WasmCompile.h @@ -57,6 +57,7 @@ struct CompileArgs : ShareableBase { bool sharedMemoryEnabled; bool forceTiering; bool gcEnabled; + bool hugeMemory; // CompileArgs has two constructors: // @@ -78,7 +79,8 @@ struct CompileArgs : ShareableBase { debugEnabled(false), sharedMemoryEnabled(false), forceTiering(false), - gcEnabled(false) {} + gcEnabled(false), + hugeMemory(false) {} }; // Return the estimated compiled (machine) code size for the given bytecode size diff --git a/js/src/wasm/WasmCraneliftCompile.cpp b/js/src/wasm/WasmCraneliftCompile.cpp index f25520581909..8d054d280a67 100644 --- a/js/src/wasm/WasmCraneliftCompile.cpp +++ b/js/src/wasm/WasmCraneliftCompile.cpp @@ -227,7 +227,17 @@ class AutoCranelift { public: explicit AutoCranelift(const ModuleEnvironment& env) - : env_(env), compiler_(nullptr) {} + : env_(env), compiler_(nullptr) { +#ifdef WASM_SUPPORTS_HUGE_MEMORY + if (env.hugeMemoryEnabled()) { + // In the huge memory configuration, we always reserve the full 4 GB + // index space for a heap. + staticEnv_.staticMemoryBound = HugeIndexRange; + } +#endif + // Otherwise, heap bounds are stored in the `boundsCheckLimit` field + // of TlsData. + } bool init() { compiler_ = cranelift_compiler_create(&staticEnv_, &env_); return !!compiler_; @@ -247,10 +257,8 @@ CraneliftFuncCompileInput::CraneliftFuncCompileInput( index(func.index), offset_in_module(func.lineOrBytecode) {} -#ifndef WASM_HUGE_MEMORY static_assert(offsetof(TlsData, boundsCheckLimit) == sizeof(size_t), "fix make_heap() in wasm2clif.rs"); -#endif CraneliftStaticEnvironment::CraneliftStaticEnvironment() : @@ -280,17 +288,7 @@ CraneliftStaticEnvironment::CraneliftStaticEnvironment() #else platformIsWindows(false), #endif - staticMemoryBound( -#ifdef WASM_HUGE_MEMORY - // In the huge memory configuration, we always reserve the full 4 GB - // index space for a heap. - IndexRange -#else - // Otherwise, heap bounds are stored in the `boundsCheckLimit` field - // of TlsData. - 0 -#endif - ), + staticMemoryBound(0), memoryGuardSize(OffsetGuardLimit), instanceTlsOffset(offsetof(TlsData, instance)), interruptTlsOffset(offsetof(TlsData, interrupt)), diff --git a/js/src/wasm/WasmGenerator.cpp b/js/src/wasm/WasmGenerator.cpp index c825b58c5e36..721cde028351 100644 --- a/js/src/wasm/WasmGenerator.cpp +++ b/js/src/wasm/WasmGenerator.cpp @@ -1075,6 +1075,7 @@ SharedMetadata ModuleGenerator::finishMetadata(const Bytes& bytecode) { metadata_->nameCustomSectionIndex = env_->nameCustomSectionIndex; metadata_->moduleName = env_->moduleName; metadata_->funcNames = std::move(env_->funcNames); + metadata_->omitsBoundsChecks = env_->hugeMemoryEnabled(); // Copy over additional debug information. diff --git a/js/src/wasm/WasmIonCompile.cpp b/js/src/wasm/WasmIonCompile.cpp index 220bc835db4c..d53571b0e3fe 100644 --- a/js/src/wasm/WasmIonCompile.cpp +++ b/js/src/wasm/WasmIonCompile.cpp @@ -575,11 +575,9 @@ class FunctionCompiler { } MWasmLoadTls* maybeLoadBoundsCheckLimit() { -#ifdef WASM_HUGE_MEMORY - if (!env_.isAsmJS()) { + if (env_.hugeMemoryEnabled()) { return nullptr; } -#endif AliasSet aliases = env_.maxMemoryLength.isSome() ? AliasSet::None() : AliasSet::Load(AliasSet::WasmHeapMeta); diff --git a/js/src/wasm/WasmJS.cpp b/js/src/wasm/WasmJS.cpp index f4e32e606ad8..ef2a8928dc7b 100644 --- a/js/src/wasm/WasmJS.cpp +++ b/js/src/wasm/WasmJS.cpp @@ -41,6 +41,7 @@ #include "wasm/WasmInstance.h" #include "wasm/WasmIonCompile.h" #include "wasm/WasmModule.h" +#include "wasm/WasmProcess.h" #include "wasm/WasmSignalHandlers.h" #include "wasm/WasmStubs.h" #include "wasm/WasmValidate.h" @@ -494,6 +495,10 @@ bool wasm::CompileAndSerialize(const ShareableBytes& bytecode, compileArgs->baselineEnabled = false; compileArgs->ionEnabled = true; + // The caller must ensure that huge memory support is configured the same in + // the receiving process of this serialized module. + compileArgs->hugeMemory = wasm::IsHugeMemoryEnabled(); + SerializeListener listener(serialized); UniqueChars error; diff --git a/js/src/wasm/WasmValidate.cpp b/js/src/wasm/WasmValidate.cpp index 564105863caf..4f6134b4e05d 100644 --- a/js/src/wasm/WasmValidate.cpp +++ b/js/src/wasm/WasmValidate.cpp @@ -2901,10 +2901,11 @@ bool wasm::Validate(JSContext* cx, const ShareableBytes& bytecode, bool gcTypesConfigured = HasGcSupport(cx); bool refTypesConfigured = HasReftypesSupport(cx); + bool hugeMemory = false; - CompilerEnvironment compilerEnv(CompileMode::Once, Tier::Optimized, - OptimizedBackend::Ion, DebugEnabled::False, - refTypesConfigured, gcTypesConfigured); + CompilerEnvironment compilerEnv( + CompileMode::Once, Tier::Optimized, OptimizedBackend::Ion, + DebugEnabled::False, refTypesConfigured, gcTypesConfigured, hugeMemory); ModuleEnvironment env( &compilerEnv, cx->realm()->creationOptions().getSharedMemoryAndAtomicsEnabled() diff --git a/js/src/wasm/WasmValidate.h b/js/src/wasm/WasmValidate.h index c5db44b29e16..979a8a25e0b8 100644 --- a/js/src/wasm/WasmValidate.h +++ b/js/src/wasm/WasmValidate.h @@ -71,6 +71,7 @@ struct CompilerEnvironment { DebugEnabled debug_; bool refTypes_; bool gcTypes_; + bool hugeMemory_; }; }; @@ -85,7 +86,7 @@ struct CompilerEnvironment { CompilerEnvironment(CompileMode mode, Tier tier, OptimizedBackend optimizedBackend, DebugEnabled debugEnabled, bool refTypesConfigured, - bool gcTypesConfigured); + bool gcTypesConfigured, bool hugeMemory); // Compute any remaining compilation parameters. void computeParameters(Decoder& d, bool gcFeatureOptIn); @@ -120,6 +121,10 @@ struct CompilerEnvironment { MOZ_ASSERT(isComputed()); return refTypes_; } + bool hugeMemory() const { + MOZ_ASSERT(isComputed()); + return hugeMemory_; + } }; // ModuleEnvironment contains all the state necessary to process or render @@ -210,6 +215,9 @@ struct ModuleEnvironment { bool debugEnabled() const { return compilerEnv->debug() == DebugEnabled::True; } + bool hugeMemoryEnabled() const { + return !isAsmJS() && compilerEnv->hugeMemory(); + } bool funcIsImport(uint32_t funcIndex) const { return funcIndex < funcImportGlobalDataOffsets.length(); }