Firefox crashed [@ _get_bitmap_surface] div zero in _cairo_malloc_ab macro

RESOLVED FIXED

Status

()

Core
Graphics
--
critical
RESOLVED FIXED
11 years ago
7 years ago

People

(Reporter: Ginn Chen, Assigned: vlad)

Tracking

({crash})

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

11 years ago
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
}
(Reporter)

Comment 1

11 years ago
Created attachment 275564 [details] [diff] [review]
to avoid divide by zero
Assignee: nobody → ginn.chen
Status: NEW → ASSIGNED
Attachment #275564 - Flags: review?(vladimir)
(Reporter)

Updated

11 years ago
Summary: Firefox crashed [@_cairo_malloc_ab] → Firefox crashed [@ _get_bitmap_surface] div zero in _cairo_malloc_ab macro
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) && "...
Attachment #275564 - Flags: review?(vladimir) → review+
(Reporter)

Updated

11 years ago
Attachment #275564 - Flags: approval1.9?
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.
(Reporter)

Comment 4

11 years ago
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!
(Reporter)

Comment 5

11 years ago
Created attachment 276745 [details] [diff] [review]
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
Attachment #276745 - Flags: review?(vladimir)
Comment on attachment 276745 [details] [diff] [review]
patch v2

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

Comment 7

11 years ago
Created attachment 276884 [details] [diff] [review]
patch v3
Attachment #275564 - Attachment is obsolete: true
Attachment #276745 - Attachment is obsolete: true
Attachment #276884 - Flags: review?(vladimir)
Attachment #275564 - Flags: approval1.9?
Attachment #276745 - Flags: review?(vladimir)
Comment on attachment 276884 [details] [diff] [review]
patch v3

Looks great, thanks!
Attachment #276884 - Flags: review?(vladimir)
Attachment #276884 - Flags: review+
Attachment #276884 - Flags: approval1.9+
(Reporter)

Comment 9

11 years ago
committed.

leave this bug open for vladimir for upstreaming.
Assignee: ginn.chen → vladimir
Status: ASSIGNED → NEW
Pushed upstream, really this time.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Crash Signature: [@ _get_bitmap_surface]
You need to log in before you can comment on or make changes to this bug.