Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 9 Jan 2023 18:45:33 +0000
From:      Jessica Clarke <jrtc27@freebsd.org>
To:        "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org>
Subject:   Re: git: 9fb118bebced - main - libc: Fix longjmp/_longjmp(buf, 0) for AArch64 and RISC-V
Message-ID:  <0B085CFE-D71D-486D-9367-885417F9A8C5@freebsd.org>
In-Reply-To: <202301091835.309IZLck039260@gitrepo.freebsd.org>
References:  <202301091835.309IZLck039260@gitrepo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 9 Jan 2023, at 18:35, Jessica Clarke <jrtc27@FreeBSD.org> wrote:
>=20
> The branch main has been updated by jrtc27:
>=20
> URL: =
https://cgit.FreeBSD.org/src/commit/?id=3D9fb118bebced1452a46756a13be01610=
21b10905
>=20
> 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
>=20
>    libc: Fix longjmp/_longjmp(buf, 0) for AArch64 and RISC-V
>=20
>    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).
>=20
>    PR:             268684

Also PR: 268521

Jess

>    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(-)
>=20
> 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
>=20
> static int expectsignal;
>=20
> @@ -101,12 +104,16 @@ h_check(int test)
> 	sigjmp_buf sjb;
> 	sigset_t ss;
> 	int i, x;
> +	volatile bool did_longjmp;
>=20
> 	i =3D getpid();
> +	did_longjmp =3D false;
>=20
> -	if (test =3D=3D TEST_SETJMP || test =3D=3D TEST_SIGSETJMP_SAVE)
> +	if (test =3D=3D TEST_SETJMP || test =3D=3D TEST_SIGSETJMP_SAVE =
||
> +	    test =3D=3D TEST_LONGJMP_ZERO)
> 		expectsignal =3D 0;
> -	else if (test =3D=3D TEST_U_SETJMP || test =3D=3D =
TEST_SIGSETJMP_NOSAVE)
> +	else if (test =3D=3D TEST_U_SETJMP || test =3D=3D =
TEST_SIGSETJMP_NOSAVE ||
> +	    test =3D=3D TEST_U_LONGJMP_ZERO)
> 		expectsignal =3D 1;
> 	else
> 		atf_tc_fail("unknown test");
> @@ -119,26 +126,37 @@ h_check(int test)
> 	REQUIRE_ERRNO(sigaddset(&ss, SIGABRT) !=3D -1);
> 	REQUIRE_ERRNO(sigprocmask(SIG_BLOCK, &ss, NULL) !=3D -1);
>=20
> -	if (test =3D=3D TEST_SETJMP)
> +	if (test =3D=3D TEST_SETJMP || test =3D=3D TEST_LONGJMP_ZERO)
> 		x =3D setjmp(jb);
> -	else if (test =3D=3D TEST_U_SETJMP)
> +	else if (test =3D=3D TEST_U_SETJMP || test =3D=3D =
TEST_U_LONGJMP_ZERO)
> 		x =3D _setjmp(jb);
> 	else=20
> 		x =3D sigsetjmp(sjb, !expectsignal);
>=20
> 	if (x !=3D 0) {
> -		ATF_REQUIRE_MSG(x =3D=3D i, "setjmp returned wrong =
value");
> +		if (test =3D=3D TEST_LONGJMP_ZERO || test =3D=3D =
TEST_U_LONGJMP_ZERO)
> +			ATF_REQUIRE_MSG(x =3D=3D 1, "setjmp returned =
wrong value");
> +		else
> +			ATF_REQUIRE_MSG(x =3D=3D 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");
> 	}
>=20
> 	REQUIRE_ERRNO(sigprocmask(SIG_UNBLOCK, &ss, NULL) !=3D -1);
>=20
> +	did_longjmp =3D true;
> 	if (test =3D=3D TEST_SETJMP)
> 		longjmp(jb, i);
> +	else if (test =3D=3D TEST_LONGJMP_ZERO)
> +		longjmp(jb, 0);
> 	else if (test =3D=3D TEST_U_SETJMP)
> 		_longjmp(jb, i);
> +	else if (test =3D=3D TEST_U_LONGJMP_ZERO)
> +		_longjmp(jb, 0);
> 	else=20
> 		siglongjmp(sjb, i);
>=20
> @@ -185,12 +203,34 @@ ATF_TC_BODY(sigsetjmp_nosave, tc)
> 	h_check(TEST_SIGSETJMP_NOSAVE);
> }
>=20
> +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);
>=20
> 	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
>=20
> 	/* Load the return value */
> -	mov	x0, x1
> +	cmp	x1, #0
> +	csinc	x0, x1, xzr, ne
> 	ret
>=20
> 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]
>=20
> 	/* Load the return value */
> -	mov	x0, x1
> +	cmp	x1, #0
> +	csinc	x0, x1, xzr, ne
> 	ret
>=20
> 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)
>=20
> 	/* Load the return value */
> 	mv	a0, a1
> +	bnez	a1, 1f
> +	li	a0, 1
> +1:
> 	ret
>=20
> 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)
>=20
> 	/* Load the return value */
> 	mv	a0, a1
> +	bnez	a1, 1f
> +	li	a0, 1
> +1:
> 	ret
>=20
> botch:




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?0B085CFE-D71D-486D-9367-885417F9A8C5>