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

[yoshinori . sato]

On Thu, 17 Sep 2026 16:17:00 +0900,
John Paul Adrian Glaubitz wrote:
> 
> 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@xxxxxxxxx>
> > ---
> > 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

Here is the modified code snippet.

	and	#31,r0
	mov.l	r4,@-r15
	mov	r0,r4
	mova	ashlsi3_table,r0
	mov.b	@(r0,r4),r4
	add	r4,r0
	mov.l	@r15+,r4
	jmp	@r0
	mov	r4,r0

I understand that r4 no longer changes as a result of this modification.
While the sh function-calling convention permits the `r4` register to be
clobbered, the `libgcc` included with GCC does not actually clobber `r4`,
and GCC's own rules appear to rely on `r4` remaining intact during such calls.

This change won't break builds with older versions of GCC, so I think it's
fine to go ahead and apply it.

-- 
Yosinori Sato