RESOLVED FIXED Bug 65869
JSC should always throw when function arg list is too long
https://bugs.webkit.org/show_bug.cgi?id=65869
Summary JSC should always throw when function arg list is too long
Peter Kasting
Reported 2011-08-08 13:13:40 PDT
After http://trac.webkit.org/changeset/62432 fixed https://bugs.webkit.org/show_bug.cgi?id=41351 by making JSC truncate excessive arguments to function.apply (matching Spidermonkey), I filed https://bugzilla.mozilla.org/show_bug.cgi?id=578705 about changing Spidermonkey to simply always throw an exception for a too-long argument list. That bug has now been marked Fixed, so I think at this point Spidermonkey and V8 throw exceptions consistently, meaning JSC should probably be changed to match.
Attachments
Patch (15.38 KB, patch)
2011-08-10 11:17 PDT, Mark Hahnenberg
no flags
Patch (288.04 KB, patch)
2011-08-10 12:56 PDT, Mark Hahnenberg
no flags
Changing broken tests. (3.36 KB, patch)
2011-08-10 16:41 PDT, Mark Hahnenberg
no flags
Geoffrey Garen
Comment 1 2011-08-09 17:51:35 PDT
Mark Hahnenberg
Comment 2 2011-08-10 11:17:12 PDT
Oliver Hunt
Comment 3 2011-08-10 11:36:16 PDT
Comment on attachment 103508 [details] Patch I think it would be ebtter to just use createStackoverflow (or whatever the function is called) because that's kind of what we're pretending the issue is. Also you should probably test the behaviour of function test() { print(arguments.length); } function f() { test.apply(null, arguments); } f(0,0,0,0,0,0,0,0,...<70000 arguments>) As the jit inlines apply(..., arguments)
Mark Hahnenberg
Comment 4 2011-08-10 12:56:17 PDT
WebKit Review Bot
Comment 5 2011-08-10 14:42:41 PDT
Comment on attachment 103521 [details] Patch Clearing flags on attachment: 103521 Committed r92797: <http://trac.webkit.org/changeset/92797>
WebKit Review Bot
Comment 6 2011-08-10 14:42:46 PDT
All reviewed patches have been landed. Closing bug.
Mark Hahnenberg
Comment 7 2011-08-10 16:41:49 PDT
Created attachment 103557 [details] Changing broken tests.
Mark Hahnenberg
Comment 8 2011-08-10 16:42:31 PDT
I missed a few of the tests the change affected, so we need to fix those as well.
WebKit Review Bot
Comment 9 2011-08-10 17:50:54 PDT
Comment on attachment 103557 [details] Changing broken tests. Clearing flags on attachment: 103557 Committed r92807: <http://trac.webkit.org/changeset/92807>
WebKit Review Bot
Comment 10 2011-08-10 17:50:59 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.