[webkit-changes] [239244] trunk

Fri, 14 Dec 2018 18:30:14 -0800

Title: [239244] trunk
Revision
239244
Author
[email protected]
Date
2018-12-14 18:28:17 -0800 (Fri, 14 Dec 2018)

Log 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/_javascript_Core:

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):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (239243 => 239244)


--- trunk/JSTests/ChangeLog	2018-12-15 01:07:18 UTC (rev 239243)
+++ trunk/JSTests/ChangeLog	2018-12-15 02:28:17 UTC (rev 239244)
@@ -1,3 +1,13 @@
+2018-12-14  Mark Lam  <[email protected]>
+
+        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.
+
+        * stress/regress-192717.js: Added.
+
 2018-12-14  Commit Queue  <[email protected]>
 
         Unreviewed, rolling out r239153, r239154, and r239155.

Added: trunk/JSTests/stress/regress-192717.js (0 => 239244)


--- trunk/JSTests/stress/regress-192717.js	                        (rev 0)
+++ trunk/JSTests/stress/regress-192717.js	2018-12-15 02:28:17 UTC (rev 239244)
@@ -0,0 +1,16 @@
+//@ runDefault("--useLLInt=false", "--forceCodeBlockToJettisonDueToOldAge=true", "--maxPerThreadStackUsage=200000", "--exceptionStackTraceLimit=1", "--defaultErrorStackTraceLimit=1")
+//@ skip if $memoryLimited or $buildType == "debug"
+
+let foo = 'let a';
+for (let i = 0; i < 400000; i++)
+  foo += ',a' + i;
+
+var exception;
+try {
+    new Function(foo)();
+} catch (e) {
+    exception = e;
+}
+
+if (exception != "RangeError: Maximum call stack size exceeded.")
+    throw "FAILED";

Modified: trunk/Source/_javascript_Core/ChangeLog (239243 => 239244)


--- trunk/Source/_javascript_Core/ChangeLog	2018-12-15 01:07:18 UTC (rev 239243)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-12-15 02:28:17 UTC (rev 239244)
@@ -1,3 +1,48 @@
+2018-12-14  Mark Lam  <[email protected]>
+
+        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.
+
+        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):
+
 2018-12-14  David Kilzer  <[email protected]>
 
         clang-tidy: Fix unnecessary copy of objects for operator==() methods

Modified: trunk/Source/_javascript_Core/interpreter/CallFrame.cpp (239243 => 239244)


--- trunk/Source/_javascript_Core/interpreter/CallFrame.cpp	2018-12-15 01:07:18 UTC (rev 239243)
+++ trunk/Source/_javascript_Core/interpreter/CallFrame.cpp	2018-12-15 02:28:17 UTC (rev 239244)
@@ -337,9 +337,10 @@
     return buffer;
 }
 
-void CallFrame::convertToStackOverflowFrame(VM& vm)
+void CallFrame::convertToStackOverflowFrame(VM& vm, CodeBlock* codeBlockToKeepAliveUntilFrameIsUnwound)
 {
     ASSERT(!isGlobalExec());
+    ASSERT(codeBlockToKeepAliveUntilFrameIsUnwound->inherits<CodeBlock>(vm));
 
     EntryFrame* entryFrame = vm.topEntryFrame;
     CallFrame* throwOriginFrame = this;
@@ -350,7 +351,7 @@
     JSObject* originCallee = throwOriginFrame ? throwOriginFrame->jsCallee() : vmEntryRecord(vm.topEntryFrame)->callee();
     JSObject* stackOverflowCallee = originCallee->globalObject()->stackOverflowFrameCallee();
 
-    setCodeBlock(nullptr);
+    setCodeBlock(codeBlockToKeepAliveUntilFrameIsUnwound);
     setCallee(stackOverflowCallee);
     setArgumentCountIncludingThis(0);
 }

Modified: trunk/Source/_javascript_Core/interpreter/CallFrame.h (239243 => 239244)


--- trunk/Source/_javascript_Core/interpreter/CallFrame.h	2018-12-15 01:07:18 UTC (rev 239243)
+++ trunk/Source/_javascript_Core/interpreter/CallFrame.h	2018-12-15 02:28:17 UTC (rev 239244)
@@ -257,7 +257,7 @@
             return callerFrameAndPC().callerFrame == noCaller() && callerFrameAndPC().returnPC == nullptr;
         }
 
-        void convertToStackOverflowFrame(VM&);
+        void convertToStackOverflowFrame(VM&, CodeBlock* codeBlockToKeepAliveUntilFrameIsUnwound);
         inline bool isStackOverflowFrame() const;
         inline bool isWasmFrame() const;

Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (239243 => 239244)


--- trunk/Source/_javascript_Core/jit/JITOperations.cpp	2018-12-15 01:07:18 UTC (rev 239243)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp	2018-12-15 02:28:17 UTC (rev 239244)
@@ -102,7 +102,7 @@
     // We pass in our own code block, because the callframe hasn't been populated.
     VM* vm = codeBlock->vm();
     auto scope = DECLARE_THROW_SCOPE(*vm);
-    exec->convertToStackOverflowFrame(*vm);
+    exec->convertToStackOverflowFrame(*vm, codeBlock);
     NativeCallFrameTracer tracer(vm, exec);
     throwStackOverflowError(exec, scope);
 }
@@ -113,8 +113,9 @@
     auto scope = DECLARE_THROW_SCOPE(*vm);
 
     int32_t missingArgCount = CommonSlowPaths::arityCheckFor(exec, *vm, CodeForCall);
-    if (missingArgCount < 0) {
-        exec->convertToStackOverflowFrame(*vm);
+    if (UNLIKELY(missingArgCount < 0)) {
+        CodeBlock* codeBlock = CommonSlowPaths::codeBlockFromCallFrameCallee(exec, CodeForCall);
+        exec->convertToStackOverflowFrame(*vm, codeBlock);
         NativeCallFrameTracer tracer(vm, exec);
         throwStackOverflowError(vm->topCallFrame, scope);
     }
@@ -128,8 +129,9 @@
     auto scope = DECLARE_THROW_SCOPE(*vm);
 
     int32_t missingArgCount = CommonSlowPaths::arityCheckFor(exec, *vm, CodeForConstruct);
-    if (missingArgCount < 0) {
-        exec->convertToStackOverflowFrame(*vm);
+    if (UNLIKELY(missingArgCount < 0)) {
+        CodeBlock* codeBlock = CommonSlowPaths::codeBlockFromCallFrameCallee(exec, CodeForCall);
+        exec->convertToStackOverflowFrame(*vm, codeBlock);
         NativeCallFrameTracer tracer(vm, exec);
         throwStackOverflowError(vm->topCallFrame, scope);
     }
@@ -2432,7 +2434,8 @@
 
 void JIT_OPERATION lookupExceptionHandlerFromCallerFrame(VM* vm, ExecState* exec)
 {
-    exec->convertToStackOverflowFrame(*vm);
+    ASSERT(exec->isStackOverflowFrame());
+    ASSERT(jsCast<ErrorInstance*>(vm->exceptionForInspection()->value().asCell())->isStackOverflowError());
     lookupExceptionHandler(vm, exec);
 }

Modified: trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp (239243 => 239244)


--- trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2018-12-15 01:07:18 UTC (rev 239243)
+++ trunk/Source/_javascript_Core/llint/LLIntSlowPaths.cpp	2018-12-15 02:28:17 UTC (rev 239244)
@@ -559,7 +559,7 @@
     }
 #endif
 
-    exec->convertToStackOverflowFrame(vm);
+    exec->convertToStackOverflowFrame(vm, codeBlock);
     ErrorHandlingScope errorScope(vm);
     throwStackOverflowError(exec, throwScope);
     pc = returnToThrow(exec);

Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp (239243 => 239244)


--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp	2018-12-15 01:07:18 UTC (rev 239243)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.cpp	2018-12-15 02:28:17 UTC (rev 239244)
@@ -177,8 +177,9 @@
 {
     BEGIN();
     int slotsToAdd = CommonSlowPaths::arityCheckFor(exec, vm, CodeForCall);
-    if (slotsToAdd < 0) {
-        exec->convertToStackOverflowFrame(vm);
+    if (UNLIKELY(slotsToAdd < 0)) {
+        CodeBlock* codeBlock = CommonSlowPaths::codeBlockFromCallFrameCallee(exec, CodeForCall);
+        exec->convertToStackOverflowFrame(vm, codeBlock);
         NativeCallFrameTracer tracer(&vm, exec);
         ErrorHandlingScope errorScope(vm);
         throwScope.release();
@@ -192,8 +193,9 @@
 {
     BEGIN();
     int slotsToAdd = CommonSlowPaths::arityCheckFor(exec, vm, CodeForConstruct);
-    if (slotsToAdd < 0) {
-        exec->convertToStackOverflowFrame(vm);
+    if (UNLIKELY(slotsToAdd < 0)) {
+        CodeBlock* codeBlock = CommonSlowPaths::codeBlockFromCallFrameCallee(exec, CodeForCall);
+        exec->convertToStackOverflowFrame(vm, codeBlock);
         NativeCallFrameTracer tracer(&vm, exec);
         ErrorHandlingScope errorScope(vm);
         throwArityCheckStackOverflowError(exec, throwScope);

Modified: trunk/Source/_javascript_Core/runtime/CommonSlowPaths.h (239243 => 239244)


--- trunk/Source/_javascript_Core/runtime/CommonSlowPaths.h	2018-12-15 01:07:18 UTC (rev 239243)
+++ trunk/Source/_javascript_Core/runtime/CommonSlowPaths.h	2018-12-15 02:28:17 UTC (rev 239244)
@@ -72,11 +72,16 @@
     return numberOfStackPaddingSlots(codeBlock, argumentCountIncludingThis) + numberOfExtraSlots(argumentCountIncludingThis);
 }
 
-ALWAYS_INLINE int arityCheckFor(ExecState* exec, VM& vm, CodeSpecializationKind kind)
+ALWAYS_INLINE CodeBlock* codeBlockFromCallFrameCallee(ExecState* exec, CodeSpecializationKind kind)
 {
     JSFunction* callee = jsCast<JSFunction*>(exec->jsCallee());
     ASSERT(!callee->isHostFunction());
-    CodeBlock* newCodeBlock = callee->jsExecutable()->codeBlockFor(kind);
+    return callee->jsExecutable()->codeBlockFor(kind);
+}
+
+ALWAYS_INLINE int arityCheckFor(ExecState* exec, VM& vm, CodeSpecializationKind kind)
+{
+    CodeBlock* newCodeBlock = codeBlockFromCallFrameCallee(exec, kind);
     ASSERT(exec->argumentCountIncludingThis() < static_cast<unsigned>(newCodeBlock->numParameters()));
     int padding = numberOfStackPaddingSlotsWithExtraSlots(newCodeBlock, exec->argumentCountIncludingThis());

Modified: trunk/Source/_javascript_Core/runtime/VM.h (239243 => 239244)


--- trunk/Source/_javascript_Core/runtime/VM.h	2018-12-15 01:07:18 UTC (rev 239243)
+++ trunk/Source/_javascript_Core/runtime/VM.h	2018-12-15 02:28:17 UTC (rev 239244)
@@ -693,6 +693,10 @@
     Exception* lastException() const { return m_lastException; }
     JSCell** addressOfLastException() { return reinterpret_cast<JSCell**>(&m_lastException); }
 
+    // This should only be used for test or assertion code that wants to inspect
+    // the pending exception without interfering with Throw/CatchScopes.
+    Exception* exceptionForInspection() const { return m_exception; }
+
     void setFailNextNewCodeBlock() { m_failNextNewCodeBlock = true; }
     bool getAndClearFailNextNewCodeBlock()
     {

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to