750983 - ICO crash [mozilla::image::Decoder::Write]

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Christoph Diehl [:posidron]]

Attachment: testcase

Comment 1

[Christoph Diehl [:posidron]]

Attachment: callstack

Comment 2

[Alice0775 White]

Browser does not crash in Windows build.
However, the following error message appears in Error console without showing image.

Error: Image corrupt or truncated: https://bugzilla.mozilla.org/attachment.cgi?id=620160
Source file: https://bugzilla.mozilla.org/attachment.cgi?id=620160

FYI,
this starts from the following range.
(central)
Something image displayed:
http://hg.mozilla.org/mozilla-central/rev/e6893e6c883f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111104 Firefox/10.0a1 ID:20111104020439
Error message appears in Error console without showing image:
http://hg.mozilla.org/mozilla-central/rev/5ebd59b5a94a
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111104 Firefox/10.0a1 ID:20111104112939
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e6893e6c883f&tochange=5ebd59b5a94a

(inbound)
Something image displayed:
http://hg.mozilla.org/integration/mozilla-inbound/rev/c535d936df7f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111103 Firefox/10.0a1 ID:20111104035242
Error message appears in Error console without showing image:
http://hg.mozilla.org/integration/mozilla-inbound/rev/dd25b9224c76
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111104 Firefox/10.0a1 ID:20111104070042
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c535d936df7f&tochange=dd25b9224c76

Comment 3

[Scoobidiver (away)]

The error message on Windows is bug 704421, but the crash on Mac is a different issue.

Comment 4

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

Basically we were not carrying over the decoder errors, only the data errors

Comment 5

[Brian R. Bondy [:bbondy]]

The testcase doesn't crash on osx by the way, but I can see from the callstack that the problem is with us not carrying over the decoder error. 

The error message found for the regression range is not a bug but an invalid icon file that we shouldn't handle.  The test case icon doesn't work in IE nor Chrome.

Comment 6

[Christoph Diehl [:posidron]]

The testcase works on osx. It's the platform on which I have tested it.

Comment 7

[Brian R. Bondy [:bbondy]]

I loaded the test case there and could not reproduce, but the problem seems pretty clear from the callstack.  Once this lands maybe you could verify it since I can't reproduce the crash myself.

Comment 8

[Christoph Diehl [:posidron]]

Applied the patch to my trunk build but shows no affect.

Comment 9

[Brian R. Bondy [:bbondy]]

Still crashes after building image? 
Could you attach a new callstack?

Comment 10

[Brian R. Bondy [:bbondy]]

Comment on attachment 620555 [details] [diff] [review]
Patch v1.

Cancelling review for now since it still crashes.

Comment 11

[Christoph Diehl [:posidron]]

The callstack is the same.

Comment 12

[Brian R. Bondy [:bbondy]]

I reproduced this on OSX with a debug build; however...  

I then applied the fix and I could no longer reproduce.  I then popped off the fix and I could reproduce again.  I then pushed the fix and could not reproduce again.  I then just for good measure popped one last time and could reproduce again.

It also would not make any logical sense to me how the error could happen with the fix applied. 

Are you sure that you applied the fix and built properly?

Comment 13

[Christoph Diehl [:posidron]]

Hmm,

$ hg qapplied
4 A bug746891.patch

Applied without errors, compiled without errors (make -sf client.mk build) but still can reproduce it.

Comment 14

[Christoph Diehl [:posidron]]

Ops, wrong patch!

Comment 15

[Christoph Diehl [:posidron]]

Yes, fixed! Sorry for the trouble and confusion.

Comment 16

[Brian R. Bondy [:bbondy]]

no problem, it happens :)

Comment 17

[Joe Drew (not getting mail)]

Comment on attachment 620555 [details] [diff] [review]
Patch v1.

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

lovely

Comment 18

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/4eda2c7b640c

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4eda2c7b640c

Comment 20

[Paul Silaghi, QA [:pauly]]

Not able to reproduce the crash on FF 13.0.1, FF 14.0.1 and Nightly 2012-05-01 on Mac OS X 10.6.8.
Is there something qa can do to verify this fix ?

Comment 21

[Brian R. Bondy [:bbondy]]

I think you have to use a debug build and then load the test case image attached to this bug.  Any platform should be fine.

Comment 22

[Paul Silaghi, QA [:pauly]]

Thanks Brian.
Able to see the crash on 2012-05-01-mozilla-central-debug on Win 7, Ubuntu 12.04 and Mac OS X 10.6.
No crashes on 2012-08-18-mozilla-beta-debug. Verified fixed.