Changeset 239244 in webkit


Ignore:
Timestamp:
Dec 14, 2018 6:28:17 PM (5 years ago)
Author:
mark.lam@apple.com
Message:

CallFrame::convertToStackOverflowFrame() needs to keep the top CodeBlock alive.
​https://bugs.webkit.org/show_bug.cgi?id=192717
<rdar://problem/46660677>

Reviewed by Saam Barati.

JSTests:

  • stress/regress-192717.js: Added.

Source/JavaScriptCore:

When throwing a StackOverflowError, we convert the topCallFrame into a
StackOverflowFrame. Previously, we would nullify the codeBlock field in the frame
because a StackOverflowFrame is only a sentinel and doesn't really correspond to
any CodeBlocks. However, this is a problem because the topCallFrame may be the
only remaining place that references the CodeBlock that the stack overflow is
triggered in. The way we handle exceptions in JIT code is to return (from the
runtime operation function throwing the exception) to the JIT code to check for
the exception and if needed, do some clean up before jumping to the exception
handling thunk. As a result, we need to keep that JIT code alive, which means we
need to keep its CodeBlock alive. We only need to keep this CodeBlock alive until
we've unwound (in terms of exception handling) out of it.

We fix this issue by storing the CodeBlock to keep alive in the StackOverflowFrame
for the GC to scan while the frame is still on the stack.

We removed the call to convertToStackOverflowFrame() in
lookupExceptionHandlerFromCallerFrame() because it is redundant.
lookupExceptionHandlerFromCallerFrame() will only every be called after
a StackOverFlowError has been thrown. Hence, the top frame is already
guaranteed to be a StackOverflowFrame, and there should always be a
StackOverFlowError exception pending. We added assertions for these
instead.

  • interpreter/CallFrame.cpp:

(JSC::CallFrame::convertToStackOverflowFrame):

  • interpreter/CallFrame.h:
  • jit/JITOperations.cpp:
  • llint/LLIntSlowPaths.cpp:

(JSC::LLInt::LLINT_SLOW_PATH_DECL):

  • runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

  • runtime/CommonSlowPaths.h:

(JSC::CommonSlowPaths::codeBlockFromCallFrameCallee):
(JSC::CommonSlowPaths::arityCheckFor):

  • runtime/VM.h:

(JSC::VM::exceptionForInspection const):

Location:
trunk
Files:
1 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r239231 r239244  
     12018-12-14  Mark Lam  <mark.lam@apple.com>
     2
     3        CallFrame::convertToStackOverflowFrame() needs to keep the top CodeBlock alive.
     4        https://bugs.webkit.org/show_bug.cgi?id=192717
     5        <rdar://problem/46660677>
     6
     7        Reviewed by Saam Barati.
     8
     9        * stress/regress-192717.js: Added.
     10
    1112018-12-14  Commit Queue  <commit-queue@webkit.org>
    212

  • trunk/Source/JavaScriptCore/ChangeLog

    r239237 r239244  
     12018-12-14  Mark Lam  <mark.lam@apple.com>
     2
     3        CallFrame::convertToStackOverflowFrame() needs to keep the top CodeBlock alive.
     4        https://bugs.webkit.org/show_bug.cgi?id=192717
     5        <rdar://problem/46660677>
     6
     7        Reviewed by Saam Barati.
     8
     9        When throwing a StackOverflowError, we convert the topCallFrame into a
     10        StackOverflowFrame.  Previously, we would nullify the codeBlock field in the frame
     11        because a StackOverflowFrame is only a sentinel and doesn't really correspond to
     12        any CodeBlocks.  However, this is a problem because the topCallFrame may be the
     13        only remaining place that references the CodeBlock that the stack overflow is
     14        triggered in.  The way we handle exceptions in JIT code is to return (from the
     15        runtime operation function throwing the exception) to the JIT code to check for
     16        the exception and if needed, do some clean up before jumping to the exception
     17        handling thunk.  As a result, we need to keep that JIT code alive, which means we
     18        need to keep its CodeBlock alive.  We only need to keep this CodeBlock alive until
     19        we've unwound (in terms of exception handling) out of it.
     20
     21        We fix this issue by storing the CodeBlock to keep alive in the StackOverflowFrame
     22        for the GC to scan while the frame is still on the stack.
     23
     24        We removed the call to convertToStackOverflowFrame() in
     25        lookupExceptionHandlerFromCallerFrame() because it is redundant.
     26        lookupExceptionHandlerFromCallerFrame() will only every be called after
     27        a StackOverFlowError has been thrown.  Hence, the top frame is already
     28        guaranteed to be a StackOverflowFrame, and there should always be a
     29        StackOverFlowError exception pending.  We added assertions for these
     30        instead.
     31
     32        * interpreter/CallFrame.cpp:
     33        (JSC::CallFrame::convertToStackOverflowFrame):
     34        * interpreter/CallFrame.h:
     35        * jit/JITOperations.cpp:
     36        * llint/LLIntSlowPaths.cpp:
     37        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
     38        * runtime/CommonSlowPaths.cpp:
     39        (JSC::SLOW_PATH_DECL):
     40        * runtime/CommonSlowPaths.h:
     41        (JSC::CommonSlowPaths::codeBlockFromCallFrameCallee):
     42        (JSC::CommonSlowPaths::arityCheckFor):
     43        * runtime/VM.h:
     44        (JSC::VM::exceptionForInspection const):
     45
    1462018-12-14  David Kilzer  <ddkilzer@apple.com>
    247

  • trunk/Source/JavaScriptCore/interpreter/CallFrame.cpp

    r237547 r239244  
    338338}
    339339
    340 void CallFrame::convertToStackOverflowFrame(VM& vm)
     340void CallFrame::convertToStackOverflowFrame(VM& vm, CodeBlock* codeBlockToKeepAliveUntilFrameIsUnwound)
    341341{
    342342    ASSERT(!isGlobalExec());
     343    ASSERT(codeBlockToKeepAliveUntilFrameIsUnwound->inherits<CodeBlock>(vm));
    343344
    344345    EntryFrame* entryFrame = vm.topEntryFrame;
    … …  
    351352    JSObject* stackOverflowCallee = originCallee->globalObject()->stackOverflowFrameCallee();
    352353
    353     setCodeBlock(nullptr);
     354    setCodeBlock(codeBlockToKeepAliveUntilFrameIsUnwound);
    354355    setCallee(stackOverflowCallee);
    355356    setArgumentCountIncludingThis(0);

  • trunk/Source/JavaScriptCore/interpreter/CallFrame.h

    r237547 r239244  
    258258        }
    259259
    260         void convertToStackOverflowFrame(VM&);
     260        void convertToStackOverflowFrame(VM&, CodeBlock* codeBlockToKeepAliveUntilFrameIsUnwound);
    261261        inline bool isStackOverflowFrame() const;
    262262        inline bool isWasmFrame() const;

  • trunk/Source/JavaScriptCore/jit/JITOperations.cpp

    r238425 r239244  
    103103    VM* vm = codeBlock->vm();
    104104    auto scope = DECLARE_THROW_SCOPE(*vm);
    105     exec->convertToStackOverflowFrame(*vm);
     105    exec->convertToStackOverflowFrame(*vm, codeBlock);
    106106    NativeCallFrameTracer tracer(vm, exec);
    107107    throwStackOverflowError(exec, scope);
    … …  
    114114
    115115    int32_t missingArgCount = CommonSlowPaths::arityCheckFor(exec, *vm, CodeForCall);
    116     if (missingArgCount < 0) {
    117         exec->convertToStackOverflowFrame(*vm);
     116    if (UNLIKELY(missingArgCount < 0)) {
     117        CodeBlock* codeBlock = CommonSlowPaths::codeBlockFromCallFrameCallee(exec, CodeForCall);
     118        exec->convertToStackOverflowFrame(*vm, codeBlock);
    118119        NativeCallFrameTracer tracer(vm, exec);
    119120        throwStackOverflowError(vm->topCallFrame, scope);
    … …  
    129130
    130131    int32_t missingArgCount = CommonSlowPaths::arityCheckFor(exec, *vm, CodeForConstruct);
    131     if (missingArgCount < 0) {
    132         exec->convertToStackOverflowFrame(*vm);
     132    if (UNLIKELY(missingArgCount < 0)) {
     133        CodeBlock* codeBlock = CommonSlowPaths::codeBlockFromCallFrameCallee(exec, CodeForCall);
     134        exec->convertToStackOverflowFrame(*vm, codeBlock);
    133135        NativeCallFrameTracer tracer(vm, exec);
    134136        throwStackOverflowError(vm->topCallFrame, scope);
    … …  
    24332435void JIT_OPERATION lookupExceptionHandlerFromCallerFrame(VM* vm, ExecState* exec)
    24342436{
    2435     exec->convertToStackOverflowFrame(*vm);
     2437    ASSERT(exec->isStackOverflowFrame());
     2438    ASSERT(jsCast<ErrorInstance*>(vm->exceptionForInspection()->value().asCell())->isStackOverflowError());
    24362439    lookupExceptionHandler(vm, exec);
    24372440}

  • trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

    r238141 r239244  
    560560#endif
    561561
    562     exec->convertToStackOverflowFrame(vm);
     562    exec->convertToStackOverflowFrame(vm, codeBlock);
    563563    ErrorHandlingScope errorScope(vm);
    564564    throwStackOverflowError(exec, throwScope);

  • trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.cpp

    r238790 r239244  
    178178    BEGIN();
    179179    int slotsToAdd = CommonSlowPaths::arityCheckFor(exec, vm, CodeForCall);
    180     if (slotsToAdd < 0) {
    181         exec->convertToStackOverflowFrame(vm);
     180    if (UNLIKELY(slotsToAdd < 0)) {
     181        CodeBlock* codeBlock = CommonSlowPaths::codeBlockFromCallFrameCallee(exec, CodeForCall);
     182        exec->convertToStackOverflowFrame(vm, codeBlock);
    182183        NativeCallFrameTracer tracer(&vm, exec);
    183184        ErrorHandlingScope errorScope(vm);
    … …  
    193194    BEGIN();
    194195    int slotsToAdd = CommonSlowPaths::arityCheckFor(exec, vm, CodeForConstruct);
    195     if (slotsToAdd < 0) {
    196         exec->convertToStackOverflowFrame(vm);
     196    if (UNLIKELY(slotsToAdd < 0)) {
     197        CodeBlock* codeBlock = CommonSlowPaths::codeBlockFromCallFrameCallee(exec, CodeForCall);
     198        exec->convertToStackOverflowFrame(vm, codeBlock);
    197199        NativeCallFrameTracer tracer(&vm, exec);
    198200        ErrorHandlingScope errorScope(vm);

  • trunk/Source/JavaScriptCore/runtime/CommonSlowPaths.h

    r238543 r239244  
    7373}
    7474
    75 ALWAYS_INLINE int arityCheckFor(ExecState* exec, VM& vm, CodeSpecializationKind kind)
     75ALWAYS_INLINE CodeBlock* codeBlockFromCallFrameCallee(ExecState* exec, CodeSpecializationKind kind)
    7676{
    7777    JSFunction* callee = jsCast<JSFunction*>(exec->jsCallee());
    7878    ASSERT(!callee->isHostFunction());
    79     CodeBlock* newCodeBlock = callee->jsExecutable()->codeBlockFor(kind);
     79    return callee->jsExecutable()->codeBlockFor(kind);
     80}
     81
     82ALWAYS_INLINE int arityCheckFor(ExecState* exec, VM& vm, CodeSpecializationKind kind)
     83{
     84    CodeBlock* newCodeBlock = codeBlockFromCallFrameCallee(exec, kind);
    8085    ASSERT(exec->argumentCountIncludingThis() < static_cast<unsigned>(newCodeBlock->numParameters()));
    8186    int padding = numberOfStackPaddingSlotsWithExtraSlots(newCodeBlock, exec->argumentCountIncludingThis());

  • trunk/Source/JavaScriptCore/runtime/VM.h

    r239231 r239244  
    694694    JSCell** addressOfLastException() { return reinterpret_cast<JSCell**>(&m_lastException); }
    695695
     696    // This should only be used for test or assertion code that wants to inspect
     697    // the pending exception without interfering with Throw/CatchScopes.
     698    Exception* exceptionForInspection() const { return m_exception; }
     699
    696700    void setFailNextNewCodeBlock() { m_failNextNewCodeBlock = true; }
    697701    bool getAndClearFailNextNewCodeBlock()
Note: See TracChangeset for help on using the changeset viewer.