Bug 141676

Summary: DFG JIT needs to check for stack overflow at the start of Program and Eval execution
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, buildbot, fpizlo, ossy, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 141848    
Bug Blocks: 141098    
Attachments:
Description Flags
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews105 for mac-mavericks-wk2
none
Archive of layout-test-results from ews101 for mac-mavericks
none
Updated test to work as both a layout test and a JSC test fpizlo: review+

Michael Saboff
Reported 2015-02-16 16:22:03 PST
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.
Attachments
Patch (12.25 KB, patch)
2015-02-19 22:23 PST, Michael Saboff 		buildbot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (666.29 KB, application/zip)
2015-02-19 23:14 PST, Build Bot 		no flags
Details
Archive of layout-test-results from ews101 for mac-mavericks (519.47 KB, application/zip)
2015-02-19 23:34 PST, Build Bot 		no flags
Details
Updated test to work as both a layout test and a JSC test (12.27 KB, patch)
2015-02-20 07:41 PST, Michael Saboff 		fpizlo: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2015-02-16 16:22:32 PST
<rdar://problem/19854012>
Michael Saboff
Comment 2 2015-02-19 22:23:53 PST
Created attachment 246938 [details] Patch
Build Bot
Comment 3 2015-02-19 23:14:42 PST
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
Comment 4 2015-02-19 23:14:45 PST
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 5 2015-02-19 23:34:45 PST
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
Comment 6 2015-02-19 23:34:48 PST
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
Comment 7 2015-02-20 07:41:49 PST
Created attachment 246970 [details] Updated test to work as both a layout test and a JSC test
Filip Pizlo
Comment 8 2015-02-20 10:31:59 PST
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
Comment 9 2015-02-20 10:33:26 PST
(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
Comment 10 2015-02-20 10:39:08 PST
Committed r180423: <http://trac.webkit.org/changeset/180423>
Csaba Osztrogonác
Comment 11 2015-02-20 12:10:25 PST
(In reply to comment #10) > Committed r180423: <http://trac.webkit.org/changeset/180423> The new test fails on the 32 bit bots.
Michael Saboff
Comment 12 2015-02-20 12:31:17 PST
(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
Comment 13 2015-02-20 12:32:20 PST
It fails on the 32-bit Windows. I filed Bug 141848 to track this new problem.
Brent Fulgham
Comment 14 2015-02-20 12:39:54 PST
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
Comment 15 2015-02-24 15:33:09 PST
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>
Note You need to log in before you can comment on or make changes to this bug.