Bug 148076

Created attachment 259145 [details] Patch

Attachment 259145 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/jit/JITExceptions.h:38: The parameter name "unwindStart" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 259145 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259145&action=review > Source/JavaScriptCore/ChangeLog:32 > + (JSC::LLInt::llint_stack_check): Changed to process the exception in the currnet frame. currnet -> current > Source/JavaScriptCore/jit/JITExceptions.cpp:67 > + callFrame = callFrame->callerFrame(vmEntryFrame); Nit: You can directly use callFrame->callerFrame() here. > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:487 > - CommonSlowPaths::interpreterThrowInCaller(exec, createStackOverflowError(exec)); > - pc = returnToThrowForThrownException(exec); > + vm.throwException(exec, createStackOverflowError(exec)); > + pc = returnToThrow(exec); I think this should be consistent amongst all the tiers, see below. > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:923 > - subp maxFrameExtentForSlowPathCall, sp # Set up temporary stack pointer for call > + # Set up temporary stack pointer for call including callee saves > + subp maxFrameExtentForSlowPathCall + CalleeSaveSpaceStackAligned, sp If I understand correctly, the callee save registers are saved below the stack pointer at this point. Could we instead move the saving of callee save after the stack check/setup (which would allow us to continue throwing in the caller)?

(In reply to comment #3) > Comment on attachment 259145 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=259145&action=review > > > Source/JavaScriptCore/ChangeLog:32 > > + (JSC::LLInt::llint_stack_check): Changed to process the exception in the currnet frame. > > currnet -> current Fixed locally. > > Source/JavaScriptCore/jit/JITExceptions.cpp:67 > > + callFrame = callFrame->callerFrame(vmEntryFrame); > > Nit: You can directly use callFrame->callerFrame() here. Changed locally. > > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:487 > > - CommonSlowPaths::interpreterThrowInCaller(exec, createStackOverflowError(exec)); > > - pc = returnToThrowForThrownException(exec); > > + vm.throwException(exec, createStackOverflowError(exec)); > > + pc = returnToThrow(exec); > > I think this should be consistent amongst all the tiers, see below. The reason for the difference is that the LLInt makes a call to lint_stack_check() and the way these slow path calls work is that they pass the PC as the second argument and return the possibly modified PC as the first of two values. In the LLInt, the PC value is a callee save and therefore it is easiest to preserve the callee saves, set up the PC before making the stack check call. Given that we need to unwind and restore callee saves for the frame where the stack overflow occurred. The JITs don't have this issue. > > Source/JavaScriptCore/llint/LowLevelInterpreter.asm:923 > > - subp maxFrameExtentForSlowPathCall, sp # Set up temporary stack pointer for call > > + # Set up temporary stack pointer for call including callee saves > > + subp maxFrameExtentForSlowPathCall + CalleeSaveSpaceStackAligned, sp > > If I understand correctly, the callee save registers are saved below the > stack pointer at this point. Could we instead move the saving of callee save > after the stack check/setup (which would allow us to continue throwing in > the caller)? See above.

Committed r188555: <http://trac.webkit.org/changeset/188555>

*** This bug has been marked as a duplicate of bug 148666 ***

Comment on attachment 259145 [details] Patch Cleared review? from attachment 259145 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).