15054 2007-08-22 16:54:52 -0700 Divide by 0 in AutoTableLayout::layout 2007-12-06 16:20:00 -0800 1 1 1 Unclassified WebKit Tables 523.x (Safari 3) All All RESOLVED FIXED P2 Normal --- 1 brettw brettw koivisto mitz oldest_to_newest 1676 0 brettw 2007-08-22 16:54:52 -0700 This is based on a crash dump, so I don't have repro steps. However, it looks like the code can produce divide by 0. Here's the code in question: // spread over the rest if (available > 0 && nEffCols > numAutoEmptyCellsOnly) { int total = nEffCols - numAutoEmptyCellsOnly; // still have some width to spread int i = nEffCols; while (i--) { // variable columns with empty cells only don't get any width if (m_layoutStruct[i].width.isAuto() && m_layoutStruct[i].emptyCellsOnly) continue; int w = available / total; <<<< divide by 0 available -= w; total--; m_layoutStruct[i].calcWidth += w; } } The crash/divide by 0 is in "int w = available / total;" It looks like this crash will occur any time available > 0, nEffCols > numAutoEmptyCellsOnly, and numAutoEmptyCellsOnly > 0 and nEffCols > numAutoEmptyCellsOnly + 1 It appears that the common case is numAutoEmptyCellsOnly is 0, I did not see any other value in my testing, and I don't know how to get this value to be different. Maybe somebody familiar with table layout can produce a testcase. For example, if nEffCols = 3, numAutoEmptyCellsOnly = 1: We will get: i = 3 total = 2 The first time through the loop: i will be changed from 3 to 2 total -- means total = 1 The second time through the loop: i will be changed from 2 to 1 total -- means total = 0 The third time through the loop: i will be changed from 1 to 0 w = available / total -> DIVIDE BY 0 1644 1 ddkilzer 2007-08-22 22:03:59 -0700 Marvin, could you please attach the crash log to this bug? Thanks! 1631 2 koivisto 2007-08-23 02:17:45 -0700 It is not immediatly obvious to me how you would get this crash. numAutoEmptyCellsOnly should be equal to amount of cases where (m_layoutStruct[i].width.isAuto() && m_layoutStruct[i].emptyCellsOnly) == true. This crash should be reproducible. Could you find the page where it happened? 1338 3 brettw 2007-08-28 11:00:31 -0700 This crash is from my internal crash system, so it wouldn't be useful to you. I do not have a URL. Here is the not very exciting stack: 0x102b79ea [autotablelayout.cpp:695] WebCore::AutoTableLayout::layout() 0x102179ac [rendertable.cpp:287] WebCore::RenderTable::layout() 0x1026b9ec [renderblock.cpp:1214] WebCore::RenderBlock::layoutBlockChildren(bool) 0x1026f02a [renderblock.cpp:585] WebCore::RenderBlock::layoutBlock(bool) 0x102a80dc [rendertablecell.cpp:135] WebCore::RenderTableCell::layout() 0x102aa8ad [rendertablerow.cpp:131] WebCore::RenderTableRow::layout() 0x102b41c1 [rendercontainer.cpp:494] WebCore::RenderContainer::layout() 0x10217a4e [rendertable.cpp:299] WebCore::RenderTable::layout() 0x1026b9ec [renderblock.cpp:1214] WebCore::RenderBlock::layoutBlockChildren(bool) 0x1026f02a [renderblock.cpp:585] WebCore::RenderBlock::layoutBlock(bool) 0x102647ca [renderblock.cpp:494] WebCore::RenderBlock::layout() 0x1026b9ec [renderblock.cpp:1214] WebCore::RenderBlock::layoutBlockChildren(bool) 0x1026f02a [renderblock.cpp:585] WebCore::RenderBlock::layoutBlock(bool) 0x102647ca [renderblock.cpp:494] WebCore::RenderBlock::layout() 0x1026b9ec [renderblock.cpp:1214] WebCore::RenderBlock::layoutBlockChildren(bool) 0x1026f02a [renderblock.cpp:585] WebCore::RenderBlock::layoutBlock(bool) 0x102647ca [renderblock.cpp:494] WebCore::RenderBlock::layout() 0x10210220 [renderview.cpp:111] WebCore::RenderView::layout() 0x101d1915 [frameview.cpp:425] WebCore::FrameView::layout(bool) 0x1023589a [timer.h:96] WebCore::Timer<WebCore::PageCache>::fired() 0x102056f8 [timer.cpp:336] WebCore::TimerBase::fireTimers(double,WTF::Vector<WebCore::TimerBase *,0> const &) I will keep my eyes out for more crashes like this. 1335 4 aroben 2007-08-28 11:02:28 -0700 (In reply to comment #3) > This crash is from my internal crash system, so it wouldn't be useful to you. Maybe not, but an "internal crash system" certainly sounds useful! 63531 5 17760 brettw 2007-12-06 14:35:11 -0800 Created attachment 17760 Reduced test case Here are the requirements for the crash to occur in AutoTableLayout::layout(): * The width of the table must be not evenly divisible by the number of nonempty columns such that there is a remainder of 1 when you get to the block commented "// spread over the rest". * There must be a nonempty column, as well as an empty column with a fixed width, and there must be a row with colspan covering both of them. * When this bug is triggered, the empty column will get a width of type "fixed" and a number, and the effective width will be 0 (Auto). If you remove the colspan, the effective width will be the same as the width. * The setting of the effWidth to the width happens at the top of AutoTableLayout::calcEffectiveWidth. In the case of this bug, it is reset to zero on the line "m_layoutStruct[pos].effWidth = Length();" The bug occurs because when the number of fixed, relative, etc. columns is computed near the top of layout(), it uses effWidth, but in the "rest" clause, it uses "width". I think this is just a one line fix to use "effWidth" below. I will put a patch together. 63536 6 mitz 2007-12-06 14:52:44 -0800 I am sorry, this is <rdar://problem/5612459> which has recently been fixed in <http://trac.webkit.org/projects/webkit/changeset/28118>. I did not realize that it was the same as this bug. Please verify and close if you agree. 63545 7 brettw 2007-12-06 16:20:00 -0800 Wow, I wasted a long time :( 17760 2007-12-06 14:35:11 -0800 2007-12-06 14:35:11 -0800 Reduced test case reduced.html text/html 269 brettw PHRhYmxlIGNlbGxzcGFjaW5nPTAgY2VsbHBhZGRpbmc9MCBzdHlsZT0id2lkdGg6NzU5cHg7Ij48 dHI+DQogIDx0ZD48L3RkPg0KICA8dGQ+PC90ZD4NCiAgPHRkIHdpZHRoPSIxMCUiPjwvdGQ+DQo8 L3RyPjx0cj4NCiAgPHRkIHdpZHRoPSI5MCUiPkhlbGxvPC90ZD4NCiAgPHRkIHN0eWxlPXdpZHRo OjE1cHg7Ij48L3RkPg0KICA8dGQ+PC90ZD4NCjwvdHI+PHRyPg0KICA8dGQgY29sc3Bhbj0iMyIg c3R5bGU9IndpZHRoOjEwMCU7Ij48L3RkPg0KPC90cj48dGFibGU+DQo=