548793 - valgrind: cairo-xlib-display.c: failure of XRenderQueryVersion silently ignored

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Julian Seward [:jseward]]

Running mochitests-plain on Valgrind (something I'm trying to
automate) produces a flood of errors of this form at Fx startup:


Conditional jump or move depends on uninitialised value(s)
   at 0x620C85D: _cairo_xlib_surface_create_internal (cairo-xlib-surface.c:2652)
   by 0x620CED4: _moz_cairo_xlib_surface_create (cairo-xlib-surface.c:2807)
   by 0x61AC0DC: gfxXlibSurface::gfxXlibSurface(_XDisplay*, unsigned long, Visual*, gfxIntSize const&) (gfxXlibSurface.cpp:70)
   by 0x5F725EC: nsWindow::GetThebesSurface() (nsWindow.cpp:7513)
   by 0x5FA46B3: nsThebesDeviceContext::CreateRenderingContext(nsIWidget*, nsIRenderingContext*&) (nsThebesDeviceContext.cpp:745)
   by 0x5768FD5: PresShell::CreateRenderingContext(nsIFrame*, nsIRenderingContext**) (nsPresShell.cpp:3569)
   by 0x5771F27: PresShell::DoReflow(nsIFrame*, int) (nsPresShell.cpp:7229)
   by 0x57723FD: PresShell::ProcessReflowCommands(int) (nsPresShell.cpp:7410)
   by 0x5772663: PresShell::FlushPendingNotifications(mozFlushType) (nsPresShell.cpp:4784)
   by 0x5907C59: nsDocument::FlushPendingNotifications(mozFlushType) (nsDocument.cpp:6342)
   by 0x5DAA9CC: nsDocLoader::DocLoaderIsEmpty(int) (nsDocLoader.cpp:756)
   by 0x5DAAB22: nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (nsDocLoader.cpp:697)
 Uninitialised value was created by a stack allocation
   at 0x6214020: _cairo_xlib_display_get (cairo-xlib-display.c:212


It appears the root cause is in _cairo_xlib_display_get:

  cairo_status_t
  _cairo_xlib_display_get (Display *dpy,
  			   cairo_xlib_display_t **out)
  {
      int render_major, render_minor;
      ...
      XRenderQueryVersion (dpy, &render_major, &render_minor);
      ...
      display->render_major = render_major;
      display->render_minor = render_minor;
      ...
      *out = display;
      return status;
  }

If XRenderQueryVersion fails for any reason then render_minor/_major
are not set, as per
http://www.x.org/releases/X11R7.5/doc/libXrender/libXrender.txt
and we carry merrily on with uninitialised values for them.

Adding the following 3 lines confirms this suspicion:

  +    render_major = 1234; render_minor = 5678;
       XRenderQueryVersion (dpy, &render_major, &render_minor);
  +    printf("QQQQQQQQQQQQQQQQQQ render_major = %d\n", (int)render_major);
  +    printf("QQQQQQQQQQQQQQQQQQ render_minor = %d\n", (int)render_minor);

Running

  $ DISPLAY=:1.0 firefox-ff-dev ""

produces

  Xlib:  extension "RANDR" missing on display ":1.0".
  Xlib:  extension "Generic Event Extension" missing on display ":1.0".
  Xlib:  extension "Generic Event Extension" missing on display ":1.0".
  Xlib:  extension "Generic Event Extension" missing on display ":1.0".

  (firefox-bin:18470): GLib-WARNING **: g_set_prgname() called multiple times
  Xlib:  extension "Generic Event Extension" missing on display ":1.0".
  QQQQQQQQQQQQQQQQQQ render_major = 1234
  QQQQQQQQQQQQQQQQQQ render_minor = 5678

What mystifies me is that I haven't seen this before.
cairo-xlib-display.c hasn't been touched since 14 Sept 09 and I have
been valgrinding frequently since then.


Maybe it's X server specific?  This is a VNC server, w/ following
info:

$ DISPLAY=:1.0 xdpyinfo

name of display:    :1.0
version number:    11.0
vendor string:    The X.Org Foundation
vendor release number:    70000000
X.Org version: 7.0.0
maximum request size:  16777212 bytes
motion buffer size:  256
bitmap unit, bit order, padding:    32, LSBFirst, 32
image byte order:    LSBFirst
number of supported pixmap formats:    2
supported pixmap formats:
    depth 1, bits_per_pixel 1, scanline_pad 32
    depth 16, bits_per_pixel 16, scanline_pad 32
keycode range:    minimum 8, maximum 255
focus:  window 0x25, revert to None
number of extensions:    20
    BIG-REQUESTS
    DAMAGE
    DEC-XTRAP
    DOUBLE-BUFFER
    Extended-Visual-Information
    MIT-SCREEN-SAVER
    MIT-SHM
    MIT-SUNDRY-NONSTANDARD
    RECORD
    SECURITY
    SHAPE
    SYNC
    TOG-CUP
    VNC-EXTENSION
    X-Resource
    XC-APPGROUP
    XC-MISC
    XFree86-Bigfont
    XTEST
    XVideo
default screen number:    0
number of screens:    1

screen #0:
  dimensions:    960x720 pixels (244x183 millimeters)
  resolution:    100x100 dots per inch
  depths (2):    1, 16
  root window id:    0x25
  depth of root window:    16 planes
  number of colormaps:    minimum 1, maximum 1
  default colormap:    0x20
  default number of colormap cells:    64
  preallocated pixels:    black 0, white 65535
  options:    backing-store YES, save-unders YES
  largest cursor:    960x720
  current input event mask:    0x20000
    StructureNotifyMask
  number of visuals:    2
  default visual id:  0x21
  visual:
    visual id:    0x21
    class:    TrueColor
    depth:    16 planes
    available colormap entries:    64 per subfield
    red, green, blue masks:    0xf800, 0x7e0, 0x1f
    significant bits in color specification:    8 bits
  visual:
    visual id:    0x22
    class:    DirectColor
    depth:    16 planes
    available colormap entries:    64 per subfield
    red, green, blue masks:    0x3f, 0x7c0, 0xf800
    significant bits in color specification:    8 bits

Comment 1

[Boris Zbarsky [:bzbarsky]]

> Maybe it's X server specific? 

Yes.  The key is that your server doesn't have the RENDER extension.

Not sure whether this should just be filed as an upstream cairo bug or not...

Comment 2

[Julian Seward [:jseward]]

Attachment: initialise version numbers even if XRenderQueryVersion fails

Comment 3

[Robert Longson [:longsonr]]

You really want the cairo project http://www.cairographics.org/ to take this patch so that we can catch it next time we do an update. If at all possible we don't want to maintain our own changes.

Comment 4

[Julian Seward [:jseward]]

I'm all in favour of pushing the fix upstream, if that's where 
it belongs.  However, does this mean there's zero chance of having
it even temporarily in m-c, until the next time a sync with cairo
happens?  On the basis that (1) it interferes with
Valgrinded-Mochitesting on Linux by causing dozens of UMRs to be reported
in various locations in gfx, and (2) for folks running Unix builds of
m-c on VNC servers that (AUIU) don't have the RENDER extension, m-c is
in this regard broken (perhaps inconsequentially) as it stands.

Comment 5

[Robert Longson [:longsonr]]

No, it doesn't mean we won't take it locally too. Jeff does synchronise with upstream cairo pretty regularly too.

Comment 6

[Julian Seward [:jseward]]

I'm pleased to hear that.  r?-ing Jeff.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 429117 [details] [diff] [review]
initialise version numbers even if XRenderQueryVersion fails

Please use "See mozilla.org #548793" or something like that because cairo has it's own bug tracking system.

We also keep the patches against upstream in the cairo directory. You can add a patch there and add a note to the readme if you want to commit this yourself. Otherwise, I can take care of that if you're not in too much of hurry.

Comment 8

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

http://hg.mozilla.org/mozilla-central/rev/ac9b76514eec