526976 - SIGFPE [@ _moz_cairo_format_stride_for_width] on BGR frame buffer

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Ginn Chen]

Step:
Try to browse some sites.
Try to switch between Firefox and terminal.

This bug is reproducible with DISPLAY on Sun Ray client, XVR-600, XVR-1200.
xdpyinfo shows red, green, blue masks: 0xff, 0xff00, 0xff0000

It doesn't matter which machine Firefox is running on.
You can use "ssh -X" to start Firefox from another Solaris machine.

I think it's not reproducible with Linux, because gdb treats the following line differently.
#define CAIRO_FORMAT_VALID(format) ((format) <= CAIRO_FORMAT_A1)

For Sun Studio, if I change it to
#define CAIRO_FORMAT_VALID(format) ((unsigned int)(format) <= CAIRO_FORMAT_A1)
The crash is gone.

This bug is not reproducible with Firefox 3.5.x or system cairo library (1.8.8).
The CAIRO_FORMAT_VALID macro are same.
So there must be another change triggered this bug.

For current trunk, I didn't recreate the crash, but all the fonts are not readable with BGR fb, it must be another regression.

Comment 1

[Ginn Chen]

Filed Bug 526977 for the fonts issue on trunk.

Comment 2

[Ginn Chen]

The crash is also reproducible with mozilla-central.

Comment 3

[Ginn Chen]

It is triggered by Bug 484076.
http://hg.mozilla.org/mozilla-central/rev/1effb72d30cf

Filed https://bugs.freedesktop.org/show_bug.cgi?id=24995 for CAIRO_FORMAT_VALID.

Comment 4

[Ginn Chen]

I didn't recreate this bug with pixman and cairo outside of firefox. Weird.
I tried the rev in Bug 484076 comment #2 and HEAD.

Comment 5

[Ginn Chen]

Apparently the trigger is wrap-source_image.patch.
I should notice that earlier.

This bug is fixed if we fix CAIRO_FORMAT_VALID.
But it looks like
struct acquire_source_image_data *data
is leaked if cairo_image_surface_create_for_data() or _cairo_user_date_array_set_data() fails.

Jeff, am I right?

Comment 6

[Ginn Chen]

Attachment: stack

Comment 7

[Jeff Muizelaar [:jrmuizel]]

The format passed to cairo_image_surface_create_for_data() should always be vaild. We need to figure out why it's not valid in this case.

Comment 8

[Ginn Chen]

image->pixman_format is PIXMAN_x8b8g8r8
There's no valid cairo_format for it.

Perhaps we should use _cairo_image_surface_create_with_pixman_image() ?

Although we don't expect these functions fail, we still need to free(data) before return.

Comment 9

[Ginn Chen]

Attachment: use _cairo_image_surface_create_for_pixman_image

use _cairo_image_surface_create_for_pixman_image, free(data) on exceptions.
Also changed some coding styles to match other cairo code.

Comment 10

[Ginn Chen]

http://hg.mozilla.org/mozilla-central/rev/cd3c2f16360c

Comment 11

[Ginn Chen]

Backed out due to crashes on Mac and Windows

http://hg.mozilla.org/mozilla-central/rev/63690d0994c3

Comment 13

[Ginn Chen]

Attachment: use _cairo_image_surface_create_with_pixman_format

_cairo_image_surface_create_with_pixman_format() is more close to cairo_image_surface_for_data(),
and it bypasses pixman_format <-> cairo format.

Try builds pass all tests on Mac OS X. There're a few yellow on Linux and Win, but I don't think it's related.
Passes all tests on Solaris on my machine.

Comment 14

[Ginn Chen]

http://hg.mozilla.org/mozilla-central/rev/b81b287ca111