linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jingwei Wang <wangjingwei@iscas.ac.cn>
To: Palmer Dabbelt <palmer@dabbelt.com>,
	Alexandre Ghiti <alexghiti@rivosinc.com>
Cc: alexghiti@rivosinc.com, Nelson Chu <nelson@rivosinc.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	aou@eecs.berkeley.edu, Alexandre Ghiti <alex@ghiti.fr>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: Stop considering R_RISCV_NONE as bad relocations
Date: Fri, 11 Jul 2025 21:49:09 +0800	[thread overview]
Message-ID: <20250711134909.GA73430@iscas.ac.cn> (raw)
In-Reply-To: <mhng-7CDAD4A1-707B-4ACE-BAF0-643C281B9AD7@palmerdabbelt-mac>

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@rivosinc.com 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@rivosinc.com>
> > ---
> >  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.

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@iscas.ac.cn>

Thanks,
Jingwei


  reply	other threads:[~2025-07-11 13:49 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-10  8:34 [PATCH] riscv: Stop considering R_RISCV_NONE as bad relocations Alexandre Ghiti
2025-07-10 12:25 ` Ron Economos
2025-07-10 18:43 ` Palmer Dabbelt
2025-07-11 13:49   ` Jingwei Wang [this message]
2025-07-16 15:17     ` Palmer Dabbelt
2025-07-16 15:21 ` patchwork-bot+linux-riscv

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250711134909.GA73430@iscas.ac.cn \
    --to=wangjingwei@iscas.ac.cn \
    --cc=alex@ghiti.fr \
    --cc=alexghiti@rivosinc.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=nelson@rivosinc.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).