Re: [PATCH] rust: kasan/kbuild: fix missing flags on first build

[Alice Ryhl <aliceryhl@google.com>]

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

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)

Alice