| Message ID | 20190703005202.7578-1-alistair.francis@wdc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | RISC-V: Handle the siginfo_t offset problem | expand |
On Jul 02 2019, Alistair Francis <alistair.francis@wdc.com> wrote: > In the RISC-V 32-bit glibc port [1] the siginfo_t struct in the kernel > doesn't line up with the struct in glibc. In glibc world the _sifields > union is 8 byte alligned (although I can't figure out why) Try ptype/o in gdb. Andreas.
On Wed, Jul 3, 2019 at 12:08 AM Andreas Schwab <schwab@suse.de> wrote: > > On Jul 02 2019, Alistair Francis <alistair.francis@wdc.com> wrote: > > > In the RISC-V 32-bit glibc port [1] the siginfo_t struct in the kernel > > doesn't line up with the struct in glibc. In glibc world the _sifields > > union is 8 byte alligned (although I can't figure out why) > > Try ptype/o in gdb. That's a useful tip, I'll be sure to use that next time. Alistair > > Andreas. > > -- > Andreas Schwab, SUSE Labs, schwab@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different."
On Jul 03 2019, Alistair Francis <alistair23@gmail.com> wrote: > On Wed, Jul 3, 2019 at 12:08 AM Andreas Schwab <schwab@suse.de> wrote: >> >> On Jul 02 2019, Alistair Francis <alistair.francis@wdc.com> wrote: >> >> > In the RISC-V 32-bit glibc port [1] the siginfo_t struct in the kernel >> > doesn't line up with the struct in glibc. In glibc world the _sifields >> > union is 8 byte alligned (although I can't figure out why) >> >> Try ptype/o in gdb. > > That's a useful tip, I'll be sure to use that next time. It was a serious note. If the structs don't line up then there is a mismatch in types that cannot be solved by adding spurious padding. You need to fix the types instead. Andreas.
On Thu, Jul 4, 2019 at 9:20 AM Andreas Schwab <schwab@suse.de> wrote: > > On Jul 03 2019, Alistair Francis <alistair23@gmail.com> wrote: > > > On Wed, Jul 3, 2019 at 12:08 AM Andreas Schwab <schwab@suse.de> wrote: > >> > >> On Jul 02 2019, Alistair Francis <alistair.francis@wdc.com> wrote: > >> > >> > In the RISC-V 32-bit glibc port [1] the siginfo_t struct in the kernel > >> > doesn't line up with the struct in glibc. In glibc world the _sifields > >> > union is 8 byte alligned (although I can't figure out why) > >> > >> Try ptype/o in gdb. > > > > That's a useful tip, I'll be sure to use that next time. > > It was a serious note. If the structs don't line up then there is a > mismatch in types that cannot be solved by adding spurious padding. You > need to fix the types instead. Would it be an option to align all the basic typedefs (off_t, time_t, clock_t, ...) between glibc and kernel then, and just use the existing sysdeps/unix/sysv/linux/generic/bits/typesizes.h after all to avoid surprises like this? As of v2 the functional difference is -#define __INO_T_TYPE __ULONGWORD_TYPE +#define __INO_T_TYPE __UQUAD_TYPE #define __INO64_T_TYPE __UQUAD_TYPE #define __MODE_T_TYPE __U32_TYPE -#define __NLINK_T_TYPE __U32_TYPE -#define __OFF_T_TYPE __SLONGWORD_TYPE +#define __NLINK_T_TYPE __UQUAD_TYPE +#define __OFF_T_TYPE __SQUAD_TYPE #define __OFF64_T_TYPE __SQUAD_TYPE -#define __RLIM_T_TYPE __ULONGWORD_TYPE +#define __RLIM_T_TYPE __UQUAD_TYPE #define __RLIM64_T_TYPE __UQUAD_TYPE -#define __BLKCNT_T_TYPE __SLONGWORD_TYPE +#define __BLKCNT_T_TYPE __SQUAD_TYPE #define __BLKCNT64_T_TYPE __SQUAD_TYPE -#define __FSBLKCNT_T_TYPE __ULONGWORD_TYPE +#define __FSBLKCNT_T_TYPE __UQUAD_TYPE #define __FSBLKCNT64_T_TYPE __UQUAD_TYPE -#define __FSFILCNT_T_TYPE __ULONGWORD_TYPE +#define __FSFILCNT_T_TYPE __UQUAD_TYPE #define __FSFILCNT64_T_TYPE __UQUAD_TYPE -#define __FSWORD_T_TYPE __SWORD_TYPE +#define __FSWORD_T_TYPE __SQUAD_TYPE -#define __CLOCK_T_TYPE __SLONGWORD_TYPE -#define __TIME_T_TYPE __SLONGWORD_TYPE +#define __CLOCK_T_TYPE __SQUAD_TYPE +#define __TIME_T_TYPE __SQUAD_TYPE #define __USECONDS_T_TYPE __U32_TYPE -#define __SUSECONDS_T_TYPE __SLONGWORD_TYPE +#define __SUSECONDS_T_TYPE __SQUAD_TYPE -#define __BLKSIZE_T_TYPE __S32_TYPE +#define __BLKSIZE_T_TYPE __SQUAD_TYPE #define __FSID_T_TYPE struct { int __val[2]; } #define __SSIZE_T_TYPE __SWORD_TYPE -#define __SYSCALL_SLONG_TYPE __SLONGWORD_TYPE -#define __SYSCALL_ULONG_TYPE __ULONGWORD_TYPE -#define __CPU_MASK_TYPE __ULONGWORD_TYPE +#define __SYSCALL_SLONG_TYPE __SQUAD_TYPE +#define __SYSCALL_ULONG_TYPE __UQUAD_TYPE +#define __CPU_MASK_TYPE __UQUAD_TYPE -#ifdef __LP64__ # define __RLIM_T_MATCHES_RLIM64_T 1 -#else -# define __RLIM_T_MATCHES_RLIM64_T 0 -#endif +#define __ASSUME_TIME64_SYSCALLS 1 +#define __ASSUME_RLIM64_SYSCALLS 1 Since the sysdeps/unix/sysv/linux/generic/bits/typesizes.h definitions generally match the kernel, anything diverging from that has the potential of breaking it, so the difference should probably be kept to the absolute minimum. I think these ones are wrong and will cause bugs similar to the clock_t issue if they are used with kernel interfaces: __NLINK_T_TYPE, __FSWORD_T_TYPE, __CLOCK_T_TYPE, __BLKSIZE_T_TYPE, __SYSCALL_ULONG_TYPE, __SYSCALL_SLONG_TYPE, __CPU_MASK_TYPE These are fine as long as they are only used in user space and to wrap kernel syscalls, but I think most of them can end up being passed to the kernel, so it seems safer not to have rv32 diverge without a good reason. The remaining ones (__INO_T_TYPE, __OFF_T_TYPE, __BLKCNT_T_TYPE, __FSBLKCNT_T_TYPE, __FSFILCNT_T_TYPE, __TIME_T_TYPE) all follow the pattern where the kernel has an old 32-bit type and a new 64-bit type, but the kernel tries not to expose the 32-bit interfaces to user space on new architectures and only provide the 64-bit replacements, but there are a couple of interfaces that never got replaced, typically in driver and file system ioctls. Since glibc already has code to deal with the 64-bit types and that is well tested, it would seem safer to me to just #undef the old types completely rather than defining them to 64-bit, which would make them incompatible with the kernel's types. Arnd
On Thu, Jul 4, 2019 at 2:19 AM Arnd Bergmann <arnd@arndb.de> wrote: > > On Thu, Jul 4, 2019 at 9:20 AM Andreas Schwab <schwab@suse.de> wrote: > > > > On Jul 03 2019, Alistair Francis <alistair23@gmail.com> wrote: > > > > > On Wed, Jul 3, 2019 at 12:08 AM Andreas Schwab <schwab@suse.de> wrote: > > >> > > >> On Jul 02 2019, Alistair Francis <alistair.francis@wdc.com> wrote: > > >> > > >> > In the RISC-V 32-bit glibc port [1] the siginfo_t struct in the kernel > > >> > doesn't line up with the struct in glibc. In glibc world the _sifields > > >> > union is 8 byte alligned (although I can't figure out why) > > >> > > >> Try ptype/o in gdb. > > > > > > That's a useful tip, I'll be sure to use that next time. > > > > It was a serious note. If the structs don't line up then there is a > > mismatch in types that cannot be solved by adding spurious padding. You > > need to fix the types instead. > > Would it be an option to align all the basic typedefs (off_t, time_t, > clock_t, ...) > between glibc and kernel then, and just use the existing > sysdeps/unix/sysv/linux/generic/bits/typesizes.h after all to avoid > surprises like this? > > As of v2 the functional difference is > > -#define __INO_T_TYPE __ULONGWORD_TYPE > +#define __INO_T_TYPE __UQUAD_TYPE > #define __INO64_T_TYPE __UQUAD_TYPE > #define __MODE_T_TYPE __U32_TYPE > -#define __NLINK_T_TYPE __U32_TYPE > -#define __OFF_T_TYPE __SLONGWORD_TYPE > +#define __NLINK_T_TYPE __UQUAD_TYPE > +#define __OFF_T_TYPE __SQUAD_TYPE > #define __OFF64_T_TYPE __SQUAD_TYPE > -#define __RLIM_T_TYPE __ULONGWORD_TYPE > +#define __RLIM_T_TYPE __UQUAD_TYPE > #define __RLIM64_T_TYPE __UQUAD_TYPE > -#define __BLKCNT_T_TYPE __SLONGWORD_TYPE > +#define __BLKCNT_T_TYPE __SQUAD_TYPE > #define __BLKCNT64_T_TYPE __SQUAD_TYPE > -#define __FSBLKCNT_T_TYPE __ULONGWORD_TYPE > +#define __FSBLKCNT_T_TYPE __UQUAD_TYPE > #define __FSBLKCNT64_T_TYPE __UQUAD_TYPE > -#define __FSFILCNT_T_TYPE __ULONGWORD_TYPE > +#define __FSFILCNT_T_TYPE __UQUAD_TYPE > #define __FSFILCNT64_T_TYPE __UQUAD_TYPE > -#define __FSWORD_T_TYPE __SWORD_TYPE > +#define __FSWORD_T_TYPE __SQUAD_TYPE > -#define __CLOCK_T_TYPE __SLONGWORD_TYPE > -#define __TIME_T_TYPE __SLONGWORD_TYPE > +#define __CLOCK_T_TYPE __SQUAD_TYPE > +#define __TIME_T_TYPE __SQUAD_TYPE > #define __USECONDS_T_TYPE __U32_TYPE > -#define __SUSECONDS_T_TYPE __SLONGWORD_TYPE > +#define __SUSECONDS_T_TYPE __SQUAD_TYPE > -#define __BLKSIZE_T_TYPE __S32_TYPE > +#define __BLKSIZE_T_TYPE __SQUAD_TYPE > #define __FSID_T_TYPE struct { int __val[2]; } > #define __SSIZE_T_TYPE __SWORD_TYPE > -#define __SYSCALL_SLONG_TYPE __SLONGWORD_TYPE > -#define __SYSCALL_ULONG_TYPE __ULONGWORD_TYPE > -#define __CPU_MASK_TYPE __ULONGWORD_TYPE > +#define __SYSCALL_SLONG_TYPE __SQUAD_TYPE > +#define __SYSCALL_ULONG_TYPE __UQUAD_TYPE > +#define __CPU_MASK_TYPE __UQUAD_TYPE > > -#ifdef __LP64__ > # define __RLIM_T_MATCHES_RLIM64_T 1 > -#else > -# define __RLIM_T_MATCHES_RLIM64_T 0 > -#endif > > +#define __ASSUME_TIME64_SYSCALLS 1 > +#define __ASSUME_RLIM64_SYSCALLS 1 > > Since the sysdeps/unix/sysv/linux/generic/bits/typesizes.h definitions > generally match the kernel, anything diverging from that has the potential > of breaking it, so the difference should probably be kept to the absolute > minimum. > > I think these ones are wrong and will cause bugs similar to the clock_t > issue if they are used with kernel interfaces: > __NLINK_T_TYPE, __FSWORD_T_TYPE, __CLOCK_T_TYPE, > __BLKSIZE_T_TYPE, __SYSCALL_ULONG_TYPE, > __SYSCALL_SLONG_TYPE, __CPU_MASK_TYPE > > These are fine as long as they are only used in user space and to > wrap kernel syscalls, but I think most of them can end up being > passed to the kernel, so it seems safer not to have rv32 diverge > without a good reason. > > The remaining ones (__INO_T_TYPE, __OFF_T_TYPE, __BLKCNT_T_TYPE, > __FSBLKCNT_T_TYPE, __FSFILCNT_T_TYPE, __TIME_T_TYPE) all > follow the pattern where the kernel has an old 32-bit type and a new > 64-bit type, but the kernel tries not to expose the 32-bit interfaces > to user space on new architectures and only provide the 64-bit > replacements, but there are a couple of interfaces that never got > replaced, typically in driver and file system ioctls. > > Since glibc already has code to deal with the 64-bit types and that > is well tested, it would seem safer to me to just #undef the old > types completely rather than defining them to 64-bit, which would > make them incompatible with the kernel's types. #undef-ing these results in build failures unfortunately, it seems like they are required. I'm sending a v3 RFC to the glibc list right now. We can continue the discussion there. Alistair > > Arnd