Attachment 342976 Details for Bug 186540 – Patch

[patch] Patch

bug-186540-20180619001549.patch (text/plain), 16.38 KB, created by Tadeu Zagallo on 2018-06-18 15:15:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2018-06-18 15:15:51 PDT

Size: 16.38 KB

patch

obsolete

>Subversion Revision: 232892
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 8269510ccccf9e87a1b566b5a455d82f6e5bd852..9e4e861e938dd3603e2281bc04ca1b433b37047a 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,37 @@
>+2018-06-18  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        ShadowChicken crashes with stack overflow in the LLInt
>+        https://bugs.webkit.org/show_bug.cgi?id=186540
>+        <rdar://problem/39682133>
>+
>+        Reviewed by Saam Barati.
>+
>+        Stack overflows in the LLInt were being handled with the incomplete
>+        frame as the top frame, causing ShadowChicken to crash while unwinding
>+        the stack. Introduce a new opcode that handles the unwinding with the
>+        caller frame.
>+
>+        * bytecode/BytecodeList.json:
>+        * dfg/DFGCapabilities.cpp:
>+        (JSC::DFG::capabilityLevel):
>+        * interpreter/Interpreter.cpp:
>+        (JSC::Interpreter::unwind):
>+        * interpreter/Interpreter.h:
>+        * jit/JITExceptions.cpp:
>+        (JSC::genericUnwind):
>+        * jit/JITOperations.cpp:
>+        * llint/LLIntData.cpp:
>+        (JSC::LLInt::initialize):
>+        * llint/LLIntData.h:
>+        (JSC::LLInt::callerExceptionInstructions):
>+        * llint/LLIntSlowPaths.cpp:
>+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
>+        * llint/LLIntSlowPaths.h:
>+        * llint/LowLevelInterpreter.cpp:
>+        (JSC::CLoop::execute):
>+        * llint/LowLevelInterpreter32_64.asm:
>+        * llint/LowLevelInterpreter64.asm:
>+
> 2018-06-15  Saam Barati  <sbarati@apple.com>
> 
>         Make ForceOSRExit CFG pruning in bytecode parser more aggressive by making the original block to ignore be the plan's osrEntryBytecodeIndex
>diff --git a/Source/JavaScriptCore/bytecode/BytecodeList.json b/Source/JavaScriptCore/bytecode/BytecodeList.json
>index f5bdc49a7a671b8de9cb348b7ebba936748b2f42..60af7a5ea36bdde141a6073cab0b92f2eef55d85 100644
>--- a/Source/JavaScriptCore/bytecode/BytecodeList.json
>+++ b/Source/JavaScriptCore/bytecode/BytecodeList.json
>@@ -225,6 +225,7 @@
>             { "name" : "llint_function_for_construct_arity_check" },
>             { "name" : "llint_generic_return_point" },
>             { "name" : "llint_throw_from_slow_path_trampoline" },
>+            { "name" : "llint_throw_from_slow_path_caller_exception" },
>             { "name" : "llint_throw_during_call_trampoline" },
>             { "name" : "llint_native_call_trampoline" },
>             { "name" : "llint_native_construct_trampoline" },
>diff --git a/Source/JavaScriptCore/dfg/DFGCapabilities.cpp b/Source/JavaScriptCore/dfg/DFGCapabilities.cpp
>index dadc92d867e4c44a3462d8d6dab6458075acdd75..ce0aaaa91ecedb8cdb84846e7dfacee920d279ea 100644
>--- a/Source/JavaScriptCore/dfg/DFGCapabilities.cpp
>+++ b/Source/JavaScriptCore/dfg/DFGCapabilities.cpp
>@@ -289,6 +289,7 @@ CapabilityLevel capabilityLevel(OpcodeID opcodeID, CodeBlock* codeBlock, Instruc
>     case llint_function_for_construct_arity_check:
>     case llint_generic_return_point:
>     case llint_throw_from_slow_path_trampoline:
>+    case llint_throw_from_slow_path_caller_exception:
>     case llint_throw_during_call_trampoline:
>     case llint_native_call_trampoline:
>     case llint_native_construct_trampoline:
>diff --git a/Source/JavaScriptCore/interpreter/Interpreter.cpp b/Source/JavaScriptCore/interpreter/Interpreter.cpp
>index a96b75618a2019dcde6595b650664666057a802c..b549916adb862b4ecc72dad81611be5845e5e59e 100644
>--- a/Source/JavaScriptCore/interpreter/Interpreter.cpp
>+++ b/Source/JavaScriptCore/interpreter/Interpreter.cpp
>@@ -718,17 +718,12 @@ private:
>     HandlerInfo*& m_handler;
> };
> 
>-NEVER_INLINE HandlerInfo* Interpreter::unwind(VM& vm, CallFrame*& callFrame, Exception* exception, UnwindStart unwindStart)
>+NEVER_INLINE HandlerInfo* Interpreter::unwind(VM& vm, CallFrame*& callFrame, Exception* exception)
> {
>-    auto scope = DECLARE_CATCH_SCOPE(vm);
>-
>-    if (unwindStart == UnwindFromCallerFrame) {
>-        if (callFrame->callerFrameOrEntryFrame() == vm.topEntryFrame)
>-            return nullptr;
>+    if ((void *)callFrame == vm.topEntryFrame)
>+        return nullptr;
> 
>-        callFrame = callFrame->callerFrame();
>-        vm.topCallFrame = callFrame;
>-    }
>+    auto scope = DECLARE_CATCH_SCOPE(vm);
> 
>     CodeBlock* codeBlock = callFrame->codeBlock();
> 
>diff --git a/Source/JavaScriptCore/interpreter/Interpreter.h b/Source/JavaScriptCore/interpreter/Interpreter.h
>index 49227ebe515663ffde03c9e0a3fcb64967a0f568..6db68aab1d009f8589dc34026feeafbd0e5edb01 100644
>--- a/Source/JavaScriptCore/interpreter/Interpreter.h
>+++ b/Source/JavaScriptCore/interpreter/Interpreter.h
>@@ -117,7 +117,7 @@ namespace JSC {
> 
>         void getArgumentsData(CallFrame*, JSFunction*&, ptrdiff_t& firstParameterIndex, Register*& argv, int& argc);
>         
>-        NEVER_INLINE HandlerInfo* unwind(VM&, CallFrame*&, Exception*, UnwindStart);
>+        NEVER_INLINE HandlerInfo* unwind(VM&, CallFrame*&, Exception*);
>         void notifyDebuggerOfExceptionToBeThrown(VM&, CallFrame*, Exception*);
>         NEVER_INLINE void debug(CallFrame*, DebugHookType);
>         static String stackTraceAsString(VM&, const Vector<StackFrame>&);
>diff --git a/Source/JavaScriptCore/jit/JITExceptions.cpp b/Source/JavaScriptCore/jit/JITExceptions.cpp
>index 055945f795217c1aff7c259edf2f1f89ad239c76..36ef606eeb93586c57c11df2008c247b7176b30b 100644
>--- a/Source/JavaScriptCore/jit/JITExceptions.cpp
>+++ b/Source/JavaScriptCore/jit/JITExceptions.cpp
>@@ -42,8 +42,17 @@
> 
> namespace JSC {
> 
>-void genericUnwind(VM* vm, ExecState* callFrame, UnwindStart unwindStart)
>+void genericUnwind(VM* vm, ExecState* exec, UnwindStart unwindStart)
> {
>+    CallFrame* callFrame = exec;
>+    if (unwindStart == UnwindFromCallerFrame) {
>+        EntryFrame* topEntryFrame = vm->topEntryFrame;
>+        callFrame = callFrame->callerFrame(topEntryFrame);
>+        if (!callFrame)
>+            callFrame = exec;
>+        vm->topCallFrame = callFrame;
>+    }
>+
>     auto scope = DECLARE_CATCH_SCOPE(*vm);
>     if (Options::breakOnThrow()) {
>         CodeBlock* codeBlock = callFrame->codeBlock();
>@@ -54,16 +63,11 @@ void genericUnwind(VM* vm, ExecState* callFrame, UnwindStart unwindStart)
>         CRASH();
>     }
>     
>-    ExecState* shadowChickenTopFrame = callFrame;
>-    if (unwindStart == UnwindFromCallerFrame) {
>-        EntryFrame* topEntryFrame = vm->topEntryFrame;
>-        shadowChickenTopFrame = callFrame->callerFrame(topEntryFrame);
>-    }
>-    vm->shadowChicken().log(*vm, shadowChickenTopFrame, ShadowChicken::Packet::throwPacket());
>+    vm->shadowChicken().log(*vm, callFrame, ShadowChicken::Packet::throwPacket());
>     
>     Exception* exception = scope.exception();
>     RELEASE_ASSERT(exception);
>-    HandlerInfo* handler = vm->interpreter->unwind(*vm, callFrame, exception, unwindStart); // This may update callFrame.
>+    HandlerInfo* handler = vm->interpreter->unwind(*vm, callFrame, exception);
> 
>     void* catchRoutine;
>     Instruction* catchPCForInterpreter = 0;
>diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
>index 5381dd6d04e69a9240ace66db2ee4e3bfaf36bbc..80396da746907a15379b25035fe69428c488b0d7 100644
>--- a/Source/JavaScriptCore/jit/JITOperations.cpp
>+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
>@@ -2471,7 +2471,6 @@ void JIT_OPERATION lookupExceptionHandler(VM* vm, ExecState* exec)
> 
> void JIT_OPERATION lookupExceptionHandlerFromCallerFrame(VM* vm, ExecState* exec)
> {
>-    vm->topCallFrame = exec->callerFrame();
>     genericUnwind(vm, exec, UnwindFromCallerFrame);
>     ASSERT(vm->targetMachinePCForThrow);
> }
>diff --git a/Source/JavaScriptCore/llint/LLIntData.cpp b/Source/JavaScriptCore/llint/LLIntData.cpp
>index c93d15c74c58ff5d96aa0aad943413f085a3d92e..67f77c928f617c8f179c953594b0e2e5bb07f02e 100644
>--- a/Source/JavaScriptCore/llint/LLIntData.cpp
>+++ b/Source/JavaScriptCore/llint/LLIntData.cpp
>@@ -44,6 +44,7 @@
> namespace JSC { namespace LLInt {
> 
> Instruction Data::s_exceptionInstructions[maxOpcodeLength + 1] = { };
>+Instruction Data::s_callerExceptionInstructions[maxOpcodeLength + 1] = { };
> Opcode Data::s_opcodeMap[numOpcodeIDs] = { };
> 
> #if ENABLE(JIT)
>@@ -62,8 +63,11 @@ void initialize()
>         Data::s_opcodeMap[i] = tagCodePtr(Data::s_opcodeMap[i], BytecodePtrTag);
> 
>     void* handler = Data::s_opcodeMap[llint_throw_from_slow_path_trampoline];
>-    for (int i = 0; i < maxOpcodeLength + 1; ++i)
>+    void* callerHandler = Data::s_opcodeMap[llint_throw_from_slow_path_caller_exception];
>+    for (int i = 0; i < maxOpcodeLength + 1; ++i) {
>         Data::s_exceptionInstructions[i].u.pointer = handler;
>+        Data::s_callerExceptionInstructions[i].u.pointer = callerHandler;
>+    }
> #endif // ENABLE(JIT)
> }
> 
>diff --git a/Source/JavaScriptCore/llint/LLIntData.h b/Source/JavaScriptCore/llint/LLIntData.h
>index be58c00ae5c66ac30581ae3d4849428e5bb301d0..e3debd7708eacc279bf76f0784b6ddfe5ea2737e 100644
>--- a/Source/JavaScriptCore/llint/LLIntData.h
>+++ b/Source/JavaScriptCore/llint/LLIntData.h
>@@ -48,11 +48,13 @@ public:
> 
> private:
>     static Instruction s_exceptionInstructions[maxOpcodeLength + 1];
>+    static Instruction s_callerExceptionInstructions[maxOpcodeLength + 1];
>     static Opcode s_opcodeMap[numOpcodeIDs];
> 
>     friend void initialize();
> 
>     friend Instruction* exceptionInstructions();
>+    friend Instruction* callerExceptionInstructions();
>     friend Opcode* opcodeMap();
>     friend Opcode getOpcode(OpcodeID);
>     template<PtrTag tag> friend MacroAssemblerCodePtr<tag> getCodePtr(OpcodeID);
>@@ -65,6 +67,11 @@ inline Instruction* exceptionInstructions()
> {
>     return Data::s_exceptionInstructions;
> }
>+
>+inline Instruction* callerExceptionInstructions()
>+{
>+    return Data::s_callerExceptionInstructions;
>+}
>     
> inline Opcode* opcodeMap()
> {
>diff --git a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>index c8d3e4714cefd9249feb9885f05fcf68b695e520..6954ec72c9c5e7ddbd52a1ed252e640972ac4b55 100644
>--- a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>+++ b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>@@ -525,7 +525,7 @@ LLINT_SLOW_PATH_DECL(stack_check)
> 
>     ErrorHandlingScope errorScope(vm);
>     throwStackOverflowError(callerFrame, throwScope);
>-    pc = returnToThrow(callerFrame);
>+    pc = LLInt::callerExceptionInstructions();
>     LLINT_RETURN_TWO(pc, exec);
> }
> 
>@@ -1685,6 +1685,14 @@ LLINT_SLOW_PATH_DECL(slow_path_handle_exception)
>     LLINT_END_IMPL();
> }
> 
>+LLINT_SLOW_PATH_DECL(slow_path_handle_exception_from_caller)
>+{
>+    LLINT_BEGIN_NO_SET_PC();
>+    UNUSED_PARAM(throwScope);
>+    genericUnwind(&vm, exec, UnwindFromCallerFrame);
>+    LLINT_END_IMPL();
>+}
>+
> LLINT_SLOW_PATH_DECL(slow_path_get_from_scope)
> {
>     LLINT_BEGIN();
>diff --git a/Source/JavaScriptCore/llint/LLIntSlowPaths.h b/Source/JavaScriptCore/llint/LLIntSlowPaths.h
>index 104a522602d9515cb296669dfb8fc60fdd9f46b0..b76e682f871cacaa2c48454945d3ae185e7cf270 100644
>--- a/Source/JavaScriptCore/llint/LLIntSlowPaths.h
>+++ b/Source/JavaScriptCore/llint/LLIntSlowPaths.h
>@@ -124,6 +124,7 @@ LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_throw);
> LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_handle_traps);
> LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_debug);
> LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_handle_exception);
>+LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_handle_exception_from_caller);
> LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_get_from_scope);
> LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_put_to_scope);
> LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_check_if_exception_is_uncatchable_and_notify_profiler);
>diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp b/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp
>index 78bff0884c4802939a4de860f76b582eaa9a4265..bc5071d161d2424a66e19cbf9ff5bc19e6f58578 100644
>--- a/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp
>+++ b/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp
>@@ -287,9 +287,13 @@ JSValue CLoop::execute(OpcodeID entryOpcodeID, void* executableAddress, VM* vm,
>         // initialized the opcodeMap above. This is because getCodePtr()
>         // can depend on the opcodeMap.
>         Instruction* exceptionInstructions = LLInt::exceptionInstructions();
>-        for (int i = 0; i < maxOpcodeLength + 1; ++i)
>+        Instruction* callerExceptionInstructions = LLInt::callerExceptionInstructions();
>+        for (int i = 0; i < maxOpcodeLength + 1; ++i) {
>             exceptionInstructions[i].u.pointer =
>                 LLInt::getCodePtr(llint_throw_from_slow_path_trampoline);
>+            callerExceptionInstructions[i].u.pointer =
>+                LLInt::getCodePtr(llint_throw_from_slow_path_caller_exception);
>+        }
> 
>         return JSValue();
>     }
>diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm b/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
>index 80f41d804a6dfa0d9124c94ec41dd11061e06489..54fb6cef8f528e6c8c59f16139a4fe5edd2b5a35 100644
>--- a/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
>+++ b/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm
>@@ -2097,8 +2097,8 @@ _llint_op_end:
>     doReturn()
> 
> 
>-_llint_throw_from_slow_path_trampoline:
>-    callSlowPath(_llint_slow_path_handle_exception)
>+macro llintThrowFromSlowPath(slowPath)
>+    callSlowPath(slowPath)
> 
>     # When throwing from the interpreter (i.e. throwing from LLIntSlowPaths), so
>     # the throw target is not necessarily interpreted code, we come to here.
>@@ -2108,6 +2108,14 @@ _llint_throw_from_slow_path_trampoline:
>     loadp MarkedBlockFooterOffset + MarkedBlock::Footer::m_vm[t1], t1
>     copyCalleeSavesToVMEntryFrameCalleeSavesBuffer(t1, t2)
>     jmp VM::targetMachinePCForThrow[t1]
>+end
>+
>+
>+_llint_throw_from_slow_path_trampoline:
>+    llintThrowFromSlowPath(_llint_slow_path_handle_exception)
>+
>+_llint_throw_from_slow_path_caller_exception:
>+    llintThrowFromSlowPath(_llint_slow_path_handle_exception_from_caller)
> 
> 
> _llint_throw_during_call_trampoline:
>diff --git a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm b/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
>index f867597fc46e531a385561f271b871a98422bab0..1eedf6a426106ff6728c25f6890c3500595cc1e9 100644
>--- a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
>+++ b/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
>@@ -2147,13 +2147,13 @@ _llint_op_end:
>     doReturn()
> 
> 
>-_llint_throw_from_slow_path_trampoline:
>+macro llintThrowFromSlowPath(slowPath)
>     loadp Callee[cfr], t1
>     andp MarkedBlockMask, t1
>     loadp MarkedBlockFooterOffset + MarkedBlock::Footer::m_vm[t1], t1
>     copyCalleeSavesToVMEntryFrameCalleeSavesBuffer(t1, t2)
> 
>-    callSlowPath(_llint_slow_path_handle_exception)
>+    callSlowPath(slowPath)
> 
>     # When throwing from the interpreter (i.e. throwing from LLIntSlowPaths), so
>     # the throw target is not necessarily interpreted code, we come to here.
>@@ -2162,6 +2162,13 @@ _llint_throw_from_slow_path_trampoline:
>     andp MarkedBlockMask, t1
>     loadp MarkedBlockFooterOffset + MarkedBlock::Footer::m_vm[t1], t1
>     jmp VM::targetMachinePCForThrow[t1], ExceptionHandlerPtrTag
>+end
>+
>+_llint_throw_from_slow_path_trampoline:
>+    llintThrowFromSlowPath(_llint_slow_path_handle_exception)
>+
>+_llint_throw_from_slow_path_caller_exception:
>+    llintThrowFromSlowPath(_llint_slow_path_handle_exception_from_caller)
> 
> 
> _llint_throw_during_call_trampoline:
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 1e3361169b6966070b13c2386d5ef3798a82589b..e8844fad2e586e604025d068b69afcb58203ba9c 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-06-18  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        ShadowChicken crashes with stack overflow in the LLInt
>+        https://bugs.webkit.org/show_bug.cgi?id=186540
>+        <rdar://problem/39682133>
>+
>+        Reviewed by Saam Barati.
>+
>+        Add test that stack overflows and crashes on ShadowChicken when JIT is
>+        disabled and forceDebuggerBytecodeGeneration is enabled.
>+
>+        * stress/llint-stack-overflow-debugging-opcodes.js: Added.
>+        (foo):
>+        (catch):
>+
> 2018-06-14  Leo Balter  <leonardo.balter@gmail.com>
> 
>         Test262-Runner: Update config list with some failing tests
>diff --git a/JSTests/stress/llint-stack-overflow-debugging-opcodes.js b/JSTests/stress/llint-stack-overflow-debugging-opcodes.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..bd9cfe227f4cb0cfa6822dab9df1ab8f52d7e83b
>--- /dev/null
>+++ b/JSTests/stress/llint-stack-overflow-debugging-opcodes.js
>@@ -0,0 +1,8 @@
>+//@ runNoCJIT("--forceDebuggerBytecodeGeneration=true", "--useBaselineJIT=0", "--alwaysUseShadowChicken=true")
>+
>+function foo() {
>+    foo()
>+}
>+try {
>+    foo();
>+} catch(e) { }

Actions: View | Formatted Diff | Diff

Attachments on bug 186540: 342465 | 342478 | 342480 | 342496 | 342507 | 342565 | 342595 | 342634 | 342805 | 342806 | 342813 | 342976 | 342990 | 342992 | 342993 | 342995 | 343093 | 343096