In a certain app state, Array.prototype.filter() returns incorrect results

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Ryan Grove]

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

[Benjamin Poulain]

<rdar://problem/17186034>

[Michael Saboff]

Original radar - <rdar://problem/17019752>

[Michael Saboff]

Created attachment 232757 [details]
Patch

[Oliver Hunt]

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?

[Michael Saboff]

Created attachment 232796 [details]
Patch with updated test from comment

[Michael Saboff]

Committed r169751: <http://trac.webkit.org/changeset/169751>

[Geoffrey Garen]

Comment on attachment 232796 [details]
Patch with updated test from comment

r=me