diff mbox series

[v2,2/7] linux-user/alpha: Rename the sigaction restorer field

Message ID 20210422230227.314751-3-richard.henderson@linaro.org
State New
Headers show
Series linux-user: sigaction fixes/cleanups | expand

Commit Message

Richard Henderson April 22, 2021, 11:02 p.m. UTC
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

Comments

Alex Bennée April 23, 2021, 10:16 a.m. UTC | #1
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
Richard Henderson April 23, 2021, 5:04 p.m. UTC | #2
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 mbox series

Patch

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;
             }