nsCanvasRenderingContext2D::GetMozCurrentTransformInverse crash with large canvas

RESOLVED FIXED in mozilla16

Status

()

Core
Canvas: 2D
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: cjones)

Tracking

(Blocks: 1 bug, {crash, testcase})

Trunk
mozilla16
crash, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 616369 [details]
testcase (crashes Firefox when loaded)
(Reporter)

Comment 1

5 years ago
Created attachment 616370 [details]
stack trace
(Reporter)

Comment 2

5 years ago
Opt: bp-bdbe2955-d9ce-487c-a72e-d0a0a2120419
Needs moar EnsureCurrentSurface.

phone

Comment 4

5 years ago
On Windows 7: bp-72e70eb8-0995-401d-b377-406672120419
Crash Signature: [@ gfxContext::CurrentMatrix ] [@ nsCanvasRenderingContext2D::GetMozCurrentTransformInverse ] → [@ gfxContext::CurrentMatrix ] [@ nsCanvasRenderingContext2D::GetMozCurrentTransformInverse ] [@ gfxContext::CurrentMatrix()]
OS: Linux → All
Hardware: x86_64 → All
Created attachment 616774 [details] [diff] [review]
Check for context allocation failure before returning inverse transform

Thanks Jesse!
Assignee: nobody → jones.chris.g
Attachment #616774 - Flags: review?(joe)
Comment on attachment 616774 [details] [diff] [review]
Check for context allocation failure before returning inverse transform

Review of attachment 616774 [details] [diff] [review]:
-----------------------------------------------------------------

This needs to be fixed in nsCanvasRenderingContext2DAzure as well - otherwise we'll a) have this bug on some systems but not others and b) make crashtest go orange.
Attachment #616774 - Flags: review?(joe) → review-
Well, OK, but in the port to azure canvas many/most of the EnsureCurrentSurface()-style checks were lost.  This is putting ones finger in dike.  Is there a bug on file for restoring them?  Are we fuzzing azure canvas?
(Reporter)

Comment 8

5 years ago
We are fuzzing azure canvas, but only on the (OS version, hardware) combinations in the build pool.  The fuzzer randomizes gfx.canvas.azure.enabled so both azure and non-azure canvas are tested on as many platforms as possible.
OK.  There are many azure-canvas interfaces that don't check for a valid surface whereas the same cairo-canvas interface does.  (I assumed the checks weren't needed for azure-canvas.)  Apparently we have a dearth of tests for those cases, and perhaps our fuzzer is getting unlucky?

Comment 10

5 years ago
fwiw, crash automation reproduced this on all three branches: Beta/12, Aurora/13, Nightly/14 and all 3 platforms.
Created attachment 631615 [details] [diff] [review]
Check for context allocation failure before returning inverse transform, v2

Was cleaning mq house and came back across this.

This version puts a few more fingers in the azure context dike.
Attachment #616774 - Attachment is obsolete: true
Attachment #631615 - Flags: review?(joe)
Comment on attachment 631615 [details] [diff] [review]
Check for context allocation failure before returning inverse transform, v2

Review of attachment 631615 [details] [diff] [review]:
-----------------------------------------------------------------

Well, not azure content so much as azure canvas, but hooray. And thanks for fixing both implementations!
Attachment #631615 - Flags: review?(joe) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/a07fd72eb33d
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/a07fd72eb33d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.