See JSStack::enableErrorStackReserve() and JSStack::disableErrorStackReserve(). Just like this is needed for handling StackOverflowErrors when using the separate JSStack, we need to implement the equivalent for the C stack.
Can we just rely on handling errors inside the host zone? Do we need another concept?
(In reply to comment #1) > Can we just rely on handling errors inside the host zone? Do we need another concept? We need a distinction because we need to allow the VM to use some additional stack space in order to do the work of throwing a StackOverflowError after we have reached the stack limit for normal execution. Otherwise, we’ll encounter a stack overflow while trying to throw the StackOverflowError.
(In reply to comment #2) > (In reply to comment #1) > > Can we just rely on handling errors inside the host zone? Do we need another concept? > > We need a distinction because we need to allow the VM to use some additional stack space in order to do the work of throwing a StackOverflowError after we have reached the stack limit for normal execution. Otherwise, we’ll encounter a stack overflow while trying to throw the StackOverflowError. Yes, that explains what we're trying to do. I have a suggestion for how: What you're calling "JSStack:: enableErrorStackReserve()" should just adjust the stack limit by half the size of the host zone.
(In reply to comment #3) > (In reply to comment #2) > > (In reply to comment #1) > > > Can we just rely on handling errors inside the host zone? Do we need another concept? > > > > We need a distinction because we need to allow the VM to use some additional stack space in order to do the work of throwing a StackOverflowError after we have reached the stack limit for normal execution. Otherwise, we’ll encounter a stack overflow while trying to throw the StackOverflowError. > > Yes, that explains what we're trying to do. I have a suggestion for how: What you're calling "JSStack:: enableErrorStackReserve()" should just adjust the stack limit by half the size of the host zone. Yes, that is pretty much what I had in mind. I hadn’t thought of how much to adjust it by yet, but half the host zone sounds reasonable.
The C stack limit tracking mechanism in the VMEntryScope class already has allowance for a smaller host zone when the Interpreter is in an ErrorMode. We just need to update the VMEntryScope's stack limit whenever the Interpreter enters and exits its ErrorMode.
Created attachment 219845 [details] the patch.
Comment on attachment 219845 [details] the patch. Wrong patch.
Created attachment 219846 [details] the real patch.
Comment on attachment 219846 [details] the real patch. View in context: https://bugs.webkit.org/attachment.cgi?id=219846&action=review > Source/JavaScriptCore/interpreter/JSStack.cpp:189 > +#else > + if (m_vm.entryScope) > + m_vm.entryScope->updateStackLimit(); > +#endif After thinking about this some more, the C stack update should be unconditional independent of whether the JS stack is on the C stack or its own separate stack. We may have detected a stack overflow due to the C stack reaching it's limit. Hence, the VM may need more C stack space to process the error even if the JS stack is on its own separate stack. I will upload an updated patch.
Created attachment 219847 [details] patch 2.
Landed in r160967 on the jsCStack branch: <http://trac.webkit.org/r160967>.
Thanks for the review. Review status updated in r161117: <http://trac.webkit.org/r161117>.