Patchwork [4/6] alpha-linux-user: Fix sigaction

login
register
mail settings
Submitter Richard Henderson
Date Sept. 15, 2012, 8:24 p.m.
Message ID <1347740649-28646-5-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/184099/
State New
Headers show

Comments

Richard Henderson - Sept. 15, 2012, 8:24 p.m.
Unconditional bswap replaced by __get_user/__put_user.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 linux-user/signal.c       | 22 ++++++++--------------
 linux-user/syscall_defs.h |  2 +-
 2 files changed, 9 insertions(+), 15 deletions(-)
Andreas Färber - Sept. 16, 2012, 12:58 p.m.
Am 15.09.2012 22:24, schrieb Richard Henderson:
> Unconditional bswap replaced by __get_user/__put_user.
> 
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Does that still take care of swapping with the correct size?

The issue for mips was that some oddballs are 32-bit, others long.
Please make sure that mipsn32/mipsn64 (disabled by default due to the
unresolved signal warnings) still compile, too.

Andreas

> ---
>  linux-user/signal.c       | 22 ++++++++--------------
>  linux-user/syscall_defs.h |  2 +-
>  2 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 7869147..bf2dfb8 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -586,28 +586,22 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>              sig, act, oact);
>  #endif
>      if (oact) {
> -        oact->_sa_handler = tswapal(k->_sa_handler);
> -#if defined(TARGET_MIPS) || defined (TARGET_ALPHA)
> -        oact->sa_flags = bswap32(k->sa_flags);
> -#else
> -        oact->sa_flags = tswapal(k->sa_flags);
> -#endif
> +        __put_user(k->_sa_handler, &oact->_sa_handler);
> +        __put_user(k->sa_flags, &oact->sa_flags);
>  #if !defined(TARGET_MIPS)
> -        oact->sa_restorer = tswapal(k->sa_restorer);
> +        __put_user(k->sa_restorer, &oact->sa_restorer);
>  #endif
> +        /* Not swapped.  */
>          oact->sa_mask = k->sa_mask;
>      }
>      if (act) {
>          /* FIXME: This is not threadsafe.  */
> -        k->_sa_handler = tswapal(act->_sa_handler);
> -#if defined(TARGET_MIPS) || defined (TARGET_ALPHA)
> -        k->sa_flags = bswap32(act->sa_flags);
> -#else
> -        k->sa_flags = tswapal(act->sa_flags);
> -#endif
> +        __get_user(k->_sa_handler, &act->_sa_handler);
> +        __get_user(k->sa_flags, &act->sa_flags);
>  #if !defined(TARGET_MIPS)
> -        k->sa_restorer = tswapal(act->sa_restorer);
> +        __get_user(k->sa_restorer, &act->sa_restorer);
>  #endif
> +        /* To be swapped in target_to_host_sigset.  */
>          k->sa_mask = act->sa_mask;
>  
>          /* we update the host linux signal state */
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index a98cbf7..8ca70b9 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -540,7 +540,7 @@ int do_sigaction(int sig, const struct target_sigaction *act,
>  struct target_old_sigaction {
>      abi_ulong _sa_handler;
>      abi_ulong sa_mask;
> -    abi_ulong sa_flags;
> +    int32_t sa_flags;
>  };
>  
>  struct target_rt_sigaction {
>
Peter Maydell - Sept. 16, 2012, 1:21 p.m.
On 16 September 2012 13:58, Andreas Färber <afaerber@suse.de> wrote:
> Am 15.09.2012 22:24, schrieb Richard Henderson:
>> Unconditional bswap replaced by __get_user/__put_user.
>>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>
> Does that still take care of swapping with the correct size?

Yes; they do target-swapping based on the type of pointer you
pass them.


-- PMM
Richard Henderson - Sept. 16, 2012, 4:54 p.m.
On 2012-09-16 05:58, Andreas Färber wrote:
> Am 15.09.2012 22:24, schrieb Richard Henderson:
>> Unconditional bswap replaced by __get_user/__put_user.
>>
>> Signed-off-by: Richard Henderson <rth@twiddle.net>
> 
> Does that still take care of swapping with the correct size?

Yes, and automatically too.

> The issue for mips was that some oddballs are 32-bit, others long.
> Please make sure that mipsn32/mipsn64 (disabled by default due to the
> unresolved signal warnings) still compile, too.

Alpha is one of the odd balls.


r~

Patch

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 7869147..bf2dfb8 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -586,28 +586,22 @@  int do_sigaction(int sig, const struct target_sigaction *act,
             sig, act, oact);
 #endif
     if (oact) {
-        oact->_sa_handler = tswapal(k->_sa_handler);
-#if defined(TARGET_MIPS) || defined (TARGET_ALPHA)
-        oact->sa_flags = bswap32(k->sa_flags);
-#else
-        oact->sa_flags = tswapal(k->sa_flags);
-#endif
+        __put_user(k->_sa_handler, &oact->_sa_handler);
+        __put_user(k->sa_flags, &oact->sa_flags);
 #if !defined(TARGET_MIPS)
-        oact->sa_restorer = tswapal(k->sa_restorer);
+        __put_user(k->sa_restorer, &oact->sa_restorer);
 #endif
+        /* Not swapped.  */
         oact->sa_mask = k->sa_mask;
     }
     if (act) {
         /* FIXME: This is not threadsafe.  */
-        k->_sa_handler = tswapal(act->_sa_handler);
-#if defined(TARGET_MIPS) || defined (TARGET_ALPHA)
-        k->sa_flags = bswap32(act->sa_flags);
-#else
-        k->sa_flags = tswapal(act->sa_flags);
-#endif
+        __get_user(k->_sa_handler, &act->_sa_handler);
+        __get_user(k->sa_flags, &act->sa_flags);
 #if !defined(TARGET_MIPS)
-        k->sa_restorer = tswapal(act->sa_restorer);
+        __get_user(k->sa_restorer, &act->sa_restorer);
 #endif
+        /* To be swapped in target_to_host_sigset.  */
         k->sa_mask = act->sa_mask;
 
         /* we update the host linux signal state */
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index a98cbf7..8ca70b9 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -540,7 +540,7 @@  int do_sigaction(int sig, const struct target_sigaction *act,
 struct target_old_sigaction {
     abi_ulong _sa_handler;
     abi_ulong sa_mask;
-    abi_ulong sa_flags;
+    int32_t sa_flags;
 };
 
 struct target_rt_sigaction {