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@google.com Cc: Sami Tolvanen samitolvanen@google.com Cc: stable@vger.kernel.org Fixes: e3117404b411 ("kbuild: rust: Enable KASAN support") Signed-off-by: Miguel Ojeda ojeda@kernel.org --- By the way, I noticed that we are not getting `asan-instrument-allocas` enabled in neither C nor Rust -- upstream LLVM renamed it in commit 8176ee9b5dda ("[asan] Rename asan-instrument-allocas -> asan-instrument-dynamic-allocas")). But it happened a very long time ago (9 years ago), and the addition in the kernel is fairly old too, in 342061ee4ef3 ("kasan: support alloca() poisoning"). I assume it should either be renamed or removed? Happy to send a patch if so.
scripts/Makefile.compiler | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 8956587b8547..7ed7f92a7daa 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -80,7 +80,7 @@ ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3)) # TODO: remove RUSTC_BOOTSTRAP=1 when we raise the minimum GNU Make version to 4.4 __rustc-option = $(call try-run,\ echo '#![allow(missing_docs)]#![feature(no_core)]#![no_core]' | RUSTC_BOOTSTRAP=1\ - $(1) --sysroot=/dev/null $(filter-out --sysroot=/dev/null,$(2)) $(3)\ + $(1) --sysroot=/dev/null $(filter-out --sysroot=/dev/null --target=%,$(2)) $(3)\ --crate-type=rlib --out-dir=$(TMPOUT) --emit=obj=- - >/dev/null,$(3),$(4))
# rustc-option
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8 -- 2.49.0
On Tue, Apr 8, 2025 at 3:03 PM Miguel Ojeda ojeda@kernel.org wrote:
If KASAN is enabled, and one runs in a clean repository e.g.:
make LLVM=1 prepare make LLVM=1 prepareThen 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=1Further 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 existThere 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@google.com Cc: Sami Tolvanen samitolvanen@google.com Cc: stable@vger.kernel.org Fixes: e3117404b411 ("kbuild: rust: Enable KASAN support") Signed-off-by: Miguel Ojeda ojeda@kernel.org
By the way, I noticed that we are not getting `asan-instrument-allocas` enabled in neither C nor Rust -- upstream LLVM renamed it in commit 8176ee9b5dda ("[asan] Rename asan-instrument-allocas -> asan-instrument-dynamic-allocas")). But it happened a very long time ago (9 years ago), and the addition in the kernel is fairly old too, in 342061ee4ef3 ("kasan: support alloca() poisoning"). I assume it should either be renamed or removed? Happy to send a patch if so.
scripts/Makefile.compiler | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 8956587b8547..7ed7f92a7daa 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -80,7 +80,7 @@ ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3)) # TODO: remove RUSTC_BOOTSTRAP=1 when we raise the minimum GNU Make version to 4.4 __rustc-option = $(call try-run,\ echo '#![allow(missing_docs)]#![feature(no_core)]#![no_core]' | RUSTC_BOOTSTRAP=1\
$(1) --sysroot=/dev/null $(filter-out --sysroot=/dev/null,$(2)) $(3)\
$(1) --sysroot=/dev/null $(filter-out --sysroot=/dev/null --target=%,$(2)) $(3)\ --crate-type=rlib --out-dir=$(TMPOUT) --emit=obj=- - >/dev/null,$(3),$(4))
The problem with this change is that some `rustc` flags will only be valid on some platforms. For example, if we check if a `-Zsanitizer=shadow-call-stack` is available, it will fail for non aarch64 targets. I don't think we're currently directly detecting any of these, because all of the stuff we're using is known present by virtue of minimum compiler version, which means we can possibly get away with this change for now. That said, this isn't a long term solution unless we are getting rid of target.json files altogether, as one of the main adaptations we've been putting in those is to enable additional target features.
# rustc-option
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
2.49.0
On Wed, Apr 9, 2025 at 11:35 PM Matthew Maurer mmaurer@google.com wrote:
The problem with this change is that some `rustc` flags will only be valid on some platforms. For example, if we check if a
Indeed -- this limitation is acknowledged in the commit message. The priority is fixing the issue, not future features/needs, so I went with this.
In any case, if we need to test a flag that requires the target, it is likely we can do the check based on the Rust version, the architecture, the Rust's LLVM version, etc.
As for `target.json`s, the plan is indeed to get rid of them, since they are permanently unstable. So any features used by the `target.json`s need to be requested to upstream Rust (e.g. flags) so that we can do the same without a custom target specification, so that eventually we can remove all of them (and thus `rustc-option` will work in all cases again).
Thanks for taking a look!
Cheers, Miguel
On Wed, Apr 9, 2025 at 12:03 AM Miguel Ojeda ojeda@kernel.org wrote:
Thus filter out the target.
Applied to `rust-fixes` -- thanks everyone!
I am applying this one fairly quickly, to start to get wider testing, but it would be nice to get some tags, so I am happy to rebase to add them for the next day or two.
Cheers, Miguel
On Wed, Apr 9, 2025 at 12:03 AM Miguel Ojeda ojeda@kernel.org wrote:
If KASAN is enabled, and one runs in a clean repository e.g.:
make LLVM=1 prepare make LLVM=1 prepareThen 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=1Further 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 existThere 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@google.com Cc: Sami Tolvanen samitolvanen@google.com Cc: stable@vger.kernel.org Fixes: e3117404b411 ("kbuild: rust: Enable KASAN support") Signed-off-by: Miguel Ojeda ojeda@kernel.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@google.com
Alice
linux-stable-mirror@lists.linaro.org
-
Alice Ryhl -
Matthew Maurer -
Miguel Ojeda -
Miguel Ojeda