New issue
Advanced search Search tips

Issue 713024 link

Starred by 1 user
Status: Fixed
Owner:
f...@opera.com
Closed: May 2017
Cc:
f...@opera.com
ifratric@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Crash in blink::SVGInlineTextBox::constructTextRun

Project Member Reported by ClusterFuzz, Apr 19 2017

Comment 1 by mummare...@chromium.org, Apr 21 2017

Cc: wangxianzhu@chromium.org f...@opera.com
Components: Blink>SVG
Labels: M-60 Test-Predator-Wrong
Predator and regression range did n't given any suspected CL. could someone please take a look?
Thank you.

Comment 2 by f...@opera.com, Apr 21 2017

Owner: f...@opera.com
Status: Assigned (was: Untriaged)
Will take a look

Comment 3 by f...@opera.com, Apr 21 2017 

The testcase appears to be timing sensitive in some way, which means the regression range is probably unreliable (wrong). It also makes reduction a somewhat tedious process...

Comment 4 by schenney@chromium.org, Apr 21 2017

Labels: PaintTeamTriaged-20170421 BugSource-Chromium

Comment 5 by f...@opera.com, Apr 24 2017

Cc: -wangxianzhu@chromium.org
It looks like this is triggered by the selection style handling in the SVG text painting code (SVGResourcesCache::ClientStyleChanged calls in SVGInlineTextBoxPainter::PaintText), pretty sure I've seen this being triggered before... relevant callstack excerpt below. Attaching minimized TC.

[32425:32425:0424/142720.838178:13751527619047:FATAL:FrameView.cpp(2260)] Check failed: false. 
#0 0x7f3d051577db base::debug::StackTrace::StackTrace()
#1 0x7f3d051564dc base::debug::StackTrace::StackTrace()
#2 0x7f3d051c9bf3 logging::LogMessage::~LogMessage()
#3 0x7f3cfcdd5df7 blink::FrameView::CheckLayoutInvalidationIsAllowed()
#4 0x7f3cfcdd627e blink::FrameView::ScheduleRelayoutOfSubtree()
#5 0x7f3cfd414b47 blink::LayoutObject::ScheduleRelayout()
#6 0x7f3cfd41470a blink::LayoutObject::MarkContainerChainForLayout()
#7 0x7f3cfca76427 blink::LayoutObject::SetNeedsLayout()
#8 0x7f3cfcde7c1d blink::LayoutObject::SetNeedsLayoutAndFullPaintInvalidation()
#9 0x7f3cfd5898b6 blink::LayoutSVGResourceContainer::MarkForLayoutAndParentResourceInvalidation()
#10 0x7f3cfd589489 blink::LayoutSVGResourceContainer::MarkAllClientsForInvalidation()
#11 0x7f3cfd58dd3c blink::LayoutSVGResourceFilter::RemoveAllClientsFromCache()
#12 0x7f3cfd589949 blink::LayoutSVGResourceContainer::MarkForLayoutAndParentResourceInvalidation()
#13 0x7f3cfd589f93 blink::RemoveFromCacheAndInvalidateDependencies()
#14 0x7f3cfd589905 blink::LayoutSVGResourceContainer::MarkForLayoutAndParentResourceInvalidation()
#15 0x7f3cfd5c0a31 blink::SVGResourcesCache::ClientStyleChanged()
#16 0x7f3cfd7a9a3a blink::SVGInlineTextBoxPainter::PaintText()
svg-selection-pseudo-elm-resource-invalidation.html
531 bytes View Download

Comment 6 by f...@opera.com, Apr 26 2017 

There was one "obvious" way to address this (tried at https://codereview.chromium.org/2846513002), but it turns out other things depend on that behavior. Fixing this "the right way (TM)" will require more work...
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 28 2017 

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

commit aa5f200c624aeb429e0d8084a11670feccf4f540
Author: fs <fs@opera.com>
Date: Fri Apr 28 19:09:40 2017

Refactor FilterData::state_ handling in SVGFilterPainter

Make FilterData::state_ only be checked and updated within
SVGFilterPainter, and not by SVGFilterRecordingContext or the local
painting helper.
Instead SVGFilterRecordingContext only manages the recording state, and
gets passed bounds while returning a paint record.
This simplifies some of the corner-cases with regards to how "aborted"
filters are handled, getting rid of some "FilterData is null" in many
cases.

BUG= 713024 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2851753002
Cr-Commit-Position: refs/heads/master@{#468085}

[modify] https://crrev.com/aa5f200c624aeb429e0d8084a11670feccf4f540/third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp
[modify] https://crrev.com/aa5f200c624aeb429e0d8084a11670feccf4f540/third_party/WebKit/Source/core/paint/SVGFilterPainter.h
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 28 2017 

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

commit b599adbfc98de4cf9d71047c5ca092c9ce51e302
Author: fs <fs@opera.com>
Date: Fri Apr 28 19:34:41 2017

Abort the SVG filter content recording if the FilterData was dropped

In the (rare) case of a recording being started and the FilterData
structure being yanked away (and destroyed) from under
SVGFilterPainter's feet, we need to put the PaintController in a
consistent state before destroying it.
Add a new method SVGFilterRecordingContext::Abort() and call that when
SVGFilterPainter::FinishEffect encounters a null FilterData for the
LayoutObject.

BUG= 713024 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2847133002
Cr-Commit-Position: refs/heads/master@{#468092}

[modify] https://crrev.com/b599adbfc98de4cf9d71047c5ca092c9ce51e302/third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp
[modify] https://crrev.com/b599adbfc98de4cf9d71047c5ca092c9ce51e302/third_party/WebKit/Source/core/paint/SVGFilterPainter.h
Project Member

Comment 9 by bugdroid1@chromium.org, May 2 2017 

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

commit 9eba37a11e54d4f0808f43db1371dc2e062efb8d
Author: fs <fs@opera.com>
Date: Tue May 02 16:06:31 2017

More targeted resource-switching mechanism for SVG selection painting

The mechanism by which resources are generated for painting using
selection style for SVG text is a bit too heavy-handed, and can end up
invalidating both layout and other things. All that is needed is looking
up any <paint> ('fill' or 'stroke') references and invalidating any
state from the non-selection style.

Use a reduced/tailored version of SVGResourcesCache::ClientStyleChanged
that only recreates/swaps the SVGResources object for the LayoutObject
and wrap that mechanism in a scope object.

BUG= 713024 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2846513002
Cr-Commit-Position: refs/heads/master@{#468658}

[add] https://crrev.com/9eba37a11e54d4f0808f43db1371dc2e062efb8d/third_party/WebKit/LayoutTests/svg/text/selection-pseudo-resource-invalidation-crash.html
[modify] https://crrev.com/9eba37a11e54d4f0808f43db1371dc2e062efb8d/third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp
[modify] https://crrev.com/9eba37a11e54d4f0808f43db1371dc2e062efb8d/third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.h
[modify] https://crrev.com/9eba37a11e54d4f0808f43db1371dc2e062efb8d/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp
Project Member

Comment 10 by bugdroid1@chromium.org, May 2 2017 

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

commit 9eba37a11e54d4f0808f43db1371dc2e062efb8d
Author: fs <fs@opera.com>
Date: Tue May 02 16:06:31 2017

More targeted resource-switching mechanism for SVG selection painting

The mechanism by which resources are generated for painting using
selection style for SVG text is a bit too heavy-handed, and can end up
invalidating both layout and other things. All that is needed is looking
up any <paint> ('fill' or 'stroke') references and invalidating any
state from the non-selection style.

Use a reduced/tailored version of SVGResourcesCache::ClientStyleChanged
that only recreates/swaps the SVGResources object for the LayoutObject
and wrap that mechanism in a scope object.

BUG= 713024 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2846513002
Cr-Commit-Position: refs/heads/master@{#468658}

[add] https://crrev.com/9eba37a11e54d4f0808f43db1371dc2e062efb8d/third_party/WebKit/LayoutTests/svg/text/selection-pseudo-resource-invalidation-crash.html
[modify] https://crrev.com/9eba37a11e54d4f0808f43db1371dc2e062efb8d/third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.cpp
[modify] https://crrev.com/9eba37a11e54d4f0808f43db1371dc2e062efb8d/third_party/WebKit/Source/core/layout/svg/SVGResourcesCache.h
[modify] https://crrev.com/9eba37a11e54d4f0808f43db1371dc2e062efb8d/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp

Comment 11 by schenney@chromium.org, May 2 2017

Status: Fixed (was: Assigned)
Project Member

Comment 12 by ClusterFuzz, May 3 2017 

ClusterFuzz has detected this issue as fixed in range 468634:468686.

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

Fuzzer: ifratric-browserfuzzer-v3
Job Type: windows_asan_chrome
Platform Id: windows

Crash Type: UNKNOWN READ
Crash Address: 0x00000000
Crash State:
  blink::SVGInlineTextBox::constructTextRun
  blink::SVGInlineTextBox::selectionRectForTextFragment
  blink::SVGInlineTextBox::localSelectionRect
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome&range=457732:457737
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome&range=468634:468686

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


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