457920 - regexp-dna.js and generality want JSOP_GETELEM(dense array, "0")

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Brendan Eich [:brendan]]

And so on -- for-in loops produce string property names, not uints, but array_getProperty can cope. So must TM.

/be

Comment 1

[Brendan Eich [:brendan]]

Attachment: proposed fix

This patch guards on holes in dense arrays. We can't see through them, except in the general case of obj[str] = foo where obj is a dense array -- in that case the Array_dense_setelem built-in, like the interpreter's JSOP_SETELEM case, will test the new runtime-wide anyArrayProtoHasElement flag and cope.

On the up side, this uses LIR_ult to combine two guards into one under guardDenseArrayIndex.

/be

Comment 2

[Brendan Eich [:brendan]]

Correctness issues discovered and patched here, needed for b2.

/be

Comment 3

[Andreas Gal :gal]

Comment on attachment 341340 [details] [diff] [review]
proposed fix

Need a 2nd pair of eyes for the interpreter side of things. Blake?

Comment 4

[Brendan Eich [:brendan]]

Attachment: wdiff

Comment 5

[Andreas Gal :gal]

Comment on attachment 341340 [details] [diff] [review]
proposed fix

I am unsure about this part: 

-    guard(false, lir->ins_eq0(dslots_ins), MISMATCH_EXIT);

Maybe you could add a code comment why thats not necessary.

Comment 6

[Brendan Eich [:brendan]]

Commented and committed, but I didn't check for orange first! Something committed before 21:02:51 seems to have turned the TraceMonkey Windows box orange.

http://hg.mozilla.org/tracemonkey/rev/98d37f459fef

/be

Comment 7

[Brendan Eich [:brendan]]

Orange looks like sync xhr test flapping, went green again. Hope it's gone when we update from m-c next.

/be

Comment 8

[Brendan Eich [:brendan]]

Fixed on m-c:

http://hg.mozilla.org/mozilla-central/rev/98d37f459fef

/be

Comment 9

[Aakash Desai [:aakashd]]

Seeing as there hasn't been any discussions about this bug for 5 months, I'm
guessing there aren't any residual issues. I'm moving this to verified as a
result. If anyone has any qualms, feel free to bring them up.