1364691 - Crash in SkScan::AAAFillPath

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-ab08dcde-ed7d-4baa-9de6-864bd0170513.
=============================================================

There are 3 crashes (from the same installation) in nightly 55 with buildid 20170512030203. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1340627.

[1] https://hg.mozilla.org/mozilla-central/rev?node=af6f19870b2a00759ac1d83dedc3db57213abfee

Comment 2

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Hi Vincent,
Are you able to reproduce the crash with the test https://bugzilla.mozilla.org/attachment.cgi?id=8867562 ?

Comment 3

[Vincent Liu[:vliu]]

(In reply to Jerry Shih[:jerry] (UTC+8) from comment #1)
> Hi Vincent,
> Are you able to reproduce the crash with the test
> https://bugzilla.mozilla.org/attachment.cgi?id=8867562 ?

The crash I hit showed below. But I am not sure I see the same compared with bug reported.

/Volumes/firefoxos/gecko-dev/gfx/skia/skia/src/core/SkScan_AAAPath.cpp:1675: fatal error: "assert(count >= 2)"
Abort from sk_abort
Hit MOZ_CRASH() at /Volumes/firefoxos/gecko-dev/memory/mozalloc/mozalloc_abort.cpp:33

Comment 4

[Lee Salzman [:lsalzman]]

So the rough source of the regression is that Skia added analytic AA code for their path rendering. As a big sledgehammer fix for this, we can define SK_NO_ANALYTIC_AA in the user config. But I fear if we do that, without finding the source of the problem, that this issue will go unfixed in Skia indefinitely. So if someone is both able to reproduce the crash and track it down, that would probably be the best outcome.

Comment 5

[Nicholas Nethercote [inactive]]

This is the #2 Mac topcrash in Nightly 20170519030205.

Comment 6

[Marcia Knous [:marcia]]

Tracking 55+ for this crash since it is #4 top overall Mac crash.

Comment 7

[Marcia Knous [:marcia]]

Some URLs:

https://manuals.co/desktop/images/infobox/303.jpg
http://codepen.io/search/pens?q=dom&limit=all&type=type-pens
http://doctorwhoworld.net/Images/timelords-et.jpg
https://cloud.digitalocean.com/droplets?i=2815ea

Comment 8

[Lee Salzman [:lsalzman]]

(In reply to Marcia Knous [:marcia - use ni] from comment #7)
> Some URLs:
> 
> https://manuals.co/desktop/images/infobox/303.jpg
> http://codepen.io/search/pens?q=dom&limit=all&type=type-pens
> http://doctorwhoworld.net/Images/timelords-et.jpg
> https://cloud.digitalocean.com/droplets?i=2815ea

I tried viewing all of those links and messing around in the pages, and I was not able to reproduce the crash at all.

Comment 9

[Lee Salzman [:lsalzman]]

Upstream bug report: https://bugs.chromium.org/p/skia/issues/detail?id=6684

Hoping Skia people might have some insight. But without a reliable and reproducible testcase, I am not very hopeful on this one. Random sites that may only sometimes trigger it makes it especially hard for them to investigate since they generally do not get involved with building Firefox to inspect Skia bugs. :/

... So it would be really really helpful if we could start narrowing this down to a minimal testcase somehow.

Comment 10

[Lee Salzman [:lsalzman]]

Attachment: add assertions to help figure out why Skia's AAA is crashing

Per the recommendations of the Skia developer here: https://bugs.chromium.org/p/skia/issues/detail?id=6684#c3

Adds some assertions to check if either both conditions fail, or one or the other, plus a plea for users who trigger the crash to report to us where it occurred to help us out.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/44fbcc3d084a
add assertions to help figure out why Skia's AAA is crashing. r=jrmuizel

Comment 12

[Marcia Knous [:marcia]]

I did crash in this stack once today - https://crash-stats.mozilla.com/report/index/f8883e85-a33d-401c-8c09-f1ffb0170525 - but I don't know which site triggered the crash. I may have had the sites in Comment 7 loaded in various tabs.

Comment 13

[Tyson Smith [:tsmith] (Back Aug 12)]

Attachment: test_case.html

Adding test case from bug 1364778

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/44fbcc3d084a

Comment 15

[Lee Salzman [:lsalzman]]

Attachment: don't use walk_convex_edges in Skia path rendering unless edge count is at least 2

For certain paths that are subsequently clipped, SkEdgeBuilder returns not enough edges for the convex path rendering code to actually work, which leads to the crash.

Skia upstream is working on a more involved fix for this here: https://skia-review.googlesource.com/c/17983/

I've distilled that fix down to the simplest-fix-that-could-possibly-work for our purposes, which is just to punt to the non-convex path when there are not >= 2 edges. Their patch also does this, but they're doing other drive-by cleanups in it we don't need for now.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4787ff70ae13
don't use walk_convex_edges in Skia path rendering unless edge count is at least 2. r=mchang

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/4787ff70ae13