1276353 - Crashes in mozilla::jsipc::DOMInstanceOf

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This bug was filed from the Socorro interface and is 
report bp-abca1cad-d5e0-4fab-b17c-8fba32160527.
=============================================================

There have been crashes with signature mozilla::jsipc::DOMInstanceOf in the May 25 and May 26 nightlies.  They seem to be across a bunch of users, though each user seems to be crashing multiple times.  They're all on 32-bit Windows, which is actually unusual for nightly, and they all have a crash address of 0x8, which means a null dereference.

Query:
https://crash-stats.mozilla.com/signature/?product=Firefox&release_channel=nightly&date=%3E%3D2016-05-01&signature=mozilla%3A%3Ajsipc%3A%3ADOMInstanceOf

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Any idea where I should direct this?  I don't see obvious code changes in this area recently.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Somewhat suspicious that all of the crashes were in an 11 minute window (2016-05-27 02:28 to 02:39, UTC), even though from multiple users.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Er, actually, I don't think it's multiple users.  It looks like a single user who crashed twice, updated nightly, and then crashed 11 more times.  I think I was confusing things with the previous crash I looked at.

So maybe not something we should worry about at this stage.

Comment 4

[Boris Zbarsky [:bzbarsky]]

It's hard to say where inside the FORWARD macro we're crashing here, but presumably either proxy or OwnerOf(proxy) is null.

"proxy" comes from dom::InterfaceHasInstance, but that code hasn't changed since Firefox 33 either.

The other weird thing here is that the caller in InterfaceHasInstance is conditioned on IsWrappedCPOW and then does a CheckedUnwrap to get the "proxy".  IsWrappedCPOW used to do a CheckedUnwrap as well, and return false if that returned null.  But it was changed to an UncheckedUnwrap in bug 1057601... in Firefox 34.

My best bet is that for some reason we started having security wrappers around CPOWs that in fact _can't_ be unwrapped by CheckedUnwrap coming through this code, triggering the latent bug with a null "proxy".  But I'm not sure where that change would have been, and I'm not sure that matters.

Bill, what should dom::InterfaceHasInstance be doing when CheckedUnwrap on instance fails?  Should it just return false, or should it be doing an UncheckedUnwrap?

It's also possible, I guess, that OwnerOf(proxy) started returning null.  That seems less likely to me.

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

I guess I missed this callsite in bug 1057601. We should be using UncheckedUnwrap here.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/577123ff73d3

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/577123ff73d3