[PATCH] sh4: ensure FPSCR.PR==0 when executing FRCHG [BZ #27543]

[Adhemerval Zanella Netto]

On 03/01/25 16:23, mirabilos wrote:
> If the bit is not 0, the operations FRCHG and FSCHG are
> undefined and cause a trap; qemu now checks for this as
> well, so we set it to 0 temporarily and restore the old
> value in getcontext afterwards (setcontext/swapcontext
> already do so).
> 
> From the discussion in the bugreport, this can probably
> be optimised in one place but none of the people involved
> are SH4 assembly experts, this patch is field-tested, and
> it’s not a code path run often. The other question, what
> happens if a signal occurs while the bit is temporarily 0,
> is also still unsolved, but to fix that a kernel change is
> most likely needed; this patch changes a certain trap on
> many CPUs for a hard-to-get trap in a signal handler if a
> signal is delivered during the few instructions the PR bit
> is temporarily set to 0, so it’s not a regression for most
> users.
> 
> See BZ and https://bugs.launchpad.net/qemu/+bug/1796520 for
> related discussion, references and review comments.
> 
> Signed-off-by: mirabilos <tg@debian.org>
> Reviewed-by: Oleg Endo <olegendo@gcc.gnu.org>
> Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> ---
>  sysdeps/unix/sysv/linux/sh/sh4/getcontext.S  | 6 ++++++
>  sysdeps/unix/sysv/linux/sh/sh4/setcontext.S  | 2 ++
>  sysdeps/unix/sysv/linux/sh/sh4/swapcontext.S | 2 ++
>  3 files changed, 10 insertions(+)

The patch does not apply from patchwork, but I could apply it manually.

Besides it, LGTM although I don't have a working hardware to actually
test it (and qemu-sh4-static shows a failure, setjmp/tst-setjmp-fp). 

> From b9015c5a51bfbf3b7dc59f52620a8d5ca307c54b Mon Sep 17 00:00:00 2001
> From: mirabilos <tg@debian.org>
> Date: Fri, 3 Jan 2025 18:53:21 +0000
> Subject: [PATCH] sh4: ensure FPSCR.PR==0 when executing FRCHG [BZ #27543]
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> To: libc-alpha@sourceware.org
> Cc: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>,
>     John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> 
> If the bit is not 0, the operations FRCHG and FSCHG are
> undefined and cause a trap; qemu now checks for this as
> well, so we set it to 0 temporarily and restore the old
> value in getcontext afterwards (setcontext/swapcontext
> already do so).
> 
> From the discussion in the bugreport, this can probably
> be optimised in one place but none of the people involved
> are SH4 assembly experts, this patch is field-tested, and
> it’s not a code path run often. The other question, what
> happens if a signal occurs while the bit is temporarily 0,
> is also still unsolved, but to fix that a kernel change is
> most likely needed; this patch changes a certain trap on
> many CPUs for a hard-to-get trap in a signal handler if a
> signal is delivered during the few instructions the PR bit
> is temporarily set to 0, so it’s not a regression for most
> users.
> 
> See BZ and https://bugs.launchpad.net/qemu/+bug/1796520 for
> related discussion, references and review comments.
> 
> Signed-off-by: mirabilos <tg@debian.org>
> Reviewed-by: Oleg Endo <olegendo@gcc.gnu.org>
> Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> ---
>  sysdeps/unix/sysv/linux/sh/sh4/getcontext.S  | 6 ++++++
>  sysdeps/unix/sysv/linux/sh/sh4/setcontext.S  | 2 ++
>  sysdeps/unix/sysv/linux/sh/sh4/swapcontext.S | 2 ++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/sh/sh4/getcontext.S b/sysdeps/unix/sysv/linux/sh/sh4/getcontext.S
> index 4470e5730b..329a790cd6 100644
> --- a/sysdeps/unix/sysv/linux/sh/sh4/getcontext.S
> +++ b/sysdeps/unix/sysv/linux/sh/sh4/getcontext.S
> @@ -67,6 +67,8 @@ ENTRY(__getcontext)
>  	add	#(oFPUL+4-124),r0
>  	sts.l	fpul, @-r0
>  	sts.l	fpscr, @-r0
> +	mov	#0, r6
> +	lds	r6, fpscr
>  	frchg
>  	fmov.s	fr15, @-r0
>  	fmov.s	fr14, @-r0
> @@ -101,6 +103,10 @@ ENTRY(__getcontext)
>  	fmov.s	fr2, @-r0
>  	fmov.s	fr1, @-r0
>  	fmov.s	fr0, @-r0
> +	mov	r4, r0
> +	add	#124, r0
> +	add	#(oFPSCR-124), r0
> +	lds.l	@r0+, fpscr
>  #endif /* __SH_FPU_ANY__ */
>  
>  	/* sigprocmask (SIG_BLOCK, NULL, &uc->uc_sigmask).  */
> diff --git a/sysdeps/unix/sysv/linux/sh/sh4/setcontext.S b/sysdeps/unix/sysv/linux/sh/sh4/setcontext.S
> index a6d1de960c..60aff78256 100644
> --- a/sysdeps/unix/sysv/linux/sh/sh4/setcontext.S
> +++ b/sysdeps/unix/sysv/linux/sh/sh4/setcontext.S
> @@ -50,6 +50,8 @@ ENTRY(__setcontext)
>  
>  .Lsetcontext_restore:
>  #ifdef __SH_FPU_ANY__
> +	mov	#0, r9
> +	lds	r9, fpscr
>  	mov	r8, r0
>  	add	#(oFR0),r0
>  	fmov.s	@r0+, fr0
> diff --git a/sysdeps/unix/sysv/linux/sh/sh4/swapcontext.S b/sysdeps/unix/sysv/linux/sh/sh4/swapcontext.S
> index a299e05b41..6cf88f2b68 100644
> --- a/sysdeps/unix/sysv/linux/sh/sh4/swapcontext.S
> +++ b/sysdeps/unix/sysv/linux/sh/sh4/swapcontext.S
> @@ -67,6 +67,8 @@ ENTRY(__swapcontext)
>  	add	#(oFPUL+4-124),r0
>  	sts.l	fpul, @-r0
>  	sts.l	fpscr, @-r0
> +	mov	#0, r9
> +	lds	r9, fpscr
>  	frchg
>  	fmov.s	fr15, @-r0
>  	fmov.s	fr14, @-r0
> -- 
> 2.30.2