Closed Bug 823124 Opened 7 years ago Closed 7 years ago

crash in nsRuleNode::ChildrenHashHashKey

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed

People

(Reporter: gwagner, Assigned: dougt)

References

Details

(Keywords: crash, Whiteboard: [b2g-crash])

Crash Data

Attachments

(2 files, 1 obsolete file)

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
Severity: normal → critical
Crash Signature: [@ nsRuleNode::ChildrenHashHashKey] [@ nsRuleNode::ChildrenHashHashKey(PLDHashTable*, void const*)]
Component: Graphics → Style System (CSS)
Keywords: crash
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
blocking-basecamp: --- → ?
mvines, have you seen this with testing of debug builds?
Flags: needinfo?(mvines)
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.
Flags: needinfo?(mvines)
Example crash reports with this signature and good stacks and bp-874fb4b9-0625-41b3-861d-a62da2121219 and bp-115a9fa0-8d17-4f31-9554-350c22121219.
Whiteboard: [b2g-crash]
I see about 50 crashes in the last week on crashstats.  We need to figure out what is going on.
Assignee: nobody → doug.turner
blocking-basecamp: ? → +
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)?
Depends on: 822766
Er, no, that wasn't the bug I was thinking of.
No longer depends on: 822766
Very likely a regression from bug 780692, though.
Blocks: 780692
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).
Keywords: qawanted
i am not able to reproduce this by just recv'ing a sms
Priority: -- → P1
Target Milestone: --- → B2G C3 (12dec-1jan)
Nick, any clues about this?
Flags: needinfo?(ncameron)
Attached patch speculative patch (obsolete) — Splinter Review
Attachment #695832 - Flags: review?(dbaron)
Flags: needinfo?(ncameron)
(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 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;|.)
Attachment #695832 - Flags: review?(dbaron) → review-
(And note that UpdateThrottledStyle needs it only in the animation-sheet and transition-sheet cases.)
Attachment #695832 - Attachment is obsolete: true
Attachment #695837 - Flags: review?(dbaron)
(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 on attachment 695837 [details] [diff] [review]
speculative patch

r=dbaron (though I might have actually pushed the null-check inside the branches)
Attachment #695837 - Flags: review?(dbaron) → review+
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
Attachment #694935 - Flags: review?(bzbarsky) → review+
I'll land both patches in a sec...
Whiteboard: [b2g-crash] → [b2g-crash][leave-open]
Lets see if the crash disappears when this patch gets pushed to phones...
(Recent convention has been to mark fixed when primarily b2g bugs are landed on release branch.)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
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.
Whiteboard: [b2g-crash][leave-open] → [b2g-crash]
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.
You need to log in before you can comment on or make changes to this bug.