Closed Bug 444237 Opened 16 years ago Closed 16 years ago

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

Categories

(Core :: CSS Parsing and Computation, defect)

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 (deleted) —
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) (deleted) — 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) (deleted) — 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.
Attached patch Patch 3 & crashtest (deleted) — Splinter Review
Attachment #328621 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed as 15841:31f5da857994.
Status: ASSIGNED → RESOLVED
Closed: 16 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.

Attachment

General

Created:
Updated:
Size: