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

Issue 854875 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

GrProxyProvider::findProxyByUniqueKey finds incorrect texture proxy

Project Member Reported by ClusterFuzz, Jun 21 2018

Issue description

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

Fuzzer: libFuzzer_paint_op_buffer_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: Abrt
Crash Address: 0x05390000156e
Crash State:
  sk_abort_no_print
  GrProxyProvider::findProxyByUniqueKey
  GrProxyProvider::findProxyByUniqueKey
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=555638:555648

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

Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md for more information.
 
Project Member

Comment 1 by ClusterFuzz, Jun 21 2018

Components: Internals>Skia
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Jun 21 2018

Cc: enne@chromium.org
Labels: ClusterFuzz-Auto-CC
Automatically adding ccs based on OWNERS file / target commit history.

If this is incorrect, please add ClusterFuzz-Wrong label.

Comment 3 by enne@chromium.org, Jun 27 2018

Owner: bsalomon@chromium.org
Status: Assigned (was: Untriaged)
Summary: GrProxyProvider::findProxyByUniqueKey finds incorrect texture proxy (was: Abrt in sk_abort_no_print)
bsalomon: can you triage this clusterfuzz issue? I'm not quite sure that I understand this code enough to know what this could indicate or how serious of an issue this is.

Comment 4 by bsalo...@google.com, Jun 27 2018

Cc: robertphillips@chromium.org csmartdalton@chromium.org
This is an issue in GrClipStackClip. It makes a clip mask in GrClipStackClip::createAlphaClipMask() with origin bottom left and assigns it a key.


Then later GrClipStackClip::createSoftwareClipMask() is invoked, computes the same key, but passes kTopLeft_GrSurfaceOrigin to findProxyByUniqueKey() which asserts since the proxy created by createAlphaClipMask() has a bottom left origin.

I have two questions here:

1) Should createAlphaClipMask() and createSoftwareClipMask() be sharing cache entries at all? They currently use the same function to compute their keys. Should they have their own key domains so they don't collide? (+csmartdalton@)

2) Why does findProxyByUniqueKey() take an origin just to assert on it? There are plenty of other proxy/texture parameters it doesn't do the same thing with (width, height, format, sample count, ...). This seems odd. It's always the case that the caller has to make their keys unique enough within in their domain to guarantee they get a proxy with the expected properties. Having special protections around origin doesn't seem right. (+robertphillips@)

Comment 5 by bsalo...@google.com, Jun 27 2018

Part of the reason I ask about 2) is that if it is legal for these two functions to share masks then they shouldn't have to state the expected origin if they don't know which of the two functions created the mask.

Even if that's not true in this case you can imagine a scenario like this:

proxy path_mask(path p) {
    proxy m = find_in_cache_path(p);
    if (m) {
        return m;
    }
    if (expensive_analysis(p)) {
        // happens to make a mask with TL origin
        m = make_mask_using_approach_a(p);
    } else  {
        // happens to make a mask with BL origin
        m = make_mask_using_approach_b(p);
    }
    put_in_cache(p, m);
    return m;
}

It'd be inconvenient to know the expected origin when searching for the mask and it isn't fundamentally necessary to know it.
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 27 2018

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/5491e823cca01a09435dfbd21ba189d26504baad

commit 5491e823cca01a09435dfbd21ba189d26504baad
Author: Robert Phillips <robertphillips@google.com>
Date: Wed Jun 27 18:42:15 2018

Switch createAlphaClipMask to use kTopLeft

This makes createAlphaClipMask match createSoftwareClipMask

Bug:  854875 
Change-Id: I92a81f5025a19548c6225f39c84b2b2f87c53550
Reviewed-on: https://skia-review.googlesource.com/137888
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>

[modify] https://crrev.com/5491e823cca01a09435dfbd21ba189d26504baad/src/gpu/GrClipStackClip.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 27 2018

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

commit a6af6fbbc5dc492ea915d02223fb4fbf2cd853ae
Author: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Wed Jun 27 21:08:23 2018

Roll src/third_party/skia 059a9ab4bcd0..54d7b314c84a (2 commits)

https://skia.googlesource.com/skia.git/+log/059a9ab4bcd0..54d7b314c84a


git log 059a9ab4bcd0..54d7b314c84a --date=short --no-merges --format='%ad %ae %s'
2018-06-27 mtklein@chromium.org rename Chromecast bots to Clang
2018-06-27 robertphillips@google.com Switch createAlphaClipMask to use kTopLeft


Created with:
  gclient setdep -r src/third_party/skia@54d7b314c84a

The AutoRoll server is located here: https://autoroll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:854875 
TBR=jcgregorio@chromium.org

Change-Id: Idc7a2b571f474490bdb8a1564a4bf13d80a96257
Reviewed-on: https://chromium-review.googlesource.com/1117478
Reviewed-by: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: skia-chromium-autoroll <skia-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#570900}
[modify] https://crrev.com/a6af6fbbc5dc492ea915d02223fb4fbf2cd853ae/DEPS

Project Member

Comment 8 by ClusterFuzz, Jun 28 2018

ClusterFuzz has detected this issue as fixed in range 570894:570900.

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

Fuzzer: libFuzzer_paint_op_buffer_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: Abrt
Crash Address: 0x05390000156e
Crash State:
  sk_abort_no_print
  GrProxyProvider::findProxyByUniqueKey
  GrProxyProvider::findProxyByUniqueKey
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=555638:555648
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=570894:570900

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

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reference.md 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 9 by ClusterFuzz, Jun 28 2018

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Sign in to add a comment