391184 - Firefox crashed [@ _get_bitmap_surface] div zero in _cairo_malloc_ab macro

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Ginn Chen]

Load Tp2 test page
http://localhost/page_load_test/pages/www.terra.com.br/www.terra.com.br/cap   a/index.html

Firefox crashed.

t@null (l@1) terminated by signal FPE (integer divide by zero)
Current function is _get_bitmap_surface
  758               data = _cairo_malloc_ab (height, stride);

(dbx) print stride
stride = 0
(dbx) print height
height = 0

(dbx) where                                                                  
=>[1] _get_bitmap_surface(bitmap = 0x94b0534, own_buffer = 0, font_options = 0x988161c, surface = 0x8041ec8), line 758 in "cairo-ft-font.c"
  [2] _render_glyph_bitmap(face = 0x94b02d0, font_options = 0x988161c, surface = 0x8041ec8), line 1105 in "cairo-ft-font.c"
  [3] _cairo_ft_scaled_glyph_init(abstract_font = 0x9881518, scaled_glyph = 0x94b0c88, info = 3), line 2018 in "cairo-ft-font.c"

(dbx) frame 2 
Current function is _render_glyph_bitmap
 1105       status = _get_bitmap_surface (&glyphslot->bitmap, FALSE, font_options, surface);
(dbx) list -10
 1095        * the opportunity to convert such to
 1096        * bitmap. FT_GLYPH_FORMAT_COMPOSITE will not be encountered since
 1097        * we avoid the FT_LOAD_NO_RECURSE flag.
 1098        */
 1099       error = FT_Render_Glyph (glyphslot, FT_RENDER_MODE_NORMAL);
 1100       if (error) {
 1101           _cairo_error (CAIRO_STATUS_NO_MEMORY);
 1102           return CAIRO_STATUS_NO_MEMORY;
 1103       }
 1104
(dbx) print *glyphslot
*glyphslot = {
    library           = 0x84a0278
    face              = 0x94b02d0
    next              = (nil)
    reserved          = 0
    generic           = {
        data      = (nil)
        finalizer = (nil)
    }
    metrics           = {
        width        = 0
        height       = 0
        horiBearingX = 0
        horiBearingY = 0
        horiAdvance  = 192
        vertBearingX = 0
        vertBearingY = 0
        vertAdvance  = 0
    }
    linearHoriAdvance = 0
    linearVertAdvance = 0
    advance           = {
        x = 192
        y = 0
    }
    format            = FT_GLYPH_FORMAT_BITMAP
    bitmap            = {
        rows         = 0
        width        = 0
        pitch        = 0
        buffer       = (nil)
        num_grays    = 1
        pixel_mode   = '\001'
        palette_mode = '\0'
        palette      = (nil)
    }
    bitmap_left       = 0
    bitmap_top        = 0
    outline           = {
        n_contours = 0
        n_points   = 0
        points     = (nil)
        tags       = (nil)
        contours   = (nil)
        flags      = 0
    }
    num_subglyphs     = 0
    subglyphs         = (nil)
    control_data      = (nil)
    control_len       = 0
    lsb_delta         = 0
    rsb_delta         = 0
    other             = (nil)
    internal          = 0x9601128
}

Comment 1

[Ginn Chen]

Attachment: to avoid divide by zero

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 275564 [details] [diff] [review]
to avoid divide by zero

Looks good, I need to get this upstream as well.  Add in some parens around size?  "(size) && "...

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

So wait.. this ends up doing malloc(0) if size == 0, and we're explicitly trying to avoid that situation (because the result is nonzero on some OSs).  That's bad, we shouldn't do that.

Comment 4

[Ginn Chen]

malloc(0) is non zero on Solaris, I didn't get a problem.

Yes, it could be a bad thing.
I'll work on another patch.

Thanks!

Comment 5

[Ginn Chen]

Attachment: patch v2

check and return NULL for malloc(0)

or we could do
 #define _cairo_malloc(size) \
    ((size) ? malloc((unsigned) (size)) : NULL)

Verified on Solaris x86

Comment 6

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 276745 [details] [diff] [review]
patch v2

hmm, yeah, I'd prefer the _cairo_malloc #define -- seems cleaner (and useful by itself)

Comment 7

[Ginn Chen]

Attachment: patch v3

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 276884 [details] [diff] [review]
patch v3

Looks great, thanks!

Comment 9

[Ginn Chen]

committed.

leave this bug open for vladimir for upstreaming.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Pushed upstream, really this time.