Closed Bug 413169 Opened 15 years ago Closed 15 years ago

[valgrind] cairo: uninitialized memory read during printing

Categories

(Core :: Graphics, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9beta3

People

(Reporter: sayrer, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: valgrind)

Attachments

(1 file)

Conditional jump or move depends on uninitialised value(s)
    at 0x62F12A0: _cairo_rectangle_intersect (cairo-rectangle.c:71)
    by 0x6321F54: _cairo_analysis_surface_show_glyphs (cairo-analysis-surface.c:552)
    by 0x62F4C9D: _cairo_surface_show_glyphs (cairo-surface.c:2092)
    by 0x6314E02: _cairo_meta_surface_replay_internal (cairo-meta-surface.c:815)
    by 0x631721B: _paint_page (cairo-paginated-surface.c:300)
    by 0x63174D6: _cairo_paginated_surface_show_page (cairo-paginated-surface.c:449)
    by 0x62F52B5: _moz_cairo_surface_show_page (cairo-surface.c:1708)
    by 0x62DD405: gfxPSSurface::EndPage() (gfxPSSurface.cpp:94)
    by 0x7891101: nsThebesDeviceContext::EndPage() (nsThebesDeviceContext.cpp:584)
    by 0x723B399: nsSimplePageSequenceFrame::DoPageEnd() (nsSimplePageSequence.cpp:651)
    by 0x754F290: nsPrintEngine::PrintPage(nsPrintObject*, int&) (nsPrintEngine.cpp:2344)
    by 0x755362B: nsPagePrintTimer::Notify(nsITimer*) (nsPagePrintTimer.cpp:90)
Blocks: 411369
You can hit this by printing attachment 296882 [details] from bug 411369.
Version: unspecified → Trunk
Summary: [valgrind] uninitialized memory read during printing → [valgrind] cairo: uninitialized memory read during printing
Component: GFX → GFX: Thebes
QA Contact: general → thebes
We give a cairo_rectangle_int_t* to cairo_scaled_font_glyph_device_extents()
that expects a cairo_rectangle_int16_t*:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/gfx/cairo/cairo/src/cairo-analysis-surface.c&rev=1.13&root=/cvsroot&mark=515,545,548,552#505
so 'glyph_extents' will have some uninitialized bytes, which 
cairo_rectangle_intersect() uses, which valgrind complains about.

gcc actually warns about the problem:
cairo-analysis-surface.c: In function '_cairo_analysis_surface_show_glyphs':
cairo-analysis-surface.c:548: warning: passing argument 4 of '_cairo_scaled_font_glyph_device_extents' from incompatible pointer type
Attached patch Patch rev. 1Splinter Review
I changed cairo_scaled_font_glyph_device_extents() to take a
cairo_rectangle_int_t* instead.
Attachment #298117 - Flags: superreview?(vladimir)
Attachment #298117 - Flags: review?(vladimir)
Assignee: nobody → mats.palmgren
(In reply to comment #2)
> gcc actually warns about the problem:
> cairo-analysis-surface.c: In function '_cairo_analysis_surface_show_glyphs':
> cairo-analysis-surface.c:548: warning: passing argument 4 of
> '_cairo_scaled_font_glyph_device_extents' from incompatible pointer type

Why again do we not use warnings-as-errors in more places? Warnings are not usually good, yet we seem to ignore them throughout the tree. :(
Comment on attachment 298117 [details] [diff] [review]
Patch rev. 1


Ah ha, good catch -- I guess I need to audit usage of rectangle_int16_t again, since either I missed some or some snuck in after my big change.

I'll get this into upstream cairo.
Attachment #298117 - Flags: superreview?(vladimir)
Attachment #298117 - Flags: superreview+
Attachment #298117 - Flags: review?(vladimir)
Attachment #298117 - Flags: review+
Attachment #298117 - Flags: approval1.9?
Keywords: valgrind
Attachment #298117 - Flags: approval1.9? → approval1.9+
mozilla/gfx/cairo/cairo/src/cairo-scaled-font.c 	1.27
mozilla/gfx/cairo/cairo/src/cairo-surface-fallback.c 	1.23
mozilla/gfx/cairo/cairo/src/cairo-xcb-surface.c 	1.24
mozilla/gfx/cairo/cairo/src/cairo-xlib-surface.c 	1.32
mozilla/gfx/cairo/cairo/src/cairoint.h 	1.52
mozilla/gfx/cairo/cairo/src/cairo-analysis-surface.c 	1.14 

-> FIXED
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M11
You need to log in before you can comment on or make changes to this bug.