* [musl] SA_RESTORER for rv64?
@ 2022-11-10 15:44 enh
2022-11-10 17:18 ` Rich Felker
0 siblings, 1 reply; 10+ messages in thread
From: enh @ 2022-11-10 15:44 UTC (permalink / raw)
To: musl
[-- Attachment #1: Type: text/plain, Size: 282 bytes --]
arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
the initial commit, but i think that's just copy & paste from whichever
architecture the rv64 headers were based on? the linux kernel itself
doesn't have SA_RESTORER for rv64, unless i'm missing something?
[-- Attachment #2: Type: text/html, Size: 320 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] SA_RESTORER for rv64?
2022-11-10 15:44 [musl] SA_RESTORER for rv64? enh
@ 2022-11-10 17:18 ` Rich Felker
2022-11-10 17:26 ` enh
2022-11-10 17:31 ` Khem Raj
0 siblings, 2 replies; 10+ messages in thread
From: Rich Felker @ 2022-11-10 17:18 UTC (permalink / raw)
To: enh; +Cc: musl
On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
> the initial commit, but i think that's just copy & paste from whichever
> architecture the rv64 headers were based on? the linux kernel itself
> doesn't have SA_RESTORER for rv64, unless i'm missing something?
I suspect this is just a mistake. Have you seen any ill effects from
it? If riscv folks can confirm it's wrong, I'll remove it.
Rich
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] SA_RESTORER for rv64?
2022-11-10 17:18 ` Rich Felker
@ 2022-11-10 17:26 ` enh
2022-11-10 17:31 ` Khem Raj
1 sibling, 0 replies; 10+ messages in thread
From: enh @ 2022-11-10 17:26 UTC (permalink / raw)
To: dalias; +Cc: musl
[-- Attachment #1: Type: text/plain, Size: 834 bytes --]
On Thu, Nov 10, 2022 at 9:19 AM Rich Felker <dalias@libc.org> wrote:
> On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> > arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER
> since
> > the initial commit, but i think that's just copy & paste from whichever
> > architecture the rv64 headers were based on? the linux kernel itself
> > doesn't have SA_RESTORER for rv64, unless i'm missing something?
>
> I suspect this is just a mistake. Have you seen any ill effects from
> it? If riscv folks can confirm it's wrong, I'll remove it.
>
i noticed this by inspection after someone had some code that (correctly)
didn't compile with bionic or glibc but did with musl. in addition to
rejecting the proposal to add a rv64 SA_RESTORER to bionic, i said i'd
reach out to musl about removing it there :-)
> Rich
>
[-- Attachment #2: Type: text/html, Size: 1371 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] SA_RESTORER for rv64?
2022-11-10 17:18 ` Rich Felker
2022-11-10 17:26 ` enh
@ 2022-11-10 17:31 ` Khem Raj
2023-02-03 18:44 ` enh
1 sibling, 1 reply; 10+ messages in thread
From: Khem Raj @ 2022-11-10 17:31 UTC (permalink / raw)
To: musl; +Cc: enh
On Thu, Nov 10, 2022 at 9:19 AM Rich Felker <dalias@libc.org> wrote:
>
> On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> > arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
> > the initial commit, but i think that's just copy & paste from whichever
> > architecture the rv64 headers were based on? the linux kernel itself
> > doesn't have SA_RESTORER for rv64, unless i'm missing something?
>
> I suspect this is just a mistake. Have you seen any ill effects from
> it? If riscv folks can confirm it's wrong, I'll remove it.
Yeah I think it should be removed. Perhaps mips is in same boat.
>
> Rich
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] SA_RESTORER for rv64?
2022-11-10 17:31 ` Khem Raj
@ 2023-02-03 18:44 ` enh
2023-02-05 23:54 ` Rich Felker
0 siblings, 1 reply; 10+ messages in thread
From: enh @ 2023-02-03 18:44 UTC (permalink / raw)
To: Khem Raj; +Cc: musl
[-- Attachment #1: Type: text/plain, Size: 783 bytes --]
oops, never actually sent the patch. attached...
On Thu, Nov 10, 2022 at 9:31 AM Khem Raj <raj.khem@gmail.com> wrote:
>
> On Thu, Nov 10, 2022 at 9:19 AM Rich Felker <dalias@libc.org> wrote:
> >
> > On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> > > arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
> > > the initial commit, but i think that's just copy & paste from whichever
> > > architecture the rv64 headers were based on? the linux kernel itself
> > > doesn't have SA_RESTORER for rv64, unless i'm missing something?
> >
> > I suspect this is just a mistake. Have you seen any ill effects from
> > it? If riscv folks can confirm it's wrong, I'll remove it.
>
> Yeah I think it should be removed. Perhaps mips is in same boat.
>
> >
> > Rich
[-- Attachment #2: 0001-risc-v-does-not-have-SA_RESTORER.patch --]
[-- Type: text/x-patch, Size: 795 bytes --]
From 6413de6d9f785c98e5bc0cf40be947f1169d2fd7 Mon Sep 17 00:00:00 2001
From: Elliott Hughes <enh@google.com>
Date: Fri, 3 Feb 2023 10:42:55 -0800
Subject: [PATCH] risc-v does not have SA_RESTORER.
The kernel's include/uapi/asm-generic/signal-defs.h explicitly calls
this out as obsolete. New architectures like risc-v do not define it.
---
arch/riscv64/bits/signal.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
index 287367db..fd6157a3 100644
--- a/arch/riscv64/bits/signal.h
+++ b/arch/riscv64/bits/signal.h
@@ -76,7 +76,6 @@ typedef struct __ucontext
#define SA_RESTART 0x10000000
#define SA_NODEFER 0x40000000
#define SA_RESETHAND 0x80000000
-#define SA_RESTORER 0x04000000
#endif
--
2.39.1.519.gcb327c4b5f-goog
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] SA_RESTORER for rv64?
2023-02-03 18:44 ` enh
@ 2023-02-05 23:54 ` Rich Felker
2023-02-06 16:51 ` enh
0 siblings, 1 reply; 10+ messages in thread
From: Rich Felker @ 2023-02-05 23:54 UTC (permalink / raw)
To: enh; +Cc: Khem Raj, musl
On Fri, Feb 03, 2023 at 10:44:56AM -0800, enh wrote:
> oops, never actually sent the patch. attached...
>
> On Thu, Nov 10, 2022 at 9:31 AM Khem Raj <raj.khem@gmail.com> wrote:
> >
> > On Thu, Nov 10, 2022 at 9:19 AM Rich Felker <dalias@libc.org> wrote:
> > >
> > > On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> > > > arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
> > > > the initial commit, but i think that's just copy & paste from whichever
> > > > architecture the rv64 headers were based on? the linux kernel itself
> > > > doesn't have SA_RESTORER for rv64, unless i'm missing something?
> > >
> > > I suspect this is just a mistake. Have you seen any ill effects from
> > > it? If riscv folks can confirm it's wrong, I'll remove it.
> >
> > Yeah I think it should be removed. Perhaps mips is in same boat.
> >
> > >
> > > Rich
> From 6413de6d9f785c98e5bc0cf40be947f1169d2fd7 Mon Sep 17 00:00:00 2001
> From: Elliott Hughes <enh@google.com>
> Date: Fri, 3 Feb 2023 10:42:55 -0800
> Subject: [PATCH] risc-v does not have SA_RESTORER.
>
> The kernel's include/uapi/asm-generic/signal-defs.h explicitly calls
> this out as obsolete. New architectures like risc-v do not define it.
> ---
> arch/riscv64/bits/signal.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
> index 287367db..fd6157a3 100644
> --- a/arch/riscv64/bits/signal.h
> +++ b/arch/riscv64/bits/signal.h
> @@ -76,7 +76,6 @@ typedef struct __ucontext
> #define SA_RESTART 0x10000000
> #define SA_NODEFER 0x40000000
> #define SA_RESETHAND 0x80000000
> -#define SA_RESTORER 0x04000000
>
> #endif
>
> --
> 2.39.1.519.gcb327c4b5f-goog
>
I don't think this patch works as-is, since musl unconditionally uses
SA_RESTORER. We probably need to make that conditional on its
presence, and it looks like there's also a wrong-struct-layout issue
on archs where it's absent...
Rich
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] SA_RESTORER for rv64?
2023-02-05 23:54 ` Rich Felker
@ 2023-02-06 16:51 ` enh
2023-02-06 17:49 ` Rich Felker
0 siblings, 1 reply; 10+ messages in thread
From: enh @ 2023-02-06 16:51 UTC (permalink / raw)
To: Rich Felker; +Cc: Khem Raj, musl
On Sun, Feb 5, 2023 at 3:54 PM Rich Felker <dalias@libc.org> wrote:
>
> On Fri, Feb 03, 2023 at 10:44:56AM -0800, enh wrote:
> > oops, never actually sent the patch. attached...
> >
> > On Thu, Nov 10, 2022 at 9:31 AM Khem Raj <raj.khem@gmail.com> wrote:
> > >
> > > On Thu, Nov 10, 2022 at 9:19 AM Rich Felker <dalias@libc.org> wrote:
> > > >
> > > > On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> > > > > arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
> > > > > the initial commit, but i think that's just copy & paste from whichever
> > > > > architecture the rv64 headers were based on? the linux kernel itself
> > > > > doesn't have SA_RESTORER for rv64, unless i'm missing something?
> > > >
> > > > I suspect this is just a mistake. Have you seen any ill effects from
> > > > it? If riscv folks can confirm it's wrong, I'll remove it.
> > >
> > > Yeah I think it should be removed. Perhaps mips is in same boat.
> > >
> > > >
> > > > Rich
>
> > From 6413de6d9f785c98e5bc0cf40be947f1169d2fd7 Mon Sep 17 00:00:00 2001
> > From: Elliott Hughes <enh@google.com>
> > Date: Fri, 3 Feb 2023 10:42:55 -0800
> > Subject: [PATCH] risc-v does not have SA_RESTORER.
> >
> > The kernel's include/uapi/asm-generic/signal-defs.h explicitly calls
> > this out as obsolete. New architectures like risc-v do not define it.
> > ---
> > arch/riscv64/bits/signal.h | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
> > index 287367db..fd6157a3 100644
> > --- a/arch/riscv64/bits/signal.h
> > +++ b/arch/riscv64/bits/signal.h
> > @@ -76,7 +76,6 @@ typedef struct __ucontext
> > #define SA_RESTART 0x10000000
> > #define SA_NODEFER 0x40000000
> > #define SA_RESETHAND 0x80000000
> > -#define SA_RESTORER 0x04000000
> >
> > #endif
> >
> > --
> > 2.39.1.519.gcb327c4b5f-goog
> >
>
> I don't think this patch works as-is, since musl unconditionally uses
> SA_RESTORER. We probably need to make that conditional on its
> presence, and it looks like there's also a wrong-struct-layout issue
> on archs where it's absent...
yeah, bionic just uses the kernel uapi headers directly, and they look
like this:
struct sigaction {
__sighandler_t sa_handler;
unsigned long sa_flags;
#ifdef SA_RESTORER
__sigrestore_t sa_restorer;
#endif
sigset_t sa_mask;
};
> Rich
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] SA_RESTORER for rv64?
2023-02-06 16:51 ` enh
@ 2023-02-06 17:49 ` Rich Felker
2023-02-08 21:45 ` Rich Felker
0 siblings, 1 reply; 10+ messages in thread
From: Rich Felker @ 2023-02-06 17:49 UTC (permalink / raw)
To: enh; +Cc: Khem Raj, musl
On Mon, Feb 06, 2023 at 08:51:13AM -0800, enh wrote:
> On Sun, Feb 5, 2023 at 3:54 PM Rich Felker <dalias@libc.org> wrote:
> >
> > On Fri, Feb 03, 2023 at 10:44:56AM -0800, enh wrote:
> > > oops, never actually sent the patch. attached...
> > >
> > > On Thu, Nov 10, 2022 at 9:31 AM Khem Raj <raj.khem@gmail.com> wrote:
> > > >
> > > > On Thu, Nov 10, 2022 at 9:19 AM Rich Felker <dalias@libc.org> wrote:
> > > > >
> > > > > On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> > > > > > arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
> > > > > > the initial commit, but i think that's just copy & paste from whichever
> > > > > > architecture the rv64 headers were based on? the linux kernel itself
> > > > > > doesn't have SA_RESTORER for rv64, unless i'm missing something?
> > > > >
> > > > > I suspect this is just a mistake. Have you seen any ill effects from
> > > > > it? If riscv folks can confirm it's wrong, I'll remove it.
> > > >
> > > > Yeah I think it should be removed. Perhaps mips is in same boat.
> > > >
> > > > >
> > > > > Rich
> >
> > > From 6413de6d9f785c98e5bc0cf40be947f1169d2fd7 Mon Sep 17 00:00:00 2001
> > > From: Elliott Hughes <enh@google.com>
> > > Date: Fri, 3 Feb 2023 10:42:55 -0800
> > > Subject: [PATCH] risc-v does not have SA_RESTORER.
> > >
> > > The kernel's include/uapi/asm-generic/signal-defs.h explicitly calls
> > > this out as obsolete. New architectures like risc-v do not define it.
> > > ---
> > > arch/riscv64/bits/signal.h | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
> > > index 287367db..fd6157a3 100644
> > > --- a/arch/riscv64/bits/signal.h
> > > +++ b/arch/riscv64/bits/signal.h
> > > @@ -76,7 +76,6 @@ typedef struct __ucontext
> > > #define SA_RESTART 0x10000000
> > > #define SA_NODEFER 0x40000000
> > > #define SA_RESETHAND 0x80000000
> > > -#define SA_RESTORER 0x04000000
> > >
> > > #endif
> > >
> > > --
> > > 2.39.1.519.gcb327c4b5f-goog
> > >
> >
> > I don't think this patch works as-is, since musl unconditionally uses
> > SA_RESTORER. We probably need to make that conditional on its
> > presence, and it looks like there's also a wrong-struct-layout issue
> > on archs where it's absent...
>
> yeah, bionic just uses the kernel uapi headers directly, and they look
> like this:
>
> struct sigaction {
> __sighandler_t sa_handler;
> unsigned long sa_flags;
> #ifdef SA_RESTORER
> __sigrestore_t sa_restorer;
> #endif
> sigset_t sa_mask;
> };
OK. It looks like we need to remove the wrong SA_RESTORER for archs
that aren't supposed to have it *and* add such an #ifdef. Right now,
we're passing bogus sa_mask on these archs... :(
Rich
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] SA_RESTORER for rv64?
2023-02-06 17:49 ` Rich Felker
@ 2023-02-08 21:45 ` Rich Felker
2023-02-08 21:53 ` enh
0 siblings, 1 reply; 10+ messages in thread
From: Rich Felker @ 2023-02-08 21:45 UTC (permalink / raw)
To: enh; +Cc: Khem Raj, musl
[-- Attachment #1: Type: text/plain, Size: 2964 bytes --]
On Mon, Feb 06, 2023 at 12:49:53PM -0500, Rich Felker wrote:
> On Mon, Feb 06, 2023 at 08:51:13AM -0800, enh wrote:
> > On Sun, Feb 5, 2023 at 3:54 PM Rich Felker <dalias@libc.org> wrote:
> > >
> > > On Fri, Feb 03, 2023 at 10:44:56AM -0800, enh wrote:
> > > > oops, never actually sent the patch. attached...
> > > >
> > > > On Thu, Nov 10, 2022 at 9:31 AM Khem Raj <raj.khem@gmail.com> wrote:
> > > > >
> > > > > On Thu, Nov 10, 2022 at 9:19 AM Rich Felker <dalias@libc.org> wrote:
> > > > > >
> > > > > > On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> > > > > > > arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
> > > > > > > the initial commit, but i think that's just copy & paste from whichever
> > > > > > > architecture the rv64 headers were based on? the linux kernel itself
> > > > > > > doesn't have SA_RESTORER for rv64, unless i'm missing something?
> > > > > >
> > > > > > I suspect this is just a mistake. Have you seen any ill effects from
> > > > > > it? If riscv folks can confirm it's wrong, I'll remove it.
> > > > >
> > > > > Yeah I think it should be removed. Perhaps mips is in same boat.
> > > > >
> > > > > >
> > > > > > Rich
> > >
> > > > From 6413de6d9f785c98e5bc0cf40be947f1169d2fd7 Mon Sep 17 00:00:00 2001
> > > > From: Elliott Hughes <enh@google.com>
> > > > Date: Fri, 3 Feb 2023 10:42:55 -0800
> > > > Subject: [PATCH] risc-v does not have SA_RESTORER.
> > > >
> > > > The kernel's include/uapi/asm-generic/signal-defs.h explicitly calls
> > > > this out as obsolete. New architectures like risc-v do not define it.
> > > > ---
> > > > arch/riscv64/bits/signal.h | 1 -
> > > > 1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
> > > > index 287367db..fd6157a3 100644
> > > > --- a/arch/riscv64/bits/signal.h
> > > > +++ b/arch/riscv64/bits/signal.h
> > > > @@ -76,7 +76,6 @@ typedef struct __ucontext
> > > > #define SA_RESTART 0x10000000
> > > > #define SA_NODEFER 0x40000000
> > > > #define SA_RESETHAND 0x80000000
> > > > -#define SA_RESTORER 0x04000000
> > > >
> > > > #endif
> > > >
> > > > --
> > > > 2.39.1.519.gcb327c4b5f-goog
> > > >
> > >
> > > I don't think this patch works as-is, since musl unconditionally uses
> > > SA_RESTORER. We probably need to make that conditional on its
> > > presence, and it looks like there's also a wrong-struct-layout issue
> > > on archs where it's absent...
> >
> > yeah, bionic just uses the kernel uapi headers directly, and they look
> > like this:
> >
> > struct sigaction {
> > __sighandler_t sa_handler;
> > unsigned long sa_flags;
> > #ifdef SA_RESTORER
> > __sigrestore_t sa_restorer;
> > #endif
> > sigset_t sa_mask;
> > };
>
> OK. It looks like we need to remove the wrong SA_RESTORER for archs
> that aren't supposed to have it *and* add such an #ifdef. Right now,
> we're passing bogus sa_mask on these archs... :(
How does the attached look?
Rich
[-- Attachment #2: SA_RESTORER.diff --]
[-- Type: text/plain, Size: 4369 bytes --]
diff --git a/arch/microblaze/bits/signal.h b/arch/microblaze/bits/signal.h
index 490f83bf..f25b7c6a 100644
--- a/arch/microblaze/bits/signal.h
+++ b/arch/microblaze/bits/signal.h
@@ -46,7 +46,6 @@ typedef struct __ucontext {
#define SA_RESTART 0x10000000
#define SA_NODEFER 0x40000000
#define SA_RESETHAND 0x80000000
-#define SA_RESTORER 0x04000000
#endif
diff --git a/arch/mips/bits/signal.h b/arch/mips/bits/signal.h
index 1b69e762..a3b3857a 100644
--- a/arch/mips/bits/signal.h
+++ b/arch/mips/bits/signal.h
@@ -66,7 +66,6 @@ typedef struct __ucontext {
#define SA_RESTART 0x10000000
#define SA_NODEFER 0x40000000
#define SA_RESETHAND 0x80000000
-#define SA_RESTORER 0x04000000
#undef SIG_BLOCK
#undef SIG_UNBLOCK
diff --git a/arch/mips/ksigaction.h b/arch/mips/ksigaction.h
index 63fdfab0..2141a0a2 100644
--- a/arch/mips/ksigaction.h
+++ b/arch/mips/ksigaction.h
@@ -4,10 +4,6 @@ struct k_sigaction {
unsigned flags;
void (*handler)(int);
unsigned long mask[4];
- /* The following field is past the end of the structure the
- * kernel will read or write, and exists only to avoid having
- * mips-specific preprocessor conditionals in sigaction.c. */
- void (*restorer)();
};
hidden void __restore(), __restore_rt();
diff --git a/arch/mips64/bits/signal.h b/arch/mips64/bits/signal.h
index 4f91c9fc..ffec7fd0 100644
--- a/arch/mips64/bits/signal.h
+++ b/arch/mips64/bits/signal.h
@@ -85,7 +85,6 @@ typedef struct __ucontext {
#define SA_RESTART 0x10000000
#define SA_NODEFER 0x40000000
#define SA_RESETHAND 0x80000000
-#define SA_RESTORER 0x04000000
#undef SIG_BLOCK
#undef SIG_UNBLOCK
diff --git a/arch/mips64/ksigaction.h b/arch/mips64/ksigaction.h
index c16e4731..c0b73ae9 100644
--- a/arch/mips64/ksigaction.h
+++ b/arch/mips64/ksigaction.h
@@ -4,7 +4,6 @@ struct k_sigaction {
unsigned flags;
void (*handler)(int);
unsigned long mask[2];
- void (*restorer)();
};
hidden void __restore(), __restore_rt();
diff --git a/arch/mipsn32/bits/signal.h b/arch/mipsn32/bits/signal.h
index 4f91c9fc..ffec7fd0 100644
--- a/arch/mipsn32/bits/signal.h
+++ b/arch/mipsn32/bits/signal.h
@@ -85,7 +85,6 @@ typedef struct __ucontext {
#define SA_RESTART 0x10000000
#define SA_NODEFER 0x40000000
#define SA_RESETHAND 0x80000000
-#define SA_RESTORER 0x04000000
#undef SIG_BLOCK
#undef SIG_UNBLOCK
diff --git a/arch/mipsn32/ksigaction.h b/arch/mipsn32/ksigaction.h
index b565f1fc..2141a0a2 100644
--- a/arch/mipsn32/ksigaction.h
+++ b/arch/mipsn32/ksigaction.h
@@ -4,7 +4,6 @@ struct k_sigaction {
unsigned flags;
void (*handler)(int);
unsigned long mask[4];
- void (*restorer)();
};
hidden void __restore(), __restore_rt();
diff --git a/arch/or1k/bits/signal.h b/arch/or1k/bits/signal.h
index be576d1d..c45be676 100644
--- a/arch/or1k/bits/signal.h
+++ b/arch/or1k/bits/signal.h
@@ -43,7 +43,6 @@ typedef struct __ucontext {
#define SA_RESTART 0x10000000
#define SA_NODEFER 0x40000000
#define SA_RESETHAND 0x80000000
-#define SA_RESTORER 0x04000000
#endif
diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
index 287367db..fd6157a3 100644
--- a/arch/riscv64/bits/signal.h
+++ b/arch/riscv64/bits/signal.h
@@ -76,7 +76,6 @@ typedef struct __ucontext
#define SA_RESTART 0x10000000
#define SA_NODEFER 0x40000000
#define SA_RESETHAND 0x80000000
-#define SA_RESTORER 0x04000000
#endif
diff --git a/src/internal/ksigaction.h b/src/internal/ksigaction.h
index 8ebd5938..f0b6a837 100644
--- a/src/internal/ksigaction.h
+++ b/src/internal/ksigaction.h
@@ -6,7 +6,9 @@
struct k_sigaction {
void (*handler)(int);
unsigned long flags;
+#ifdef SA_RESTORER
void (*restorer)(void);
+#endif
unsigned mask[2];
};
diff --git a/src/signal/sigaction.c b/src/signal/sigaction.c
index 2203471b..e45308fa 100644
--- a/src/signal/sigaction.c
+++ b/src/signal/sigaction.c
@@ -44,8 +44,11 @@ int __libc_sigaction(int sig, const struct sigaction *restrict sa, struct sigact
}
}
ksa.handler = sa->sa_handler;
- ksa.flags = sa->sa_flags | SA_RESTORER;
+ ksa.flags = sa->sa_flags;
+#ifdef SA_RESTORER
+ ksa.flags |= SA_RESTORER;
ksa.restorer = (sa->sa_flags & SA_SIGINFO) ? __restore_rt : __restore;
+#endif
memcpy(&ksa.mask, &sa->sa_mask, _NSIG/8);
}
int r = __syscall(SYS_rt_sigaction, sig, sa?&ksa:0, old?&ksa_old:0, _NSIG/8);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [musl] SA_RESTORER for rv64?
2023-02-08 21:45 ` Rich Felker
@ 2023-02-08 21:53 ` enh
0 siblings, 0 replies; 10+ messages in thread
From: enh @ 2023-02-08 21:53 UTC (permalink / raw)
To: Rich Felker; +Cc: Khem Raj, musl
yes, that looks like what we have in bionic for riscv64 (and what we
had for mips before it was removed). thanks!
On Wed, Feb 8, 2023 at 1:45 PM Rich Felker <dalias@libc.org> wrote:
>
> On Mon, Feb 06, 2023 at 12:49:53PM -0500, Rich Felker wrote:
> > On Mon, Feb 06, 2023 at 08:51:13AM -0800, enh wrote:
> > > On Sun, Feb 5, 2023 at 3:54 PM Rich Felker <dalias@libc.org> wrote:
> > > >
> > > > On Fri, Feb 03, 2023 at 10:44:56AM -0800, enh wrote:
> > > > > oops, never actually sent the patch. attached...
> > > > >
> > > > > On Thu, Nov 10, 2022 at 9:31 AM Khem Raj <raj.khem@gmail.com> wrote:
> > > > > >
> > > > > > On Thu, Nov 10, 2022 at 9:19 AM Rich Felker <dalias@libc.org> wrote:
> > > > > > >
> > > > > > > On Thu, Nov 10, 2022 at 07:44:23AM -0800, enh wrote:
> > > > > > > > arch/riscv64/bits/signal.h has contained a definition for SA_RESTORER since
> > > > > > > > the initial commit, but i think that's just copy & paste from whichever
> > > > > > > > architecture the rv64 headers were based on? the linux kernel itself
> > > > > > > > doesn't have SA_RESTORER for rv64, unless i'm missing something?
> > > > > > >
> > > > > > > I suspect this is just a mistake. Have you seen any ill effects from
> > > > > > > it? If riscv folks can confirm it's wrong, I'll remove it.
> > > > > >
> > > > > > Yeah I think it should be removed. Perhaps mips is in same boat.
> > > > > >
> > > > > > >
> > > > > > > Rich
> > > >
> > > > > From 6413de6d9f785c98e5bc0cf40be947f1169d2fd7 Mon Sep 17 00:00:00 2001
> > > > > From: Elliott Hughes <enh@google.com>
> > > > > Date: Fri, 3 Feb 2023 10:42:55 -0800
> > > > > Subject: [PATCH] risc-v does not have SA_RESTORER.
> > > > >
> > > > > The kernel's include/uapi/asm-generic/signal-defs.h explicitly calls
> > > > > this out as obsolete. New architectures like risc-v do not define it.
> > > > > ---
> > > > > arch/riscv64/bits/signal.h | 1 -
> > > > > 1 file changed, 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/riscv64/bits/signal.h b/arch/riscv64/bits/signal.h
> > > > > index 287367db..fd6157a3 100644
> > > > > --- a/arch/riscv64/bits/signal.h
> > > > > +++ b/arch/riscv64/bits/signal.h
> > > > > @@ -76,7 +76,6 @@ typedef struct __ucontext
> > > > > #define SA_RESTART 0x10000000
> > > > > #define SA_NODEFER 0x40000000
> > > > > #define SA_RESETHAND 0x80000000
> > > > > -#define SA_RESTORER 0x04000000
> > > > >
> > > > > #endif
> > > > >
> > > > > --
> > > > > 2.39.1.519.gcb327c4b5f-goog
> > > > >
> > > >
> > > > I don't think this patch works as-is, since musl unconditionally uses
> > > > SA_RESTORER. We probably need to make that conditional on its
> > > > presence, and it looks like there's also a wrong-struct-layout issue
> > > > on archs where it's absent...
> > >
> > > yeah, bionic just uses the kernel uapi headers directly, and they look
> > > like this:
> > >
> > > struct sigaction {
> > > __sighandler_t sa_handler;
> > > unsigned long sa_flags;
> > > #ifdef SA_RESTORER
> > > __sigrestore_t sa_restorer;
> > > #endif
> > > sigset_t sa_mask;
> > > };
> >
> > OK. It looks like we need to remove the wrong SA_RESTORER for archs
> > that aren't supposed to have it *and* add such an #ifdef. Right now,
> > we're passing bogus sa_mask on these archs... :(
>
> How does the attached look?
>
> Rich
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-02-08 21:53 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-10 15:44 [musl] SA_RESTORER for rv64? enh
2022-11-10 17:18 ` Rich Felker
2022-11-10 17:26 ` enh
2022-11-10 17:31 ` Khem Raj
2023-02-03 18:44 ` enh
2023-02-05 23:54 ` Rich Felker
2023-02-06 16:51 ` enh
2023-02-06 17:49 ` Rich Felker
2023-02-08 21:45 ` Rich Felker
2023-02-08 21:53 ` enh
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
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).