* [PATCH v2 0/7] linux-user: sigaction fixes/cleanups
@ 2021-04-22 23:02 Richard Henderson
2021-04-22 23:02 ` [PATCH v2 1/7] linux-user/alpha: Fix rt sigframe return Richard Henderson
` (8 more replies)
0 siblings, 9 replies; 21+ messages in thread
From: Richard Henderson @ 2021-04-22 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
Alpha had two bugs, one with the non-ka_restorer fallback
using the wrong offset, and the other with the ka_restorer
value getting lost in do_sigaction.
Sparc had another bug, where the ka_restorer field was
written to user memory.
Version 2 splits patch 2 into 6.
r~
Richard Henderson (7):
linux-user/alpha: Fix rt sigframe return
linux-user/alpha: Rename the sigaction restorer field
linux-user: Pass ka_restorer to do_sigaction
linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall
linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER
linux-user/alpha: Share code for TARGET_NR_sigaction
linux-user: Tidy TARGET_NR_rt_sigaction
linux-user/alpha/target_signal.h | 1 +
linux-user/syscall_defs.h | 29 ++-------
linux-user/alpha/signal.c | 10 +--
linux-user/signal.c | 5 +-
linux-user/syscall.c | 107 ++++++++-----------------------
5 files changed, 43 insertions(+), 109 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/7] linux-user/alpha: Fix rt sigframe return
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
@ 2021-04-22 23:02 ` Richard Henderson
2021-04-23 10:00 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 2/7] linux-user/alpha: Rename the sigaction restorer field Richard Henderson
` (7 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Richard Henderson @ 2021-04-22 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent, Philippe Mathieu-Daudé
We incorrectly used the offset of the non-rt sigframe.
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/alpha/signal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/alpha/signal.c b/linux-user/alpha/signal.c
index c5c27ce084..86f5d2276d 100644
--- a/linux-user/alpha/signal.c
+++ b/linux-user/alpha/signal.c
@@ -200,7 +200,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
&frame->retcode[1]);
__put_user(INSN_CALLSYS, &frame->retcode[2]);
/* imb(); */
- r26 = frame_addr + offsetof(struct target_sigframe, retcode);
+ r26 = frame_addr + offsetof(struct target_rt_sigframe, retcode);
}
if (err) {
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/7] linux-user/alpha: Rename the sigaction restorer field
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
2021-04-22 23:02 ` [PATCH v2 1/7] linux-user/alpha: Fix rt sigframe return Richard Henderson
@ 2021-04-22 23:02 ` Richard Henderson
2021-04-23 10:16 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 3/7] linux-user: Pass ka_restorer to do_sigaction Richard Henderson
` (6 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Richard Henderson @ 2021-04-22 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
Use ka_restorer, in line with TARGET_ARCH_HAS_KA_RESTORER
vs TARGET_ARCH_HAS_SA_RESTORER, since Alpha passes this
field as a syscall argument.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall_defs.h | 2 +-
linux-user/alpha/signal.c | 8 ++++----
linux-user/syscall.c | 4 ++--
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 25be414727..693d4f3788 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -519,7 +519,7 @@ struct target_sigaction {
abi_ulong _sa_handler;
abi_ulong sa_flags;
target_sigset_t sa_mask;
- abi_ulong sa_restorer;
+ abi_ulong ka_restorer;
};
#elif defined(TARGET_MIPS)
struct target_sigaction {
diff --git a/linux-user/alpha/signal.c b/linux-user/alpha/signal.c
index 86f5d2276d..3aa4b339a4 100644
--- a/linux-user/alpha/signal.c
+++ b/linux-user/alpha/signal.c
@@ -138,8 +138,8 @@ void setup_frame(int sig, struct target_sigaction *ka,
setup_sigcontext(&frame->sc, env, frame_addr, set);
- if (ka->sa_restorer) {
- r26 = ka->sa_restorer;
+ if (ka->ka_restorer) {
+ r26 = ka->ka_restorer;
} else {
__put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
__put_user(INSN_LDI_R0 + TARGET_NR_sigreturn,
@@ -192,8 +192,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
__put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
}
- if (ka->sa_restorer) {
- r26 = ka->sa_restorer;
+ if (ka->ka_restorer) {
+ r26 = ka->ka_restorer;
} else {
__put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
__put_user(INSN_LDI_R0 + TARGET_NR_rt_sigreturn,
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 95d79ddc43..ee21eb5e6f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8989,7 +8989,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
act._sa_handler = old_act->_sa_handler;
target_siginitset(&act.sa_mask, old_act->sa_mask);
act.sa_flags = old_act->sa_flags;
- act.sa_restorer = 0;
+ act.ka_restorer = 0;
unlock_user_struct(old_act, arg2, 0);
pact = &act;
}
@@ -9085,7 +9085,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
act._sa_handler = rt_act->_sa_handler;
act.sa_mask = rt_act->sa_mask;
act.sa_flags = rt_act->sa_flags;
- act.sa_restorer = arg5;
+ act.ka_restorer = arg5;
unlock_user_struct(rt_act, arg2, 0);
pact = &act;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 3/7] linux-user: Pass ka_restorer to do_sigaction
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
2021-04-22 23:02 ` [PATCH v2 1/7] linux-user/alpha: Fix rt sigframe return Richard Henderson
2021-04-22 23:02 ` [PATCH v2 2/7] linux-user/alpha: Rename the sigaction restorer field Richard Henderson
@ 2021-04-22 23:02 ` Richard Henderson
2021-04-23 12:55 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 4/7] linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall Richard Henderson
` (5 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Richard Henderson @ 2021-04-22 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
The value of ka_restorer needs to be saved in sigact_table.
At the moment, the attempt to save it in do_syscall is
improperly clobbering user memory.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall_defs.h | 2 +-
linux-user/signal.c | 5 ++++-
linux-user/syscall.c | 19 ++++++-------------
3 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 693d4f3788..e4aaf8412f 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -492,7 +492,7 @@ void target_to_host_old_sigset(sigset_t *sigset,
const abi_ulong *old_sigset);
struct target_sigaction;
int do_sigaction(int sig, const struct target_sigaction *act,
- struct target_sigaction *oact);
+ struct target_sigaction *oact, abi_ulong ka_restorer);
#include "target_signal.h"
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 7eecec46c4..44a5012930 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -830,7 +830,7 @@ out:
/* do_sigaction() return target values and host errnos */
int do_sigaction(int sig, const struct target_sigaction *act,
- struct target_sigaction *oact)
+ struct target_sigaction *oact, abi_ulong ka_restorer)
{
struct target_sigaction *k;
struct sigaction act1;
@@ -863,6 +863,9 @@ int do_sigaction(int sig, const struct target_sigaction *act,
__get_user(k->sa_flags, &act->sa_flags);
#ifdef TARGET_ARCH_HAS_SA_RESTORER
__get_user(k->sa_restorer, &act->sa_restorer);
+#endif
+#ifdef TARGET_ARCH_HAS_KA_RESTORER
+ k->ka_restorer = ka_restorer;
#endif
/* To be swapped in target_to_host_sigset. */
k->sa_mask = act->sa_mask;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ee21eb5e6f..36169a0ded 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8989,11 +8989,10 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
act._sa_handler = old_act->_sa_handler;
target_siginitset(&act.sa_mask, old_act->sa_mask);
act.sa_flags = old_act->sa_flags;
- act.ka_restorer = 0;
unlock_user_struct(old_act, arg2, 0);
pact = &act;
}
- ret = get_errno(do_sigaction(arg1, pact, &oact));
+ ret = get_errno(do_sigaction(arg1, pact, &oact, 0));
if (!is_error(ret) && arg3) {
if (!lock_user_struct(VERIFY_WRITE, old_act, arg3, 0))
return -TARGET_EFAULT;
@@ -9017,7 +9016,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
pact = NULL;
}
- ret = get_errno(do_sigaction(arg1, pact, &oact));
+ ret = get_errno(do_sigaction(arg1, pact, &oact, 0));
if (!is_error(ret) && arg3) {
if (!lock_user_struct(VERIFY_WRITE, old_act, arg3, 0))
@@ -9040,15 +9039,12 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
target_siginitset(&act.sa_mask, old_act->sa_mask);
act.sa_flags = old_act->sa_flags;
act.sa_restorer = old_act->sa_restorer;
-#ifdef TARGET_ARCH_HAS_KA_RESTORER
- act.ka_restorer = 0;
-#endif
unlock_user_struct(old_act, arg2, 0);
pact = &act;
} else {
pact = NULL;
}
- ret = get_errno(do_sigaction(arg1, pact, &oact));
+ ret = get_errno(do_sigaction(arg1, pact, &oact, 0));
if (!is_error(ret) && arg3) {
if (!lock_user_struct(VERIFY_WRITE, old_act, arg3, 0))
return -TARGET_EFAULT;
@@ -9085,11 +9081,10 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
act._sa_handler = rt_act->_sa_handler;
act.sa_mask = rt_act->sa_mask;
act.sa_flags = rt_act->sa_flags;
- act.ka_restorer = arg5;
unlock_user_struct(rt_act, arg2, 0);
pact = &act;
}
- ret = get_errno(do_sigaction(arg1, pact, &oact));
+ ret = get_errno(do_sigaction(arg1, pact, &oact, arg5));
if (!is_error(ret) && arg3) {
if (!lock_user_struct(VERIFY_WRITE, rt_act, arg3, 0))
return -TARGET_EFAULT;
@@ -9104,6 +9099,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
target_ulong sigsetsize = arg5;
#else
target_ulong sigsetsize = arg4;
+ target_ulong restorer = 0;
#endif
struct target_sigaction *act;
struct target_sigaction *oact;
@@ -9115,9 +9111,6 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) {
return -TARGET_EFAULT;
}
-#ifdef TARGET_ARCH_HAS_KA_RESTORER
- act->ka_restorer = restorer;
-#endif
} else {
act = NULL;
}
@@ -9128,7 +9121,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
}
} else
oact = NULL;
- ret = get_errno(do_sigaction(arg1, act, oact));
+ ret = get_errno(do_sigaction(arg1, act, oact, restorer));
rt_sigaction_fail:
if (act)
unlock_user_struct(act, arg2, 0);
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 4/7] linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
` (2 preceding siblings ...)
2021-04-22 23:02 ` [PATCH v2 3/7] linux-user: Pass ka_restorer to do_sigaction Richard Henderson
@ 2021-04-22 23:02 ` Richard Henderson
2021-04-23 10:21 ` Philippe Mathieu-Daudé
2021-04-23 13:14 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 5/7] linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER Richard Henderson
` (4 subsequent siblings)
8 siblings, 2 replies; 21+ messages in thread
From: Richard Henderson @ 2021-04-22 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
Do not access a field that may not be present. This will
become an issue when sharing more code in the next patch.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 36169a0ded..89d641856c 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9038,7 +9038,9 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
act._sa_handler = old_act->_sa_handler;
target_siginitset(&act.sa_mask, old_act->sa_mask);
act.sa_flags = old_act->sa_flags;
+#ifdef TARGET_ARCH_HAS_SA_RESTORER
act.sa_restorer = old_act->sa_restorer;
+#endif
unlock_user_struct(old_act, arg2, 0);
pact = &act;
} else {
@@ -9051,7 +9053,9 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
old_act->_sa_handler = oact._sa_handler;
old_act->sa_mask = oact.sa_mask.sig[0];
old_act->sa_flags = oact.sa_flags;
+#ifdef TARGET_ARCH_HAS_SA_RESTORER
old_act->sa_restorer = oact.sa_restorer;
+#endif
unlock_user_struct(old_act, arg3, 1);
}
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 5/7] linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
` (3 preceding siblings ...)
2021-04-22 23:02 ` [PATCH v2 4/7] linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall Richard Henderson
@ 2021-04-22 23:02 ` Richard Henderson
2021-04-23 13:17 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 6/7] linux-user/alpha: Share code for TARGET_NR_sigaction Richard Henderson
` (3 subsequent siblings)
8 siblings, 1 reply; 21+ messages in thread
From: Richard Henderson @ 2021-04-22 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
This means that we can share the TARGET_NR_rt_sigaction code,
and the target_rt_sigaction structure is unused. Untangling
the ifdefs so that target_sigaction can be shared will wait
until the next patch.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/alpha/target_signal.h | 1 +
linux-user/syscall_defs.h | 6 ------
linux-user/syscall.c | 37 ++++++--------------------------
3 files changed, 7 insertions(+), 37 deletions(-)
diff --git a/linux-user/alpha/target_signal.h b/linux-user/alpha/target_signal.h
index 0b90d3a897..250642913e 100644
--- a/linux-user/alpha/target_signal.h
+++ b/linux-user/alpha/target_signal.h
@@ -92,6 +92,7 @@ typedef struct target_sigaltstack {
#define TARGET_GEN_SUBRNG7 -25
#define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_KA_RESTORER
/* bit-flags */
#define TARGET_SS_AUTODISARM (1U << 31) /* disable sas during sighandling */
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index e4aaf8412f..7a1d3b239c 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -507,12 +507,6 @@ struct target_old_sigaction {
int32_t sa_flags;
};
-struct target_rt_sigaction {
- abi_ulong _sa_handler;
- abi_ulong sa_flags;
- target_sigset_t sa_mask;
-};
-
/* This is the struct used inside the kernel. The ka_restorer
field comes from the 5th argument to sys_rt_sigaction. */
struct target_sigaction {
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 89d641856c..216ee4ca47 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9064,41 +9064,17 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
#endif
case TARGET_NR_rt_sigaction:
{
-#if defined(TARGET_ALPHA)
- /* For Alpha and SPARC this is a 5 argument syscall, with
+ /*
+ * For Alpha and SPARC this is a 5 argument syscall, with
* a 'restorer' parameter which must be copied into the
* sa_restorer field of the sigaction struct.
* For Alpha that 'restorer' is arg5; for SPARC it is arg4,
* and arg5 is the sigsetsize.
- * Alpha also has a separate rt_sigaction struct that it uses
- * here; SPARC uses the usual sigaction struct.
*/
- struct target_rt_sigaction *rt_act;
- struct target_sigaction act, oact, *pact = 0;
-
- if (arg4 != sizeof(target_sigset_t)) {
- return -TARGET_EINVAL;
- }
- if (arg2) {
- if (!lock_user_struct(VERIFY_READ, rt_act, arg2, 1))
- return -TARGET_EFAULT;
- act._sa_handler = rt_act->_sa_handler;
- act.sa_mask = rt_act->sa_mask;
- act.sa_flags = rt_act->sa_flags;
- unlock_user_struct(rt_act, arg2, 0);
- pact = &act;
- }
- ret = get_errno(do_sigaction(arg1, pact, &oact, arg5));
- if (!is_error(ret) && arg3) {
- if (!lock_user_struct(VERIFY_WRITE, rt_act, arg3, 0))
- return -TARGET_EFAULT;
- rt_act->_sa_handler = oact._sa_handler;
- rt_act->sa_mask = oact.sa_mask;
- rt_act->sa_flags = oact.sa_flags;
- unlock_user_struct(rt_act, arg3, 1);
- }
-#else
-#ifdef TARGET_SPARC
+#if defined(TARGET_ALPHA)
+ target_ulong sigsetsize = arg4;
+ target_ulong restorer = arg5;
+#elif defined(TARGET_SPARC)
target_ulong restorer = arg4;
target_ulong sigsetsize = arg5;
#else
@@ -9131,7 +9107,6 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
unlock_user_struct(act, arg2, 0);
if (oact)
unlock_user_struct(oact, arg3, 1);
-#endif
}
return ret;
#ifdef TARGET_NR_sgetmask /* not on alpha */
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 6/7] linux-user/alpha: Share code for TARGET_NR_sigaction
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
` (4 preceding siblings ...)
2021-04-22 23:02 ` [PATCH v2 5/7] linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER Richard Henderson
@ 2021-04-22 23:02 ` Richard Henderson
2021-04-23 14:29 ` Alex Bennée
2021-04-23 15:08 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 7/7] linux-user: Tidy TARGET_NR_rt_sigaction Richard Henderson
` (2 subsequent siblings)
8 siblings, 2 replies; 21+ messages in thread
From: Richard Henderson @ 2021-04-22 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
There's no longer a difference between the alpha code and
the generic code.
There is a type difference in target_old_sigaction.sa_flags,
which can be resolved with a very much smaller ifdef, which
allows us to finish sharing the target_sigaction definition.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall_defs.h | 21 ++++++---------------
linux-user/syscall.c | 23 +----------------------
2 files changed, 7 insertions(+), 37 deletions(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 7a1d3b239c..18b031a2f6 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -501,21 +501,12 @@ int do_sigaction(int sig, const struct target_sigaction *act,
#endif
#if defined(TARGET_ALPHA)
-struct target_old_sigaction {
- abi_ulong _sa_handler;
- abi_ulong sa_mask;
- int32_t sa_flags;
-};
+typedef int32_t target_old_sa_flags;
+#else
+typedef abi_ulong target_old_sa_flags;
+#endif
-/* This is the struct used inside the kernel. The ka_restorer
- field comes from the 5th argument to sys_rt_sigaction. */
-struct target_sigaction {
- abi_ulong _sa_handler;
- abi_ulong sa_flags;
- target_sigset_t sa_mask;
- abi_ulong ka_restorer;
-};
-#elif defined(TARGET_MIPS)
+#if defined(TARGET_MIPS)
struct target_sigaction {
uint32_t sa_flags;
#if defined(TARGET_ABI_MIPSN32)
@@ -533,7 +524,7 @@ struct target_sigaction {
struct target_old_sigaction {
abi_ulong _sa_handler;
abi_ulong sa_mask;
- abi_ulong sa_flags;
+ target_old_sa_flags sa_flags;
#ifdef TARGET_ARCH_HAS_SA_RESTORER
abi_ulong sa_restorer;
#endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 216ee4ca47..9bcd485423 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8980,28 +8980,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
#ifdef TARGET_NR_sigaction
case TARGET_NR_sigaction:
{
-#if defined(TARGET_ALPHA)
- struct target_sigaction act, oact, *pact = 0;
- struct target_old_sigaction *old_act;
- if (arg2) {
- if (!lock_user_struct(VERIFY_READ, old_act, arg2, 1))
- return -TARGET_EFAULT;
- act._sa_handler = old_act->_sa_handler;
- target_siginitset(&act.sa_mask, old_act->sa_mask);
- act.sa_flags = old_act->sa_flags;
- unlock_user_struct(old_act, arg2, 0);
- pact = &act;
- }
- ret = get_errno(do_sigaction(arg1, pact, &oact, 0));
- if (!is_error(ret) && arg3) {
- if (!lock_user_struct(VERIFY_WRITE, old_act, arg3, 0))
- return -TARGET_EFAULT;
- old_act->_sa_handler = oact._sa_handler;
- old_act->sa_mask = oact.sa_mask.sig[0];
- old_act->sa_flags = oact.sa_flags;
- unlock_user_struct(old_act, arg3, 1);
- }
-#elif defined(TARGET_MIPS)
+#if defined(TARGET_MIPS)
struct target_sigaction act, oact, *pact, *old_act;
if (arg2) {
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 7/7] linux-user: Tidy TARGET_NR_rt_sigaction
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
` (5 preceding siblings ...)
2021-04-22 23:02 ` [PATCH v2 6/7] linux-user/alpha: Share code for TARGET_NR_sigaction Richard Henderson
@ 2021-04-22 23:02 ` Richard Henderson
2021-04-23 10:24 ` Philippe Mathieu-Daudé
2021-04-23 15:10 ` Alex Bennée
2021-04-22 23:24 ` [PATCH v2 0/7] linux-user: sigaction fixes/cleanups no-reply
2021-05-15 19:52 ` Laurent Vivier
8 siblings, 2 replies; 21+ messages in thread
From: Richard Henderson @ 2021-04-22 23:02 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, laurent
Initialize variables instead of elses.
Use an else instead of a goto.
Add braces.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9bcd485423..c7c3257f40 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9060,32 +9060,26 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
target_ulong sigsetsize = arg4;
target_ulong restorer = 0;
#endif
- struct target_sigaction *act;
- struct target_sigaction *oact;
+ struct target_sigaction *act = NULL;
+ struct target_sigaction *oact = NULL;
if (sigsetsize != sizeof(target_sigset_t)) {
return -TARGET_EINVAL;
}
- if (arg2) {
- if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) {
- return -TARGET_EFAULT;
- }
- } else {
- act = NULL;
+ if (arg2 && !lock_user_struct(VERIFY_READ, act, arg2, 1)) {
+ return -TARGET_EFAULT;
}
- if (arg3) {
- if (!lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) {
- ret = -TARGET_EFAULT;
- goto rt_sigaction_fail;
+ if (arg3 && !lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) {
+ ret = -TARGET_EFAULT;
+ } else {
+ ret = get_errno(do_sigaction(arg1, act, oact, restorer));
+ if (oact) {
+ unlock_user_struct(oact, arg3, 1);
}
- } else
- oact = NULL;
- ret = get_errno(do_sigaction(arg1, act, oact, restorer));
- rt_sigaction_fail:
- if (act)
+ }
+ if (act) {
unlock_user_struct(act, arg2, 0);
- if (oact)
- unlock_user_struct(oact, arg3, 1);
+ }
}
return ret;
#ifdef TARGET_NR_sgetmask /* not on alpha */
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/7] linux-user: sigaction fixes/cleanups
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
` (6 preceding siblings ...)
2021-04-22 23:02 ` [PATCH v2 7/7] linux-user: Tidy TARGET_NR_rt_sigaction Richard Henderson
@ 2021-04-22 23:24 ` no-reply
2021-05-15 19:52 ` Laurent Vivier
8 siblings, 0 replies; 21+ messages in thread
From: no-reply @ 2021-04-22 23:24 UTC (permalink / raw)
To: richard.henderson; +Cc: alex.bennee, qemu-devel, laurent
Patchew URL: https://patchew.org/QEMU/20210422230227.314751-1-richard.henderson@linaro.org/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210422230227.314751-1-richard.henderson@linaro.org
Subject: [PATCH v2 0/7] linux-user: sigaction fixes/cleanups
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210422230227.314751-1-richard.henderson@linaro.org -> patchew/20210422230227.314751-1-richard.henderson@linaro.org
Switched to a new branch 'test'
9eee746 linux-user: Tidy TARGET_NR_rt_sigaction
46c2541 linux-user/alpha: Share code for TARGET_NR_sigaction
c69776b linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER
de9e5c2 linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall
57bd960 linux-user: Pass ka_restorer to do_sigaction
ef4054e linux-user/alpha: Rename the sigaction restorer field
df4fac9 linux-user/alpha: Fix rt sigframe return
=== OUTPUT BEGIN ===
1/7 Checking commit df4fac977c4c (linux-user/alpha: Fix rt sigframe return)
2/7 Checking commit ef4054e42574 (linux-user/alpha: Rename the sigaction restorer field)
3/7 Checking commit 57bd9604ef18 (linux-user: Pass ka_restorer to do_sigaction)
ERROR: code indent should never use tabs
#64: FILE: linux-user/syscall.c:9019:
+^I ret = get_errno(do_sigaction(arg1, pact, &oact, 0));$
total: 1 errors, 0 warnings, 97 lines checked
Patch 3/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/7 Checking commit de9e5c267f85 (linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall)
5/7 Checking commit c69776bf2b07 (linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER)
6/7 Checking commit 46c2541b617d (linux-user/alpha: Share code for TARGET_NR_sigaction)
7/7 Checking commit 9eee7464d318 (linux-user: Tidy TARGET_NR_rt_sigaction)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210422230227.314751-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/7] linux-user/alpha: Fix rt sigframe return
2021-04-22 23:02 ` [PATCH v2 1/7] linux-user/alpha: Fix rt sigframe return Richard Henderson
@ 2021-04-23 10:00 ` Alex Bennée
0 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-04-23 10:00 UTC (permalink / raw)
To: Richard Henderson; +Cc: Philippe Mathieu-Daudé, qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> We incorrectly used the offset of the non-rt sigframe.
>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/7] linux-user/alpha: Rename the sigaction restorer field
2021-04-22 23:02 ` [PATCH v2 2/7] linux-user/alpha: Rename the sigaction restorer field Richard Henderson
@ 2021-04-23 10:16 ` Alex Bennée
2021-04-23 17:04 ` Richard Henderson
0 siblings, 1 reply; 21+ messages in thread
From: Alex Bennée @ 2021-04-23 10:16 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> Use ka_restorer, in line with TARGET_ARCH_HAS_KA_RESTORER
> vs TARGET_ARCH_HAS_SA_RESTORER, since Alpha passes this
> field as a syscall argument.
I'm still slightly confused - but that's to be expected from signals :-/
Anyway I understand that the SA_RESTORER points to the vdso trampoline
(at least according to man sigreturn). What is ka_restorer - if this the
in sigframe restorer?
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall_defs.h | 2 +-
> linux-user/alpha/signal.c | 8 ++++----
> linux-user/syscall.c | 4 ++--
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 25be414727..693d4f3788 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -519,7 +519,7 @@ struct target_sigaction {
> abi_ulong _sa_handler;
> abi_ulong sa_flags;
> target_sigset_t sa_mask;
> - abi_ulong sa_restorer;
> + abi_ulong ka_restorer;
> };
Maybe this is something we could expand a little on in the difference
between the two here (or maybe in the later commit?).
> #elif defined(TARGET_MIPS)
> struct target_sigaction {
> diff --git a/linux-user/alpha/signal.c b/linux-user/alpha/signal.c
> index 86f5d2276d..3aa4b339a4 100644
> --- a/linux-user/alpha/signal.c
> +++ b/linux-user/alpha/signal.c
> @@ -138,8 +138,8 @@ void setup_frame(int sig, struct target_sigaction *ka,
>
> setup_sigcontext(&frame->sc, env, frame_addr, set);
>
> - if (ka->sa_restorer) {
> - r26 = ka->sa_restorer;
> + if (ka->ka_restorer) {
> + r26 = ka->ka_restorer;
> } else {
> __put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
> __put_user(INSN_LDI_R0 + TARGET_NR_sigreturn,
> @@ -192,8 +192,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
> __put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
> }
>
> - if (ka->sa_restorer) {
> - r26 = ka->sa_restorer;
> + if (ka->ka_restorer) {
> + r26 = ka->ka_restorer;
> } else {
> __put_user(INSN_MOV_R30_R16, &frame->retcode[0]);
> __put_user(INSN_LDI_R0 + TARGET_NR_rt_sigreturn,
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 95d79ddc43..ee21eb5e6f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -8989,7 +8989,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> act._sa_handler = old_act->_sa_handler;
> target_siginitset(&act.sa_mask, old_act->sa_mask);
> act.sa_flags = old_act->sa_flags;
> - act.sa_restorer = 0;
> + act.ka_restorer = 0;
> unlock_user_struct(old_act, arg2, 0);
> pact = &act;
> }
> @@ -9085,7 +9085,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> act._sa_handler = rt_act->_sa_handler;
> act.sa_mask = rt_act->sa_mask;
> act.sa_flags = rt_act->sa_flags;
> - act.sa_restorer = arg5;
> + act.ka_restorer = arg5;
> unlock_user_struct(rt_act, arg2, 0);
> pact = &act;
> }
Otherwise looks fine to me:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/7] linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall
2021-04-22 23:02 ` [PATCH v2 4/7] linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall Richard Henderson
@ 2021-04-23 10:21 ` Philippe Mathieu-Daudé
2021-04-23 13:14 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-23 10:21 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee, laurent
On 4/23/21 1:02 AM, Richard Henderson wrote:
> Do not access a field that may not be present. This will
> become an issue when sharing more code in the next patch.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall.c | 4 ++++
> 1 file changed, 4 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 7/7] linux-user: Tidy TARGET_NR_rt_sigaction
2021-04-22 23:02 ` [PATCH v2 7/7] linux-user: Tidy TARGET_NR_rt_sigaction Richard Henderson
@ 2021-04-23 10:24 ` Philippe Mathieu-Daudé
2021-04-23 15:10 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-23 10:24 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee, laurent
On 4/23/21 1:02 AM, Richard Henderson wrote:
> Initialize variables instead of elses.
> Use an else instead of a goto.
> Add braces.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall.c | 32 +++++++++++++-------------------
> 1 file changed, 13 insertions(+), 19 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/7] linux-user: Pass ka_restorer to do_sigaction
2021-04-22 23:02 ` [PATCH v2 3/7] linux-user: Pass ka_restorer to do_sigaction Richard Henderson
@ 2021-04-23 12:55 ` Alex Bennée
0 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-04-23 12:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> The value of ka_restorer needs to be saved in sigact_table.
It would be nice to add a comment to sigact_table to describe what it
is. This is some of the oldest code in the tree so there seems to be a
fair bit of implicit knowledge is the way things are done.
> At the moment, the attempt to save it in do_syscall is
> improperly clobbering user memory.
Otherwise to my slightly confused eyes it looks pretty sane:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/7] linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall
2021-04-22 23:02 ` [PATCH v2 4/7] linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall Richard Henderson
2021-04-23 10:21 ` Philippe Mathieu-Daudé
@ 2021-04-23 13:14 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-04-23 13:14 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> Do not access a field that may not be present. This will
> become an issue when sharing more code in the next patch.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 5/7] linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER
2021-04-22 23:02 ` [PATCH v2 5/7] linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER Richard Henderson
@ 2021-04-23 13:17 ` Alex Bennée
0 siblings, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-04-23 13:17 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> This means that we can share the TARGET_NR_rt_sigaction code,
> and the target_rt_sigaction structure is unused. Untangling
> the ifdefs so that target_sigaction can be shared will wait
> until the next patch.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 6/7] linux-user/alpha: Share code for TARGET_NR_sigaction
2021-04-22 23:02 ` [PATCH v2 6/7] linux-user/alpha: Share code for TARGET_NR_sigaction Richard Henderson
@ 2021-04-23 14:29 ` Alex Bennée
2021-04-23 15:08 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-04-23 14:29 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> There's no longer a difference between the alpha code and
> the generic code.
>
> There is a type difference in target_old_sigaction.sa_flags,
> which can be resolved with a very much smaller ifdef, which
> allows us to finish sharing the target_sigaction definition.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 6/7] linux-user/alpha: Share code for TARGET_NR_sigaction
2021-04-22 23:02 ` [PATCH v2 6/7] linux-user/alpha: Share code for TARGET_NR_sigaction Richard Henderson
2021-04-23 14:29 ` Alex Bennée
@ 2021-04-23 15:08 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-04-23 15:08 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> There's no longer a difference between the alpha code and
> the generic code.
>
> There is a type difference in target_old_sigaction.sa_flags,
> which can be resolved with a very much smaller ifdef, which
> allows us to finish sharing the target_sigaction definition.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 7/7] linux-user: Tidy TARGET_NR_rt_sigaction
2021-04-22 23:02 ` [PATCH v2 7/7] linux-user: Tidy TARGET_NR_rt_sigaction Richard Henderson
2021-04-23 10:24 ` Philippe Mathieu-Daudé
@ 2021-04-23 15:10 ` Alex Bennée
1 sibling, 0 replies; 21+ messages in thread
From: Alex Bennée @ 2021-04-23 15:10 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, laurent
Richard Henderson <richard.henderson@linaro.org> writes:
> Initialize variables instead of elses.
> Use an else instead of a goto.
> Add braces.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall.c | 32 +++++++++++++-------------------
> 1 file changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9bcd485423..c7c3257f40 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9060,32 +9060,26 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> target_ulong sigsetsize = arg4;
> target_ulong restorer = 0;
> #endif
> - struct target_sigaction *act;
> - struct target_sigaction *oact;
> + struct target_sigaction *act = NULL;
> + struct target_sigaction *oact = NULL;
>
> if (sigsetsize != sizeof(target_sigset_t)) {
> return -TARGET_EINVAL;
> }
> - if (arg2) {
> - if (!lock_user_struct(VERIFY_READ, act, arg2, 1)) {
> - return -TARGET_EFAULT;
> - }
> - } else {
> - act = NULL;
> + if (arg2 && !lock_user_struct(VERIFY_READ, act, arg2, 1)) {
> + return -TARGET_EFAULT;
> }
> - if (arg3) {
> - if (!lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) {
> - ret = -TARGET_EFAULT;
> - goto rt_sigaction_fail;
> + if (arg3 && !lock_user_struct(VERIFY_WRITE, oact, arg3, 0)) {
> + ret = -TARGET_EFAULT;
> + } else {
> + ret = get_errno(do_sigaction(arg1, act, oact, restorer));
> + if (oact) {
> + unlock_user_struct(oact, arg3, 1);
> }
This does make me idly wonder if there is a way to handle unlocking in a
similar way to our autofree and LOCK_GUARD stuff. But that's not getting
in the way of approving of this improvement.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/7] linux-user/alpha: Rename the sigaction restorer field
2021-04-23 10:16 ` Alex Bennée
@ 2021-04-23 17:04 ` Richard Henderson
0 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2021-04-23 17:04 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel, laurent
On 4/23/21 3:16 AM, Alex Bennée wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> Use ka_restorer, in line with TARGET_ARCH_HAS_KA_RESTORER
>> vs TARGET_ARCH_HAS_SA_RESTORER, since Alpha passes this
>> field as a syscall argument.
>
> I'm still slightly confused - but that's to be expected from signals :-/
>
> Anyway I understand that the SA_RESTORER points to the vdso trampoline
> (at least according to man sigreturn). What is ka_restorer - if this the
> in sigframe restorer?
Both sa_restorer and ka_restorer pre-date the vdso trampoline. They allow libc
to register a trampoline itself.
The difference is that sa_restorer is a field in struct sigaction, and
ka_restorer is an extra argument to the sigaction syscall. Different targets
used different approaches.
>> - abi_ulong sa_restorer;
>> + abi_ulong ka_restorer;
>> };
>
> Maybe this is something we could expand a little on in the difference
> between the two here (or maybe in the later commit?).
Perhaps elsewhere. This change is simply to make alpha line up with the
generic code that will replace it.
r~
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 0/7] linux-user: sigaction fixes/cleanups
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
` (7 preceding siblings ...)
2021-04-22 23:24 ` [PATCH v2 0/7] linux-user: sigaction fixes/cleanups no-reply
@ 2021-05-15 19:52 ` Laurent Vivier
8 siblings, 0 replies; 21+ messages in thread
From: Laurent Vivier @ 2021-05-15 19:52 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee
Le 23/04/2021 à 01:02, Richard Henderson a écrit :
> Alpha had two bugs, one with the non-ka_restorer fallback
> using the wrong offset, and the other with the ka_restorer
> value getting lost in do_sigaction.
>
> Sparc had another bug, where the ka_restorer field was
> written to user memory.
>
> Version 2 splits patch 2 into 6.
>
>
> r~
>
>
> Richard Henderson (7):
> linux-user/alpha: Fix rt sigframe return
> linux-user/alpha: Rename the sigaction restorer field
> linux-user: Pass ka_restorer to do_sigaction
> linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall
> linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER
> linux-user/alpha: Share code for TARGET_NR_sigaction
> linux-user: Tidy TARGET_NR_rt_sigaction
>
> linux-user/alpha/target_signal.h | 1 +
> linux-user/syscall_defs.h | 29 ++-------
> linux-user/alpha/signal.c | 10 +--
> linux-user/signal.c | 5 +-
> linux-user/syscall.c | 107 ++++++++-----------------------
> 5 files changed, 43 insertions(+), 109 deletions(-)
>
Applied to my linux-user-for-6.1 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2021-05-15 19:54 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 23:02 [PATCH v2 0/7] linux-user: sigaction fixes/cleanups Richard Henderson
2021-04-22 23:02 ` [PATCH v2 1/7] linux-user/alpha: Fix rt sigframe return Richard Henderson
2021-04-23 10:00 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 2/7] linux-user/alpha: Rename the sigaction restorer field Richard Henderson
2021-04-23 10:16 ` Alex Bennée
2021-04-23 17:04 ` Richard Henderson
2021-04-22 23:02 ` [PATCH v2 3/7] linux-user: Pass ka_restorer to do_sigaction Richard Henderson
2021-04-23 12:55 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 4/7] linux-user: Honor TARGET_ARCH_HAS_SA_RESTORER in do_syscall Richard Henderson
2021-04-23 10:21 ` Philippe Mathieu-Daudé
2021-04-23 13:14 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 5/7] linux-user/alpha: Define TARGET_ARCH_HAS_KA_RESTORER Richard Henderson
2021-04-23 13:17 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 6/7] linux-user/alpha: Share code for TARGET_NR_sigaction Richard Henderson
2021-04-23 14:29 ` Alex Bennée
2021-04-23 15:08 ` Alex Bennée
2021-04-22 23:02 ` [PATCH v2 7/7] linux-user: Tidy TARGET_NR_rt_sigaction Richard Henderson
2021-04-23 10:24 ` Philippe Mathieu-Daudé
2021-04-23 15:10 ` Alex Bennée
2021-04-22 23:24 ` [PATCH v2 0/7] linux-user: sigaction fixes/cleanups no-reply
2021-05-15 19:52 ` Laurent Vivier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.