| Message ID | CAMe9rOo4WVe6peMR=jHCU3SvU-9ABWinskaybWsRzt492_WNyQ@mail.gmail.com |
|---|---|
| State | New, archived |
| Headers |
Received: (qmail 47375 invoked by alias); 3 Sep 2018 13:35:52 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 47361 invoked by uid 89); 3 Sep 2018 13:35:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=queued, danger, 59, Protection X-HELO: mail-oi0-f66.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=8IdWpr005XHdnZmOp+GlcYtLKI8veLo2R6m67nU+k7o=; b=IVexfDI/5P109maqzryuqgogH7ckR1KSIaxlu6sOzK1qktK76akhGzx95o+YZ2Ima8 8oReE80eqp/NN9gN6ws95BhtD/3ckSgGTbm8zk9MpYBVqrtOXt9PpbgkThzh0syQR38H 7oSavfLLdduy5rNBIR4qkWcfNnnWxoGtGGWvY7+H6FkmXtDHdYNu1Aa4gH9LBSMuwdyj f5iq/l1eieCqWroIOEmRTi3Y27cupUfnwMYB+95WMpmc+5/JHGe7wMRqWdZmdJ/t0+O6 qGRejbooAkL7OoKOmiXfPK8xH/nTOX2UXowJfdWIIq7M5VXFwMzGohmcUzZDrdhRudpt M+iA== MIME-Version: 1.0 In-Reply-To: <alpine.DEB.2.21.1808301557410.4027@digraph.polyomino.org.uk> References: <alpine.DEB.2.21.1808301557410.4027@digraph.polyomino.org.uk> From: "H.J. Lu" <hjl.tools@gmail.com> Date: Mon, 3 Sep 2018 06:35:47 -0700 Message-ID: <CAMe9rOo4WVe6peMR=jHCU3SvU-9ABWinskaybWsRzt492_WNyQ@mail.gmail.com> Subject: Re: Update siginfo constants from Linux kernel (bug 21286) To: Joseph Myers <joseph@codesourcery.com> Cc: GNU C Library <libc-alpha@sourceware.org> Content-Type: text/plain; charset="UTF-8" |
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
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).
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.
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 --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 /*