Bug 1332493 - Initialize wasm::DebugFrame before stack overflow check. r=luke
authorYury Delendik <ydelendik@mozilla.com>
Fri, 20 Jan 2017 12:48:49 -0600
changeset 358474 329a64f144d29a7ba8237c0f4d55414ec2ba546d
parent 358473 b3f4b8aa71e00e282583f9383267c17acc00a108
child 358475 84923aad340241cf2a60dae38a23d2b39f22bd23
push id10621
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 16:02:43 +0000
treeherdermozilla-aurora@dca7b42e6c67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs1332493
milestone53.0a1
Bug 1332493 - Initialize wasm::DebugFrame before stack overflow check. r=luke MozReview-Commit-ID: 3r3PU4TC64P
js/src/jit-test/tests/debug/bug1332493.js
js/src/wasm/WasmBaselineCompile.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/debug/bug1332493.js
@@ -0,0 +1,20 @@
+// |jit-test| test-also-wasm-baseline; exitstatus: 3
+// Checking in debug frame is initialized properly during stack overflow.
+
+if (!wasmIsSupported())
+    quit(3);
+
+var dbg;
+(function () { dbg = new (newGlobal().Debugger)(this); })();
+
+if (!wasmIsSupported())
+     throw "TestComplete";
+
+var m = new WebAssembly.Module(wasmTextToBinary(`(module
+    (import "a" "b" (result f64))
+    ;; function that allocated large space for locals on the stack
+    (func (export "f") ${Array(200).join("(param f64)")} (result f64) (call 0))
+)`));
+var f = () => i.exports.f();
+var i = new WebAssembly.Instance(m, {a: {b: f}});
+f();
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -2071,16 +2071,28 @@ class BaseCompiler
 
         SigIdDesc sigId = env_.funcSigs[func_.index()]->id;
         GenerateFunctionPrologue(masm, localSize_, sigId, &offsets_);
 
         MOZ_ASSERT(masm.framePushed() == uint32_t(localSize_));
 
         maxFramePushed_ = localSize_;
 
+        // The TLS pointer is always passed as a hidden argument in WasmTlsReg.
+        // Save it into its assigned local slot.
+        storeToFramePtr(WasmTlsReg, localInfo_[tlsSlot_].offs());
+        if (debugEnabled_) {
+            // Initialize funcIndex and flag fields of DebugFrame.
+            size_t debugFrame = masm.framePushed() - DebugFrame::offsetOfFrame();
+            masm.store32(Imm32(func_.index()),
+                         Address(masm.getStackPointer(), debugFrame + DebugFrame::offsetOfFuncIndex()));
+            masm.storePtr(ImmWord(0),
+                          Address(masm.getStackPointer(), debugFrame + DebugFrame::offsetOfFlagsWord()));
+        }
+
         // We won't know until after we've generated code how big the frame will
         // be (we may need arbitrary spill slots and outgoing param slots) so
         // emit a patchable add that is patched in endFunction().
         //
         // ScratchReg may be used by branchPtr(), so use ABINonArgReg0 for the
         // effective address.
 
         stackAddOffset_ = masm.add32ToPtrWithPatch(StackPointer, ABINonArgReg0);
@@ -2112,28 +2124,16 @@ class BaseCompiler
                 if (i->argInRegister())
                     storeToFrameF32(i->fpu(), l.offs());
                 break;
               default:
                 MOZ_CRASH("Function argument type");
             }
         }
 
-        // The TLS pointer is always passed as a hidden argument in WasmTlsReg.
-        // Save it into its assigned local slot.
-        storeToFramePtr(WasmTlsReg, localInfo_[tlsSlot_].offs());
-        if (debugEnabled_) {
-            // Initialize funcIndex and flag fields of DebugFrame.
-            size_t debugFrame = masm.framePushed() - DebugFrame::offsetOfFrame();
-            masm.store32(Imm32(func_.index()),
-                         Address(masm.getStackPointer(), debugFrame + DebugFrame::offsetOfFuncIndex()));
-            masm.storePtr(ImmWord(0),
-                          Address(masm.getStackPointer(), debugFrame + DebugFrame::offsetOfFlagsWord()));
-        }
-
         // Initialize the stack locals to zero.
         //
         // The following are all Bug 1316820:
         //
         // TODO / OPTIMIZE: on x64, at least, scratch will be a 64-bit
         // register and we can move 64 bits at a time.
         //
         // TODO / OPTIMIZE: On SSE2 or better SIMD systems we may be
@@ -2209,21 +2209,26 @@ class BaseCompiler
 
         // Patch the add in the prologue so that it checks against the correct
         // frame size.
         MOZ_ASSERT(maxFramePushed_ >= localSize_);
         masm.patchAdd32ToPtr(stackAddOffset_, Imm32(-int32_t(maxFramePushed_ - localSize_)));
 
         // Since we just overflowed the stack, to be on the safe side, pop the
         // stack so that, when the trap exit stub executes, it is a safe
-        // distance away from the end of the native stack.
+        // distance away from the end of the native stack. If debugEnabled_ is
+        // set, we pop all locals space except allocated for DebugFrame to
+        // maintain the invariant that, when debugEnabled_, all wasm::Frames
+        // are valid wasm::DebugFrames which is observable by WasmHandleThrow.
         masm.bind(&stackOverflowLabel_);
-        if (localSize_)
-            masm.addToStackPtr(Imm32(localSize_));
-        masm.jump(TrapDesc(prologueTrapOffset_, Trap::StackOverflow, /* framePushed = */ 0));
+        int32_t debugFrameReserved = debugEnabled_ ? DebugFrame::offsetOfFrame() : 0;
+        MOZ_ASSERT(localSize_ >= debugFrameReserved);
+        if (localSize_ > debugFrameReserved)
+            masm.addToStackPtr(Imm32(localSize_ - debugFrameReserved));
+        masm.jump(TrapDesc(prologueTrapOffset_, Trap::StackOverflow, debugFrameReserved));
 
         masm.bind(&returnLabel_);
 
         if (debugEnabled_) {
             // Store and reload the return value from DebugFrame::return so that
             // it can be clobbered, and/or modified by the debug trap.
             saveResult();
             insertBreakablePoint(CallSiteDesc::LeaveFrame);