Bug 97974 - Xwayland crash in glamor code path
Summary: Xwayland crash in glamor code path
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/Acceleration/glamor (show other bugs)
Version: git
Hardware: Other All
: medium normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-29 12:50 UTC by Olivier Fourdan
Modified: 2016-10-06 13:04 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Negate dst_x/off for fbCopy*to* (834 bytes, patch)
2016-09-30 08:57 UTC, Michel Dänzer
no flags Details | Splinter Review
[PATCH xserver] glamor: Fix pixmap offset for bitplane in glamor_copy_fbo_cpu (1.98 KB, patch)
2016-10-04 11:22 UTC, Olivier Fourdan
no flags Details | Splinter Review

Description Olivier Fourdan 2016-09-29 12:50:47 UTC
Decription:

Updated to the latest xserver code from git master (curently at commit 7a5ddf8 - test: Re-enable a couple of GetImage tests) and found a repeatable crash in Xwayland.

How reproducible:

100%

Steps to reproduce:

1. Run weston nested
2. Run xterm
3. Open a menu (Ctrl + left click)

Actual result:

Xwayland segfaults

Expected result:

No crash

Additional info:

I file this in Xorg's server/acceleration/glamopr as I find nothing specific to Xwayland in the backtrace, but it would as well be Xwayland code. Also I see no change in fb code since 2012 (according to git log) so I'd rule out a recent regression in this area.

(gdb) bt
#0  0x00000000004b2304 in fbBltOne (src=0x16f0f54, srcStride=<optimized out>, srcStride@entry=1, srcX=23, dst=0x1d35004, dstStride=220, dstStride@entry=229, 
    dstX=<optimized out>, dstBpp=32, width=288, height=7, fgand=0, fgxor=0, bgand=0, bgxor=16777215) at fbbltone.c:334
#1  0x000000000049e8d7 in fbCopy1toN (pSrcDrawable=pSrcDrawable@entry=0x16f1240, pDstDrawable=pDstDrawable@entry=0x1cceeb0, pGC=pGC@entry=0xed5c90, 
    pbox=pbox@entry=0x7ffd6d75ce50, nbox=<optimized out>, nbox@entry=1, dx=-551, dy=-687, reverse=0, upsidedown=0, bitplane=1, closure=0x0) at fbcopy.c:123
#2  0x00000000004830c1 in glamor_copy_cpu_fbo (closure=0x0, bitplane=1, upsidedown=0, reverse=0, dy=-455, dx=-286, nbox=1, box=0x7ffd6d75ce50, gc=0xed5c90, dst=0xed5c90, 
    src=0x16f1240) at glamor_copy.c:241
#3  glamor_copy_gl (closure=0x0, bitplane=1, upsidedown=0, reverse=0, dy=-455, dx=<optimized out>, nbox=1, box=0x7ffd6d75ce50, gc=0xed5c90, dst=0xed5c90, src=0x16f1240)
    at glamor_copy.c:649
#4  glamor_copy (src=src@entry=0x16f1240, dst=dst@entry=0x1a98280, gc=gc@entry=0xed5c90, box=box@entry=0x7ffd6d75ce50, nbox=nbox@entry=1, dx=-286, dy=-455, reverse=0, 
    upsidedown=0, bitplane=1, closure=0x0) at glamor_copy.c:676
#5  0x00000000004c1f4c in miCopyRegion (pSrcDrawable=pSrcDrawable@entry=0x16f1240, pDstDrawable=pDstDrawable@entry=0x1a98280, pGC=pGC@entry=0xed5c90, 
    pDstRegion=pDstRegion@entry=0x7ffd6d75ce50, dx=dx@entry=-286, dy=dy@entry=-455, copyProc=0x482530 <glamor_copy>, bitPlane=1, closure=0x0) at micopy.c:121
#6  0x00000000004c24cc in miDoCopy (pSrcDrawable=0x16f1240, pDstDrawable=0x1a98280, pGC=0xed5c90, xIn=0, yIn=0, widthSrc=9, heightSrc=8, xOut=286, yOut=455, 
    copyProc=0x482530 <glamor_copy>, bitPlane=1, closure=0x0) at micopy.c:296
#7  0x00000000004831ce in glamor_copy_plane (src=<optimized out>, dst=<optimized out>, gc=<optimized out>, srcx=<optimized out>, srcy=<optimized out>, 
    width=<optimized out>, height=8, dstx=19, dsty=221, bitplane=1) at glamor_copy.c:698
#8  0x000000000054354f in damageCopyPlane (pSrc=0x16f1240, pDst=0x1a98280, pGC=0xed5c90, srcx=0, srcy=<optimized out>, width=9, height=8, dstx=19, dsty=221, bitPlane=1)
    at damage.c:799
#9  0x0000000000436bcc in ProcCopyPlane (client=0x189be00) at dispatch.c:1758
#10 0x000000000043a506 in Dispatch () at dispatch.c:469
#11 0x000000000043e4f8 in dix_main (argc=10, argv=0x7ffd6d75d178, envp=<optimized out>) at main.c:287
#12 0x00007f4f2c5f9401 in __libc_start_main (main=0x423050 <main>, argc=10, argv=0x7ffd6d75d178, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, 
    stack_end=0x7ffd6d75d168) at ../csu/libc-start.c:289
#13 0x000000000042308a in _start ()


(gdb) bt full
#0  0x00000000004b2304 in fbBltOne (src=0x16f0f54, srcStride=<optimized out>, srcStride@entry=1, srcX=23, dst=0x1d35004, dstStride=220, dstStride@entry=229, 
    dstX=<optimized out>, dstBpp=32, width=288, height=7, fgand=0, fgxor=0, bgand=0, bgxor=16777215) at fbbltone.c:334
        fbBits = 0x59fa30 <fbStipple1Bits>
        srcEnd = <optimized out>
        pixelsPerDst = 1
        unitsPerSrc = 32
        leftShift = 23
        rightShift = 9
        startmask = 0
        endmask = 0
        bits = <optimized out>
        bitsLeft = 16
        bitsRight = <optimized out>
        left = <optimized out>
        mask = <optimized out>
        nDst = 9
        w = 0
        n = <optimized out>
        nmiddle = <optimized out>
        dstS = 0
        copy = <optimized out>
        transparent = <optimized out>
        srcinc = <optimized out>
        endNeedsLoad = 0
        startbyte = 0
        endbyte = 0
#1  0x000000000049e8d7 in fbCopy1toN (pSrcDrawable=pSrcDrawable@entry=0x16f1240, pDstDrawable=pDstDrawable@entry=0x1cceeb0, pGC=pGC@entry=0xed5c90, 
    pbox=pbox@entry=0x7ffd6d75ce50, nbox=<optimized out>, nbox@entry=1, dx=-551, dy=-687, reverse=0, upsidedown=0, bitplane=1, closure=0x0) at fbcopy.c:123
        src = 0x16f1310
        srcStride = 1
        srcBpp = 1
        srcXoff = 0
        srcYoff = 0
        dst = 0x1ccef80
        dstStride = 229
        dstBpp = 32
        dstXoff = 0
        dstYoff = 0
#2  0x00000000004830c1 in glamor_copy_cpu_fbo (closure=0x0, bitplane=1, upsidedown=0, reverse=0, dy=-455, dx=-286, nbox=1, box=0x7ffd6d75ce50, gc=0xed5c90, dst=0xed5c90, 
    src=0x16f1240) at glamor_copy.c:241
        src_pix = 0x1cceeb0
        src_stride = 229
        src_yoff = <optimized out>
        screen = <optimized out>
        src_bits = 0x1ccef80
        dst_xoff = -265
        dst_pixmap = 0x1a98520
        src_bpp = <optimized out>
        src_xoff = <optimized out>
        dst_yoff = -232
---Type <return> to continue, or q <return> to quit---
#3  glamor_copy_gl (closure=0x0, bitplane=1, upsidedown=0, reverse=0, dy=-455, dx=<optimized out>, nbox=1, box=0x7ffd6d75ce50, gc=0xed5c90, dst=0xed5c90, src=0x16f1240)
    at glamor_copy.c:649
        src_pixmap = <optimized out>
        dst_pixmap = <optimized out>
#4  glamor_copy (src=src@entry=0x16f1240, dst=dst@entry=0x1a98280, gc=gc@entry=0xed5c90, box=box@entry=0x7ffd6d75ce50, nbox=nbox@entry=1, dx=-286, dy=-455, reverse=0, 
    upsidedown=0, bitplane=1, closure=0x0) at glamor_copy.c:676
        nbox = 1
        box = 0x7ffd6d75ce50
        closure = 0x0
        reverse = 0
        dx = <optimized out>
        gc = 0xed5c90
        src = 0x16f1240
        bitplane = 1
        upsidedown = 0
        dy = -455
        dst = 0x1a98280
#5  0x00000000004c1f4c in miCopyRegion (pSrcDrawable=pSrcDrawable@entry=0x16f1240, pDstDrawable=pDstDrawable@entry=0x1a98280, pGC=pGC@entry=0xed5c90, 
    pDstRegion=pDstRegion@entry=0x7ffd6d75ce50, dx=dx@entry=-286, dy=dy@entry=-455, copyProc=0x482530 <glamor_copy>, bitPlane=1, closure=0x0) at micopy.c:121
        careful = <optimized out>
        reverse = <optimized out>
        upsidedown = <optimized out>
        pbox = 0x7ffd6d75ce50
        pboxNew1 = <optimized out>
        pboxNew2 = 0x0
        pboxBase = <optimized out>
        pboxNext = <optimized out>
        pboxTmp = <optimized out>
#6  0x00000000004c24cc in miDoCopy (pSrcDrawable=0x16f1240, pDstDrawable=0x1a98280, pGC=0xed5c90, xIn=0, yIn=0, widthSrc=9, heightSrc=8, xOut=286, yOut=455, 
    copyProc=0x482530 <glamor_copy>, bitPlane=1, closure=0x0) at micopy.c:296
        prgnSrcClip = 0x0
        freeSrcClip = 0
        prgnExposed = 0x0
        rgnDst = {extents = {x1 = 286, y1 = 455, x2 = 295, y2 = 463}, data = 0x0}
        dx = -286
        dy = -455
        box_x1 = <optimized out>
        box_y1 = <optimized out>
        box_x2 = <optimized out>
        box_y2 = <optimized out>
        fastSrc = <optimized out>
        fastDst = <optimized out>
        fastExpose = <optimized out>
#7  0x00000000004831ce in glamor_copy_plane (src=<optimized out>, dst=<optimized out>, gc=<optimized out>, srcx=<optimized out>, srcy=<optimized out>, 
    width=<optimized out>, height=8, dstx=19, dsty=221, bitplane=1) at glamor_copy.c:698
No locals.
#8  0x000000000054354f in damageCopyPlane (pSrc=0x16f1240, pDst=0x1a98280, pGC=0xed5c90, srcx=0, srcy=<optimized out>, width=9, height=8, dstx=19, dsty=221, bitPlane=1)
    at damage.c:799
        ret = <optimized out>
        pDamage = 0x16f1c90
---Type <return> to continue, or q <return> to quit---
        oldFuncs = 0x7fff00 <damageGCFuncs>
#9  0x0000000000436bcc in ProcCopyPlane (client=0x189be00) at dispatch.c:1758
        psrcDraw = 0x16f1240
        pdstDraw = 0x1a98280
        pGC = 0xed5c90
        stuff = <optimized out>
        pRgn = <optimized out>
        rc = <optimized out>
#10 0x000000000043a506 in Dispatch () at dispatch.c:469
        result = <optimized out>
        start_tick = 55
#11 0x000000000043e4f8 in dix_main (argc=10, argv=0x7ffd6d75d178, envp=<optimized out>) at main.c:287
        i = <optimized out>
        alwaysCheckForInput = {0, 1}
#12 0x00007f4f2c5f9401 in __libc_start_main (main=0x423050 <main>, argc=10, argv=0x7ffd6d75d178, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, 
    stack_end=0x7ffd6d75d168) at ../csu/libc-start.c:289
        result = <optimized out>
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {0, 1201675685679621017, 4337760, 140726439891312, 0, 0, -1200226956092087399, -1282560909969300583}, 
              mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x7ffd6d75d1d0, 0x7f4f3235d128}, data = {prev = 0x0, cleanup = 0x0, canceltype = 1836437968}}}
        not_first_call = <optimized out>
#13 0x000000000042308a in _start ()
Comment 1 Olivier Fourdan 2016-09-29 12:54:58 UTC
The commit that introduced this regression is:

commit cba28d572ac799391beacd89d57e69d0d7ed70e7
Author: Michel Dänzer <michel.daenzer@amd.com>
Date:   Tue Jul 12 12:25:13 2016 +0900

    glamor: Handle bitplane in glamor_copy_fbo_cpu
    
    This can significantly speed up at least some CopyPlane cases, e.g.
    indirectly for stippled fills.
    
    v2:
    * Make temporary pixmap the same size as the destination pixmap
      (instead of the destination drawable size), and fix coordinate
      parameters passed to fbCopyXtoX and glamor_upload_boxes. Fixes
      incorrect rendering with x11perf -copyplane* and crashes with the
      xscreensaver phosphor hack.
    v3:
    * Make the change a bit more compact and hopefully more readable by
      re-using the existing src_* locals in the bitplane case as well.
    
    Reported-by: Keith Raghubar <keith.raghubar@amd.com>
    Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
    Acked-by: Eric Anholt <eric@anholt.net>

Reverting this commit makes the rpoblem go away.
Comment 2 Olivier Fourdan 2016-09-29 12:56:38 UTC
=> Adding Michel in CC.
Comment 3 Michel Dänzer 2016-09-30 08:55:57 UTC
Removing myself from CC, I get updates to this report via the xorg-team mailing list.
Comment 4 Michel Dänzer 2016-09-30 08:57:39 UTC
Created attachment 126890 [details] [review]
Negate dst_x/off for fbCopy*to*

Does this patch help by any chance?
Comment 5 Olivier Fourdan 2016-09-30 09:34:34 UTC
(In reply to Michel Dänzer from comment #4)
> Created attachment 126890 [details] [review] [review]
> Negate dst_x/off for fbCopy*to*
> 
> Does this patch help by any chance?

Unfortunately it doesn't, I still get the same backtrace as in comment 0 with that patch applied.
Comment 6 Olivier Fourdan 2016-09-30 09:39:26 UTC
FWIW:

Program received signal SIGSEGV, Segmentation fault.
0x00000000004b2374 in fbBltOne (src=0x19e11c4, srcStride=<optimized out>, srcStride@entry=1, srcX=19, dst=0x1b8cb70, dstStride=220, dstStride@entry=229, 
    dstX=<optimized out>, dstBpp=32, width=288, height=7, fgand=0, fgxor=0, bgand=0, bgxor=16777215) at fbbltone.c:334
334	                        WRITE(dst, FbOpaqueStipple(mask, fgxor, bgxor));
(gdb) bt
#0  0x00000000004b2374 in fbBltOne (src=0x19e11c4, srcStride=<optimized out>, srcStride@entry=1, srcX=19, dst=0x1b8cb70, dstStride=220, dstStride@entry=229, 
    dstX=<optimized out>, dstBpp=32, width=288, height=7, fgand=0, fgxor=0, bgand=0, bgxor=16777215) at fbbltone.c:334
#1  0x000000000049e947 in fbCopy1toN (pSrcDrawable=pSrcDrawable@entry=0x19e0d10, pDstDrawable=pDstDrawable@entry=0x1b2bcc0, pGC=pGC@entry=0x18f6710, 
    pbox=pbox@entry=0x7ffeb088ffe0, nbox=<optimized out>, nbox@entry=1, dx=-21, dy=-191, reverse=0, upsidedown=0, bitplane=1, closure=0x0) at fbcopy.c:123
#2  0x0000000000483131 in glamor_copy_cpu_fbo (closure=0x0, bitplane=1, upsidedown=0, reverse=0, dy=-432, dx=-264, nbox=1, box=0x7ffeb088ffe0, gc=0x18f6710, 
    dst=0x18f6710, src=0x19e0d10) at glamor_copy.c:241
#3  glamor_copy_gl (closure=0x0, bitplane=1, upsidedown=0, reverse=0, dy=-432, dx=<optimized out>, nbox=1, box=0x7ffeb088ffe0, gc=0x18f6710, dst=0x18f6710, 
    src=0x19e0d10) at glamor_copy.c:649
#4  glamor_copy (src=src@entry=0x19e0d10, dst=dst@entry=0x18f69b0, gc=gc@entry=0x18f6710, box=box@entry=0x7ffeb088ffe0, nbox=nbox@entry=1, dx=-264, dy=-432, 
    reverse=0, upsidedown=0, bitplane=1, closure=0x0) at glamor_copy.c:676
#5  0x00000000004c1fbc in miCopyRegion (pSrcDrawable=pSrcDrawable@entry=0x19e0d10, pDstDrawable=pDstDrawable@entry=0x18f69b0, pGC=pGC@entry=0x18f6710, 
    pDstRegion=pDstRegion@entry=0x7ffeb088ffe0, dx=dx@entry=-264, dy=dy@entry=-432, copyProc=0x4825a0 <glamor_copy>, bitPlane=1, closure=0x0) at micopy.c:121
#6  0x00000000004c253c in miDoCopy (pSrcDrawable=0x19e0d10, pDstDrawable=0x18f69b0, pGC=0x18f6710, xIn=0, yIn=0, widthSrc=9, heightSrc=8, xOut=264, yOut=432, 
    copyProc=0x4825a0 <glamor_copy>, bitPlane=1, closure=0x0) at micopy.c:296
#7  0x000000000048323e in glamor_copy_plane (src=<optimized out>, dst=<optimized out>, gc=<optimized out>, srcx=<optimized out>, srcy=<optimized out>, 
    width=<optimized out>, height=8, dstx=19, dsty=189, bitplane=1) at glamor_copy.c:698
#8  0x00000000005435bf in damageCopyPlane (pSrc=0x19e0d10, pDst=0x18f69b0, pGC=0x18f6710, srcx=0, srcy=<optimized out>, width=9, height=8, dstx=19, dsty=189, 
    bitPlane=1) at damage.c:799
#9  0x0000000000436bec in ProcCopyPlane (client=0x1738a50) at dispatch.c:1758
#10 0x000000000043a526 in Dispatch () at dispatch.c:469
#11 0x000000000043e518 in dix_main (argc=10, argv=0x7ffeb0890308, envp=<optimized out>) at main.c:287
#12 0x00007f6029aef401 in __libc_start_main (main=0x423050 <main>, argc=10, argv=0x7ffeb0890308, init=<optimized out>, fini=<optimized out>, 
    rtld_fini=<optimized out>, stack_end=0x7ffeb08902f8) at ../csu/libc-start.c:289
#13 0x000000000042308a in _start ()
(gdb) f 2
#2  0x0000000000483131 in glamor_copy_cpu_fbo (closure=0x0, bitplane=1, upsidedown=0, reverse=0, dy=-432, dx=-264, nbox=1, box=0x7ffeb088ffe0, gc=0x18f6710, 
    dst=0x18f6710, src=0x19e0d10) at glamor_copy.c:241
241	            fbCopy1toN(src, &src_pix->drawable, gc, box, nbox,
(gdb) list
236	        if (src->bitsPerPixel > 1)
237	            fbCopyNto1(src, &src_pix->drawable, gc, box, nbox,
238	                       dx - dst_xoff, dy - dst_yoff, reverse, upsidedown,
239	                       bitplane, closure);
240	        else
241	            fbCopy1toN(src, &src_pix->drawable, gc, box, nbox,
242	                       dx - dst_xoff, dy - dst_yoff, reverse, upsidedown,
243	                       bitplane, closure);
244	
245	        glamor_upload_boxes(dst_pixmap, box, nbox, 0, 0, 0, 0,
(gdb) p dx
$1 = -264
(gdb) p dst_xoff
$2 = -243
(gdb) p dy
$3 = -432
(gdb) p dst_yoff
$4 = -241
(gdb) p reverse
$5 = 0
(gdb) p upsidedown
$6 = 0
Comment 7 Olivier Fourdan 2016-10-04 10:06:54 UTC
I think this is a mistake in the coords somehow, I have a patch that avoids the crash but I am not sure the end result is correct (actually I suspect it is not...)
Comment 8 Olivier Fourdan 2016-10-04 11:22:54 UTC
Created attachment 126989 [details] [review]
[PATCH xserver] glamor: Fix pixmap offset for bitplane in glamor_copy_fbo_cpu

That seems to work and give the appropriate result (visualy speaking, tested with xterm's contextual menu, the check marks are drawn correctly where they should be)
Comment 9 Olivier Fourdan 2016-10-06 13:04:42 UTC
Patch v2 has landed as commit 1c2fcb9, closing.


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.