392285 - Crash [@gklayout!nsIFrame::GetStateBits(void)]

Status: VERIFIED FIXED

(Core :: General, defect)

Description

[Paul Nickerson]

This bug has been automatically processed, reduced, and uploaded by Paul's Automated Pen-Tester alpha. It still may require more work.

Firefox version:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.6) Gecko/20070812 BonEcho/2.0.0.6

Details:
eax=dddddddd ebx=7ffde000 ecx=dddddddd edx=dddddddd esi=00a078d8 edi=00011970
eip=0197f53a esp=0012df74 ebp=0012df78 iopl=0         nv up ei ng nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000286
*** WARNING: Unable to verify checksum for C:\mozilla\mozilla\firefox-debug\dist\bin\components\gklayout.dll
gklayout!nsIFrame::GetStateBits+0xa:
0197f53a 8b4024          mov     eax,dword ptr [eax+24h] ds:0023:ddddde01=????????


Stack trace:
gklayout!nsIFrame::GetStateBits(void)
gklayout!IsFrameSpecial(

                        class nsIFrame * aFrame = 0xdddddddd)
gklayout!GetIBContainingBlockFor(

                        class nsIFrame * aFrame = 0x03c9fa60)
gklayout!nsCSSFrameConstructor::ReframeContainingBlock(

                        class nsIFrame * aFrame = 0x03c9fa60)
gklayout!nsCSSFrameConstructor::ContentRemoved(

                        class nsIContent * aContainer = 0x03c56a20, 

                        class nsIContent * aChild = 0x03c57148, 

                        int aIndexInContainer = 0, 

                        int aInReinsertContent = 0)
gklayout!nsCSSFrameConstructor::RecreateFramesForContent(

                        class nsIContent * aContent = 0x03c57148)
gklayout!nsCSSFrameConstructor::ProcessRestyledFrames(

                        class nsStyleChangeList * aChangeList = 0x0012e284)
gklayout!nsIPresShell::ReconstructStyleDataInternal(void)
gklayout!nsIPresShell::ReconstructStyleData(void)
gklayout!PresShell::EndUpdate(

                        class nsIDocument * aDocument = 0x033978c8, 

                        unsigned int aUpdateType = 2)
gklayout!nsDocument::EndUpdate(

                        unsigned int aUpdateType = 2)
gklayout!nsStyleLinkElement::UpdateStyleSheet(

                        class nsIDocument * aOldDocument = 0x033978c8, 

                        class nsICSSLoaderObserver * aObserver = 0x00000000)
gklayout!nsHTMLStyleElement::UnbindFromTree(

                        int aDeep = 1, 

                        int aNullParent = 1)
gklayout!doRemoveChildAt(

                        unsigned int aIndex = 4, 

                        int aNotify = 1, 

                        class nsIContent * aKid = 0x03cb7848, 

                        class nsIContent * aParent = 0x035cadc8, 

                        class nsIDocument * aDocument = 0x033978c8, 

                        class nsAttrAndChildArray * aChildArray = 0x035cade0)
gklayout!nsGenericElement::RemoveChildAt(

                        unsigned int aIndex = 4, 

                        int aNotify = 1)
gklayout!nsGenericElement::RemoveChild(

                        class nsIDOMNode * aOldChild = 0x03cb7864, 

                        class nsIDOMNode ** aReturn = 0x0012ea68)
gklayout!nsHTMLHeadElement::RemoveChild(

                        class nsIDOMNode * aOldChild = 0x03cb7864, 

                        class nsIDOMNode ** aReturn = 0x0012ea68)
xpcom_core!XPTC_InvokeByIndex(

                        class nsISupports * that = 0x035cade4, 

                        unsigned int methodIndex = 0x11, 

                        unsigned int paramCount = 2, 

                        struct nsXPTCVariant * params = 0x0012ea58)
xpc3250!XPCWrappedNative::CallMethod(

                        class XPCCallContext * ccx = 0x0012ebd4, 

                        XPCWrappedNative::CallMode mode = CALL_METHOD (0))
xpc3250!XPC_WN_CallMethod(

                        struct JSContext * cx = 0x032f2e60, 

                        struct JSObject * obj = 0x02d246b8, 

                        unsigned int argc = 1, 

                        long * argv = 0x03ab64b4, 

                        long * vp = 0x0012ed34)


A hash of the backtrace has been used to distinguish this from other bugs already reported by the tester. However, the automated pen-tester is still in development and this may not be the case. Also, I haven't added stack hashes to bugs that were not uploaded by the tester, so this bug may very well exist on bugzilla already.

Comment 1

[Paul Nickerson]

[STACKHASH:6EC9645E4CF4BDF1C13A75AA31BF2BFD]

Comment 2

[Paul Nickerson]

Well, on the plus side, the pen-tester worked (almost) the whole way through. Still gotta work on the attachment uploader. On the down side, it reported the same bug as last time :P

Comment 3

[Paul Nickerson]

Actually, looking back at this thing, I changed my mind about the duplicate.

Comment 4

[Paul Nickerson]

Attachment: Manually uploaded testcase

Comment 5

[Mats Palmgren (inactive)]

Attachment: Stack (prior to crash)

Comment 6

[Mats Palmgren (inactive)]

NotifyListBoxBody() calls listBoxObject->GetListBoxBody() in an attempt
to get the frame, but that flushes frames so end up with a recursive call
to RecreateFramesForContent()... see attached stack.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsCSSFrameConstructor.cpp&rev=1.1110.6.81&root=/cvsroot&mark=9220#9196
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/xul/base/src/nsBoxObject.cpp&rev=1.50.10.6&root=/cvsroot&mark=170#164

Comment 7

[Mats Palmgren (inactive)]

Attachment: Wallpaper

This is a somewhat low-risk wallpaper we could take on branches unless
someone has a better idea.

Comment 8

[Boris Zbarsky [:bzbarsky]]

We need a version of GetListBoxBody that doesn't flush frames to use here.  The weakframe approach is sorta-maybe ok, but I'm not sure I would trust it to insure against all possible ways to try to damage the frame tree....

Comment 9

[Mats Palmgren (inactive)]

Attachment: Branch patch

Comment 10

[Mats Palmgren (inactive)]

It looks to me like this could also crash on trunk since GetListBoxBody()
leads to a Flush_Frames there too.  But the testcase doesn't trigger the
RecreateFramesForContent() call on trunk so we don't reach
NotifyListBoxBody().  I'm guessing a slightly different testcase would
crash on trunk though.

Comment 11

[Mats Palmgren (inactive)]

Attachment: Branch patch

Comment 12

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 276930 [details] [diff] [review]
Branch patch

We're not changing interfaces on branch.  Please add a new interface with the new method on it instead, and QI as needed.  See the various MOZILLA_1_8_BRANCH stuff on the branch.

On trunk this approach looks good, though.

Comment 13

[Mats Palmgren (inactive)]

> We're not changing interfaces on branch.

Ok, I somehow thought we could change the "private" ones, i.e. those
with "nsPI" in the name...

Comment 14

[Mats Palmgren (inactive)]

Attachment: Branch patch, rev. 3

Comment 15

[Mats Palmgren (inactive)]

Attachment: Branch patch, rev. 4

... shouldn't change IID for nsPIListBoxObject of course...

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 276969 [details] [diff] [review]
Branch patch, rev. 4

Looks good.  I agree that we should do this on trunk too.

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 276969 [details] [diff] [review]
Branch patch, rev. 4

Is this attachment ready for a branch approval request? If this is an appropriate fix for the trunk could we land it there first for some sanity checking?

Comment 18

[Daniel Veditz [:dveditz]]

Mats: is this fix going to make the 1.8.1.8 code freeze (Oct 3)?

Comment 19

[Mats Palmgren (inactive)]

Comment on attachment 276969 [details] [diff] [review]
Branch patch, rev. 4

It's ready for both branches.

Comment 20

[Mats Palmgren (inactive)]

Attachment: Trunk patch, rev. 1

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 283297 [details] [diff] [review]
Trunk patch, rev. 1

r+sr=bzbarsky

Comment 22

[Mats Palmgren (inactive)]

Comment on attachment 283297 [details] [diff] [review]
Trunk patch, rev. 1

Low-risk crash fix, if I can get this landed *now* it will give some "baking value" for the branch fix as well.

Comment 23

[Daniel Veditz [:dveditz]]

Comment on attachment 276969 [details] [diff] [review]
Branch patch, rev. 4

approved for 1.8.1.8 and 1.8.0.14, a=dveditz

Comment 24

[Mats Palmgren (inactive)]

MOZILLA_1_8_BRANCH
mozilla/layout/base/nsCSSFrameConstructor.cpp 	1.1110.6.90
mozilla/layout/xul/base/src/nsListBoxObject.cpp 	1.16.12.3
mozilla/layout/xul/base/src/nsPIListBoxObject.h 	1.1.2.3

MOZILLA_1_8_0_BRANCH
mozilla/layout/base/nsCSSFrameConstructor.cpp 	1.1110.6.12.2.65
mozilla/layout/xul/base/src/nsListBoxObject.cpp 	1.16.16.3
mozilla/layout/xul/base/src/nsPIListBoxObject.h 	1.1.4.3

Comment 25

[Mats Palmgren (inactive)]

trunk:
mozilla/layout/base/nsCSSFrameConstructor.cpp 	1.1407
mozilla/layout/xul/base/src/nsListBoxObject.cpp 	1.25
mozilla/layout/xul/base/src/nsPIListBoxObject.h 	1.3 

-> FIXED

Comment 26

[Carsten Book [:Tomcat]]

verified fixed on trunk with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9a9pre) Gecko/2007101204 Minefield/3.0a9pre ID:2007101204

also verified fixed 1.8.1.8 using  Mozilla/5.0 (Windows; U; Windows NT 5.2; de; rv:1.8.1.8) Gecko/2007100816 Firefox/2.0.0.8 and Mozilla/5.0 (Macintosh; U; Intel Mac OS X; ja-JP-mac; rv:1.8.1.8) Gecko/2007100816 Firefox/2.0.0.8

No crash on testcase - adding verified keyword

Comment 27

[Al Billings [:abillings - ex-MoCo]]

Verified in Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre. It still crashes in 1.5.0.12 Firefox but not in the current branch build above.

Marked as "Fixed" since this is a branch only bug.

Comment 31

[Mats Palmgren (inactive)]

Attachment: REINTRODUCE-BUG-392285

I tried reintroducing the bug to see if the attached test would crash
but it didn't.