Bug 38331 - libxi causes segfault on mips64 abi n32
libxi causes segfault on mips64 abi n32
Status: NEEDINFO
Product: xorg
Classification: Unclassified
Component: Lib/Xi
unspecified
SGI Linux (All)
: high critical
Assigned To: Peter Hutterer
Xorg Project Team
2011BRB_Reviewed
: patch
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-15 00:08 UTC by xihh
Modified: 2012-02-20 11:19 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
patch that fixes the issue (951 bytes, text/plain)
2011-06-15 00:08 UTC, xihh
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description xihh 2011-06-15 00:08:00 UTC
Created attachment 47980 [details]
patch that fixes the issue

Some calls to libxi cause segmentation fault on mips64(el) abi n32.

Attached is a fix for the problem.
Comment 1 Julien Cristau 2011-06-15 13:12:27 UTC
Would be better without the ifdefs...
Comment 2 Michał Masłowski 2011-06-16 07:35:40 UTC
(In reply to comment #1)
> Would be better without the ifdefs...

The change just adds some unneeded space to align the structures to 8 bytes.  It should not cause additional problems if enabled on other architectures, except for increasing memory use by less than 8 bytes per each structure allocated in copy_classes.  I don't know any other architecture/ABI where such alignment is required but is not made without padding by structures having appropriate size.
Comment 3 Jeremy Huddleston 2011-10-03 17:38:59 UTC
Can this be addressed instead with an __attribute__ ((aligned (8))) somewhere?
Comment 4 Michał Masłowski 2011-10-12 07:56:34 UTC
(In reply to comment #3)
> Can this be addressed instead with an __attribute__ ((aligned (8))) somewhere?

All structures used are already aligned to 8 bytes, but the code puts there also arrays of 4 byte objects.  I don't see any other solution than manually padding it to start XIValuatorClassInfo structures at addresses aligned to 8 bytes.
Comment 5 Julien Cristau 2011-10-13 11:29:01 UTC
> --- Comment #4 from Michał Masłowski <mtjm@mtjm.eu> 2011-10-12 07:56:34 PDT ---
> (In reply to comment #3)
> > Can this be addressed instead with an __attribute__ ((aligned (8))) somewhere?
> 
> All structures used are already aligned to 8 bytes, but the code puts there
> also arrays of 4 byte objects.  I don't see any other solution than manually
> padding it to start XIValuatorClassInfo structures at addresses aligned to 8
> bytes.
> 
Would there be a downside to doing that for all archs (or all 64bit
archs), rather than just one mips abi? (aside from 4-bytes holes here
and there)
Comment 6 Michał Masłowski 2011-10-13 11:46:30 UTC
(In reply to comment #5)
> > All structures used are already aligned to 8 bytes, but the code puts there
> > also arrays of 4 byte objects.  I don't see any other solution than manually
> > padding it to start XIValuatorClassInfo structures at addresses aligned to 8
> > bytes.
> > 
> Would there be a downside to doing that for all archs (or all 64bit
> archs), rather than just one mips abi? (aside from 4-bytes holes here
> and there)

There would be no other downside.  It probably is already 8 byte aligned on arches with 64-bit longs and pointers (N32 uses 32-bit longs and pointers, while long longs and doubles use native registers and need 8 byte alignment).
Comment 7 Jeremy Huddleston 2011-10-15 11:33:42 UTC
=/

Ok, would you please send your patch to xorg-devel for review?
Comment 8 Peter Hutterer 2012-02-19 15:10:16 UTC
07ced7b48219e3bc0c98806f3d7106f86d1b2ca0 added a pad_to_xid() to avoid 4-byte alignment of a 8-byte XID. afaict, your patch could be worked into this function to always align on 8 bytes on the mips.

a simple
#ifdef _ABIN32
  padsize = 8;
#endif

or somesuch in pad_to_xi2() should do, right?
Comment 9 Michał Masłowski 2012-02-20 11:19:00 UTC
The pad_to_xid change affects other architectures for an error in different part of the library.  I don't see a simpler solution for this bug than manually padding XIValuatorClassInfo on MIPS N32 in copy_classes, like in my patch (a modified version of which I have sent to the list).