Comment 1 Mark Linimon 2013-06-17 00:57:46 UTC

Responsible Changed
From-To: freebsd-amd64->freebsd-x11

reclassify and assign.

Comment 2 Dimitry Andric 2013-06-17 12:13:40 UTC

As Jung-uk Kim points out in his reply to the thread about this bug on
-current <http://docs.freebsd.org/cgi/mid.cgi?51BB5F3A.60504>, this may
be due to a clang optimizer bug, specifically LLVM PR 16130
<http://llvm.org/bugs/show_bug.cgi?id=16130>.

Can you please verify it is exactly this bug, by applying the attached
patch to your r249781 tree (I assume you have not yet upgraded), then
rebuild clang by doing:

   cd /usr/src/lib/clang && make
   cd /usr/src/usr.bin/clang && make && sudo make install

After the patched clang has been installed, rebuild your xorg-server
without the USE_GCC=any setting, and check if the startup problem was
resolved.

-Dimitry

The patch applied cleanly (yes, I'm still on r249781).
The commands you wrote also run with no problems.
However, rebuilding xorg-server with patched clang,
and removing USE_GCC, did not help. The X error
is still the same:

(EE) Jun 17 12:51:23 NVIDIA(0): Failed to initialize default colormap
(EE) NVIDIA(0):  *** Aborting ***

Fatal server error:
AddScreen/ScreenInit failed for driver 0

Anton

I too have this issue.

When xorg-server is compiled with clang, the following appears in /var/log/Xorg.0.log:

----------------------------------------------------------------------
<everything looks normal up to this point>
(II) VBoxVideo(0): Using default gamma of (1.0, 1.0, 1.0) unless 
otherwise stated.
(II) VBoxVideo(0): RandR 1.2 enabled, ignore the following RandR disabled 
message.

Fatal server error:
AddScreen/ScreenInit failed for driver 0


Please consult the The X.Org Foundation support 
	 at http://wiki.x.org
 for help. 
Please also check the log file at "/var/log/Xorg.0.log" for additional 
information.

Segmentation fault at address 0x290
----------------------------------------------------------------------
This is in a VM, which reports the graphics adapter as:

vgapci0@pci0:0:2:0:	class=0x030000 card=0x00000000 chip=0xbeef80ee 
rev=0x00 hdr=0x00
    vendor     = 'InnoTek Systemberatung GmbH'
    device     = 'VirtualBox Graphics Adapter'
    class      = display
    subclass   = VGA

Comment 5 Dimitry Andric 2013-06-17 22:18:43 UTC

On Jun 17, 2013, at 14:00, Anton Shterenlikht <mexas@bris.ac.uk> wrote:
> The patch applied cleanly (yes, I'm still on r249781).
> The commands you wrote also run with no problems.
> However, rebuilding xorg-server with patched clang,
> and removing USE_GCC, did not help. The X error
> is still the same:
> 
> (EE) Jun 17 12:51:23 NVIDIA(0): Failed to initialize default colormap
> (EE) NVIDIA(0):  *** Aborting ***
> 
> Fatal server error:
> AddScreen/ScreenInit failed for driver 0


Okay, so the optimizer was not the issue then. :-)

Luckily I was able to reproduce your error, after realizing you were
using the old X.org server (version 1.7.7).  As far as I can tell, this
is the following bug:

  https://bugs.freedesktop.org/show_bug.cgi?id=18451

which is fixed by this commit:

  http://cgit.freedesktop.org/xorg/xserver/commit/?id=6dae7f3792611aace1df0cca63bf50c50d93de43

I'm attaching a diff for our x11-servers/xorg-server port, can you
please try it?  E.g. from the root of your ports tree, run:

  patch -p0 -f -F0 -i x11-servers__xorg-server__fix-xace-callback-1.diff

which should modify x11-servers/xorg-server/Makefile, and add an
additional patch (extra-Xext-xace.c) to the files subdirectory.  Then
rebuild and reinstall the xorg-server port.

-Dimitry

Comment 6 Jung-uk Kim 2013-06-17 22:46:37 UTC

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2013-06-17 17:18:43 -0400, Dimitry Andric wrote:
> Luckily I was able to reproduce your error, after realizing you
> were using the old X.org server (version 1.7.7).  As far as I can
> tell, this is the following bug:
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=18451
> 
> which is fixed by this commit:
> 
> http://cgit.freedesktop.org/xorg/xserver/commit/?id=6dae7f3792611aace1df0cca63bf50c50d93de43
...

Yes,
> 
that did it!  Thanks for the nice detective work!

Jung-uk Kim
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.20 (FreeBSD)

iQEcBAEBAgAGBQJRv4O9AAoJECXpabHZMqHOqHoIAIzq09axU9KP+OapYffHhM0G
LrBSLQt0eBUzAKeNAbxGCh243zmvSv8GKt503JQf01IOtUlf1C3nh2htKiskH0Gw
5Y2hxU8obv4dQqhj2VfM2N3YshNMIwzVv+BRuR+jRZXwkO6AzhG3QetbS4j24yhl
QgoU/9xKznqK3DG/dMS1UhiPSIBXEtPJQydQJqENOFNWF1vfLLFnyeRqlh9/CXxn
1EsHm+Vck1c/HOID0v7yajjBazOQmlSVNqmX6YPDsaDqa4OeZ9OKFAXM9bk2Fcjf
xXPJWiFBxHrgdVx01X2zdPSRut4pxfIBXXC70fMy2/wwdxauxgOmEMWbNkJOPm4=
=Fm1K
-----END PGP SIGNATURE-----

Comment 7 dfilter service 2013-06-17 22:51:57 UTC

Author: jkim
Date: Mon Jun 17 21:51:48 2013
New Revision: 321157
URL: http://svnweb.freebsd.org/changeset/ports/321157

Log:
  Add a patch to fix upstream bug 18451 for 1.7.7.
  
  https://bugs.freedesktop.org/show_bug.cgi?id=18451
  http://cgit.freedesktop.org/xorg/xserver/patch/?id=6dae7f3
  
  Analyzed by:	dim
  PR:		ports/179625

Added:
  head/x11-servers/xorg-server/files/extra-Xext-xace.c   (contents, props changed)
Modified:
  head/x11-servers/xorg-server/Makefile

Modified: head/x11-servers/xorg-server/Makefile
==============================================================================
--- head/x11-servers/xorg-server/Makefile	Mon Jun 17 21:37:24 2013	(r321156)
+++ head/x11-servers/xorg-server/Makefile	Mon Jun 17 21:51:48 2013	(r321157)
@@ -27,12 +27,13 @@ PLIST_SUB+=	OLD="@comment " NEW=""
 EXTRA_PATCHES+=	${FILESDIR}/extra-clang
 .else
 XORG_VERSION=	1.7.7
-XORG_REVISION=	6
+XORG_REVISION=	7
 PLIST_SUB+=	OLD="" NEW="@comment "
-EXTRA_PATCHES+=	${FILESDIR}/extra-include_eventstr.h \
-		${FILESDIR}/extra-os-utils.c \
+EXTRA_PATCHES+=	${FILESDIR}/extra-Xext-xace.c \
 		${FILESDIR}/extra-Xserver-hw-xfree86-os-support-bsd-sparc64_video.c \
 		${FILESDIR}/extra-Xserver-os-xprintf.c \
+		${FILESDIR}/extra-include_eventstr.h \
+		${FILESDIR}/extra-os-utils.c \
 		${FILESDIR}/extra-servermd.h
 .endif
 

Added: head/x11-servers/xorg-server/files/extra-Xext-xace.c
==============================================================================
--- /dev/null	00:00:00 1970	(empty, because file is newly added)
+++ head/x11-servers/xorg-server/files/extra-Xext-xace.c	Mon Jun 17 21:51:48 2013	(r321157)
@@ -0,0 +1,229 @@
+From 6dae7f3792611aace1df0cca63bf50c50d93de43 Mon Sep 17 00:00:00 2001
+From: Chris Wilson <chris@chris-wilson.co.uk>
+Date: Tue, 10 Aug 2010 18:30:20 +0000
+Subject: xace: Invalid reference to out-of-scope data.
+
+The callback data passed by reference to the hook was allocated on stack
+within the scope of the case statement. The compiler is free to reuse
+any of that stack space whilst making the function call so we may end up
+passing garbage into the callback.
+
+References:
+
+  Bug 18451 - Xorg server 1.5.2 SEGV during XFixesGetCursorImage()
+  https://bugs.freedesktop.org/show_bug.cgi?id=18451
+
+v2: Drop the unrelated hunk that snuck in when ammending the commit
+message.
+
+Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
+Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
+Signed-off-by: Keith Packard <keithp@keithp.com>
+---
+diff --git a/Xext/xace.c b/Xext/xace.c
+index e10d837..c757cad 100644
+--- Xext/xace.c
++++ Xext/xace.c
+@@ -87,7 +87,18 @@ void XaceHookAuditEnd(ClientPtr ptr, int result)
+  */
+ int XaceHook(int hook, ...)
+ {
+-    pointer calldata;	/* data passed to callback */
++    union {
++	XaceResourceAccessRec res;
++	XaceDeviceAccessRec dev;
++	XaceSendAccessRec send;
++	XaceReceiveAccessRec recv;
++	XaceClientAccessRec client;
++	XaceExtAccessRec ext;
++	XaceServerAccessRec server;
++	XaceScreenAccessRec screen;
++	XaceAuthAvailRec auth;
++	XaceKeyAvailRec key;
++    } u;
+     int *prv = NULL;	/* points to return value from callback */
+     va_list ap;		/* argument list */
+     va_start(ap, hook);
+@@ -99,117 +110,86 @@ int XaceHook(int hook, ...)
+      */
+     switch (hook)
+     {
+-	case XACE_RESOURCE_ACCESS: {
+-	    XaceResourceAccessRec rec;
+-	    rec.client = va_arg(ap, ClientPtr);
+-	    rec.id = va_arg(ap, XID);
+-	    rec.rtype = va_arg(ap, RESTYPE);
+-	    rec.res = va_arg(ap, pointer);
+-	    rec.ptype = va_arg(ap, RESTYPE);
+-	    rec.parent = va_arg(ap, pointer);
+-	    rec.access_mode = va_arg(ap, Mask);
+-	    rec.status = Success; /* default allow */
+-	    calldata = &rec;
+-	    prv = &rec.status;
++	case XACE_RESOURCE_ACCESS:
++	    u.res.client = va_arg(ap, ClientPtr);
++	    u.res.id = va_arg(ap, XID);
++	    u.res.rtype = va_arg(ap, RESTYPE);
++	    u.res.res = va_arg(ap, pointer);
++	    u.res.ptype = va_arg(ap, RESTYPE);
++	    u.res.parent = va_arg(ap, pointer);
++	    u.res.access_mode = va_arg(ap, Mask);
++	    u.res.status = Success; /* default allow */
++	    prv = &u.res.status;
+ 	    break;
+-	}
+-	case XACE_DEVICE_ACCESS: {
+-	    XaceDeviceAccessRec rec;
+-	    rec.client = va_arg(ap, ClientPtr);
+-	    rec.dev = va_arg(ap, DeviceIntPtr);
+-	    rec.access_mode = va_arg(ap, Mask);
+-	    rec.status = Success; /* default allow */
+-	    calldata = &rec;
+-	    prv = &rec.status;
++	case XACE_DEVICE_ACCESS:
++	    u.dev.client = va_arg(ap, ClientPtr);
++	    u.dev.dev = va_arg(ap, DeviceIntPtr);
++	    u.dev.access_mode = va_arg(ap, Mask);
++	    u.dev.status = Success; /* default allow */
++	    prv = &u.dev.status;
+ 	    break;
+-	}
+-	case XACE_SEND_ACCESS: {
+-	    XaceSendAccessRec rec;
+-	    rec.client = va_arg(ap, ClientPtr);
+-	    rec.dev = va_arg(ap, DeviceIntPtr);
+-	    rec.pWin = va_arg(ap, WindowPtr);
+-	    rec.events = va_arg(ap, xEventPtr);
+-	    rec.count = va_arg(ap, int);
+-	    rec.status = Success; /* default allow */
+-	    calldata = &rec;
+-	    prv = &rec.status;
++	case XACE_SEND_ACCESS:
++	    u.send.client = va_arg(ap, ClientPtr);
++	    u.send.dev = va_arg(ap, DeviceIntPtr);
++	    u.send.pWin = va_arg(ap, WindowPtr);
++	    u.send.events = va_arg(ap, xEventPtr);
++	    u.send.count = va_arg(ap, int);
++	    u.send.status = Success; /* default allow */
++	    prv = &u.send.status;
+ 	    break;
+-	}
+-	case XACE_RECEIVE_ACCESS: {
+-	    XaceReceiveAccessRec rec;
+-	    rec.client = va_arg(ap, ClientPtr);
+-	    rec.pWin = va_arg(ap, WindowPtr);
+-	    rec.events = va_arg(ap, xEventPtr);
+-	    rec.count = va_arg(ap, int);
+-	    rec.status = Success; /* default allow */
+-	    calldata = &rec;
+-	    prv = &rec.status;
++	case XACE_RECEIVE_ACCESS:
++	    u.recv.client = va_arg(ap, ClientPtr);
++	    u.recv.pWin = va_arg(ap, WindowPtr);
++	    u.recv.events = va_arg(ap, xEventPtr);
++	    u.recv.count = va_arg(ap, int);
++	    u.recv.status = Success; /* default allow */
++	    prv = &u.recv.status;
+ 	    break;
+-	}
+-	case XACE_CLIENT_ACCESS: {
+-	    XaceClientAccessRec rec;
+-	    rec.client = va_arg(ap, ClientPtr);
+-	    rec.target = va_arg(ap, ClientPtr);
+-	    rec.access_mode = va_arg(ap, Mask);
+-	    rec.status = Success; /* default allow */
+-	    calldata = &rec;
+-	    prv = &rec.status;
++	case XACE_CLIENT_ACCESS:
++	    u.client.client = va_arg(ap, ClientPtr);
++	    u.client.target = va_arg(ap, ClientPtr);
++	    u.client.access_mode = va_arg(ap, Mask);
++	    u.client.status = Success; /* default allow */
++	    prv = &u.client.status;
+ 	    break;
+-	}
+-	case XACE_EXT_ACCESS: {
+-	    XaceExtAccessRec rec;
+-	    rec.client = va_arg(ap, ClientPtr);
+-	    rec.ext = va_arg(ap, ExtensionEntry*);
+-	    rec.access_mode = DixGetAttrAccess;
+-	    rec.status = Success; /* default allow */
+-	    calldata = &rec;
+-	    prv = &rec.status;
++	case XACE_EXT_ACCESS:
++	    u.ext.client = va_arg(ap, ClientPtr);
++	    u.ext.ext = va_arg(ap, ExtensionEntry*);
++	    u.ext.access_mode = DixGetAttrAccess;
++	    u.ext.status = Success; /* default allow */
++	    prv = &u.ext.status;
+ 	    break;
+-	}
+-	case XACE_SERVER_ACCESS: {
+-	    XaceServerAccessRec rec;
+-	    rec.client = va_arg(ap, ClientPtr);
+-	    rec.access_mode = va_arg(ap, Mask);
+-	    rec.status = Success; /* default allow */
+-	    calldata = &rec;
+-	    prv = &rec.status;
++	case XACE_SERVER_ACCESS:
++	    u.server.client = va_arg(ap, ClientPtr);
++	    u.server.access_mode = va_arg(ap, Mask);
++	    u.server.status = Success; /* default allow */
++	    prv = &u.server.status;
+ 	    break;
+-	}
+ 	case XACE_SCREEN_ACCESS:
+-	case XACE_SCREENSAVER_ACCESS: {
+-	    XaceScreenAccessRec rec;
+-	    rec.client = va_arg(ap, ClientPtr);
+-	    rec.screen = va_arg(ap, ScreenPtr);
+-	    rec.access_mode = va_arg(ap, Mask);
+-	    rec.status = Success; /* default allow */
+-	    calldata = &rec;
+-	    prv = &rec.status;
++	case XACE_SCREENSAVER_ACCESS:
++	    u.screen.client = va_arg(ap, ClientPtr);
++	    u.screen.screen = va_arg(ap, ScreenPtr);
++	    u.screen.access_mode = va_arg(ap, Mask);
++	    u.screen.status = Success; /* default allow */
++	    prv = &u.screen.status;
+ 	    break;
+-	}
+-	case XACE_AUTH_AVAIL: {
+-	    XaceAuthAvailRec rec;
+-	    rec.client = va_arg(ap, ClientPtr);
+-	    rec.authId = va_arg(ap, XID);
+-	    calldata = &rec;
++	case XACE_AUTH_AVAIL:
++	    u.auth.client = va_arg(ap, ClientPtr);
++	    u.auth.authId = va_arg(ap, XID);
+ 	    break;
+-	}
+-	case XACE_KEY_AVAIL: {
+-	    XaceKeyAvailRec rec;
+-	    rec.event = va_arg(ap, xEventPtr);
+-	    rec.keybd = va_arg(ap, DeviceIntPtr);
+-	    rec.count = va_arg(ap, int);
+-	    calldata = &rec;
++	case XACE_KEY_AVAIL:
++	    u.key.event = va_arg(ap, xEventPtr);
++	    u.key.keybd = va_arg(ap, DeviceIntPtr);
++	    u.key.count = va_arg(ap, int);
+ 	    break;
+-	}
+-	default: {
++	default:
+ 	    va_end(ap);
+ 	    return 0;	/* unimplemented hook number */
+-	}
+     }
+     va_end(ap);
+  
+     /* call callbacks and return result, if any. */
+-    CallCallbacks(&XaceHooks[hook], calldata);
++    CallCallbacks(&XaceHooks[hook], &u);
+     return prv ? *prv : Success;
+ }
+ 
+--
+cgit v0.9.0.2-2-gbebe
_______________________________________________
svn-ports-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-ports-all
To unsubscribe, send any mail to "svn-ports-all-unsubscribe@freebsd.org"

Comment 8 Jung-uk Kim 2013-06-17 22:54:15 UTC

State Changed
From-To: open->closed

Committed (r321157), thanks!

Yes, this fixed the issue for me too, thank you.

Shall I revert the clang patch?

Thanks

Anton