git: 9fb118bebced - main - libc: Fix longjmp/_longjmp(buf, 0) for AArch64 and RISC-V

Reply: Jessica Clarke : "Re: git: 9fb118bebced - main - libc: Fix longjmp/_longjmp(buf, 0) for AArch64 and RISC-V"
Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Jessica Clarke <jrtc27_at_FreeBSD.org>
Date: Mon, 09 Jan 2023 18:35:21 UTC
The branch main has been updated by jrtc27:

URL: https://cgit.FreeBSD.org/src/commit/?id=9fb118bebced1452a46756a13be0161021b10905

commit 9fb118bebced1452a46756a13be0161021b10905
Author:     Jessica Clarke <jrtc27@FreeBSD.org>
AuthorDate: 2023-01-09 18:34:43 +0000
Commit:     Jessica Clarke <jrtc27@FreeBSD.org>
CommitDate: 2023-01-09 18:34:43 +0000

    libc: Fix longjmp/_longjmp(buf, 0) for AArch64 and RISC-V
    
    These architectures fail to handle this special case, and will cause the
    corresponding setjmp/_setjmp to return 0 rather than 1. Fix this and add
    regression tests (also committed upstream).
    
    PR:             268684
    Reviewed by:    arichardson, jhb
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D29363
---
 contrib/netbsd-tests/lib/libc/setjmp/t_setjmp.c | 50 ++++++++++++++++++++++---
 lib/libc/aarch64/gen/_setjmp.S                  |  3 +-
 lib/libc/aarch64/gen/setjmp.S                   |  3 +-
 lib/libc/riscv/gen/_setjmp.S                    |  3 ++
 lib/libc/riscv/gen/setjmp.S                     |  3 ++
 5 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/contrib/netbsd-tests/lib/libc/setjmp/t_setjmp.c b/contrib/netbsd-tests/lib/libc/setjmp/t_setjmp.c
index 1f0f1ed5ea89..1b1baa584468 100644
--- a/contrib/netbsd-tests/lib/libc/setjmp/t_setjmp.c
+++ b/contrib/netbsd-tests/lib/libc/setjmp/t_setjmp.c
@@ -70,6 +70,7 @@ __RCSID("$NetBSD: t_setjmp.c,v 1.2 2017/01/14 21:08:17 christos Exp $");
 #include <errno.h>
 #include <setjmp.h>
 #include <signal.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -83,6 +84,8 @@ __RCSID("$NetBSD: t_setjmp.c,v 1.2 2017/01/14 21:08:17 christos Exp $");
 #define TEST_U_SETJMP 1
 #define TEST_SIGSETJMP_SAVE 2
 #define TEST_SIGSETJMP_NOSAVE 3
+#define TEST_LONGJMP_ZERO 4
+#define TEST_U_LONGJMP_ZERO 5
 
 static int expectsignal;
 
@@ -101,12 +104,16 @@ h_check(int test)
 	sigjmp_buf sjb;
 	sigset_t ss;
 	int i, x;
+	volatile bool did_longjmp;
 
 	i = getpid();
+	did_longjmp = false;
 
-	if (test == TEST_SETJMP || test == TEST_SIGSETJMP_SAVE)
+	if (test == TEST_SETJMP || test == TEST_SIGSETJMP_SAVE ||
+	    test == TEST_LONGJMP_ZERO)
 		expectsignal = 0;
-	else if (test == TEST_U_SETJMP || test == TEST_SIGSETJMP_NOSAVE)
+	else if (test == TEST_U_SETJMP || test == TEST_SIGSETJMP_NOSAVE ||
+	    test == TEST_U_LONGJMP_ZERO)
 		expectsignal = 1;
 	else
 		atf_tc_fail("unknown test");
@@ -119,26 +126,37 @@ h_check(int test)
 	REQUIRE_ERRNO(sigaddset(&ss, SIGABRT) != -1);
 	REQUIRE_ERRNO(sigprocmask(SIG_BLOCK, &ss, NULL) != -1);
 
-	if (test == TEST_SETJMP)
+	if (test == TEST_SETJMP || test == TEST_LONGJMP_ZERO)
 		x = setjmp(jb);
-	else if (test == TEST_U_SETJMP)
+	else if (test == TEST_U_SETJMP || test == TEST_U_LONGJMP_ZERO)
 		x = _setjmp(jb);
 	else 
 		x = sigsetjmp(sjb, !expectsignal);
 
 	if (x != 0) {
-		ATF_REQUIRE_MSG(x == i, "setjmp returned wrong value");
+		if (test == TEST_LONGJMP_ZERO || test == TEST_U_LONGJMP_ZERO)
+			ATF_REQUIRE_MSG(x == 1, "setjmp returned wrong value");
+		else
+			ATF_REQUIRE_MSG(x == i, "setjmp returned wrong value");
+
 		kill(i, SIGABRT);
 		ATF_REQUIRE_MSG(!expectsignal, "kill(SIGABRT) failed");
 		atf_tc_pass();
+	} else if (did_longjmp) {
+		atf_tc_fail("setjmp returned zero after longjmp");
 	}
 
 	REQUIRE_ERRNO(sigprocmask(SIG_UNBLOCK, &ss, NULL) != -1);
 
+	did_longjmp = true;
 	if (test == TEST_SETJMP)
 		longjmp(jb, i);
+	else if (test == TEST_LONGJMP_ZERO)
+		longjmp(jb, 0);
 	else if (test == TEST_U_SETJMP)
 		_longjmp(jb, i);
+	else if (test == TEST_U_LONGJMP_ZERO)
+		_longjmp(jb, 0);
 	else 
 		siglongjmp(sjb, i);
 
@@ -185,12 +203,34 @@ ATF_TC_BODY(sigsetjmp_nosave, tc)
 	h_check(TEST_SIGSETJMP_NOSAVE);
 }
 
+ATF_TC(longjmp_zero);
+ATF_TC_HEAD(longjmp_zero, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Checks longjmp(3) with a zero value");
+}
+ATF_TC_BODY(longjmp_zero, tc)
+{
+	h_check(TEST_LONGJMP_ZERO);
+}
+
+ATF_TC(_longjmp_zero);
+ATF_TC_HEAD(_longjmp_zero, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Checks _longjmp(3) with a zero value");
+}
+ATF_TC_BODY(_longjmp_zero, tc)
+{
+	h_check(TEST_U_LONGJMP_ZERO);
+}
+
 ATF_TP_ADD_TCS(tp)
 {
 	ATF_TP_ADD_TC(tp, setjmp);
 	ATF_TP_ADD_TC(tp, _setjmp);
 	ATF_TP_ADD_TC(tp, sigsetjmp_save);
 	ATF_TP_ADD_TC(tp, sigsetjmp_nosave);
+	ATF_TP_ADD_TC(tp, longjmp_zero);
+	ATF_TP_ADD_TC(tp, _longjmp_zero);
 
 	return atf_no_error();
 }
diff --git a/lib/libc/aarch64/gen/_setjmp.S b/lib/libc/aarch64/gen/_setjmp.S
index 49bf4df4f524..94a58d774f2b 100644
--- a/lib/libc/aarch64/gen/_setjmp.S
+++ b/lib/libc/aarch64/gen/_setjmp.S
@@ -91,7 +91,8 @@ ENTRY(_longjmp)
 #endif
 
 	/* Load the return value */
-	mov	x0, x1
+	cmp	x1, #0
+	csinc	x0, x1, xzr, ne
 	ret
 
 botch:
diff --git a/lib/libc/aarch64/gen/setjmp.S b/lib/libc/aarch64/gen/setjmp.S
index b302594ff549..e6cdba801e19 100644
--- a/lib/libc/aarch64/gen/setjmp.S
+++ b/lib/libc/aarch64/gen/setjmp.S
@@ -113,7 +113,8 @@ ENTRY(longjmp)
 	ldp	d14, d15, [x0]
 
 	/* Load the return value */
-	mov	x0, x1
+	cmp	x1, #0
+	csinc	x0, x1, xzr, ne
 	ret
 
 botch:
diff --git a/lib/libc/riscv/gen/_setjmp.S b/lib/libc/riscv/gen/_setjmp.S
index ded6705ef7ee..94b4e90b6f42 100644
--- a/lib/libc/riscv/gen/_setjmp.S
+++ b/lib/libc/riscv/gen/_setjmp.S
@@ -131,6 +131,9 @@ ENTRY(_longjmp)
 
 	/* Load the return value */
 	mv	a0, a1
+	bnez	a1, 1f
+	li	a0, 1
+1:
 	ret
 
 botch:
diff --git a/lib/libc/riscv/gen/setjmp.S b/lib/libc/riscv/gen/setjmp.S
index c0458e907ce0..1d5b4d5fc0ca 100644
--- a/lib/libc/riscv/gen/setjmp.S
+++ b/lib/libc/riscv/gen/setjmp.S
@@ -161,6 +161,9 @@ ENTRY(longjmp)
 
 	/* Load the return value */
 	mv	a0, a1
+	bnez	a1, 1f
+	li	a0, 1
+1:
 	ret
 
 botch: