Bug 154475 - Builtins that should not rely on iteration do.
Summary: Builtins that should not rely on iteration do.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-02-19 14:52 PST by Mark S. Miller
Modified: 2016-02-22 11:43 PST (History)
6 users (show)

See Also:


Attachments
Open this web page in Safari and Webkit Nightly, and open the console (1.47 KB, text/html)
2016-02-19 14:52 PST, Mark S. Miller
no flags Details
Screenshot showing Safari working and WebKit failing the badbind.html test for this bug (1.18 MB, image/png)
2016-02-19 14:53 PST, Mark S. Miller
no flags Details
Patch (2.98 KB, patch)
2016-02-22 09:41 PST, Keith Miller
no flags Details | Formatted Diff | Diff
Benchmark results (64.81 KB, text/plain)
2016-02-22 11:08 PST, Keith Miller
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark S. Miller 2016-02-19 14:52:11 PST
Created attachment 271806 [details]
Open this web page in Safari and Webkit Nightly, and open the console

Now that the latest WebKit seems to mostly work again on the SES selftest page
https://rawgit.com/tvcutsem/es-lab/master/src/ses/contract.html
I was seeing some new failures. But this time I was able to debug (thanks!)
and isolate the problem.

The new problem is that the builtins used by
http://wiki.ecmascript.org/doku.php?id=conventions:safe_meta_programming
all of which redate ES6 now depend on iterator.next behavior in violation of the ES6 spec.
You can see it yourself by opening the badbind.html in Safari and Webkit Nightly, opening
the console, and seeing what's there. Works fine on Safari and on every other browser I
tested. Fails on the latest WebKit nightly.

Note that I'll be updating the SES selftest page to stop removing .next, because of this
bug, so the SES selftest page will stop being a good test.
Comment 1 Mark S. Miller 2016-02-19 14:53:57 PST
Created attachment 271807 [details]
Screenshot showing Safari working and WebKit failing the badbind.html test for this bug
Comment 2 Keith Miller 2016-02-19 15:09:14 PST
I think this is because of my patch (http://trac.webkit.org/changeset/196734). I didn't realize that arguments iterator was observable from outside it's scope. Working on this now.
Comment 3 Mark S. Miller 2016-02-19 15:58:36 PST
Although this specific test passes on all other browsers, note that Chrome50 Canary has a similar but different problem: 
https://bugs.chromium.org/p/v8/issues/detail?id=4769
Comment 4 Keith Miller 2016-02-22 09:41:37 PST
Created attachment 271926 [details]
Patch
Comment 5 Keith Miller 2016-02-22 11:08:36 PST
Created attachment 271931 [details]
Benchmark results
Comment 6 Geoffrey Garen 2016-02-22 11:38:07 PST
Comment on attachment 271926 [details]
Patch

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

r=me

> Source/JavaScriptCore/builtins/FunctionPrototype.js:33
> +        @putByValDirect(argumentValues, i-1, arguments[i]);

Spacing around - 1.
Comment 7 WebKit Commit Bot 2016-02-22 11:42:58 PST
Comment on attachment 271926 [details]
Patch

Clearing flags on attachment: 271926

Committed r196949: <http://trac.webkit.org/changeset/196949>
Comment 8 WebKit Commit Bot 2016-02-22 11:43:03 PST
All reviewed patches have been landed.  Closing bug.