Re: [PATCH][AArch64] change greg_t definition in ucontext.h

[Szabolcs Nagy <szabolcs dot nagy at arm dot com>]

On 21/05/15 19:21, Carlos O'Donell wrote:
> On 05/21/2015 12:36 PM, Szabolcs Nagy wrote:
>> This change assumes greg_t is not used outside of libc.
>> In glibc it is usually used for defining gregset_t, but
>> on aarch64 elf_greg_t is used for that, greg_t is not
>> used anywhere (so it is meant to be a cosmetic fix).
>>
>> If greg_t is used outside of libc, then this is an ABI
>> change, but
>>
>> typedef int greg_t;
>>
>> is not a useful definition on aarch64.
>>
>> Some targets (powerpc) don't define greg_t at all so
>> removing it should be a valid fix too. It seems the
>> signedness of greg_t varies between targets so i settled
>> for using elf_greg_t (unsigned long) for the typedef to
>> be consistent with the members of gregset_t.
>>
>>
>> Changelog:
>>
>> 2015-05-07  Szabolcs Nagy  <szabolcs.nagy@arm.com>
>>
>> 	* sysdeps/unix/sysv/linux/aarch64/sys/ucontext.h (greg_t): Change the
>> 	definition to elf_greg_t.
>>
> 
> OK for me.
> 
> I agree it is not a useful definition for aarch64,
> however, it is technically an ABI break.
> 
> As long as you acknowledge that you are breaking
> the ABI for this type, then you can go ahead, but
> something in userspace *might* break.
> 

i was thinking about it, but i don't see a useful way
to annotate this change other than adding a comment.

since greg_t is not used in libc, it seems it only
breaks code that either uses it in an extern api
between modules or the source code somehow depends
on it being int.

i can include a comment about the abi break after
glibc 2.21 next to the typedef.

> The only other component I've seen use greg_t is
> gdb, but on aarch64 you don't use greg_t, and if
> you did you would have discovered this was broken
> earlier.

the semantics of greg_t is target specific.

it seems to be used in a few places (gdb, qemu,..)
but always under a target specific ifdef to access
greg_t members in mcontext.

> Cheers,
> Carlos.
>