CHECK failure: !next_sibling || next_sibling->IsBox() in layout_block_flow.cc |
||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=6500804354375680 Fuzzer: bj_broddelwerk Job Type: linux_ubsan_vptr_chrome Platform Id: linux Crash Type: CHECK failure Crash Address: Crash State: !next_sibling || next_sibling->IsBox() in layout_block_flow.cc blink::LayoutBlockFlow::LayoutBlockChildren blink::LayoutBlockFlow::LayoutChildren Sanitizer: undefined (UBSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=502802:502818 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6500804354375680 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Jun 14 2018
Unable to find actual suspect through code search and also observing no suspected CL's under regression range, hence adding appropriate label and requesting someone from blink team to look in to this issue. Thanks!
,
Jun 15 2018
Would you mind looking into this one Morten?
,
Jun 15 2018
I now understand what goes wrong, and there are a couple of ways of fixing it:
A. A ruby specific fix that copes with situations that shouldn't occur (easy fix, probably safe)
B. A block-flow generic fix that makes sure this situation doesn't occur in the first place (it's causing other bugs too)
I have a reduction (attached). Writing a slightly more minimized testcase should be possible, but I need to study the ruby code a bit more first.
We have a ruby element with a mix of blocks and inlines, roughly like this: Block #1, inline, block #2, out-of-flow block. The inline is obviously wrapped inside an anonymous block. Block #2 is actually a table, but that doesn't really matter, as long as it's something block-level.
block #1
anon block
inline
block #2
out-of-flow
Now, if we remove block #2, the out-of-flow block sibling should be wrapped inside the anonymous block wrapper of the inline. We *should* have got this:
block #1
anon block
inline
out-of-flow
But this doesn't happen. The out-of-flow block remains where it is. So we get this:
block #1
anon block
inline
out-of-flow
If we then add an inline-level element after the out-of-flow block, we'll realize that it needs to be inside an anonymous block. Since there's no anonymous block adjacent to its insertion point, we create one, and wrap the inline AND the out-of-flow block inside it (which is kind of correct). The result is this:
block #1
anon block
inline
anon block #2
out-of-flow
inline
So we have two sibling anonymous blocks. This should never happen (unless we have continuations).
While this is happening, something is happening to the ruby layout structure as well. We split into two ruby runs and bases, and then merge it back later. It's during this merge that we end up with a layout block marked as having block children that has an inline child, which later on triggers the CHECK failure.
It happens from LayoutRubyBase::MoveBlockChildren(). There's first some code that merges content inside the last anonymous block in the first ruby base with the first anonymous block in the second base. At this point the second base contains basically this:
anon block #1
inline
anon block #2
out-of-flow
inline
(two nonsensical anonymous blocks, in other words - we shouldn't have any of them)
By emptying the first anonymous block into the first base, some piece of code FINALLY discovers that we don't need anonymous blocks, so all we have left in the second base:
out-of-flow
inline
The base is properly marked as having inline children now. Calling MoveAllChildrenIncludingFloatsTo() now without full_remove_insert is disastrous. We end up with one single base (marked as having block children) that contains:
something block level
anon block
inline
out-of-flow
inline
And we crash.
,
Jun 15 2018
,
Jun 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2394d0acf8bf0b0236b2646e4c823a57de7bd941 commit 2394d0acf8bf0b0236b2646e4c823a57de7bd941 Author: Morten Stenshorne <mstensho@chromium.org> Date: Sat Jun 16 15:44:49 2018 Floats and out-of-flow objects may not be adjacent to anonymous blocks. Floats and out-of-flow objects need to be true layout siblings of the inlines, or rendering will be wrong. This means that such objects should never be siblings of anonymous blocks, but rather inside them. This already works correctly for initial layout tree building, and also for many DOM manipulations. However, code was missing to satisfy this requirement if we removed a regular block that was a sibling of an anonymous block and either a float or out-of-flow positioned object. This even caused a crash triggered by ruby code, which ended up mixing inline and block children within the same container. That is not allowed. This happened in the MoveAllChildrenIncludingFloatsTo() call inside LayoutRubyBase::MoveBlockChildren(). Added a DCHECK to MoveAllChildrenIncludingFloatsTo() (which could fail prior to this fix); When moving children from one container to another, either both or none of the containers must have inline children. Bug: 852640 Change-Id: I51d3de12c73ddd07d6b4c1aa55221b4f92359ca7 Reviewed-on: https://chromium-review.googlesource.com/1102690 Reviewed-by: Emil A Eklund <eae@chromium.org> Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#567882} [add] https://crrev.com/2394d0acf8bf0b0236b2646e4c823a57de7bd941/third_party/WebKit/LayoutTests/external/wpt/css/CSS2/abspos/remove-block-between-inline-and-abspos.html [add] https://crrev.com/2394d0acf8bf0b0236b2646e4c823a57de7bd941/third_party/WebKit/LayoutTests/external/wpt/css/CSS2/floats/remove-block-between-inline-and-float.html [add] https://crrev.com/2394d0acf8bf0b0236b2646e4c823a57de7bd941/third_party/WebKit/LayoutTests/fast/ruby/merge-base-anonymous-blocks-crash.html [modify] https://crrev.com/2394d0acf8bf0b0236b2646e4c823a57de7bd941/third_party/blink/renderer/core/layout/layout_block_flow.cc
,
Jun 17 2018
ClusterFuzz has detected this issue as fixed in range 567880:567882. Detailed report: https://clusterfuzz.com/testcase?key=6500804354375680 Fuzzer: bj_broddelwerk Job Type: linux_ubsan_vptr_chrome Platform Id: linux Crash Type: CHECK failure Crash Address: Crash State: !next_sibling || next_sibling->IsBox() in layout_block_flow.cc blink::LayoutBlockFlow::LayoutBlockChildren blink::LayoutBlockFlow::LayoutChildren Sanitizer: undefined (UBSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=502802:502818 Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_chrome&range=567880:567882 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6500804354375680 See https://github.com/google/clusterfuzz-tools for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Jun 17 2018
ClusterFuzz testcase 6500804354375680 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Jun 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e9d1ed958ab494661acc228f31d879e490352d7a commit e9d1ed958ab494661acc228f31d879e490352d7a Author: Abhishek Arya <inferno@chromium.org> Date: Sun Jun 17 18:28:17 2018 Revert "Floats and out-of-flow objects may not be adjacent to anonymous blocks." This reverts commit 2394d0acf8bf0b0236b2646e4c823a57de7bd941. Reason for revert: Caused ClusterFuzz regressions - 853522, 853537, 853538, 853540. Original change's description: > Floats and out-of-flow objects may not be adjacent to anonymous blocks. > > Floats and out-of-flow objects need to be true layout siblings of the > inlines, or rendering will be wrong. This means that such objects should > never be siblings of anonymous blocks, but rather inside them. This > already works correctly for initial layout tree building, and also for > many DOM manipulations. However, code was missing to satisfy this > requirement if we removed a regular block that was a sibling of an > anonymous block and either a float or out-of-flow positioned object. > > This even caused a crash triggered by ruby code, which ended up mixing > inline and block children within the same container. That is not > allowed. This happened in the MoveAllChildrenIncludingFloatsTo() call > inside LayoutRubyBase::MoveBlockChildren(). Added a DCHECK to > MoveAllChildrenIncludingFloatsTo() (which could fail prior to this fix); > When moving children from one container to another, either both or none > of the containers must have inline children. > > Bug: 852640 > Change-Id: I51d3de12c73ddd07d6b4c1aa55221b4f92359ca7 > Reviewed-on: https://chromium-review.googlesource.com/1102690 > Reviewed-by: Emil A Eklund <eae@chromium.org> > Commit-Queue: Morten Stenshorne <mstensho@chromium.org> > Cr-Commit-Position: refs/heads/master@{#567882} TBR=eae@chromium.org,mstensho@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 852640 Change-Id: I0af60d9eac770f21b6a0fbeccf88bb43d5efd6fb Reviewed-on: https://chromium-review.googlesource.com/1103503 Reviewed-by: Abhishek Arya <inferno@chromium.org> Commit-Queue: Abhishek Arya <inferno@chromium.org> Cr-Commit-Position: refs/heads/master@{#567914} [delete] https://crrev.com/1333bf9732bbf928df2d75d62bfef3791cb097de/third_party/WebKit/LayoutTests/external/wpt/css/CSS2/abspos/remove-block-between-inline-and-abspos.html [delete] https://crrev.com/1333bf9732bbf928df2d75d62bfef3791cb097de/third_party/WebKit/LayoutTests/external/wpt/css/CSS2/floats/remove-block-between-inline-and-float.html [delete] https://crrev.com/1333bf9732bbf928df2d75d62bfef3791cb097de/third_party/WebKit/LayoutTests/fast/ruby/merge-base-anonymous-blocks-crash.html [modify] https://crrev.com/e9d1ed958ab494661acc228f31d879e490352d7a/third_party/blink/renderer/core/layout/layout_block_flow.cc
,
Jun 18 2018
,
Jun 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b414bf176fc6451c33c2c5c4b9609bd28ea3e5a1 commit b414bf176fc6451c33c2c5c4b9609bd28ea3e5a1 Author: Morten Stenshorne <mstensho@chromium.org> Date: Mon Jun 18 22:21:54 2018 Reland: Floats and out-of-flow objects may not be adjacent to anonymous blocks. Floats and out-of-flow objects need to be true layout siblings of the inlines, or rendering will be wrong. This means that such objects should never be siblings of anonymous blocks, but rather inside them. This already works correctly for initial layout tree building, and also for many DOM manipulations. However, code was missing to satisfy this requirement if we removed a regular block that was a sibling of an anonymous block and either a float or out-of-flow positioned object. This even caused a crash triggered by ruby code, which ended up mixing inline and block children within the same container. That is not allowed. This happened in the MoveAllChildrenIncludingFloatsTo() call inside LayoutRubyBase::MoveBlockChildren(). Added a DCHECK to MoveAllChildrenIncludingFloatsTo() (which could fail prior to this fix); When moving children from one container to another, either both or none of the containers must have inline children. This is a reland of https://chromium-review.googlesource.com/1102690 with some modifications to avoid bug 853552 . Bug: 852640 Change-Id: I0f8a0aa5523e8fe60c841164d25aad088f4b728f Reviewed-on: https://chromium-review.googlesource.com/1104900 Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Commit-Queue: Emil A Eklund <eae@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#568196} [add] https://crrev.com/b414bf176fc6451c33c2c5c4b9609bd28ea3e5a1/third_party/WebKit/LayoutTests/external/wpt/css/CSS2/abspos/remove-block-between-inline-and-abspos.html [add] https://crrev.com/b414bf176fc6451c33c2c5c4b9609bd28ea3e5a1/third_party/WebKit/LayoutTests/external/wpt/css/CSS2/floats/remove-block-between-inline-and-float.html [add] https://crrev.com/b414bf176fc6451c33c2c5c4b9609bd28ea3e5a1/third_party/WebKit/LayoutTests/fast/block/toggle-float-add-continuation-crash.html [add] https://crrev.com/b414bf176fc6451c33c2c5c4b9609bd28ea3e5a1/third_party/WebKit/LayoutTests/fast/ruby/merge-base-anonymous-blocks-crash.html [modify] https://crrev.com/b414bf176fc6451c33c2c5c4b9609bd28ea3e5a1/third_party/blink/renderer/core/layout/layout_block_flow.cc
,
Jun 19 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by ClusterFuzz
, Jun 14 2018Labels: Test-Predator-Auto-Components