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
> 

Please correct me if I'm wrong, but after reading the code in [1], it looks
to me as your patch doesn't preserve r4 but it's actually storing it into
r0 to be used by the selected shift sequence.

With the previous code, r4 is pushed onto the stack first but not restored
before the shift sequence is jumped to, so what your patch does not make sure
that r4 is restored across the complete call of the shift helper but rather
restore the input value in r4 from the stack before calling the jump sequence.

What am I missing?

Adrian

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/sh/lib/ashlsi3.S

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