New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 722041 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
NOT IN USE
Closed: Jun 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Floating-point-exception in blink::LayoutMultiColumnSet::PageRemainingLogicalHeightForOffset

Project Member Reported by ClusterFuzz, May 13 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5591976857829376

Fuzzer: marty_html_twiddler
Job Type: mac_asan_chrome
Platform Id: mac

Crash Type: Floating-point-exception
Crash Address: 
Crash State:
  blink::LayoutMultiColumnSet::PageRemainingLogicalHeightForOffset
  blink::LayoutBlockFlow::AdjustedMarginBeforeForPagination
  blink::LayoutBlockFlow::EstimateLogicalTopPosition
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=452830:452941

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5591976857829376


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Project Member

Comment 1 by ClusterFuzz, May 13 2017

Labels: OS-Android
Components: Blink>Layout
[mac bug triage] 

Comment 3 by e...@chromium.org, May 24 2017

Owner: msten...@opera.com
Status: Assigned (was: Untriaged)

Comment 4 by msten...@opera.com, May 24 2017

Got rid of the script.
tc.html
386 bytes View Download

Comment 5 by msten...@opera.com, May 24 2017

tc-minimal.html
247 bytes View Download
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/84653db9a6606722184ac94f6216224400738ba4

commit 84653db9a6606722184ac94f6216224400738ba4
Author: Morten Stenshorne <mstensho@opera.com>
Date: Thu Jun 01 13:45:55 2017

No longer let 0 mean that fragmentainer height is unknown.

For multicol, PageLogicalHeightForOffset() would normally figure out what to
return by consulting the flow thread, which would find the appropriate column
set, which in turn would find the appropriate fragmentainer group and return
its height.

We used to treat a 0 value as "unknown" most of the time (but there were also
cases where we'd accept it as a column height). We now always have to call
IsPageLogicalHeightKnown() first to tell whether it's known or not. This is
reasonable, since the calling code always has to act upon the situation of not
knowing the height (typically skip some steps, since fragmentation is
impossible until height is known).

It is now forbidden to call PageLogicalHeightForOffset() if height is unknown
(there are DCHECKs). The height is unknown in many cases in the first multicol
balancing pass. The height will be known once we have made a column height
estimate. It doesn't have to be the final and correct height. This CL doesn't
change anything in that regard, but now we are required to be sure that we
have some clue at all before dealing with fragmentainer heights.

MultiColumnFragmentainerGroup now has a flag that tells whether the logical
height is known or not. We need the flag, because the logical height may
actually end up as 0, e.g. when a multicol container just has zero-height
content, or when the multicol container itself has a specified height of 0.
This unclamped height will be used as block progression for the column row,
which will contribute to the final height of the multicol container. The
actual column height will be clamped to not be less than 1px. This is in
accordance with the spec [1]. We previously used to treat truly zero-height
fragmentainer groups as having an unknown height in some parts of the code,
while in other parts of the code we'd just accept it and end up dividing by
it (to convert a flow thread offset to a column index, for instance).

This is a clean-up CL that happens to fix bugs.

[1] https://drafts.csswg.org/css-break/#breaking-rules

BUG= 722041 , 722754 

Change-Id: I63550d804bef073a5c24570d63bd55176ec5e396
Reviewed-on: https://chromium-review.googlesource.com/514049
Commit-Queue: Morten Stenshorne <mstensho@opera.com>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#476270}
[add] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/LayoutTests/fast/multicol/doubly-nested-with-zero-width-flexbox-and-forced-break-crash.html
[add] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/LayoutTests/fast/multicol/nested-with-spanner-inside-margins-crash.html
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/ColumnBalancer.h
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/FragmentationContext.h
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/LayoutBlockFlow.cpp
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/LayoutMultiColumnFlowThread.cpp
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/LayoutMultiColumnSet.cpp
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/LayoutPagedFlowThread.cpp
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/LayoutTable.cpp
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.cpp
[modify] https://crrev.com/84653db9a6606722184ac94f6216224400738ba4/third_party/WebKit/Source/core/layout/MultiColumnFragmentainerGroup.h

Comment 7 by msten...@opera.com, Jun 1 2017

Status: Fixed (was: Assigned)
Project Member

Comment 8 by ClusterFuzz, Jun 2 2017

ClusterFuzz has detected this issue as fixed in range 476262:476271.

Detailed report: https://clusterfuzz.com/testcase?key=5591976857829376

Fuzzer: marty_html_twiddler
Job Type: mac_asan_chrome
Platform Id: mac

Crash Type: Floating-point-exception
Crash Address: 
Crash State:
  blink::LayoutMultiColumnSet::PageRemainingLogicalHeightForOffset
  blink::LayoutBlockFlow::AdjustedMarginBeforeForPagination
  blink::LayoutBlockFlow::EstimateLogicalTopPosition
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=452830:452941
Fixed: https://clusterfuzz.com/revisions?job=mac_asan_chrome&range=476262:476271

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5591976857829376


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

Sign in to add a comment