[Xcb] mesa +xcb = assertion death

[Jamey Sharp]

On Wed, 2005-05-04 at 22:14 -0400, Jeremy Kolb wrote:
> Jamey Sharp wrote:
> > On Wed, 2005-05-04 at 17:52 -0400, Jeremy Kolb wrote:
> > I'm just taking a moment to study glxext.c, which looks to me like the
> > code on the client side that's making both of the wacky requests, in
> > AllocAndFetchScreenConfigs. First thing I noticed is a double-unlock bug
> > in the case that a screen doesn't support GL rendering. I also suspect
> > that most of the error-handling code leaves the X protocol stream
> > unsynchronized, so that later chatter with the server would lose if
> > anything goes wrong in this function. (Needs some calls to _XEatData.)
> 
> Where/what is that exactly?  Does it make a difference?

Immediately after the call to _XReply is an unconditional UnlockDisplay,
which is followed by an 'if(!reply.numVisuals) { UnlockDisplay ...'. On
further inspection the unconditional unlock looks wildly wrong: it means
no lock is held later during the _XRead call, and in fact a
double-unlock occurs no matter what code path is taken to exit the
function.

As for the need for _XEatData calls, every UnlockDisplay call, after the
_XReply and before the for-loop that calls _XRead, should be preceded by
_XEatData(dpy, numVisuals * numProps * 2). Or something like that. More
or less.

Fixing these should not affect the bugs we're actually seeing, I think,
because as you point out below, we're not reaching any of those
statements before XCB assert-fails. They might be more visibly buggy on
an XCB-based Xlib after the big bug is fixed, though; and regardless
they're easy to fix and so ought to be squashed.

> > It looks like the three kinds of requests supported by this function
> > (GLXVendorPrivateWithReply/GetFBConfigsSGIX, GLXGetFBConfigs, and
> > GLXGetVisualConfigs) all reply with a numProps and a numVisuals field,
> > and the number of words returned is something like numProps times
> > numVisuals (times 2, when not using GLXGetVisualConfigs). The length
> > field of the reply is never consulted. So that much of my hypothesis is
> > confirmed, at least.
> 
> In the docs the reply length is numProps*numVisuals*2 so you are 
> correct.  I'm a bit confused about checking the length field as it bombs 
> within _XReply() in this case with the assertion error so it never 
> reaches past that point.

XCB is checking the length field and treats the data following that
length as a separate response; but the following data is garbage, and
triggers an assertion. (Note that XCB doesn't actually spot anything
wrong with the reply itself, only dying when it tries to read the end of
the reply as if that were itself a response. Mostly we got lucky: the
end of these replies doesn't look anything like a valid response.)

Traditionally, Xlib and Mesa have ignored the length field and only
examined the other reply fields. When running on Xlib/XCB, Mesa doesn't
get to ignore the length field because XCB has already choked.

er... Did I explain it any better that time?

On Wed, 2005-05-04 at 22:30 -0400, Jeremy Kolb wrote: 
> Ack sorry! I didn't mean anything by the 'does it make a difference?'... 
> I mean does it make a difference in terms of the bug :)

Don't panic! I knew what you meant. ;-)

--Jamey