| Message ID | 555E0986.3010608@arm.com |
|---|---|
| State | New |
| Headers | show |
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. 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. Cheers, Carlos.
On 05/28/2015 09:37 AM, Szabolcs Nagy wrote: > 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. Is it used for AArch64 though? As far as I can tell gdb for AArch64 does not use greg_t. Either way, the present definition is broken, so the user could not have expected it to work in any way, but if the type was embedded into a structure and unused, then you might have an ABI breaking situation. I do not think you are in that case because mcontext on AArch64 doesn't use greg_t. Cheers, Carlos.
On 29 May 2015 at 14:55, Carlos O'Donell <carlos@redhat.com> wrote: > On 05/28/2015 09:37 AM, Szabolcs Nagy wrote: >> 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. > > Is it used for AArch64 though? As far as I can tell gdb > for AArch64 does not use greg_t. > > Either way, the present definition is broken, so the user > could not have expected it to work in any way, but if the > type was embedded into a structure and unused, then you > might have an ABI breaking situation. I do not think you > are in that case because mcontext on AArch64 doesn't use > greg_t. Hi. The analysis Carlos provided looks sensible to me, go ahead and commit your patch. /Marcus
diff --git a/sysdeps/unix/sysv/linux/aarch64/sys/ucontext.h b/sysdeps/unix/sysv/linux/aarch64/sys/ucontext.h index db7a7c1..e7268ce 100644 --- a/sysdeps/unix/sysv/linux/aarch64/sys/ucontext.h +++ b/sysdeps/unix/sysv/linux/aarch64/sys/ucontext.h @@ -29,7 +29,7 @@ included in <signal.h>. */ #include <bits/sigcontext.h> -typedef int greg_t; +typedef elf_greg_t greg_t; /* Container for all general registers. */ typedef elf_gregset_t gregset_t;