Bug 133577 - In a certain app state, Array.prototype.filter() returns incorrect results
Summary: In a certain app state, Array.prototype.filter() returns incorrect results
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.9
: P2 Normal
Assignee: Michael Saboff
URL: http://jsbin.com/potewaye/13/edit?js,...
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-06-06 09:59 PDT by Ryan Grove
Modified: 2014-06-10 11:00 PDT (History)
4 users (show)

See Also:


Attachments
Array.prototype.filter() bug test case (1.22 KB, text/html)
2014-06-06 09:59 PDT, Ryan Grove
no flags Details
Patch (6.36 KB, patch)
2014-06-09 18:51 PDT, Michael Saboff
no flags Details | Formatted Diff | Diff
Patch with updated test from comment (6.44 KB, patch)
2014-06-10 10:53 PDT, Michael Saboff
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Grove 2014-06-06 09:59:19 PDT
Created attachment 232618 [details]
Array.prototype.filter() bug test case

The Router component in the YUI JS library somehow induces a certain state that temporarily causes Array.prototype.filter() to return incorrect results (an empty array instead of a correctly filtered array).

I've been unable to track down the precise cause of this state, but it's at least consistently reproducible using the attached test case, which is also visible at http://jsbin.com/potewaye/13/edit?js,console

The test succeeds in all browsers except Safari 8 on OS X Yosemite and recent WebKit nightlies (I tested 9537.76.4, r169635). I don't think the problem lies with YUI, since YUI's router has been in wide production use for several years and this problem only surfaced in these brand new WebKit builds.

http://www.smugmug.com/ is one production website affected by this bug, but there are many others using the YUI Router, including various Yahoo sites.

Steps to Reproduce:
1. Run the attached test case.

Expected Results:
Three separate arrays should be logged to the console demonstrating that Array.prototype.filter() is working properly: ["started"], ["finished"], and ["foo"].

Actual Results:
In Safari 8 and WebKit 9537.76.4, r169635, the logged arrays are ["started"], ["finished"], and [], and you'll see the error message "Array.prototype.filter() failed!" indicating that Array.prototype.filter() returned incorrect results when run inside a YUI router callback.

I also filed this as rdar://17186034
Comment 1 Benjamin Poulain 2014-06-06 14:45:44 PDT
<rdar://problem/17186034>
Comment 2 Michael Saboff 2014-06-09 10:31:59 PDT
Original radar - <rdar://problem/17019752>
Comment 3 Michael Saboff 2014-06-09 18:51:20 PDT
Created attachment 232757 [details]
Patch
Comment 4 Oliver Hunt 2014-06-09 21:17:13 PDT
Comment on attachment 232757 [details]
Patch

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

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:1714
> +    bieq ArrayStorage::m_vector + TagOffset[t0, t3, 8], EmptyValueTag, .opPutByValArrayStorageEmpty

Could you add a test to make sure that we don't call setters or anything when doing a put by val direct on a hole?
Comment 5 Michael Saboff 2014-06-10 10:53:29 PDT
Created attachment 232796 [details]
Patch with updated test from comment
Comment 6 Michael Saboff 2014-06-10 10:58:37 PDT
Committed r169751: <http://trac.webkit.org/changeset/169751>
Comment 7 Geoffrey Garen 2014-06-10 11:00:16 PDT
Comment on attachment 232796 [details]
Patch with updated test from comment

r=me