diff mbox series

mips: FIx clone3 implementation (BZ 31325)

Message ID 20240201174103.798138-1-adhemerval.zanella@linaro.org
State Accepted
Commit bbd248ac0d75efdef8fe61ea69b1fb25fb95b6e7
Headers show
Series mips: FIx clone3 implementation (BZ 31325) | expand

Commit Message

Adhemerval Zanella Feb. 1, 2024, 5:41 p.m. UTC
For o32 we need to setup a minimal stack frame to allow cprestore
on __thread_start_clone3 (which instruct the linker to save the
gp for PIC).  Also, there is no guarantee by kABI that $8 will be
preserved after syscall execution, so we need to save it on the
provided stack.

Checked on mipsel-linux-gnu.

Reported-by: Khem Raj <raj.khem@gmail.com>
---
 sysdeps/unix/sysv/linux/mips/clone3.S | 32 ++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

Comments

Khem Raj Feb. 2, 2024, 12:06 a.m. UTC | #1
On Thu, Feb 1, 2024 at 9:41 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
> For o32 we need to setup a minimal stack frame to allow cprestore
> on __thread_start_clone3 (which instruct the linker to save the
> gp for PIC).  Also, there is no guarantee by kABI that $8 will be
> preserved after syscall execution, so we need to save it on the
> provided stack.
>
> Checked on mipsel-linux-gnu.

works ok on yocto reference mips ( Big-endian ) ( mips-linux-gnu )

Tested-by: Khem Raj <raj.khem@gmail.com>

>
> Reported-by: Khem Raj <raj.khem@gmail.com>
> ---
>  sysdeps/unix/sysv/linux/mips/clone3.S | 32 ++++++++++++++++++++++-----
>  1 file changed, 27 insertions(+), 5 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/mips/clone3.S b/sysdeps/unix/sysv/linux/mips/clone3.S
> index e9fec2fa47..481b8ae963 100644
> --- a/sysdeps/unix/sysv/linux/mips/clone3.S
> +++ b/sysdeps/unix/sysv/linux/mips/clone3.S
> @@ -37,11 +37,6 @@
>
>         .text
>         .set            nomips16
> -#if _MIPS_SIM == _ABIO32
> -# define EXTRA_LOCALS 1
> -#else
> -# define EXTRA_LOCALS 0
> -#endif
>  #define FRAMESZ ((NARGSAVE*SZREG)+ALSZ)&ALMASK
>  GPOFF= FRAMESZ-(1*SZREG)
>  NESTED(__clone3, SZREG, sp)
> @@ -68,8 +63,31 @@ NESTED(__clone3, SZREG, sp)
>         beqz    a0, L(error)    /* No NULL cl_args pointer.  */
>         beqz    a2, L(error)    /* No NULL function pointer.  */
>
> +#if _MIPS_SIM == _ABIO32
> +       /* Both stack and stack_size on clone_args are defined as uint64_t, and
> +          there is no need to handle values larger than to 32 bits for o32.  */
> +# if __BYTE_ORDER == __BIG_ENDIAN
> +#  define CL_STACKPOINTER_OFFSET  44
> +#  define CL_STACKSIZE_OFFSET     52
> +# else
> +#  define CL_STACKPOINTER_OFFSET  40
> +#  define CL_STACKSIZE_OFFSET     48
> +# endif
> +
> +       /* For o32 we need to setup a minimal stack frame to allow cprestore
> +          on __thread_start_clone3.  Also there is no guarantee by kABI that
> +          $8 will be preserved after syscall execution (so we need to save it
> +          on the provided stack).  */
> +       lw      t0, CL_STACKPOINTER_OFFSET(a0)  /* Load the stack pointer.  */
> +       lw      t1, CL_STACKSIZE_OFFSET(a0)     /* Load the stack_size.  */
> +       addiu   t1, -32                         /* Update the stack size.  */
> +       addu    t2, t1, t0                      /* Calculate the thread stack.  */
> +       sw      a3, 0(t2)                       /* Save argument pointer.  */
> +       sw      t1, CL_STACKSIZE_OFFSET(a0)     /* Save the new stack size.  */
> +#else
>         move    $8, a3          /* a3 is set to 0/1 for syscall success/error
>                                    while a4/$8 is returned unmodified.  */
> +#endif
>
>         /* Do the system call, the kernel expects:
>            v0: system call number
> @@ -125,7 +143,11 @@ L(thread_start_clone3):
>
>         /* Restore the arg for user's function.  */
>         move            t9, a2          /* Function pointer.  */
> +#if _MIPS_SIM == _ABIO32
> +       PTR_L           a0, 0(sp)
> +#else
>         move            a0, $8          /* Argument pointer.  */
> +#endif
>
>         /* Call the user's function.  */
>         jal             t9
> --
> 2.34.1
>
Adhemerval Zanella Feb. 2, 2024, 1:28 p.m. UTC | #2
On 01/02/24 21:06, Khem Raj wrote:
> On Thu, Feb 1, 2024 at 9:41 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>> For o32 we need to setup a minimal stack frame to allow cprestore
>> on __thread_start_clone3 (which instruct the linker to save the
>> gp for PIC).  Also, there is no guarantee by kABI that $8 will be
>> preserved after syscall execution, so we need to save it on the
>> provided stack.
>>
>> Checked on mipsel-linux-gnu.
> 
> works ok on yocto reference mips ( Big-endian ) ( mips-linux-gnu )
> 
> Tested-by: Khem Raj <raj.khem@gmail.com>

Thanks, I will install it.
diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/mips/clone3.S b/sysdeps/unix/sysv/linux/mips/clone3.S
index e9fec2fa47..481b8ae963 100644
--- a/sysdeps/unix/sysv/linux/mips/clone3.S
+++ b/sysdeps/unix/sysv/linux/mips/clone3.S
@@ -37,11 +37,6 @@ 
 
 	.text
 	.set		nomips16
-#if _MIPS_SIM == _ABIO32
-# define EXTRA_LOCALS 1
-#else
-# define EXTRA_LOCALS 0
-#endif
 #define FRAMESZ ((NARGSAVE*SZREG)+ALSZ)&ALMASK
 GPOFF= FRAMESZ-(1*SZREG)
 NESTED(__clone3, SZREG, sp)
@@ -68,8 +63,31 @@  NESTED(__clone3, SZREG, sp)
 	beqz	a0, L(error)	/* No NULL cl_args pointer.  */
 	beqz	a2, L(error)	/* No NULL function pointer.  */
 
+#if _MIPS_SIM == _ABIO32
+	/* Both stack and stack_size on clone_args are defined as uint64_t, and
+	   there is no need to handle values larger than to 32 bits for o32.  */
+# if __BYTE_ORDER == __BIG_ENDIAN
+#  define CL_STACKPOINTER_OFFSET  44
+#  define CL_STACKSIZE_OFFSET     52
+# else
+#  define CL_STACKPOINTER_OFFSET  40
+#  define CL_STACKSIZE_OFFSET     48
+# endif
+
+	/* For o32 we need to setup a minimal stack frame to allow cprestore
+	   on __thread_start_clone3.  Also there is no guarantee by kABI that
+	   $8 will be preserved after syscall execution (so we need to save it
+	   on the provided stack).  */
+	lw	t0, CL_STACKPOINTER_OFFSET(a0)	/* Load the stack pointer.  */
+	lw	t1, CL_STACKSIZE_OFFSET(a0)	/* Load the stack_size.  */
+	addiu	t1, -32				/* Update the stack size.  */
+	addu	t2, t1, t0			/* Calculate the thread stack.  */
+	sw	a3, 0(t2)			/* Save argument pointer.  */
+	sw	t1, CL_STACKSIZE_OFFSET(a0)	/* Save the new stack size.  */
+#else
 	move	$8, a3		/* a3 is set to 0/1 for syscall success/error
 				   while a4/$8 is returned unmodified.  */
+#endif
 
 	/* Do the system call, the kernel expects:
 	   v0: system call number
@@ -125,7 +143,11 @@  L(thread_start_clone3):
 
 	/* Restore the arg for user's function.  */
 	move		t9, a2		/* Function pointer.  */
+#if _MIPS_SIM == _ABIO32
+	PTR_L		a0, 0(sp)
+#else
 	move		a0, $8		/* Argument pointer.  */
+#endif
 
 	/* Call the user's function.  */
 	jal		t9