Bug 15054 – Divide by 0 in AutoTableLayout::layout

• Status: RESOLVED
• Resolution: FIXED
• Component: Tables

[Brett Wilson (Google)]

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

[David Kilzer (:ddkilzer)]

Marvin, could you please attach the crash log to this bug?  Thanks!

[Antti Koivisto]

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?

[Brett Wilson (Google)]

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.

[Adam Roben (:aroben)]

(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!

[Brett Wilson (Google)]

Created attachment 17760 [details]
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.

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.

[Brett Wilson (Google)]

Wow, I wasted a long time :(