Bug 56940

Summary: x-server segfault in X(CompositeRects)
Product: xorg Reporter: Igor Vagulin <igor.vagulin>
Component: Server/Acceleration/glamorAssignee: Zhigang Gong <zhigang.gong>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: michel
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments:
Description Flags
xorg log with brief backtrace
none
dmesg output
none
xorg.conf
none
xorg.log from x-server 1.12.4
none
bt-with-x-server-debug-info
none
patch to fix compositerects segfault bug none

Description Igor Vagulin 2012-11-09 23:32:31 UTC
Created attachment 69842 [details]
xorg log with brief backtrace

steps to reproduce:
- start x-server with glamor enabled
- start firefox

result:
firefox render blank window just after that x-server crashes

Unfortunately I was unable to generate good backtrace: I don't have another machine to ssh, when I run with NoTrapSignals machine hangs and don't produce crashdump.

some info
gentoo
hd7850
glamor-0.5
xf86-video-ati-7.0
mesa-9.0
libdrm-2.4.40
kernel-3.6.6
Comment 1 Igor Vagulin 2012-11-09 23:32:48 UTC
Created attachment 69843 [details]
dmesg output
Comment 2 Igor Vagulin 2012-11-09 23:33:00 UTC
Created attachment 69844 [details]
xorg.conf
Comment 3 Alex Deucher 2012-11-09 23:33:53 UTC
glamor doesn't work with xserver 1.13 yet.  You'll need to use an older xserver.
Comment 4 Igor Vagulin 2012-11-09 23:51:33 UTC
Created attachment 69845 [details]
xorg.log from x-server 1.12.4

Actually I started with 1.12.2 then checked 1.12.4 then tried 1.13. On 1.12 x-server crashes early on start. xorg log from 1.12.4 attached.
Comment 5 Alex Deucher 2012-11-09 23:56:46 UTC
try building glamor with --enable-glx-tls
Comment 6 Igor Vagulin 2012-11-10 00:31:31 UTC
with --enable-glx-tls xfce renders desktop and machine hangs: mouse cursor doesn't move, ctrl-alt-f1 doesn't switch to console. After restart xorg log is empty.

exact configure command line:
/var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/configure --prefix=/usr --build=i686-pc-linux-gnu --host=i686-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib --disable-dependency-tracking --docdir=/usr/share/doc/glamor-0.5 --enable-shared --disable-static --disable-glamor-gles2 --enable-glx-tls

I will try to connect through ssh from my android device to get better backtrace without enable-glx-tls.
Comment 7 Igor Vagulin 2012-11-10 01:06:05 UTC
Was able to get backtrace.

Program received signal SIGSEGV, Segmentation fault.x08149d02 in ?? ()
(gdb) bt
#0  0x08149d02 in ?? ()
#1  0xf7217e03 in glamor_composite_rectangles (op=3 '\003', dst=0xa6e60d0, color=0xa6a6d2c, num_rects=4, rects=0xa6a6d34)
   at /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/glamor_compositerects.c:270
#2  0x0813da52 in CompositeRects ()
#3  0x08140958 in ?? ()
#4  0x0813e113 in ?? ()
#5  0x08077f65 in ?? ()
#6  0x08065d7a in _start ()
(gdb) frame 1
1  0xf7217e03 in glamor_composite_rectangles (op=3 '\003', dst=0xa6e60d0, color=0xa6a6d2c, num_rects=4, rects=0xa6a6d34)
   at /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/glamor_compositerects.c:270
270     /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/glamor_compositerects.c: Нет такого файла или каталога.
       in /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/glamor_compositerects.c
Comment 8 Igor Vagulin 2012-11-10 02:11:52 UTC
Created attachment 69846 [details]
bt-with-x-server-debug-info
Comment 9 Zhigang Gong 2012-11-12 17:14:14 UTC
could you confirm that when you build mesa, you did use --enable-glx-tls? If not, please try to use that parameter to rebuild mesa.

(In reply to comment #7)
> Was able to get backtrace.
> 
> Program received signal SIGSEGV, Segmentation fault.x08149d02 in ?? ()
> (gdb) bt
> #0  0x08149d02 in ?? ()
> #1  0xf7217e03 in glamor_composite_rectangles (op=3 '\003', dst=0xa6e60d0,
> color=0xa6a6d2c, num_rects=4, rects=0xa6a6d34)
>    at
> /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/
> glamor_compositerects.c:270
> #2  0x0813da52 in CompositeRects ()
> #3  0x08140958 in ?? ()
> #4  0x0813e113 in ?? ()
> #5  0x08077f65 in ?? ()
> #6  0x08065d7a in _start ()
> (gdb) frame 1
> 1  0xf7217e03 in glamor_composite_rectangles (op=3 '\003', dst=0xa6e60d0,
> color=0xa6a6d2c, num_rects=4, rects=0xa6a6d34)
>    at
> /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/
> glamor_compositerects.c:270
> 270    
> /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/
> glamor_compositerects.c: Нет такого файла или каталога.
>        in
> /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/
> glamor_compositerects.c
Comment 10 Igor Vagulin 2012-11-13 00:10:59 UTC
I am sure enable-glx-tls is used. Below is exact configure cmd line.
./configure --prefix=/usr --build=i686-pc-linux-gnu --host=i686-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib --disable-dependency-tracking --enable-dri --enable-glx --enable-texture-float --disable-debug --enable-egl --enable-gbm --disable-gles1 --disable-gles2 --enable-glx-tls --disable-osmesa --enable-asm --enable-shared-glapi --disable-xa --disable-xorg --with-dri-drivers= --with-gallium-drivers=,swrast,radeonsi,r300,r600,swrast,radeonsi,r300,r600,swrast,radeonsi,r300,r600 --with-egl-platforms=x11,drm --enable-gallium-egl --disable-gallium-g3dvl --enable-gallium-llvm --disable-openvg --disable-r600-llvm-compiler --disable-vdpau --disable-xvmc

I cant understand why there is so much attention to enable-glx-tls. For me bug is pretty obvious: local region not initialized in fallback mode, but used in DamageRegionAppend. 

look like following patch fixes problem for me:
ivagulin ~ # diff -u glamor-0.5/src/glamor_compositerects.c /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/glamor_compositerects.c
--- glamor-0.5/src/glamor_compositerects.c	2012-08-10 09:46:42.000000000 +0400
+++ /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/glamor_compositerects.c	2012-11-13 03:50:25.838203549 +0400
@@ -261,15 +261,17 @@
 				goto done;
 		}
 	}
-fallback:
-	miCompositeRects(op, dst, color, num_rects, rects);
-done:
+
 	/* XXX xserver-1.8: CompositeRects is not tracked by Damage, so we must
 	 * manually append the damaged regions ourselves.
 	 */
 	DamageRegionAppend(&pixmap->drawable, &region);
 	DamageRegionProcessPending(&pixmap->drawable);
 
+fallback:
+	miCompositeRects(op, dst, color, num_rects, rects);
+done:
+
 	if (need_free_region)
 		pixman_region_fini(&region);
 	if (source)
Comment 11 Zhigang Gong 2012-11-13 02:14:46 UTC
(In reply to comment #10)
> I am sure enable-glx-tls is used. Below is exact configure cmd line.
> ./configure --prefix=/usr --build=i686-pc-linux-gnu --host=i686-pc-linux-gnu
> --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share
> --sysconfdir=/etc --localstatedir=/var/lib --disable-dependency-tracking
> --enable-dri --enable-glx --enable-texture-float --disable-debug
> --enable-egl --enable-gbm --disable-gles1 --disable-gles2 --enable-glx-tls
> --disable-osmesa --enable-asm --enable-shared-glapi --disable-xa
> --disable-xorg --with-dri-drivers=
> --with-gallium-drivers=,swrast,radeonsi,r300,r600,swrast,radeonsi,r300,r600,
> swrast,radeonsi,r300,r600 --with-egl-platforms=x11,drm --enable-gallium-egl
> --disable-gallium-g3dvl --enable-gallium-llvm --disable-openvg
> --disable-r600-llvm-compiler --disable-vdpau --disable-xvmc
> 
> I cant understand why there is so much attention to enable-glx-tls. For me
The reason is not very obvious, as glamor is in xserver process and has a dri loader for it, glx(xserver side) also haas a dri loader. And by default, glx (xserver side) is built by tls enabled, but mesa is not, so that will cause segfault when you call gl functions.
> bug is pretty obvious: local region not initialized in fallback mode, but
> used in DamageRegionAppend. 
That's the point. Your case hit a fallback path, and the region is not initialized.
> 
> look like following patch fixes problem for me:
> ivagulin ~ # diff -u glamor-0.5/src/glamor_compositerects.c
> /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/
> glamor_compositerects.c
> --- glamor-0.5/src/glamor_compositerects.c	2012-08-10 09:46:42.000000000
> +0400
> +++
> /var/tmp/portage/x11-libs/glamor-0.5/work/glamor-0.5/src/
> glamor_compositerects.c	2012-11-13 03:50:25.838203549 +0400
> @@ -261,15 +261,17 @@
>  				goto done;
>  		}
>  	}
> -fallback:
> -	miCompositeRects(op, dst, color, num_rects, rects);
> -done:
> +
>  	/* XXX xserver-1.8: CompositeRects is not tracked by Damage, so we must
>  	 * manually append the damaged regions ourselves.
>  	 */
>  	DamageRegionAppend(&pixmap->drawable, &region);
>  	DamageRegionProcessPending(&pixmap->drawable);
>  
> +fallback:
> +	miCompositeRects(op, dst, color, num_rects, rects);
> +done:
> +
>  	if (need_free_region)
>  		pixman_region_fini(&region);
>  	if (source)
The patch is not totally correct. We need to call DamageRegionAppend anyway and the reason is according to the comments above. I refine the patch slightly and will attach it below, could you test it?
Comment 12 Zhigang Gong 2012-11-13 02:15:41 UTC
Created attachment 69975 [details] [review]
patch to fix compositerects segfault bug
Comment 13 Igor Vagulin 2012-11-13 03:10:19 UTC
Segfault is not reproduced with your patch. Thank you.
Comment 14 Zhigang Gong 2012-11-13 03:24:12 UTC
I just pushed the patch, and mark this bug as fixed.

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.