Message-ID: <CAH5fLgjb3Wxbkzvvy9H6QUYVpxXvkse1rnDmmR3nVHjp6zEx9A@mail.gmail.com>
Date: Mon, 14 Apr 2025 13:22:10 +0200
From: Alice Ryhl <aliceryhl@...gle.com>
To: Miguel Ojeda <ojeda@...nel.org>
Cc: 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 Wed, Apr 9, 2025 at 12:03 AM Miguel Ojeda <ojeda@...nel.org> 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.
>
> We could only filter in the case `target.json` is used, to make it work
> in more cases, but then it would be harder to notice that it may not
> work in a couple architectures.
>
> Cc: Matthew Maurer <mmaurer@...gle.com>
> Cc: Sami Tolvanen <samitolvanen@...gle.com>
> Cc: stable@...r.kernel.org
> Fixes: e3117404b411 ("kbuild: rust: Enable KASAN support")
> Signed-off-by: Miguel Ojeda <ojeda@...nel.org>

I've boot-tested Android's KASAN configuration with this patch, and it
continues to work. It also passes Android CI [1].

Tested-by: Alice Ryhl <aliceryhl@...gle.com>

Alice

[1]: http://r.android.com/3584874

Powered by blists - more mailing lists