1746062 - test case for bug 1742382 still crashes the browser

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Frederik Braun [:freddy]]

Attachment: asan.log

When looking at the test case for bug 1742382 (during our bounty meeting), the linked 1MB test case still crashed Firefox Nightly on Linux.

Looks like a rust panic (causing MOZ_CRASH) due to unwrapping a None, but not in the software backend for webrender. Lee, is this bad?

Dan said it also crashed the browser for him on Mac OS, so he'll probably attach a log file as well.

Comment 1

[Daniel Veditz [:dveditz]]

Rust panic on Mac, too: bp-3ae99a01-4ee0-4f6f-b446-c630b0211214

Comment 2

[Lee Salzman [:lsalzman]]

The rust panic does not represent a security issue nor what occurred in bug 1742383.

Maybe Glenn should take a look to see what's going on.

Comment 3

[Darkspirit]

Test case is not public.

Comment 4

[Glenn Watson [:gw]]

Attachment: Bug 1746062 - Support > 2^16 clip masks

In some extreme test cases, we have > 65535 clip masks being
created. Long term, we might want to restrict the page content
since we can't really render hundreds of thousands of clip masks
for performance reasons anyway. For now, just use a u32 for the
clip mask index so we can handle these cases a bit better.

Comment 6

[Frederik Braun [:freddy]]

Aren't we just kicking the can down the road into a memory/resource exhaustion problem at the OS level? It looks like this will still lead to some other crashy or instable situation. If nobody's stopping the test case from exceeding u16 why should u32 be an improvement? Can we find an operation in there where we can cancel/throw and do that instead?

Comment 7

[Glenn Watson [:gw]]

It is kicking the can down the road (I alluded to this in the commit comment), you're right.

The benefit, I think, of landing this patch now is that we'll be able to see from crash-stats if this crash completely disappears, or if there is also some other bug that can cause us to incorrectly index into the clip instances array.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/41bafe86e91c
Support > 2^16 clip masks r=gfx-reviewers,lsalzman

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/41bafe86e91c

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Hi Lee, I'm not sure if this is worth backporting to Beta/ESR or not. Seems like the patch would be low-risk and give parity with the branches receiving the original fix, but it also feels pretty edge case and unlikely to bite us in the wild. What are your thoughts?