81207 - [MLK] Leaking gs->transparent_pixel / trashing memory

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect, P1)

Description

[Patrick C. Beard]

The reason for the leak is that the code that deallocates is #if 0'd out on line 
553 of mozilla/modules/libpr0n/decoders/gif/GIF2.cpp. Simply removing the #if 0 
though doesn't fix the whole problem as there is a later bogus free occuring that 
will likely crash the browser. The Boehm GC Leak Detectorª asserts when you pass 
it a bogus pointer to free. The real problem that blakeross was masking happens 
in gif_clear_screen(), where the gs->transparent_pixel is getting freed, and then 
written over with garbage from an unitialized variable. Enclosing a patch which 
fixes the leak and the problem in gif_clear_screen().

This all begs the question, why is gs->transparent_pixel dynamically allocated at 
all? It only takes up 4-bytes, and so should just be a nested member of 
gif_struct.

Comment 1

[Patrick C. Beard]

Attachment: Fixes memory leak and bogus free.

Comment 2

[Patrick C. Beard]

Reassigning to ImageLib.

Comment 3

[Patrick C. Beard]

This is a memory trashing bug, in addition to al leak, because gif_clear_screen() 
writes over a good pointer with a random value. I think this bug is a show 
stopper.

Comment 4

[Stuart Parmenter]

over to saari... this is a dup of another bug of his, but leaving open since 
this talks about thrashing

Comment 5

[Chris Hiner]

Related to 78796?

Comment 6

[saari (gone)]

r=saari. This transparent pixel stuff is cruft, and should be static as beard
said, along with other dynamic allocations in the decoders.

Comment 7

[Bruce Mitchener]

Noticed this under Purify...

Comment 8

[saari (gone)]

*** Bug 77961 has been marked as a duplicate of this bug. ***

Comment 9

[Peter Trudelle]

->arik

Comment 10

[arik devens]

i don't understand, is the patch good? does it just need an sr=? is there still
a bug here?

Comment 11

[David Hyatt]

sr=hyatt

Comment 12

[arik devens]

done.

Comment 13

[Terri Preston]

Verified fix checked into lxr.mozilla.org