diff --git a/js/src/jit/MacroAssembler.cpp b/js/src/jit/MacroAssembler.cpp index 56441c8a704f..fdebb13dd800 100644 --- a/js/src/jit/MacroAssembler.cpp +++ b/js/src/jit/MacroAssembler.cpp @@ -3292,47 +3292,34 @@ MacroAssembler::wasmEmitOldTrapOutOfLineCode() } } - if (site.trap == wasm::Trap::IndirectCallBadSig) { - // The indirect call bad-signature trap is a special case for two - // reasons: - // - the check happens in the very first instructions of the - // prologue, before the stack frame has been set up which messes - // up everything (stack depth computations, unwinding) - // - the check happens in the callee while the trap should be - // reported at the caller's call_indirect - // To solve both problems at once, the out-of-line path (far) jumps - // directly to the trap exit stub. This takes advantage of the fact - // that there is already a CallSite for call_indirect and the - // current pre-prologue stack/register state. - append(wasm::OldTrapFarJump(site.trap, farJumpWithPatch())); - } else { - // Inherit the frame depth of the trap site. This value is captured - // by the wasm::CallSite to allow unwinding this frame. - setFramePushed(site.framePushed); + MOZ_ASSERT(site.trap != wasm::Trap::IndirectCallBadSig); - // Align the stack for a nullary call. - size_t alreadyPushed = sizeof(wasm::Frame) + framePushed(); - size_t toPush = ABIArgGenerator().stackBytesConsumedSoFar(); - if (size_t dec = StackDecrementForCall(ABIStackAlignment, alreadyPushed, toPush)) - reserveStack(dec); + // Inherit the frame depth of the trap site. This value is captured + // by the wasm::CallSite to allow unwinding this frame. + setFramePushed(site.framePushed); - // To call the trap handler function, we must have the WasmTlsReg - // filled since this is the normal calling ABI. To avoid requiring - // every trapping operation to have the TLS register filled for the - // rare case that it takes a trap, we restore it from the frame on - // the out-of-line path. However, there are millions of out-of-line - // paths (viz. for loads/stores), so the load is factored out into - // the shared FarJumpIsland generated by patchCallSites. + // Align the stack for a nullary call. + size_t alreadyPushed = sizeof(wasm::Frame) + framePushed(); + size_t toPush = ABIArgGenerator().stackBytesConsumedSoFar(); + if (size_t dec = StackDecrementForCall(ABIStackAlignment, alreadyPushed, toPush)) + reserveStack(dec); - // Call the trap's exit, using the bytecode offset of the trap site. - // Note that this code is inside the same CodeRange::Function as the - // trap site so it's as if the trapping instruction called the - // trap-handling function. The frame iterator knows to skip the trap - // exit's frame so that unwinding begins at the frame and offset of - // the trapping instruction. - wasm::CallSiteDesc desc(site.offset, wasm::CallSiteDesc::OldTrapExit); - call(desc, site.trap); - } + // To call the trap handler function, we must have the WasmTlsReg + // filled since this is the normal calling ABI. To avoid requiring + // every trapping operation to have the TLS register filled for the + // rare case that it takes a trap, we restore it from the frame on + // the out-of-line path. However, there are millions of out-of-line + // paths (viz. for loads/stores), so the load is factored out into + // the shared FarJumpIsland generated by patchCallSites. + + // Call the trap's exit, using the bytecode offset of the trap site. + // Note that this code is inside the same CodeRange::Function as the + // trap site so it's as if the trapping instruction called the + // trap-handling function. The frame iterator knows to skip the trap + // exit's frame so that unwinding begins at the frame and offset of + // the trapping instruction. + wasm::CallSiteDesc desc(site.offset, wasm::CallSiteDesc::OldTrapExit); + call(desc, site.trap); #ifdef DEBUG // Traps do not return, so no need to freeStack(). diff --git a/js/src/jit/arm/Simulator-arm.cpp b/js/src/jit/arm/Simulator-arm.cpp index 9feca8f36d00..8835c7b10530 100644 --- a/js/src/jit/arm/Simulator-arm.cpp +++ b/js/src/jit/arm/Simulator-arm.cpp @@ -1646,7 +1646,12 @@ Simulator::handleWasmSegFault(int32_t addr, unsigned numBytes) const wasm::ModuleSegment* moduleSegment = segment->asModule(); wasm::Instance* instance = wasm::LookupFaultingInstance(*moduleSegment, pc, fp); - if (!instance || !instance->memoryAccessInGuardRegion((uint8_t*)addr, numBytes)) + if (!instance) + return false; + + MOZ_RELEASE_ASSERT(&instance->code() == &codeSegment.code()); + + if (!instance->memoryAccessInGuardRegion((uint8_t*)addr, numBytes)) return false; const wasm::MemoryAccess* memoryAccess = instance->code().lookupMemoryAccess(pc); diff --git a/js/src/vm/Stack.cpp b/js/src/vm/Stack.cpp index 6853367a7753..28e995fc8d50 100644 --- a/js/src/vm/Stack.cpp +++ b/js/src/vm/Stack.cpp @@ -1823,13 +1823,27 @@ jit::JitActivation::wasmInterruptResumePC() const } void -jit::JitActivation::startWasmTrap(wasm::Trap trap, uint32_t bytecodeOffset, void* pc, void* fp) +jit::JitActivation::startWasmTrap(wasm::Trap trap, uint32_t bytecodeOffset, + const wasm::RegisterState& state) { - MOZ_ASSERT(pc); - MOZ_ASSERT(fp); + bool unwound; + wasm::UnwindState unwindState; + MOZ_ALWAYS_TRUE(wasm::StartUnwinding(state, &unwindState, &unwound)); + MOZ_ASSERT(unwound == (trap == wasm::Trap::IndirectCallBadSig)); + + void* pc = unwindState.pc; + wasm::Frame* fp = unwindState.fp; + + const wasm::Code& code = fp->tls->instance->code(); + MOZ_RELEASE_ASSERT(&code == wasm::LookupCode(pc)); + + // If the frame was unwound, the bytecodeOffset must be recovered from the + // callsite so that it is accurate. + if (unwound) + bytecodeOffset = code.lookupCallSite(pc)->lineOrBytecode(); cx_->runtime()->wasmUnwindData.ref().construct(pc, trap, bytecodeOffset); - setWasmExitFP((wasm::Frame*)fp); + setWasmExitFP(fp); } void diff --git a/js/src/vm/Stack.h b/js/src/vm/Stack.h index 29afbff01e2f..c7fa3b36fbca 100644 --- a/js/src/vm/Stack.h +++ b/js/src/vm/Stack.h @@ -1667,13 +1667,13 @@ class JitActivation : public Activation // when the interrupt is handled. // Returns true iff we've entered interrupted state. - bool startWasmInterrupt(const JS::ProfilingFrameIterator::RegisterState& state); + bool startWasmInterrupt(const wasm::RegisterState& state); void finishWasmInterrupt(); bool isWasmInterrupted() const; void* wasmInterruptUnwindPC() const; void* wasmInterruptResumePC() const; - void startWasmTrap(wasm::Trap trap, uint32_t bytecodeOffset, void* pc, void* fp); + void startWasmTrap(wasm::Trap trap, uint32_t bytecodeOffset, const wasm::RegisterState& state); void finishWasmTrap(); bool isWasmTrapping() const; void* wasmTrapPC() const; diff --git a/js/src/wasm/WasmFrameIter.cpp b/js/src/wasm/WasmFrameIter.cpp index 05209f4eb115..2f5c82c8f292 100644 --- a/js/src/wasm/WasmFrameIter.cpp +++ b/js/src/wasm/WasmFrameIter.cpp @@ -459,18 +459,26 @@ wasm::GenerateFunctionPrologue(MacroAssembler& masm, uint32_t framePushed, IsLea masm.flushBuffer(); masm.haltingAlign(CodeAlignment); - // Generate table entry: + // The table entry falls through into the normal entry after it has checked + // the signature. + Label normalEntry; + + // Generate table entry. The BytecodeOffset of the trap is fixed up to be + // the bytecode offset of the callsite by JitActivation::startWasmTrap. offsets->begin = masm.currentOffset(); - OldTrapDesc trap(trapOffset, Trap::IndirectCallBadSig, 0); switch (sigId.kind()) { case SigIdDesc::Kind::Global: { Register scratch = WasmTableCallScratchReg; masm.loadWasmGlobalPtr(sigId.globalDataOffset(), scratch); - masm.branchPtr(Assembler::Condition::NotEqual, WasmTableCallSigReg, scratch, trap); + masm.branchPtr(Assembler::Condition::Equal, WasmTableCallSigReg, scratch, + &normalEntry); + masm.wasmTrap(Trap::IndirectCallBadSig, BytecodeOffset(0)); break; } case SigIdDesc::Kind::Immediate: { - masm.branch32(Assembler::Condition::NotEqual, WasmTableCallSigReg, Imm32(sigId.immediate()), trap); + masm.branch32(Assembler::Condition::Equal, WasmTableCallSigReg, Imm32(sigId.immediate()), + &normalEntry); + masm.wasmTrap(Trap::IndirectCallBadSig, BytecodeOffset(0)); break; } case SigIdDesc::Kind::None: @@ -483,6 +491,7 @@ wasm::GenerateFunctionPrologue(MacroAssembler& masm, uint32_t framePushed, IsLea // Generate normal entry: masm.nopAlign(CodeAlignment); + masm.bind(&normalEntry); GenerateCallablePrologue(masm, &offsets->normalEntry); // Tiering works as follows. The Code owns a jumpTable, which has one @@ -1272,20 +1281,21 @@ wasm::LookupFaultingInstance(const ModuleSegment& codeSegment, void* pc, void* f return nullptr; size_t offsetInModule = ((uint8_t*)pc) - codeSegment.base(); - if (offsetInModule < codeRange->funcNormalEntry() + SetFP) - return nullptr; - if (offsetInModule >= codeRange->ret() - PoppedFP && offsetInModule <= codeRange->ret()) + if ((offsetInModule >= codeRange->funcNormalEntry() && + offsetInModule < codeRange->funcNormalEntry() + SetFP) || + (offsetInModule >= codeRange->ret() - PoppedFP && + offsetInModule <= codeRange->ret())) + { return nullptr; + } Instance* instance = reinterpret_cast(fp)->tls->instance; - // TODO: when Trap::IndirectCallBadSig is converted away from being an - // OldTrap, this could become a release assert again. The reason for the - // check is the out-of-line trap stub for the table entry's signature check, - // which executes before fp has been updated. + // TODO: In the special case of a cross-instance indirect call bad-signature + // fault, fp can point to the caller frame which is in a different + // instance/module than pc. This special case should go away when old-style + // traps go away and signal handling is reworked. //MOZ_RELEASE_ASSERT(&instance->code() == &codeSegment.code()); - if (&instance->code() != &codeSegment.code()) - return nullptr; return instance; } diff --git a/js/src/wasm/WasmSignalHandlers.cpp b/js/src/wasm/WasmSignalHandlers.cpp index 87b0e7fac3cd..f528ed524766 100644 --- a/js/src/wasm/WasmSignalHandlers.cpp +++ b/js/src/wasm/WasmSignalHandlers.cpp @@ -1039,11 +1039,13 @@ HandleFault(PEXCEPTION_POINTERS exception) if (!moduleSegment->code().lookupTrap(pc, &trap, &bytecode)) return false; - activation->startWasmTrap(trap, bytecode.offset, pc, ContextToFP(context)); + activation->startWasmTrap(trap, bytecode.offset, ToRegisterState(context)); *ppc = moduleSegment->trapCode(); return true; } + MOZ_RELEASE_ASSERT(&instance->code() == &moduleSegment->code()); + if (record->NumberParameters < 2) return false; @@ -1170,9 +1172,11 @@ HandleMachException(JSContext* cx, const ExceptionRequest& request) if (!moduleSegment->code().lookupTrap(pc, &trap, &bytecode)) return false; - activation->startWasmTrap(trap, bytecode.offset, pc, ContextToFP(&context)); + activation->startWasmTrap(trap, bytecode.offset, ToRegisterState(&context)); *ppc = moduleSegment->trapCode(); } else { + MOZ_RELEASE_ASSERT(&instance->code() == &moduleSegment->code()); + MOZ_ASSERT(request.body.exception == EXC_BAD_ACCESS); if (request.body.codeCnt != 2) return false; @@ -1394,11 +1398,13 @@ HandleFault(int signum, siginfo_t* info, void* ctx) if (!moduleSegment->code().lookupTrap(pc, &trap, &bytecode)) return false; - activation->startWasmTrap(trap, bytecode.offset, pc, ContextToFP(context)); + activation->startWasmTrap(trap, bytecode.offset, ToRegisterState(context)); *ppc = moduleSegment->trapCode(); return true; } + MOZ_RELEASE_ASSERT(&instance->code() == &moduleSegment->code()); + uint8_t* faultingAddress = reinterpret_cast(info->si_addr); // Although it's not strictly necessary, to make sure we're not covering up diff --git a/js/src/wasm/WasmStubs.cpp b/js/src/wasm/WasmStubs.cpp index 98edaf10bc88..deb298645e57 100644 --- a/js/src/wasm/WasmStubs.cpp +++ b/js/src/wasm/WasmStubs.cpp @@ -1730,14 +1730,14 @@ wasm::GenerateStubs(const ModuleEnvironment& env, const FuncImportVector& import case Trap::InvalidConversionToInteger: case Trap::IntegerDivideByZero: case Trap::IndirectCallToNull: + case Trap::IndirectCallBadSig: case Trap::ImpreciseSimdConversion: case Trap::StackOverflow: case Trap::ThrowReported: break; // The TODO list of "old" traps to convert to new traps: case Trap::OutOfBounds: - case Trap::UnalignedAccess: - case Trap::IndirectCallBadSig: { + case Trap::UnalignedAccess: { CallableOffsets offsets; if (!GenerateOldTrapExit(masm, trap, &throwLabel, &offsets)) return false;