LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb.de>
To: Andreas Schwab <schwab@suse.de>
Cc: Alistair Francis <alistair23@gmail.com>,
	Alistair Francis <alistair.francis@wdc.com>,
	linux-riscv@lists.infradead.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RESEND 0/2] RISC-V: Handle the siginfo_t offset problem
Date: Thu, 4 Jul 2019 11:19:13 +0200	[thread overview]
Message-ID: <CAK8P3a2NmdoHzFGKrzw4CBYDOBtZHDQCGsWE_L0UbG+w0bGWkA@mail.gmail.com> (raw)
In-Reply-To: <mvmpnmqfepx.fsf@suse.de>

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

  reply	other threads:[~2019-07-04  9:19 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-03  0:52 Alistair Francis
2019-07-03  0:52 ` [PATCH RESEND 1/2] uapi/asm-generic: Allow defining a custom __SIGINFO struct Alistair Francis
2019-07-03  0:52 ` [PATCH RESEND 2/2] riscv/include/uapi: Define a custom __SIGINFO struct for RV32 Alistair Francis
2019-07-03  7:08 ` [PATCH RESEND 0/2] RISC-V: Handle the siginfo_t offset problem Andreas Schwab
2019-07-03 18:40   ` Alistair Francis
2019-07-04  7:20     ` Andreas Schwab
2019-07-04  9:19       ` Arnd Bergmann [this message]
2019-07-17  0:02         ` Alistair Francis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAK8P3a2NmdoHzFGKrzw4CBYDOBtZHDQCGsWE_L0UbG+w0bGWkA@mail.gmail.com \
    --to=arnd@arndb.de \
    --cc=alistair.francis@wdc.com \
    --cc=alistair23@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=schwab@suse.de \
    --subject='Re: [PATCH RESEND 0/2] RISC-V: Handle the siginfo_t offset problem' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).