Closed Bug 444237 Opened 11 years ago Closed 11 years ago

Crash [@ nsCSSValueList::`scalar deleting destructor'] with -moz-box-shadow: -moz-initial on input

Categories

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

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.9.1a1

People

(Reporter: martijn.martijn, Assigned: ventnor.bugzilla)

References

Details

(4 keywords)

Crash Data

Attachments

(2 files, 2 obsolete files)

Attached file testcase
See testcase, which crashes current trunk build on load.

Soe breakpad ids:
http://crash-stats.mozilla.com/report/pending/0541552e-4d4b-11dd-8725-001a4bd43ef6
http://crash-stats.mozilla.com/report/pending/ddf8c163-4d4a-11dd-b137-0013211cbf8a
http://crash-stats.mozilla.com/report/pending/cfa39ab3-4d4a-11dd-a542-001cc45a2c28

Unfortunately, breakpad server is really slow, so I don't know currently what the stacktrace is. I'm just assuming this is a thebes issue.
I reproduced the crash and it is happening in nsRuleNode::HasAuthorSpecifiedRules. But I have absolutely no idea what is happening.

It segfaults when it tries to destroy the box shadow array in ~nsCSSMargin, but it shouldn't because it should be null. So the box shadow array pointer must be uninitialized memory or corrupted memory, but I can't explain what is happening because I tried to break in HasAuthorSpecifiedRules if mBoxShadow was not-null and it was never reached before the crash.

It doesn't help that I've never seen the code before... dbaron, would you know what is going on?
OK I've made some progress...

It seems rule->MapRuleInfoInto is being naughty because when I null-out mBoxShadow afterwards the crash goes away. I'll look into it and see if I can fix it the right way.
I can't see why MapRuleInfoInto is corrupting the mBoxShadow pointer, most implementations of MapRuleInfoInto are just to translate attributes into CSS properties and the one in nsCSSCompressedDataBlock doesn't seem to do anything suspicious. I'll post the wallpaper patch in the hopes of fixing this crash sooner rather than later.
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → ventnor.bugzilla
Status: NEW → ASSIGNED
Attachment #328617 - Flags: superreview?(dbaron)
Attachment #328617 - Flags: review?(dbaron)
Comment on attachment 328617 [details] [diff] [review]
Patch

r+sr=dbaron, except:

 * you should copy the comment from GetBorderData's doing the same thing

 * before that comment, you should say:  Do the same nulling out that is done in GetBackgroundData, GetBorderData, and GetPaddingData

 * in GetBackgroundData, GetBorderData, and GetPaddingData, you should add a comment saying that each member that needs to be nulled out there also needs to be nulled out in HasAuthorSpecifiedRules.



In the long run, we should really fix the nsCSS* structs so they don't own their members; there are no longer any users of these structs that use them for ownership, and everybody else has to work around their ownership pattern.  Could you file a followup bug on that?
Attachment #328617 - Flags: superreview?(dbaron)
Attachment #328617 - Flags: superreview+
Attachment #328617 - Flags: review?(dbaron)
Attachment #328617 - Flags: review+
Component: GFX: Thebes → Style System (CSS)
Flags: blocking1.9.1?
OS: Windows XP → All
QA Contact: thebes → style-system
Hardware: PC → All
I'm curious why none of our automated tests caught this; in any case, please add a crashtest as well.
GetBackgroundData and GetPaddingdata doesn't do any kind of nulling out. Did you mean GetBorderData and GetTextData?
Attached patch Patch 2 (obsolete) — Splinter Review
Never mind, I think I understand what you mean now. Crashtest coming soon.
Attachment #328617 - Attachment is obsolete: true
Yes, except you forgot the comment in GetBorderData, and you should wrap the comments at less than 80 characters.
Attachment #328621 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed as 15841:31f5da857994.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a1
From http://crash-stats.mozilla.com/report/index/0541552e-4d4b-11dd-8725-001a4bd43ef6
0  	xul.dll  	nsCSSValueList::`scalar deleting destructor'  	
1 	xul.dll 	nsCSSValueList::`scalar deleting destructor' 	
2 	xul.dll 	nsCSSValueList::`scalar deleting destructor' 	
3 	xul.dll 	xul.dll@0x26e182 	
4 	xul.dll 	nsPresContext::HasAuthorSpecifiedRules 	layout/base/nsPresContext.cpp:1505
5 	xul.dll 	xul.dll@0x25ee00 	
6 	xul.dll 	nsHTMLReflowState::InitConstraints 	layout/generic/nsHTMLReflowState.cpp:1823
7 	xul.dll 	nsHTMLReflowState::Init 	layout/generic/nsHTMLReflowState.cpp:307
8 	xul.dll 	nsHTMLReflowState::nsHTMLReflowState 	layout/generic/nsHTMLReflowState.cpp:176
9 	xul.dll 	nsLineLayout::ReflowFrame 	layout/generic/nsLineLayout.cpp:772
10 	xul.dll 	nsBlockFrame::ReflowInlineFrame 	layout/generic/nsBlockFrame.cpp:3583
11 	xul.dll 	nsBlockFrame::DoReflowInlineFrames 	layout/generic/nsBlockFrame.cpp:3405
12 	xul.dll 	nsBlockFrame::ReflowInlineFrames 	layout/generic/nsBlockFrame.cpp:3254
13 	xul.dll 	nsBlockFrame::ReflowLine 	layout/generic/nsBlockFrame.cpp:2321
14 	xul.dll 	nsBlockFrame::ReflowDirtyLines 	layout/generic/nsBlockFrame.cpp:1902
15 	xul.dll 	nsBlockFrame::Reflow 	layout/generic/nsBlockFrame.cpp:959
16 	xul.dll 	nsBlockReflowContext::ReflowBlock 	layout/generic/nsBlockReflowContext.cpp:311
17 	xul.dll 	nsBlockFrame::ReflowBlockFrame 	layout/generic/nsBlockFrame.cpp:2994
18 	xul.dll 	nsBlockFrame::ReflowLine 	layout/generic/nsBlockFrame.cpp:2266
19 	xul.dll 	nsBlockFrame::ReflowDirtyLines 	layout/generic/nsBlockFrame.cpp:1902
20 	xul.dll 	nsBlockFrame::Reflow 	layout/generic/nsBlockFrame.cpp:959
21 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:771
22 	xul.dll 	CanvasFrame::Reflow 	layout/generic/nsHTMLFrame.cpp:584
23 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:771
24 	xul.dll 	nsHTMLScrollFrame::ReflowScrolledFrame 	layout/generic/nsGfxScrollFrame.cpp:499
25 	xul.dll 	nsHTMLScrollFrame::ReflowContents 	layout/generic/nsGfxScrollFrame.cpp:593
26 	xul.dll 	nsHTMLScrollFrame::Reflow 	layout/generic/nsGfxScrollFrame.cpp:794
27 	xul.dll 	nsContainerFrame::ReflowChild 	layout/generic/nsContainerFrame.cpp:771
28 	xul.dll 	ViewportFrame::Reflow 	layout/generic/nsViewportFrame.cpp:286
29 	xul.dll 	PresShell::DoReflow 	layout/base/nsPresShell.cpp:6287
30 	xul.dll 	PresShell::ProcessReflowCommands 	layout/base/nsPresShell.cpp:6393
31 	xul.dll 	PresShell::DoFlushPendingNotifications 	layout/base/nsPresShell.cpp:4581
32 	xul.dll 	PresShell::ReflowEvent::Run 	layout/base/nsPresShell.cpp:6152
33 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
34 	nspr4.dll 	PR_GetEnv 	

Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a1pre) Gecko/2008071203 Minefield/3.1a1pre
Status: RESOLVED → VERIFIED
Summary: Crash with -moz-box-shadow: -moz-initial on input → Crash [@ nsCSSValueList::`scalar deleting destructor'] with -moz-box-shadow: -moz-initial on input
Flags: blocking1.9.1? → blocking1.9.1+
Keywords: fixed1.9.1
Keywords: verified1.9.1
Keywords: fixed1.9.1
Crash Signature: [@ nsCSSValueList::`scalar deleting destructor']
You need to log in before you can comment on or make changes to this bug.