Message-ID: <CAH5fLggmXaa9JJ-yGdyH06Um8FopvYh97=rANLcoLc+60_HGqA@mail.gmail.com>
Date: Tue, 2 Sep 2025 12:23:26 +0200
From: Alice Ryhl <aliceryhl@...gle.com>
To: Conor Dooley <conor@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Andrey Ryabinin <ryabinin.a.a@...il.com>, Masahiro Yamada <masahiroy@...nel.org>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, rust-for-linux@...r.kernel.org, 
	Alexander Potapenko <glider@...gle.com>, Andrey Konovalov <andreyknvl@...il.com>, 
	Dmitry Vyukov <dvyukov@...gle.com>, Vincenzo Frascino <vincenzo.frascino@....com>, 
	kasan-dev@...glegroups.com, Nathan Chancellor <nathan@...nel.org>, 
	Nicolas Schier <nicolas@...sle.eu>, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org, patches@...ts.linux.dev, 
	Matthew Maurer <mmaurer@...gle.com>, Sami Tolvanen <samitolvanen@...gle.com>, stable@...r.kernel.org
Subject: Re: [PATCH] rust: kasan/kbuild: fix missing flags on first build

On Tue, Sep 2, 2025 at 12:12 PM Conor Dooley <conor@...nel.org> wrote:
>
> On Tue, Sep 02, 2025 at 08:29:29AM +0000, Alice Ryhl wrote:
> > On Mon, Sep 01, 2025 at 06:45:54PM +0100, Conor Dooley wrote:
> > > Yo,
> > >
> > > On Wed, Apr 09, 2025 at 12:03:11AM +0200, Miguel Ojeda wrote:
> > > > If KASAN is enabled, and one runs in a clean repository e.g.:
> > > >
> > > >     make LLVM=1 prepare
> > > >     make LLVM=1 prepare
> > > >
> > > > Then the Rust code gets rebuilt, which should not happen.
> > > >
> > > > The reason is some of the LLVM KASAN `rustc` flags are added in the
> > > > second run:
> > > >
> > > >     -Cllvm-args=-asan-instrumentation-with-call-threshold=10000
> > > >     -Cllvm-args=-asan-stack=0
> > > >     -Cllvm-args=-asan-globals=1
> > > >     -Cllvm-args=-asan-kernel-mem-intrinsic-prefix=1
> > > >
> > > > Further runs do not rebuild Rust because the flags do not change anymore.
> > > >
> > > > Rebuilding like that in the second run is bad, even if this just happens
> > > > with KASAN enabled, but missing flags in the first one is even worse.
> > > >
> > > > The root issue is that we pass, for some architectures and for the moment,
> > > > a generated `target.json` file. That file is not ready by the time `rustc`
> > > > gets called for the flag test, and thus the flag test fails just because
> > > > the file is not available, e.g.:
> > > >
> > > >     $ ... --target=./scripts/target.json ... -Cllvm-args=...
> > > >     error: target file "./scripts/target.json" does not exist
> > > >
> > > > There are a few approaches we could take here to solve this. For instance,
> > > > we could ensure that every time that the config is rebuilt, we regenerate
> > > > the file and recompute the flags. Or we could use the LLVM version to
> > > > check for these flags, instead of testing the flag (which may have other
> > > > advantages, such as allowing us to detect renames on the LLVM side).
> > > >
> > > > However, it may be easier than that: `rustc` is aware of the `-Cllvm-args`
> > > > regardless of the `--target` (e.g. I checked that the list printed
> > > > is the same, plus that I can check for these flags even if I pass
> > > > a completely unrelated target), and thus we can just eliminate the
> > > > dependency completely.
> > > >
> > > > Thus filter out the target.
> > >
> > >
> > >
> > >
> > > > This does mean that `rustc-option` cannot be used to test a flag that
> > > > requires the right target, but we don't have other users yet, it is a
> > > > minimal change and we want to get rid of custom targets in the future.
> > >
> > > Hmm, while this might be true, I think it should not actually have been
> > > true. Commit ca627e636551e ("rust: cfi: add support for CFI_CLANG with Rust")
> > > added a cc-option check to the rust kconfig symbol, checking if the c
> > > compiler supports the integer normalisations stuff:
> > >     depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers)
> > > and also sets the relevant options in the makefile:
> > >     ifdef CONFIG_RUST
> > >            # Always pass -Zsanitizer-cfi-normalize-integers as CONFIG_RUST selects
> > >            # CONFIG_CFI_ICALL_NORMALIZE_INTEGERS.
> > >            RUSTC_FLAGS_CFI   := -Zsanitizer=kcfi -Zsanitizer-cfi-normalize-integers
> > >            KBUILD_RUSTFLAGS += $(RUSTC_FLAGS_CFI)
> > >            export RUSTC_FLAGS_CFI
> > >     endif
> > > but it should also have added a rustc-option check as, unfortunately,
> > > support for kcfi in rustc is target specific. This results in build
> > > breakages where the arch supports CFI_CLANG and RUST, but the target in
> > > use does not have the kcfi flag set.
> > > I attempted to fix this by adding:
> > >     diff --git a/arch/Kconfig b/arch/Kconfig
> > >     index d1b4ffd6e0856..235709fb75152 100644
> > >     --- a/arch/Kconfig
> > >     +++ b/arch/Kconfig
> > >     @@ -916,6 +916,7 @@ config HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG
> > >      config HAVE_CFI_ICALL_NORMALIZE_INTEGERS_RUSTC
> > >             def_bool y
> > >             depends on HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG
> > >     +       depends on $(rustc-option,-C panic=abort -Zsanitizer=kcfi -Zsanitizer-cfi-normalize-integers)
> > >             depends on RUSTC_VERSION >= 107900
> > >             # With GCOV/KASAN we need this fix: https://github.com/rust-lang/rust/pull/129373
> > >             depends on (RUSTC_LLVM_VERSION >= 190103 && RUSTC_VERSION >= 108200) || \
> > > but of course this does not work for cross compilation, as you're
> > > stripping the target information out and so the check passes on my host
> > > even though my intended
> > > RUSTC_BOOTSTRAP=1 rustc -C panic=abort -Zsanitizer=kcfi -Zsanitizer-cfi-normalize-integers -Ctarget-cpu=generic-rv64 --target=riscv64imac-unknown-none-elf
> > > is a failure.
> > >
> > > I dunno too much about rustc itself, but I suspect that adding kcfi to
> > > the target there is a "free" win, but that'll take time to trickle down
> > > and the minimum version rustc version for the kernel isn't going to have
> > > that.
> > >
> > > I'm not really sure what your target.json suggestion below is, so just
> > > reporting so that someone that understands the alternative solutions can
> > > fix this.
> >
> > Probably right now we have to do this cfg by
> >
> >       depends on CONFIG_ARM
>
> It's valid on x86 too, right?
>
> >
> > to prevent riscv if rustc has the missing setting
> > set on riscv. Once we add it to riscv, we change it to
> >
> >       depends on CONFIG_ARM || (RUSTC_VERSION >= ??? || CONFIG_RISCV)
>
> I kinda shied away from something like this since there was already a
> cc-option on the other half and checking different versions per arch
> becomes a mess - but yeah it kinda is a no-brainer to do it here when
> rustc-option is kinda broken.
>
> I guess the temporary fix is then:
>
> config HAVE_CFI_ICALL_NORMALIZE_INTEGERS_RUSTC
>         def_bool y
>         depends on HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG
>         depends on ARM64 || x86_64
>         depends on RUSTC_VERSION >= 107900
>         # With GCOV/KASAN we need this fix: https://github.com/rust-lang/rust/pull/129373
>         depends on (RUSTC_LLVM_VERSION >= 190103 && RUSTC_VERSION >= 108200) || \
>                 (!GCOV_KERNEL && !KASAN_GENERIC && !KASAN_SW_TAGS)
>
> because there's no 32-bit target with SanitizerSet::KCFI in rustc either
> AFAICT. Then later on it'd become more like:
>
> config HAVE_CFI_ICALL_NORMALIZE_INTEGERS_RUSTC
>         def_bool y
>         depends on HAVE_CFI_ICALL_NORMALIZE_INTEGERS_CLANG
>         depends on RISCV || ((ARM64 || x86_64) && RUSTC_VERSION >= 107900)
>         depends on (ARM64 || x86_64) || (RISCV && RUSTC_VERSION >= 999999)
>         # With GCOV/KASAN we need this fix: https://github.com/rust-lang/rust/pull/129373
>         depends on (RUSTC_LLVM_VERSION >= 190103 && RUSTC_VERSION >= 108200) || \
>                 (!GCOV_KERNEL && !KASAN_GENERIC && !KASAN_SW_TAGS)
>
> but that exact sort of mess is what becomes unwieldy fast since that
> doesn't even cover 32-bit arm.

I think a better way of writing it is like this:

depends on ARCH1 || ARCH2 || ARCH3
depends on !ARCH1 || RUSTC_VERSION >= 000000
depends on !ARCH2 || RUSTC_VERSION >= 000000
depends on !ARCH3 || RUSTC_VERSION >= 000000

Alice

Powered by blists - more mailing lists