1401865 - Crash in mozilla::dom::XULElementBinding::Wrap

Status: RESOLVED WORKSFORME

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

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

This bug was filed from the Socorro interface and is 
report bp-93400d48-049f-455d-a2da-aaf330170921.
=============================================================

This signature is old but the volume had increased recently since late August.

bz, any idea?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Looks like we're crashing on this line:

  if (!canonicalProto) {

with a near-null deref (0x48).

I recall us having some bug along these lines (near-null crash in binding code on a pointer that should not have been null) that we hadn't really made any progress on, but can't find it right now...  :(

Comment 2

[Kan-Ru Chen [:kanru] (UTC+9)]

That reminds me bug 1335122

Comment 3

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #1)
> Looks like we're crashing on this line:
> 
>   if (!canonicalProto) {
> 
> with a near-null deref (0x48).
> 
> I recall us having some bug along these lines (near-null crash in binding
> code on a pointer that should not have been null) that we hadn't really made
> any progress on, but can't find it right now...  :(

Since we have some visibility to the heap now, I can inspect the content of aObject.

aObject and aCache points to the same thing and their mWrapper is 0x0.

Which means we should fail at https://crash-stats.mozilla.com/sources/highlight/?url=https://gecko-generated-sources.s3.amazonaws.com/4a6c89070f489cd05d302e3637c54ebeea12ea68b2ce72aeec24208e90c0000d3d89dc31390e80052a2e26e0a439097785f4004a68d33422ae1bf88b8009515f/dom/bindings/XULElementBinding.cpp#L-9087 if it's a debug build.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> That reminds me bug 1335122

Ah, yes, that is the one I was looking for!

> Which means we should fail at

Why?  Having a null mWrapper is the right thing here: that means we haven't created the wrapper yet.  And having a null mWrapper is what makes the assert you link to _pass_.

Comment 5

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #4)
> > That reminds me bug 1335122
> 
> Ah, yes, that is the one I was looking for!
> 
> > Which means we should fail at
> 
> Why?  Having a null mWrapper is the right thing here: that means we haven't
> created the wrapper yet.  And having a null mWrapper is what makes the
> assert you link to _pass_.

Yes, silly me misread the condition.

Comment 6

[Hsin-Yi Tsai (she/her) [:hsinyi]]

The crash volume isn't high, but I think it'd be great at least to know first what caused the a bit spike recently.
Kan-Ru, would you be able to take a look once you're back?

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Closing because no crashes reported for 12 weeks.