Title: [265470] branches/safari-610.1.25.10-branch
Revision
265470
Author
[email protected]
Date
2020-08-10 16:47:57 -0700 (Mon, 10 Aug 2020)

Log Message

Cherry-pick r265272. rdar://problem/66604070

    CheckpointSideState shoud play nicely with StackOverflowException unwinding.
    https://bugs.webkit.org/show_bug.cgi?id=215114

    Reviewed by Saam Barati.

    JSTests:

    * stress/stack-overflow-into-frame-with-pending-checkpoint.js: Added.
    (foo.catch.async bar):
    (foo.catch):
    (foo):

    Source/_javascript_Core:

    This patch fixes an issue where we the StackVisitor would
    automatically unwind into the first frame before calling into the
    provided functor. As a note, we do this because the first frame is
    not fully initialized at the time we check for stack
    overflow. When this happened we would fail to clear the side state
    causing a memory leak. To fix this the unwind function now clears
    every checkpoint up to and including the call frame containing our
    handler. Some care needs to be taken that we don't clear
    checkpoint side state for other threads, which could happen if
    there are no checkpoints on the current thread and an API
    miggrated us from another thread below the current thread.

    This patch also makes two refacorings. The first is to make the
    checkpoint side state into a stack, which is how we used it
    anyway. The second is that CallFrame::dump and everything associated
    with it is now marked const so we can PointerDump a CallFrame*.

    * dfg/DFGOSRExit.cpp:
    (JSC::DFG::OSRExit::compileExit):
    * ftl/FTLOSRExitCompiler.cpp:
    (JSC::FTL::compileStub):
    * interpreter/CallFrame.cpp:
    (JSC::CallFrame::bytecodeIndex const):
    (JSC::CallFrame::codeOrigin const):
    (JSC::CallFrame::dump const):
    (JSC::CallFrame::bytecodeIndex): Deleted.
    (JSC::CallFrame::codeOrigin): Deleted.
    (JSC::CallFrame::dump): Deleted.
    * interpreter/CallFrame.h:
    (JSC::CallFrame::argument const):
    (JSC::CallFrame::uncheckedArgument const):
    (JSC::CallFrame::getArgumentUnsafe const):
    (JSC::CallFrame::thisValue const):
    (JSC::CallFrame::newTarget const):
    (JSC::CallFrame::argument): Deleted.
    (JSC::CallFrame::uncheckedArgument): Deleted.
    (JSC::CallFrame::getArgumentUnsafe): Deleted.
    (JSC::CallFrame::thisValue): Deleted.
    (JSC::CallFrame::newTarget): Deleted.
    * interpreter/CheckpointOSRExitSideState.h:
    (JSC::CheckpointOSRExitSideState::CheckpointOSRExitSideState):
    * interpreter/Interpreter.cpp:
    (JSC::UnwindFunctor::operator() const):
    (JSC::Interpreter::unwind):
    (): Deleted.
    * llint/LLIntSlowPaths.cpp:
    (JSC::LLInt::slow_path_checkpoint_osr_exit_from_inlined_call):
    (JSC::LLInt::slow_path_checkpoint_osr_exit):
    * runtime/VM.cpp:
    (JSC::VM::scanSideState const):
    (JSC::VM::pushCheckpointOSRSideState):
    (JSC::VM::popCheckpointOSRSideState):
    (JSC::VM::popAllCheckpointOSRSideStateUntil):
    (JSC::VM::addCheckpointOSRSideState): Deleted.
    (JSC::VM::findCheckpointOSRSideState): Deleted.
    * runtime/VM.h:

    Source/WTF:

    Add a helper so we can have soft stack bounds.

    * wtf/StackBounds.h:
    (WTF::StackBounds::withSoftOrigin const):

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

Modified Paths

Added Paths

Diff

Modified: branches/safari-610.1.25.10-branch/JSTests/ChangeLog (265469 => 265470)


--- branches/safari-610.1.25.10-branch/JSTests/ChangeLog	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/JSTests/ChangeLog	2020-08-10 23:47:57 UTC (rev 265470)
@@ -1,3 +1,100 @@
+2020-08-10  Alan Coon  <[email protected]>
+
+        Cherry-pick r265272. rdar://problem/66604070
+
+    CheckpointSideState shoud play nicely with StackOverflowException unwinding.
+    https://bugs.webkit.org/show_bug.cgi?id=215114
+    
+    Reviewed by Saam Barati.
+    
+    JSTests:
+    
+    * stress/stack-overflow-into-frame-with-pending-checkpoint.js: Added.
+    (foo.catch.async bar):
+    (foo.catch):
+    (foo):
+    
+    Source/_javascript_Core:
+    
+    This patch fixes an issue where we the StackVisitor would
+    automatically unwind into the first frame before calling into the
+    provided functor. As a note, we do this because the first frame is
+    not fully initialized at the time we check for stack
+    overflow. When this happened we would fail to clear the side state
+    causing a memory leak. To fix this the unwind function now clears
+    every checkpoint up to and including the call frame containing our
+    handler. Some care needs to be taken that we don't clear
+    checkpoint side state for other threads, which could happen if
+    there are no checkpoints on the current thread and an API
+    miggrated us from another thread below the current thread.
+    
+    This patch also makes two refacorings. The first is to make the
+    checkpoint side state into a stack, which is how we used it
+    anyway. The second is that CallFrame::dump and everything associated
+    with it is now marked const so we can PointerDump a CallFrame*.
+    
+    * dfg/DFGOSRExit.cpp:
+    (JSC::DFG::OSRExit::compileExit):
+    * ftl/FTLOSRExitCompiler.cpp:
+    (JSC::FTL::compileStub):
+    * interpreter/CallFrame.cpp:
+    (JSC::CallFrame::bytecodeIndex const):
+    (JSC::CallFrame::codeOrigin const):
+    (JSC::CallFrame::dump const):
+    (JSC::CallFrame::bytecodeIndex): Deleted.
+    (JSC::CallFrame::codeOrigin): Deleted.
+    (JSC::CallFrame::dump): Deleted.
+    * interpreter/CallFrame.h:
+    (JSC::CallFrame::argument const):
+    (JSC::CallFrame::uncheckedArgument const):
+    (JSC::CallFrame::getArgumentUnsafe const):
+    (JSC::CallFrame::thisValue const):
+    (JSC::CallFrame::newTarget const):
+    (JSC::CallFrame::argument): Deleted.
+    (JSC::CallFrame::uncheckedArgument): Deleted.
+    (JSC::CallFrame::getArgumentUnsafe): Deleted.
+    (JSC::CallFrame::thisValue): Deleted.
+    (JSC::CallFrame::newTarget): Deleted.
+    * interpreter/CheckpointOSRExitSideState.h:
+    (JSC::CheckpointOSRExitSideState::CheckpointOSRExitSideState):
+    * interpreter/Interpreter.cpp:
+    (JSC::UnwindFunctor::operator() const):
+    (JSC::Interpreter::unwind):
+    (): Deleted.
+    * llint/LLIntSlowPaths.cpp:
+    (JSC::LLInt::slow_path_checkpoint_osr_exit_from_inlined_call):
+    (JSC::LLInt::slow_path_checkpoint_osr_exit):
+    * runtime/VM.cpp:
+    (JSC::VM::scanSideState const):
+    (JSC::VM::pushCheckpointOSRSideState):
+    (JSC::VM::popCheckpointOSRSideState):
+    (JSC::VM::popAllCheckpointOSRSideStateUntil):
+    (JSC::VM::addCheckpointOSRSideState): Deleted.
+    (JSC::VM::findCheckpointOSRSideState): Deleted.
+    * runtime/VM.h:
+    
+    Source/WTF:
+    
+    Add a helper so we can have soft stack bounds.
+    
+    * wtf/StackBounds.h:
+    (WTF::StackBounds::withSoftOrigin const):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@265272 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-08-04  Keith Miller  <[email protected]>
+
+            CheckpointSideState shoud play nicely with StackOverflowException unwinding.
+            https://bugs.webkit.org/show_bug.cgi?id=215114
+
+            Reviewed by Saam Barati.
+
+            * stress/stack-overflow-into-frame-with-pending-checkpoint.js: Added.
+            (foo.catch.async bar):
+            (foo.catch):
+            (foo):
+
 2020-07-30  Saam Barati  <[email protected]>
 
         Skip stress/operand-should-fit-in-abstract-heap-encoded-payload-format.js

Added: branches/safari-610.1.25.10-branch/JSTests/stress/stack-overflow-into-frame-with-pending-checkpoint.js (0 => 265470)


--- branches/safari-610.1.25.10-branch/JSTests/stress/stack-overflow-into-frame-with-pending-checkpoint.js	                        (rev 0)
+++ branches/safari-610.1.25.10-branch/JSTests/stress/stack-overflow-into-frame-with-pending-checkpoint.js	2020-08-10 23:47:57 UTC (rev 265470)
@@ -0,0 +1,19 @@
+//@ requireOptions("--jitPolicyScale=0")
+
+function foo() {
+  'use strict'
+  try {
+    foo();
+  } catch(e) {
+    ({__proto__:0});
+    async function bar(a0) {
+      bar(a0, ...[0]);
+    }
+    for (let i=0; i<1000; i++) {
+      bar();
+    }
+    bar(0,  0);
+  }
+}
+
+foo();

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/ChangeLog (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/ChangeLog	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/ChangeLog	2020-08-10 23:47:57 UTC (rev 265470)
@@ -1,3 +1,152 @@
+2020-08-10  Alan Coon  <[email protected]>
+
+        Cherry-pick r265272. rdar://problem/66604070
+
+    CheckpointSideState shoud play nicely with StackOverflowException unwinding.
+    https://bugs.webkit.org/show_bug.cgi?id=215114
+    
+    Reviewed by Saam Barati.
+    
+    JSTests:
+    
+    * stress/stack-overflow-into-frame-with-pending-checkpoint.js: Added.
+    (foo.catch.async bar):
+    (foo.catch):
+    (foo):
+    
+    Source/_javascript_Core:
+    
+    This patch fixes an issue where we the StackVisitor would
+    automatically unwind into the first frame before calling into the
+    provided functor. As a note, we do this because the first frame is
+    not fully initialized at the time we check for stack
+    overflow. When this happened we would fail to clear the side state
+    causing a memory leak. To fix this the unwind function now clears
+    every checkpoint up to and including the call frame containing our
+    handler. Some care needs to be taken that we don't clear
+    checkpoint side state for other threads, which could happen if
+    there are no checkpoints on the current thread and an API
+    miggrated us from another thread below the current thread.
+    
+    This patch also makes two refacorings. The first is to make the
+    checkpoint side state into a stack, which is how we used it
+    anyway. The second is that CallFrame::dump and everything associated
+    with it is now marked const so we can PointerDump a CallFrame*.
+    
+    * dfg/DFGOSRExit.cpp:
+    (JSC::DFG::OSRExit::compileExit):
+    * ftl/FTLOSRExitCompiler.cpp:
+    (JSC::FTL::compileStub):
+    * interpreter/CallFrame.cpp:
+    (JSC::CallFrame::bytecodeIndex const):
+    (JSC::CallFrame::codeOrigin const):
+    (JSC::CallFrame::dump const):
+    (JSC::CallFrame::bytecodeIndex): Deleted.
+    (JSC::CallFrame::codeOrigin): Deleted.
+    (JSC::CallFrame::dump): Deleted.
+    * interpreter/CallFrame.h:
+    (JSC::CallFrame::argument const):
+    (JSC::CallFrame::uncheckedArgument const):
+    (JSC::CallFrame::getArgumentUnsafe const):
+    (JSC::CallFrame::thisValue const):
+    (JSC::CallFrame::newTarget const):
+    (JSC::CallFrame::argument): Deleted.
+    (JSC::CallFrame::uncheckedArgument): Deleted.
+    (JSC::CallFrame::getArgumentUnsafe): Deleted.
+    (JSC::CallFrame::thisValue): Deleted.
+    (JSC::CallFrame::newTarget): Deleted.
+    * interpreter/CheckpointOSRExitSideState.h:
+    (JSC::CheckpointOSRExitSideState::CheckpointOSRExitSideState):
+    * interpreter/Interpreter.cpp:
+    (JSC::UnwindFunctor::operator() const):
+    (JSC::Interpreter::unwind):
+    (): Deleted.
+    * llint/LLIntSlowPaths.cpp:
+    (JSC::LLInt::slow_path_checkpoint_osr_exit_from_inlined_call):
+    (JSC::LLInt::slow_path_checkpoint_osr_exit):
+    * runtime/VM.cpp:
+    (JSC::VM::scanSideState const):
+    (JSC::VM::pushCheckpointOSRSideState):
+    (JSC::VM::popCheckpointOSRSideState):
+    (JSC::VM::popAllCheckpointOSRSideStateUntil):
+    (JSC::VM::addCheckpointOSRSideState): Deleted.
+    (JSC::VM::findCheckpointOSRSideState): Deleted.
+    * runtime/VM.h:
+    
+    Source/WTF:
+    
+    Add a helper so we can have soft stack bounds.
+    
+    * wtf/StackBounds.h:
+    (WTF::StackBounds::withSoftOrigin const):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@265272 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-08-04  Keith Miller  <[email protected]>
+
+            CheckpointSideState shoud play nicely with StackOverflowException unwinding.
+            https://bugs.webkit.org/show_bug.cgi?id=215114
+
+            Reviewed by Saam Barati.
+
+            This patch fixes an issue where we the StackVisitor would
+            automatically unwind into the first frame before calling into the
+            provided functor. As a note, we do this because the first frame is
+            not fully initialized at the time we check for stack
+            overflow. When this happened we would fail to clear the side state
+            causing a memory leak. To fix this the unwind function now clears
+            every checkpoint up to and including the call frame containing our
+            handler. Some care needs to be taken that we don't clear
+            checkpoint side state for other threads, which could happen if
+            there are no checkpoints on the current thread and an API
+            miggrated us from another thread below the current thread.
+
+            This patch also makes two refacorings. The first is to make the
+            checkpoint side state into a stack, which is how we used it
+            anyway. The second is that CallFrame::dump and everything associated
+            with it is now marked const so we can PointerDump a CallFrame*.
+
+            * dfg/DFGOSRExit.cpp:
+            (JSC::DFG::OSRExit::compileExit):
+            * ftl/FTLOSRExitCompiler.cpp:
+            (JSC::FTL::compileStub):
+            * interpreter/CallFrame.cpp:
+            (JSC::CallFrame::bytecodeIndex const):
+            (JSC::CallFrame::codeOrigin const):
+            (JSC::CallFrame::dump const):
+            (JSC::CallFrame::bytecodeIndex): Deleted.
+            (JSC::CallFrame::codeOrigin): Deleted.
+            (JSC::CallFrame::dump): Deleted.
+            * interpreter/CallFrame.h:
+            (JSC::CallFrame::argument const):
+            (JSC::CallFrame::uncheckedArgument const):
+            (JSC::CallFrame::getArgumentUnsafe const):
+            (JSC::CallFrame::thisValue const):
+            (JSC::CallFrame::newTarget const):
+            (JSC::CallFrame::argument): Deleted.
+            (JSC::CallFrame::uncheckedArgument): Deleted.
+            (JSC::CallFrame::getArgumentUnsafe): Deleted.
+            (JSC::CallFrame::thisValue): Deleted.
+            (JSC::CallFrame::newTarget): Deleted.
+            * interpreter/CheckpointOSRExitSideState.h:
+            (JSC::CheckpointOSRExitSideState::CheckpointOSRExitSideState):
+            * interpreter/Interpreter.cpp:
+            (JSC::UnwindFunctor::operator() const):
+            (JSC::Interpreter::unwind):
+            (): Deleted.
+            * llint/LLIntSlowPaths.cpp:
+            (JSC::LLInt::slow_path_checkpoint_osr_exit_from_inlined_call):
+            (JSC::LLInt::slow_path_checkpoint_osr_exit):
+            * runtime/VM.cpp:
+            (JSC::VM::scanSideState const):
+            (JSC::VM::pushCheckpointOSRSideState):
+            (JSC::VM::popCheckpointOSRSideState):
+            (JSC::VM::popAllCheckpointOSRSideStateUntil):
+            (JSC::VM::addCheckpointOSRSideState): Deleted.
+            (JSC::VM::findCheckpointOSRSideState): Deleted.
+            * runtime/VM.h:
+
 2020-08-04  Alan Coon  <[email protected]>
 
         Cherry-pick r265186. rdar://problem/66528563

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/dfg/DFGOSRExit.cpp (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/dfg/DFGOSRExit.cpp	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/dfg/DFGOSRExit.cpp	2020-08-10 23:47:57 UTC (rev 265470)
@@ -42,6 +42,8 @@
 #include "OperandsInlines.h"
 #include "ProbeContext.h"
 
+#include <wtf/Scope.h>
+
 namespace JSC { namespace DFG {
 
 OSRExit::OSRExit(ExitKind kind, JSValueSource jsValueSource, MethodOfGettingAValueProfile valueProfile, SpeculativeJIT* jit, unsigned streamIndex, unsigned recoveryIndex)
@@ -591,8 +593,16 @@
         VM* vmPtr = &vm;
         auto* tmpScratch = scratch + operands.tmpIndex(0);
         jit.probe([=, values = WTFMove(values)] (Probe::Context& context) {
+            Vector<std::unique_ptr<CheckpointOSRExitSideState>> sideStates;
+            sideStates.reserveInitialCapacity(exit.m_codeOrigin.inlineDepth());
+            auto sideStateCommitter = makeScopeExit([&] {
+                for (size_t i = sideStates.size(); i--;)
+                    vmPtr->pushCheckpointOSRSideState(WTFMove(sideStates[i]));
+            });
+
+
             auto addSideState = [&] (CallFrame* frame, BytecodeIndex index, size_t tmpOffset) {
-                std::unique_ptr<CheckpointOSRExitSideState> sideState = WTF::makeUnique<CheckpointOSRExitSideState>();
+                std::unique_ptr<CheckpointOSRExitSideState> sideState = WTF::makeUnique<CheckpointOSRExitSideState>(frame);
 
                 sideState->bytecodeIndex = index;
                 for (size_t i = 0; i < maxNumCheckpointTmps; ++i) {
@@ -650,7 +660,7 @@
                     }
                 }
 
-                vmPtr->addCheckpointOSRSideState(frame, WTFMove(sideState));
+                sideStates.append(WTFMove(sideState));
             };
 
             const CodeOrigin* codeOrigin;

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp	2020-08-10 23:47:57 UTC (rev 265470)
@@ -43,6 +43,8 @@
 #include "OperandsInlines.h"
 #include "ProbeContext.h"
 
+#include <wtf/Scope.h>
+
 namespace JSC { namespace FTL {
 
 using namespace DFG;
@@ -481,14 +483,21 @@
         JSValue* tmpScratch = reinterpret_cast<JSValue*>(scratch + exit.m_descriptor->m_values.tmpIndex(0));
         VM* vmPtr = &vm;
         jit.probe([=] (Probe::Context& context) {
+            Vector<std::unique_ptr<CheckpointOSRExitSideState>> sideStates;
+            sideStates.reserveInitialCapacity(exit.m_codeOrigin.inlineDepth());
+            auto sideStateCommitter = makeScopeExit([&] {
+                for (size_t i = sideStates.size(); i--;)
+                    vmPtr->pushCheckpointOSRSideState(WTFMove(sideStates[i]));
+            });
+
             auto addSideState = [&] (CallFrame* frame, BytecodeIndex index, size_t tmpOffset) {
-                std::unique_ptr<CheckpointOSRExitSideState> sideState = WTF::makeUnique<CheckpointOSRExitSideState>();
+                std::unique_ptr<CheckpointOSRExitSideState> sideState = WTF::makeUnique<CheckpointOSRExitSideState>(frame);
 
                 sideState->bytecodeIndex = index;
                 for (size_t i = 0; i < maxNumCheckpointTmps; ++i)
                     sideState->tmps[i] = tmpScratch[i + tmpOffset];
 
-                vmPtr->addCheckpointOSRSideState(frame, WTFMove(sideState));
+                sideStates.append(WTFMove(sideState));
             };
 
             const CodeOrigin* codeOrigin;

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/CallFrame.cpp (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/CallFrame.cpp	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/CallFrame.cpp	2020-08-10 23:47:57 UTC (rev 265470)
@@ -127,7 +127,7 @@
     return callSiteIndex().bits();
 }
 
-BytecodeIndex CallFrame::bytecodeIndex()
+BytecodeIndex CallFrame::bytecodeIndex() const
 {
     ASSERT(!callee().isWasm());
     if (!codeBlock())
@@ -147,7 +147,7 @@
     return callSiteIndex().bytecodeIndex();
 }
 
-CodeOrigin CallFrame::codeOrigin()
+CodeOrigin CallFrame::codeOrigin() const
 {
     if (!codeBlock())
         return CodeOrigin(BytecodeIndex(0));
@@ -267,7 +267,7 @@
     return emptyString();
 }
 
-void CallFrame::dump(PrintStream& out)
+void CallFrame::dump(PrintStream& out) const
 {
     if (CodeBlock* codeBlock = this->codeBlock()) {
         out.print(codeBlock->inferredName(), "#", codeBlock->hashAsStringIfPossible(), " [", codeBlock->jitType(), " ", bytecodeIndex(), "]");

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/CallFrame.h (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/CallFrame.h	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/CallFrame.h	2020-08-10 23:47:57 UTC (rev 265470)
@@ -174,11 +174,11 @@
         // also return 0 if the call frame has no notion of bytecode offsets (for
         // example if it's native code).
         // https://bugs.webkit.org/show_bug.cgi?id=121754
-        BytecodeIndex bytecodeIndex();
+        BytecodeIndex bytecodeIndex() const;
         
         // This will get you a CodeOrigin. It will always succeed. May return
         // CodeOrigin(BytecodeIndex(0)) if we're in native code.
-        JS_EXPORT_PRIVATE CodeOrigin codeOrigin();
+        JS_EXPORT_PRIVATE CodeOrigin codeOrigin() const;
 
         inline Register* topOfFrame();
     
@@ -211,13 +211,13 @@
         // use thisValue() and setThisValue() below.
 
         JSValue* addressOfArgumentsStart() const { return bitwise_cast<JSValue*>(this + argumentOffset(0)); }
-        JSValue argument(size_t argument)
+        JSValue argument(size_t argument) const
         {
             if (argument >= argumentCount())
                  return jsUndefined();
             return getArgumentUnsafe(argument);
         }
-        JSValue uncheckedArgument(size_t argument)
+        JSValue uncheckedArgument(size_t argument) const
         {
             ASSERT(argument < argumentCount());
             return getArgumentUnsafe(argument);
@@ -227,7 +227,7 @@
             this[argumentOffset(argument)] = value;
         }
 
-        JSValue getArgumentUnsafe(size_t argIndex)
+        JSValue getArgumentUnsafe(size_t argIndex) const
         {
             // User beware! This method does not verify that there is a valid
             // argument at the specified argIndex. This is used for debugging
@@ -237,12 +237,12 @@
         }
 
         static int thisArgumentOffset() { return argumentOffsetIncludingThis(0); }
-        JSValue thisValue() { return this[thisArgumentOffset()].jsValue(); }
+        JSValue thisValue() const { return this[thisArgumentOffset()].jsValue(); }
         void setThisValue(JSValue value) { this[thisArgumentOffset()] = value; }
 
         // Under the constructor implemented in C++, thisValue holds the newTarget instead of the automatically constructed value.
         // The result of this function is only effective under the "construct" context.
-        JSValue newTarget() { return thisValue(); }
+        JSValue newTarget() const { return thisValue(); }
 
         JSValue argumentAfterCapture(size_t argument);
 
@@ -278,7 +278,7 @@
             StackVisitor::visit<action, Functor>(this, vm, functor);
         }
 
-        void dump(PrintStream&);
+        void dump(PrintStream&) const;
 
         // This function is used in LLDB btjs.
         JS_EXPORT_PRIVATE const char* describeFrame();

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/CheckpointOSRExitSideState.h (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/CheckpointOSRExitSideState.h	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/CheckpointOSRExitSideState.h	2020-08-10 23:47:57 UTC (rev 265470)
@@ -33,7 +33,11 @@
 struct CheckpointOSRExitSideState {
     WTF_MAKE_FAST_ALLOCATED;
 public:
+    CheckpointOSRExitSideState(CallFrame* frame)
+        : associatedCallFrame(frame)
+    { }
 
+    CallFrame* associatedCallFrame;
     BytecodeIndex bytecodeIndex;
     JSValue tmps[maxNumCheckpointTmps] { };
 };

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/Interpreter.cpp (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/Interpreter.cpp	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/interpreter/Interpreter.cpp	2020-08-10 23:47:57 UTC (rev 265470)
@@ -524,11 +524,6 @@
 
         m_handler = nullptr;
         if (m_codeBlock) {
-            // FIXME: We should support exception handling in checkpoints.
-#if ENABLE(DFG_JIT)
-            if (removeCodePtrTag(m_returnPC) == LLInt::getCodePtr<NoPtrTag>(checkpoint_osr_exit_from_inlined_call_trampoline).executableAddress())
-                m_codeBlock->vm().findCheckpointOSRSideState(m_callFrame);
-#endif
             if (!m_isTermination) {
                 m_handler = findExceptionHandler(visitor, m_codeBlock, RequiredHandler::AnyHandler);
                 if (m_handler)
@@ -551,7 +546,6 @@
         if (shouldStopUnwinding)
             return StackVisitor::Done;
 
-        m_returnPC = m_callFrame->returnPC().value();
         return StackVisitor::Continue;
     }
 
@@ -588,7 +582,6 @@
     bool m_isTermination;
     CodeBlock*& m_codeBlock;
     HandlerInfo*& m_handler;
-    mutable const void* m_returnPC { nullptr };
 };
 
 NEVER_INLINE HandlerInfo* Interpreter::unwind(VM& vm, CallFrame*& callFrame, Exception* exception)
@@ -612,6 +605,9 @@
     HandlerInfo* handler = nullptr;
     UnwindFunctor functor(vm, callFrame, isTerminatedExecutionException(vm, exception), codeBlock, handler);
     StackVisitor::visit<StackVisitor::TerminateIfTopEntryFrameIsEmpty>(callFrame, vm, functor);
+    if (vm.hasCheckpointOSRSideState())
+        vm.popAllCheckpointOSRSideStateUntil(callFrame);
+
     if (!handler)
         return nullptr;
 

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2020-08-10 23:47:57 UTC (rev 265470)
@@ -2202,7 +2202,7 @@
     SlowPathFrameTracer tracer(vm, callFrame);
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    std::unique_ptr<CheckpointOSRExitSideState> sideState = vm.findCheckpointOSRSideState(callFrame);
+    std::unique_ptr<CheckpointOSRExitSideState> sideState = vm.popCheckpointOSRSideState(callFrame);
     BytecodeIndex bytecodeIndex = sideState->bytecodeIndex;
     ASSERT(bytecodeIndex.checkpoint());
 
@@ -2253,7 +2253,7 @@
 
     JSGlobalObject* globalObject = codeBlock->globalObject();
 
-    std::unique_ptr<CheckpointOSRExitSideState> sideState = vm.findCheckpointOSRSideState(callFrame);
+    std::unique_ptr<CheckpointOSRExitSideState> sideState = vm.popCheckpointOSRSideState(callFrame);
     BytecodeIndex bytecodeIndex = sideState->bytecodeIndex;
     ASSERT(bytecodeIndex.checkpoint());
 

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/runtime/VM.cpp (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/runtime/VM.cpp	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/runtime/VM.cpp	2020-08-10 23:47:57 UTC (rev 265470)
@@ -1078,27 +1078,43 @@
 void VM::scanSideState(ConservativeRoots& roots) const
 {
     ASSERT(heap.worldIsStopped());
-    for (const auto& iter : m_checkpointSideState) {
-        static_assert(sizeof(iter.value->tmps) / sizeof(JSValue) == maxNumCheckpointTmps);
-        roots.add(iter.value->tmps, iter.value->tmps + maxNumCheckpointTmps);
+    for (const auto& sideState : m_checkpointSideState) {
+        static_assert(sizeof(sideState->tmps) / sizeof(JSValue) == maxNumCheckpointTmps);
+        roots.add(sideState->tmps, sideState->tmps + maxNumCheckpointTmps);
     }
 }
 #endif
 
-void VM::addCheckpointOSRSideState(CallFrame* callFrame, std::unique_ptr<CheckpointOSRExitSideState>&& payload)
+void VM::pushCheckpointOSRSideState(std::unique_ptr<CheckpointOSRExitSideState>&& payload)
 {
     ASSERT(currentThreadIsHoldingAPILock());
-    auto addResult = m_checkpointSideState.add(callFrame, WTFMove(payload));
-    ASSERT_UNUSED(addResult, addResult.isNewEntry);
+    ASSERT(payload->associatedCallFrame);
+#if ASSERT_ENABLED
+    for (const auto& sideState : m_checkpointSideState)
+        ASSERT(sideState->associatedCallFrame != payload->associatedCallFrame);
+#endif
+    m_checkpointSideState.append(WTFMove(payload));
 }
 
-std::unique_ptr<CheckpointOSRExitSideState> VM::findCheckpointOSRSideState(CallFrame* callFrame)
+std::unique_ptr<CheckpointOSRExitSideState> VM::popCheckpointOSRSideState(CallFrame* expectedCallFrame)
 {
     ASSERT(currentThreadIsHoldingAPILock());
-    auto sideState = m_checkpointSideState.take(callFrame);
+    auto sideState = m_checkpointSideState.takeLast();
+    RELEASE_ASSERT(sideState->associatedCallFrame == expectedCallFrame);
     return sideState;
 }
 
+void VM::popAllCheckpointOSRSideStateUntil(CallFrame* target)
+{
+    ASSERT(currentThreadIsHoldingAPILock());
+    auto bounds = StackBounds::currentThreadStackBounds().withSoftOrigin(target);
+    ASSERT(bounds.contains(target));
+
+    // We have to worry about migrating from another thread since there may be no checkpoints in our thread but one in the other threads.
+    while (m_checkpointSideState.size() && bounds.contains(m_checkpointSideState.last()->associatedCallFrame))
+        m_checkpointSideState.takeLast();
+}
+
 void logSanitizeStack(VM& vm)
 {
     if (Options::verboseSanitizeStack() && vm.topCallFrame) {

Modified: branches/safari-610.1.25.10-branch/Source/_javascript_Core/runtime/VM.h (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/_javascript_Core/runtime/VM.h	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/_javascript_Core/runtime/VM.h	2020-08-10 23:47:57 UTC (rev 265470)
@@ -944,8 +944,9 @@
 
     void gatherScratchBufferRoots(ConservativeRoots&);
 
-    void addCheckpointOSRSideState(CallFrame*, std::unique_ptr<CheckpointOSRExitSideState>&&);
-    std::unique_ptr<CheckpointOSRExitSideState> findCheckpointOSRSideState(CallFrame*);
+    void pushCheckpointOSRSideState(std::unique_ptr<CheckpointOSRExitSideState>&&);
+    std::unique_ptr<CheckpointOSRExitSideState> popCheckpointOSRSideState(CallFrame* expectedFrame);
+    void popAllCheckpointOSRSideStateUntil(CallFrame* targetFrame);
     bool hasCheckpointOSRSideState() const { return m_checkpointSideState.size(); }
     void scanSideState(ConservativeRoots&) const;
 
@@ -1206,7 +1207,7 @@
     Lock m_scratchBufferLock;
     Vector<ScratchBuffer*> m_scratchBuffers;
     size_t m_sizeOfLastScratchBuffer { 0 };
-    HashMap<CallFrame*, std::unique_ptr<CheckpointOSRExitSideState>> m_checkpointSideState;
+    Vector<std::unique_ptr<CheckpointOSRExitSideState>, 4> m_checkpointSideState;
     InlineWatchpointSet m_primitiveGigacageEnabled;
     FunctionHasExecutedCache m_functionHasExecutedCache;
     std::unique_ptr<ControlFlowProfiler> m_controlFlowProfiler;

Modified: branches/safari-610.1.25.10-branch/Source/WTF/ChangeLog (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/WTF/ChangeLog	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/WTF/ChangeLog	2020-08-10 23:47:57 UTC (rev 265470)
@@ -1,5 +1,102 @@
 2020-08-10  Alan Coon  <[email protected]>
 
+        Cherry-pick r265272. rdar://problem/66604070
+
+    CheckpointSideState shoud play nicely with StackOverflowException unwinding.
+    https://bugs.webkit.org/show_bug.cgi?id=215114
+    
+    Reviewed by Saam Barati.
+    
+    JSTests:
+    
+    * stress/stack-overflow-into-frame-with-pending-checkpoint.js: Added.
+    (foo.catch.async bar):
+    (foo.catch):
+    (foo):
+    
+    Source/_javascript_Core:
+    
+    This patch fixes an issue where we the StackVisitor would
+    automatically unwind into the first frame before calling into the
+    provided functor. As a note, we do this because the first frame is
+    not fully initialized at the time we check for stack
+    overflow. When this happened we would fail to clear the side state
+    causing a memory leak. To fix this the unwind function now clears
+    every checkpoint up to and including the call frame containing our
+    handler. Some care needs to be taken that we don't clear
+    checkpoint side state for other threads, which could happen if
+    there are no checkpoints on the current thread and an API
+    miggrated us from another thread below the current thread.
+    
+    This patch also makes two refacorings. The first is to make the
+    checkpoint side state into a stack, which is how we used it
+    anyway. The second is that CallFrame::dump and everything associated
+    with it is now marked const so we can PointerDump a CallFrame*.
+    
+    * dfg/DFGOSRExit.cpp:
+    (JSC::DFG::OSRExit::compileExit):
+    * ftl/FTLOSRExitCompiler.cpp:
+    (JSC::FTL::compileStub):
+    * interpreter/CallFrame.cpp:
+    (JSC::CallFrame::bytecodeIndex const):
+    (JSC::CallFrame::codeOrigin const):
+    (JSC::CallFrame::dump const):
+    (JSC::CallFrame::bytecodeIndex): Deleted.
+    (JSC::CallFrame::codeOrigin): Deleted.
+    (JSC::CallFrame::dump): Deleted.
+    * interpreter/CallFrame.h:
+    (JSC::CallFrame::argument const):
+    (JSC::CallFrame::uncheckedArgument const):
+    (JSC::CallFrame::getArgumentUnsafe const):
+    (JSC::CallFrame::thisValue const):
+    (JSC::CallFrame::newTarget const):
+    (JSC::CallFrame::argument): Deleted.
+    (JSC::CallFrame::uncheckedArgument): Deleted.
+    (JSC::CallFrame::getArgumentUnsafe): Deleted.
+    (JSC::CallFrame::thisValue): Deleted.
+    (JSC::CallFrame::newTarget): Deleted.
+    * interpreter/CheckpointOSRExitSideState.h:
+    (JSC::CheckpointOSRExitSideState::CheckpointOSRExitSideState):
+    * interpreter/Interpreter.cpp:
+    (JSC::UnwindFunctor::operator() const):
+    (JSC::Interpreter::unwind):
+    (): Deleted.
+    * llint/LLIntSlowPaths.cpp:
+    (JSC::LLInt::slow_path_checkpoint_osr_exit_from_inlined_call):
+    (JSC::LLInt::slow_path_checkpoint_osr_exit):
+    * runtime/VM.cpp:
+    (JSC::VM::scanSideState const):
+    (JSC::VM::pushCheckpointOSRSideState):
+    (JSC::VM::popCheckpointOSRSideState):
+    (JSC::VM::popAllCheckpointOSRSideStateUntil):
+    (JSC::VM::addCheckpointOSRSideState): Deleted.
+    (JSC::VM::findCheckpointOSRSideState): Deleted.
+    * runtime/VM.h:
+    
+    Source/WTF:
+    
+    Add a helper so we can have soft stack bounds.
+    
+    * wtf/StackBounds.h:
+    (WTF::StackBounds::withSoftOrigin const):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@265272 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-08-04  Keith Miller  <[email protected]>
+
+            CheckpointSideState shoud play nicely with StackOverflowException unwinding.
+            https://bugs.webkit.org/show_bug.cgi?id=215114
+
+            Reviewed by Saam Barati.
+
+            Add a helper so we can have soft stack bounds.
+
+            * wtf/StackBounds.h:
+            (WTF::StackBounds::withSoftOrigin const):
+
+2020-08-10  Alan Coon  <[email protected]>
+
         Cherry-pick r265252. rdar://problem/66645897
 
     about: scheme URL constants should be backed by StaticStringImpl

Modified: branches/safari-610.1.25.10-branch/Source/WTF/wtf/StackBounds.h (265469 => 265470)


--- branches/safari-610.1.25.10-branch/Source/WTF/wtf/StackBounds.h	2020-08-10 23:47:53 UTC (rev 265469)
+++ branches/safari-610.1.25.10-branch/Source/WTF/wtf/StackBounds.h	2020-08-10 23:47:57 UTC (rev 265470)
@@ -104,6 +104,12 @@
         return startOfUserStack - maxUserStackWithReservedZone;
     }
 
+    StackBounds withSoftOrigin(void* origin) const
+    {
+        ASSERT(contains(origin));
+        return StackBounds(origin, m_bound);
+    }
+
 private:
     StackBounds(void* origin, void* end)
         : m_origin(origin)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to