| Message ID | 20210422230227.314751-3-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: sigaction fixes/cleanups | expand |
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
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~
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; }
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(-) -- 2.25.1