Re: [PATCH] riscv: Stop considering R_RISCV_NONE as bad relocations

[Palmer Dabbelt]

On Fri, 11 Jul 2025 06:49:09 PDT (-0700), wangjingwei@xxxxxxxxxxx wrote:
> Hi all,
>
> On Thu, Jul 10, 2025 at 11:43:00AM -0700, Palmer Dabbelt wrote:
> > On Thu, 10 Jul 2025 01:34:31 PDT (-0700), alexghiti@xxxxxxxxxxxx wrote:
> > > Even though those relocations should not be present in the final
> > > vmlinux, there are a lot of them. And since those relocations are
> > > considered "bad", they flood the compilation output which may hide some
> > > legitimate bad relocations.
> > >
> > > Signed-off-by: Alexandre Ghiti <alexghiti@xxxxxxxxxxxx>
> > > ---
> > >  arch/riscv/tools/relocs_check.sh | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/riscv/tools/relocs_check.sh b/arch/riscv/tools/relocs_check.sh
> > > index baeb2e7b2290558d696afbc5429d6a3c69ae49e1..742993e6a8cba72c657dd2f8f5dabc4c415e84bd 100755
> > > --- a/arch/riscv/tools/relocs_check.sh
> > > +++ b/arch/riscv/tools/relocs_check.sh
> > > @@ -14,7 +14,9 @@ bad_relocs=$(
> > >  ${srctree}/scripts/relocs_check.sh "$@" |
> > >  	# These relocations are okay
> > >  	#	R_RISCV_RELATIVE
> > > -	grep -F -w -v 'R_RISCV_RELATIVE'
> > > +	#	R_RISCV_NONE
> > > +	grep -F -w -v 'R_RISCV_RELATIVE
> > > +R_RISCV_NONE'
> > >  )
> >
> > I'm not super opposed to it, but is there a way to just warn once or
> > something?  It's probably best to still report something, as there's likely
> > some sort of toolchain issue here.
>
> I think Alexandre's approach is ideal from the kernel's perspective.
> This doesn't really seem to be a bug; I see it more as a case where the
> toolchain's handling of R_RISCV_NONE doesn't quite match the kernel's
> expectations.
>
> I found the large number of R_RISCV_NONE relocs only appear in the final
> vmlinux. The key difference is the kernel build's --emit-relocs flag
> and the *(.rela.text*) directive in vmlinux.lds.S. This combination
> forces all relocation entries, including those marked as R_RISCV_NONE,
> to be written verbatim into the final vmlinux.

Ah, OK, if it's coming from "--emit-relocs" then actually it seems fine.  
So I think this is the right way to go, then.  It's on fixes, should 
show up for Linus later this week.

> I traced this back to BFD's implementation and found that during
> relaxation (e.g., when an auipc+jalr is optimized to a jal), the linker
> modifies the first reloc slot to R_RISCV_JAL and marks the second,
> now-useless slot as R_RISCV_DELETE. In the cleanup phase, to prevent
> reprocessing, BFD then changes the cleaned-up DELETE marker to
> R_RISCV_NONE. This is how, when the kernel specifies --emit-relocs,
> these R_RISCV_NONE markers get preserved in the final .rela.text section.
>
> To truly change this, it seems to depend on whether the binutils
> is willing to add a stage to clean up these harmless but
> useless markers.
>
> If possible, I was thinking we could perhaps iterate and remove the
> R_RISCV_NONE entries from .rela.text before the alignment pass.
>
> But if there's no agreement on the BFD side, Alexandre's approach still
> seems correct and aligns with the psABI, where R_RISCV_NONE has no
> operational meaning.
>
> > Also: if you can reproduce it, Nelson can probably fix it.  I'm CCing him.
>
> Reproducing the issue is simple: you just need a call instruction to
> create a relaxation opportunity, then link with --emit-relocs and a
> linker script that includes *(.rela.text*). :)
>
> For convenience, I've put a minimal, self-contained reproduction case
> here: https://gist.github.com/Jingwiw/762606e1dc3b77c352b394e8c5e846de
>
> > >
> > >  if [ -z "$bad_relocs" ]; then
>
> Reviewed-by: Jingwei Wang <wangjingwei@xxxxxxxxxxx>
>
> Thanks,
> Jingwei