-
Notifications
You must be signed in to change notification settings - Fork 464
KASAN Support #1087
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KASAN Support #1087
Conversation
Creates flag probe macro variants for `rustc`. These are helpful because: 1. `rustc` support will soon be a minimum rather than a pinned version. 2. We already support multiple LLVMs linked into `rustc`, and these are needed to probe what LLVM parameters `rustc` will accept. Signed-off-by: Matthew Maurer <[email protected]>
Rust supports KASAN via LLVM, but prior to this patch, the flags aren't set properly. Suggested-by: Miguel Ojeda <[email protected]> Signed-off-by: Matthew Maurer <[email protected]>
In case it is useful, I had this in a WIP branch -- I was concerned about flags being conditionally supported for some targets only. diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
index 8fcb427405a6..d9505c83d099 100644
--- a/scripts/Makefile.compiler
+++ b/scripts/Makefile.compiler
@@ -20,6 +20,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$
# Exit code chooses option. "$$TMP" serves as a temporary file and is
# automatically cleaned up.
try-run = $(shell set -e; \
+ TMPOUT=$(TMPOUT); \
TMP=$(TMPOUT)/tmp; \
trap "rm -rf $(TMPOUT)" EXIT; \
mkdir -p $(TMPOUT); \
@@ -61,6 +62,26 @@ cc-option-yn = $(call try-run,\
cc-disable-warning = $(call try-run,\
$(CC) -Werror $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) -W$(strip $(1)) -c -x c /dev/null -o "$$TMP",-Wno-$(strip $(1)))
+# rustc-option
+#
+# Currently, this is only meant to be used by rustc-param, since we only support
+# a single Rust version for the time being.
+#
+# In addition, for the moment, we are passing here the base builtin target here
+# as an approximation of the actual target we will use, since in some cases we
+# still use the target.json spec, which is generated based on the configuration
+# (and thus it is not available where we would like to use rustc-option). When
+# we do not use target.json anymore, we should switch to pass KBUILD_RUSTFLAGS
+# which contains all the target-related flags, like it is done in cc-option.
+#
+# Moreover, please note that -Dwarnings currently does not convert every single
+# warning into an error [1], thus flags such as -Ctarget-feature cannot be
+# tested this way, but it does work for rustc-param.
+#
+# [1]: https://github.com/rust-lang/compiler-team/issues/473
+rustc-option = $(call try-run,\
+ echo '#![feature(no_core)]#![no_core]' | RUSTC_BOOTSTRAP=1 $(RUSTC) -Dwarnings --target=$(RUSTC_TARGET) $(1) --out-dir="$$TMPOUT" --emit=obj="$$TMP" --crate-type=lib -,$(1),$(2))
+
# gcc-min-version
# Usage: cflags-$(call gcc-min-version, 70100) += -foo
gcc-min-version = $(call test-ge, $(CONFIG_GCC_VERSION), $1)
diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index 390658a2d5b7..3159c2d2a7cb 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -12,6 +12,7 @@ endif
KASAN_SHADOW_OFFSET ?= $(CONFIG_KASAN_SHADOW_OFFSET)
cc-param = $(call cc-option, -mllvm -$(1), $(call cc-option, --param $(1)))
+rustc-param = $(call rustc-option, -Cllvm-args=-$(1))
ifdef CONFIG_KASAN_STACK
stack_enable := 1 |
I see you use |
$(1) $(2) $(3) --crate-type=rlib /dev/null -o "$$TMP",$(3),$(4)) | ||
|
||
# rustc-option | ||
# Usage: rustflags-y += $(call rustc-option,-Cinstrument-coverage,-Zinstrument-coverage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For flags that can be determined by the version, I wonder if it would be best to tell people to use rustc-min-version
instead (I sent a (broken) patch adding that one and an example use case).
On one hand, having them versioned means we can easily grep & remove them when upgrading the compiler minimum.
On the other hand, it could be that someone has patched their compiler, e.g. backporting support for a given flag in a Linux distribution. So there is value, but I am not sure if it is just a theoretical advantage (and even then, a Linux distribution could also patch their kernel instead, too).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Of course, we still need it for rustc-param
, that is what I did too -- but perhaps we can put a comment about it if we think one or the other way is better).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that in many cases rustc-min-version
would be preferable, esp. because with -Z
flags, the feature may be available but incomplete in some cases. However, I'd point out that users needing to look up what version of Rust added a flag they're going to use every time seems a bit more onerous than the situation in C.
I'll add a note to the docs that if you're testing for a -Z
flag, you should consider rustc-min-version
instead. I think rustc-option
is probably better for -C
options in most cases.
And if some of the above does not make sense, please ignore it -- I am just doing a brain dump here of things I was considering back then (a few months ago). |
Yes, I'm now convinced that my current approach here is a bad idea, because availability of flags will vary based on targets. I think what I'll do here is just remove this part from the patch - we should add support for this, but it doesn't need to be this patch, as we don't need it for |
I take that back - |
Closing as it's been posted to the list for discussion. |
Creates flag probe macro variants for `rustc`. These are helpful because: 1. The kernel now supports a minimum `rustc` version rather than a single version. 2. `rustc` links against a range of LLVM revisions, occasionally even ones without an official release number. Since the availability of some Rust flags depends on which LLVM it has been linked against, probing is necessary. Signed-off-by: Matthew Maurer <[email protected]> Link: #1087 Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>
Looking for a quick pre-review before sending these to the list
Right now, if we turn on KASAN, Rust code will cause violations because it's not enabled properly. Miguel previously gave us a patch that got this working by manually setting the right flags for current LLVM.
This PR: