REGRESSION (r226068): [X86] Crash in JavaScriptCore ShadowChicken when handling excep...
authormsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Jan 2018 18:44:30 +0000 (18:44 +0000)
committermsaboff@apple.com <msaboff@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 18 Jan 2018 18:44:30 +0000 (18:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181802

Reviewed by Filip Pizlo.

There where a few places where the stack isn't properly aligned for X86 when we call into C++ code.
Two places are where we call into exception handling code, the LLInt and from nativeForGenerator.
The other place was when we call into the operationOSRWriteBarrier().

Added an assert check that the stack is aligned on X86 platforms in the native call tracing code.
This helped find the other cases beyond the original problem.

* dfg/DFGOSRExitCompilerCommon.cpp:
(JSC::DFG::osrWriteBarrier):
* interpreter/FrameTracers.h:
(JSC::assertStackPointerIsAligned):
(JSC::NativeCallFrameTracer::NativeCallFrameTracer):
(JSC::NativeCallFrameTracerWithRestore::NativeCallFrameTracerWithRestore):
* jit/ThunkGenerators.cpp:
(JSC::nativeForGenerator):
* llint/LowLevelInterpreter32_64.asm:

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
Source/JavaScriptCore/interpreter/FrameTracers.h
Source/JavaScriptCore/jit/ThunkGenerators.cpp
Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

index 71edda7..ec5c6d0 100644 (file)
@@ -1,3 +1,27 @@
+2018-01-18  Michael Saboff  <msaboff@apple.com>
+
+        REGRESSION (r226068): [X86] Crash in JavaScriptCore ShadowChicken when handling exceptions
+        https://bugs.webkit.org/show_bug.cgi?id=181802
+
+        Reviewed by Filip Pizlo.
+
+        There where a few places where the stack isn't properly aligned for X86 when we call into C++ code.
+        Two places are where we call into exception handling code, the LLInt and from nativeForGenerator.
+        The other place was when we call into the operationOSRWriteBarrier().
+
+        Added an assert check that the stack is aligned on X86 platforms in the native call tracing code.
+        This helped find the other cases beyond the original problem.
+
+        * dfg/DFGOSRExitCompilerCommon.cpp:
+        (JSC::DFG::osrWriteBarrier):
+        * interpreter/FrameTracers.h:
+        (JSC::assertStackPointerIsAligned):
+        (JSC::NativeCallFrameTracer::NativeCallFrameTracer):
+        (JSC::NativeCallFrameTracerWithRestore::NativeCallFrameTracerWithRestore):
+        * jit/ThunkGenerators.cpp:
+        (JSC::nativeForGenerator):
+        * llint/LowLevelInterpreter32_64.asm:
+
 2018-01-18  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r227096.
index 2151172..5d63048 100644 (file)
@@ -258,7 +258,7 @@ static void osrWriteBarrier(CCallHelpers& jit, GPRReg owner, GPRReg scratch)
 
     // We need these extra slots because setupArgumentsWithExecState will use poke on x86.
 #if CPU(X86)
-    jit.subPtr(MacroAssembler::TrustedImm32(sizeof(void*) * 3), MacroAssembler::stackPointerRegister);
+    jit.subPtr(MacroAssembler::TrustedImm32(sizeof(void*) * 4), MacroAssembler::stackPointerRegister);
 #endif
 
     jit.setupArgumentsWithExecState(owner);
@@ -266,7 +266,7 @@ static void osrWriteBarrier(CCallHelpers& jit, GPRReg owner, GPRReg scratch)
     jit.call(scratch);
 
 #if CPU(X86)
-    jit.addPtr(MacroAssembler::TrustedImm32(sizeof(void*) * 3), MacroAssembler::stackPointerRegister);
+    jit.addPtr(MacroAssembler::TrustedImm32(sizeof(void*) * 4), MacroAssembler::stackPointerRegister);
 #endif
 
     ownerIsRememberedOrInEden.link(&jit);
index 76acd64..0e3f853 100644 (file)
@@ -26,6 +26,7 @@
 #pragma once
 
 #include "CatchScope.h"
+#include "StackAlignment.h"
 #include "VM.h"
 
 namespace JSC {
@@ -68,6 +69,18 @@ private:
     CallFrame* oldCallFrame;
 };
 
+ALWAYS_INLINE static void assertStackPointerIsAligned()
+{
+#ifndef NDEBUG
+#if CPU(X86)
+    uintptr_t stackPointer;
+
+    asm("movl %%esp,%0" : "=r"(stackPointer));
+    ASSERT(!(stackPointer % stackAlignmentBytes()));
+#endif
+#endif
+}
+
 class NativeCallFrameTracer {
 public:
     ALWAYS_INLINE NativeCallFrameTracer(VM* vm, CallFrame* callFrame)
@@ -75,6 +88,7 @@ public:
         ASSERT(vm);
         ASSERT(callFrame);
         ASSERT(reinterpret_cast<void*>(callFrame) < reinterpret_cast<void*>(vm->topEntryFrame));
+        assertStackPointerIsAligned();
         vm->topCallFrame = callFrame;
     }
 };
@@ -86,6 +100,7 @@ public:
     {
         ASSERT(vm);
         ASSERT(callFrame);
+        assertStackPointerIsAligned();
         m_savedTopEntryFrame = vm->topEntryFrame;
         m_savedTopCallFrame = vm->topCallFrame;
         vm->topEntryFrame = EntryFrame;
index a45c3a5..83bd8c4 100644 (file)
@@ -400,7 +400,7 @@ static MacroAssemblerCodeRef nativeForGenerator(VM* vm, ThunkFunctionType thunkF
     jit.storePtr(JSInterfaceJIT::callFrameRegister, &vm->topCallFrame);
 
 #if CPU(X86) && USE(JSVALUE32_64)
-    jit.addPtr(JSInterfaceJIT::TrustedImm32(-12), JSInterfaceJIT::stackPointerRegister);
+    jit.subPtr(JSInterfaceJIT::TrustedImm32(4), JSInterfaceJIT::stackPointerRegister);
     jit.move(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::regT0);
     jit.push(JSInterfaceJIT::regT0);
 #else
@@ -413,7 +413,7 @@ static MacroAssemblerCodeRef nativeForGenerator(VM* vm, ThunkFunctionType thunkF
     jit.move(JSInterfaceJIT::TrustedImmPtr(FunctionPtr(operationVMHandleException).value()), JSInterfaceJIT::regT3);
     jit.call(JSInterfaceJIT::regT3);
 #if CPU(X86) && USE(JSVALUE32_64)
-    jit.addPtr(JSInterfaceJIT::TrustedImm32(16), JSInterfaceJIT::stackPointerRegister);
+    jit.addPtr(JSInterfaceJIT::TrustedImm32(8), JSInterfaceJIT::stackPointerRegister);
 #elif OS(WINDOWS)
     jit.addPtr(JSInterfaceJIT::TrustedImm32(4 * sizeof(int64_t)), JSInterfaceJIT::stackPointerRegister);
 #endif
index 24e6785..5816866 100644 (file)
@@ -2124,6 +2124,9 @@ macro nativeCallTrampoline(executableOffsetToFunction)
     ret
 
 .handleException:
+if X86 or X86_WIN
+    subp 8, sp # align stack pointer
+end
     storep cfr, VM::topCallFrame[t3]
     jmp _llint_throw_from_slow_path_trampoline
 end
@@ -2176,6 +2179,9 @@ macro internalFunctionCallTrampoline(offsetOfFunction)
     ret
 
 .handleException:
+if X86 or X86_WIN
+    subp 8, sp # align stack pointer
+end
     storep cfr, VM::topCallFrame[t3]
     jmp _llint_throw_from_slow_path_trampoline
 end