Last Comment Bug 372237 - Crash [@ GetChildListNameFor] with -moz-box, float, position: fixed, block-in-inline
: Crash [@ GetChildListNameFor] with -moz-box, float, position: fixed, block-in...
Status: RESOLVED FIXED
[sg:critical?]
: crash, fixed1.8.0.12, fixed1.8.1.4, testcase
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 Mac OS X
: -- critical (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (email my personal email if necessary)
:
Mentors:
Depends on:
Blocks: randomclasses 374356 CVE-2008-2811
  Show dependency treegraph
 
Reported: 2007-03-01 01:33 PST by Jesse Ruderman
Modified: 2011-06-13 10:01 PDT (History)
5 users (show)
dveditz: blocking1.8.1.4+
dveditz: blocking1.8.0.12+
jruderman: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (crashes Firefox debug build when loaded) (594 bytes, text/html)
2007-03-01 01:33 PST, Jesse Ruderman
no flags Details
fix problem #3 (1.28 KB, patch)
2007-03-08 20:11 PST, Robert O'Callahan (:roc) (email my personal email if necessary)
dbaron: review+
dbaron: superreview+
dveditz: approval1.8.1.4+
dveditz: approval1.8.0.12+
Details | Diff | Splinter Review
fix problem #2 (NS_FRAME_SET_TRUNCATION) (4.36 KB, patch)
2007-03-08 20:12 PST, Robert O'Callahan (:roc) (email my personal email if necessary)
dbaron: review+
dbaron: superreview+
Details | Diff | Splinter Review
branch patch (1.33 KB, patch)
2007-03-28 14:00 PDT, Robert O'Callahan (:roc) (email my personal email if necessary)
no flags Details | Diff | Splinter Review

Description Jesse Ruderman 2007-03-01 01:33:15 PST
Created attachment 256897 [details]
testcase (crashes Firefox debug build when loaded)

###!!! ASSERTION: out-of-flow is already in the destroy queue: 'aDestroyQueue.IndexOf(outOfFlowFrame) == kNotFound', file /Users/jruderman/trunk/mozilla/layout/base/nsCSSFrameConstructor.cpp, line 9233

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0xddddddfd

Thread 0 Crashed:
0   libgklayout.dylib        	0x18a84b94 nsCachedStyleData::GetStyleDisplay() + 12 (nsStyleStructList.h:95)
1   libgklayout.dylib        	0x1851dfdc nsStyleContext::GetStyleDisplay() + 20 (nsStyleStructList.h:95)
2   libgklayout.dylib        	0x18a3765b nsIFrame::GetStyleDisplay() const + 83 (nsStyleStructList.h:95)
3   libgklayout.dylib        	0x1838c60e GetChildListNameFor(nsIFrame*) + 52 (nsCSSFrameConstructor.cpp:1716)
4   libgklayout.dylib        	0x1838e116 DeletingFrameSubtree(nsFrameManager*, nsIFrame*) + 296 (nsCSSFrameConstructor.cpp:9303)
5   libgklayout.dylib        	0x183a68ba nsCSSFrameConstructor::ContentRemoved(nsIContent*, nsIContent*, int, int) + 1008 (nsCSSFrameConstructor.cpp:9453)
6   libgklayout.dylib        	0x183a6208 nsCSSFrameConstructor::ReinsertContent(nsIContent*, nsIContent*) + 74 (nsCSSFrameConstructor.cpp:9161)
7   libgklayout.dylib        	0x183a63c6 nsCSSFrameConstructor::ReframeContainingBlock(nsIFrame*) + 374 (nsCSSFrameConstructor.cpp:12698)
8   libgklayout.dylib        	0x183a64b3 nsCSSFrameConstructor::MaybeRecreateContainerForIBSplitterFrame(nsIFrame*, unsigned*) + 141 (nsCSSFrameConstructor.cpp:10995)
9   libgklayout.dylib        	0x183a43a5 nsCSSFrameConstructor::RecreateFramesForContent(nsIContent*) + 173 (nsCSSFrameConstructor.cpp:11026)
10  libgklayout.dylib        	0x183a4a3e nsCSSFrameConstructor::RestyleElement(nsIContent*, nsIFrame*, nsChangeHint) + 192 (nsCSSFrameConstructor.cpp:9924)
11  libgklayout.dylib        	0x183a4c4d nsCSSFrameConstructor::ProcessOneRestyle(nsIContent*, nsReStyleHint, nsChangeHint) + 215 (nsCSSFrameConstructor.cpp:12787)
12  libgklayout.dylib        	0x183a4e88 nsCSSFrameConstructor::ProcessPendingRestyles() + 420 (nsCSSFrameConstructor.cpp:12834)
13  libgklayout.dylib        	0x183a4fd2 nsCSSFrameConstructor::RestyleEvent::Run() + 200 (nsCSSFrameConstructor.cpp:12904)
14  libxpcom_core.dylib      	0x013499ea nsThread::ProcessNextEvent(int, int*) + 556 (nsThread.cpp:483)
Comment 1 Jesse Ruderman 2007-03-01 01:41:06 PST
Similar assertion and stack in bug 364427 (fixed).
Comment 2 Window Snyder 2007-03-08 13:31:47 PST
Jesse suggested in irc that we ask BZ to look at this next week.
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-08 16:10:33 PST
No, I'll take it
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2007-03-08 18:36:16 PST
Mats, you interested in this?  You've been in this code a lot recently...
Comment 5 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-08 18:58:38 PST
I'm already mostly done here.
Comment 6 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-08 20:10:42 PST
There are three bugs here. Here's what happens:

1) The -moz-box DIV with overflow:hidden containing an IMG goes haywire and the image (which is box-wrapped, of course) gets a crazy ascent (NS_INTRINSICSIZE to be precise). This appears to be ultimately sourced from the NS_INTRINSICSIZE passed in as the height here:
http://lxr.mozilla.org/seamonkey/source/layout/generic/nsFrame.cpp#5829
I'm not really sure what's going on. I'll file a followup bug with a nice reduced testcase. Anyway, this leads to some frames getting an insane height.

2) NS_FRAME_SET_TRUNCATION then decides that frames have exceeded the available height (which was meant to be unconstrained) and pushes them to overflow lists. I have a patch that fixes this by never setting truncation if the available height is NS_INTRINSICSIZE, even if absurd heights are running around.

This is bad but still shouldn't be fatal, we're supposed to not crash even when there are frames hanging around in overflow lists.

3) Later we crash deleting some frames. This problem is quite simple, nsCSSFrameConstructor::DoDeletingFrameSubtree wants to skip iterating through child lists which contain out-of-flow-frames, preferring to delete frames through placeholders instead. The problem is just that it needs skip the overflowOutOfFlow list and doesn't. The simple patch for that fixes the crash.
Comment 7 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-08 20:11:43 PST
Created attachment 257910 [details] [diff] [review]
fix problem #3

As described in the comment above
Comment 8 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-08 20:12:52 PST
Created attachment 257911 [details] [diff] [review]
fix problem #2 (NS_FRAME_SET_TRUNCATION)

as described in the comment above
Comment 9 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2007-03-11 09:49:34 PDT
Comment on attachment 257910 [details] [diff] [review]
fix problem #3

r+sr=dbaron
Comment 10 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2007-03-11 09:49:56 PDT
Comment on attachment 257911 [details] [diff] [review]
fix problem #2 (NS_FRAME_SET_TRUNCATION)

r+sr=dbaron.  Please get that other bug filed, though, if you have a testcase. :-)
Comment 11 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-11 13:40:53 PDT
Both patches checked in. Filed bug 373566 on problem #1.
Comment 12 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-11 13:42:16 PDT
I'm not really sure how to write a reftest for this. Maybe we should just have a collection of "pages that should not crash the browser"?
Comment 13 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-11 13:45:08 PDT
Comment on attachment 257910 [details] [diff] [review]
fix problem #3

I think we should take this on branches. It probably fixes some dangerous crashes, even if this particular crash is not present on branch. It's very small, obviously correct, and I think very low risk.
Comment 14 Jesse Ruderman 2007-03-11 16:46:50 PDT
"Pages that should not crash the browser" become mochitests, right?  But see bug 368573 for an issue with assertions that might result in a split.

I'm guessing this page shouldn't go into a public test suite until the bug is fixed on branches, though.
Comment 15 Daniel Veditz [:dveditz] 2007-03-21 15:43:20 PDT
Comment on attachment 257911 [details] [diff] [review]
fix problem #2 (NS_FRAME_SET_TRUNCATION)

Do we not want this part on the branches?
Comment 16 Daniel Veditz [:dveditz] 2007-03-21 15:43:39 PDT
Comment on attachment 257910 [details] [diff] [review]
fix problem #3

approved for 1.8.0.12 and 1.8.1.4, a=dveditz for release-drivers
Comment 17 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-21 15:56:59 PDT
(In reply to comment #15)
> (From update of attachment 257911 [details] [diff] [review])
> Do we not want this part on the branches?

It might help, but I didn't nominate it because I don't know of any crashes that it fixes other than this one, and we have a better fix for this crash.
Comment 18 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-28 14:00:25 PDT
Created attachment 259945 [details] [diff] [review]
branch patch
Comment 19 Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-28 14:04:34 PDT
Fixed on branches.
Comment 20 Jesse Ruderman 2007-12-15 16:51:35 PST
I checked in the testcase as a crashtest, but since there were multiple patches, it might be good to have more tests.

Note You need to log in before you can comment on or make changes to this bug.