Null-dereference READ in blink::LowestCommonAncestor |
|||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5120408169480192 Fuzzer: ifratric-browserfuzzer-v3 Job Type: linux_msan_chrome Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000008 Crash State: blink::LowestCommonAncestor blink::PaintPropertyNode<blink::EffectPaintPropertyNode>::Changed blink::CompositedLayerRasterInvalidator::ChunkPropertiesChanged Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=537453:537463 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5120408169480192 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Feb 17 2018
Cause: A GraphicsLayer has a mask but there is no mask paint property allocated.
,
Feb 17 2018
,
Feb 17 2018
The root cause appears to be a LayoutObject with mask that is not a stacking context, because it was never marked has having a stacking context during style recalc.
,
Feb 17 2018
,
Feb 17 2018
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/bf54c05ccf13ff3b26a180f81186426bc56e4ccb ([SPv175] Enable SlimmingPaintV175 for experimental). If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
,
Feb 18 2018
DOM tree in this case:
<HTML>
<BODY>
<SELECT>
<OPTION>
The stacking context for the <option> element is not computed because
Element::RecalcShadowIncludingDescendantStylesForReattach has an early
out if HasCustomStyleCallbacks() returns true, which means that
Element::RecalcDescendantStylesForReattach() is never called for children.
In this case HasCustomStyleCallbacks is true for the <SELECT> element.
@Rune: why? What's the best way to fix this bug in CSS?
In the meantime we can work around it in paint with HasMask(), though
I don't quite understand how the computed style is created for the <option>
if the above is true.
,
Feb 18 2018
,
Feb 18 2018
Issue 813343 has been merged into this issue.
,
Feb 18 2018
Issue 813341 has been merged into this issue.
,
Feb 18 2018
,
Feb 18 2018
Issue 813302 has been merged into this issue.
,
Feb 18 2018
Issue 813293 has been merged into this issue.
,
Feb 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/36475d6932e756181d77e416d4d2ded7c8d76e36 commit 36475d6932e756181d77e416d4d2ded7c8d76e36 Author: Chris Harrelson <chrishtr@chromium.org> Date: Sun Feb 18 17:47:18 2018 [SPv175] Work around style bug for mask stacking contexts. In certain scenarios, there is a CSS bug in which an element with mask fails to be a stacking context, but HasMask still returns true. Bug: 813348 Change-Id: I693cd9a8139cda335ac144ce600538b274e75c7a Reviewed-on: https://chromium-review.googlesource.com/924767 Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Cr-Commit-Position: refs/heads/master@{#537566} [add] https://crrev.com/36475d6932e756181d77e416d4d2ded7c8d76e36/third_party/WebKit/LayoutTests/paint/masks/option-select-mask-crash.html [modify] https://crrev.com/36475d6932e756181d77e416d4d2ded7c8d76e36/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
,
Feb 18 2018
,
Feb 18 2018
Issue 813423 has been merged into this issue.
,
Feb 18 2018
,
Feb 19 2018
ClusterFuzz testcase 5041368691638272 is verified as fixed, so closing issue as verified. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Feb 19 2018
> DOM tree in this case: > > <HTML> > <BODY> > <SELECT> > <OPTION> > > The stacking context for the <option> element is not computed because > Element::RecalcShadowIncludingDescendantStylesForReattach has an early > out if HasCustomStyleCallbacks() returns true, which means that > Element::RecalcDescendantStylesForReattach() is never called for children. > > In this case HasCustomStyleCallbacks is true for the <SELECT> element. > > @Rune: why? What's the best way to fix this bug in CSS? This is actually about Squad and the fact that some CustomStyleForLayoutObject methods currently rely on parent LayoutObject which means we cannot compute style for those before we have attached the parent layout object. This is not the case for option, but the current solution is to skip all subtrees with custom hooks to avoid the problematic ones. However, style is then supposed to be calculated during AttachLayoutTree, so the fact that we skip ComputedStyle generation during style recalc should not be a problem as such. I can take a look at the test case and see why it fails to update opacity. > In the meantime we can work around it in paint with HasMask(), though > I don't quite understand how the computed style is created for the <option> > if the above is true. One thing that's special for OPTION is that we don't generate layout objects for them. I think this is an optimization which has been there since WebKit. It means that we'll have special handling of OPTION and OPTGROUP a couple of places and store ComputedStyle on the Element instead of the layout object. As mentioned above, the ComputedStyle for OPTION should have been createn in HTMLOptionElement::AttachLayoutTree in your case.
,
Feb 19 2018
ClusterFuzz has detected this issue as fixed in range 537565:537566. Detailed report: https://clusterfuzz.com/testcase?key=5120408169480192 Fuzzer: ifratric-browserfuzzer-v3 Job Type: linux_msan_chrome Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000008 Crash State: blink::LowestCommonAncestor blink::PaintPropertyNode<blink::EffectPaintPropertyNode>::Changed blink::CompositedLayerRasterInvalidator::ChunkPropertiesChanged Sanitizer: memory (MSAN) Regressed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=537453:537463 Fixed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=537565:537566 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5120408169480192 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.
,
Feb 19 2018
I've commented a bit in the review. We do create layout objects for option elements inside <select multiple>.
Even this works :-o (also in Firefox):
<!DOCTYPE html>
<style>
.row { display: table-row }
.cell { display: table-cell }
</style>
<select multiple>
<option class="row">
A
</option>
<option class="cell">
B
</option>
<option class="cell">
C
</option>
</select>
Anyway, I think it's most likely the problem arises later than style recalc.
,
Feb 21 2018
Issue 814010 has been merged into this issue.
,
Feb 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4b96f800bd6d6c017ac14ab19e7b341c1a6efcc6 commit 4b96f800bd6d6c017ac14ab19e7b341c1a6efcc6 Author: Rune Lillesveen <futhark@chromium.org> Date: Mon Feb 26 09:23:43 2018 Use StyleForLayoutObject for option to update stacking context properly. We used OriginalStyleForLayoutObject which doesn't update animations or stacking context. Hopefully a better fix for 813348. Bug: 813348 , 813439 , 813836 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I93584261a935c4813f618a9291d384a5b1203ec7 Reviewed-on: https://chromium-review.googlesource.com/924705 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#539093} [modify] https://crrev.com/4b96f800bd6d6c017ac14ab19e7b341c1a6efcc6/third_party/WebKit/Source/core/html/forms/HTMLOptionElement.cpp [modify] https://crrev.com/4b96f800bd6d6c017ac14ab19e7b341c1a6efcc6/third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp [modify] https://crrev.com/4b96f800bd6d6c017ac14ab19e7b341c1a6efcc6/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
,
Feb 26 2018
,
Feb 26 2018
ClusterFuzz testcase 6019770009518080 is still reproducing on tip-of-tree build (trunk). Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label.
,
Feb 26 2018
The repro is not the same issue. It's not triggered by my change either (I tried reverting my change reintroducing the previous fix, and it still fails a DCHECK). The new issue is a DCHECK triggering in GraphicsContext::EndLayer() and not a crash. It does not require SPv175 to reproduce. Interestingly, Clusterfuzz detected the issue as fixed on Feb 19th, which could mean this is a recent regression.
,
Feb 26 2018
It needs the full test-case to trigger the DCHECK. The minimized case does not have this problem.
,
Feb 26 2018
I can reproduce the DCHECK failure on rev 537627 from Feb 19th which includes chrishtr's fix. as well, so if ClusterFuzz really detected this as fixed in 537565:537566, it would mean there was a regression in 537566:537627?
,
Feb 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a8e991bcfdf10e4e5eb268f6eea06535dd35fc0e commit a8e991bcfdf10e4e5eb268f6eea06535dd35fc0e Author: Henrik Boström <hbos@chromium.org> Date: Mon Feb 26 14:56:27 2018 Revert "Use StyleForLayoutObject for option to update stacking context properly." This reverts commit 4b96f800bd6d6c017ac14ab19e7b341c1a6efcc6. Reason for revert: Suspected for being culprit of these consistent failures: https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Android%20%28Nexus4%29/builds/74755 Original change's description: > Use StyleForLayoutObject for option to update stacking context properly. > > We used OriginalStyleForLayoutObject which doesn't update animations or > stacking context. Hopefully a better fix for 813348. > > Bug: 813348 , 813439 , 813836 > Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 > Change-Id: I93584261a935c4813f618a9291d384a5b1203ec7 > Reviewed-on: https://chromium-review.googlesource.com/924705 > Reviewed-by: Chris Harrelson <chrishtr@chromium.org> > Commit-Queue: Rune Lillesveen <futhark@chromium.org> > Cr-Commit-Position: refs/heads/master@{#539093} TBR=chrishtr@chromium.org,futhark@chromium.org Change-Id: Ia2651fbe12ba83c4af65174370c969652fa61c65 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 813348 , 813439 , 813836 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Reviewed-on: https://chromium-review.googlesource.com/937621 Reviewed-by: Henrik Boström <hbos@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Cr-Commit-Position: refs/heads/master@{#539130} [modify] https://crrev.com/a8e991bcfdf10e4e5eb268f6eea06535dd35fc0e/third_party/WebKit/Source/core/html/forms/HTMLOptionElement.cpp [modify] https://crrev.com/a8e991bcfdf10e4e5eb268f6eea06535dd35fc0e/third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp [modify] https://crrev.com/a8e991bcfdf10e4e5eb268f6eea06535dd35fc0e/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
,
Feb 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/84bf0a62682d82bdfcdd5d3c3a6f3a89a7b2cc6b commit 84bf0a62682d82bdfcdd5d3c3a6f3a89a7b2cc6b Author: Rune Lillesveen <futhark@chromium.org> Date: Tue Feb 27 16:11:35 2018 Reland: Use StyleForLayoutObject for option to update stacking context. We used OriginalStyleForLayoutObject which doesn't update animations or stacking context. This is a reland of [1] which was reverted because the added unit test failed on Android because option elements do not create layout objects with the platform's theme. [1] https://crrev.com/4b96f800bd6d6c017ac14ab19e7b341c1a6efcc6 Bug: 813348 , 813439 , 813836 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I55d5f944e86ccaa326f01caa7f48606021f8df14 Reviewed-on: https://chromium-review.googlesource.com/939163 Commit-Queue: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#539445} [modify] https://crrev.com/84bf0a62682d82bdfcdd5d3c3a6f3a89a7b2cc6b/third_party/WebKit/Source/core/html/forms/HTMLOptionElement.cpp [modify] https://crrev.com/84bf0a62682d82bdfcdd5d3c3a6f3a89a7b2cc6b/third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp [modify] https://crrev.com/84bf0a62682d82bdfcdd5d3c3a6f3a89a7b2cc6b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by ClusterFuzz
, Feb 17 2018Labels: Test-Predator-Auto-Components