Bug 1360248 - Baldr: remove WasmActivation::exitSP_, using unwinding instead (r=bbouvier)

MozReview-Commit-ID: 1vh274E0A5O

--HG--
extra : rebase_source : c23043a8e4bbf0178fb5efa8fc51e59e91907e19

js/src/jit-test/tests/wasm/profiling.js

@@ -192,7 +192,7 @@ testError(
     (func (export "") (call $foo))
 )`,
 WebAssembly.RuntimeError,
-["", ">", "1,>", "0,1,>", "interstitial,0,1,>", "trap handling,0,1,>", ""]);
+["", ">", "1,>", "0,1,>", "interstitial,0,1,>", "trap handling,0,1,>", "", ">", ""]);
 
 testError(
 `(module
@@ -207,7 +207,7 @@ WebAssembly.RuntimeError,
 // Technically we have this one *one-instruction* interval where
 // the caller is lost (the stack with "1,>"). It's annoying to fix and shouldn't
 // mess up profiles in practice so we ignore it.
-["", ">", "0,>", "1,0,>", "1,>", "trap handling,0,>", ""]);
+["", ">", "0,>", "1,0,>", "1,>", "trap handling,0,>", "", ">", ""]);
 
 (function() {
     var e = wasmEvalText(`

js/src/jit/MacroAssembler.h

@@ -464,6 +464,7 @@ class MacroAssembler : public MacroAssemblerSpecific
     void Pop(FloatRegister t) PER_SHARED_ARCH;
     void Pop(const ValueOperand& val) PER_SHARED_ARCH;
     void PopFlags() DEFINED_ON(x86_shared);
+    void PopStackPtr() PER_SHARED_ARCH;
     void popRooted(VMFunction::RootType rootType, Register cellReg, const ValueOperand& valueReg);
 
     // Move the stack pointer based on the requested amount.

js/src/jit/arm/MacroAssembler-arm.cpp

@@ -4980,6 +4980,13 @@ MacroAssembler::Pop(const ValueOperand& val)
     adjustFrame(-sizeof(Value));
 }
 
+void
+MacroAssembler::PopStackPtr()
+{
+    as_dtr(IsLoad, 32, Offset, sp, DTRAddr(sp, DtrOffImm(0)));
+    adjustFrame(-sizeof(intptr_t));
+}
+
 // ===============================================================
 // Simple call functions.
 

js/src/jit/arm64/MacroAssembler-arm64.cpp

@@ -494,6 +494,12 @@ MacroAssembler::Pop(const ValueOperand& val)
     adjustFrame(-1 * int64_t(sizeof(int64_t)));
 }
 
+void
+MacroAssembler::PopStackPtr()
+{
+    MOZ_CRASH("NYI");
+}
+
 // ===============================================================
 // Simple call functions.
 

js/src/jit/mips-shared/MacroAssembler-mips-shared.cpp

@@ -1548,6 +1548,12 @@ MacroAssembler::Pop(const ValueOperand& val)
     framePushed_ -= sizeof(Value);
 }
 
+void
+MacroAssembler::PopStackPtr()
+{
+    MOZ_CRASH("NYI");
+}
+
 
 // ===============================================================
 // Simple call functions.

js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp

@@ -642,6 +642,12 @@ MacroAssembler::PopFlags()
     implicitPop(sizeof(intptr_t));
 }
 
+void
+MacroAssembler::PopStackPtr()
+{
+    Pop(StackPointer);
+}
+
 // ===============================================================
 // Simple call functions.
 

js/src/vm/Stack.cpp

@@ -1644,12 +1644,9 @@ jit::JitActivation::traceIonRecovery(JSTracer* trc)
 
 WasmActivation::WasmActivation(JSContext* cx)
   : Activation(cx, Wasm),
-    entrySP_(nullptr),
     exitFP_(nullptr),
     exitReason_(wasm::ExitReason::Fixed::None)
 {
-    (void) entrySP_;  // silence "unused private member" warning
-
     prevWasm_ = cx->wasmActivationStack_;
     cx->wasmActivationStack_ = this;
 

js/src/vm/Stack.h

@@ -1733,7 +1733,6 @@ class InterpreterFrameIterator
 class WasmActivation : public Activation
 {
     WasmActivation* prevWasm_;
-    void* entrySP_;
     uint8_t* exitFP_;
     wasm::ExitReason exitReason_;
 
@@ -1755,7 +1754,6 @@ class WasmActivation : public Activation
     wasm::ExitReason exitReason() const { return exitReason_; }
 
     // Written by JIT code:
-    static unsigned offsetOfEntrySP() { return offsetof(WasmActivation, entrySP_); }
     static unsigned offsetOfExitFP() { return offsetof(WasmActivation, exitFP_); }
     static unsigned offsetOfExitReason() { return offsetof(WasmActivation, exitReason_); }
 

js/src/wasm/WasmBuiltins.cpp

@@ -147,7 +147,11 @@ WasmHandleDebugTrap()
     return true;
 }
 
-static WasmActivation*
+// Unwind the entire activation in response to a thrown exception. This function
+// is responsible for notifying the debugger of each unwound frame. The return
+// value is the new stack address which the calling stub will set to the sp
+// register before executing a return instruction.
+static void*
 WasmHandleThrow()
 {
     JSContext* cx = TlsContext.get();
@@ -163,10 +167,7 @@ WasmHandleThrow()
     // (which would lead to the frame being re-added to the map of live frames,
     // right as it becomes trash).
     FrameIterator iter(activation, FrameIterator::Unwind::True);
-    if (iter.done()) {
-        MOZ_ASSERT(!activation->interrupted());
-        return activation;
-    }
+    MOZ_ASSERT(!iter.done());
 
     // Live wasm code on the stack is kept alive (in wasm::TraceActivations) by
     // marking the instance of every wasm::Frame found by FrameIterator.
@@ -208,7 +209,7 @@ WasmHandleThrow()
      }
 
     MOZ_ASSERT(!activation->interrupted(), "unwinding clears the interrupt");
-    return activation;
+    return iter.unwoundAddressOfReturnAddress();
 }
 
 static void

js/src/wasm/WasmFrameIterator.cpp

@@ -56,7 +56,8 @@ FrameIterator::FrameIterator()
     callsite_(nullptr),
     codeRange_(nullptr),
     fp_(nullptr),
-    unwind_(Unwind::False)
+    unwind_(Unwind::False),
+    unwoundAddressOfReturnAddress_(nullptr)
 {
     MOZ_ASSERT(done());
 }
@@ -133,22 +134,25 @@ FrameIterator::operator++()
 void
 FrameIterator::popFrame()
 {
-    void* returnAddress = ReturnAddressFromFP(fp_);
-
-    fp_ = CallerFPFromFP(fp_);
+    void* prevFP = fp_;
+    fp_ = CallerFPFromFP(prevFP);
 
     if (!fp_) {
         code_ = nullptr;
         codeRange_ = nullptr;
         callsite_ = nullptr;
 
-        if (unwind_ == Unwind::True)
+        if (unwind_ == Unwind::True) {
             activation_->unwindExitFP(nullptr);
+            unwoundAddressOfReturnAddress_ = &reinterpret_cast<Frame*>(prevFP)->returnAddress;
+        }
 
         MOZ_ASSERT(done());
         return;
     }
 
+    void* returnAddress = ReturnAddressFromFP(prevFP);
+
     code_ = activation_->compartment()->wasm.lookupCode(returnAddress);
     MOZ_ASSERT(code_);
 
@@ -212,6 +216,15 @@ FrameIterator::instance() const
     return FrameToDebugFrame(fp_)->instance();
 }
 
+void**
+FrameIterator::unwoundAddressOfReturnAddress() const
+{
+    MOZ_ASSERT(done());
+    MOZ_ASSERT(unwind_ == Unwind::True);
+    MOZ_ASSERT(unwoundAddressOfReturnAddress_);
+    return unwoundAddressOfReturnAddress_;
+}
+
 bool
 FrameIterator::debugEnabled() const
 {

js/src/wasm/WasmFrameIterator.h

@@ -35,6 +35,7 @@ class Code;
 class CodeRange;
 class DebugFrame;
 class DebugState;
+class Frame;
 class Instance;
 class SigIdDesc;
 struct FuncOffsets;
@@ -60,6 +61,7 @@ class FrameIterator
     const CodeRange* codeRange_;
     uint8_t* fp_;
     Unwind unwind_;
+    void** unwoundAddressOfReturnAddress_;
 
     void popFrame();
 
@@ -75,6 +77,7 @@ class FrameIterator
     unsigned lineOrBytecode() const;
     const CodeRange* codeRange() const { return codeRange_; }
     Instance* instance() const;
+    void** unwoundAddressOfReturnAddress() const;
     bool debugEnabled() const;
     DebugFrame* debugFrame() const;
     const CallSite* debugTrapCallsite() const;

js/src/wasm/WasmStubs.cpp

@@ -178,7 +178,7 @@ SetupABIArguments(MacroAssembler& masm, const FuncExport& fe, Register argv, Reg
 static void
 StoreABIReturn(MacroAssembler& masm, const FuncExport& fe, Register argv)
 {
-    // Store the return value in argv[0]
+    // Store the return value in argv[0].
     switch (fe.sig().ret()) {
       case ExprType::Void:
         break;
@@ -232,18 +232,17 @@ static const LiveRegisterSet NonVolatileRegs =
 #endif
 
 #if defined(JS_CODEGEN_MIPS32)
-// Mips is using one more double slot due to stack alignment for double values.
-// Look at MacroAssembler::PushRegsInMask(RegisterSet set)
-static const unsigned FramePushedAfterSave = NonVolatileRegs.gprs().size() * sizeof(intptr_t) +
-                                             NonVolatileRegs.fpus().getPushSizeInBytes() +
-                                             sizeof(double);
+static const unsigned NonVolatileRegsPushSize = NonVolatileRegs.gprs().size() * sizeof(intptr_t) +
+                                                NonVolatileRegs.fpus().getPushSizeInBytes() +
+                                                sizeof(double);
 #elif defined(JS_CODEGEN_NONE)
-static const unsigned FramePushedAfterSave = 0;
+static const unsigned NonVolatileRegsPushSize = 0;
 #else
-static const unsigned FramePushedAfterSave = NonVolatileRegs.gprs().size() * sizeof(intptr_t)
-                                           + NonVolatileRegs.fpus().getPushSizeInBytes();
+static const unsigned NonVolatileRegsPushSize = NonVolatileRegs.gprs().size() * sizeof(intptr_t) +
+                                                NonVolatileRegs.fpus().getPushSizeInBytes();
 #endif
-static const unsigned FramePushedForEntrySP = FramePushedAfterSave + sizeof(void*);
+static const unsigned FramePushedBeforeAlign = NonVolatileRegsPushSize + sizeof(void*);
+static const unsigned FailFP = 0xbad;
 
 // Generate a stub that enters wasm from a C++ caller via the native ABI. The
 // signature of the entry point is Module::ExportFuncPtr. The exported wasm
@@ -268,10 +267,11 @@ wasm::GenerateEntry(MacroAssembler& masm, const FuncExport& fe)
     // the asm.js callee (which does not preserve non-volatile registers).
     masm.setFramePushed(0);
     masm.PushRegsInMask(NonVolatileRegs);
-    MOZ_ASSERT(masm.framePushed() == FramePushedAfterSave);
+    MOZ_ASSERT(masm.framePushed() == NonVolatileRegsPushSize);
 
     // Put the 'argv' argument into a non-argument/return/TLS register so that
     // we can use 'argv' while we fill in the arguments for the asm.js callee.
+    // Use a second non-argument/return register as temporary scratch.
     Register argv = ABINonArgReturnReg0;
     Register scratch = ABINonArgReturnReg1;
 
@@ -295,63 +295,76 @@ wasm::GenerateEntry(MacroAssembler& masm, const FuncExport& fe)
     else
         masm.loadPtr(Address(masm.getStackPointer(), argBase + arg.offsetFromArgBase()), WasmTlsReg);
 
-    // Setup pinned registers that are assumed throughout wasm code.
-    masm.loadWasmPinnedRegsFromTls();
-
-    // Save 'argv' on the stack so that we can recover it after the call. Use
-    // a second non-argument/return register as temporary scratch.
+    // Save 'argv' on the stack so that we can recover it after the call.
     masm.Push(argv);
 
-    // Save the stack pointer in the WasmActivation right before dynamically
-    // aligning the stack so that it may be recovered on return or throw.
-    MOZ_ASSERT(masm.framePushed() == FramePushedForEntrySP);
-    masm.loadWasmActivationFromTls(scratch);
-    masm.storeStackPtr(Address(scratch, WasmActivation::offsetOfEntrySP()));
+    // Since we're about to dynamically align the stack, reset the frame depth
+    // so we can still assert static stack depth balancing.
+    MOZ_ASSERT(masm.framePushed() == FramePushedBeforeAlign);
+    masm.setFramePushed(0);
 
     // Dynamically align the stack since ABIStackAlignment is not necessarily
-    // WasmStackAlignment. We'll use entrySP to recover the original stack
-    // pointer on return.
+    // WasmStackAlignment. Preserve SP so it can be restored after the call.
+    masm.moveStackPtrTo(scratch);
     masm.andToStackPtr(Imm32(~(WasmStackAlignment - 1)));
+    masm.Push(scratch);
 
-    // Bump the stack for the call.
-    masm.reserveStack(AlignBytes(StackArgBytes(fe.sig().args()), WasmStackAlignment));
+    // Reserve stack space for the call.
+    unsigned argDecrement = StackDecrementForCall(WasmStackAlignment,
+                                                  masm.framePushed(),
+                                                  StackArgBytes(fe.sig().args()));
+    masm.reserveStack(argDecrement);
 
     // Copy parameters out of argv and into the wasm ABI registers/stack-slots.
     SetupABIArguments(masm, fe, argv, scratch);
 
-    // Set the FramePointer to null for the benefit of debugging.
+    // Setup wasm register state. The nullness of the frame pointer is used to
+    // determine whether the call ended in success or failure.
     masm.movePtr(ImmWord(0), FramePointer);
+    masm.loadWasmPinnedRegsFromTls();
 
-    // Call into the real function.
+    // Call into the real function. Note that, due to the throw stub, fp, tls
+    // and pinned registers may be clobbered.
     masm.assertStackAlignment(WasmStackAlignment);
     masm.call(CallSiteDesc(CallSiteDesc::Func), fe.funcIndex());
     masm.assertStackAlignment(WasmStackAlignment);
 
-#ifdef DEBUG
-    // Assert FramePointer was returned to null by the callee.
-    Label ok;
-    masm.branchTestPtr(Assembler::Zero, FramePointer, FramePointer, &ok);
-    masm.breakpoint();
-    masm.bind(&ok);
-#endif
+    // Pop the arguments pushed after the dynamic alignment.
+    masm.freeStack(argDecrement);
 
-    // Recover the stack pointer value before dynamic alignment.
-    masm.loadWasmActivationFromTls(scratch);
-    masm.wasmAssertNonExitInvariants(scratch);
-    masm.loadStackPtr(Address(scratch, WasmActivation::offsetOfEntrySP()));
-    masm.setFramePushed(FramePushedForEntrySP);
+    // Pop the stack pointer to its value right before dynamic alignment.
+    masm.PopStackPtr();
+    MOZ_ASSERT(masm.framePushed() == 0);
+    masm.setFramePushed(FramePushedBeforeAlign);
 
     // Recover the 'argv' pointer which was saved before aligning the stack.
     masm.Pop(argv);
 
-    // Store the return value into argv.
+    // Store the return value in argv[0].
     StoreABIReturn(masm, fe, argv);
 
+    // After the ReturnReg is stored into argv[0] but before fp is clobbered by
+    // the PopRegsInMask(NonVolatileRegs) below, set the return value based on
+    // whether fp is null (which is the case for successful returns) or the
+    // FailFP magic value (set by the throw stub);
+    Label success, join;
+    masm.branchTestPtr(Assembler::Zero, FramePointer, FramePointer, &success);
+#ifdef DEBUG
+    Label ok;
+    masm.branchPtr(Assembler::Equal, FramePointer, Imm32(FailFP), &ok);
+    masm.breakpoint();
+    masm.bind(&ok);
+#endif
+    masm.move32(Imm32(false), ReturnReg);
+    masm.jump(&join);
+    masm.bind(&success);
+    masm.move32(Imm32(true), ReturnReg);
+    masm.bind(&join);
+
     // Restore clobbered non-volatile registers of the caller.
     masm.PopRegsInMask(NonVolatileRegs);
     MOZ_ASSERT(masm.framePushed() == 0);
 
-    masm.move32(Imm32(true), ReturnReg);
     masm.ret();
 
     FinishOffsets(masm, &offsets);
@@ -1240,23 +1253,19 @@ wasm::GenerateThrowStub(MacroAssembler& masm, Label* throwLabel)
     Offsets offsets;
     offsets.begin = masm.currentOffset();
 
-    // The following HandleThrow call sets fp of this WasmActivation to null.
+    // The throw stub can be jumped to from an async interrupt that is halting
+    // execution. Thus the stack pointer can be unaligned and we must align it
+    // dynamically.
     masm.andToStackPtr(Imm32(~(ABIStackAlignment - 1)));
     if (ShadowStackSpace)
         masm.subFromStackPtr(Imm32(ShadowStackSpace));
+
+    // WasmHandleThrow unwinds WasmActivation::exitFP and returns the address of
+    // the return address on the stack this stub should return to. Set the
+    // FramePointer to a magic value to indicate a return by throw.
     masm.call(SymbolicAddress::HandleThrow);
-
-    // HandleThrow returns the innermost WasmActivation* in ReturnReg.
-    Register act = ReturnReg;
-    masm.wasmAssertNonExitInvariants(act);
-
-    masm.setFramePushed(FramePushedForEntrySP);
-    masm.loadStackPtr(Address(act, WasmActivation::offsetOfEntrySP()));
-    masm.Pop(ReturnReg);
-    masm.PopRegsInMask(NonVolatileRegs);
-    MOZ_ASSERT(masm.framePushed() == 0);
-
-    masm.mov(ImmWord(0), ReturnReg);
+    masm.moveToStackPtr(ReturnReg);
+    masm.move32(Imm32(FailFP), FramePointer);
     masm.ret();
 
     FinishOffsets(masm, &offsets);