Bug 1032 - Damage causes rootless XDarwin to crash
Summary: Damage causes rootless XDarwin to crash
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/DDX/XQuartz (show other bugs)
Version: unspecified
Hardware: PowerPC All
: high normal
Assignee: Torrey T. Lyons
QA Contact: Xorg Project Team
Depends on:
Reported: 2004-08-10 18:39 UTC by Torrey T. Lyons
Modified: 2008-05-06 01:17 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:

Patch to make rootless acceleration compatible with Damage (9.15 KB, patch)
2004-08-18 13:16 UTC, Torrey T. Lyons
no flags Details | Splinter Review

Description Torrey T. Lyons 2004-08-10 18:39:42 UTC
Since the most recent damage changes XDarwin crashes frequently in rootless mode. I've seen several 
crashes, but here are two backtraces:

#0  0x001dc958 in damagePaintWindow (pWindow=0x575a8a0, prgn=0xf005fc60, what=1) at 
#1  0x00027abc in Darwin_X_ChangeWindowAttributes (pWin=0x575a8a0, vmask=8, 
vlist=0x2117854, client=0x5758050) at window.c:1499
#2  0x00034cf0 in ProcChangeWindowAttributes (client=0x5758050) at dispatch.c:560
#3  0x00034868 in Dispatch () at dispatch.c:455
#4  0x0000a624 in main (argc=2, argv=0xbffffe6c, envp=0xbffffe78) at main.c:442

This looks like a crash on getWindowDamage (pWindow).


0   org.xfree86.XDarwin      	0x001d67e8 damageDamageRegion + 0x154 (damage.c:139)
1   org.xfree86.XDarwin      	0x001d6e0c damageDamageBox + 0x48 (damage.c:276)
2   org.xfree86.XDarwin      	0x001db834 damageDamageChars + 0x114 (damage.c:1245)
3   org.xfree86.XDarwin      	0x001db9c8 damageText + 0x180 (damage.c:1289)
4   org.xfree86.XDarwin      	0x001dbb78 damagePolyText8 + 0x108 (damage.c:1311)
5   org.xfree86.XDarwin      	0x00031fa4 doPolyText + 0x750 (dixfonts.c:1403)
6   org.xfree86.XDarwin      	0x00032224 PolyText + 0xdc (dixfonts.c:1485)
7   org.xfree86.XDarwin      	0x0003bf60 ProcPolyText + 0x2b0 (dispatch.c:2372)
8   org.xfree86.XDarwin      	0x00034868 Dispatch + 0x338 (dispatch.c:455)
9   org.xfree86.XDarwin      	0x0000a624 main + 0x7b4 (main.c:445)
10  org.xfree86.XDarwin      	0x00088be8 -[XServer run] + 0x9c (XServer.m:848)
11  com.apple.Foundation     	0x90a39b74 forkThreadForFunction + 0x6c
12  libSystem.B.dylib        	0x900246e8 _pthread_body + 0x28

This looks like the problem is that drawableDamage(pDrawable) sets a NULL pDamage. These crashes 
only happen in rootless mode so they must be due to some bad interaction with the generic rootless 
layer. Any ideas to help debug this?
Comment 1 Keith Packard 2004-08-10 19:53:10 UTC
I'd see if GetDrawablePixmap (damage.c:65) returns a pixmap which hasn't been
correctly initialized (no pixmap private allocated).  Is it possible that
DamageSetup hasn't been called?

(at some point, we should be able to use damage and composite to implement
pieces of the rootless code too).
Comment 2 Kevin E. Martin 2004-08-10 21:06:08 UTC
Perhaps bug 961 is related?
Comment 3 Keith Packard 2004-08-11 15:46:01 UTC
I've whacked Damage to deal with DDXen which have no GetWindowPixmap (or where
GetWindowPixmap returns NULL).  Would you check and see if this helps XDarwin at
Comment 4 Torrey T. Lyons 2004-08-11 19:22:01 UTC
Damage still have issues. Some notes:

DamageSetup() is being called; it is called twice actually. It is called first by miSpriteInitialize() and once 
by DamageExtensionInit().

A new backtrace:

#0  0x001d6938 in damageDamageRegion (pDrawable=0x57588a0, pRegion=0xf005fbf0, clip=1) at 
#1  0x001d6f28 in damageDamageBox (pDrawable=0x57588a0, pBox=0xf005fc60) at damage.c:288
#2  0x001da1cc in damagePolyRectangle (pDrawable=0x57588a0, pGC=0x5755560, nRects=1, 
pRects=0x210d40c) at damage.c:1004
#3  0x00039ddc in ProcPolyRectangle (client=0x57541e0) at dispatch.c:1904
#4  0x00034868 in Dispatch () at dispatch.c:455
#5  0x0000a624 in main (argc=2, argv=0xbffffe68, envp=0xbffffe74) at main.c:442

It looks like pDamage is garbage. For this crash it was:

(gdb) print *pDamage
$2 = {
  pNext = 0xa0a0a038, 
  pNextWin = 0xa0a05a18, 
  damage = {
    extents = {
      x1 = -28493, 
      y1 = 15668, 
      x2 = 0, 
      y2 = 0
    data = 0x189
  damageLevel = DamageReportNone, 
  isInternal = 0, 
  closure = 0xc091b0, 
  isWindow = -1870004328, 
  pDrawable = 0x0, 
  damageReport = 0, 
  damageDestroy = 0

Also note the generic rootless layer does try to setup a trivial screen pixmap. See 
RootlessUpdateScreenPixmap() in miext/rootless/rootlessScreen.c for details.

Could there be a problem with the ordering of the wrapping? Rootless wraps screen functions so it can 
put valid window pixmaps in place as they are needed. Subsequent drawing calls to a window are 
grouped together, but between drawing calls groups the window pixmaps are not valid.
Comment 5 Keith Packard 2004-08-11 19:42:48 UTC
Yes, that pointer is clearly garbage...

DamageSetup should be invoked by any subsystem which uses the miext/damage code
-- software cursors use it, the miext/shadow frame buffer code uses it, and both
the Damage and Composite extensions use it.  That function should be dealing
with multiple invocations just fine.

Damage uses pixmap privates to hold a list of clients with interest in damage to
a particular pixmap -- it uses pixmaps so that rendering to any window in the
hierarchy which shares the same pixmap will be noted as damage.

It expects that the pixmap for each window will not change except through calls
to SetWindowPixmap, which it should handle correctly.

I fear you're correct about the initialization order; it's always tricky to get
things layered correctly in the X server, and with multiple sub-systems using
the Damage code, it's probably even harder...

Damage must be initialized *after* all of the rendering functions are set in the
screen structure.  For the Composite extension to work properly, it must be *on
top* of the new Composite Wrapper interface.

If the rootless code is swapping window pixmaps around, it may be that the
damage code needs to be underneath that though, just as it must be underneath
the Composite code (which does the same thing).

A stack trace from something like damageCreateGC might show us where the order
is incorrect.
Comment 6 Torrey T. Lyons 2004-08-12 13:12:10 UTC
Switching the initialization order to make sure DamageSetup() is run before RootlessInit() fixed the 

There is probably a small issue remaining, but it is likely not trivial to fix. The generic rootless layer will 
sometimes call accelerated drawing functions instead of unwrapping and calling GC ops below it. In this 
case, Damage would never know a part of the window had been changed. Probably the best fix is to just 
have the accelerated routines call the corresponding Damage routine directly. Any thoughts?

In any case the major blocker has been fixed. I'm not sure I'll have time to fix this other issue until late 
next week.
Comment 7 Kevin E. Martin 2004-08-13 13:47:58 UTC
The release wranglers decided to leave this as a blocker for now to keep track
of whether the bug can be fixed before the release, but it won't hold up the
release if the fix is too invasive or can't be fixed in time.
Comment 8 Kevin E. Martin 2004-08-16 11:48:53 UTC
Moving to "held open" bug since this is no longer a blocker
Comment 9 Torrey T. Lyons 2004-08-18 13:16:52 UTC
Created attachment 668 [details] [review]
Patch to make rootless acceleration compatible with Damage

This patch makes rootless acceleration compatible with Damage. It does this by
pulling the rootless acceleration wrapping out of the generic rootless layer's
CreateGC function. Rootless acceleration must now be separately initialized
after fb but before DamageSetup(). The patch adds the new initialization
function, RootlessAccelInit() to XDarwin's xpr and cr implementations and to
Cygwin's corresponding screen initialization. Cygwin guys, please check that
everything is kosher for you.
Comment 10 Kevin E. Martin 2004-08-19 13:53:21 UTC
Could the other people who depend on the the rootless layer please test the
patch to see if it works for them and doesn't cause any other problems?  It
would be nice to get this fixed for the release but want to make sure that the
change does not cause other problems.
Comment 11 Torrey T. Lyons 2004-08-20 10:32:07 UTC
Hmpf. Even with the above patch there is at least one place that the damage functions will not get 
called. The rootless screen function for CopyWindow nevers calls to any underlying wrapped 
CopyWindow function. The relevant code is RootlessCopyWindow() in Xserver/miext/rootless/
rootlessWindow.c. One solution is to split RootlessCopyWindow() into two levels of screen function 
wrapping but this is suboptimal and not trivial to get right for this function. Probably the best solution 
is just to call DamageDamageRegion() directly inside RootlessCopyWindow(). Is there anything to watch 
out for? What tests can I use to verify that damage is actually workinig?
Comment 12 Torrey T. Lyons 2004-09-17 17:42:21 UTC
Comment on attachment 668 [details] [review]
Patch to make rootless acceleration compatible with Damage

The patch for rootless acceleration has been applied. RootlessCopyWindow()
still needs to be fixed for Rootless to be completely compatible with Damage.
Comment 13 Adam Jackson 2005-12-09 06:42:41 UTC
aiui Damage is still disabled in the rootless servers, so this isn't a blocker
for 7.
Comment 14 Adam Jackson 2006-04-25 05:34:47 UTC
(In reply to comment #13)
> aiui Damage is still disabled in the rootless servers, so this isn't a blocker
> for 7.

Kicking to 7.2, not strong enough to block 7.1.
Comment 15 Daniel Stone 2006-11-04 10:01:10 UTC
shunting off the radar, since we don't appear to have any more maintained
rootless ddxes.  if someone really cares, a big FIXME should do the trick.
Comment 16 Daniel Stone 2007-02-27 01:23:48 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.

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.