Re: [PATCH RESEND 0/2] RISC-V: Handle the siginfo_t offset problem
On Thu, Jul 4, 2019 at 2:19 AM Arnd Bergmann wrote: > > On Thu, Jul 4, 2019 at 9:20 AM Andreas Schwab wrote: > > > > On Jul 03 2019, Alistair Francis wrote: > > > > > On Wed, Jul 3, 2019 at 12:08 AM Andreas Schwab wrote: > > >> > > >> On Jul 02 2019, Alistair Francis 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_TYPEstruct { 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_T1 > -#else > -# define __RLIM_T_MATCHES_RLIM64_T0 > -#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
Re: [PATCH RESEND 0/2] RISC-V: Handle the siginfo_t offset problem
On Thu, Jul 4, 2019 at 9:20 AM Andreas Schwab wrote: > > On Jul 03 2019, Alistair Francis wrote: > > > On Wed, Jul 3, 2019 at 12:08 AM Andreas Schwab wrote: > >> > >> On Jul 02 2019, Alistair Francis 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_TYPEstruct { 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_T1 -#else -# define __RLIM_T_MATCHES_RLIM64_T0 -#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
Re: [PATCH RESEND 0/2] RISC-V: Handle the siginfo_t offset problem
On Jul 03 2019, Alistair Francis wrote: > On Wed, Jul 3, 2019 at 12:08 AM Andreas Schwab wrote: >> >> On Jul 02 2019, Alistair Francis 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. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [PATCH RESEND 0/2] RISC-V: Handle the siginfo_t offset problem
On Wed, Jul 3, 2019 at 12:08 AM Andreas Schwab wrote: > > On Jul 02 2019, Alistair Francis 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, sch...@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different."
Re: [PATCH RESEND 0/2] RISC-V: Handle the siginfo_t offset problem
On Jul 02 2019, Alistair Francis 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. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
[PATCH RESEND 0/2] RISC-V: Handle the siginfo_t offset problem
Resending the the correct linux-riscv address. 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) while in the kernel wordl the _sifields union is 4 bytes alligned. This results in information being lost in the waitid syscall. This doesn't seem to be a great fix, but it is somewhat similar to what x32 does (which has 64-bit time_t like RV32) and I can't figure out why the two allignments are different. 1: https://github.com/alistair23/glibc/commits/alistair/rv32.next Alistair Francis (2): uapi/asm-generic: Allow defining a custom __SIGINFO struct riscv/include/uapi: Define a custom __SIGINFO struct for RV32 arch/riscv/include/uapi/asm/siginfo.h | 32 +++ include/uapi/asm-generic/siginfo.h| 32 ++- 2 files changed, 49 insertions(+), 15 deletions(-) create mode 100644 arch/riscv/include/uapi/asm/siginfo.h -- 2.22.0