diff mbox series

[v2,2/5] linux-user: Move tswap_siginfo out of target code

Message ID 20240307182623.1450717-2-gustavo.romero@linaro.org
State New
Headers show
Series [v2,1/5] gdbstub: Rename back gdb_handlesig | expand

Commit Message

Gustavo Romero March 7, 2024, 6:26 p.m. UTC
Move tswap_siginfo from target code to handle_pending_signal. This will
allow some cleanups and having the siginfo ready to be used in gdbstub.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/signal.c     |  2 +-
 linux-user/alpha/signal.c       |  2 +-
 linux-user/arm/signal.c         |  2 +-
 linux-user/hexagon/signal.c     |  2 +-
 linux-user/hppa/signal.c        |  2 +-
 linux-user/i386/signal.c        |  6 +++---
 linux-user/loongarch64/signal.c |  2 +-
 linux-user/m68k/signal.c        |  4 ++--
 linux-user/microblaze/signal.c  |  2 +-
 linux-user/mips/signal.c        |  4 ++--
 linux-user/nios2/signal.c       |  2 +-
 linux-user/openrisc/signal.c    |  2 +-
 linux-user/ppc/signal.c         |  4 ++--
 linux-user/riscv/signal.c       |  2 +-
 linux-user/s390x/signal.c       |  2 +-
 linux-user/sh4/signal.c         |  2 +-
 linux-user/signal-common.h      |  2 --
 linux-user/signal.c             | 10 ++++++++--
 linux-user/sparc/signal.c       |  2 +-
 linux-user/xtensa/signal.c      |  2 +-
 20 files changed, 31 insertions(+), 27 deletions(-)

Comments

Alex Bennée March 7, 2024, 7:21 p.m. UTC | #1
Gustavo Romero <gustavo.romero@linaro.org> writes:

> Move tswap_siginfo from target code to handle_pending_signal. This will
> allow some cleanups and having the siginfo ready to be used in gdbstub.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/aarch64/signal.c     |  2 +-
>  linux-user/alpha/signal.c       |  2 +-
>  linux-user/arm/signal.c         |  2 +-
>  linux-user/hexagon/signal.c     |  2 +-
>  linux-user/hppa/signal.c        |  2 +-
>  linux-user/i386/signal.c        |  6 +++---
>  linux-user/loongarch64/signal.c |  2 +-
>  linux-user/m68k/signal.c        |  4 ++--
>  linux-user/microblaze/signal.c  |  2 +-
>  linux-user/mips/signal.c        |  4 ++--
>  linux-user/nios2/signal.c       |  2 +-
>  linux-user/openrisc/signal.c    |  2 +-
>  linux-user/ppc/signal.c         |  4 ++--
>  linux-user/riscv/signal.c       |  2 +-
>  linux-user/s390x/signal.c       |  2 +-
>  linux-user/sh4/signal.c         |  2 +-
>  linux-user/signal-common.h      |  2 --
>  linux-user/signal.c             | 10 ++++++++--
>  linux-user/sparc/signal.c       |  2 +-
>  linux-user/xtensa/signal.c      |  2 +-
>  20 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
<snip>
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -409,8 +409,8 @@ static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
>      tinfo->si_code = deposit32(si_code, 16, 16, si_type);
>  }
>  
> -void tswap_siginfo(target_siginfo_t *tinfo,
> -                   const target_siginfo_t *info)
> +static void tswap_siginfo(target_siginfo_t *tinfo,
> +                          const target_siginfo_t *info)
>  {
>      int si_type = extract32(info->si_code, 16, 16);
>      int si_code = sextract32(info->si_code, 0, 16);
> @@ -1180,6 +1180,12 @@ static void handle_pending_signal(CPUArchState *cpu_env, int sig,
>      /* dequeue signal */
>      k->pending = 0;
>  
> +    /*
> +     * Writes out siginfo values byteswapped, accordingly to the target. It also
> +     * cleans the si_type from si_code making it correct for the target.
> +     */
> +    tswap_siginfo(&k->info, &k->info);
> +

I'm not sure I like this, you have the same pointer to both a const and
non-const arg. Do we assert we come through this once per signal and
don't risk double swapping the contents?
Richard Henderson March 7, 2024, 8:44 p.m. UTC | #2
On 3/7/24 08:26, Gustavo Romero wrote:
> Move tswap_siginfo from target code to handle_pending_signal. This will
> allow some cleanups and having the siginfo ready to be used in gdbstub.
> 
> Signed-off-by: Gustavo Romero<gustavo.romero@linaro.org>
> Suggested-by: Richard Henderson<richard.henderson@linaro.org>
> ---
>   linux-user/aarch64/signal.c     |  2 +-
>   linux-user/alpha/signal.c       |  2 +-
>   linux-user/arm/signal.c         |  2 +-
>   linux-user/hexagon/signal.c     |  2 +-
>   linux-user/hppa/signal.c        |  2 +-
>   linux-user/i386/signal.c        |  6 +++---
>   linux-user/loongarch64/signal.c |  2 +-
>   linux-user/m68k/signal.c        |  4 ++--
>   linux-user/microblaze/signal.c  |  2 +-
>   linux-user/mips/signal.c        |  4 ++--
>   linux-user/nios2/signal.c       |  2 +-
>   linux-user/openrisc/signal.c    |  2 +-
>   linux-user/ppc/signal.c         |  4 ++--
>   linux-user/riscv/signal.c       |  2 +-
>   linux-user/s390x/signal.c       |  2 +-
>   linux-user/sh4/signal.c         |  2 +-
>   linux-user/signal-common.h      |  2 --
>   linux-user/signal.c             | 10 ++++++++--
>   linux-user/sparc/signal.c       |  2 +-
>   linux-user/xtensa/signal.c      |  2 +-
>   20 files changed, 31 insertions(+), 27 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Richard Henderson March 7, 2024, 8:47 p.m. UTC | #3
On 3/7/24 09:21, Alex Bennée wrote:
>> +    /*
>> +     * Writes out siginfo values byteswapped, accordingly to the target. It also
>> +     * cleans the si_type from si_code making it correct for the target.
>> +     */
>> +    tswap_siginfo(&k->info, &k->info);
>> +
> 
> I'm not sure I like this, you have the same pointer to both a const and
> non-const arg. Do we assert we come through this once per signal and
> don't risk double swapping the contents?
> 

I suggested this as an intermediate step -- the function will work perfectly fine with 
dest == source.  Follow-up patches should clean this up further, but I told Gustavo to not 
get distracted from his goal with *too* much cleanup.


r~
diff mbox series

Patch

diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index a1e22d526d..bc7a13800d 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -670,7 +670,7 @@  static void target_setup_frame(int usig, struct target_sigaction *ka,
     aarch64_set_svcr(env, 0, R_SVCR_SM_MASK | R_SVCR_ZA_MASK);
 
     if (info) {
-        tswap_siginfo(&frame->info, info);
+        frame->info = *info;
         env->xregs[1] = frame_addr + offsetof(struct target_rt_sigframe, info);
         env->xregs[2] = frame_addr + offsetof(struct target_rt_sigframe, uc);
     }
diff --git a/linux-user/alpha/signal.c b/linux-user/alpha/signal.c
index 4ec42994d4..896c2c148a 100644
--- a/linux-user/alpha/signal.c
+++ b/linux-user/alpha/signal.c
@@ -173,7 +173,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
         goto give_sigsegv;
     }
 
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
 
     __put_user(0, &frame->uc.tuc_flags);
     __put_user(0, &frame->uc.tuc_link);
diff --git a/linux-user/arm/signal.c b/linux-user/arm/signal.c
index f77f692c63..420fc04cfa 100644
--- a/linux-user/arm/signal.c
+++ b/linux-user/arm/signal.c
@@ -357,7 +357,7 @@  void setup_rt_frame(int usig, struct target_sigaction *ka,
 
     info_addr = frame_addr + offsetof(struct rt_sigframe, info);
     uc_addr = frame_addr + offsetof(struct rt_sigframe, sig.uc);
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
 
     setup_sigframe(&frame->sig.uc, set, env);
 
diff --git a/linux-user/hexagon/signal.c b/linux-user/hexagon/signal.c
index 60fa7e1bce..492b51f155 100644
--- a/linux-user/hexagon/signal.c
+++ b/linux-user/hexagon/signal.c
@@ -162,7 +162,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     }
 
     setup_ucontext(&frame->uc, env, set);
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
     /*
      * The on-stack signal trampoline is no longer executed;
      * however, the libgcc signal frame unwinding code checks
diff --git a/linux-user/hppa/signal.c b/linux-user/hppa/signal.c
index d08a97dae6..8960175da3 100644
--- a/linux-user/hppa/signal.c
+++ b/linux-user/hppa/signal.c
@@ -127,7 +127,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
         goto give_sigsegv;
     }
 
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
     frame->uc.tuc_flags = 0;
     frame->uc.tuc_link = 0;
 
diff --git a/linux-user/i386/signal.c b/linux-user/i386/signal.c
index bc5d45302e..cfe70fc5cf 100644
--- a/linux-user/i386/signal.c
+++ b/linux-user/i386/signal.c
@@ -430,7 +430,7 @@  void setup_frame(int sig, struct target_sigaction *ka,
     setup_sigcontext(&frame->sc, &frame->fpstate, env, set->sig[0],
             frame_addr + offsetof(struct sigframe, fpstate));
 
-    for(i = 1; i < TARGET_NSIG_WORDS; i++) {
+    for (i = 1; i < TARGET_NSIG_WORDS; i++) {
         __put_user(set->sig[i], &frame->extramask[i - 1]);
     }
 
@@ -490,7 +490,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     __put_user(addr, &frame->puc);
 #endif
     if (ka->sa_flags & TARGET_SA_SIGINFO) {
-        tswap_siginfo(&frame->info, info);
+        frame->info = *info;
     }
 
     /* Create the ucontext.  */
@@ -504,7 +504,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     setup_sigcontext(&frame->uc.tuc_mcontext, &frame->fpstate, env,
             set->sig[0], frame_addr + offsetof(struct rt_sigframe, fpstate));
 
-    for(i = 0; i < TARGET_NSIG_WORDS; i++) {
+    for (i = 0; i < TARGET_NSIG_WORDS; i++) {
         __put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
     }
 
diff --git a/linux-user/loongarch64/signal.c b/linux-user/loongarch64/signal.c
index 39ea82c814..1a322f9697 100644
--- a/linux-user/loongarch64/signal.c
+++ b/linux-user/loongarch64/signal.c
@@ -376,7 +376,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
         extctx.end.haddr = (void *)frame + (extctx.end.gaddr - frame_addr);
     }
 
-    tswap_siginfo(&frame->rs_info, info);
+    frame->rs_info = *info;
 
     __put_user(0, &frame->rs_uc.tuc_flags);
     __put_user(0, &frame->rs_uc.tuc_link);
diff --git a/linux-user/m68k/signal.c b/linux-user/m68k/signal.c
index 5f35354487..77555781aa 100644
--- a/linux-user/m68k/signal.c
+++ b/linux-user/m68k/signal.c
@@ -295,7 +295,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     uc_addr = frame_addr + offsetof(struct target_rt_sigframe, uc);
     __put_user(uc_addr, &frame->puc);
 
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
 
     /* Create the ucontext */
 
@@ -307,7 +307,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     if (err)
         goto give_sigsegv;
 
-    for(i = 0; i < TARGET_NSIG_WORDS; i++) {
+    for (i = 0; i < TARGET_NSIG_WORDS; i++) {
         __put_user(set->sig[i], &frame->uc.tuc_sigmask.sig[i]);
     }
 
diff --git a/linux-user/microblaze/signal.c b/linux-user/microblaze/signal.c
index 5188d74025..f6d47d76ff 100644
--- a/linux-user/microblaze/signal.c
+++ b/linux-user/microblaze/signal.c
@@ -147,7 +147,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
         return;
     }
 
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
 
     __put_user(0, &frame->uc.tuc_flags);
     __put_user(0, &frame->uc.tuc_link);
diff --git a/linux-user/mips/signal.c b/linux-user/mips/signal.c
index 58a9d7a8a3..d69a5d73dd 100644
--- a/linux-user/mips/signal.c
+++ b/linux-user/mips/signal.c
@@ -303,7 +303,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
         goto give_sigsegv;
     }
 
-    tswap_siginfo(&frame->rs_info, info);
+    frame->rs_info = *info;
 
     __put_user(0, &frame->rs_uc.tuc_flags);
     __put_user(0, &frame->rs_uc.tuc_link);
@@ -311,7 +311,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
 
     setup_sigcontext(env, &frame->rs_uc.tuc_mcontext);
 
-    for(i = 0; i < TARGET_NSIG_WORDS; i++) {
+    for (i = 0; i < TARGET_NSIG_WORDS; i++) {
         __put_user(set->sig[i], &frame->rs_uc.tuc_sigmask.sig[i]);
     }
 
diff --git a/linux-user/nios2/signal.c b/linux-user/nios2/signal.c
index 32b3dc99c6..64c345f409 100644
--- a/linux-user/nios2/signal.c
+++ b/linux-user/nios2/signal.c
@@ -157,7 +157,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
         return;
     }
 
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
 
     /* Create the ucontext.  */
     __put_user(0, &frame->uc.tuc_flags);
diff --git a/linux-user/openrisc/signal.c b/linux-user/openrisc/signal.c
index be8b68784a..cb74a9fe5e 100644
--- a/linux-user/openrisc/signal.c
+++ b/linux-user/openrisc/signal.c
@@ -103,7 +103,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     }
 
     if (ka->sa_flags & SA_SIGINFO) {
-        tswap_siginfo(&frame->info, info);
+        frame->info = *info;
     }
 
     __put_user(0, &frame->uc.tuc_flags);
diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index 7e7302823b..0ebfc0b26b 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -493,7 +493,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     if (!lock_user_struct(VERIFY_WRITE, rt_sf, rt_sf_addr, 1))
         goto sigsegv;
 
-    tswap_siginfo(&rt_sf->info, info);
+    rt_sf->info = *info;
 
     __put_user(0, &rt_sf->uc.tuc_flags);
     __put_user(0, &rt_sf->uc.tuc_link);
@@ -502,7 +502,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     __put_user(h2g (&rt_sf->uc.tuc_mcontext),
                &rt_sf->uc.tuc_regs);
 #endif
-    for(i = 0; i < TARGET_NSIG_WORDS; i++) {
+    for (i = 0; i < TARGET_NSIG_WORDS; i++) {
         __put_user(set->sig[i], &rt_sf->uc.tuc_sigmask.sig[i]);
     }
 
diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
index 941eadce87..358fa1d82d 100644
--- a/linux-user/riscv/signal.c
+++ b/linux-user/riscv/signal.c
@@ -125,7 +125,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     }
 
     setup_ucontext(&frame->uc, env, set);
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
 
     env->pc = ka->_sa_handler;
     env->gpr[xSP] = frame_addr;
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index b40f738a70..df49c24708 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -267,7 +267,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     }
 
     /* Create siginfo on the signal stack. */
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
 
     /* Create ucontext on the signal stack. */
     uc_flags = 0;
diff --git a/linux-user/sh4/signal.c b/linux-user/sh4/signal.c
index c16c2c2d57..9ecc026fae 100644
--- a/linux-user/sh4/signal.c
+++ b/linux-user/sh4/signal.c
@@ -233,7 +233,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
         goto give_sigsegv;
     }
 
-    tswap_siginfo(&frame->info, info);
+    frame->info = *info;
 
     /* Create the ucontext.  */
     __put_user(0, &frame->uc.tuc_flags);
diff --git a/linux-user/signal-common.h b/linux-user/signal-common.h
index 3e2dc604c2..f02d2b6b64 100644
--- a/linux-user/signal-common.h
+++ b/linux-user/signal-common.h
@@ -43,8 +43,6 @@  void host_to_target_sigset_internal(target_sigset_t *d,
                                     const sigset_t *s);
 void target_to_host_sigset_internal(sigset_t *d,
                                     const target_sigset_t *s);
-void tswap_siginfo(target_siginfo_t *tinfo,
-                   const target_siginfo_t *info);
 void set_sigmask(const sigset_t *set);
 void force_sig(int sig);
 void force_sigsegv(int oldsig);
diff --git a/linux-user/signal.c b/linux-user/signal.c
index a57c45de35..7a4c8e416e 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -409,8 +409,8 @@  static inline void host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
     tinfo->si_code = deposit32(si_code, 16, 16, si_type);
 }
 
-void tswap_siginfo(target_siginfo_t *tinfo,
-                   const target_siginfo_t *info)
+static void tswap_siginfo(target_siginfo_t *tinfo,
+                          const target_siginfo_t *info)
 {
     int si_type = extract32(info->si_code, 16, 16);
     int si_code = sextract32(info->si_code, 0, 16);
@@ -1180,6 +1180,12 @@  static void handle_pending_signal(CPUArchState *cpu_env, int sig,
     /* dequeue signal */
     k->pending = 0;
 
+    /*
+     * Writes out siginfo values byteswapped, accordingly to the target. It also
+     * cleans the si_type from si_code making it correct for the target.
+     */
+    tswap_siginfo(&k->info, &k->info);
+
     sig = gdb_handlesig(cpu, sig, NULL);
     if (!sig) {
         sa = NULL;
diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c
index c2dc1000e2..f164b74032 100644
--- a/linux-user/sparc/signal.c
+++ b/linux-user/sparc/signal.c
@@ -333,7 +333,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
 
     __put_user(0, &sf->rwin_save);  /* TODO: save_rwin_state */
 
-    tswap_siginfo(&sf->info, info);
+    sf->info = *info;
     tswap_sigset(&sf->mask, set);
     target_save_altstack(&sf->stack, env);
 
diff --git a/linux-user/xtensa/signal.c b/linux-user/xtensa/signal.c
index 32dcfa5229..11e60c3c82 100644
--- a/linux-user/xtensa/signal.c
+++ b/linux-user/xtensa/signal.c
@@ -184,7 +184,7 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     }
 
     if (ka->sa_flags & SA_SIGINFO) {
-        tswap_siginfo(&frame->info, info);
+        frame->info = *info;
     }
 
     __put_user(0, &frame->uc.tuc_flags);