New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix out of bounds access of clip tasks in rare conditions. #3436

Merged
merged 1 commit into from Dec 19, 2018

Conversation

Projects
None yet
3 participants
@gw3583
Copy link
Collaborator

gw3583 commented Dec 18, 2018

In rare cases, a picture primitive may have a surface (e.g. due
to opacity) on one frame, and then become a pass-through primitive
on the next frame (e.g. opacity is animated to 1).

In these cases, it was possible that the primitive instance for
the picture has a clip task index set from the previous frame
that is not correctly reset. This can cause an out of bounds
access of the scratch buffer clip instances array during
batching.

To ensure this can't happen, reset the clip task index for the
primitive instance to invalid as early as possible (at the same
time the primitive instance visibility is reset).


This change is Reviewable

Fix out of bounds access of clip tasks in rare conditions.
In rare cases, a picture primitive may have a surface (e.g. due
to opacity) on one frame, and then become a pass-through primitive
on the next frame (e.g. opacity is animated to 1).

In these cases, it was possible that the primitive instance for
the picture has a clip task index set from the previous frame
that is not correctly reset. This can cause an out of bounds
access of the scratch buffer clip instances array during
batching.

To ensure this can't happen, reset the clip task index for the
primitive instance to invalid as early as possible (at the same
time the primitive instance visibility is reset).
@gw3583

This comment has been minimized.

@gw3583

This comment has been minimized.

Copy link
Collaborator

gw3583 commented Dec 18, 2018

Try run looks good.

@kvark

This comment has been minimized.

Copy link
Member

kvark commented Dec 19, 2018

We should also make those clip task indices to be generational under debug, so that we can panic on using a stale index.
@bors-servo r+

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 19, 2018

📌 Commit e0e1caa has been approved by kvark

@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 19, 2018

⌛️ Testing commit e0e1caa with merge 6af87e7...

bors-servo added a commit that referenced this pull request Dec 19, 2018

Auto merge of #3436 - gw3583:fix-another-crash, r=kvark
Fix out of bounds access of clip tasks in rare conditions.

In rare cases, a picture primitive may have a surface (e.g. due
to opacity) on one frame, and then become a pass-through primitive
on the next frame (e.g. opacity is animated to 1).

In these cases, it was possible that the primitive instance for
the picture has a clip task index set from the previous frame
that is not correctly reset. This can cause an out of bounds
access of the scratch buffer clip instances array during
batching.

To ensure this can't happen, reset the clip task index for the
primitive instance to invalid as early as possible (at the same
time the primitive instance visibility is reset).

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/3436)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Copy link
Contributor

bors-servo commented Dec 19, 2018

☀️ Test successful - status-appveyor, status-taskcluster
Approved by: kvark
Pushing 6af87e7 to master...

@bors-servo bors-servo merged commit e0e1caa into servo:master Dec 19, 2018

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 19, 2018

Bug 1515241 - Update webrender to commit 6af87e7d8552d461b3744eb39ef0…
…d6c618b5261a (WR PR #3436). r=kats

servo/webrender#3436

Differential Revision: https://phabricator.services.mozilla.com/D14972

--HG--
extra : moz-landing-system : lando

mykmelez pushed a commit to mykmelez/gecko that referenced this pull request Dec 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment