1053159 - gfxPattern::mFilter is used uninitialised

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Julian Seward [:jseward]]

content/base/test/csp/test_CSP.html produces Valgrind complaints
as shown in the next comment.

I have considerable difficulty navigating the maze of gfx plumbing,
but the impression I get is that GfxFilterToCairoFilter(Filter filter)
(HelpersCairo.h:98) is called with an undefined |filter| value.

And that is because 
gfxPattern::gfxPattern(SourceSurface *aSurface, const Matrix &aTransform)
doesn't set mFilter to anything.  AFAICS mFilter is a plain enum type
so it won't get automagically initialised by the compiler.

I was a bit surprised to see that none of the gfxPattern::gfxPattern
constructors set mFilter to anything.  Maybe the plan is that
::SetFilter should later be called on the object before mFilter
is used, but that didn't happen, in this case?

Any suggestions how to further diagnose and/or fix this?

Possibly related: bug 1000072 (but this one has better analysis)

Comment 1

[Julian Seward [:jseward]]

Valgrind complaint (one of several)

Conditional jump or move depends on uninitialised value(s)
   at 0x5FBBD52: GfxFilterToCairoFilter (gfx/2d/HelpersCairo.h:100)
   by 0x5FBBD52: mozilla::gfx::GfxPatternToCairoPattern(mozilla::gfx::Pattern const&, float) (gfx/2d/DrawTargetCairo.cpp:324)
   by 0x5FBC215: mozilla::gfx::DrawTargetCairo::DrawPattern(mozilla::gfx::Pattern const&, mozilla::gfx::StrokeOptions const&, mozilla::gfx::DrawOptions const&, mozilla::gfx::DrawTargetCairo::DrawPatternType, bool) (gfx/2d/DrawTargetCairo.cpp:703)
   by 0x5FBC832: mozilla::gfx::DrawTargetCairo::FillRect(mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::Pattern const&, mozilla::gfx::DrawOptions const&) (gfx/2d/DrawTargetCairo.cpp:766)
   by 0x6A2221D: nsWindow::UpdateAlpha(gfxPattern*, nsIntRect) (widget/gtk/nsWindow.cpp:2294)
   by 0x6A22AB0: nsWindow::OnExposeEvent(_GdkEventExpose*) (widget/gtk/nsWindow.cpp:2233)
   by 0x6A22CDE: expose_event_cb(_GtkWidget*, _GdkEventExpose*) (widget/gtk/nsWindow.cpp:5143)
   by 0x333CB4DA1B: _gtk_marshal_BOOLEAN__BOXED (/usr/src/debug/gtk+-2.24.13/gtk/gtkmarshalers.c:86)
   by 0x34A1C0F663: g_closure_invoke (/usr/src/debug/glib-2.32.4/gobject/gclosure.c:777)
   by 0x34A1C206D7: signal_emit_unlocked_R (/usr/src/debug/glib-2.32.4/gobject/gsignal.c:3551)
   by 0x34A1C282A6: g_signal_emit_valist (/usr/src/debug/glib-2.32.4/gobject/gsignal.c:3310)
   by 0x34A1C287C1: g_signal_emit (/usr/src/debug/glib-2.32.4/gobject/gsignal.c:3356)
   by 0x333CC8148D: gtk_widget_event_internal (/usr/src/debug/gtk+-2.24.13/gtk/gtkwidget.c:5017)
   by 0x333CAC2887: gtk_container_propagate_expose (/usr/src/debug/gtk+-2.24.13/gtk/gtkcontainer.c:2757)
   by 0x333CAC124E: gtk_container_expose (/usr/src/debug/gtk+-2.24.13/gtk/gtkcontainer.c:2661)
   by 0x333CB4DA1B: _gtk_marshal_BOOLEAN__BOXED (/usr/src/debug/gtk+-2.24.13/gtk/gtkmarshalers.c:86)
   by 0x34A1C0F663: g_closure_invoke (/usr/src/debug/glib-2.32.4/gobject/gclosure.c:777)

 Uninitialised value was created by a heap allocation
   at 0x4809064: malloc (/home/sewardj/VgTRUNK/mozhx/coregrind/m_replacemalloc/vg_replace_malloc.c:298)
   by 0x481486B: moz_xmalloc (memory/mozalloc/mozalloc.cpp:52)
   by 0x60F6548: operator new (ff-O-linux64/gfx/thebes/../../dist/include/mozilla/mozalloc.h:201)
   by 0x60F6548: gfxContext::PopGroup() (gfx/thebes/gfxContext.cpp:1150)
   by 0x6A22A83: nsWindow::OnExposeEvent(_GdkEventExpose*) (widget/gtk/nsWindow.cpp:2231)
   by 0x6A22CDE: expose_event_cb(_GtkWidget*, _GdkEventExpose*) (widget/gtk/nsWindow.cpp:5143)
   by 0x333CB4DA1B: _gtk_marshal_BOOLEAN__BOXED (/usr/src/debug/gtk+-2.24.13/gtk/gtkmarshalers.c:86)
   by 0x34A1C0F663: g_closure_invoke (/usr/src/debug/glib-2.32.4/gobject/gclosure.c:777)
   by 0x34A1C206D7: signal_emit_unlocked_R (/usr/src/debug/glib-2.32.4/gobject/gsignal.c:3551)
   by 0x34A1C282A6: g_signal_emit_valist (/usr/src/debug/glib-2.32.4/gobject/gsignal.c:3310)
   by 0x34A1C287C1: g_signal_emit (/usr/src/debug/glib-2.32.4/gobject/gsignal.c:3356)
   by 0x333CC8148D: gtk_widget_event_internal (/usr/src/debug/gtk+-2.24.13/gtk/gtkwidget.c:5017)
   by 0x333CAC2887: gtk_container_propagate_expose (/usr/src/debug/gtk+-2.24.13/gtk/gtkcontainer.c:2757)
   by 0x333CAC124E: gtk_container_expose (/usr/src/debug/gtk+-2.24.13/gtk/gtkcontainer.c:2661)
   by 0x333CB4DA1B: _gtk_marshal_BOOLEAN__BOXED (/usr/src/debug/gtk+-2.24.13/gtk/gtkmarshalers.c:86)
   by 0x34A1C0F663: g_closure_invoke (/usr/src/debug/glib-2.32.4/gobject/gclosure.c:777)
   by 0x34A1C2044F: signal_emit_unlocked_R (/usr/src/debug/glib-2.32.4/gobject/gsignal.c:3589)

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: Initialize mFilter

Comment 3

[Matt Woodrow (:mattwoodrow)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/64c414de6db5

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/64c414de6db5
https://hg.mozilla.org/mozilla-central/rev/842b44860ee2

Comment 5

[Julian Seward [:jseward]]

Perhaps a little late in the day to say this, but .. would it be
wise to initialise mFilter in all of the gfxPattern constructors
instead of only in this one?

Comment 6

[Matt Woodrow (:mattwoodrow)]

mFilter is unused with the other constructors, it wouldn't have hurt though. This bug inspired me to rewrite most of gfxPattern, so it'll get fixed that way.