Bug 38331 - libxi causes segfault on mips64 abi n32
Summary: libxi causes segfault on mips64 abi n32
Status: RESOLVED WONTFIX
Alias: None
Product: xorg
Classification: Unclassified
Component: Lib/Xi (show other bugs)
Version: unspecified
Hardware: SGI Linux (All)
: high critical
Assignee: Peter Hutterer
QA Contact: Xorg Project Team
URL:
Whiteboard: 2011BRB_Reviewed
Keywords: patch
Depends on:
Blocks:
 
Reported: 2011-06-15 00:08 UTC by xihh
Modified: 2016-11-28 04:40 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 Sequoia 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 Sequoia 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).
Comment 10 Peter Hutterer 2016-11-28 04:40:04 UTC
This is a mass change of bugs. Bugs assigned to me that haven't been updated in the last 3 years are closed as WONTFIX, because, well, let's at least be honest about it.

Please do not re-open unless you have a really good reason to do so (e.g. you're fixing it yourself). If it hasn't been fixed in the last 3 years, it probably won't be fixed anytime soon either. Sorry.


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.