Attachment 342565 Details for Bug 186540 – Patch

[patch] Patch

bug-186540-20180612112533.patch (text/plain), 11.94 KB, created by Tadeu Zagallo on 2018-06-12 11:25:34 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2018-06-12 11:25:34 PDT

Size: 11.94 KB

patch

obsolete

>Subversion Revision: 232634
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index e89b57395009cc09d3fd8a8460af9319cfa14bf1..06e3ca03d617e0e02434fb46a6ac5b465bfc564c 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2018-06-11  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 NOBODY (OOPS!).
>+
>+        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):
>+        * 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-07  Chris Dumez  <cdumez@apple.com>
> 
>         Add base class to get WeakPtrFactory member and avoid some boilerplate code
>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/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 e2db45d1057dc542e2fbcebcb82c96ad55f9d880..2d1ed4deb859b5de23295d6be1e7514b81046a38 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..c5ca3efe72e64ad5bbd37847267cf04c8889e1df 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,15 @@ 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);
>+    vm.topCallFrame = exec->callerFrame();
>+    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 51c84e8e636b6bbe19fbbf0797748e70dc2782fc..960afe6e97480dd9d01699881006e1514bdf0c5d 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 8fc47a12ef6e5a01c558af3ee886958e7406e58a..11d72aa169109fc6d8668c980231120e3ffbc032 100644
>--- a/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
>+++ b/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm
>@@ -2148,13 +2148,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.
>@@ -2163,6 +2163,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 64119c9eaf9c0a8ffa0a4073c802b1c8e71ad37e..1c279aac030283ef1ebcb6896c77bead7316948a 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-06-11  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 NOBODY (OOPS!).
>+
>+        Add test that 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-07  Saam Barati  <sbarati@apple.com>
> 
>         Make DFG to FTL OSR entry code more sane by removing bad RELEASE_ASSERTS and making it trigger compiles in outer loops before inner ones
>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