Crash when dragging selection [@ fbFetchPixel_a8r8g8b8][@ fbFetchPixel_x8r8g8b8] [64bit]

RESOLVED FIXED in mozilla1.9beta1

Status

()

P4
critical
RESOLVED FIXED
11 years ago
8 years ago

People

(Reporter: mats, Assigned: stransky)

Tracking

({64bit, crash, regression})

Trunk
mozilla1.9beta1
x86
Linux
64bit, crash, regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
Crash when dragging selection [@ fbFetchPixel_a8r8g8b8]

STEPS TO REPRODUCE
1. click-and-drag to make a selection
2. click on it and drag it around for a bit
3. repeat

BUILDS AND PLATFORMS TESTED
Bug occurs in a local Firefox debug build on Linux x86_64 (Ubuntu 7.04).
Bug does NOT occur in a Firefox nightly build (32-bit) on the same host.
Not sure if it's a 64-bit issue, it could also be that my local build
uses native theme also.
Flags: blocking1.9?
(Reporter)

Comment 1

11 years ago
Created attachment 275216 [details]
stack ("bt" and "bt full")
(Reporter)

Comment 2

11 years ago
Created attachment 275217 [details]
stack + some data

FWIW, it appears as if the gfxContext and surface objects are valid...
(Reporter)

Comment 3

11 years ago
Created attachment 275218 [details]
Test page

Easily reproducible on this content.  I can reproduce it on several other
pages too so it's not specific to this content.
Can anyone reproduce this with 32 bit linux?  I don't have easy access to a 64-bit machine, though I can try with vmware.
Flags: blocking1.9? → blocking1.9+

Comment 5

11 years ago
Seems to be 64bit related. I couldn't reproduce with your testpage, but when dragging a selection containing an image.

in fbFetchTransformed, line 3890 of gfx/cairo/libpixman/src/pixman-compose.c

  b = bits + (y1)*stride;

I'm seeing a negative y1 (-1), which makes b point outside of the buffer.
(Reporter)

Comment 6

11 years ago
Created attachment 275912 [details] [diff] [review]
fix?

It looks like there are some MOD()'s missing -- which clamps the values
to valid boundaries.  We do it in the
"if (pict->common.repeat == PIXMAN_REPEAT_NORMAL)" block, but not in the
else-block:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/cairo/libpixman/src/pixman-compose.c&rev=1.5&root=/cvsroot&mark=3753-3758,3817-3822,3890,3956#3720
Attachment #275912 - Flags: superreview?(vladimir)
Attachment #275912 - Flags: review?(vladimir)
(Reporter)

Comment 7

11 years ago
FWIW, Select All (CTRL+A) on http://www.mozilla.org/projects/minefield/
and then drag that selection is 100% reproducible crash for me.
Comment on attachment 275912 [details] [diff] [review]
fix?

So let's take this to avoid the crash for now, but I'm not sure if this is the right fix.

The case above is PIXMAN_REPEAT_NORMAL, which means just normal repeating.  We hit the else case for PIXMAN_REPEAT_NONE, as well as PIXMAN_REPEAT_PAD and PIXMAN_REPEAT_MIRROR.  Doing MOD for all 3 cases doesn't seem to be correct -- and in fact, it's probably not correct for any of them; e.g. for NONE, we really shouldn't be seeing the wrong coordinates at all.

However, like I said, let's bandaid, and I'll send this upstream for discussion.
Attachment #275912 - Flags: superreview?(vladimir)
Attachment #275912 - Flags: superreview+
Attachment #275912 - Flags: review?(vladimir)
Attachment #275912 - Flags: review+

Comment 9

11 years ago
Looks like my bug 390611 is kinda duplicate of this one.

Adding duplicate on my old bug (opened since 12 days) showing a crash with thunderbird / SM MailNews.

Updated

11 years ago
Duplicate of this bug: 390611
(Reporter)

Comment 11

11 years ago
I added a XXX comment before the added blocks and checked in:
mozilla/gfx/cairo/libpixman/src/pixman-compose.c 	1.6 

-> FIXED
Assignee: nobody → mats.palmgren
Whiteboard: [wallpaper]
Target Milestone: --- → mozilla1.9 M8
(Reporter)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
This (or bug 391243) has caused bug 392170.
(Reporter)

Updated

11 years ago
Depends on: 392214
(Reporter)

Comment 13

11 years ago
I backed out this since it caused crashes on Windows (bug 392214).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 14

11 years ago
Is there any hope to fix again this bug ? Because seeing crash on a single drag and drop is very annoying and simply kills usability. Too bad it crashes on the worse OS family.

Sorry for the spam.
(Reporter)

Comment 15

11 years ago
Created attachment 278524 [details]
Trace log

FWIW, the transform sometimes makes the 'v.vector' values become negative.
I also saw some cases where they were still positive after the transform
but less than v.vector[2] / 2 so they became negative here instead:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/cairo/libpixman/src/pixman-compose.c&rev=1.7&root=/cvsroot&mark=3715-3716#3700
(Reporter)

Updated

11 years ago
Attachment #275912 - Attachment is obsolete: true
(Reporter)

Comment 16

11 years ago
Vlad, did you get any response from the Cairo folks on this?
Assignee: mats.palmgren → nobody
Status: REOPENED → NEW
Whiteboard: [wallpaper]

Comment 17

11 years ago
It seems it kills also drag and drop in places organizer.

It will be great to see this fixed for Gran Paradiso M9, don't you think so ?

Comment 18

11 years ago
It seems it kills also printing in linux.

I did a debug build, and try to print gmail.com start page.

Firefox hangs, and when I typed bt in gdb, I got this :

#0  0x00002b773a1cbb03 in fbFetchPixel_a8r8g8b8 (image=0x1a95020, 
    bits=0x401a94d80, offset=0, indexed=0x0) at pixman-compose.c:705
Cannot access memory at address 0x7fff71b0c138

Updated

11 years ago
Priority: -- → P4
Summary: Crash when dragging selection [@ fbFetchPixel_a8r8g8b8] → Crash when dragging selection [@ fbFetchPixel_a8r8g8b8] [64bit]
Target Milestone: mozilla1.9 M8 → ---

Updated

11 years ago
Keywords: 64bit
(Reporter)

Updated

11 years ago
Duplicate of this bug: 400606
(Reporter)

Updated

11 years ago
Summary: Crash when dragging selection [@ fbFetchPixel_a8r8g8b8] [64bit] → Crash when dragging selection [@ fbFetchPixel_a8r8g8b8][@ fbFetchPixel_x8r8g8b8] [64bit]
(Assignee)

Comment 20

11 years ago
Created attachment 286274 [details] [diff] [review]
proposed patch

This bug is caused by different size of pointer and integer on 64bit arches, when -1 in 32bit integer != -1 in 64bit integer. Looks partially fixed but there're still some unfixed places...
Attachment #286274 - Flags: superreview?
Attachment #286274 - Flags: review?

Comment 21

11 years ago
I tested this patch with both firefox and thunderbird source code. And it was wonderful to see that I can do some drag and drop again. For the last two months or so, a single drap and drop meant immediate crash.

I'm using Ubuntu 7.10 AMD64.

Updated

11 years ago
Attachment #286274 - Flags: review? → review?(vladimir)

Updated

11 years ago
Target Milestone: --- → mozilla1.9 M10
Comment on attachment 286274 [details] [diff] [review]
proposed patch

This looks fine, though it needs to be upstreamed to pixman as well.  Do you have an idea of what/where the 'missing places' are, or are you just assuming that there would be some?
Attachment #286274 - Flags: review?(vladimir) → review+
(Assignee)

Comment 23

11 years ago
The missing places are covered by the attached patch. 

Iteration via. multiplication ((y2)*stride) is already used in the pixman code but not everywhere...

Comment on attachment 286274 [details] [diff] [review]
proposed patch

Looks fine then; let's see if we can get this in for M9, I'll check it in if we can.
Attachment #286274 - Flags: superreview?
Attachment #286274 - Flags: superreview+
Attachment #286274 - Flags: approvalM9?
Attachment #286274 - Flags: approval1.9+
Assignee: nobody → stransky
Comment on attachment 286274 [details] [diff] [review]
proposed patch

a=endgame drivers for M9
Attachment #286274 - Flags: approvalM9? → approvalM9+

Comment 26

11 years ago
checked in.
Status: NEW → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9 M10 → mozilla1.9 M9
Does this need to be in a list of local patches to cairo so it doesn't get lost when we upgrade?
No need; I'm going to do a sync post-M9 and I usually do a diff first from the last time I did a cairo update -- so I see all that went in.
(Reporter)

Updated

11 years ago
Blocks: 399307
Crash Signature: [@ fbFetchPixel_a8r8g8b8] [@ fbFetchPixel_x8r8g8b8]
You need to log in before you can comment on or make changes to this bug.