682598 - Crash, null pointer deref [@ GraphWalker<scanVisitor>::DoWalk ]

Status: RESOLVED DUPLICATE of bug 500105

(Core :: XPCOM, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

I was just looking at the list of top crashers for Firefox 9.0a1 / Linux. This one is ranking 30th with... 2 crashes last week.

https://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A9.0a1&platform=linux&query_search=signature&query_type=contains&reason_type=contains&date=&range_value=1&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=GraphWalker%3CscanVisitor%3E%3A%3ADoWalk

Here's one of the crashes:

https://crash-stats.mozilla.com/report/index/beb2c20e-600a-4478-a7e3-4017f2110825

It's crashing here with a null deref, suggesting that |pi| is null:

http://hg.mozilla.org/mozilla-central/annotate/7eb1a56eaaf1/xpcom/base/nsCycleCollector.cpp#l1886

The DoWalk function has been inlined, but it seems that the |pi| pointer is null here:

http://hg.mozilla.org/mozilla-central/annotate/7eb1a56eaaf1/xpcom/base/nsCycleCollector.cpp#l1336

Comment 1

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: avoid null deref

Is this the right fix? I don't want to hide a bug, if the real bug is that a null pointer was there in the first place.

Comment 2

[Robert Kaiser]

https://crash-stats.mozilla.com/report/list?range_value=4&range_unit=weeks&signature=GraphWalker%3CscanVisitor%3E::DoWalk says this a cross-Firefox-version (since 4) crash on both Linux and Mac.

Comment 3

[Andrew McCreight [:mccr8]]

Yeah, this looks like the same thing as bug 500105, which happens on all platforms.  About half of these crashes show up as null dereferences, but I think pi shouldn't be null here.  I'm not sure if there is just sometimes an extra null being stored, so we could skip over it and all would be fine, or of this means we've totally gone off the rails, and null-checking will just make us crash later.

Comment 4

[Andrew McCreight [:mccr8]]

Comment on attachment 556291 [details] [diff] [review]
avoid null deref

So, Peter agrees with me, that a null showing up here is a violation of some basic CC invariant, so things are probably haywire anyways.  He is suspicious of the nsDeque, so maybe we'll try out the JS queue for a few weeks to see if that helps this.