From 6abe4661c12ebc182bebcd7b84008bc4feb336e2 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 29 Jul 2016 11:45:15 +0200 Subject: [PATCH] Bug 1290421: Throw ahead of time on statically known out-of-bounds; r=sunfish MozReview-Commit-ID: Bj5MKn9w3vw --HG-- extra : rebase_source : e70fab6c482de3ceffaac876f3e6beee69f778a6 --- js/src/asmjs/WasmIonCompile.cpp | 23 ++++++++++++++---- js/src/jit/CodeGenerator.cpp | 4 ++-- js/src/jit/CodeGenerator.h | 2 +- js/src/jit/Lowering.cpp | 4 ++-- js/src/jit/Lowering.h | 2 +- js/src/jit/MIR.h | 16 ++++++++++--- js/src/jit/MOpcodes.h | 2 +- js/src/jit/arm/CodeGenerator-arm.cpp | 24 ++++--------------- .../mips-shared/CodeGenerator-mips-shared.cpp | 17 +++---------- js/src/jit/shared/LIR-shared.h | 10 +++++--- js/src/jit/shared/LOpcodes-shared.h | 2 +- js/src/jit/x64/CodeGenerator-x64.cpp | 12 ++-------- .../x86-shared/CodeGenerator-x86-shared.cpp | 11 ++++----- js/src/jit/x86/CodeGenerator-x86.cpp | 14 ++--------- 14 files changed, 61 insertions(+), 82 deletions(-) diff --git a/js/src/asmjs/WasmIonCompile.cpp b/js/src/asmjs/WasmIonCompile.cpp index 0339a9c625c9..8740da1b6ccd 100644 --- a/js/src/asmjs/WasmIonCompile.cpp +++ b/js/src/asmjs/WasmIonCompile.cpp @@ -698,6 +698,19 @@ class FunctionCompiler } private: + // False means we're sure to be out-of-bounds after this bounds check. + bool maybeAddBoundsCheck(MDefinition* base, const MWasmMemoryAccess& access) + { + if (access.offset() > uint32_t(INT32_MAX)) { + curBlock_->end(MWasmTrap::New(alloc(), Trap::OutOfBounds)); + curBlock_ = nullptr; + return false; + } + if (!mg().usesSignal.forOOB) + curBlock_->add(MWasmBoundsCheck::New(alloc(), base, access)); + return true; + } + MDefinition* loadHeapPrivate(MDefinition* base, const MWasmMemoryAccess& access, bool isInt64 = false) { @@ -708,8 +721,8 @@ class FunctionCompiler if (mg().isAsmJS()) { load = MAsmJSLoadHeap::New(alloc(), base, access); } else { - if (!mg().usesSignal.forOOB) - curBlock_->add(MWasmBoundsCheck::New(alloc(), base, access)); + if (!maybeAddBoundsCheck(base, access)) + return nullptr; load = MWasmLoad::New(alloc(), base, access, isInt64); } @@ -726,8 +739,8 @@ class FunctionCompiler if (mg().isAsmJS()) { store = MAsmJSStoreHeap::New(alloc(), base, access, v); } else { - if (!mg().usesSignal.forOOB) - curBlock_->add(MWasmBoundsCheck::New(alloc(), base, access)); + if (!maybeAddBoundsCheck(base, access)) + return; store = MWasmStore::New(alloc(), base, access, v); } @@ -1109,7 +1122,7 @@ class FunctionCompiler if (inDeadCode()) return; - auto* ins = MAsmThrowUnreachable::New(alloc()); + auto* ins = MWasmTrap::New(alloc(), wasm::Trap::Unreachable); curBlock_->end(ins); curBlock_ = nullptr; } diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 288736ac5b81..8041f523e39d 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -11520,10 +11520,10 @@ CodeGenerator::visitAsmJSInterruptCheck(LAsmJSInterruptCheck* lir) } void -CodeGenerator::visitAsmThrowUnreachable(LAsmThrowUnreachable* lir) +CodeGenerator::visitWasmTrap(LWasmTrap* lir) { MOZ_ASSERT(gen->compilingAsmJS()); - masm.jump(wasm::JumpTarget::Unreachable); + masm.jump(wasm::JumpTarget(lir->mir()->trap())); } typedef bool (*RecompileFn)(JSContext*); diff --git a/js/src/jit/CodeGenerator.h b/js/src/jit/CodeGenerator.h index 72e598136cad..f4d0ddb7634f 100644 --- a/js/src/jit/CodeGenerator.h +++ b/js/src/jit/CodeGenerator.h @@ -415,7 +415,7 @@ class CodeGenerator final : public CodeGeneratorSpecific void visitInterruptCheck(LInterruptCheck* lir); void visitOutOfLineInterruptCheckImplicit(OutOfLineInterruptCheckImplicit* ins); void visitAsmJSInterruptCheck(LAsmJSInterruptCheck* lir); - void visitAsmThrowUnreachable(LAsmThrowUnreachable* lir); + void visitWasmTrap(LWasmTrap* lir); void visitRecompileCheck(LRecompileCheck* ins); void visitRotate(LRotate* ins); diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index 0fb91e7e74be..55dd27ffa3ea 100644 --- a/js/src/jit/Lowering.cpp +++ b/js/src/jit/Lowering.cpp @@ -2530,9 +2530,9 @@ LIRGenerator::visitAsmJSInterruptCheck(MAsmJSInterruptCheck* ins) } void -LIRGenerator::visitAsmThrowUnreachable(MAsmThrowUnreachable* ins) +LIRGenerator::visitWasmTrap(MWasmTrap* ins) { - add(new(alloc()) LAsmThrowUnreachable, ins); + add(new(alloc()) LWasmTrap, ins); } void diff --git a/js/src/jit/Lowering.h b/js/src/jit/Lowering.h index 9b51bd071a1c..13a54dbbca46 100644 --- a/js/src/jit/Lowering.h +++ b/js/src/jit/Lowering.h @@ -191,7 +191,7 @@ class LIRGenerator : public LIRGeneratorSpecific void visitFunctionEnvironment(MFunctionEnvironment* ins); void visitInterruptCheck(MInterruptCheck* ins); void visitAsmJSInterruptCheck(MAsmJSInterruptCheck* ins); - void visitAsmThrowUnreachable(MAsmThrowUnreachable* ins); + void visitWasmTrap(MWasmTrap* ins); void visitAsmReinterpret(MAsmReinterpret* ins); void visitStoreSlot(MStoreSlot* ins); void visitFilterTypeSet(MFilterTypeSet* ins); diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index d403352789c2..aaaeb2f446ee 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -7738,18 +7738,28 @@ class MAsmJSInterruptCheck TRIVIAL_NEW_WRAPPERS }; -// Directly jumps to the unreachable trap handler. -class MAsmThrowUnreachable +// Directly jumps to the indicated trap, leaving Wasm code and reporting a +// runtime error. + +class MWasmTrap : public MAryControlInstruction<0, 0>, public NoTypePolicy::Data { + wasm::Trap trap_; + + explicit MWasmTrap(wasm::Trap trap) + : trap_(trap) + {} + public: - INSTRUCTION_HEADER(AsmThrowUnreachable) + INSTRUCTION_HEADER(WasmTrap) TRIVIAL_NEW_WRAPPERS AliasSet getAliasSet() const override { return AliasSet::None(); } + + wasm::Trap trap() const { return trap_; } }; // Checks if a value is JS_UNINITIALIZED_LEXICAL, bailout out if so, leaving diff --git a/js/src/jit/MOpcodes.h b/js/src/jit/MOpcodes.h index a48b536c3f0a..ee1468ebb081 100644 --- a/js/src/jit/MOpcodes.h +++ b/js/src/jit/MOpcodes.h @@ -262,7 +262,6 @@ namespace jit { _(CallInstanceOf) \ _(InterruptCheck) \ _(AsmJSInterruptCheck) \ - _(AsmThrowUnreachable) \ _(GetDOMProperty) \ _(GetDOMMember) \ _(SetDOMProperty) \ @@ -274,6 +273,7 @@ namespace jit { _(WasmBoundsCheck) \ _(WasmLoad) \ _(WasmStore) \ + _(WasmTrap) \ _(WasmTruncateToInt32) \ _(AsmJSNeg) \ _(AsmJSUnsignedToDouble) \ diff --git a/js/src/jit/arm/CodeGenerator-arm.cpp b/js/src/jit/arm/CodeGenerator-arm.cpp index e5139128fbab..4fb88fc63062 100644 --- a/js/src/jit/arm/CodeGenerator-arm.cpp +++ b/js/src/jit/arm/CodeGenerator-arm.cpp @@ -2291,11 +2291,7 @@ CodeGeneratorARM::visitWasmBoundsCheck(LWasmBoundsCheck* ins) { MWasmBoundsCheck* mir = ins->mir(); - uint32_t offset = mir->offset(); - if (offset > INT32_MAX) { - masm.as_b(wasm::JumpTarget::OutOfBounds); - return; - } + MOZ_ASSERT(mir->offset() <= INT32_MAX); if (!mir->isRedundant()) { // No guarantee that heapBase + endOffset can be properly encoded in @@ -2348,11 +2344,7 @@ CodeGeneratorARM::emitWasmLoad(T* lir) MOZ_ASSERT(!mir->barrierBefore() && !mir->barrierAfter(), "atomics NYI"); uint32_t offset = mir->offset(); - if (offset > INT32_MAX) { - // This is unreachable because of bounds checks. - masm.breakpoint(); - return; - } + MOZ_ASSERT(offset <= INT32_MAX); Register ptr = ToRegister(lir->ptr()); Scalar::Type type = mir->accessType(); @@ -2419,11 +2411,7 @@ CodeGeneratorARM::emitWasmUnalignedLoad(T* lir) MOZ_ASSERT(!mir->barrierBefore() && !mir->barrierAfter(), "atomics NYI"); uint32_t offset = mir->offset(); - if (offset > INT32_MAX) { - // This is unreachable because of bounds checks. - masm.breakpoint(); - return; - } + MOZ_ASSERT(offset <= INT32_MAX); Register ptr = ToRegister(lir->ptrCopy()); if (offset) @@ -2503,11 +2491,7 @@ CodeGeneratorARM::emitWasmStore(T* lir) MOZ_ASSERT(!mir->barrierBefore() && !mir->barrierAfter(), "atomics NYI"); uint32_t offset = mir->offset(); - if (offset > INT32_MAX) { - // This is unreachable because of bounds checks. - masm.breakpoint(); - return; - } + MOZ_ASSERT(offset <= INT32_MAX); Register ptr = ToRegister(lir->ptr()); unsigned byteSize = mir->byteSize(); diff --git a/js/src/jit/mips-shared/CodeGenerator-mips-shared.cpp b/js/src/jit/mips-shared/CodeGenerator-mips-shared.cpp index 9d04ed9ca987..6eeadbdaa3fc 100644 --- a/js/src/jit/mips-shared/CodeGenerator-mips-shared.cpp +++ b/js/src/jit/mips-shared/CodeGenerator-mips-shared.cpp @@ -1666,10 +1666,7 @@ CodeGeneratorMIPSShared::visitWasmBoundsCheck(LWasmBoundsCheck* ins) MWasmBoundsCheck* mir = ins->mir(); uint32_t offset = mir->offset(); - if (offset > INT32_MAX) { - masm.jump(wasm::JumpTarget::OutOfBounds); - return; - } + MOZ_ASSERT(offset <= INT32_MAX); uint32_t endOffset = mir->endOffset(); Register ptr = ToRegister(ins->ptr()); @@ -1693,11 +1690,7 @@ CodeGeneratorMIPSShared::visitWasmLoad(LWasmLoad* lir) MOZ_ASSERT(!mir->barrierBefore() && !mir->barrierAfter(), "atomics NYI"); uint32_t offset = mir->offset(); - if (offset > INT32_MAX) { - // This is unreachable because of bounds checks. - masm.breakpoint(); - return; - } + MOZ_ASSERT(offset <= INT32_MAX); Register ptr = ToRegister(lir->ptr()); @@ -1745,11 +1738,7 @@ CodeGeneratorMIPSShared::visitWasmStore(LWasmStore* lir) MOZ_ASSERT(!mir->barrierBefore() && !mir->barrierAfter(), "atomics NYI"); uint32_t offset = mir->offset(); - if (offset > INT32_MAX) { - // This is unreachable because of bounds checks. - masm.breakpoint(); - return; - } + MOZ_ASSERT(offset <= INT32_MAX); Register ptr = ToRegister(lir->ptr()); diff --git a/js/src/jit/shared/LIR-shared.h b/js/src/jit/shared/LIR-shared.h index 9f643a258290..d171357bb3a6 100644 --- a/js/src/jit/shared/LIR-shared.h +++ b/js/src/jit/shared/LIR-shared.h @@ -1404,13 +1404,17 @@ class LAsmJSInterruptCheck : public LInstructionHelper<0, 0, 0> } }; -class LAsmThrowUnreachable : public LInstructionHelper<0, 0, 0> +class LWasmTrap : public LInstructionHelper<0, 0, 0> { public: - LIR_HEADER(AsmThrowUnreachable); + LIR_HEADER(WasmTrap); - LAsmThrowUnreachable() + LWasmTrap() { } + + const MWasmTrap* mir() const { + return mir_->toWasmTrap(); + } }; template diff --git a/js/src/jit/shared/LOpcodes-shared.h b/js/src/jit/shared/LOpcodes-shared.h index e59c593ee65a..058e09efda26 100644 --- a/js/src/jit/shared/LOpcodes-shared.h +++ b/js/src/jit/shared/LOpcodes-shared.h @@ -369,7 +369,7 @@ _(CallInstanceOf) \ _(InterruptCheck) \ _(AsmJSInterruptCheck) \ - _(AsmThrowUnreachable) \ + _(WasmTrap) \ _(AsmReinterpret) \ _(AsmReinterpretToI64) \ _(AsmReinterpretFromI64) \ diff --git a/js/src/jit/x64/CodeGenerator-x64.cpp b/js/src/jit/x64/CodeGenerator-x64.cpp index acf69abe21c1..22b6b7851bca 100644 --- a/js/src/jit/x64/CodeGenerator-x64.cpp +++ b/js/src/jit/x64/CodeGenerator-x64.cpp @@ -538,11 +538,7 @@ CodeGeneratorX64::emitWasmLoad(T* ins) Scalar::Type accessType = mir->accessType(); MOZ_ASSERT(!Scalar::isSimdType(accessType), "SIMD NYI"); MOZ_ASSERT(!mir->barrierBefore() && !mir->barrierAfter(), "atomics NYI"); - - if (mir->offset() > INT32_MAX) { - masm.jump(wasm::JumpTarget::OutOfBounds); - return; - } + MOZ_ASSERT(mir->offset() <= INT32_MAX); const LAllocation* ptr = ins->ptr(); Operand srcAddr = ptr->isBogus() @@ -583,11 +579,7 @@ CodeGeneratorX64::emitWasmStore(T* ins) Scalar::Type accessType = mir->accessType(); MOZ_ASSERT(!Scalar::isSimdType(accessType), "SIMD NYI"); MOZ_ASSERT(!mir->barrierBefore() && !mir->barrierAfter(), "atomics NYI"); - - if (mir->offset() > INT32_MAX) { - masm.jump(wasm::JumpTarget::OutOfBounds); - return; - } + MOZ_ASSERT(mir->offset() <= INT32_MAX); const LAllocation* value = ins->getOperand(ins->ValueIndex); const LAllocation* ptr = ins->ptr(); diff --git a/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp b/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp index 5283bc1cdb8f..53243322a2e6 100644 --- a/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp +++ b/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp @@ -494,11 +494,9 @@ void CodeGeneratorX86Shared::visitWasmBoundsCheck(LWasmBoundsCheck* ins) { const MWasmBoundsCheck* mir = ins->mir(); + MOZ_ASSERT(gen->needsBoundsCheckBranch(mir)); - if (mir->offset() > INT32_MAX) { - masm.jump(wasm::JumpTarget::OutOfBounds); - return; - } + MOZ_ASSERT(mir->offset() <= INT32_MAX); Register ptrReg = ToRegister(ins->ptr()); maybeEmitWasmBoundsCheckBranch(mir, ptrReg, mir->isRedundant()); @@ -548,9 +546,8 @@ CodeGeneratorX86Shared::maybeEmitWasmBoundsCheckBranch(const MWasmMemoryAccess* MOZ_ASSERT(mir->endOffset() >= 1, "need to subtract 1 to use JAE, see also AssemblerX86Shared::UpdateBoundsCheck"); - /* - * TODO: See 1287224 Unify MWasmBoundsCheck::redunant_ and needsBoundsCheck - */ + + // TODO: See 1287224 Unify MWasmBoundsCheck::redunant_ and needsBoundsCheck if (!redundant) { uint32_t cmpOffset = masm.cmp32WithPatch(ptr, Imm32(1 - mir->endOffset())).offset(); masm.j(Assembler::AboveOrEqual, wasm::JumpTarget::OutOfBounds); diff --git a/js/src/jit/x86/CodeGenerator-x86.cpp b/js/src/jit/x86/CodeGenerator-x86.cpp index 8358c1d7c8c5..fd4bfd9dd73c 100644 --- a/js/src/jit/x86/CodeGenerator-x86.cpp +++ b/js/src/jit/x86/CodeGenerator-x86.cpp @@ -500,12 +500,7 @@ CodeGeneratorX86::emitWasmLoad(T* ins) Scalar::Type accessType = mir->accessType(); MOZ_ASSERT(!Scalar::isSimdType(accessType), "SIMD NYI"); MOZ_ASSERT(!mir->barrierBefore() && !mir->barrierAfter(), "atomics NYI"); - - if (mir->offset() > INT32_MAX) { - // This is unreachable because of the bounds check. - masm.breakpoint(); - return; - } + MOZ_ASSERT(mir->offset() <= INT32_MAX); const LAllocation* ptr = ins->ptr(); Operand srcAddr = ptr->isBogus() @@ -539,12 +534,7 @@ CodeGeneratorX86::emitWasmStore(T* ins) Scalar::Type accessType = mir->accessType(); MOZ_ASSERT(!Scalar::isSimdType(accessType), "SIMD NYI"); MOZ_ASSERT(!mir->barrierBefore() && !mir->barrierAfter(), "atomics NYI"); - - if (mir->offset() > INT32_MAX) { - // This is unreachable because of the bounds check. - masm.breakpoint(); - return; - } + MOZ_ASSERT(mir->offset() <= INT32_MAX); const LAllocation* ptr = ins->ptr(); Operand dstAddr = ptr->isBogus()