[PATCH v1 2/2] LoongArch: Handle jump tables option for RUST

Tiezhu Yang posted 2 patches 3 weeks, 2 days ago
[PATCH v1 2/2] LoongArch: Handle jump tables option for RUST
Posted by Tiezhu Yang 3 weeks, 2 days ago
When compiling with LLVM and CONFIG_RUST is set, there exist objtool
warnings in rust/core.o and rust/kernel.o, like this:

    rust/core.o: warning: objtool:
_RNvXs1_NtNtCs5QSdWC790r4_4core5ascii10ascii_charNtB5_9AsciiCharNtNtB9_3fmt5Debug3fmt+0x54:
sibling call from callable instruction with modified stack frame

For this special case, the related object file shows that there is no
generated relocation section '.rela.discard.tablejump_annotate' for the
table jump instruction jirl, thus objtool can not know that what is the
actual destination address.

If the rustc has the option "-Cllvm-args=--loongarch-annotate-tablejump",
pass the option to enable jump tables for objtool, otherwise it should
pass "-Zno-jump-tables" to keep compatibility with older rustc.

How to test:

  $ rustup component add rust-src
  $ make LLVM=1 rustavailable
  $ make ARCH=loongarch LLVM=1 clean defconfig
  $ scripts/config -d MODVERSIONS \
    -e RUST -e SAMPLES -e SAMPLES_RUST \
    -e SAMPLE_RUST_CONFIGFS -e SAMPLE_RUST_MINIMAL \
    -e SAMPLE_RUST_MISC_DEVICE -e SAMPLE_RUST_PRINT \
    -e SAMPLE_RUST_DMA -e SAMPLE_RUST_DRIVER_PCI \
    -e SAMPLE_RUST_DRIVER_PLATFORM -e SAMPLE_RUST_DRIVER_FAUX \
    -e SAMPLE_RUST_DRIVER_AUXILIARY -e SAMPLE_RUST_HOSTPROGS
  $ make ARCH=loongarch LLVM=1 olddefconfig all

Reported-by: Miguel Ojeda <ojeda@kernel.org>
Closes: https://lore.kernel.org/rust-for-linux/CANiq72mNeCuPkCDrG2db3w=AX+O-zYrfprisDPmRac_qh65Dmg@mail.gmail.com/
Suggested-by: WANG Rui <wangrui@loongson.cn>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/Kconfig  | 4 ++++
 arch/loongarch/Makefile | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index f0abc38c40ac..57933a717e92 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -298,6 +298,10 @@ config AS_HAS_LVZ_EXTENSION
 config CC_HAS_ANNOTATE_TABLEJUMP
 	def_bool $(cc-option,-mannotate-tablejump)
 
+config RUSTC_HAS_ANNOTATE_TABLEJUMP
+	depends on RUST
+	def_bool $(rustc-option,-Cllvm-args=--loongarch-annotate-tablejump)
+
 menu "Kernel type and options"
 
 source "kernel/Kconfig.hz"
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index 9d80af7f75c8..b26d47707031 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -106,6 +106,12 @@ KBUILD_CFLAGS			+= -mannotate-tablejump
 else
 KBUILD_CFLAGS			+= -fno-jump-tables # keep compatibility with older compilers
 endif
+ifdef CONFIG_RUSTC_HAS_ANNOTATE_TABLEJUMP
+# Pass '--loongarch-annotate-tablejump' via '-Cllvm-args' to rustc when RUST is enabled.
+KBUILD_RUSTFLAGS		+= -Cllvm-args=--loongarch-annotate-tablejump
+else
+KBUILD_RUSTFLAGS		+= -Zno-jump-tables # keep compatibility with older compilers
+endif
 ifdef CONFIG_LTO_CLANG
 # The annotate-tablejump option can not be passed to LLVM backend when LTO is enabled.
 # Ensure it is aware of linker with LTO, '--loongarch-annotate-tablejump' also needs to
-- 
2.42.0
Re: [PATCH v1 2/2] LoongArch: Handle jump tables option for RUST
Posted by Miguel Ojeda 3 weeks, 2 days ago
On Tue, Sep 9, 2025 at 11:27 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> +config RUSTC_HAS_ANNOTATE_TABLEJUMP
> +       depends on RUST
> +       def_bool $(rustc-option,-Cllvm-args=--loongarch-annotate-tablejump)

I am not sure if this needs the `depends on` -- from a quick test, it
seems to run regardless of it (and your `ifdef` below doesn't care
either). Cc'ing Matthew in case he remembers why the docs mention that
the Kconfig one should be guarded by `RUST_IS_AVAILABLE`.

In any case, it shouldn't hurt and it is only on the LoongArch Makefile.

If you will be taking this through the LoongArch tree:

Acked-by: Miguel Ojeda <ojeda@kernel.org>

Thanks again for fixing this!

Cheers,
Miguel
Re: [PATCH v1 2/2] LoongArch: Handle jump tables option for RUST
Posted by Matthew Maurer 3 weeks, 2 days ago
On Tue, Sep 9, 2025 at 3:16 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Tue, Sep 9, 2025 at 11:27 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> > +config RUSTC_HAS_ANNOTATE_TABLEJUMP
> > +       depends on RUST
> > +       def_bool $(rustc-option,-Cllvm-args=--loongarch-annotate-tablejump)
>
> I am not sure if this needs the `depends on` -- from a quick test, it
> seems to run regardless of it (and your `ifdef` below doesn't care
> either). Cc'ing Matthew in case he remembers why the docs mention that
> the Kconfig one should be guarded by `RUST_IS_AVAILABLE`.

I think this isn't needed. I added it initially because Kconfig was
dying when `rustc` was not on the path, but I'm not sure how I managed
that, since `success,trap` should guard against that (perhaps I was
doing something wrong in earlier development, fixed it, and didn't
remove the restriction). I have tested `rustc-option` with an
intentionally missing `rustc`, and Kconfig doesn't seem to be taken
down (and it gets a no answer), so the change to the macro means that
doc warning should be removed.

>
> In any case, it shouldn't hurt and it is only on the LoongArch Makefile.
>
> If you will be taking this through the LoongArch tree:
>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>
>
> Thanks again for fixing this!
>
> Cheers,
> Miguel
Re: [PATCH v1 2/2] LoongArch: Handle jump tables option for RUST
Posted by Miguel Ojeda 3 weeks, 2 days ago
On Tue, Sep 9, 2025 at 8:06 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> I think this isn't needed. I added it initially because Kconfig was
> dying when `rustc` was not on the path, but I'm not sure how I managed
> that, since `success,trap` should guard against that (perhaps I was
> doing something wrong in earlier development, fixed it, and didn't
> remove the restriction). I have tested `rustc-option` with an
> intentionally missing `rustc`, and Kconfig doesn't seem to be taken
> down (and it gets a no answer), so the change to the macro means that
> doc warning should be removed.

Yeah, that matches my test then, thanks! (and for the patch)

Cheers,
Miguel