Re: [PATCH] sh: lib: Restore r4 in shift helpers

[John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>]

Hi Florian,

On Tue, 2026-07-14 at 12:41 +0200, Florian Fuchs wrote:
> Commit 940d4113f330 ("sh: New gcc support") added new shift helpers
> that use r4 as a scratch register while dispatching to the selected shift
> sequence. But, the helpers return without restoring r4. With GCC 17, the
> register allocator can keep a live value in r4 across the helper call.
> Clobbering it results in runtime data corruption. Restore r4 before
> jumping to the selected shift sequence.
> 
> Fixes: 940d4113f330 ("sh: New gcc support")
> Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
> ---
> Without the patch, the early boot on e.g J2 gets a kernel BUG at
> mm/percpu.c:2604 / "can't handle more than one group."
>         PCPU_SETUP_BUG_ON(pcpu_verify_alloc_info(ai) < 0);
> As the static condition in mm/percpu-km.c wasn't true:
>         ai->nr_groups != 1
> nr_groups contained 60 - the clobbered value from the shift helper.
> 
> This change was tested on the J2 core on the Mimas v2 board. It can
> theoretically also target other SH2 devices, but I don't have any other
> than J2 sadly.
> 
> The flow of operations matches now the state in the libgcc, see also
> in gcc: libgcc/config/sh/lib1funcs.S
> ---
>  arch/sh/lib/ashlsi3.S | 3 ++-
>  arch/sh/lib/ashrsi3.S | 3 ++-
>  arch/sh/lib/lshrsi3.S | 3 ++-
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/sh/lib/ashlsi3.S b/arch/sh/lib/ashlsi3.S
> index 4df4401cdf31..73a9d709b169 100644
> --- a/arch/sh/lib/ashlsi3.S
> +++ b/arch/sh/lib/ashlsi3.S
> @@ -63,8 +63,9 @@ __ashlsi3_r0:
>  	mova	ashlsi3_table,r0
>  	mov.b	@(r0,r4),r4
>  	add	r4,r0
> +	mov.l	@r15+,r4
>  	jmp	@r0
> -	mov.l	@r15+,r0
> +	mov	r4,r0
>  
>  	.align	2
>  ashlsi3_table:
> diff --git a/arch/sh/lib/ashrsi3.S b/arch/sh/lib/ashrsi3.S
> index bf3c4e03e6ff..9962d7c587df 100644
> --- a/arch/sh/lib/ashrsi3.S
> +++ b/arch/sh/lib/ashrsi3.S
> @@ -62,8 +62,9 @@ __ashrsi3_r0:
>  	mova	ashrsi3_table,r0
>  	mov.b	@(r0,r4),r4
>  	add	r4,r0
> +	mov.l	@r15+,r4
>  	jmp	@r0
> -	mov.l	@r15+,r0
> +	mov	r4,r0
>  
>  	.align	2
>  ashrsi3_table:
> diff --git a/arch/sh/lib/lshrsi3.S b/arch/sh/lib/lshrsi3.S
> index b79b8170061f..9218f0bad1cc 100644
> --- a/arch/sh/lib/lshrsi3.S
> +++ b/arch/sh/lib/lshrsi3.S
> @@ -62,8 +62,9 @@ __lshrsi3_r0:
>  	mova	lshrsi3_table,r0
>  	mov.b	@(r0,r4),r4
>  	add	r4,r0
> +	mov.l	@r15+,r4
>  	jmp	@r0
> -	mov.l	@r15+,r0
> +	mov	r4,r0
>  
>  	.align	2
>  lshrsi3_table:

The change looks reasonable to me, but I'm not 100% sure I understand the
code in detail at the moment, so I'll have to take another look tomorrow
when I'm more awake and had a coffee.

Given the fact that this unbreaks J2, we should pick it up for v7.4 once
I have fully understood the changes.

@Geert: Could you have another look at this and let me know what you think?

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913