1500902 - Crash in InvalidArrayIndex_CRASH | nsTArray_Impl<T>::RemoveElementsAt | nsDisplayMasksAndClipPaths::PaintWithContentsPaintCallback

Status: RESOLVED WONTFIX

(Core :: Graphics: WebRender, defect, P3)

Description

[Gabriele Svelto [:gsvelto]]

This bug was filed from the Socorro interface and is
report bp-1907a1ff-1b31-444b-ab59-9e0260181015.
=============================================================

Top 10 frames of crashing thread:

0 mozglue.dll MOZ_CrashPrintf mfbt/Assertions.cpp:67
1 xul.dll InvalidArrayIndex_CRASH xpcom/ds/nsTArray.cpp:26
2 xul.dll nsTArray_Impl<gfxContext::AzureState::PushedClip, nsTArrayInfallibleAllocator>::RemoveElementsAt xpcom/ds/nsTArray.h:2400
3 xul.dll nsDisplayMasksAndClipPaths::PaintWithContentsPaintCallback layout/painting/nsDisplayList.cpp:9961
4 xul.dll void mozilla::layers::Grouper::PaintContainerItem gfx/layers/wr/WebRenderCommandBuilder.cpp:918
5 xul.dll void mozilla::layers::DIGroup::PaintItemRange gfx/layers/wr/WebRenderCommandBuilder.cpp:774
6 xul.dll void mozilla::layers::Grouper::PaintContainerItem gfx/layers/wr/WebRenderCommandBuilder.cpp:946
7 xul.dll void mozilla::layers::DIGroup::PaintItemRange gfx/layers/wr/WebRenderCommandBuilder.cpp:774
8 xul.dll void mozilla::layers::DIGroup::EndGroup gfx/layers/wr/WebRenderCommandBuilder.cpp:659
9 xul.dll void mozilla::layers::Grouper::ConstructGroups gfx/layers/wr/WebRenderCommandBuilder.cpp:1083

=============================================================

Comment 1

[Andrew McCreight [:mccr8]]

Based on the release assert, somebody is passing -1 to RemoveElementsAt. My guess is that an empty array is getting RemoveLastElement() called on it here in gfxContext::PopClip():
  CurrentState().pushedClips.RemoveLastElement();

Comment 2

[Andrew McCreight [:mccr8]]

The first instance of this crash is in the 20181015100128 build, but it doesn't seem to happen every build, so it is hard to say when the code might have changed.

It looks like Jeff added the call to PopClip in bug 1447880. Any ideas, Jeff? I have no idea where the bug might be coming from.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

PopClip from line: https://searchfox.org/mozilla-central/rev/fcfb479e6ff63aea017d063faa17877ff750b4e5/layout/painting/nsDisplayList.cpp#9961

pairs with https://searchfox.org/mozilla-central/rev/fcfb479e6ff63aea017d063faa17877ff750b4e5/layout/painting/nsDisplayList.cpp#9942


These not matching up suggests an non match deeper inside perhaps?

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Doesn't happen often enough to block.

Comment 5

[Mayank Bansal]

I got this crash by opening the testcase from bug 1513634 . 
https://crash-stats.mozilla.com/report/index/90ecd29f-0f37-48c0-ad9b-4bf7e0181212#tab-details

Comment 6

[BugBot [:suhaib / :marco]]

Closing because no crashes reported for 12 weeks.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

I think closing crashreport bugs as WONTFIX is misleading. "WONTFIX" generally means that "we won't accept a patch even if one is submitted" which is not the case here. INCOMPLETE would a better resolution, no?

Comment 8

[Sylvestre Ledru [:Sylvestre]]

TBH, I don't care that much about the resolution. What matters to me if the fact that it is closed.

If you really care about that, we can change it. This is just a one liner.

Comment 9

[Calixte Denizet (:calixte)]

WONTFIX is not so bad here since we don't have any crashes and then we'll never fix it, probably the crash has been fixed in an other bug.
And if the crash come back then it'll probably be a new issue.
Anyway, as :sylvestre said we don't care so much about that so if you really want to change the resolution, please ping me (irc or slack).

Comment 10

[Kartikaya Gupta (email:[e-mail])]

(In reply to Sylvestre Ledru [:sylvestre] from comment #8)

TBH, I don't care that much about the resolution. What matters to me if the fact that it is closed.

I thought one of the goals of this automated bugzilla-mashing was to normalize the state of bugs and make it more amenable to machine learning and data analysis and so on. If we don't use consistent semantics for the different resolution, that works against this goal. In which case, what's the point of doing this in the first place?

If you really care about that, we can change it. This is just a one liner.

I do care about it, yes.

(In reply to Calixte Denizet (:calixte) from comment #9)

WONTFIX is not so bad here since we don't have any crashes and then we'll never fix it, probably the crash has been fixed in an other bug.

Then WORKSFORME is more appropriate. WONTFIX explicitly means "patches not welcome" which is not the case here. I actually suggested INCOMPLETE for this one because we know this is still an issue and even had a reproducer at one point, see bug 1513634, but the crash rate in the wild is too low. As a general rule though I'd be fine with WORKSFORME as a resolution.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

What matters here is closing inactive/useless bugs. The state used to close it isn't used...

Comment 12

[Kartikaya Gupta (email:[e-mail])]

(In reply to Sylvestre Ledru [:sylvestre] from comment #11)

What matters here is closing inactive/useless bugs. The state used to close it isn't used...

Maybe not today. Then someday somebody will come up for a use for it and we'll have to do this all over again. Why not just do it right the first time?

Comment 13

[Calixte Denizet (:calixte)]

WORKSFORME works for me too.
Anyway, maybe we should at least move this crash signature in bug 1513634 (if it's the same crash of course).
The bot doesn't close bugs with no crashes and with a testcase.

Comment 14

[Kartikaya Gupta (email:[e-mail])]

(In reply to Sylvestre Ledru [:sylvestre] from comment #11)

The state used to close it isn't used...

And actually this is just plain false. You mean to say it's not used by by you. Other people (myself included) do search for bugs based on specific resolution, and expect the resolution (when it shows up in search listings, etc) to accurately reflect the state of the bug, instead of having to read the entire bug to re-evaluate the state.