Bug 4689 - ppracer + xcompmgr -a kills the server reliably
Summary: ppracer + xcompmgr -a kills the server reliably
Status: CLOSED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: git
Hardware: PowerPC Linux (All)
: high normal
Assignee: Xorg Project Team
QA Contact:
URL:
Whiteboard:
Keywords: have-backtrace
Depends on:
Blocks:
 
Reported: 2005-10-05 00:37 UTC by Benjamin Herrenschmidt
Modified: 2005-10-05 08:26 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Treat DirectColor as TrueColor instead of failing (457 bytes, patch)
2005-10-05 23:31 UTC, Benjamin Herrenschmidt
no flags Details | Splinter Review

Description Benjamin Herrenschmidt 2005-10-05 00:37:44 UTC
When launching ppracer, the server gets a SEGV. I've tracked that down a bit. We
end up crashing in CreatePicture() called by compWindowUpdateAutomatic() (called
from compWindowUpdate() called from the BlockHandler). It's a nice NULL
dereference. The bug seem to be that CreatePicture() gets called with a NULL
pFormat pointer:

compWindowUpdateAutomatic (WindowPtr pWin)
{
    CompWindowPtr   cw = GetCompWindow (pWin);
    ScreenPtr       pScreen = pWin->drawable.pScreen;
    WindowPtr       pParent = pWin->parent;
    PixmapPtr       pSrcPixmap = (*pScreen->GetWindowPixmap) (pWin);
    PictFormatPtr   pSrcFormat = compWindowFormat (pWin);

*** Here, pSrcFormat is NULL ***

    PictFormatPtr   pDstFormat = compWindowFormat (pWin->parent);
    int             error;
    RegionPtr       pRegion = DamageRegion (cw->damage);
    PicturePtr      pSrcPicture = CreatePicture (0, &pSrcPixmap->drawable,
                                                 pSrcFormat, 
                                                 0, 0,
                                                 serverClient,
                                                 &error);
    
And the above CreatePicture crashes. Now why is it NULL ? Well, I've traced a
bit. It all ends up in PictureMatchVisual() which returns basically the pointer
we are getting.

First a note: There seem to be _plenty_ of cases where this can return NULL.
However, compWindowUpdateAutomatic() tests none of them. Shouldn't this function
be hardened a little bit ?

Now, why is it returning NULL ? Here is what it's called with:

pVisual points to visual ID 47 which is a visual of class 5 (DirectColor), 8
bits per RGB values, 256 colormap entries, 24 planes, looks like a 24 bits RGB
visual.

But ... Heh ! PictureMatchVisual() will always return 0 for a DirectColor visual
! It's in the switch/case...

So here, I don't know what _should_ happen as I don't completely understand the
purpose of the function here. What I can tell however, is that we are trying to
get the format pointer for a window with a DirectColor visual, which fails,
returns NULL, and passing that to CreatePicture.

So there are 2 things here:

 - compWindowUpdateAutomatic() should be hardened to bail out instead of
crashing when it gets a NULL result from compWindowFormat()

 - should compWindowFormat() (and thus maybe PictureMatchVisual) be fixed to
deal with DirectColor visuals ?
Comment 1 Benjamin Herrenschmidt 2005-10-05 23:31:50 UTC
Created attachment 3495 [details] [review]
Treat DirectColor as TrueColor instead of failing

Adter discussing with Keith Packard, we decided to simply treat DirectColor as
TrueColor and no try to be smart about handling the colormap. This patch fixe
PictureMatchVisual() to do that.
Comment 2 Benjamin Herrenschmidt 2005-10-06 01:24:10 UTC
Fixed in CVS


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.