diff mbox

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

Message ID 20200209185729.15896-1-git@xen0n.name
State New
Headers show

Commit Message

WANG Xuerui Feb. 9, 2020, 6:57 p.m. UTC
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(-)

Comments

Adhemerval Zanella Feb. 10, 2020, 4:06 p.m. UTC | #1
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 <adhemerval.zanella@linaro.org>

> ---
>  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.
Joseph Myers Feb. 10, 2020, 10:34 p.m. UTC | #2
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.
Maciej W. Rozycki Feb. 22, 2020, 10:40 p.m. UTC | #3
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
Adhemerval Zanella Feb. 25, 2020, 8:01 p.m. UTC | #4
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.
Matt Turner Feb. 27, 2020, 6:23 p.m. UTC | #5
On Sat, Feb 22, 2020 at 2:40 PM Maciej W. Rozycki <macro@linux-mips.org> 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
Maciej W. Rozycki March 17, 2020, 12:18 a.m. UTC | #6
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
diff mbox

Patch

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								\