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

RESOLVED FIXED in mozilla12

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: decoder, Assigned: Waldo)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

Trunk
mozilla12
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js-triage-done])

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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
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 <jwalden@mit.edu>
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
I should probably take this and finish it before the merge.  Patch tomorrow.
Assignee: general → jwalden+bmo
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86 → All
Whiteboard: js-triage-needed → [js-triage-done]

Comment 3

5 years ago
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.
Blocks: 532972
Created attachment 592528 [details] [diff] [review]
Patch with test
Attachment #592528 - Flags: review?(bhackett1024)
Attachment #592528 - Flags: review?(bhackett1024) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/02e01981f362
Target Milestone: --- → mozilla12

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/02e01981f362
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

4 years ago
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.