Bug 141676 – DFG JIT needs to check for stack overflow at the start of Program and Eval execution

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

The fix for bug <https://bugs.webkit.org/show_bug.cgi?id=141098 - "Google doc spreadsheet reproducibly crashes when sorting" added stack overflow checks for Eval and Program executions.  The follow on fix for bug <https://bugs.webkit.org/show_bug.cgi?id=141577> - "REGRESSION(r180060) New js/regress-141098 test crashes when LLInt is disabled." added similar checks for the baseline JIT.

This same type of checking needs to be added to the DFG.  The FTL path doesn't need this checking as it is the caller to an FTL function that needs to verify there is enough stack space to call an FTL compiled function.

[Radar WebKit Bug Importer]

<rdar://problem/19854012>

[Michael Saboff]

Created attachment 246938 [details]
Patch

[Build Bot]

Comment on attachment 246938 [details]
Patch

Attachment 246938 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4900742427049984

New failing tests:
js/regress-141098.html

[Build Bot]

Created attachment 246940 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 246938 [details]
Patch

Attachment 246938 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6270187816878080

New failing tests:
js/regress-141098.html

[Build Bot]

Created attachment 246941 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Michael Saboff]

Created attachment 246970 [details]
Updated test to work as both a layout test and a JSC test

[Filip Pizlo]

Comment on attachment 246970 [details]
Updated test to work as both a layout test and a JSC test

View in context: https://bugs.webkit.org/attachment.cgi?id=246970&action=review

> LayoutTests/js/script-tests/regress-141098.js:4
>  description("Regression test for https://webkit.org/b/141098. Make sure eval() properly handles running out of stack space. This test should run without crashing.");
>  
> -function probeAndRecurse(depth)
> +var lastEvalString = "";
> +

Somewhere at the top here you should have a comment saying that this only tests the DFG if run in run-jsc-stress-tests with the eager settings.

[Michael Saboff]

(In reply to comment #8)
> Comment on attachment 246970 [details]
> Updated test to work as both a layout test and a JSC test
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=246970&action=review
> 
> > LayoutTests/js/script-tests/regress-141098.js:4
> >  description("Regression test for https://webkit.org/b/141098. Make sure eval() properly handles running out of stack space. This test should run without crashing.");
> >  
> > -function probeAndRecurse(depth)
> > +var lastEvalString = "";
> > +
> 
> Somewhere at the top here you should have a comment saying that this only
> tests the DFG if run in run-jsc-stress-tests with the eager settings.

I'll add one.

[Michael Saboff]

Committed r180423: <http://trac.webkit.org/changeset/180423>

[Csaba Osztrogonác]

(In reply to comment #10)
> Committed r180423: <http://trac.webkit.org/changeset/180423>

The new test fails on the 32 bit bots.

[Michael Saboff]

(In reply to comment #11)
> (In reply to comment #10)
> > Committed r180423: <http://trac.webkit.org/changeset/180423>
> 
> The new test fails on the 32 bit bots.

Windows seems to be failing as well.

In both cases, it looks like we don't see the second out of stack exception.

Investigating.

[Brent Fulgham]

It fails on the 32-bit Windows. I filed Bug 141848 to track this new problem.

[Brent Fulgham]

This also introduced these JSC test failures on Windows:

** The following JSC stress test failures have been introduced:
	jsc-layout-tests.yaml/js/script-tests/regress-141098.js.layout
	jsc-layout-tests.yaml/js/script-tests/regress-141098.js.layout-dfg-eager-no-cjit
	jsc-layout-tests.yaml/js/script-tests/regress-141098.js.layout-no-cjit
	jsc-layout-tests.yaml/js/script-tests/regress-141098.js.layout-no-llint

[Michael Saboff]

The Windows / 32bit test failures were fixed as part of https://bugs.webkit.org/show_bug.cgi?id=141848 and landed in change set r180453: <http://trac.webkit.org/changeset/180453>