838617 - assertion at cairo-win32-font.c:2112 is frequently being triggered - code inspection indicates use of uninitialized object

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jonathan Kew [:jfkthame]]

Running a current debug build on XP or on Win7 without acceleration (i.e., using GDI fonts), I'm seeing the assertion at:

  http://mxr.mozilla.org/mozilla-central/source/gfx/cairo/cairo/src/cairo-win32-font.c#2112

being hit pretty frequently, which seemed worrying.

The same assertion is showing up in TBPL logs, e.g.:

  https://tbpl.mozilla.org/php/getParsedLog.php?id=19485509&tree=Firefox&full=1

but (unfortunately) it isn't picked up by the log parser, so the runs still show up as green.

This assertion (and a bunch of the surrounding code) was added in bug 717178.

Inspection of the code here shows that whenever cairo_win32_font_face_create_for_logfontw_hfont is called with a non-null HFONT parameter, the |key| variable is never initialized, as the only call that would set it is within the first |if (!font) {...}| block. So the assertion is comparing the font_face's hash against a chunk of uninitialized memory.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: don't assert equality with the key's hash unless it was actually initialized

If I'm understanding the code here correctly, I think all that's needed is to move the assertion inside the following if() block, as in the case where a non-null HFONT was passed, we aren't interested in the hash table anyway.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 710709 [details] [diff] [review]
don't assert equality with the key's hash unless it was actually initialized

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

don't forget to create a patch in gfx/cairo for this

Comment 3

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/abea8a82da63
(including adding the patch file to gfx/cairo)

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/abea8a82da63