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

Issue 846754 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

SkASSERT shouldn't crash, but operate in logging mode with dcheck_is_configurable=true

Project Member Reported by infe...@chromium.org, May 25 2018

Issue description

See 0% on skia_image_filter_proto_converter.cc in https://chromium-coverage.appspot.com/reports/561573/linux/chromium/src/testing/libfuzzer/proto/report.html (before - 92% see https://chromium-coverage.appspot.com/reports/561047/linux/chromium/src/testing/libfuzzer/proto/report.html)

This started happening when we enabled dcheck_is_configurable. Previously it worked fine.

This can be reproduced using 
gn gen //out/coverage --args='use_goma=true goma_dir="/build/goma" is_clang=true use_libfuzzer=true use_clang_coverage=true is_component_build=false pdf_enable_xfa=true proprietary_codecs=true ffmpeg_branding="ChromeOS" strip_absolute_paths_from_debug_symbols=true is_debug=false dcheck_is_configurable=true'
ninja -C out/coverage skia_image_filter_converter

Result: Target should be built, and it isn't

At first, i thought this is related to 
https://cs.chromium.org/chromium/src/testing/libfuzzer/proto/BUILD.gn?rcl=560f859dd3973c36a4e4633dc30560c51600f3fe&l=59
but no, since you should be still able to build using ninja.

Jonathan, Kevin - any idea why ?
 
I can't reproduce the issue you saw with building right now. 
I noticed that the file defining the target itself is only partially covered https://chromium-coverage.appspot.com/reports/561573/linux/chromium/src/testing/libfuzzer/fuzzers/skia_image_filter_proto_fuzzer.cc.html 

The same weirdness happens in one of the other proto fuzzers: https://chromium-coverage.appspot.com/reports/561573/linux/chromium/src/testing/libfuzzer/fuzzers/javascript_parser_proto_fuzzer.cc.html'

I think this is an LPM issue unrelated to skia. 
Cc: -reed@chromium.org -kjlubick@chromium.org
Removing skia folks since this doesn't look like a skia issue
Cc: metzman@chromium.org
Owner: ----
Summary: Skia fuzz target crashes when dcheck_is_configurable=true (was: skia_image_filter_proto_converter is not built when dcheck_is_configurable=true)
Sorry i had the wrong target name, it should have been
 skia_image_filter_proto_fuzzer

So, the issue seems to be assertion are becoming crashes. dcheck_is_configurable should be causing warnings. e.g. see
https://chromium-coverage.appspot.com/reports/561573/linux/metadata/skia_image_filter_proto_fuzzer.log
https://chromium-coverage.appspot.com/reports/561573/linux/metadata/skia_path_fuzzer.log
https://chromium-coverage.appspot.com/reports/561573/linux/metadata/skia_pathop_fuzzer.log
Cc: kjlubick@chromium.org
Owner: reed@chromium.org
Summary: SkASSERT shouldn't crash, but operate in logging mode with dcheck_is_configurable=true (was: Skia fuzz target crashes when dcheck_is_configurable=true)
Chatted with Jonathan offline, there are 2 bugs.
Lets use this one to track c#3. This is a skia issue. Skia needs to support dcheck in logging mode (purpose of dcheck_is_configurable). this helps to get code coverage reports to work. Note that code coverage is not captured if we get a crash, that is why we operate with dcheck_is_configurable. 
Labels: Coverage-v2-Blocker
Cc: reed@google.com
Cc: -reed@google.com
Owner: reed@google.com

Comment 9 by reed@google.com, May 25 2018

Cc: mtkl...@google.com
SkASSERT is used in skia sometimes to mean we-can't-continue-at-all, and sometimes just to affirm some internal consistency check. In general, we probably can't always "continues" without crash soon afterwards. Is this what you are asking for?
In Release mode, we disable SkAssert and things work with "continue". Think of this as like code coverage working in Release mode just wants to know it covered the line with SkAssert (and not show red), but it should not crash. 
Owner: reed@chromium.org

Comment 12 by reed@chromium.org, Jan 16 (6 days ago)

Owner: reed@google.com

Sign in to add a comment