Last Comment Bug 311710 - Evil xul testcase, using display:-moz-grid-group causes crash [@ nsGridRow::IsCollapsed]
: Evil xul testcase, using display:-moz-grid-group causes crash [@ nsGridRow::I...
Status: VERIFIED FIXED
[sg:critical]
: fixed1.8, testcase, verified1.7.13
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: P1 critical (vote)
: mozilla1.8rc1
Assigned To: Boris Zbarsky [:bz] (TPAC)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-10-08 13:17 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2009-04-24 11:27 PDT (History)
5 users (show)
dveditz: blocking1.7.13+
dveditz: blocking‑aviary1.0.8+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (560 bytes, application/vnd.mozilla.xul+xml)
2005-10-08 13:19 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
Proposed fix (2.19 KB, patch)
2005-10-09 12:38 PDT, Boris Zbarsky [:bz] (TPAC)
neil: review+
roc: superreview+
dveditz: approval‑aviary1.0.8+
dveditz: approval1.7.13+
asa: approval1.8rc1+
Details | Diff | Splinter Review

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2005-10-08 13:17:17 PDT
See upcoming testcase, which causes a crash when clicking on the button.
Mozilla1.7 is also crashing with the testcase.

Talkback ID: TB10399249G
nsGridRow::IsCollapsed 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridRow.cpp,
line 102]
nsGrid::GetPrefRowHeight 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGrid.cpp,
line 905]
nsGridRowGroupLayout::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridRowGroupLayout.cpp,
line 128]
nsBoxFrame::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 948]
nsSprocketLayout::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp,
line 1346]
nsGridRowGroupLayout::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridRowGroupLayout.cpp,
line 113]
nsBoxFrame::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 948]
nsStackLayout::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsStackLayout.cpp,
line 95]
nsGridLayout2::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/grid/nsGridLayout2.cpp,
line 151]
nsBoxFrame::GetPrefSize 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 948]
nsSprocketLayout::PopulateBoxSizes 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp,
line 822]
nsSprocketLayout::Layout 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsSprocketLayout.cpp,
line 265]
nsBoxFrame::DoLayout 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 1106]
nsBoxFrame::DoLayout 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsBoxFrame.cpp,
line 1106]
nsRootBoxFrame::Reflow 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/xul/base/src/nsRootBoxFrame.cpp,
line 226]
nsContainerFrame::ReflowChild 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsContainerFrame.cpp,
line 891]
ViewportFrame::Reflow 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/generic/nsViewportFrame.cpp,
line 239]
IncrementalReflow::Dispatch 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 859]
PresShell::ProcessReflowCommands 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6492]
PresShell::WillPaint 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/layout/base/nsPresShell.cpp,
line 6136]
SHELL32.dll + 0x520c24 (0x778b0c24)
Comment 1 Martijn Wargers [:mwargers] (not working for Mozilla) 2005-10-08 13:19:44 PDT
Created attachment 198952 [details]
testcase
Comment 2 Boris Zbarsky [:bz] (TPAC) 2005-10-09 12:38:19 PDT
Created attachment 199020 [details] [diff] [review]
Proposed fix

Looks like a logic bug in grid to me.  And the assert change is to just make
them fire when they should (like this case).
Comment 3 neil@parkwaycc.co.uk 2005-10-13 08:07:40 PDT
Comment on attachment 199020 [details] [diff] [review]
Proposed fix

Quickly plussing this before my eyes glaze over again ;-)
Comment 4 Boris Zbarsky [:bz] (TPAC) 2005-10-14 15:51:54 PDT
Fixed on trunk.
Comment 5 Boris Zbarsky [:bz] (TPAC) 2005-10-14 15:56:22 PDT
Comment on attachment 199020 [details] [diff] [review]
Proposed fix

We should probably take this on branch.  The reason we crash here is an
out-of-bounds access on an array, followed by a method call on what we think is
the object there.  So this is probably exploitable...

The fix is quite safe -- the only affected case is when we go from having more
columns than we have explicitly defined to having exactly the right number, and
without this patch this case is guaranteed to lead us to dereference -1 on an
array....  So the only case affected is the case when we have a problem.
Comment 6 Boris Zbarsky [:bz] (TPAC) 2005-10-14 19:11:22 PDT
Daniel, should I request 1.7/aviary branch approval?
Comment 7 Stephen Donner [:stephend] 2005-10-15 18:29:59 PDT
Verified FIXED using a self-built SeaMonkey trunk as of PDT 5:00pm this evening
on testcase: https://bugzilla.mozilla.org/attachment.cgi?id=198952 with Windows XP.
Comment 8 Boris Zbarsky [:bz] (TPAC) 2005-10-16 09:33:01 PDT
Fixed on branch
Comment 9 Daniel Veditz [:dveditz] 2006-02-06 12:37:18 PST
Comment on attachment 199020 [details] [diff] [review]
Proposed fix

aviary101/moz17 landing approval: a=dveditz for drivers. Please add the fixed1.7.13 and fixed-aviary1.0.8 keywords when landed.
Comment 10 Boris Zbarsky [:bz] (TPAC) 2006-02-06 13:32:14 PST
*** Committing to MOZILLA_1_7_BRANCH... 
new revision: 1.14.50.1; previous revision: 1.14

*** Committing layout/xul/base/src/grid/nsGrid.cpp on AVIARY_1_0_1_20050124_BRANCH
new revision: 1.14.64.1; previous revision: 1.14
Comment 11 Jay Patel [:jay] 2006-02-09 15:01:23 PST
v.fixed on 1.0.1 Aviary branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.12) Gecko/20060209 Firefox/1.0.7
Comment 12 Marcia Knous [:marcia - use ni] 2006-03-09 19:14:49 PST
verified fixed on 1.7 branch using Mozilla 1.7.13 Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.13) Gecko/20060309. No crash with testcase.
Comment 13 Bob Clary [:bc:] 2009-04-24 11:27:36 PDT
crash test landed
http://hg.mozilla.org/mozilla-central/rev/e7bb6f9cd175

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