[PATCH] mips: Fix argument passing for inlined syscalls on Linux [BZ #25523]

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] mips: Fix argument passing for inlined syscalls on Linux [BZ #25523]

WANG Xuerui
According to [gcc documentation][1], temporary variables must be used for
the desired content to not be call-clobbered.

Fix the Linux inline syscall templates by adding temporary variables,
much like what x86 did before
(commit 381a0c26d73e0f074c962e0ab53b99a6c327066d).

Tested with gcc 9.2.0, both cross-compiled and natively on Loongson
3A4000.

[1]: https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html
---
 sysdeps/unix/sysv/linux/mips/mips32/sysdep.h  | 30 ++++++---
 .../unix/sysv/linux/mips/mips64/n32/sysdep.h  | 63 ++++++++++++-------
 .../unix/sysv/linux/mips/mips64/n64/sysdep.h  | 63 ++++++++++++-------
 3 files changed, 104 insertions(+), 52 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
index beefcf284b..c275d63f67 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
@@ -178,10 +178,11 @@ union __mips_syscall_return
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
+ register long __a0 asm ("$4") = _arg1; \
  register long __a3 asm ("$7"); \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
@@ -202,11 +203,13 @@ union __mips_syscall_return
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
+ long _arg2 = (long) (arg2); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
- register long __a1 asm ("$5") = (long) (arg2); \
+ register long __a0 asm ("$4") = _arg1; \
+ register long __a1 asm ("$5") = _arg2; \
  register long __a3 asm ("$7"); \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
@@ -228,12 +231,15 @@ union __mips_syscall_return
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
+ long _arg2 = (long) (arg2); \
+ long _arg3 = (long) (arg3); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
- register long __a1 asm ("$5") = (long) (arg2); \
- register long __a2 asm ("$6") = (long) (arg3); \
+ register long __a0 asm ("$4") = _arg1; \
+ register long __a1 asm ("$5") = _arg2; \
+ register long __a2 asm ("$6") = _arg3; \
  register long __a3 asm ("$7"); \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
@@ -255,13 +261,17 @@ union __mips_syscall_return
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
+ long _arg2 = (long) (arg2); \
+ long _arg3 = (long) (arg3); \
+ long _arg4 = (long) (arg4); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
- register long __a1 asm ("$5") = (long) (arg2); \
- register long __a2 asm ("$6") = (long) (arg3); \
- register long __a3 asm ("$7") = (long) (arg4); \
+ register long __a0 asm ("$4") = _arg1; \
+ register long __a1 asm ("$5") = _arg2; \
+ register long __a2 asm ("$6") = _arg3; \
+ register long __a3 asm ("$7") = _arg4; \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
  v0_init \
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h
index f96636538a..958a889147 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h
@@ -138,10 +138,11 @@
  long _sys_result; \
  \
  { \
+ long long _arg1 = ARGIFY (arg1); \
  register long long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long long __v0 asm ("$2"); \
- register long long __a0 asm ("$4") = ARGIFY (arg1); \
+ register long long __a0 asm ("$4") = _arg1; \
  register long long __a3 asm ("$7"); \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
@@ -162,11 +163,13 @@
  long _sys_result; \
  \
  { \
+ long long _arg1 = ARGIFY (arg1); \
+ long long _arg2 = ARGIFY (arg2); \
  register long long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long long __v0 asm ("$2"); \
- register long long __a0 asm ("$4") = ARGIFY (arg1); \
- register long long __a1 asm ("$5") = ARGIFY (arg2); \
+ register long long __a0 asm ("$4") = _arg1; \
+ register long long __a1 asm ("$5") = _arg2; \
  register long long __a3 asm ("$7"); \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
@@ -188,12 +191,15 @@
  long _sys_result; \
  \
  { \
+ long long _arg1 = ARGIFY (arg1); \
+ long long _arg2 = ARGIFY (arg2); \
+ long long _arg3 = ARGIFY (arg3); \
  register long long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long long __v0 asm ("$2"); \
- register long long __a0 asm ("$4") = ARGIFY (arg1); \
- register long long __a1 asm ("$5") = ARGIFY (arg2); \
- register long long __a2 asm ("$6") = ARGIFY (arg3); \
+ register long long __a0 asm ("$4") = _arg1; \
+ register long long __a1 asm ("$5") = _arg2; \
+ register long long __a2 asm ("$6") = _arg3; \
  register long long __a3 asm ("$7"); \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
@@ -215,13 +221,17 @@
  long _sys_result; \
  \
  { \
+ long long _arg1 = ARGIFY (arg1); \
+ long long _arg2 = ARGIFY (arg2); \
+ long long _arg3 = ARGIFY (arg3); \
+ long long _arg4 = ARGIFY (arg4); \
  register long long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long long __v0 asm ("$2"); \
- register long long __a0 asm ("$4") = ARGIFY (arg1); \
- register long long __a1 asm ("$5") = ARGIFY (arg2); \
- register long long __a2 asm ("$6") = ARGIFY (arg3); \
- register long long __a3 asm ("$7") = ARGIFY (arg4); \
+ register long long __a0 asm ("$4") = _arg1; \
+ register long long __a1 asm ("$5") = _arg2; \
+ register long long __a2 asm ("$6") = _arg3; \
+ register long long __a3 asm ("$7") = _arg4; \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
  v0_init \
@@ -242,14 +252,19 @@
  long _sys_result; \
  \
  { \
+ long long _arg1 = ARGIFY (arg1); \
+ long long _arg2 = ARGIFY (arg2); \
+ long long _arg3 = ARGIFY (arg3); \
+ long long _arg4 = ARGIFY (arg4); \
+ long long _arg5 = ARGIFY (arg5); \
  register long long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long long __v0 asm ("$2"); \
- register long long __a0 asm ("$4") = ARGIFY (arg1); \
- register long long __a1 asm ("$5") = ARGIFY (arg2); \
- register long long __a2 asm ("$6") = ARGIFY (arg3); \
- register long long __a3 asm ("$7") = ARGIFY (arg4); \
- register long long __a4 asm ("$8") = ARGIFY (arg5); \
+ register long long __a0 asm ("$4") = _arg1; \
+ register long long __a1 asm ("$5") = _arg2; \
+ register long long __a2 asm ("$6") = _arg3; \
+ register long long __a3 asm ("$7") = _arg4; \
+ register long long __a4 asm ("$8") = _arg5; \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
  v0_init \
@@ -270,15 +285,21 @@
  long _sys_result; \
  \
  { \
+ long long _arg1 = ARGIFY (arg1); \
+ long long _arg2 = ARGIFY (arg2); \
+ long long _arg3 = ARGIFY (arg3); \
+ long long _arg4 = ARGIFY (arg4); \
+ long long _arg5 = ARGIFY (arg5); \
+ long long _arg6 = ARGIFY (arg6); \
  register long long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long long __v0 asm ("$2"); \
- register long long __a0 asm ("$4") = ARGIFY (arg1); \
- register long long __a1 asm ("$5") = ARGIFY (arg2); \
- register long long __a2 asm ("$6") = ARGIFY (arg3); \
- register long long __a3 asm ("$7") = ARGIFY (arg4); \
- register long long __a4 asm ("$8") = ARGIFY (arg5); \
- register long long __a5 asm ("$9") = ARGIFY (arg6); \
+ register long long __a0 asm ("$4") = _arg1; \
+ register long long __a1 asm ("$5") = _arg2; \
+ register long long __a2 asm ("$6") = _arg3; \
+ register long long __a3 asm ("$7") = _arg4; \
+ register long long __a4 asm ("$8") = _arg5; \
+ register long long __a5 asm ("$9") = _arg6; \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
  v0_init \
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h
index 9d30291f84..f47613deaf 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h
@@ -134,10 +134,11 @@
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
+ register long __a0 asm ("$4") = _arg1; \
  register long __a3 asm ("$7"); \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
@@ -158,11 +159,13 @@
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
+ long _arg2 = (long) (arg2); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
- register long __a1 asm ("$5") = (long) (arg2); \
+ register long __a0 asm ("$4") = _arg1; \
+ register long __a1 asm ("$5") = _arg2; \
  register long __a3 asm ("$7"); \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
@@ -184,12 +187,15 @@
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
+ long _arg2 = (long) (arg2); \
+ long _arg3 = (long) (arg3); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
- register long __a1 asm ("$5") = (long) (arg2); \
- register long __a2 asm ("$6") = (long) (arg3); \
+ register long __a0 asm ("$4") = _arg1; \
+ register long __a1 asm ("$5") = _arg2; \
+ register long __a2 asm ("$6") = _arg3; \
  register long __a3 asm ("$7"); \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
@@ -211,13 +217,17 @@
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
+ long _arg2 = (long) (arg2); \
+ long _arg3 = (long) (arg3); \
+ long _arg4 = (long) (arg4); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
- register long __a1 asm ("$5") = (long) (arg2); \
- register long __a2 asm ("$6") = (long) (arg3); \
- register long __a3 asm ("$7") = (long) (arg4); \
+ register long __a0 asm ("$4") = _arg1; \
+ register long __a1 asm ("$5") = _arg2; \
+ register long __a2 asm ("$6") = _arg3; \
+ register long __a3 asm ("$7") = _arg4; \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
  v0_init \
@@ -238,14 +248,19 @@
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
+ long _arg2 = (long) (arg2); \
+ long _arg3 = (long) (arg3); \
+ long _arg4 = (long) (arg4); \
+ long _arg5 = (long) (arg5); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
- register long __a1 asm ("$5") = (long) (arg2); \
- register long __a2 asm ("$6") = (long) (arg3); \
- register long __a3 asm ("$7") = (long) (arg4); \
- register long __a4 asm ("$8") = (long) (arg5); \
+ register long __a0 asm ("$4") = _arg1; \
+ register long __a1 asm ("$5") = _arg2; \
+ register long __a2 asm ("$6") = _arg3; \
+ register long __a3 asm ("$7") = _arg4; \
+ register long __a4 asm ("$8") = _arg5; \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
  v0_init \
@@ -266,15 +281,21 @@
  long _sys_result; \
  \
  { \
+ long _arg1 = (long) (arg1); \
+ long _arg2 = (long) (arg2); \
+ long _arg3 = (long) (arg3); \
+ long _arg4 = (long) (arg4); \
+ long _arg5 = (long) (arg5); \
+ long _arg6 = (long) (arg6); \
  register long __s0 asm ("$16") __attribute__ ((unused)) \
   = (number); \
  register long __v0 asm ("$2"); \
- register long __a0 asm ("$4") = (long) (arg1); \
- register long __a1 asm ("$5") = (long) (arg2); \
- register long __a2 asm ("$6") = (long) (arg3); \
- register long __a3 asm ("$7") = (long) (arg4); \
- register long __a4 asm ("$8") = (long) (arg5); \
- register long __a5 asm ("$9") = (long) (arg6); \
+ register long __a0 asm ("$4") = _arg1; \
+ register long __a1 asm ("$5") = _arg2; \
+ register long __a2 asm ("$6") = _arg3; \
+ register long __a3 asm ("$7") = _arg4; \
+ register long __a4 asm ("$8") = _arg5; \
+ register long __a5 asm ("$9") = _arg6; \
  __asm__ volatile ( \
  ".set\tnoreorder\n\t" \
  v0_init \
--
2.24.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mips: Fix argument passing for inlined syscalls on Linux [BZ #25523]

Adhemerval Zanella-2


On 09/02/2020 15:57, WANG Xuerui wrote:

> According to [gcc documentation][1], temporary variables must be used for
> the desired content to not be call-clobbered.
>
> Fix the Linux inline syscall templates by adding temporary variables,
> much like what x86 did before
> (commit 381a0c26d73e0f074c962e0ab53b99a6c327066d).
>
> Tested with gcc 9.2.0, both cross-compiled and natively on Loongson
> 3A4000.
>
> [1]: https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html

LGTM, thanks.

Reviewed-by: Adhemerval Zanella <[hidden email]>

> ---
>  sysdeps/unix/sysv/linux/mips/mips32/sysdep.h  | 30 ++++++---
>  .../unix/sysv/linux/mips/mips64/n32/sysdep.h  | 63 ++++++++++++-------
>  .../unix/sysv/linux/mips/mips64/n64/sysdep.h  | 63 ++++++++++++-------
>  3 files changed, 104 insertions(+), 52 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> index beefcf284b..c275d63f67 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> @@ -178,10 +178,11 @@ union __mips_syscall_return
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> + register long __a0 asm ("$4") = _arg1; \
>   register long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

Ok.

> @@ -202,11 +203,13 @@ union __mips_syscall_return
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
> + long _arg2 = (long) (arg2); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> - register long __a1 asm ("$5") = (long) (arg2); \
> + register long __a0 asm ("$4") = _arg1; \
> + register long __a1 asm ("$5") = _arg2; \
>   register long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

Ok.

> @@ -228,12 +231,15 @@ union __mips_syscall_return
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
> + long _arg2 = (long) (arg2); \
> + long _arg3 = (long) (arg3); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> - register long __a1 asm ("$5") = (long) (arg2); \
> - register long __a2 asm ("$6") = (long) (arg3); \
> + register long __a0 asm ("$4") = _arg1; \
> + register long __a1 asm ("$5") = _arg2; \
> + register long __a2 asm ("$6") = _arg3; \
>   register long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

Ok.

> @@ -255,13 +261,17 @@ union __mips_syscall_return
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
> + long _arg2 = (long) (arg2); \
> + long _arg3 = (long) (arg3); \
> + long _arg4 = (long) (arg4); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> - register long __a1 asm ("$5") = (long) (arg2); \
> - register long __a2 asm ("$6") = (long) (arg3); \
> - register long __a3 asm ("$7") = (long) (arg4); \
> + register long __a0 asm ("$4") = _arg1; \
> + register long __a1 asm ("$5") = _arg2; \
> + register long __a2 asm ("$6") = _arg3; \
> + register long __a3 asm ("$7") = _arg4; \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \
>   v0_init \

Ok.

> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h
> index f96636538a..958a889147 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/sysdep.h
> @@ -138,10 +138,11 @@
>   long _sys_result; \
>   \
>   { \
> + long long _arg1 = ARGIFY (arg1); \
>   register long long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long long __v0 asm ("$2"); \
> - register long long __a0 asm ("$4") = ARGIFY (arg1); \
> + register long long __a0 asm ("$4") = _arg1; \
>   register long long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

Ok.

> @@ -162,11 +163,13 @@
>   long _sys_result; \
>   \
>   { \
> + long long _arg1 = ARGIFY (arg1); \
> + long long _arg2 = ARGIFY (arg2); \
>   register long long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long long __v0 asm ("$2"); \
> - register long long __a0 asm ("$4") = ARGIFY (arg1); \
> - register long long __a1 asm ("$5") = ARGIFY (arg2); \
> + register long long __a0 asm ("$4") = _arg1; \
> + register long long __a1 asm ("$5") = _arg2; \
>   register long long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

Ok.

> @@ -188,12 +191,15 @@
>   long _sys_result; \
>   \
>   { \
> + long long _arg1 = ARGIFY (arg1); \
> + long long _arg2 = ARGIFY (arg2); \
> + long long _arg3 = ARGIFY (arg3); \
>   register long long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long long __v0 asm ("$2"); \
> - register long long __a0 asm ("$4") = ARGIFY (arg1); \
> - register long long __a1 asm ("$5") = ARGIFY (arg2); \
> - register long long __a2 asm ("$6") = ARGIFY (arg3); \
> + register long long __a0 asm ("$4") = _arg1; \
> + register long long __a1 asm ("$5") = _arg2; \
> + register long long __a2 asm ("$6") = _arg3; \
>   register long long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

Ok.

> @@ -215,13 +221,17 @@
>   long _sys_result; \
>   \
>   { \
> + long long _arg1 = ARGIFY (arg1); \
> + long long _arg2 = ARGIFY (arg2); \
> + long long _arg3 = ARGIFY (arg3); \
> + long long _arg4 = ARGIFY (arg4); \
>   register long long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long long __v0 asm ("$2"); \
> - register long long __a0 asm ("$4") = ARGIFY (arg1); \
> - register long long __a1 asm ("$5") = ARGIFY (arg2); \
> - register long long __a2 asm ("$6") = ARGIFY (arg3); \
> - register long long __a3 asm ("$7") = ARGIFY (arg4); \
> + register long long __a0 asm ("$4") = _arg1; \
> + register long long __a1 asm ("$5") = _arg2; \
> + register long long __a2 asm ("$6") = _arg3; \
> + register long long __a3 asm ("$7") = _arg4; \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \
>   v0_init \

Ok.

> @@ -242,14 +252,19 @@
>   long _sys_result; \
>   \
>   { \
> + long long _arg1 = ARGIFY (arg1); \
> + long long _arg2 = ARGIFY (arg2); \
> + long long _arg3 = ARGIFY (arg3); \
> + long long _arg4 = ARGIFY (arg4); \
> + long long _arg5 = ARGIFY (arg5); \
>   register long long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long long __v0 asm ("$2"); \
> - register long long __a0 asm ("$4") = ARGIFY (arg1); \
> - register long long __a1 asm ("$5") = ARGIFY (arg2); \
> - register long long __a2 asm ("$6") = ARGIFY (arg3); \
> - register long long __a3 asm ("$7") = ARGIFY (arg4); \
> - register long long __a4 asm ("$8") = ARGIFY (arg5); \
> + register long long __a0 asm ("$4") = _arg1; \
> + register long long __a1 asm ("$5") = _arg2; \
> + register long long __a2 asm ("$6") = _arg3; \
> + register long long __a3 asm ("$7") = _arg4; \
> + register long long __a4 asm ("$8") = _arg5; \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \
>   v0_init \

Ok.

> @@ -270,15 +285,21 @@
>   long _sys_result; \
>   \
>   { \
> + long long _arg1 = ARGIFY (arg1); \
> + long long _arg2 = ARGIFY (arg2); \
> + long long _arg3 = ARGIFY (arg3); \
> + long long _arg4 = ARGIFY (arg4); \
> + long long _arg5 = ARGIFY (arg5); \
> + long long _arg6 = ARGIFY (arg6); \
>   register long long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long long __v0 asm ("$2"); \
> - register long long __a0 asm ("$4") = ARGIFY (arg1); \
> - register long long __a1 asm ("$5") = ARGIFY (arg2); \
> - register long long __a2 asm ("$6") = ARGIFY (arg3); \
> - register long long __a3 asm ("$7") = ARGIFY (arg4); \
> - register long long __a4 asm ("$8") = ARGIFY (arg5); \
> - register long long __a5 asm ("$9") = ARGIFY (arg6); \
> + register long long __a0 asm ("$4") = _arg1; \
> + register long long __a1 asm ("$5") = _arg2; \
> + register long long __a2 asm ("$6") = _arg3; \
> + register long long __a3 asm ("$7") = _arg4; \
> + register long long __a4 asm ("$8") = _arg5; \
> + register long long __a5 asm ("$9") = _arg6; \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \
>   v0_init \

Ok.

> diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h
> index 9d30291f84..f47613deaf 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/sysdep.h
> @@ -134,10 +134,11 @@
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> + register long __a0 asm ("$4") = _arg1; \
>   register long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

Ok.

> @@ -158,11 +159,13 @@
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
> + long _arg2 = (long) (arg2); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> - register long __a1 asm ("$5") = (long) (arg2); \
> + register long __a0 asm ("$4") = _arg1; \
> + register long __a1 asm ("$5") = _arg2; \
>   register long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

Ok.

> @@ -184,12 +187,15 @@
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
> + long _arg2 = (long) (arg2); \
> + long _arg3 = (long) (arg3); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> - register long __a1 asm ("$5") = (long) (arg2); \
> - register long __a2 asm ("$6") = (long) (arg3); \
> + register long __a0 asm ("$4") = _arg1; \
> + register long __a1 asm ("$5") = _arg2; \
> + register long __a2 asm ("$6") = _arg3; \
>   register long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

Ok.

> @@ -211,13 +217,17 @@
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
> + long _arg2 = (long) (arg2); \
> + long _arg3 = (long) (arg3); \
> + long _arg4 = (long) (arg4); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> - register long __a1 asm ("$5") = (long) (arg2); \
> - register long __a2 asm ("$6") = (long) (arg3); \
> - register long __a3 asm ("$7") = (long) (arg4); \
> + register long __a0 asm ("$4") = _arg1; \
> + register long __a1 asm ("$5") = _arg2; \
> + register long __a2 asm ("$6") = _arg3; \
> + register long __a3 asm ("$7") = _arg4; \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \
>   v0_init \

Ok.

> @@ -238,14 +248,19 @@
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
> + long _arg2 = (long) (arg2); \
> + long _arg3 = (long) (arg3); \
> + long _arg4 = (long) (arg4); \
> + long _arg5 = (long) (arg5); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> - register long __a1 asm ("$5") = (long) (arg2); \
> - register long __a2 asm ("$6") = (long) (arg3); \
> - register long __a3 asm ("$7") = (long) (arg4); \
> - register long __a4 asm ("$8") = (long) (arg5); \
> + register long __a0 asm ("$4") = _arg1; \
> + register long __a1 asm ("$5") = _arg2; \
> + register long __a2 asm ("$6") = _arg3; \
> + register long __a3 asm ("$7") = _arg4; \
> + register long __a4 asm ("$8") = _arg5; \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \
>   v0_init \

Ok.

> @@ -266,15 +281,21 @@
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
> + long _arg2 = (long) (arg2); \
> + long _arg3 = (long) (arg3); \
> + long _arg4 = (long) (arg4); \
> + long _arg5 = (long) (arg5); \
> + long _arg6 = (long) (arg6); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> - register long __a1 asm ("$5") = (long) (arg2); \
> - register long __a2 asm ("$6") = (long) (arg3); \
> - register long __a3 asm ("$7") = (long) (arg4); \
> - register long __a4 asm ("$8") = (long) (arg5); \
> - register long __a5 asm ("$9") = (long) (arg6); \
> + register long __a0 asm ("$4") = _arg1; \
> + register long __a1 asm ("$5") = _arg2; \
> + register long __a2 asm ("$6") = _arg3; \
> + register long __a3 asm ("$7") = _arg4; \
> + register long __a4 asm ("$8") = _arg5; \
> + register long __a5 asm ("$9") = _arg6; \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \
>   v0_init \
>

Ok.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mips: Fix argument passing for inlined syscalls on Linux [BZ #25523]

Joseph Myers
In reply to this post by WANG Xuerui
On Mon, 10 Feb 2020, WANG Xuerui wrote:

> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> index beefcf284b..c275d63f67 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> @@ -178,10 +178,11 @@ union __mips_syscall_return
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \

The glibc style is to use "long int" in place of "long", so I think the
new variables should do that throughout the patch (and likewise "long long
int").

Please post a revised patch, and say if you need someone to commit it for
you.

--
Joseph S. Myers
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mips: Fix argument passing for inlined syscalls on Linux [BZ #25523]

Maciej W. Rozycki
In reply to this post by WANG Xuerui
On Mon, 10 Feb 2020, WANG Xuerui wrote:

> According to [gcc documentation][1], temporary variables must be used for
> the desired content to not be call-clobbered.

 Why does it specifically matter here?

> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> index beefcf284b..c275d63f67 100644
> --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
[...]

> @@ -202,11 +203,13 @@ union __mips_syscall_return
>   long _sys_result; \
>   \
>   { \
> + long _arg1 = (long) (arg1); \
> + long _arg2 = (long) (arg2); \
>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>    = (number); \
>   register long __v0 asm ("$2"); \
> - register long __a0 asm ("$4") = (long) (arg1); \
> - register long __a1 asm ("$5") = (long) (arg2); \
> + register long __a0 asm ("$4") = _arg1; \
> + register long __a1 asm ("$5") = _arg2; \
>   register long __a3 asm ("$7"); \
>   __asm__ volatile ( \
>   ".set\tnoreorder\n\t" \

 Can e.g. `(long) (arg1)' end up as a library call?  Do you have a case,
even potential, where such clobbering actually happens?

  Maciej
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mips: Fix argument passing for inlined syscalls on Linux [BZ #25523]

Adhemerval Zanella-2


On 22/02/2020 19:40, Maciej W. Rozycki wrote:

> On Mon, 10 Feb 2020, WANG Xuerui wrote:
>
>> According to [gcc documentation][1], temporary variables must be used for
>> the desired content to not be call-clobbered.
>
>  Why does it specifically matter here?
>
>> diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
>> index beefcf284b..c275d63f67 100644
>> --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
>> +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> [...]
>> @@ -202,11 +203,13 @@ union __mips_syscall_return
>>   long _sys_result; \
>>   \
>>   { \
>> + long _arg1 = (long) (arg1); \
>> + long _arg2 = (long) (arg2); \
>>   register long __s0 asm ("$16") __attribute__ ((unused)) \
>>    = (number); \
>>   register long __v0 asm ("$2"); \
>> - register long __a0 asm ("$4") = (long) (arg1); \
>> - register long __a1 asm ("$5") = (long) (arg2); \
>> + register long __a0 asm ("$4") = _arg1; \
>> + register long __a1 asm ("$5") = _arg2; \
>>   register long __a3 asm ("$7"); \
>>   __asm__ volatile ( \
>>   ".set\tnoreorder\n\t" \
>
>  Can e.g. `(long) (arg1)' end up as a library call?  Do you have a case,
> even potential, where such clobbering actually happens?

On sysdeps/unix/sysv/linux/spawni.c:

188   if ((attr->__flags & POSIX_SPAWN_RESETIDS) != 0
189       && (local_seteuid (__getuid ()) != 0
190           || local_setegid (__getgid ()) != 0))
191     goto fail;

And local_seteuid/local_setegid are defined as:

sysdeps/unix/sysv/linux/local-setxid.h:
  5 #ifdef __NR_setresuid32
  6 # define local_seteuid(id) INLINE_SYSCALL (setresuid32, 3, -1, id, -1)
  7 #else
  8 # define local_seteuid(id) INLINE_SYSCALL (setresuid, 3, -1, id, -1)
  9 #endif
 10
 11
 12 #ifdef __NR_setresgid32
 13 # define local_setegid(id) INLINE_SYSCALL (setresgid32, 3, -1, id, -1)
 14 #else
 15 # define local_setegid(id) INLINE_SYSCALL (setresgid, 3, -1, id, -1)
 16 #endif

In any case, the previous usage of inline syscall is indeed fragile
and subject to such potential breakage.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mips: Fix argument passing for inlined syscalls on Linux [BZ #25523]

Matt Turner-3
In reply to this post by Maciej W. Rozycki
On Sat, Feb 22, 2020 at 2:40 PM Maciej W. Rozycki <[hidden email]> wrote:

>
> On Mon, 10 Feb 2020, WANG Xuerui wrote:
>
> > According to [gcc documentation][1], temporary variables must be used for
> > the desired content to not be call-clobbered.
>
>  Why does it specifically matter here?
>
> > diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> > index beefcf284b..c275d63f67 100644
> > --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> [...]
> > @@ -202,11 +203,13 @@ union __mips_syscall_return
> >       long _sys_result;                                               \
> >                                                                       \
> >       {                                                               \
> > +     long _arg1 = (long) (arg1);                                     \
> > +     long _arg2 = (long) (arg2);                                     \
> >       register long __s0 asm ("$16") __attribute__ ((unused))         \
> >         = (number);                                                   \
> >       register long __v0 asm ("$2");                                  \
> > -     register long __a0 asm ("$4") = (long) (arg1);                  \
> > -     register long __a1 asm ("$5") = (long) (arg2);                  \
> > +     register long __a0 asm ("$4") = _arg1;                          \
> > +     register long __a1 asm ("$5") = _arg2;                          \
> >       register long __a3 asm ("$7");                                  \
> >       __asm__ volatile (                                              \
> >       ".set\tnoreorder\n\t"                                           \
>
>  Can e.g. `(long) (arg1)' end up as a library call?  Do you have a case,
> even potential, where such clobbering actually happens?

We found that GNU make 4.3 fails to work on MIPS without this patch to
glibc. See https://bugs.gentoo.org/708758
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mips: Fix argument passing for inlined syscalls on Linux [BZ #25523]

Maciej W. Rozycki
On Thu, 27 Feb 2020, Matt Turner wrote:

> > > diff --git a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> > > index beefcf284b..c275d63f67 100644
> > > --- a/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> > > +++ b/sysdeps/unix/sysv/linux/mips/mips32/sysdep.h
> > [...]
> > > @@ -202,11 +203,13 @@ union __mips_syscall_return
> > >       long _sys_result;                                               \
> > >                                                                       \
> > >       {                                                               \
> > > +     long _arg1 = (long) (arg1);                                     \
> > > +     long _arg2 = (long) (arg2);                                     \
> > >       register long __s0 asm ("$16") __attribute__ ((unused))         \
> > >         = (number);                                                   \
> > >       register long __v0 asm ("$2");                                  \
> > > -     register long __a0 asm ("$4") = (long) (arg1);                  \
> > > -     register long __a1 asm ("$5") = (long) (arg2);                  \
> > > +     register long __a0 asm ("$4") = _arg1;                          \
> > > +     register long __a1 asm ("$5") = _arg2;                          \
> > >       register long __a3 asm ("$7");                                  \
> > >       __asm__ volatile (                                              \
> > >       ".set\tnoreorder\n\t"                                           \
> >
> >  Can e.g. `(long) (arg1)' end up as a library call?  Do you have a case,
> > even potential, where such clobbering actually happens?
>
> We found that GNU make 4.3 fails to work on MIPS without this patch to
> glibc. See https://bugs.gentoo.org/708758

 Ah, indeed it can then, when whatever is passed as `arg1' gets inlined.
Thanks for the pointer and sorry about the confusion.

  Maciej