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)
:
: Jet Villegas (:jet)
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 User image 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 User image Jesse Ruderman 2007-03-01 01:41:06 PST
Similar assertion and stack in bug 364427 (fixed).
Comment 2 User image 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 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-08 16:10:33 PST
No, I'll take it
Comment 4 User image 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 User image 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 User image 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 User image 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 User image 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 User image David Baron :dbaron: ⌚️UTC-8 2007-03-11 09:49:34 PDT
Comment on attachment 257910 [details] [diff] [review]
fix problem #3

r+sr=dbaron
Comment 10 User image David Baron :dbaron: ⌚️UTC-8 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-28 14:04:34 PDT
Fixed on branches.
Comment 20 User image 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.