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

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Martijn Wargers (dead)]

Attachment: 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.

Comment 1

[Michael Ventnor]

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?

Comment 2

[Michael Ventnor]

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.

Comment 3

[Michael Ventnor]

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.

Comment 4

[Michael Ventnor]

Attachment: Patch

Comment 5

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

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?

Comment 6

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

I'm curious why none of our automated tests caught this; in any case, please add a crashtest as well.

Comment 7

[Michael Ventnor]

GetBackgroundData and GetPaddingdata doesn't do any kind of nulling out. Did you mean GetBorderData and GetTextData?

Comment 8

[Michael Ventnor]

Attachment: Patch 2

Never mind, I think I understand what you mean now. Crashtest coming soon.

Comment 9

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

Yes, except you forgot the comment in GetBorderData, and you should wrap the comments at less than 80 characters.

Comment 10

[Michael Ventnor]

Attachment: Patch 3 & crashtest

Comment 11

[Reed Loden [:reed]]

Pushed as 15841:31f5da857994.

Comment 12

[Martijn Wargers (dead)]

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