722023 - Assertion failure: !isIndex(&dummy), at js/src/vm/String.h:857

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test asserts on mozilla-central revision 8a59519e137e (options -m -n -a, 32 bit):


var obj = new Object();
var index = [ (null ), 1073741824, 1073741825 ];
for (var j in index) { 
  testProperty(index[j]);
}
function testProperty(i) { 
  actual = obj[String(i)];
}


This bug pops up way too often during fuzzing (and I thought it was already filed/fixed because the same assert was there some weeks ago already), would be good if someone could fix it :D

Comment 1

[Jeff Walden [:Waldo]]

This assertion's at a pinch-point, so you have to look at the caller location to bucket instances of it.  And if you hit it, please do report it quickly.  When the actual property storage split happens, this assertion will become significantly more worrisome if hit.  Right now, because everything still uses the same property store, an atom that's not a PropertyName is somewhat less likely to indicate substantial cause for alarm, in terms of the failure mode afterward in a release build.

You're likely to find more of this assertion when fuzzing.  Prior code didn't make the distinction PropertyName is meant to enforce, and it's not always easy to make it because the values in question flow in from only semi-typed sources (like immediates in bytecode).  There's also the problem of conflation of concepts here amongst jsid, JSAtom*, and integer-valued jsids.  Here the problem lies with this code and jsid/atom:

(gdb) up
#4  0x00000000007da90c in js::mjit::ic::GetElementIC::update (this=0xbe25c0, f=..., obj=0x7ffff150f740, v=..., id=..., vp=0x7ffff1741138)
    at /home/jwalden/moz/slots/js/src/methodjit/PolyIC.cpp:2611
2611	        return attachGetProp(f, obj, v, JSID_TO_ATOM(id)->asPropertyName(), vp);
(gdb) lis
2606	     * GETPROP IC assumes the id has already gone through filtering for string
2607	     * indexes in the emitter, i.e. js_GetProtoIfDenseArray is only valid to
2608	     * use when looking up non-integer identifiers.
2609	     */
2610	    if (v.isString() && js_CheckForStringIndex(id) == id)
2611	        return attachGetProp(f, obj, v, JSID_TO_ATOM(id)->asPropertyName(), vp);
...
(gdb) p js_DumpAtom(this)
JSAtom* (0x7ffff1627f40) = JSString* (0x7ffff1627f40) = jschar * (0x7ffff1627f50) = "1073741825"

The reference to "string indexes" refers to the distinction jsid makes, which says -2**31 to 2**31 - 1 (possibly with some exponent off-by-ones or something) is a special kind of id.  But "index" in the context of PropertyName means 0 to 2**32 - 1.  You can see how this is more than a bit confusing.

The fix here probably involves reverting some portions of this change, until the actual split happens a bit more:

changeset:   83717:d0e3133d19e2
user:        Jeff Walden <[e-mail]>
date:        Tue Dec 27 02:27:02 2011 -0600
summary:     Bug 713183 - Make JSOP_*PROP and JSOP_*NAME store a PropertyName immediate, not a JSAtom immediate, and take advantage of this fact.  r=bhackett

Comment 2

[Jeff Walden [:Waldo]]

I should probably take this and finish it before the merge.  Patch tomorrow.

Comment 3

[Bob Clary [:bc] (inactive)]

This is reproducible at http://www.nfl.com/gamecenter/2012012201/2011/POST20/giants%4049ers#menu=drivechart%26tab=track and another url which I'll omit since it locates someones house.

Comment 4

[Jeff Walden [:Waldo]]

Attachment: Patch with test

Comment 5

[Jeff Walden [:Waldo]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/02e01981f362

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/02e01981f362

Comment 7

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929