480897 - mac valgrind warnings within _moz_cairo_pattern_create_rgba

Status: RESOLVED INVALID

(Core :: Graphics, defect)

Description

[Robert Sayre]

will attach logs from startup

Comment 1

[Robert Sayre]

Attachment: valgrind logs from starting up with http://www.google.com homepage

Comment 2

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

Not sure how this is possible:

==65811== Conditional jump or move depends on uninitialised value(s)
==65811==    at 0x3D98F85: _cairo_restrict_value
==65811==    by 0x3D9F7C3: _moz_cairo_pattern_create_rgba
==65811==    by 0x3D83514: _moz_cairo_set_source_rgba
==65811==    by 0x3CF31C6: gfxContext::SetColor(gfxRGBA const&)
==65811==    by 0x3B395F1: nsThebesRenderingContext::SetColor(unsigned int)
==65811==    by 0x30037AD: nsImageFrame::DisplayAltFeedback(nsIRenderingContext&, nsRect const&, imgIRequest*, nsPoint)
==65811==    by 0x300396C: PaintAltFeedback(nsIFrame*, nsIRenderingContext*, nsRect const&, nsPoint)
==65811==    by 0x2FBEEBD: nsDisplayGeneric::Paint(nsDisplayListBuilder*, nsIRenderingContext*, nsRect const&)

The problem is likely somewhere halfway through there, since the top few functions on the stack just pass the data straight through.  The gfxRGBA constructor that's called in ::SetColor gets passed the integer and then does some math on it, but it's still marked as uninitialized memory.  However, in DisplayAltFeedback there's this (irrelevant bits removed):

      nscolor oldColor;
      aRenderingContext.GetColor(oldColor);
      aRenderingContext.SetColor(NS_RGB(0xFF,0,0));
      aRenderingContext.SetColor(oldColor);

So this could be bad if GetColor either failed or if it had bogus data, but GetColor just does aColor = mColor, and mColor is initialized in nsThebesRenderingContext's (only) constructor.

Presumably we could hit this if beforehand someone called SetColor with an invalid value but we'd hit an earlier valgrind warning from through SetColor in the exact same way.

Comment 3

[Jesse Ruderman]

You're using a build that has the patch from bug 480032, right?  Can you attach a log that uses --track-origins=yes?

Comment 4

[Robert Sayre]

(In reply to comment #3)
> You're using a build that has the patch from bug 480032, right?  Can you attach
> a log that uses --track-origins=yes?

ah, maybe not. I'll close this if it's broken.

Comment 5

[JP Rosevear [:jpr]]

Going to assume it was broken and close this out.