Fwd: Re: bug in XFree86 4.1.0 with Rage 128 driver?

Fwd: Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin B. Hendricks]

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

Hi Michel (and FYI Herbert).

The bug was in XF 4.1.0 in Xrender.c not in OpenOffice

The patch is attached.  They (XF4 guys) simply missed changing a variable
name and ended up using random garbage in an "if"

Dan and Olaf, if you haven't gone to press with new releases, you probably
should include this one in your next respin builds of XFree86 4.1.0

Michel will you see that this makes it into the next XF 4.X release for us.

Thanks,

Kevin



> Will you please look at the following diff from Xrender.c in XF 4.0.2 to
> Xrender.c in XF410.  It looks very strange.  Starting in lie 190.
> It seems they no longer Xmalloc a value for xFormat (they now use xData)
> but just one line later they check if xFormat is zero.

> The code before that in XRenderQueryFormats() doesNOT set a value for
> xFormat at all after its is declared and before it is tested.

> I think someone just missed changing and xFormat to an xData unless I am
> missing something here.

> Kevin



>@@ -190,19 +191,20 @@
>               rep.numScreens * sizeof (xPictScreen) +
>               rep.numDepths * sizeof (xPictDepth) +
>               rep.numVisuals * sizeof (xPictVisual));
>-    xFormat = (xPictFormInfo *) Xmalloc (rlength);
>+    xData = (void *) Xmalloc (rlength);
>
>     if (!xri || !xFormat)
>     {
>        if (xri) Xfree (xri);
>-       if (xFormat) Xfree (xFormat);
>+       if (xData) Xfree (xData);
>        _XEatData (dpy, rlength);
>        UnlockDisplay (dpy);
>        SyncHandle ();
>        return 0;

[-- Attachment #2: xf410_render.patch --]
[-- Type: text/x-diff, Size: 340 bytes --]

--- xc/lib/Xrender/Xrender.c.prev	Wed Oct 10 22:38:25 2001
+++ xc/lib/Xrender/Xrender.c	Wed Oct 10 22:39:09 2001
@@ -193,7 +193,7 @@
 	       rep.numVisuals * sizeof (xPictVisual));
     xData = (void *) Xmalloc (rlength);
     
-    if (!xri || !xFormat)
+    if (!xri || !xData)
     {
 	if (xri) Xfree (xri);
 	if (xData) Xfree (xData);

Re: Fwd: Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Michel Dänzer]

"Kevin B. Hendricks" wrote:

> The bug was in XF 4.1.0 in Xrender.c not in OpenOffice
>
> The patch is attached.  They (XF4 guys) simply missed changing a variable
> name and ended up using random garbage in an "if"

Why does it work with the fbdev driver then?


> Michel will you see that this makes it into the next XF 4.X release for us.

I don't feel qualified to do that. Xpert@XFree86.Org is a mailing list which
the author of this code and other very competent XFree86 hackers read, or if
you're absolutely sure the fix is correct, please submit it to
fixes@XFree86.Org .


--
Earthling Michel Dänzer (MrCooper)    \   Debian GNU/Linux (powerpc) developer
CS student, Free Software enthusiast   \        XFree86 and DRI project member

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: Fwd: Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Olaf Hering]

On Thu, Oct 11, Michel Dänzer wrote:

> "Kevin B. Hendricks" wrote:
>
> > The bug was in XF 4.1.0 in Xrender.c not in OpenOffice
> >
> > The patch is attached.  They (XF4 guys) simply missed changing a variable
> > name and ended up using random garbage in an "if"
>
> Why does it work with the fbdev driver then?
>
>
> > Michel will you see that this makes it into the next XF 4.X release for us.
>
> I don't feel qualified to do that. Xpert@XFree86.Org is a mailing list which
> the author of this code and other very competent XFree86 hackers read, or if
> you're absolutely sure the fix is correct, please submit it to
> fixes@XFree86.Org .


keithp already put something in the CVS. It did not work with "ati", but
with fbdev (clgen).
The patch fixed it for me on the Pismo.



Gruss Olaf

--
 $ man clone

BUGS
       Main feature not yet implemented...

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin Hendricks]

Hi All.

>> I don't feel qualified to do that. Xpert@XFree86.Org is a mailing list
>> which
>> the author of this code and other very competent XFree86 hackers read,
>> or if
>> you're absolutely sure the fix is correct, please submit it to
>> fixes@XFree86.Org .

Michel:

I just don't see how using an uninitialized variable in an "if"
statement can be anything else but a bug.  You could have at least
looked at the code in question before replying.  You are a member of
XFree representing ppc linux aren't you?  I assumed with your signature
and your earlier responses, you were the right person to contact.  If
not, I apologize for bothering you.

> keithp already put something in the CVS. It did not work with "ati", but
> with fbdev (clgen).
> The patch fixed it for me on the Pismo.

Thanks Olaf,

Glad to see it helped and is in CVS.

Kevin


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Olaf Hering]

On Thu, Oct 11, Michel Dänzer wrote:

> On Thu, 2001-10-11 at 14:53, Kevin Hendricks wrote:
>
> > >> I don't feel qualified to do that. Xpert@XFree86.Org is a mailing list
> > >> which the author of this code and other very competent XFree86 hackers read,
> > >> or if you're absolutely sure the fix is correct, please submit it to
> > >> fixes@XFree86.Org .
> >
> > Michel:
> >
> > I just don't see how using an uninitialized variable in an "if"
> > statement can be anything else but a bug.
>
> You're probably right. Never meant to say otherwise.
>
> > You could have at least looked at the code in question before replying.
>
> If I had enough time...
>
> > You are a member of XFree representing ppc linux aren't you?
>
> Yes, but I don't have any experience with this code, and I'm not member of
> the core team so I have to submit patches for inclusion as well. I was
> referring you to places I thought would allow for faster inclusion.
>
>
> > > keithp already put something in the CVS. It did not work with "ati", but
> > > with fbdev (clgen).
> > > The patch fixed it for me on the Pismo.
>
> So what's in CVS only works for fbdev, Kevin's patch is needed for r128?

Its clearly needed for everyone, Keith already did the commit a few
hours ago.



Gruss Olaf

--
 $ man clone

BUGS
       Main feature not yet implemented...

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin Hendricks]

Hi Michel,

> So what's in CVS only works for fbdev, Kevin's patch is needed for r128?

I went and used cvsweb on Xfree86.org to see what keithp actually
committed.

The bug I found was actually *already* fixed by XF4 guys  soon after
XF410 was released (they simply removed the || !xFormat in the "if")
(given the dates on the Xrender.c changes).

So all keithp had to add was the code to check to make sure xData
malloced okay (i.e put back in || !xData in the "if").

So what is in cvs head right now for this code snippet turns out to be
exactly what my  patch will do for XF 4.1.0.

I assume the r128 versus framebuffer issue was only due to random memory
being tested so the results were/are random.

The patch is needed for all 4.1.0 respins since the bug only exists in
4.1.0 but anything based on cvs recently should be okay as is.

Hope this helps.

Kevin


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Michel Dänzer]

On Thu, 2001-10-11 at 17:37, Kevin Hendricks wrote:

> > So what's in CVS only works for fbdev, Kevin's patch is needed for
> > r128?
>
> I went and used cvsweb on Xfree86.org to see what keithp actually
> committed.
>
> The bug I found was actually *already* fixed by XF4 guys  soon after
> XF410 was released (they simply removed the || !xFormat in the "if")
> (given the dates on the Xrender.c changes).
>
> So all keithp had to add was the code to check to make sure xData
> malloced okay (i.e put back in || !xData in the "if").
>
> So what is in cvs head right now for this code snippet turns out to be
> exactly what my  patch will do for XF 4.1.0.
>
> I assume the r128 versus framebuffer issue was only due to random memory
> being tested so the results were/are random.
>
> The patch is needed for all 4.1.0 respins since the bug only exists in
> 4.1.0 but anything based on cvs recently should be okay as is.

The fix is only in the trunk, not the 4.1 branch?


--
Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer
XFree86 and DRI project member   /  CS student, Free Software enthusiast

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin Hendricks]

Hi,

> The fix is only in the trunk, not the 4.1 branch?

I did not see it but perhaps cvsweb is not yet up to date.

I looked under xf-4_1-branch using cvsweb and it still needs to be
patched (with my patch).

Hope this helps,

Kevin

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin Hendricks]

Hi,

>> Why does it work with the fbdev driver then?

Just a guess.

Since we are using an uninitialized variable whatever was in memory
previously determined the value of xFormat.  So the result was random
behavior when checking for formats.

So anything that changed the stack usage or memory layout would / should
have an impact.

Kevin

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin B. Hendricks]

Hi,

I am trying to track down a bug that causes OpenOffice to segfault under
XFree86 4.1.0 with rage 128 driver.  This same problem does NOT happen
with XFree 4.0.2.

The problem stems from incorrect or missing alpha information in the
visual.  Olaf tried it with a straight fbdev driver and the problem
disappears so it appears to be linked to changes in the Rage 128 driver
from XF 4.0.2 to XF 4.1.0.

I am trying to look at what changed and was wondering about the lack of
support for depth 32 in the r128 driver.

If I specify depth 32, I used to get 24 bits of color depth with 32 bpp in
the Rage 128 driver.  Now if I try to specify a  depth of 32 in the
XF86Config file, I get the following message:

"Given depth (32) is not supported by the r128 driver".

But if I specify 24 bits of color depth I seem to be getting cfb24 being
used instead of cfb32.

Does anyone know why this change was made?  The kernel itself does support
depths of "32" in the kernel rage 128 driver.

I agree this confuses the depth versus bpp issue but different code is
being used (cfb24 versus cfb32) and the resulting visuals have changed.

Any help would be greatly appreciated.

Thanks,

Kevin


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: bug in XFree86 4.1.0 with Rage 128 driver?

[Kevin B. Hendricks]

Hi,

> > If I specify depth 32, I used to get 24 bits of color depth with 32
> > bpp in the Rage 128 driver.
>
> No you didn't. XFree86 4 has never supported depth 32.

I thought that the 24 depth was the broken one that was not supported
early on and all that did work was 8, 16, and 32 (or am I confusing the
kernel driver versus the XFree86 driver).

> > But if I specify 24 bits of color depth I seem to be getting cfb24
> > being used instead of cfb32.
>
> Only if you specify DefaultFbBpp 24 in Section "Screen" or -fbbpp 24 on
> the command line. The default for depth 24 is 32 bpp in the r128 driver
> - has always been AFAIR.

Okay, I must have misread it.

> In contrast to framebuffer devices, XFree86 strictly separates the terms
> 'depth' and 'bits per pixel'.

It never used to ;-)   It used to confuse them all the time for a depth of
15 in 16bpp called a depth of 16.  That used to drive the JDK crazy.

> > and the resulting visuals have changed.
>
> The problem might be related to visuals, things have changed about them
> due to the RENDER extension and other things; I don't know details, you
> might want to check out the Xpert mailing list archives.

Yes, the error happens in the XRender lib but it is partially our fault.
I am just trying to figure out what changed.  Here is the relevant code
from OOo:

  // the 8bit alpha mask format must be there
    XRenderPictFormat aPictFormat={0,0,8,{0,0,0,0,0,0,0,0xFF},0};
    mpGlyphFormat = (*pXRenderFindFormat)( mpDisplay, 0, &aPictFormat, 1 );

    // and support for the visual
    XRenderPictFormat*  pVisualFormat =  (*pXRenderFindVisualFormat)(
mpDisplay, _pVisual );
    if( pVisualFormat != NULL )
        mbUsingXRender = true;

The problem is that the call to XRenderFindFormat() is returning 0 under
XF4.1.0 under Rage 128 but it previously returned a proper value under XF
4.0.2 using Rage 128.

> If this is the problem, it's most likely an OpenOffice bug. Applications
> tend to make false assumptions about visuals and/or not handle them
> correctly.

Quite probably, the interesting thing here is that this all works under XF
4.0.2 using Rage 128 and now fails under XF 4.1.0 using Rage 128 (but
works if the straight framebuffer is used).

So the issue is tied to the Rage 128 code in some way.

I will keep looking.

Thanks for your help!

Kevin

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/