Alignment error in libXi

[Matt Turner]

On Sun, Mar 13, 2011 at 10:11 PM, Christian Weisgerber
<naddy at mips.inka.de> wrote:
> GTK+3 blows up on LP64 archs with strict alignment.  This comes
> down to an unaligned access error in libXi, specifically in
> the XIButtonClass case in copy_classes() in XExtInt.c.
>
>    cls_lib->num_buttons = cls_wire->num_buttons;
>    cls_lib->state.mask_len = ((((cls_wire->num_buttons + 7)/8) + 3)/4) * 4;
>    cls_lib->state.mask = next_block(&ptr_lib, cls_lib->state.mask_len);
>    memcpy(cls_lib->state.mask, &cls_wire[1],
>           cls_lib->state.mask_len);
>
>    cls_lib->labels = next_block(&ptr_lib, cls_lib->num_buttons * sizeof(Atom));
>    atoms =(uint32_t*)((char*)&cls_wire[1] + cls_lib->state.mask_len);
>    for (j = 0; j < cls_lib->num_buttons; j++)
>        cls_lib->labels[j] = *atoms++;
>
> It's the   cls_lib->labels[j] = *atoms++   assignment that blows up.
>
> For state.mask, n*4 bytes are allocated from the ptr_lib area.
> labels is allocated immediately following that, but labels is an
> array of Atoms, which are longs, i.e, an 8-byte type on LP64 archs.
> labels can end up on an address that is misaligned for Atom
> assignments, like it did here:
>
> #0  0x00000001616ea910 in copy_classes (to=0x165db5600, from=0x161bae020,
>    nclasses=3) at /usr/xenocara/lib/libXi/src/XExtInt.c:1490
> 1490                            cls_lib->labels[j] = *atoms++;
> (gdb) bt
> #0  0x00000001616ea910 in copy_classes (to=0x165db5600, from=0x161bae020,
>    nclasses=3) at /usr/xenocara/lib/libXi/src/XExtInt.c:1490
> #1  0x00000001616ecd80 in XIQueryDevice (dpy=0x16b964db0, deviceid=0,
>    ndevices_return=0x1fffe44f0)
>    at /usr/xenocara/lib/libXi/src/XIQueryDevice.c:90
> #2  0x000000016c30b6b0 in gdk_x11_device_manager_xi2_constructed (
>    object=0x16f8886a0) at gdkdevicemanager-xi2.c:413
> #3  0x000000016bf08488 in g_object_newv ()
>   from /usr/local/lib/libgobject-2.0.so.2800.0
> #4  0x000000016bf08a48 in g_object_new_valist ()
>   from /usr/local/lib/libgobject-2.0.so.2800.0
> #5  0x000000016bf07dec in g_object_new ()
>   from /usr/local/lib/libgobject-2.0.so.2800.0
> #6  0x000000016c308c10 in _gdk_x11_device_manager_new (display=0x164582000)
>    at gdkdevicemanager-x11.c:59
> #7  0x000000016c310a0c in _gdk_x11_display_open (display_name=0x0)
>    at gdkdisplay-x11.c:1228
> #8  0x000000016c30dff4 in gdk_x11_display_manager_open_display (
>    manager=0x1659b0000, name=0x0) at gdkdisplaymanager-x11.c:55
> #9  0x000000016c2d4130 in gdk_display_manager_open_display (
>    manager=0x1659b0000, name=0x0) at gdkdisplaymanager.c:362
> #10 0x000000016c2d29e4 in gdk_display_open (display_name=0x0)
>    at gdkdisplay.c:1720
> #11 0x000000016c2c44b4 in gdk_display_open_default_libgtk_only () at gdk.c:341
> #12 0x0000000168a9981c in gtk_init_check (argc=0x1fffe49a8, argv=0x1fffe49b0)
>    at gtkmain.c:1132
> #13 0x0000000168a99878 in gtk_init (argc=0x1fffe49a8, argv=0x1fffe49b0)
>    at gtkmain.c:1184
> #14 0x0000000120031094 in main (argc=1, argv=0x1fffe4a70) at main.c:948
> (gdb) i loc
> cls_lib = (XIButtonClassInfo *) 0x165db5018
> cls_wire = (xXIButtonInfo *) 0x161bae020
> atoms = (uint32_t *) 0x161bae02c
> j = 0
> any_lib = (XIAnyClassInfo *) 0x165db5018
> any_wire = (xXIAnyInfo *) 0x161bae020
> ptr_lib = (void *) 0x165db5094
> ptr_wire = 0x161bae020 "\001"
> i = 0
> len = 0
> (gdb) p cls_lib->labels
> $1 = (Atom *) 0x165db5044
> (gdb) p sizeof(Atom)
> $2 = 8
> (gdb)
>
> --
> Christian "naddy" Weisgerber                          naddy at mips.inka.de

Good analysis.

Is this one of those cases where someone, long ago, thought using
longs was inherently good? Can we just change labels to be an array of
ints, or is that too easy?

There's even a comment in the code you quoted:

/* Force mask alignment with longs to avoid unaligned
 * access when accessing the atoms. */

So much for that.

Matt