Bug 36860 - X crashes when I set a Python Tkinter edit control to display a long text string in 25pt DejaVu font
Summary: X crashes when I set a Python Tkinter edit control to display a long text str...
Status: RESOLVED FIXED
Alias: None
Product: xorg
Classification: Unclassified
Component: Driver/intel (show other bugs)
Version: 7.6 (2010.12)
Hardware: All Linux (All)
: high normal
Assignee: Chris Wilson
QA Contact: Xorg Project Team
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-04 19:52 UTC by Bryce Harrington
Modified: 2011-06-28 12:27 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
check privates (392 bytes, patch)
2011-05-04 19:56 UTC, Bryce Harrington
no flags Details | Splinter Review
Same patch, against current git head (1.04 KB, patch)
2011-05-04 20:13 UTC, Bryce Harrington
no flags Details | Splinter Review
earlier (bad) shot at this patch (2.17 KB, text/plain)
2011-05-04 23:58 UTC, Bryce Harrington
no flags Details

Description Bryce Harrington 2011-05-04 19:52:32 UTC
Forwarding this bug from Ubuntu reporter Silas S. Brown:
http://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-intel/+bug/731424

[Problem]
The Tk script shown below reliably crashes the xserver in Ubuntu.  This is due to dereferencing a null privates pointer in i965_set_picture_surface_state()

[Original Description]
I am on Ubuntu 10.10 with the default X server, and the following Python code crashes it every time:

from Tkinter import *
f=Frame()
f.option_add('*font', "-family {DejaVu Sans} -size -25 -weight normal -slant roman -underline 0 -overstrike 0")
f.pack()
text = StringVar(f)
entry = Entry(f, textvariable=text)
entry.pack()
text.set("a" * 600)  # 537 is ok; values 538 and up crash X
f.mainloop()

Unfortunately, I was not able to capture a backtrace with apport.  I'm using an Asus X58Lseries laptop which I believe is 1280 x 800 (WXGA) using an Intel GMA X3100 graphics chipset.  I have set System > Preferences > Appearance > Fonts > Details > Resolution to 160 DPI for a larger display.

#0  i965_set_picture_surface_state (intel=0x883dcd0, pixmap=0xb7103008, is_dst=1, 
    picture=<value optimized out>) at ../../src/i965_render.c:1136
        priv = 0x0
        ss = <value optimized out>
        write_domain = <value optimized out>
        read_domains = <value optimized out>
        offset = <value optimized out>
#1  0x0020fa2a in i965_bind_surfaces (dest=0xb7103008, srcX=320, srcY=48, maskX=0, maskY=0, dstX=0, 
    dstY=0, w=15, h=14) at ../../src/i965_render.c:1740
        binding_table = 0x8845f24
#2  i965_composite (dest=0xb7103008, srcX=320, srcY=48, maskX=0, maskY=0, dstX=0, dstY=0, w=15, h=14)
    at ../../src/i965_render.c:1845
        scrn = 0x8839040
        intel = 0x883dcd0
        render_state = <value optimized out>
        has_mask = 0
        src_x = {320, 320, 335}
        src_y = {48, 62, 62}
        src_w = {8.8028834e-33, 8.80539896e-33, 5.77529779e-33}
        mask_x = {0, 1.875, 4.48415509e-43}
        mask_y = {5.73971851e-42, 8.74344453e-33, 1.29830863e-39}
        mask_w = {-1.50051022, 3.02446732e-39, 0}
        is_affine = 1
#3  0x002197a0 in uxa_glyphs_via_mask (op=3 '\003', pSrc=0xa3398f8, pDst=0xa36e470, 
    maskFormat=0x9efdd88, xSrc=0, ySrc=0, nlist=3, list=0xbfc017e0, glyphs=0xa36f260)
    at ../../uxa/uxa-glyphs.c:1002
        this_atlas = 0x9f2e8e8
        src_x = 320
        glyph = 0xa326798
        src_y = 48
        priv = 0x9efe490
        screen = 0x8831588
        mask = 0xa3597c0
        y = 14
        pixmap = 0xb7103008
        dst_off_x = 196611
        n = <value optimized out>
        dst_off_y = 13
        box = {x1 = 3, y1 = 13, x2 = 9003, y2 = 27}
        component_alpha = 1
        glyph_atlas = 0x9f2e8e8
        x = 0
        height = <value optimized out>
        error = 0
#4  uxa_glyphs (op=3 '\003', pSrc=0xa3398f8, pDst=0xa36e470, maskFormat=0x9efdd88, xSrc=0, ySrc=0, 
    nlist=3, list=0xbfc017e0, glyphs=0xa36f260) at ../../uxa/uxa-glyphs.c:1157
...
Comment 1 Bryce Harrington 2011-05-04 19:54:50 UTC
I was able to reproduce this error locally on an i965 laptop.

The issue appears to be that the screen structure's render_dest_picture has a null devPrivates:

(gdb) up
#2  i965_composite (dest=0xb7146008, srcX=992, srcY=48, maskX=0, maskY=0, dstX=0, dstY=0, w=15, h=14)
    at ../../src/i965_render.c:1845
1845	in ../../src/i965_render.c

(gdb) print (*(*(intel_screen_private *)(scrn->driverPrivate))->render_dest_picture)
$19 = {pDrawable = 0xb7146008, pFormat = 0x9ae6d88, format = PICT_a8r8g8b8, refcnt = 1, id = 0, 
  repeat = 0, graphicsExposures = 0, subWindowMode = 0, polyEdge = 0, polyMode = 0, freeCompClip = 1, 
  clientClipType = 0, componentAlpha = 1, repeatType = 0, filter = 0, stateChanges = 0, unused = 0, 
  pNext = 0x0, alphaMap = 0x0, alphaOrigin = {x = 0, y = 0}, clipOrigin = {x = 0, y = 0}, 
  clientClip = 0x0, serialNumber = 339102, pCompositeClip = 0x9c4de18, devPrivates = 0x0, 
  transform = 0x0, pSourcePict = 0x0, filter_params = 0x0, filter_nparams = 0}
Comment 2 Bryce Harrington 2011-05-04 19:56:54 UTC
Created attachment 46341 [details] [review]
check privates

A simple null pointer check at this point in the code is sufficient to paper over the crash.

(The tk app seems to still not work properly with a text string of this size, so there is still a bug in play, but it's unclear whether that's in X or the tk libs.)
Comment 3 Bryce Harrington 2011-05-04 20:13:39 UTC
Created attachment 46342 [details] [review]
Same patch, against current git head
Comment 4 Chris Wilson 2011-05-04 23:26:48 UTC
Right idea, but we either need to move the check higher or propagate the failure so that we can run the fallback.

However in this case, I think we can improve the implementation by clipping the mask against the dst before filling it -- i.e. prevent us from creating the oversized temporary.
Comment 5 Bryce Harrington 2011-05-04 23:58:23 UTC
Created attachment 46343 [details]
earlier (bad) shot at this patch

"we either need to move the check higher or propagate the failure so that we can run the fallback."

Originally I did a check at a higher level (see attached).  However this wasn't right; the screen flashed on and off every second or two, and after a minute or so X locked up.
Comment 6 Chris Wilson 2011-05-05 00:04:23 UTC
Yeah, that would trigger a few asserts and upset your machine. *g*
Comment 7 Bryce Harrington 2011-05-05 12:04:33 UTC
I'm going to be gone for the next week (Intel tomorrow, UDS next week), so won't be able to do further work on this for a while.  But if you make a better patch I may be able to take care of packaging it for Silas to test.
Comment 8 Bryce Harrington 2011-05-18 18:37:21 UTC
Alright, well I'll go with the original patch to paper over the crash in the distro for now.  If you come up with a better patch let me know.
Comment 9 Chris Wilson 2011-05-19 00:39:36 UTC
I have, just it's in the middle of 40,000 other lines...
Comment 10 Chris Wilson 2011-06-05 22:21:18 UTC
The right approach is included with sna.
Comment 11 Chris Wilson 2011-06-28 12:27:40 UTC
commit 18d08e49d270b7a05f14a309759c9315e5ab9679
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Jun 28 20:25:46 2011 +0100

    uxa/glyphs: Fallback instead of crashing on large strings
    
    Not ideal, but being slow is a major improvement over losing data.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=36860
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>


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.