125689 - Mozinvaders crashes Mozilla! [@ nsGrid::FindRowsAndColumns]

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Tom Parker]

Go to http://games.mozdev.org/arcade/mozinvaders/
Install Mozinvaders
Enter chrome://mozinvaders/content

Talkback IDs for this crash:
TB2942587K
TB2509208G
TB2508801M
TB2508677Y

Comment 1

[Christopher Whitt]

Reporter, please set severity -> critical and keyword: crash for crash bug
reports.  Thanks.

Comment 2

[David Bradley]

Attachment: Stack trace for TB2942587K

Added the registers and code at the end as well. Looks like a null pointer
issue.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

My Linux debug build also crahes, at layout/xul/base/src/grid/nsGrid.cpp:282 -
mBox is null.

Confirming, OS->All

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Actually, every member of this grid is NULL or 0...

Comment 5

[Robert T-BSE]

http://xulplanet.com/downloads/view.cgi?category=applications&view=prefbar
this one crashes under linux
build 20002021423
might be the same problem

Comment 6

[Brendan Eich [:brendan]]

Hyatt may not get to this before 0.9.9 freezes. Trudelle, any ideas for who
might be able to diagnose quickly? Thanks.

/be

Comment 7

[Peter Trudelle]

->bryner?  cc hewitt

Comment 8

[Tom Parker]

http://xulplanet.com/downloads/view.cgi?category=applications&view=prefbar
(as mentioned by Robert T-BSE) also crashes windows build
Build ID:2002021403

Comment 9

[Tom Parker]

Still crashes with 0.9.9, Win 98. Talkback ID: TB3990460H

Comment 10

[ly nguyen]

user comments from this stack below show more test cases for this stack sig.

     Count   Offset    Real Signature
[ 9   nsGrid::FindRowsAndColumns 00d7b599 - nsGrid::FindRowsAndColumns ]
[ 6   nsGrid::FindRowsAndColumns 3014f79a - nsGrid::FindRowsAndColumns ]
[ 2   nsGrid::FindRowsAndColumns 3615fa18 - nsGrid::FindRowsAndColumns ]
[ 1   nsGrid::FindRowsAndColumns c6f63f85 - nsGrid::FindRowsAndColumns ]
[ 1   nsGrid::FindRowsAndColumns bd33a5ad - nsGrid::FindRowsAndColumns ]
 
     Crash date range: 2002-05-25 to 2002-05-28
     Min/Max Seconds since last crash: 107 - 7628
     Min/Max Runtime: 110 - 7628
     Keyword List :  
     Count   Platform List 
     11   Windows 98 4.90 build 73010104
     6   Windows 98 4.10 build 67766446
     2   Windows NT 5.1 build 2600
 
     Count   Build Id List 
     19   2002051220
 
     No of Unique Users        18
 
 Stack trace(Frame) 

	 nsGrid::FindRowsAndColumns
[d:\builds\seamonkey\mozilla\layout\xul\base\src\grid\nsGrid.cpp  line 284] 
	 nsGrid::RebuildIfNeeded
[d:\builds\seamonkey\mozilla\layout\xul\base\src\grid\nsGrid.cpp  line 189] 
	 nsGrid::GetRowCount
[d:\builds\seamonkey\mozilla\layout\xul\base\src\grid\nsGrid.cpp  line 1348] 
	 nsGridLayout2::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\grid\nsGridLayout2.cpp  line 120] 
	 nsContainerBox::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 537] 
	 nsBoxFrame::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 1121] 
	 nsSprocketLayout::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp  line 1373] 
	 nsContainerBox::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 537] 
	 nsBoxFrame::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 1121] 
	 nsStackLayout::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsStackLayout.cpp  line 124] 
	 nsContainerBox::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 537] 
	 nsBoxFrame::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 1121] 
	 nsSprocketLayout::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp  line 1373] 
	 nsContainerBox::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 537] 
	 nsBoxFrame::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 1121] 
	 nsSprocketLayout::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp  line 1373] 
	 nsContainerBox::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 537] 
	 nsBoxFrame::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 1121] 
	 nsStackLayout::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsStackLayout.cpp  line 124] 
	 nsContainerBox::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 537] 
	 nsBoxFrame::GetMinSize
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 1121] 
	 nsBoxFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 951] 
	 nsRootBoxFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsRootBoxFrame.cpp  line 243] 
	 nsContainerFrame::ReflowChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp  line 807] 
	 ViewportFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsViewportFrame.cpp  line 588] 
	 nsHTMLReflowCommand::Dispatch
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsHTMLReflowCommand.cpp  line
218] 
	 PresShell::ProcessReflowCommand
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 6305] 
	 PresShell::ProcessReflowCommands
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 6360] 
	 PresShell::FlushPendingNotifications
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5069] 
	 nsXULDocument::FlushPendingNotifications
[d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp  line 2356] 
	 nsXBLResourceLoader::NotifyBoundElements
[d:\builds\seamonkey\mozilla\content\xbl\src\nsXBLResourceLoader.cpp  line 281] 
	 nsXBLResourceLoader::StyleSheetLoaded
[d:\builds\seamonkey\mozilla\content\xbl\src\nsXBLResourceLoader.cpp  line 207] 
	 CSSLoaderImpl::InsertSheetInDoc
[d:\builds\seamonkey\mozilla\content\html\style\src\nsCSSLoader.cpp  line 1198] 
	 InsertPendingSheet
[d:\builds\seamonkey\mozilla\content\html\style\src\nsCSSLoader.cpp  line 757] 
	 nsVoidArray::EnumerateForwards
[d:\builds\seamonkey\mozilla\xpcom\ds\nsVoidArray.cpp  line 664] 
	 CSSLoaderImpl::Cleanup
[d:\builds\seamonkey\mozilla\content\html\style\src\nsCSSLoader.cpp  line 821] 
	 CSSLoaderImpl::SheetComplete
[d:\builds\seamonkey\mozilla\content\html\style\src\nsCSSLoader.cpp  line 914] 
	 CSSLoaderImpl::ParseSheet
[d:\builds\seamonkey\mozilla\content\html\style\src\nsCSSLoader.cpp  line 949] 
	 CSSLoaderImpl::DidLoadStyle
[d:\builds\seamonkey\mozilla\content\html\style\src\nsCSSLoader.cpp  line 985] 
	 SheetLoadData::OnStreamComplete
[d:\builds\seamonkey\mozilla\content\html\style\src\nsCSSLoader.cpp  line 745] 
	 nsStreamLoader::OnStopRequest
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsStreamLoader.cpp  line 163] 
	 nsJARChannel::OnStopRequest
[d:\builds\seamonkey\mozilla\netwerk\protocol\jar\src\nsJARChannel.cpp  line 609] 
	 nsOnStopRequestEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsRequestObserverProxy.cpp  line 213] 
	 PL_HandleEvent
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 597] 
	 PL_ProcessPendingEvents
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 530] 
	 _md_EventReceiverProc
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 1078] 
	 KERNEL32.DLL + 0x248f7 (0xbff848f7)  
	 0x00688bfa  
	 0x00058f64   
 
     (6772497)	URL: www.netscape.com
     (6772497)	Comments: I was starting a connection to the Internet after rebooting from a
Netscape 7.0 installation.
     (6716442)	URL: yahoo.com
     (6716442)	Comments: attempting to connect to read mail
     (6699357)	Comments: Browser failed to launch.
     (6698671)	Comments: I was just tryin to surf
     (6695432)	Comments: Startup
     (6675877)	Comments: connecting
     (6666093)	Comments: Trying to view a picture from my hard disk

Comment 11

[ly nguyen]

Attachment: local params at line of crash

Comment 12

[timeless]

nsGrid.cpp, line 125 -- nsGrid::nsGrid():mBox(nsnull),
nsGrid.cpp, line 282 -- mBox->GetChildBox(&child);
nsGrid.h, line 94 -- void SetBox(nsIBox* aBox) { mBox = aBox; }
nsGrid.h, line 95 -- nsIBox* GetBox() { return mBox; }
nsGridRow.cpp, line 48 -- nsGridRow::nsGridRow():mBox(nsnull), 
nsGridRow.cpp, line 93 -- if (mBox) 
nsGridRow.cpp, line 94 -- mBox->MarkDirty(aState);
nsGridRow.cpp, line 102 -- if (mBox)
nsGridRow.cpp, line 103 -- mBox->IsCollapsed(aState,isCollapsed);
nsGridRow.h, line 64 -- nsIBox* GetBox() { return mBox; }

so usuaully things protect mBox. The first fix is easy, obvious, and probably 
correct.

Comment 13

[timeless]

Attachment: check for null mBox

Comment 14

[Jan Varga [:janv]]

Comment on attachment 85560 [details] [diff] [review]
check for null mBox

r=varga

Comment 15

[Brian Ryner (not reading)]

timeless, I'll just reassign this to you.  I don't know the grid code well
enough to say if there's an underlying problem, but if we null-check in other
sports this is probably not unexpected.

Comment 16

[Brian Ryner (not reading)]

taking back; timeless says he won't have time to deal with this.

Comment 17

[Mercedes]

05/06/02 16:00 PM MST -- Able to reproduce on Windows 98 SE

1. Went to http://games.mozdev.org/arcade/mozinvaders/
2. Installed Mozinvaders
3. Entered chrome://mozinvaders/content
4. Result: 

NETSCP caused an invalid page fault in
module GKLAYOUT.DLL at 016f:6045cd1f.
Registers:
EAX=00000000 CS=016f EIP=6045cd1f EFLGS=00010246
EBX=02e418f8 SS=0177 ESP=0068f128 EBP=0068f17c
ECX=02e418ec DS=0177 ESI=00000000 FS=7057
EDX=0068f178 ES=0177 EDI=02e418fc GS=0000
Bytes at CS:EIP:
8b 08 50 ff 51 54 39 75 fc 0f 84 2b 01 00 00 8b 
Stack dump:
0068f178 02e418fc 02e418ec 02e418f8 00000000 00000000 00000000 00000000 
00000000 00000000 00000000 00000000 026245e4 02e418e0 60444c2e 026245e4 

Comment 18

[timeless]

back to me...

Comment 19

[tor]

I'd be interested in finding out why we ended up with a null mBox,
to make sure we aren't fixing the symptom instead of an underlying
problem.  See if you can get a layout person to take a look.

Comment 20

[Jan Varga [:janv]]

I guess <grid> is collapsed in this case, therefore nsGridLayout2::Layout() was
not yet called to initialize nsGrid:mBox

Comment 21

[kinmoz]

Attachment: Minimal Testcase (<grid></grid>)

Here's the minimal amount of xul needed to crash:

<?xml version="1.0"?>
<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
<grid></grid>
</window>

mBox is null when grid has no children.

Comment 22

[Shiva Thirumazhusai]

Clicking on the attachment crashes the browser.  Is it possible to get this
fixed before Zarro boogs ?

Comment 23

[kinmoz]

Comment on attachment 85560 [details] [diff] [review]
check for null mBox

sr=[e-mail]

So it looks like the grid->mBox is supposed to point to the rowgroup frame in
the grid. Since the grid has no rows or columns, it's null.

I think the patch may be ok in this case. We should add a comment above the |if
(mBox)| check that states how mBox could be null, citing the <grid></grid>
case.

By the way, even with the fix, the mozinvaders game still doesn't work, and
pegs my CPU at 100%. But that's another matter.

Comment 24

[kinmoz]

As a sidenote, it looks like <grid></grid> prevents things from rendering on
screen, as can be seen with this:

    <label value="Hello"/>
    <grid></grid>    <label value="World"/>

Removing it, allows both labels to render on screen. Likewise removing it from
the mozinvaders xul causes things to render.

Comment 25

[ly nguyen]

i think jan is right. i'm running the debugger and setting a breakpoint active at 
94: void SetBox(nsIBox* aBox) { mBox = aBox; }
before clicking on the attachment and it never breaks there. since the constructor sets all its class vars to null/0 and |SetBox| never gets called, mbox will stay null.

Comment 26

[ly nguyen]

Attachment: patch for the degenerate case (null check)

since this case: <grid></grid> has no rows nor columns, it is a degenerate
case. to me, having a null check there is fine in order to handle this
particular case.

Comment 27

[timeless]

i checked in my patch with a comment.

there's no need for early return because the while loop will fail ...

i was just waiting for my checkin to clear before commenting

Comment 28

[ly nguyen]

well, the reason for the new patch is that it only has one condition to check
vs. two when |mBox| is null.

Comment 29

[timeless]

*** Bug 137249 has been marked as a duplicate of this bug. ***

Comment 30

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 31

[lchiang]

Is this still a top crash?  I would verify, but I don't want to install
mozinvaders on my machine.

Comment 32

[greer]

Lisa, 15 incidents in the past ten days of crash data for M100 (which was
pre-checkin). No incidents on the branch or Trunk.

Marking VERIFIED (on the Trunk).

However, there is not comment in response to Jud's request (comment #29) for a
branch checkin. Was this ever checked in to the branch? Or did it just go away?

Comment 33

[John Morrison]

Yes, this was checked in to the MOZILLA_1_0_BRANCH, rev 1.10.14.2 of nsGrid.cpp
by timeless, Jul 02, 12:29pm, and the minimal testcase does not crash the 8/23 
1.0.1 build.

Comment 34

[Jesse Ruderman]

in-testsuite+: I think this is covered by 321073-1.xul.