We overwrite the callee save space on the stack when throwing stack overflow from...
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 21 May 2017 21:29:57 +0000 (21:29 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 21 May 2017 21:29:57 +0000 (21:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172316

Reviewed by Mark Lam.

When throwing a stack overflow exception, the overflow
thunk would do the following:
  move fp, sp
  populate argument registers
  call C code

However, the C function is allowed to clobber our spilled
callee saves that live below fp. The reason I did this move is that
when we jump to this code, we've proven that sp is out of bounds on
the stack. So we're not allowed to just use its value or keep growing
the stack from that point. However, this patch revises this approach
to be the same in spirit, but actually correct. We conservatively assume
the B3 function we're coming from could have saved all callee saves.
So we emit code like this now:
  add -maxNumCalleeSaveSpace, fp, sp
  populate argument registers
  call C code

This ensures our callee saves will not be overwritten. Note
that fp is still in a valid stack range here, since the thing
calling the wasm code did a stack check. Also note that maxNumCalleeSaveSpace
is less than our redzone size, so it's safe to decrement sp by
this amount.

The previously added wasm stack overflow test is an instance crash
without this change on arm64. It also appears that this test crashed
on some other x86 devices.

* wasm/WasmThunks.cpp:
(JSC::Wasm::throwStackOverflowFromWasmThunkGenerator):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@217198 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/wasm/WasmThunks.cpp

index 4d4a98b..f4e4343 100644 (file)
@@ -1,3 +1,41 @@
+2017-05-21  Saam Barati  <sbarati@apple.com>
+
+        We overwrite the callee save space on the stack when throwing stack overflow from wasm
+        https://bugs.webkit.org/show_bug.cgi?id=172316
+
+        Reviewed by Mark Lam.
+
+        When throwing a stack overflow exception, the overflow
+        thunk would do the following:
+          move fp, sp
+          populate argument registers
+          call C code
+        
+        However, the C function is allowed to clobber our spilled
+        callee saves that live below fp. The reason I did this move is that
+        when we jump to this code, we've proven that sp is out of bounds on
+        the stack. So we're not allowed to just use its value or keep growing
+        the stack from that point. However, this patch revises this approach
+        to be the same in spirit, but actually correct. We conservatively assume
+        the B3 function we're coming from could have saved all callee saves.
+        So we emit code like this now:
+          add -maxNumCalleeSaveSpace, fp, sp
+          populate argument registers
+          call C code
+        
+        This ensures our callee saves will not be overwritten. Note
+        that fp is still in a valid stack range here, since the thing
+        calling the wasm code did a stack check. Also note that maxNumCalleeSaveSpace
+        is less than our redzone size, so it's safe to decrement sp by 
+        this amount.
+        
+        The previously added wasm stack overflow test is an instance crash
+        without this change on arm64. It also appears that this test crashed
+        on some other x86 devices.
+
+        * wasm/WasmThunks.cpp:
+        (JSC::Wasm::throwStackOverflowFromWasmThunkGenerator):
+
 2017-05-20  Chris Dumez  <cdumez@apple.com>
 
         Drop [NoInterfaceObject] from RTCDTMFSender and RTCStatsReport
index 17e8ef4..0ef4919 100644 (file)
@@ -93,7 +93,9 @@ MacroAssemblerCodeRef throwStackOverflowFromWasmThunkGenerator(const AbstractLoc
 {
     CCallHelpers jit;
 
-    jit.move(GPRInfo::callFrameRegister, MacroAssembler::stackPointerRegister);
+    int32_t stackSpace = WTF::roundUpToMultipleOf(stackAlignmentBytes(), RegisterSet::calleeSaveRegisters().numberOfSetRegisters() * sizeof(Register));
+    ASSERT(static_cast<unsigned>(stackSpace) < Options::softReservedZoneSize());
+    jit.addPtr(CCallHelpers::TrustedImm32(-stackSpace), GPRInfo::callFrameRegister, MacroAssembler::stackPointerRegister);
     jit.move(CCallHelpers::TrustedImm32(static_cast<uint32_t>(ExceptionType::StackOverflow)), GPRInfo::argumentGPR1);
     auto jumpToExceptionHandler = jit.jump();
     LinkBuffer linkBuffer(jit, GLOBAL_THUNK_ID);