Attachment 357344 Details for Bug 192717 – proposed patch.

[patch] proposed patch.

bug-192717.patch (text/plain), 8.67 KB, created by Mark Lam on 2018-12-14 14:32:27 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Lam

Created: 2018-12-14 14:32:27 PST

Size: 8.67 KB

patch

obsolete

>Index: JSTests/ChangeLog
>===================================================================
>--- JSTests/ChangeLog	(revision 239232)
>+++ JSTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2018-12-14  Mark Lam  <mark.lam@apple.com>
>+
>+        CallFrame::convertToStackOverflowFrame() needs to keep the top CodeBlock alive.
>+        https://bugs.webkit.org/show_bug.cgi?id=192717
>+        <rdar://problem/46660677>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * stress/regress-192717.js: Added.
>+
> 2018-12-14  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r239153, r239154, and r239155.
>Index: JSTests/stress/regress-192717.js
>===================================================================
>--- JSTests/stress/regress-192717.js	(nonexistent)
>+++ JSTests/stress/regress-192717.js	(working copy)
>@@ -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";
>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 239225)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,37 @@
>+2018-12-14  Mark Lam  <mark.lam@apple.com>
>+
>+        CallFrame::convertToStackOverflowFrame() needs to keep the top CodeBlock alive.
>+        https://bugs.webkit.org/show_bug.cgi?id=192717
>+        <rdar://problem/46660677>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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.
>+
>+        * interpreter/CallFrame.cpp:
>+        (JSC::CallFrame::convertToStackOverflowFrame):
>+        * interpreter/CallFrame.h:
>+        * interpreter/Interpreter.cpp:
>+        (JSC::UnwindFunctor::copyCalleeSavesToEntryFrameCalleeSavesBuffer const):
>+        * jit/JITOperations.cpp:
>+        * llint/LLIntSlowPaths.cpp:
>+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
>+        * runtime/CommonSlowPaths.cpp:
>+        (JSC::SLOW_PATH_DECL):
>+
> 2018-12-13  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         [WinCairo][Clang] DLLLauncherMain.cpp: warning: unused function 'prependPath' and 'appleApplicationSupportDirectory'
>Index: Source/JavaScriptCore/interpreter/CallFrame.cpp
>===================================================================
>--- Source/JavaScriptCore/interpreter/CallFrame.cpp	(revision 239225)
>+++ Source/JavaScriptCore/interpreter/CallFrame.cpp	(working copy)
>@@ -337,9 +337,10 @@ const char* CallFrame::describeFrame()
>     return buffer;
> }
> 
>-void CallFrame::convertToStackOverflowFrame(VM& vm)
>+void CallFrame::convertToStackOverflowFrame(VM& vm, CodeBlock* codeBlockToKeepAliveUntilFrameIsUnwound)
> {
>     ASSERT(!isGlobalExec());
>+    ASSERT(jsDynamicCast<CodeBlock*>(vm, codeBlockToKeepAliveUntilFrameIsUnwound));
> 
>     EntryFrame* entryFrame = vm.topEntryFrame;
>     CallFrame* throwOriginFrame = this;
>@@ -350,7 +351,7 @@ void CallFrame::convertToStackOverflowFr
>     JSObject* originCallee = throwOriginFrame ? throwOriginFrame->jsCallee() : vmEntryRecord(vm.topEntryFrame)->callee();
>     JSObject* stackOverflowCallee = originCallee->globalObject()->stackOverflowFrameCallee();
> 
>-    setCodeBlock(nullptr);
>+    setCodeBlock(codeBlockToKeepAliveUntilFrameIsUnwound);
>     setCallee(stackOverflowCallee);
>     setArgumentCountIncludingThis(0);
> }
>Index: Source/JavaScriptCore/interpreter/CallFrame.h
>===================================================================
>--- Source/JavaScriptCore/interpreter/CallFrame.h	(revision 239225)
>+++ Source/JavaScriptCore/interpreter/CallFrame.h	(working copy)
>@@ -257,7 +257,7 @@ namespace JSC  {
>             return callerFrameAndPC().callerFrame == noCaller() && callerFrameAndPC().returnPC == nullptr;
>         }
> 
>-        void convertToStackOverflowFrame(VM&);
>+        void convertToStackOverflowFrame(VM&, CodeBlock* codeBlockToKeepAliveUntilFrameIsUnwound);
>         inline bool isStackOverflowFrame() const;
>         inline bool isWasmFrame() const;
> 
>Index: Source/JavaScriptCore/jit/JITOperations.cpp
>===================================================================
>--- Source/JavaScriptCore/jit/JITOperations.cpp	(revision 239225)
>+++ Source/JavaScriptCore/jit/JITOperations.cpp	(working copy)
>@@ -102,7 +102,7 @@ void JIT_OPERATION operationThrowStackOv
>     // 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);
> }
>@@ -114,7 +114,7 @@ int32_t JIT_OPERATION operationCallArity
> 
>     int32_t missingArgCount = CommonSlowPaths::arityCheckFor(exec, *vm, CodeForCall);
>     if (missingArgCount < 0) {
>-        exec->convertToStackOverflowFrame(*vm);
>+        exec->convertToStackOverflowFrame(*vm, exec->codeBlock());
>         NativeCallFrameTracer tracer(vm, exec);
>         throwStackOverflowError(vm->topCallFrame, scope);
>     }
>@@ -129,7 +129,7 @@ int32_t JIT_OPERATION operationConstruct
> 
>     int32_t missingArgCount = CommonSlowPaths::arityCheckFor(exec, *vm, CodeForConstruct);
>     if (missingArgCount < 0) {
>-        exec->convertToStackOverflowFrame(*vm);
>+        exec->convertToStackOverflowFrame(*vm, exec->codeBlock());
>         NativeCallFrameTracer tracer(vm, exec);
>         throwStackOverflowError(vm->topCallFrame, scope);
>     }
>@@ -2432,7 +2432,7 @@ void JIT_OPERATION lookupExceptionHandle
> 
> void JIT_OPERATION lookupExceptionHandlerFromCallerFrame(VM* vm, ExecState* exec)
> {
>-    exec->convertToStackOverflowFrame(*vm);
>+    exec->convertToStackOverflowFrame(*vm, exec->codeBlock());
>     lookupExceptionHandler(vm, exec);
> }
> 
>Index: Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>===================================================================
>--- Source/JavaScriptCore/llint/LLIntSlowPaths.cpp	(revision 239225)
>+++ Source/JavaScriptCore/llint/LLIntSlowPaths.cpp	(working copy)
>@@ -559,7 +559,7 @@ LLINT_SLOW_PATH_DECL(stack_check)
>     }
> #endif
> 
>-    exec->convertToStackOverflowFrame(vm);
>+    exec->convertToStackOverflowFrame(vm, codeBlock);
>     ErrorHandlingScope errorScope(vm);
>     throwStackOverflowError(exec, throwScope);
>     pc = returnToThrow(exec);
>Index: Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/CommonSlowPaths.cpp	(revision 239225)
>+++ Source/JavaScriptCore/runtime/CommonSlowPaths.cpp	(working copy)
>@@ -178,7 +178,7 @@ SLOW_PATH_DECL(slow_path_call_arityCheck
>     BEGIN();
>     int slotsToAdd = CommonSlowPaths::arityCheckFor(exec, vm, CodeForCall);
>     if (slotsToAdd < 0) {
>-        exec->convertToStackOverflowFrame(vm);
>+        exec->convertToStackOverflowFrame(vm, exec->codeBlock());
>         NativeCallFrameTracer tracer(&vm, exec);
>         ErrorHandlingScope errorScope(vm);
>         throwScope.release();
>@@ -193,7 +193,7 @@ SLOW_PATH_DECL(slow_path_construct_arity
>     BEGIN();
>     int slotsToAdd = CommonSlowPaths::arityCheckFor(exec, vm, CodeForConstruct);
>     if (slotsToAdd < 0) {
>-        exec->convertToStackOverflowFrame(vm);
>+        exec->convertToStackOverflowFrame(vm, exec->codeBlock());
>         NativeCallFrameTracer tracer(&vm, exec);
>         ErrorHandlingScope errorScope(vm);
>         throwArityCheckStackOverflowError(exec, throwScope);

Actions: View | Formatted Diff | Diff

Attachments on bug 192717: 357344 | 357357