Module Name:    src
Committed By:   riastradh
Date:           Thu Apr  4 00:46:30 UTC 2024

Modified Files:
        src/lib/libc/arch/x86_64/gen: __setjmp14.S __sigsetjmp14.S
        src/tests/lib/libc/setjmp: t_sigstack.c

Log Message:
amd64 longjmp: Restore stack first, then signal mask.

Otherwise, a pending signal may be delivered on the wrong stack when
we restore the signal mask.

While here:

- Tidy the code a little bit.
- Sprinkle comments to explain what's going on.
- Use `xorl %eXX,%eXX' instead of `xorq %rXX,%rXX'.
  => Same effect, one byte shorter, breaks dep chain on more uarches.
- Use forward branches for statically predicted not-taken.
  => val==0 is unlikely in longjmp

PR lib/57946


To generate a diff of this commit:
cvs rdiff -u -r1.3 -r1.4 src/lib/libc/arch/x86_64/gen/__setjmp14.S \
    src/lib/libc/arch/x86_64/gen/__sigsetjmp14.S
cvs rdiff -u -r1.7 -r1.8 src/tests/lib/libc/setjmp/t_sigstack.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/lib/libc/arch/x86_64/gen/__setjmp14.S
diff -u src/lib/libc/arch/x86_64/gen/__setjmp14.S:1.3 src/lib/libc/arch/x86_64/gen/__setjmp14.S:1.4
--- src/lib/libc/arch/x86_64/gen/__setjmp14.S:1.3	Thu May 22 15:01:56 2014
+++ src/lib/libc/arch/x86_64/gen/__setjmp14.S	Thu Apr  4 00:46:30 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: __setjmp14.S,v 1.3 2014/05/22 15:01:56 uebayasi Exp $	*/
+/*	$NetBSD: __setjmp14.S,v 1.4 2024/04/04 00:46:30 riastradh Exp $	*/
 
 /*
  * Copyright (c) 2001 Wasabi Systems, Inc.
@@ -40,7 +40,7 @@
 #include <machine/setjmp.h>
 
 #if defined(LIBC_SCCS)
-	RCSID("$NetBSD: __setjmp14.S,v 1.3 2014/05/22 15:01:56 uebayasi Exp $")
+	RCSID("$NetBSD: __setjmp14.S,v 1.4 2024/04/04 00:46:30 riastradh Exp $")
 #endif
 
 /*
@@ -53,7 +53,19 @@
  * The previous signal state is restored.
  */
 
+/*
+ * setjmp(jmp_buf env@rdi)
+ *
+ * ELF symbol: __setjmp14, because the size of jmp_buf changed on some
+ * platforms in 1.4.
+ */
 ENTRY(__setjmp14)
+	/*
+	 * Save the callee-saves registers: %rbx, %rbp, %r12-r15,
+	 * plus %rsp and the return address on the stack since it
+	 * will be overwritten if the caller makes any subroutine
+	 * calls before longjmp.
+	 */
 	movq	(%rsp),%r11
 	movq	%rbx,(_JB_RBX * 8)(%rdi)
 	movq	%rbp,(_JB_RBP * 8)(%rdi)
@@ -64,47 +76,62 @@ ENTRY(__setjmp14)
 	movq	%rsp,(_JB_RSP * 8)(%rdi)
 	movq	%r11,(_JB_PC  * 8)(%rdi)
 
-	leaq	(_JB_SIGMASK * 8)(%rdi),%rdx
-	xorl	%edi,%edi
-	xorq	%rsi,%rsi
+	leaq	(_JB_SIGMASK * 8)(%rdi),%rdx	/* oset@rdx */
+	xorl	%edi,%edi		/* how@edi := 0 (ignored) */
+	xorl	%esi,%esi		/* set@rsi := NULL */
 
 #ifdef __PIC__
 	call	PIC_PLT(_C_LABEL(__sigprocmask14))
 #else
 	call	_C_LABEL(__sigprocmask14)
 #endif
-2:	xorl	%eax,%eax
+	xorl	%eax,%eax
 	ret
 END(__setjmp14)
 
+/*
+ * longjmp(jmp_buf env@rdi, int val@esi)
+ *
+ * ELF symbol: __longjmp14, because the size of jmp_buf changed on some
+ * platforms in 1.4
+ */
 ENTRY(__longjmp14)
-	movq	%rdi,%r12
-	movl	%esi,%r8d
-
-	leaq	(_JB_SIGMASK * 8)(%rdi),%rsi
-	movl	$3,%edi		/* SIG_SETMASK */
-	xorq	%rdx,%rdx
+	/*
+	 * Restore the callee-saves registers: %rbx, %rbp, %r12-r15,
+	 * plus %rsp and the return address on the stack.
+	 */
+	movq	(_JB_RBX * 8)(%rdi),%rbx
+	movq	(_JB_RBP * 8)(%rdi),%rbp
+	movq	(_JB_R12 * 8)(%rdi),%r12
+	movq	(_JB_R13 * 8)(%rdi),%r13
+	movq	(_JB_R14 * 8)(%rdi),%r14
+	movq	(_JB_R15 * 8)(%rdi),%r15
+	movq	(_JB_RSP * 8)(%rdi),%rsp
+	movq	(_JB_PC  * 8)(%rdi),%r11
+	movq	%r11,0(%rsp)
+
+	/*
+	 * Use  pushq %rsi  instead of  pushl %esi  in order to keep
+	 * 16-byte stack alignment, even though we only care about the
+	 * 32-bit int in esi.
+	 */
+	pushq	%rsi		/* save val@esi */
+
+	leaq	(_JB_SIGMASK * 8)(%rdi),%rsi	/* set@rsi */
+	movl	$3,%edi		/* how@edi := SIG_SETMASK */
+	xorl	%edx,%edx	/* oset@rdx := NULL */
 
-	pushq	%r8
 #ifdef __PIC__
 	call	PIC_PLT(_C_LABEL(__sigprocmask14))
 #else
 	call	_C_LABEL(__sigprocmask14)
 #endif
-	popq	%r8
-	movq	(_JB_RBX * 8)(%r12),%rbx
-	movq	(_JB_RBP * 8)(%r12),%rbp
-	movq	(_JB_R13 * 8)(%r12),%r13
-	movq	(_JB_R14 * 8)(%r12),%r14
-	movq	(_JB_R15 * 8)(%r12),%r15
-	movq	(_JB_RSP * 8)(%r12),%rsp
-	movq	(_JB_PC  * 8)(%r12),%r11
-	movq	(_JB_R12 * 8)(%r12),%r12
-
-	movl	%r8d,%eax
-	testl	%eax,%eax
-	jnz	1f
-	incl	%eax
-1:	movq	%r11,0(%rsp)
-	ret
+
+	popq	%rax		/* restore val@eax */
+
+	testl	%eax,%eax	/* val@eax == 0? */
+	jz	1f		/* jump if val@eax == 0 */
+	ret			/* return val@eax */
+1:	incl	%eax		/* val@eax := 1 */
+	ret			/* return val@eax */
 END(__longjmp14)
Index: src/lib/libc/arch/x86_64/gen/__sigsetjmp14.S
diff -u src/lib/libc/arch/x86_64/gen/__sigsetjmp14.S:1.3 src/lib/libc/arch/x86_64/gen/__sigsetjmp14.S:1.4
--- src/lib/libc/arch/x86_64/gen/__sigsetjmp14.S:1.3	Thu May 22 15:01:56 2014
+++ src/lib/libc/arch/x86_64/gen/__sigsetjmp14.S	Thu Apr  4 00:46:30 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: __sigsetjmp14.S,v 1.3 2014/05/22 15:01:56 uebayasi Exp $	*/
+/*	$NetBSD: __sigsetjmp14.S,v 1.4 2024/04/04 00:46:30 riastradh Exp $	*/
 
 /*
  * Copyright (c) 2001 Wasabi Systems, Inc.
@@ -40,7 +40,7 @@
 #include <machine/setjmp.h>
 
 #if defined(LIBC_SCCS)
-	RCSID("$NetBSD: __sigsetjmp14.S,v 1.3 2014/05/22 15:01:56 uebayasi Exp $")
+	RCSID("$NetBSD: __sigsetjmp14.S,v 1.4 2024/04/04 00:46:30 riastradh Exp $")
 #endif
 
 /*
@@ -53,7 +53,19 @@
  * The previous signal state is restored.
  */
 
+/*
+ * sigsetjmp(sigjmp_buf env@rdi, int savemask@esi)
+ *
+ * ELF symbol: __sigsetjmp14, because the size of sigjmp_buf changed on
+ * some platforms in 1.4.
+ */
 ENTRY(__sigsetjmp14)
+	/*
+	 * Save the callee-saves registers: %rbx, %rbp, %r12-r15,
+	 * plus %rsp and the return address on the stack since it
+	 * will be overwritten if the caller makes any subroutine
+	 * calls before siglongjmp.
+	 */
 	movq	(%rsp),%r11
 	movq	%rbx,(_JB_RBX * 8)(%rdi)
 	movq	%rbp,(_JB_RBP * 8)(%rdi)
@@ -64,51 +76,69 @@ ENTRY(__sigsetjmp14)
 	movq	%rsp,(_JB_RSP * 8)(%rdi)
 	movq	%r11,(_JB_PC  * 8)(%rdi)
 
-	movq	%rsi,(_JB_SIGFLAG  * 8)(%rdi)
-	testl	%esi,%esi
-	jz	2f
-
-	leaq	(_JB_SIGMASK * 8)(%rdi),%rdx
-	xorl	%edi,%edi
-	xorq	%rsi,%rsi
+	movq	%rsi,(_JB_SIGFLAG  * 8)(%rdi)	/* store savemask */
+	testl	%esi,%esi		/* savemask == 0? */
+	jz	2f			/* jump if savemask == 0 */
+
+	leaq	(_JB_SIGMASK * 8)(%rdi),%rdx	/* oset@rdx */
+	xorl	%edi,%edi		/* how@edi := 0 (ignored) */
+	xorl	%esi,%esi		/* set@rsi := NULL */
 
 #ifdef __PIC__
 	call	PIC_PLT(_C_LABEL(__sigprocmask14))
 #else
 	call	_C_LABEL(__sigprocmask14)
 #endif
-2:	xorl	%eax,%eax
+2:	xorl	%eax,%eax		/* return 0 first time around */
 	ret
 END(__sigsetjmp14)
 
+/*
+ * siglongjmp(sigjmp_buf env@rdi, int val@esi)
+ *
+ * ELF symbol: __siglongjmp14, because the size of sigjmp_buf changed
+ * on some platforms in 1.4.
+ */
 ENTRY(__siglongjmp14)
-	movq	%rdi,%r12
-	pushq	%rsi
-	cmpl	$0, (_JB_SIGFLAG * 8)(%rdi)
-
-	jz	2f
-	leaq	(_JB_SIGMASK * 8)(%rdi),%rsi
-	movl	$3,%edi		/* SIG_SETMASK */
-	xorq	%rdx,%rdx
+	/*
+	 * Restore the callee-saves registers: %rbx, %rbp, %r12-r15,
+	 * plus %rsp and the return address on the stack.
+	 */
+	movq	(_JB_RBX * 8)(%rdi),%rbx
+	movq	(_JB_RBP * 8)(%rdi),%rbp
+	movq	(_JB_R12 * 8)(%rdi),%r12
+	movq	(_JB_R13 * 8)(%rdi),%r13
+	movq	(_JB_R14 * 8)(%rdi),%r14
+	movq	(_JB_R15 * 8)(%rdi),%r15
+	movq	(_JB_RSP * 8)(%rdi),%rsp
+	movq	(_JB_PC  * 8)(%rdi),%r11
+	movq	%r11,0(%rsp)
+
+	/*
+	 * Use  pushq %rsi  instead of  pushl %esi  in order to keep
+	 * 16-byte stack alignment, even though we only care about the
+	 * 32-bit int in esi.
+	 */
+	pushq	%rsi		/* save val@esi */
+
+	cmpl	$0, (_JB_SIGFLAG * 8)(%rdi)	/* test savemask == 0? */
+	jz	2f		/* jump if savemask == 0 */
+
+	leaq	(_JB_SIGMASK * 8)(%rdi),%rsi	/* set@rsi */
+	movl	$3,%edi		/* how@edi := SIG_SETMASK */
+	xorl	%edx,%edx	/* oset@rdx := NULL */
 
 #ifdef __PIC__
 	call	PIC_PLT(_C_LABEL(__sigprocmask14))
 #else
 	call	_C_LABEL(__sigprocmask14)
 #endif
-2:	popq	%rax
-	movq	(_JB_RBX * 8)(%r12),%rbx
-	movq	(_JB_RBP * 8)(%r12),%rbp
-	movq	(_JB_R13 * 8)(%r12),%r13
-	movq	(_JB_R14 * 8)(%r12),%r14
-	movq	(_JB_R15 * 8)(%r12),%r15
-	movq	(_JB_RSP * 8)(%r12),%rsp
-	movq	(_JB_PC  * 8)(%r12),%r11
-	movq	(_JB_R12 * 8)(%r12),%r12
-
-	testl	%eax,%eax
-	jnz	1f
-	incl	%eax
-1:	movq	%r11,0(%rsp)
-	ret
+
+2:	popq	%rax		/* restore val@eax */
+
+	testl	%eax,%eax	/* val@eax == 0? */
+	jz	1f		/* jump if val@eax == 0 */
+	ret			/* return val@eax */
+1:	incl	%eax		/* val@eax := 1 */
+	ret			/* return val@eax */
 END(__siglongjmp14)

Index: src/tests/lib/libc/setjmp/t_sigstack.c
diff -u src/tests/lib/libc/setjmp/t_sigstack.c:1.7 src/tests/lib/libc/setjmp/t_sigstack.c:1.8
--- src/tests/lib/libc/setjmp/t_sigstack.c:1.7	Mon Feb 19 19:43:27 2024
+++ src/tests/lib/libc/setjmp/t_sigstack.c	Thu Apr  4 00:46:30 2024
@@ -1,4 +1,4 @@
-/*	$NetBSD: t_sigstack.c,v 1.7 2024/02/19 19:43:27 riastradh Exp $	*/
+/*	$NetBSD: t_sigstack.c,v 1.8 2024/04/04 00:46:30 riastradh Exp $	*/
 
 /*-
  * Copyright (c) 2024 The NetBSD Foundation, Inc.
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__RCSID("$NetBSD: t_sigstack.c,v 1.7 2024/02/19 19:43:27 riastradh Exp $");
+__RCSID("$NetBSD: t_sigstack.c,v 1.8 2024/04/04 00:46:30 riastradh Exp $");
 
 #include <setjmp.h>
 #include <signal.h>
@@ -81,11 +81,12 @@ on_sigusr1(int signo, siginfo_t *si, voi
 	 * On some architectures, this is broken.  Those that appear to
 	 * get this right are:
 	 *
-	 *	aarch64, alpha, m68k, or1k, powerpc, powerpc64, riscv, vax
+	 *	aarch64, alpha, m68k, or1k, powerpc, powerpc64, riscv,
+	 *	vax, x86_64
 	 */
 #if defined __arm__ || defined __hppa__ || defined __i386__ || \
     defined __ia64__ || defined __mips__ || defined __sh3__ || \
-    defined __sparc__ || defined __sparc64__ || defined __x86_64__
+    defined __sparc__ || defined __sparc64__
 	if (nentries > 0)
 		atf_tc_expect_fail("PR lib/57946");
 #endif

Reply via email to