Crash in skia ccpr due to GrCCPathCache::find

28 views
Skip to first unread message

pr0p

unread,
May 23, 2022, 1:12:57 PMMay 23
to skia-discuss

Hi,

Issue:

We are currently using the m83 version of skia, I know it is old, and we plan to upgrade soon.

However, in m83, we got a peculiar crash with the following call stack; I'll be attaching the complete call stack:

sk_abort_no_print() SkMemory_malloc.cpp:41

GrCCPerFlushResources::recordStencilResolveInstance(SkIRect const&, SkIPoint const&, GrFillRule)::$_13::operator()() const GrCCPerFlushResources.cpp:487

GrCCPerFlushResources::recordStencilResolveInstance(SkIRect const&, SkIPoint const&, GrFillRule) GrCCPerFlushResources.cpp:487

GrCCPerFlushResources::renderShapeInAtlas(SkIRect const&, SkMatrix const&, GrShape const&, float, GrOctoBounds*, SkIRect*, SkIPoint*) GrCCPerFlushResources.cpp:429

GrCCDrawPathsOp::SingleDraw::setupResources(GrCCPathCache*, GrOnFlushResourceProvider*, GrCCPerFlushResources*, GrCCDrawPathsOp::DoCopiesToA8Coverage, GrCCDrawPathsOp*) GrCCDrawPathsOp.cpp:386

GrCCDrawPathsOp::setupResources(GrCCPathCache*, GrOnFlushResourceProvider*, GrCCPerFlushResources*, GrCCDrawPathsOp::DoCopiesToA8Coverage) GrCCDrawPathsOp.cpp:325


Possible Explanation:

So, what was essentially happening was during GrCoverageCountingPathRenderer::preFlush, while getting essential GrCCPerFlushResourceSpecs during GrCCDrawPathsOp::accountForOwnPaths, two of the draws have the same shape and essentially the same key. 

The first draw's shape entry is found in the hashtable of the path cache with the correct transform, so it is accounted for in GrCCPerFlushResourceSpecs as a copied path, but

GrCCPathCache::OnFlushEntryRef GrCCPathCache::find(..) {

....

                this->evict(*fScratchKey);

                entry = nullptr;

....

}

Which essentially destroys the cached atlas the first draw's fCacheEntry is still referring to.

This is fine; however, as skia checks for cached entry every time there is a need to use it, what is not acceptable is that the first draw that should be considered now as rendered path will still be treated as a copied path.

Hence, while setting up resources for the draws, we allocate less for the rendered paths, and the following error is hit.


Possible fixes on top of my mind:

I wanted to know what could be a fix for this - somehow include the transform information in the inherited key or recalculate GrCCPerFlushResourceSpecs until there is no change in the path cache entry or any better one?

Everything works fine if I disable caching the path.

I found out that removing the transform condition works perfectly fine; however, from one of the above fixes, I can see that some shapes were missing.


Other similar findings:

I also found 897510 - Heap-use-after-free in GrCCPathCache::find - chromium, which seems to be related, but it seems to be fixed in the m72 version.


Thoughts:

Any help regarding this and why this is happening will be helpful.

Also, I know that ccpr has been deprecated in the recent versions of skia. I wanted to know if upgrading skia would potentially fix this issue and wanted thoughts on it.

Thanks


Full Crash Log

pr0p

unread,
May 29, 2022, 6:20:46 PMMay 29
to skia-discuss
Any updates on this?

Brian Salomon

unread,
May 31, 2022, 9:15:19 AMMay 31
to skia-d...@googlegroups.com
Hi,

Sadly I don't think there is any knowledge left about this. The original author of CCPR is no longer working on Skia and I don't think anyone else has enough familiarity to assist without having to relearn it. CCPR was integrated in a quite complicated manner and wasn't much used for quite a while before it was deleted so I'm not too surprised that you found an issue.

Brian

--
You received this message because you are subscribed to the Google Groups "skia-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to skia-discuss...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/skia-discuss/8e508fa1-155f-4cc7-a4e7-c78f02c6bba9n%40googlegroups.com.


--

Brian Salomon | Office Hours: go/bsalomon-office | bsal...@google.com

pr0p

unread,
Jun 1, 2022, 1:54:39 AMJun 1
to skia-discuss
Thanks Brian,

So from this, I get that you are highly recommending us to upgrade skia to a later version so that we don't face such issues, right?

pr0p

unread,
Jun 1, 2022, 3:14:31 AMJun 1
to skia-discuss
Also, to which version of skia you would recommend upgrading?

Thanks

Brian Salomon

unread,
Jun 1, 2022, 9:20:11 AMJun 1
to skia-d...@googlegroups.com
HI,

Yes, if you found a similar bug in a recent milestone release of Skia there is a very good chance we'd be able to address it. In general we recommend using the latest branch for Chrome, which is currently m103: https://skia.googlesource.com/skia/+/refs/heads/chrome/m103

Brian

pr0p

unread,
Jun 2, 2022, 11:44:10 AMJun 2
to skia-discuss
Hi Brian,

Cool, we'll be upgrading skia to the latest.

Thanks a lot

Reply all
Reply to author
Forward
0 new messages