diff mbox

V2 [PATCH 1/5] i386: Don't unnecessarily save and restore EAX, ECX and EDX [BZ# 25262]

Message ID 20200108161535.6141-2-hjl.tools@gmail.com
State Committed
Headers

Commit Message

H.J. Lu Jan. 8, 2020, 4:15 p.m. UTC 
  On i386, since EAX, ECX and EDX are caller-saved, there are no need
to save and restore EAX, ECX and EDX in getcontext, setcontext and
swapcontext.  They just need to clear EAX on success.  The extra
scratch registers are needed to enable CET.

Tested on i386.
---
 sysdeps/unix/sysv/linux/i386/getcontext.S  |  8 +-------
 sysdeps/unix/sysv/linux/i386/setcontext.S  | 11 ++++-------
 sysdeps/unix/sysv/linux/i386/swapcontext.S | 17 +++++------------
 3 files changed, 10 insertions(+), 26 deletions(-)

Comments

Adhemerval Zanella Jan. 9, 2020, 9:13 p.m. UTC | #1 
On 08/01/2020 13:15, H.J. Lu wrote:
> On i386, since EAX, ECX and EDX are caller-saved, there are no need
> to save and restore EAX, ECX and EDX in getcontext, setcontext and
> swapcontext.  They just need to clear EAX on success.  The extra
> scratch registers are needed to enable CET.
> 
> Tested on i386.

LGTM thanks. You might also remove the oE{A,C,D}X definitions from
Linux i386 ucontext_i.sym.

As a side note, Hurd might do the same on its implementations.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/unix/sysv/linux/i386/getcontext.S  |  8 +-------
>  sysdeps/unix/sysv/linux/i386/setcontext.S  | 11 ++++-------
>  sysdeps/unix/sysv/linux/i386/swapcontext.S | 17 +++++------------
>  3 files changed, 10 insertions(+), 26 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/i386/getcontext.S b/sysdeps/unix/sysv/linux/i386/getcontext.S
> index f86df4d555..9c1df9a2aa 100644
> --- a/sysdeps/unix/sysv/linux/i386/getcontext.S
> +++ b/sysdeps/unix/sysv/linux/i386/getcontext.S
> @@ -26,13 +26,7 @@ ENTRY(__getcontext)
>  	/* Load address of the context data structure.  */
>  	movl	4(%esp), %eax
>  
> -	/* Return value of getcontext.  EAX is the only register whose
> -	   value is not preserved.  */
> -	movl	$0, oEAX(%eax)
> -
> -	/* Save the 32-bit register values and the return address.  */
> -	movl	%ecx, oECX(%eax)
> -	movl	%edx, oEDX(%eax)
> +	/* Save the preserved register values and the return address.  */
>  	movl	%edi, oEDI(%eax)
>  	movl	%esi, oESI(%eax)
>  	movl	%ebp, oEBP(%eax)

Ok.

> diff --git a/sysdeps/unix/sysv/linux/i386/setcontext.S b/sysdeps/unix/sysv/linux/i386/setcontext.S
> index b4b5c0298c..f042d80bf4 100644
> --- a/sysdeps/unix/sysv/linux/i386/setcontext.S
> +++ b/sysdeps/unix/sysv/linux/i386/setcontext.S
> @@ -65,22 +65,19 @@ ENTRY(__setcontext)
>  	cfi_offset (esi, oESI)
>  	cfi_offset (ebp, oEBP)
>  	cfi_offset (ebx, oEBX)
> -	cfi_offset (edx, oEDX)
> -	cfi_offset (ecx, oECX)
>  	movl	oESP(%eax), %esp
>  
>  	/* Push the return address on the new stack so we can return there.  */
>  	pushl	%ecx
>  
> -	/* Load the values of all the 32-bit registers (except ESP).
> -	   Since we are loading from EAX, it must be last.  */
> +	/* Load the values of all the preserved registers (except ESP).  */
>  	movl	oEDI(%eax), %edi
>  	movl	oESI(%eax), %esi
>  	movl	oEBP(%eax), %ebp
>  	movl	oEBX(%eax), %ebx
> -	movl	oEDX(%eax), %edx
> -	movl	oECX(%eax), %ecx
> -	movl	oEAX(%eax), %eax
> +
> +	/* All done, return 0 for success.  */
> +	xorl	%eax, %eax
>  
>  	/* End FDE here, we fall into another context.  */
>  	cfi_endproc

Ok.

> diff --git a/sysdeps/unix/sysv/linux/i386/swapcontext.S b/sysdeps/unix/sysv/linux/i386/swapcontext.S
> index 792bfdf7e6..090c2d8c3e 100644
> --- a/sysdeps/unix/sysv/linux/i386/swapcontext.S
> +++ b/sysdeps/unix/sysv/linux/i386/swapcontext.S
> @@ -26,13 +26,7 @@ ENTRY(__swapcontext)
>  	/* Load address of the context data structure we save in.  */
>  	movl	4(%esp), %eax
>  
> -	/* Return value of swapcontext.  EAX is the only register whose
> -	   value is not preserved.  */
> -	movl	$0, oEAX(%eax)
> -
> -	/* Save the 32-bit register values and the return address.  */
> -	movl	%ecx, oECX(%eax)
> -	movl	%edx, oEDX(%eax)
> +	/* Save the preserved register values and the return address.  */
>  	movl	%edi, oEDI(%eax)
>  	movl	%esi, oESI(%eax)
>  	movl	%ebp, oEBP(%eax)
> @@ -91,15 +85,14 @@ ENTRY(__swapcontext)
>  	/* Push the return address on the new stack so we can return there.  */
>  	pushl	%ecx
>  
> -	/* Load the values of all the 32-bit registers (except ESP).
> -	   Since we are loading from EAX, it must be last.  */
> +	/* Load the values of all the preserved registers (except ESP).  */
>  	movl	oEDI(%eax), %edi
>  	movl	oESI(%eax), %esi
>  	movl	oEBP(%eax), %ebp
>  	movl	oEBX(%eax), %ebx
> -	movl	oEDX(%eax), %edx
> -	movl	oECX(%eax), %ecx
> -	movl	oEAX(%eax), %eax
> +
> +	/* All done, return 0 for success.  */
> +	xorl	%eax, %eax
>  
>  	/* The following 'ret' will pop the address of the code and jump
>  	   to it.  */
>
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/i386/getcontext.S b/sysdeps/unix/sysv/linux/i386/getcontext.S
index f86df4d555..9c1df9a2aa 100644
--- a/sysdeps/unix/sysv/linux/i386/getcontext.S
+++ b/sysdeps/unix/sysv/linux/i386/getcontext.S
@@ -26,13 +26,7 @@  ENTRY(__getcontext)
 	/* Load address of the context data structure.  */
 	movl	4(%esp), %eax
 
-	/* Return value of getcontext.  EAX is the only register whose
-	   value is not preserved.  */
-	movl	$0, oEAX(%eax)
-
-	/* Save the 32-bit register values and the return address.  */
-	movl	%ecx, oECX(%eax)
-	movl	%edx, oEDX(%eax)
+	/* Save the preserved register values and the return address.  */
 	movl	%edi, oEDI(%eax)
 	movl	%esi, oESI(%eax)
 	movl	%ebp, oEBP(%eax)
diff --git a/sysdeps/unix/sysv/linux/i386/setcontext.S b/sysdeps/unix/sysv/linux/i386/setcontext.S
index b4b5c0298c..f042d80bf4 100644
--- a/sysdeps/unix/sysv/linux/i386/setcontext.S
+++ b/sysdeps/unix/sysv/linux/i386/setcontext.S
@@ -65,22 +65,19 @@  ENTRY(__setcontext)
 	cfi_offset (esi, oESI)
 	cfi_offset (ebp, oEBP)
 	cfi_offset (ebx, oEBX)
-	cfi_offset (edx, oEDX)
-	cfi_offset (ecx, oECX)
 	movl	oESP(%eax), %esp
 
 	/* Push the return address on the new stack so we can return there.  */
 	pushl	%ecx
 
-	/* Load the values of all the 32-bit registers (except ESP).
-	   Since we are loading from EAX, it must be last.  */
+	/* Load the values of all the preserved registers (except ESP).  */
 	movl	oEDI(%eax), %edi
 	movl	oESI(%eax), %esi
 	movl	oEBP(%eax), %ebp
 	movl	oEBX(%eax), %ebx
-	movl	oEDX(%eax), %edx
-	movl	oECX(%eax), %ecx
-	movl	oEAX(%eax), %eax
+
+	/* All done, return 0 for success.  */
+	xorl	%eax, %eax
 
 	/* End FDE here, we fall into another context.  */
 	cfi_endproc
diff --git a/sysdeps/unix/sysv/linux/i386/swapcontext.S b/sysdeps/unix/sysv/linux/i386/swapcontext.S
index 792bfdf7e6..090c2d8c3e 100644
--- a/sysdeps/unix/sysv/linux/i386/swapcontext.S
+++ b/sysdeps/unix/sysv/linux/i386/swapcontext.S
@@ -26,13 +26,7 @@  ENTRY(__swapcontext)
 	/* Load address of the context data structure we save in.  */
 	movl	4(%esp), %eax
 
-	/* Return value of swapcontext.  EAX is the only register whose
-	   value is not preserved.  */
-	movl	$0, oEAX(%eax)
-
-	/* Save the 32-bit register values and the return address.  */
-	movl	%ecx, oECX(%eax)
-	movl	%edx, oEDX(%eax)
+	/* Save the preserved register values and the return address.  */
 	movl	%edi, oEDI(%eax)
 	movl	%esi, oESI(%eax)
 	movl	%ebp, oEBP(%eax)
@@ -91,15 +85,14 @@  ENTRY(__swapcontext)
 	/* Push the return address on the new stack so we can return there.  */
 	pushl	%ecx
 
-	/* Load the values of all the 32-bit registers (except ESP).
-	   Since we are loading from EAX, it must be last.  */
+	/* Load the values of all the preserved registers (except ESP).  */
 	movl	oEDI(%eax), %edi
 	movl	oESI(%eax), %esi
 	movl	oEBP(%eax), %ebp
 	movl	oEBX(%eax), %ebx
-	movl	oEDX(%eax), %edx
-	movl	oECX(%eax), %ecx
-	movl	oEAX(%eax), %eax
+
+	/* All done, return 0 for success.  */
+	xorl	%eax, %eax
 
 	/* The following 'ret' will pop the address of the code and jump
 	   to it.  */