823124 - crash in nsRuleNode::ChildrenHashHashKey

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P1)

Description

[Gregor Wagner [:gwagner]]

I get this on a b2g-otoro phone with debug mc gecko when I open a sms after receiving it.

Program received signal SIGSEGV, Segmentation fault.
0x40fcafac in nsRuleNode::ChildrenHashHashKey (aTable=0x4bc2a660, aKey=0x0) at /Volumes/2mac/gaia/src/layout/style/nsRuleNode.cpp:96
96	  return PL_DHashVoidPtrKeyStub(aTable, key->mRule);


Program received signal SIGSEGV, Segmentation fault.
0x40fcafac in nsRuleNode::ChildrenHashHashKey (aTable=0x4bc2a660, aKey=0x0) at /Volumes/2mac/gaia/src/layout/style/nsRuleNode.cpp:96
96	  return PL_DHashVoidPtrKeyStub(aTable, key->mRule);
(gdb) [Child 986] WARNING: preserving unexpected JS escape sequence: file /Volumes/2mac/gaia/src/modules/libpref/src/prefread.cpp, line 393
[Child 986] WARNING: preserving unexpected JS escape sequence: file /Volumes/2mac/gaia/src/modules/libpref/src/prefread.cpp, line 393
[Child 986] WARNING: preserving unexpected JS escape sequence: file /Volumes/2mac/gaia/src/modules/libpref/src/prefread.cpp, line 393
[Child 986] WARNING: preserving unexpected JS escape sequence: file /Volumes/2mac/gaia/src/modules/libpref/src/prefread.cpp, line 393
[Child 986] WARNING: preserving unexpected JS escape sequence: file /Volumes/2mac/gaia/src/modules/libpref/src/prefread.cpp, line 393
[Child 986] WARNING: preserving unexpected JS escape sequence: file /Volumes/2mac/gaia/src/modules/libpref/src/prefread.cpp, line 393
[Child 949] WARNING: NS_ENSURE_TRUE(IsChromeProcess()) failed: file /Volumes/2mac/gaia/src/content/base/src/nsFrameMessageManager.cpp, line 687
[Child 949] WARNING: NS_ENSURE_TRUE(IsChromeProcess()) failed: file /Volumes/2mac/gaia/src/content/base/src/nsFrameMessageManager.cpp, line 687
Quit
(gdb) bt
#0  0x40fcafac in nsRuleNode::ChildrenHashHashKey (aTable=0x4bc2a660, aKey=0x0) at /Volumes/2mac/gaia/src/layout/style/nsRuleNode.cpp:96
#1  0x420b7c08 in PL_DHashTableOperate (table=0x4bc2a660, key=0x0, op=PL_DHASH_ADD) at /Volumes/2mac/gaia/src/otorodebug/xpcom/build/pldhash.cpp:578
#2  0x40fcda16 in nsRuleNode::ConvertChildrenToHash (this=0x4a715708) at /Volumes/2mac/gaia/src/layout/style/nsRuleNode.cpp:1422
#3  0x40fcd71a in nsRuleNode::Transition (this=0x4a715708, aRule=0x4a7b5880, aLevel=6 '\006', aIsImportantRule=false)
    at /Volumes/2mac/gaia/src/layout/style/nsRuleNode.cpp:1349
#4  0x40f57c5e in nsRuleWalker::DoForward (this=0xbeb17508, aRule=0x4a7b5880) at /Volumes/2mac/gaia/src/layout/style/nsRuleWalker.h:28
#5  0x40f57d10 in nsRuleWalker::Forward (this=0xbeb17508, aRule=0x4a7b5880) at /Volumes/2mac/gaia/src/layout/style/nsRuleWalker.h:36
#6  0x40f58ffe in nsAnimationManager::RulesMatching (this=0x48c250b0, aData=0xbeb174f8) at /Volumes/2mac/gaia/src/layout/style/nsAnimationManager.cpp:432
#7  0x40ffc612 in EnumRulesMatching<ElementRuleProcessorData> (aProcessor=0x48c250b0, aData=0xbeb174f8) at /Volumes/2mac/gaia/src/layout/style/nsStyleSet.cpp:471
#8  0x40ffa15a in nsStyleSet::FileRules (this=0x46cf6a20, aCollectorFunc=0x40ffc5f5 <EnumRulesMatching<ElementRuleProcessorData>>, aData=0xbeb174f8, 
    aContent=0x494ede20, aRuleWalker=0xbeb17508) at /Volumes/2mac/gaia/src/layout/style/nsStyleSet.cpp:814
#9  0x40ffa5c8 in nsStyleSet::ResolveStyleFor (this=0x46cf6a20, aElement=0x494ede20, aParentContext=0x495599e8, aTreeMatchContext=...)
    at /Volumes/2mac/gaia/src/layout/style/nsStyleSet.cpp:934
#10 0x40e154de in nsFrameManager::ReResolveStyleContext (this=0x4571dc00, aPresContext=0x4626d800, aFrame=0x458aab48, aParentContent=0x0, aChangeList=0xbeb17930, 
    aMinChange=0, aParentFrameHintsNotHandledForDescendants=0, aRestyleHint=eRestyle_Self, aRestyleTracker=..., 
    aDesiredA11yNotifications=nsFrameManager::eSendAllNotifications, aVisibleKidsOfHiddenElement=..., aTreeMatchContext=...)
    at /Volumes/2mac/gaia/src/layout/base/nsFrameManager.cpp:1274
#11 0x40e1668c in nsFrameManager::ComputeStyleChangeFor (this=0x4571dc00, aFrame=0x458aab48, aChangeList=0xbeb17930, aMinChange=0, aRestyleTracker=..., 
    aRestyleDescendants=false) at /Volumes/2mac/gaia/src/layout/base/nsFrameManager.cpp:1697
#12 0x40dc4506 in nsCSSFrameConstructor::RestyleElement (this=0x4571dc00, aElement=0x494ede20, aPrimaryFrame=0x458aab48, aMinHint=0, aRestyleTracker=..., 
    aRestyleDescendants=false, aTracker=...) at /Volumes/2mac/gaia/src/layout/base/nsCSSFrameConstructor.cpp:8313
#13 0x40dade14 in mozilla::css::RestyleTracker::ProcessOneRestyle (this=0x4571dd48, aElement=0x494ede20, aRestyleHint=eRestyle_Self, aChangeHint=0, aTracker=...)
    at /Volumes/2mac/gaia/src/layout/base/RestyleTracker.cpp:126
#14 0x40dae33e in mozilla::css::RestyleTracker::DoProcessRestyles (this=0x4571dd48) at /Volumes/2mac/gaia/src/layout/base/RestyleTracker.cpp:213
#15 0x40db2f4e in mozilla::css::RestyleTracker::ProcessRestyles (this=0x4571dd48) at /Volumes/2mac/gaia/src/layout/base/RestyleTracker.h:192
#16 0x40dcb75c in nsCSSFrameConstructor::ProcessPendingRestyles (this=0x4571dc00) at /Volumes/2mac/gaia/src/layout/base/nsCSSFrameConstructor.cpp:12120
#17 0x40e414d2 in PresShell::FlushPendingNotifications (this=0x456d4340, aFlush=...) at /Volumes/2mac/gaia/src/layout/base/nsPresShell.cpp:3868
#18 0x40e54624 in nsRefreshDriver::Tick (this=0x449af680, aNowEpoch=1355886308636418, aNowTime=...) at /Volumes/2mac/gaia/src/layout/base/nsRefreshDriver.cpp:883
#19 0x40e52b2a in mozilla::RefreshDriverTimer::TickDriver (driver=0x449af680, jsnow=1355886308636418, now=...)
    at /Volumes/2mac/gaia/src/layout/base/nsRefreshDriver.cpp:164
#20 0x40e52a8c in mozilla::RefreshDriverTimer::Tick (this=0x471f6f00) at /Volumes/2mac/gaia/src/layout/base/nsRefreshDriver.cpp:156
#21 0x40e52b50 in mozilla::RefreshDriverTimer::TimerTick (aTimer=0x484f0790, aClosure=0x471f6f00) at /Volumes/2mac/gaia/src/layout/base/nsRefreshDriver.cpp:181
#22 0x4211b8cc in nsTimerImpl::Fire (this=0x484f0790) at /Volumes/2mac/gaia/src/xpcom/threads/nsTimerImpl.cpp:482
#23 0x4211bc92 in nsTimerEvent::Run (this=0x40444880) at /Volumes/2mac/gaia/src/xpcom/threads/nsTimerImpl.cpp:565
#24 0x421159d8 in nsThread::ProcessNextEvent (this=0x40404390, mayWait=false, result=0xbeb18717) at /Volumes/2mac/gaia/src/xpcom/threads/nsThread.cpp:627
#25 0x420b5ca6 in NS_ProcessNextEvent_P (thread=0x40404390, mayWait=false) at /Volumes/2mac/gaia/src/otorodebug/xpcom/build/nsThreadUtils.cpp:237
#26 0x41e1ba0e in mozilla::ipc::MessagePump::Run (this=0x40402430, aDelegate=0x4042b0c0) at /Volumes/2mac/gaia/src/ipc/glue/MessagePump.cpp:82
#27 0x4216ea0c in MessageLoop::RunInternal (this=0x4042b0c0) at /Volumes/2mac/gaia/src/ipc/chromium/src/base/message_loop.cc:215
#28 0x4216e9a6 in MessageLoop::RunHandler (this=0x4042b0c0) at /Volumes/2mac/gaia/src/ipc/chromium/src/base/message_loop.cc:208
#29 0x4216e94e in MessageLoop::Run (this=0x4042b0c0) at /Volumes/2mac/gaia/src/ipc/chromium/src/base/message_loop.cc:182
#30 0x41ce860a in nsBaseAppShell::Run (this=0x44847640) at /Volumes/2mac/gaia/src/widget/xpwidgets/nsBaseAppShell.cpp:163
#31 0x41b27628 in nsAppStartup::Run (this=0x4484a3d0) at /Volumes/2mac/gaia/src/toolkit/components/startup/nsAppStartup.cpp:289
#32 0x40b2c968 in XREMain::XRE_mainRun (this=0xbeb18990) at /Volumes/2mac/gaia/src/toolkit/xre/nsAppRunner.cpp:3824
#33 0x40b2cb94 in XREMain::XRE_main (this=0xbeb18990, argc=1, argv=0xbeb1aba4, aAppData=0x37424) at /Volumes/2mac/gaia/src/toolkit/xre/nsAppRunner.cpp:3891

Comment 1

[Andrew Overholt [:overholt]]

mvines, have you seen this with testing of debug builds?

Comment 2

[Michael Vines [:m1] [:evilmachines]]

I don't think I've seen this particular stack trace. But a lot of the stack traces of seen the been pretty poor (via minidump decoding).

+1 to make this blocking.  We have massive stability issues right now.

Comment 3

[Robert Kaiser]

Example crash reports with this signature and good stacks and bp-874fb4b9-0625-41b3-861d-a62da2121219 and bp-115a9fa0-8d17-4f31-9554-350c22121219.

Comment 4

[Doug Turner (:dougt)]

I see about 50 crashes in the last week on crashstats.  We need to figure out what is going on.

Comment 5

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

This looks like a null-deref, probably resulting from somebody putting null rules into the rule tree.  Maybe will be fixed by bug 822766, though I'm guessing (somewhat)?

Comment 6

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

Er, no, that wasn't the bug I was thinking of.

Comment 7

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

Very likely a regression from bug 780692, though.

Comment 8

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

Attachment: Add diagnostic (and otherwise good-to-have) assertion that non-root rule nodes must have a non-null rule (and vice-versa).

Comment 9

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

If someone can reproduce the problem with the above patch applied, it should give a more useful stack (unless the problem is memory corruption that happens later).

Comment 11

[Doug Turner (:dougt)]

i am not able to reproduce this by just recv'ing a sms

Comment 12

[Doug Turner (:dougt)]

Nick, any clues about this?

Comment 13

[Nick Cameron [:nrc]]

Attachment: speculative patch

Comment 14

[Nick Cameron [:nrc]]

(In reply to Doug Turner (:dougt) from comment #12)
> Nick, any clues about this?

Here (patch) is a guess. Due to a broken computer, I don't have a b2g debugging setup at the moment. I couldn't recreate on my phone, nor in desktop Firefox with OMTA. If this doesn't fix things, I'll try to look into it when I get my laptop back.

Comment 15

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

Comment on attachment 695832 [details] [diff] [review]
speculative patch

I'd rather have the null-check in UpdateThrottledStyle than in nsStyleSet since I suspect UpdateThrottledStyle will be the only case that needs it.  (In fact, UpdateThrottledStyle only needs it in a rare case.)

(I think UpdateThrottledStyle would also be easier to fix with the AppendElement at the end (and with the version of AppendElement that takes an argument), which would then allow use of |continue;|.)

Comment 16

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

(And note that UpdateThrottledStyle needs it only in the animation-sheet and transition-sheet cases.)

Comment 17

[Nick Cameron [:nrc]]

Attachment: speculative patch

Comment 18

[Nick Cameron [:nrc]]

(In reply to David Baron [:dbaron] from comment #16)
> (And note that UpdateThrottledStyle needs it only in the animation-sheet and
> transition-sheet cases.)

I thought we only need it in the animation-sheet case. EnsureStyleRule should make a non-rule for transitions. But, in any case it seemed more straightforward to check for all cases rather than duplicate the append code.

Comment 19

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

Comment on attachment 695837 [details] [diff] [review]
speculative patch

r=dbaron (though I might have actually pushed the null-check inside the branches)

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 694935 [details] [diff] [review]
Add diagnostic (and otherwise good-to-have) assertion that non-root rule nodes must have a non-null rule (and vice-versa).

r=me

Comment 21

[Nick Cameron [:nrc]]

I'll land both patches in a sec...

Comment 22

[Nick Cameron [:nrc]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a836a0d544da
https://hg.mozilla.org/integration/mozilla-inbound/rev/50b3e25c750d

Comment 23

[Nick Cameron [:nrc]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8b4e58dca970
https://hg.mozilla.org/releases/mozilla-aurora/rev/2b2ed492270b

Comment 24

[Nick Cameron [:nrc]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/418b6f9524fe
https://hg.mozilla.org/releases/mozilla-b2g18/rev/2008afe879e6

Comment 25

[Nick Cameron [:nrc]]

Lets see if the crash disappears when this patch gets pushed to phones...

Comment 26

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(Recent convention has been to mark fixed when primarily b2g bugs are landed on release branch.)

Comment 27

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/mozilla-central/rev/a836a0d544da
https://hg.mozilla.org/mozilla-central/rev/50b3e25c750d

Comment 28

[Ryan VanderMeulen [:RyanVM]]

The convention is that bugs are marked fixed when they land on m-c. Status flags get set when the land on the release branches.

Comment 29

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I'm aware of the usual convention.  The recent breach in protocol comes from a desire to make it easier for downstream partners to see "fixed" status, because they don't see patches until they hit gecko18.