New issue
Advanced search Search tips

Issue 779366 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

CHECK failure: contents.DataMaybeShared() in ArrayBuffer.h

Project Member Reported by ClusterFuzz, Oct 28 2017

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: windows_asan_chrome_no_sandbox
Platform Id: windows

Crash Type: CHECK failure
Crash Address: 
Crash State:
  contents.DataMaybeShared() in ArrayBuffer.h
  blink::DOMArrayBuffer::Create
  blink::Shape::CreateRasterShape
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=480776:480824

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

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: kkaluri@chromium.org
Components: Blink>Paint
Labels: M-63 Test-Predator-Wrong
Owner: schenney@chromium.org
Status: Assigned (was: Untriaged)
As per the   Issue 759398 , assigning this issue to schenney@.

schenney@ -- Could you please look into this issue, kindly reassign if it has nothing to do with your changes.

Thanks.
Project Member

Comment 2 by ClusterFuzz, Oct 31 2017

Components: Blink>Layout
Labels: Test-Predator-AutoComponents
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Doesn't crash on Linux but the size of the SVG image reported is very very weird for what should be an empty SVG, so there seems like a real error here. Trying Windows.
Components: -Blink>Paint Blink>SVG

Comment 5 by f...@opera.com, Oct 31 2017

AFAICT though, the size used for the ImageBuffer, (DOM)ArrayBuffer et.c in CreateRasterShape is not necessarily a size derived from the SVG though (can be the content rect of the or possibly related to the reference box for the shape.) I.e how it's embedded could matter as well.
Right. That turns out to be a red herring. Looks like the CHECK is designed to fail. The ImageBuffer is created as not shareable. Digging deeper.

Comment 7 by f...@opera.com, Oct 31 2017

Maybe it's "just" a matter of ImageBuffer::GetImageData() returning false (and thus not actually allocating any backing.) That would leave the ArrayBufferContents without any data (which would mean it "may" not be shared.)
So here's the deal. There's a very fragile check that verifies that the buffer size will not be larger than the size of a decoded image, using platform numbers for max image size. On Windows that's 1.07374e+09 bytes.

The test case allocates 9.22559e+08 bytes and this works fine in Debug and Release builds. But ASAN crashes presumably because it tries to allocate larger buffers than the strict memory requested, fails and then the null buffer propagates through.

So we could just WontFix this, or we could be more robust to null images.

We're essentially crashing due to out of memory, which can happen in real life, so detecting that as soon as we fail to allocate would probably be the best thing to do. i.e. handle null images better.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 1 2017

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

commit b059a7602b287b08791353f803571408bf7c7a44
Author: Stephen Chenney <schenney@chromium.org>
Date: Wed Nov 01 20:34:06 2017

Handle failed allocations for raster shapes

Code checks for the size when allocating a raster shape
(for CSS shape-outside) but ASAN defeats this check by
allocating more than the exact image size. Be robust to
this case and other out-of-memory situations by handling
failed allocations.

R=fs@opera.com
BUG= 779366 

Change-Id: I11b1f9baaac21d89044dbaa1d5ed6cd188ccc813
Reviewed-on: https://chromium-review.googlesource.com/749320
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Stephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513251}
[add] https://crrev.com/b059a7602b287b08791353f803571408bf7c7a44/third_party/WebKit/LayoutTests/fast/shapes/crash-allocating-very-large-raster-shape-expected.txt
[add] https://crrev.com/b059a7602b287b08791353f803571408bf7c7a44/third_party/WebKit/LayoutTests/fast/shapes/crash-allocating-very-large-raster-shape.html
[modify] https://crrev.com/b059a7602b287b08791353f803571408bf7c7a44/third_party/WebKit/Source/core/layout/shapes/Shape.cpp
[modify] https://crrev.com/b059a7602b287b08791353f803571408bf7c7a44/third_party/WebKit/Source/core/layout/shapes/ShapeOutsideInfo.cpp

Status: Fixed (was: Assigned)
Project Member

Comment 11 by ClusterFuzz, Nov 2 2017

ClusterFuzz has detected this issue as fixed in range 513227:513272.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: windows_asan_chrome_no_sandbox
Platform Id: windows

Crash Type: CHECK failure
Crash Address: 
Crash State:
  contents.DataMaybeShared() in ArrayBuffer.h
  blink::DOMArrayBuffer::Create
  blink::Shape::CreateRasterShape
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=480776:480824
Fixed: https://clusterfuzz.com/revisions?job=windows_asan_chrome_no_sandbox&range=513227:513272

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

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.
Project Member

Comment 12 by ClusterFuzz, Nov 2 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 5452158714773504 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: Test-Predator-Auto-CC
Labels: -Test-Predator-AutoComponents Test-Predator-Auto-Components
Labels: -Test-Predator-Auto-CC
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 9 2017

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

commit 7c78ce6703f35662023778f9ee555ca14aa745e4
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Thu Nov 09 23:21:46 2017

[SPv2] Update flag-specific test expectation

BUG= 708175 , 779366 , 714962 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I0d60874bb3df80d7d4c288c1c1dfae544e125080
Reviewed-on: https://chromium-review.googlesource.com/758975
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515341}
[modify] https://crrev.com/7c78ce6703f35662023778f9ee555ca14aa745e4/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/7c78ce6703f35662023778f9ee555ca14aa745e4/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/7c78ce6703f35662023778f9ee555ca14aa745e4/third_party/WebKit/LayoutTests/TestExpectations

Sign in to add a comment