diff mbox

Update siginfo constants from Linux kernel (bug 21286)

Message ID CAMe9rOo4WVe6peMR=jHCU3SvU-9ABWinskaybWsRzt492_WNyQ@mail.gmail.com
State New, archived
Headers show

Commit Message

H.J. Lu Sept. 3, 2018, 1:35 p.m. UTC
On Thu, Aug 30, 2018 at 8:58 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> As of Linux 4.17, siginfo headers in the Linux kernel have been
> largely unified across architectures (so various constants are defined
> with common values in include/uapi/asm-generic/siginfo.h even if not
> all architectures can generate those particular constants).
>
> This patch makes glibc reflect that unification and the current set of
> constants in that header as of Linux 4.18.  Various constants are
> added to bits/siginfo-consts.h (under the same feature test macro
> conditions as the other constants with the same prefix), and removed
> from the ia64 bits/siginfo-consts-arch.h where they were previously
> there - this is not limited to constants added by the unification.
> Nothing is done about macros that are defined in
> include/uapi/asm-generic/siginfo.h with names with leading '__' (some
> of those are ia64-specific ones that remain in the ia64
> bits/siginfo-consts-arch.h without the leading '__' there).
>
> A consequence of these changes is that TRAP_HWBKPT becomes available
> on AArch64 and all other architectures as requested in bug 21286.
>
> Tested for x86_64; tested with build-many-glibcs.py for ia64.
>
> 2018-08-30  Joseph Myers  <joseph@codesourcery.com>
>
>         [BZ #21286]
>         * sysdeps/unix/sysv/linux/bits/siginfo-consts.h (SI_DETHREAD): New
>         constant.
>         [__USE_XOPEN_EXTENDED || __USE_XOPEN2K8] (ILL_BADIADDR): Likewise.
>         [__USE_XOPEN_EXTENDED || __USE_XOPEN2K8] (FPE_FLTUNK): Likewise.
>         [__USE_XOPEN_EXTENDED || __USE_XOPEN2K8] (FPE_CONDTRAP): Likewise.
>         [__USE_XOPEN_EXTENDED || __USE_XOPEN2K8] (SEGV_ACCADI): Likewise.
>         [__USE_XOPEN_EXTENDED || __USE_XOPEN2K8] (SEGV_ADIDERR): Likewise.
>         [__USE_XOPEN_EXTENDED || __USE_XOPEN2K8] (SEGV_ADIPERR): Likewise.
>         [__USE_XOPEN_EXTENDED] (TRAP_BRANCH): Likewise.
>         [__USE_XOPEN_EXTENDED] (TRAP_HWBKPT): Likewise.
>         [__USE_XOPEN_EXTENDED] (TRAP_UNK): Likweise.
>         * sysdeps/unix/sysv/linux/ia64/bits/siginfo-consts-arch.h
>         (ILL_BADIADDR): Remove constant.
>         (TRAP_BRANCH): Likewise.
>         (TRAP_HWBKPT): Likewise.
>
> diff --git a/sysdeps/unix/sysv/linux/bits/siginfo-consts.h b/sysdeps/unix/sysv/linux/bits/siginfo-consts.h
> index 193bd9c471..d69d27d922 100644
> --- a/sysdeps/unix/sysv/linux/bits/siginfo-consts.h
> +++ b/sysdeps/unix/sysv/linux/bits/siginfo-consts.h
> @@ -35,7 +35,9 @@
>  enum
>  {
>    SI_ASYNCNL = -60,            /* Sent by asynch name lookup completion.  */
> -  SI_TKILL = -6,               /* Sent by tkill.  */
> +  SI_DETHREAD = -7,            /* Sent by execve killing subsidiary
> +                                  threads.  */

Wasn't SI_DETHREAD there in 2012?  Should it be a separate patch?

> +  SI_TKILL,                    /* Sent by tkill.  */
>    SI_SIGIO,                    /* Sent by queued SIGIO. */
>  #if __SI_ASYNCIO_AFTER_SIGIO
>    SI_ASYNCIO,                  /* Sent by AIO completion.  */
> @@ -51,6 +53,7 @@ enum
>    SI_KERNEL = 0x80             /* Send by kernel.  */
>
>  #define SI_ASYNCNL     SI_ASYNCNL
> +#define SI_DETHREAD    SI_DETHREAD
>  #define SI_TKILL       SI_TKILL
>  #define SI_SIGIO       SI_SIGIO
>  #define SI_ASYNCIO     SI_ASYNCIO
> @@ -81,8 +84,10 @@ enum
>  #  define ILL_PRVREG   ILL_PRVREG
>    ILL_COPROC,                  /* Coprocessor error.  */
>  #  define ILL_COPROC   ILL_COPROC
> -  ILL_BADSTK                   /* Internal stack error.  */
> +  ILL_BADSTK,                  /* Internal stack error.  */
>  #  define ILL_BADSTK   ILL_BADSTK
> +  ILL_BADIADDR                 /* Unimplemented instruction address.  */
> +#  define ILL_BADIADDR ILL_BADIADDR
>  };

Kernel commit has

commit a402ab8cc7b0578c445f348c9010e62ab390bee8
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Thu Mar 15 13:30:51 2018 +0100

    asm-generic: siginfo: define ia64 si_codes unconditionally

    Unlike system call numbers the assignment of si_codes has never had a
    reason to be made per architecture.  Some architectures have had unique
    conditions to report and reporting those conditions needed new si_codes.
    Nothing has ever needed si_codes to have different values on different
    architectures.  The si_code space is vast so even with defining all
    si_codes on all architectures there is no danger in running out of
    si_code values.

    The history of the si_codes BUS_MCEERR_AR, BUS_MCEER_AO, SEGV_BNDERR,
    and SEGV_PKUERR show that a need of one architecture frequently becomes
    a need of another architecture which makes sharing si_codes between
    architectures a positive benefit and something to be encouraged.

    Where there are no conflicts with the historical ia64 arch specific
    si_codes and any other si_codes make them generic si_codes.  We might
    need them on another architecture someday.

    This leaves only the good example of arch generic si_codes in the kernel
    for future architectures and architecture enhancments to follow.
    Without bad examples to follow it should be easy to avoid the mistakes
    of the past.

    Reported-by: Eric W. Biederman <ebiederm@xmission.com>
    [arnd: took Eric's changelog text]
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>


Why are only parts of it implemented here?

>  /* `si_code' values for SIGFPE signal.  */
> @@ -102,8 +107,12 @@ enum
>  #  define FPE_FLTRES   FPE_FLTRES
>    FPE_FLTINV,                  /* Floating point invalid operation.  */
>  #  define FPE_FLTINV   FPE_FLTINV
> -  FPE_FLTSUB                   /* Subscript out of range.  */
> +  FPE_FLTSUB,                  /* Subscript out of range.  */
>  #  define FPE_FLTSUB   FPE_FLTSUB
> +  FPE_FLTUNK = 14,             /* Undiagnosed floating-point exception.  */
> +#  define FPE_FLTUNK   FPE_FLTUNK
> +  FPE_CONDTRAP                 /* Trap on condition.  */
> +#  define FPE_CONDTRAP FPE_CONDTRAP
>  };
>
>  /* `si_code' values for SIGSEGV signal.  */
> @@ -115,8 +124,14 @@ enum
>  #  define SEGV_ACCERR  SEGV_ACCERR
>    SEGV_BNDERR,                 /* Bounds checking failure.  */
>  #  define SEGV_BNDERR  SEGV_BNDERR
> -  SEGV_PKUERR                  /* Protection key checking failure.  */
> +  SEGV_PKUERR,                 /* Protection key checking failure.  */
>  #  define SEGV_PKUERR  SEGV_PKUERR
> +  SEGV_ACCADI,                 /* ADI not enabled for mapped object.  */
> +#  define SEGV_ACCADI  SEGV_ACCADI
> +  SEGV_ADIDERR,                        /* Disrupting MCD error.  */
> +#  define SEGV_ADIDERR SEGV_ADIDERR
> +  SEGV_ADIPERR                 /* Precise MCD exception.  */
> +#  define SEGV_ADIPERR SEGV_ADIPERR
>  };
>
>  /* `si_code' values for SIGBUS signal.  */
> @@ -141,8 +156,14 @@ enum
>  {
>    TRAP_BRKPT = 1,              /* Process breakpoint.  */
>  #  define TRAP_BRKPT   TRAP_BRKPT
> -  TRAP_TRACE                   /* Process trace trap.  */
> +  TRAP_TRACE,                  /* Process trace trap.  */
>  #  define TRAP_TRACE   TRAP_TRACE
> +  TRAP_BRANCH,                 /* Process taken branch trap.  */
> +#  define TRAP_BRANCH  TRAP_BRANCH
> +  TRAP_HWBKPT,                 /* Hardware breakpoint/watchpoint.  */
> +#  define TRAP_HWBKPT  TRAP_HWBKPT
> +  TRAP_UNK                     /* Undiagnosed trap.  */
> +#  define TRAP_UNK     TRAP_UNK
>  };
>  # endif
>
> diff --git a/sysdeps/unix/sysv/linux/ia64/bits/siginfo-consts-arch.h b/sysdeps/unix/sysv/linux/ia64/bits/siginfo-consts-arch.h
> index 4c5c4da516..5ef8af4ac7 100644
> --- a/sysdeps/unix/sysv/linux/ia64/bits/siginfo-consts-arch.h
> +++ b/sysdeps/unix/sysv/linux/ia64/bits/siginfo-consts-arch.h
> @@ -5,9 +5,7 @@
>  /* `si_code' values for SIGILL signal.  */
>  enum
>  {
> -  ILL_BADIADDR = ILL_BADSTK + 1, /* Unimplemented instruction address. */
> -#define ILL_BADIADDR ILL_BADIADDR
> -  ILL_BREAK
> +  ILL_BREAK = ILL_BADIADDR + 1
>  #define ILL_BREAK ILL_BREAK
>  };
>
> @@ -33,13 +31,4 @@ enum
>  #define SEGV_PSTKOVF SEGV_PSTKOVF
>  };
>
> -/* `si_code' values for SIGTRAP signal.  */
> -enum
> -{
> -  TRAP_BRANCH = TRAP_TRACE + 1,
> -#define TRAP_BRANCH TRAP_BRANCH
> -  TRAP_HWBKPT
> -#define TRAP_HWBKPT TRAP_HWBKPT
> -};
> -
>  #endif
>
> --
> Joseph S. Myers
> joseph@codesourcery.com

Comments

Joseph Myers Sept. 3, 2018, 1:51 p.m. UTC | #1
On Mon, 3 Sep 2018, H.J. Lu wrote:

> > diff --git a/sysdeps/unix/sysv/linux/bits/siginfo-consts.h b/sysdeps/unix/sysv/linux/bits/siginfo-consts.h
> > index 193bd9c471..d69d27d922 100644
> > --- a/sysdeps/unix/sysv/linux/bits/siginfo-consts.h
> > +++ b/sysdeps/unix/sysv/linux/bits/siginfo-consts.h
> > @@ -35,7 +35,9 @@
> >  enum
> >  {
> >    SI_ASYNCNL = -60,            /* Sent by asynch name lookup completion.  */
> > -  SI_TKILL = -6,               /* Sent by tkill.  */
> > +  SI_DETHREAD = -7,            /* Sent by execve killing subsidiary
> > +                                  threads.  */
> 
> Wasn't SI_DETHREAD there in 2012?  Should it be a separate patch?

The patch is generically making the header reflect the current set of 
constants in the kernel, without regard to when the kernel added them in 
what header.

> Why are only parts of it implemented here?

As I said:

    Nothing is done about macros that are defined in
    include/uapi/asm-generic/siginfo.h with names with leading '__' (some
    of those are ia64-specific ones that remain in the ia64
    bits/siginfo-consts-arch.h without the leading '__' there).

If the kernel defines __SOMETHING in the generic header, I'm not treating 
that as meaning that SOMETHING without the leading '__' should thereby 
become part of the generic API (whether or not it is part of the ia64 
API).
H.J. Lu Sept. 3, 2018, 2:30 p.m. UTC | #2
On Mon, Sep 3, 2018 at 6:51 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Mon, 3 Sep 2018, H.J. Lu wrote:
>
>> > diff --git a/sysdeps/unix/sysv/linux/bits/siginfo-consts.h b/sysdeps/unix/sysv/linux/bits/siginfo-consts.h
>> > index 193bd9c471..d69d27d922 100644
>> > --- a/sysdeps/unix/sysv/linux/bits/siginfo-consts.h
>> > +++ b/sysdeps/unix/sysv/linux/bits/siginfo-consts.h
>> > @@ -35,7 +35,9 @@
>> >  enum
>> >  {
>> >    SI_ASYNCNL = -60,            /* Sent by asynch name lookup completion.  */
>> > -  SI_TKILL = -6,               /* Sent by tkill.  */
>> > +  SI_DETHREAD = -7,            /* Sent by execve killing subsidiary
>> > +                                  threads.  */
>>
>> Wasn't SI_DETHREAD there in 2012?  Should it be a separate patch?
>
> The patch is generically making the header reflect the current set of
> constants in the kernel, without regard to when the kernel added them in
> what header.

A separate patch for SI_DETHREAD is much easier to review.  It
may be useful for backport.  Can you make a separate patch for
SI_DETHREAD?

>> Why are only parts of it implemented here?
>
> As I said:
>
>     Nothing is done about macros that are defined in
>     include/uapi/asm-generic/siginfo.h with names with leading '__' (some
>     of those are ia64-specific ones that remain in the ia64
>     bits/siginfo-consts-arch.h without the leading '__' there).
>
> If the kernel defines __SOMETHING in the generic header, I'm not treating
> that as meaning that SOMETHING without the leading '__' should thereby
> become part of the generic API (whether or not it is part of the ia64
> API).
>

That makes senses.

Thanks.
Joseph Myers Sept. 3, 2018, 3:31 p.m. UTC | #3
On Mon, 3 Sep 2018, H.J. Lu wrote:

> >> Wasn't SI_DETHREAD there in 2012?  Should it be a separate patch?
> >
> > The patch is generically making the header reflect the current set of
> > constants in the kernel, without regard to when the kernel added them in
> > what header.
> 
> A separate patch for SI_DETHREAD is much easier to review.  It
> may be useful for backport.  Can you make a separate patch for
> SI_DETHREAD?

I don't see it as useful.  It doesn't reflect the nature of the patch 
(which was based on reviewing against Linux 4.18, not checking for each 
constant when it was added to the kernel; I doubt that would be the only 
split based on kernel versions).
diff mbox

Patch

diff --git a/include/uapi/asm-generic/siginfo.h
b/include/uapi/asm-generic/siginfo.h
index b2ebf16c391a..ff13ed50dde8 100644
--- a/include/uapi/asm-generic/siginfo.h
+++ b/include/uapi/asm-generic/siginfo.h
@@ -186,11 +186,9 @@  typedef struct siginfo {
 #define ILL_PRVREG 6 /* privileged register */
 #define ILL_COPROC 7 /* coprocessor error */
 #define ILL_BADSTK 8 /* internal stack error */
-#ifdef __ia64__
-# define ILL_BADIADDR 9 /* unimplemented instruction address */
-# define __ILL_BREAK 10 /* illegal break */
-# define __ILL_BNDMOD 11 /* bundle-update (modification) in progress */
-#endif
+#define ILL_BADIADDR 9 /* unimplemented instruction address */
+#define __ILL_BREAK 10 /* illegal break */
+#define __ILL_BNDMOD 11 /* bundle-update (modification) in progress */
 #define NSIGILL 11

 /*
@@ -204,13 +202,11 @@  typedef struct siginfo {
 #define FPE_FLTRES 6 /* floating point inexact result */
 #define FPE_FLTINV 7 /* floating point invalid operation */
 #define FPE_FLTSUB 8 /* subscript out of range */
-#ifdef __ia64__
-# define __FPE_DECOVF 9 /* decimal overflow */
-# define __FPE_DECDIV 10 /* decimal division by zero */
-# define __FPE_DECERR 11 /* packed decimal error */
-# define __FPE_INVASC 12 /* invalid ASCII digit */
-# define __FPE_INVDEC 13 /* invalid decimal digit */
-#endif
+#define __FPE_DECOVF 9 /* decimal overflow */
+#define __FPE_DECDIV 10 /* decimal division by zero */
+#define __FPE_DECERR 11 /* packed decimal error */
+#define __FPE_INVASC 12 /* invalid ASCII digit */
+#define __FPE_INVDEC 13 /* invalid decimal digit */
 #define NSIGFPE 13

 /*