1138676 - Crashes in jit::PropertyReadNeedsTypeBarrier

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

We have a new topcrash on Nightly (#7 or so) in PropertyReadNeedsTypeBarrier. We're crashing here:

    JSObject *obj;
    if (key->isSingleton())
        obj = key->singleton();
    else
        obj = key->proto().toObjectOrNull();

    while (obj) {
        if (!obj->getClass()->isNative())

On the last line, crash address is 0x5 or 0x9 so most likely |obj| is 0x1, aka TaggedProto::LazyProto.

While this is easy to fix by checking for this case before we call toObjectOrNull(), I don't understand how we can have a LazyProto here. AFAIK those are only used for proxies, and in that case we should have an unknown TypeSet and we shouldn't even end up in this function.

Brian, do you know what could cause this?

https://crash-stats.mozilla.com/report/index/93aebbae-cc72-498a-a790-0fd702150225

Comment 1

[Jan de Mooij [:jandem]]

This first appeared in the 02/25 Nightly, assuming I'm using crash-stats correctly. There were some unboxed-object changes around that time, not sure what else could have caused this..

Comment 2

[Brian Hackett [Laid off!]]

Attachment: potential patch

Well, it's hard to say how we could end up here.  This is the only place I could find in the Ion frontend where we use toObjectOrNull without first checking unknownProperties, so even if the object has unknown properties we can still end up here.  Now, normally sets with unknown properties will be marked as ANYOBJECT and not get here, but since bug 1116017 we can't assume that will have happened, and maybe the set has unknown properties given to it at some point after creation.  Or maybe the type set being crawled was created by some method like the TemporaryTypeSet(LifoAlloc*, Type) constructor which doesn't have all the same checks as TypeSet::addType().

In any case, this patch improves our checking at this site, since it is definitely possible to get a group with a lazy proto here.

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8571642 [details] [diff] [review]
potential patch

Review of attachment 8571642 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 4

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bae236f4b8c1

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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