commit 09f04022c6f22371a4ad9f46f8b919e55330ae4b
parent 67f6bf8eac641ac03c46b0caf25f95a05a1aea70
Author: Ryan Hunt <rhunt@eqrion.net>
Date: Thu, 18 Dec 2025 16:45:29 +0000
Bug 2002625 - wasm: Remove save/restore of allocatable registers in some stubs. r=yury
The debug and request tier-up stubs save and restore all allocatable
registers. This is unnecessary and confusing as baseline does not have
any live registers when these stubs are called. Add an assertion
to that effect and drop this code.
A latent bug was uncovered where ARM64 would dynamically align the stack
in these stubs and store the original unaligned SP on the stack, and then
later pop the unaligned SP back using a `Pop`. This was incorrect because
`Pop` will decrement the MacroAssembler::framePushed(), but the original
push of the unaligned SP didn't actually increase it. This was benign
because we immediately would reset MacroAssembler::framePushed() after
this. But without spilling/restoring the registers we now have
framePushed() == 0, and the Pop triggers an underflow. So we need to
fix this.
Differential Revision: https://phabricator.services.mozilla.com/D274193
Diffstat:
5 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/js/src/wasm/WasmBCClass.h b/js/src/wasm/WasmBCClass.h
@@ -646,6 +646,9 @@ struct BaseCompiler final {
// Count the number of memory references on the value stack.
inline size_t countMemRefsOnStk();
+ // Check if there are any live registers on the value stack.
+ inline bool hasLiveRegsOnStk();
+
// Print the stack to stderr.
void showStack(const char* who) const;
#endif
diff --git a/js/src/wasm/WasmBCStk.h b/js/src/wasm/WasmBCStk.h
@@ -82,6 +82,8 @@ struct Stk {
static const Kind MemLast = MemRef;
static const Kind LocalLast = LocalRef;
+ static const Kind RegFirst = RegisterI32;
+ static const Kind RegLast = RegisterRef;
union {
RegI32 i32reg_;
@@ -168,6 +170,7 @@ struct Stk {
Kind kind() const { return kind_; }
bool isMem() const { return kind_ <= MemLast; }
+ bool isReg() const { return kind_ >= RegFirst && kind_ <= RegLast; }
RegI32 i32reg() const {
MOZ_ASSERT(kind_ == RegisterI32);
diff --git a/js/src/wasm/WasmBCStkMgmt-inl.h b/js/src/wasm/WasmBCStkMgmt-inl.h
@@ -35,6 +35,15 @@ size_t BaseCompiler::countMemRefsOnStk() {
}
return nRefs;
}
+
+bool BaseCompiler::hasLiveRegsOnStk() {
+ for (Stk& v : stk_) {
+ if (v.isReg()) {
+ return true;
+ }
+ }
+ return false;
+}
#endif
template <typename T>
diff --git a/js/src/wasm/WasmBaselineCompile.cpp b/js/src/wasm/WasmBaselineCompile.cpp
@@ -833,6 +833,11 @@ bool BaseCompiler::endFunction() {
void BaseCompiler::insertBreakablePoint(CallSiteKind kind) {
MOZ_ASSERT(!deadCode_);
+
+ // A sync() must happen before this. The debug stub does not save all live
+ // registers.
+ MOZ_ASSERT(!hasLiveRegsOnStk());
+
#ifndef RABALDR_PIN_INSTANCE
fr.loadInstancePtr(InstanceReg);
#endif
@@ -1200,6 +1205,10 @@ Maybe<CodeOffset> BaseCompiler::addHotnessCheck() {
AutoCreatedBy acb(masm, "BC::addHotnessCheck");
+ // A sync() must happen before this. The request tier-up stub does not save
+ // all live registers.
+ MOZ_ASSERT(!hasLiveRegsOnStk());
+
#ifdef RABALDR_PIN_INSTANCE
Register instance(InstanceReg);
#else
@@ -4048,8 +4057,12 @@ bool BaseCompiler::emitLoop() {
}
masm.nopAlign(CodeAlignment);
masm.bind(&controlItem(0).label);
- // The interrupt check barfs if there are live registers.
+
+ // The interrupt check barfs if there are live registers. The hotness check
+ // also can call the request tier-up stub, which assumes that no registers
+ // are live.
sync();
+
if (!addInterruptCheck()) {
return false;
}
@@ -10505,6 +10518,9 @@ bool BaseCompiler::emitBody() {
// TODO sync only registers that can be clobbered by the exit
// prologue/epilogue or disable these registers for use in
// baseline compiler when compilerEnv_.debugEnabled() is set.
+ //
+ // This will require the debug stub to save/restore allocatable
+ // registers.
sync();
insertBreakablePoint(CallSiteKind::Breakpoint);
diff --git a/js/src/wasm/WasmStubs.cpp b/js/src/wasm/WasmStubs.cpp
@@ -2874,10 +2874,6 @@ static bool GenerateThrowStub(MacroAssembler& masm, Label* throwLabel,
return FinishOffsets(masm, offsets);
}
-static const LiveRegisterSet AllAllocatableRegs =
- LiveRegisterSet(GeneralRegisterSet(Registers::AllocatableMask),
- FloatRegisterSet(FloatRegisters::AllMask));
-
// Generate a stub that handles toggleable enter/leave frame traps or
// breakpoints. The stub records the frame pointer (via GenerateExitPrologue)
// and saves most of registers, so as to not affect the code generated by
@@ -2891,9 +2887,6 @@ static bool GenerateDebugStub(MacroAssembler& masm, Label* throwLabel,
GenerateExitPrologue(masm, ExitReason::Fixed::DebugStub,
/*switchToMainStack*/ false, 0, 0, offsets);
- // Save all registers used between baseline compiler operations.
- masm.PushRegsInMask(AllAllocatableRegs);
-
uint32_t framePushed = masm.framePushed();
// This method might be called with unaligned stack -- aligning and
@@ -2921,12 +2914,11 @@ static bool GenerateDebugStub(MacroAssembler& masm, Label* throwLabel,
masm.addToStackPtr(Imm32(ShadowStackSpace));
}
#ifndef JS_CODEGEN_ARM64
- masm.Pop(scratch);
+ masm.pop(scratch);
masm.moveToStackPtr(scratch);
#endif
masm.setFramePushed(framePushed);
- masm.PopRegsInMask(AllAllocatableRegs);
GenerateExitEpilogue(masm, ExitReason::Fixed::DebugStub,
/*switchToMainStack*/ false, offsets);
@@ -2951,9 +2943,6 @@ static bool GenerateRequestTierUpStub(MacroAssembler& masm,
GenerateExitPrologue(masm, ExitReason::Fixed::RequestTierUp,
/*switchToMainStack*/ false, 0, 0, offsets);
- // Save all registers used between baseline compiler operations.
- masm.PushRegsInMask(AllAllocatableRegs);
-
uint32_t framePushed = masm.framePushed();
// This method might be called with unaligned stack -- aligning and
@@ -3007,12 +2996,11 @@ static bool GenerateRequestTierUpStub(MacroAssembler& masm,
masm.addToStackPtr(Imm32(ShadowStackSpace));
}
#ifndef JS_CODEGEN_ARM64
- masm.Pop(scratch);
+ masm.pop(scratch);
masm.moveToStackPtr(scratch);
#endif
masm.setFramePushed(framePushed);
- masm.PopRegsInMask(AllAllocatableRegs);
GenerateExitEpilogue(masm, ExitReason::Fixed::RequestTierUp,
/*switchToMainStack*/ false, offsets);
