rust/probe.rs | 7 +++++++ scripts/Makefile.compiler | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-)
Fix a few different compiler errors that cause rustc-option to give
wrong results.
If KBUILD_RUSTFLAGS or the flags being tested contain any -Z flags, then
the error below is generated. The RUSTC_BOOTSTRAP environment variable
is added to fix this error.
error: the option `Z` is only accepted on the nightly compiler
help: consider switching to a nightly toolchain: `rustup default nightly`
note: selecting a toolchain with `+toolchain` arguments require a rustup proxy;
see <https://rust-lang.github.io/rustup/concepts/index.html>
note: for more information about Rust's stability policy, see
<https://doc.rust-lang.org/book/appendix-07-nightly-rust.html#unstable-features>
error: 1 nightly option were parsed
The probe may also fail incorrectly with the below error message. To fix
it, the /dev/null argument is replaced with a new rust/probe.rs file
that doesn't need even the core part of the standard library.
error[E0463]: can't find crate for `std`
|
= note: the `aarch64-unknown-none` target may not be installed
= help: consider downloading the target with `rustup target add aarch64-unknown-none`
= help: consider building the standard library from source with `cargo build -Zbuild-std`
The -o and --out-dir parameters are altered to fix this warning:
warning: ignoring --out-dir flag due to -o flag
I verified that the Kconfig version of rustc-option doesn't have the
same issues.
Fixes: c42297438aee ("kbuild: rust: Define probing macros for rustc")
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v2:
- Add `export` to RUSTC_BOOTSTRAP.
- Fix error about core being missing.
- Fix warning about -o flag.
- Link to v1: https://lore.kernel.org/r/20241008-rustc-option-bootstrap-v1-1-9eb06261d4f7@google.com
---
rust/probe.rs | 7 +++++++
scripts/Makefile.compiler | 5 +++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/rust/probe.rs b/rust/probe.rs
new file mode 100644
index 000000000000..bf024e394408
--- /dev/null
+++ b/rust/probe.rs
@@ -0,0 +1,7 @@
+//! Nearly empty file passed to rustc-option by Make.
+//!
+//! The no_core attribute is needed because rustc-option otherwise fails due to
+//! not being able to find the core part of the standard library.
+
+#![feature(no_core)]
+#![no_core]
diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
index 057305eae85c..08d5b7177ea8 100644
--- a/scripts/Makefile.compiler
+++ b/scripts/Makefile.compiler
@@ -21,6 +21,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$
# automatically cleaned up.
try-run = $(shell set -e; \
TMP=$(TMPOUT)/tmp; \
+ export RUSTC_BOOTSTRAP=1; \
trap "rm -rf $(TMPOUT)" EXIT; \
mkdir -p $(TMPOUT); \
if ($(1)) >/dev/null 2>&1; \
@@ -76,7 +77,7 @@ ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
# __rustc-option
# Usage: MY_RUSTFLAGS += $(call __rustc-option,$(RUSTC),$(MY_RUSTFLAGS),-Cinstrument-coverage,-Zinstrument-coverage)
__rustc-option = $(call try-run,\
- $(1) $(2) $(3) --crate-type=rlib /dev/null --out-dir=$$TMPOUT -o "$$TMP",$(3),$(4))
+ $(1) $(2) $(3) --crate-type=rlib $(srctree)/rust/probe.rs --out-dir=$$TMP,$(3),$(4))
# rustc-option
# Usage: rustflags-y += $(call rustc-option,-Cinstrument-coverage,-Zinstrument-coverage)
@@ -86,4 +87,4 @@ rustc-option = $(call __rustc-option, $(RUSTC),\
# rustc-option-yn
# Usage: flag := $(call rustc-option-yn,-Cinstrument-coverage)
rustc-option-yn = $(call try-run,\
- $(RUSTC) $(KBUILD_RUSTFLAGS) $(1) --crate-type=rlib /dev/null --out-dir=$$TMPOUT -o "$$TMP",y,n)
+ $(RUSTC) $(KBUILD_RUSTFLAGS) $(1) --crate-type=rlib $(srctree)/rust/probe.rs --out-dir=$$TMP,y,n)
---
base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
change-id: 20241008-rustc-option-bootstrap-607e5bf3114c
Best regards,
--
Alice Ryhl <aliceryhl@google.com>On Tue, Oct 8, 2024 at 7:32 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Fix a few different compiler errors that cause rustc-option to give
> wrong results.
>
> If KBUILD_RUSTFLAGS or the flags being tested contain any -Z flags, then
> the error below is generated. The RUSTC_BOOTSTRAP environment variable
> is added to fix this error.
>
> error: the option `Z` is only accepted on the nightly compiler
> help: consider switching to a nightly toolchain: `rustup default nightly`
> note: selecting a toolchain with `+toolchain` arguments require a rustup proxy;
> see <https://rust-lang.github.io/rustup/concepts/index.html>
> note: for more information about Rust's stability policy, see
> <https://doc.rust-lang.org/book/appendix-07-nightly-rust.html#unstable-features>
> error: 1 nightly option were parsed
>
> The probe may also fail incorrectly with the below error message. To fix
> it, the /dev/null argument is replaced with a new rust/probe.rs file
> that doesn't need even the core part of the standard library.
>
> error[E0463]: can't find crate for `std`
> |
> = note: the `aarch64-unknown-none` target may not be installed
> = help: consider downloading the target with `rustup target add aarch64-unknown-none`
> = help: consider building the standard library from source with `cargo build -Zbuild-std`
>
> The -o and --out-dir parameters are altered to fix this warning:
>
> warning: ignoring --out-dir flag due to -o flag
>
> I verified that the Kconfig version of rustc-option doesn't have the
> same issues.
>
> Fixes: c42297438aee ("kbuild: rust: Define probing macros for rustc")
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Changes in v2:
> - Add `export` to RUSTC_BOOTSTRAP.
> - Fix error about core being missing.
> - Fix warning about -o flag.
> - Link to v1: https://lore.kernel.org/r/20241008-rustc-option-bootstrap-v1-1-9eb06261d4f7@google.com
v3 sent here:
https://lore.kernel.org/all/20241009-rustc-option-bootstrap-v3-1-5fa0d520efba@google.com/
Alice
On Tue, Oct 8, 2024 at 7:32 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> diff --git a/rust/probe.rs b/rust/probe.rs
> new file mode 100644
> index 000000000000..bf024e394408
> --- /dev/null
> +++ b/rust/probe.rs
> @@ -0,0 +1,7 @@
> +//! Nearly empty file passed to rustc-option by Make.
> +//!
> +//! The no_core attribute is needed because rustc-option otherwise fails due to
> +//! not being able to find the core part of the standard library.
> +
> +#![feature(no_core)]
> +#![no_core]
What I did in the GitHub issue was:
echo '#![feature(no_core)]#![no_core]' | rustc ... -
to avoid a file just for this and so that things were closer (i.e. the
comment would be then in the `Makefile`).
Not sure what Masahiro prefers.
Cheers,
Miguel
On Wed, Oct 9, 2024 at 2:32 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Fix a few different compiler errors that cause rustc-option to give
> wrong results.
>
> If KBUILD_RUSTFLAGS or the flags being tested contain any -Z flags, then
> the error below is generated. The RUSTC_BOOTSTRAP environment variable
> is added to fix this error.
>
> error: the option `Z` is only accepted on the nightly compiler
> help: consider switching to a nightly toolchain: `rustup default nightly`
> note: selecting a toolchain with `+toolchain` arguments require a rustup proxy;
> see <https://rust-lang.github.io/rustup/concepts/index.html>
> note: for more information about Rust's stability policy, see
> <https://doc.rust-lang.org/book/appendix-07-nightly-rust.html#unstable-features>
> error: 1 nightly option were parsed
>
> The probe may also fail incorrectly with the below error message. To fix
> it, the /dev/null argument is replaced with a new rust/probe.rs file
> that doesn't need even the core part of the standard library.
>
> error[E0463]: can't find crate for `std`
> |
> = note: the `aarch64-unknown-none` target may not be installed
> = help: consider downloading the target with `rustup target add aarch64-unknown-none`
> = help: consider building the standard library from source with `cargo build -Zbuild-std`
>
> The -o and --out-dir parameters are altered to fix this warning:
>
> warning: ignoring --out-dir flag due to -o flag
>
> I verified that the Kconfig version of rustc-option doesn't have the
> same issues.
>
> Fixes: c42297438aee ("kbuild: rust: Define probing macros for rustc")
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Changes in v2:
> - Add `export` to RUSTC_BOOTSTRAP.
> - Fix error about core being missing.
> - Fix warning about -o flag.
> - Link to v1: https://lore.kernel.org/r/20241008-rustc-option-bootstrap-v1-1-9eb06261d4f7@google.com
> ---
> rust/probe.rs | 7 +++++++
> scripts/Makefile.compiler | 5 +++--
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/rust/probe.rs b/rust/probe.rs
> new file mode 100644
> index 000000000000..bf024e394408
> --- /dev/null
> +++ b/rust/probe.rs
> @@ -0,0 +1,7 @@
> +//! Nearly empty file passed to rustc-option by Make.
> +//!
> +//! The no_core attribute is needed because rustc-option otherwise fails due to
> +//! not being able to find the core part of the standard library.
> +
> +#![feature(no_core)]
> +#![no_core]
> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> index 057305eae85c..08d5b7177ea8 100644
> --- a/scripts/Makefile.compiler
> +++ b/scripts/Makefile.compiler
> @@ -21,6 +21,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$
> # automatically cleaned up.
> try-run = $(shell set -e; \
> TMP=$(TMPOUT)/tmp; \
> + export RUSTC_BOOTSTRAP=1; \
try-run is not Rust-specific.
Is there any reason why you did not add it
to __rustc-option?
__rustc-option = $(call try-run,\
RUSTC_BOOTSTRAP=1 $(1) $(2) $(3) --crate-type=rlib
$(srctree)/rust/probe.rs --out-dir=$$TMP,$(3),$(4))
I guess it is still suspicious because the top-level Makefile
exports RUCTC_BOOTSTRAP.
> trap "rm -rf $(TMPOUT)" EXIT; \
> mkdir -p $(TMPOUT); \
> if ($(1)) >/dev/null 2>&1; \
> @@ -76,7 +77,7 @@ ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
> # __rustc-option
> # Usage: MY_RUSTFLAGS += $(call __rustc-option,$(RUSTC),$(MY_RUSTFLAGS),-Cinstrument-coverage,-Zinstrument-coverage)
> __rustc-option = $(call try-run,\
> - $(1) $(2) $(3) --crate-type=rlib /dev/null --out-dir=$$TMPOUT -o "$$TMP",$(3),$(4))
> + $(1) $(2) $(3) --crate-type=rlib $(srctree)/rust/probe.rs --out-dir=$$TMP,$(3),$(4))
>
> # rustc-option
> # Usage: rustflags-y += $(call rustc-option,-Cinstrument-coverage,-Zinstrument-coverage)
> @@ -86,4 +87,4 @@ rustc-option = $(call __rustc-option, $(RUSTC),\
> # rustc-option-yn
> # Usage: flag := $(call rustc-option-yn,-Cinstrument-coverage)
> rustc-option-yn = $(call try-run,\
> - $(RUSTC) $(KBUILD_RUSTFLAGS) $(1) --crate-type=rlib /dev/null --out-dir=$$TMPOUT -o "$$TMP",y,n)
> + $(RUSTC) $(KBUILD_RUSTFLAGS) $(1) --crate-type=rlib $(srctree)/rust/probe.rs --out-dir=$$TMP,y,n)
>
> ---
> base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
> change-id: 20241008-rustc-option-bootstrap-607e5bf3114c
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>
>
--
Best Regards
Masahiro Yamada
On Tue, Oct 8, 2024 at 9:00 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Wed, Oct 9, 2024 at 2:32 AM Alice Ryhl <aliceryhl@google.com> wrote: > > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler > > index 057305eae85c..08d5b7177ea8 100644 > > --- a/scripts/Makefile.compiler > > +++ b/scripts/Makefile.compiler > > @@ -21,6 +21,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$ > > # automatically cleaned up. > > try-run = $(shell set -e; \ > > TMP=$(TMPOUT)/tmp; \ > > + export RUSTC_BOOTSTRAP=1; \ > > > try-run is not Rust-specific. > > Is there any reason why you did not add it > to __rustc-option? > > > __rustc-option = $(call try-run,\ > RUSTC_BOOTSTRAP=1 $(1) $(2) $(3) --crate-type=rlib > $(srctree)/rust/probe.rs --out-dir=$$TMP,$(3),$(4)) I had an explanation for this in the commit message, but it looks like it got lost when I rewrote it for v2. Anyway, the reason is that I'd have to modify both __rustc-option and rustc-option-yn to do that, and putting it here seemed more future-proof against making the same mistake in any rustc-* commands added in the future. But I realize that it's not clear-cut. I'm happy to move it if you prefer, or perhaps add a try-run-rust. Let me know what you think. > I guess it is still suspicious because the top-level Makefile > exports RUCTC_BOOTSTRAP. Moving the declaration of RUSTC_BOOTSTRAP to the top of the Makefile seems to fix it. I guess moving it is probably a better solution than adding it in scripts/Makefile.compiler. Not that I really understand why that is. The existing invocations are in scripts/Makefile.kasan which is invoked after RUSTC_BOOTSTRAP is declared. Alice
On Wed, Oct 9, 2024 at 4:42 AM Alice Ryhl <aliceryhl@google.com> wrote: > > On Tue, Oct 8, 2024 at 9:00 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Wed, Oct 9, 2024 at 2:32 AM Alice Ryhl <aliceryhl@google.com> wrote: > > > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler > > > index 057305eae85c..08d5b7177ea8 100644 > > > --- a/scripts/Makefile.compiler > > > +++ b/scripts/Makefile.compiler > > > @@ -21,6 +21,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$ > > > # automatically cleaned up. > > > try-run = $(shell set -e; \ > > > TMP=$(TMPOUT)/tmp; \ > > > + export RUSTC_BOOTSTRAP=1; \ > > > > > > try-run is not Rust-specific. > > > > Is there any reason why you did not add it > > to __rustc-option? > > > > > > __rustc-option = $(call try-run,\ > > RUSTC_BOOTSTRAP=1 $(1) $(2) $(3) --crate-type=rlib > > $(srctree)/rust/probe.rs --out-dir=$$TMP,$(3),$(4)) > > I had an explanation for this in the commit message, but it looks like > it got lost when I rewrote it for v2. Anyway, the reason is that I'd > have to modify both __rustc-option and rustc-option-yn to do that, and > putting it here seemed more future-proof against making the same > mistake in any rustc-* commands added in the future. One solution is to delete rustc-option-yn since there are no users of it. Another solution is to refactor the code. Either way, there is no good reason for code duplication. If you keep rustc-option-yn, you can rebased v3 on top of this patch: https://lore.kernel.org/lkml/20241009102821.2675718-1-masahiroy@kernel.org/T/#u > > But I realize that it's not clear-cut. I'm happy to move it if you prefer, > or perhaps add a try-run-rust. Let me know what you think. > > > I guess it is still suspicious because the top-level Makefile > > exports RUCTC_BOOTSTRAP. > > Moving the declaration of RUSTC_BOOTSTRAP to the top of the Makefile > seems to fix it. I guess moving it is probably a better solution than > adding it in scripts/Makefile.compiler. I prefer to keep RUSTC_BOOTSTRAP close to other compiler flags. > > Not that I really understand why that is. The existing invocations are > in scripts/Makefile.kasan which is invoked after RUSTC_BOOTSTRAP is > declared. Miguel gave perfect explanation. https://lore.kernel.org/linux-kbuild/CAK7LNARDkS6uAHcdyZatc2SB7A66TWGfKZWNkYOoa7i3jo3QqA@mail.gmail.com/T/#m899bc321ae80d9c4a904680709c9a53f09e51b9e > > > Alice > -- Best Regards Masahiro Yamada
On Wed, Oct 9, 2024 at 12:32 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Wed, Oct 9, 2024 at 4:42 AM Alice Ryhl <aliceryhl@google.com> wrote: > > > > On Tue, Oct 8, 2024 at 9:00 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Wed, Oct 9, 2024 at 2:32 AM Alice Ryhl <aliceryhl@google.com> wrote: > > > > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler > > > > index 057305eae85c..08d5b7177ea8 100644 > > > > --- a/scripts/Makefile.compiler > > > > +++ b/scripts/Makefile.compiler > > > > @@ -21,6 +21,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$ > > > > # automatically cleaned up. > > > > try-run = $(shell set -e; \ > > > > TMP=$(TMPOUT)/tmp; \ > > > > + export RUSTC_BOOTSTRAP=1; \ > > > > > > > > > try-run is not Rust-specific. > > > > > > Is there any reason why you did not add it > > > to __rustc-option? > > > > > > > > > __rustc-option = $(call try-run,\ > > > RUSTC_BOOTSTRAP=1 $(1) $(2) $(3) --crate-type=rlib > > > $(srctree)/rust/probe.rs --out-dir=$$TMP,$(3),$(4)) > > > > I had an explanation for this in the commit message, but it looks like > > it got lost when I rewrote it for v2. Anyway, the reason is that I'd > > have to modify both __rustc-option and rustc-option-yn to do that, and > > putting it here seemed more future-proof against making the same > > mistake in any rustc-* commands added in the future. > > > One solution is to delete rustc-option-yn since there are no users of it. > > Another solution is to refactor the code. > > Either way, there is no good reason for code duplication. > > > If you keep rustc-option-yn, you can rebased v3 on top of this patch: > https://lore.kernel.org/lkml/20241009102821.2675718-1-masahiroy@kernel.org/T/#u I'll rebase on top of that. If we choose to delete rustc-option-yn then I think we should first merge the refactor and then delete it in a follow-up. That way, when someone does need it, they will find the refactored implementation in the git history. Alice
Thanks for catching this - I had a specialized toolchain enabled which
didn't exercise the `RUSTC_BOOTSTRAP` issue, and without an unexpected
failure, didn't see the `--out-dir` flag conflict after I got the
request to add it.
Reviewed-By: Matthew Maurer <mmaurer@google.com>
On Tue, Oct 8, 2024 at 10:32 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Fix a few different compiler errors that cause rustc-option to give
> wrong results.
>
> If KBUILD_RUSTFLAGS or the flags being tested contain any -Z flags, then
> the error below is generated. The RUSTC_BOOTSTRAP environment variable
> is added to fix this error.
>
> error: the option `Z` is only accepted on the nightly compiler
> help: consider switching to a nightly toolchain: `rustup default nightly`
> note: selecting a toolchain with `+toolchain` arguments require a rustup proxy;
> see <https://rust-lang.github.io/rustup/concepts/index.html>
> note: for more information about Rust's stability policy, see
> <https://doc.rust-lang.org/book/appendix-07-nightly-rust.html#unstable-features>
> error: 1 nightly option were parsed
>
> The probe may also fail incorrectly with the below error message. To fix
> it, the /dev/null argument is replaced with a new rust/probe.rs file
> that doesn't need even the core part of the standard library.
>
> error[E0463]: can't find crate for `std`
> |
> = note: the `aarch64-unknown-none` target may not be installed
> = help: consider downloading the target with `rustup target add aarch64-unknown-none`
> = help: consider building the standard library from source with `cargo build -Zbuild-std`
>
> The -o and --out-dir parameters are altered to fix this warning:
>
> warning: ignoring --out-dir flag due to -o flag
>
> I verified that the Kconfig version of rustc-option doesn't have the
> same issues.
>
> Fixes: c42297438aee ("kbuild: rust: Define probing macros for rustc")
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> Changes in v2:
> - Add `export` to RUSTC_BOOTSTRAP.
> - Fix error about core being missing.
> - Fix warning about -o flag.
> - Link to v1: https://lore.kernel.org/r/20241008-rustc-option-bootstrap-v1-1-9eb06261d4f7@google.com
> ---
> rust/probe.rs | 7 +++++++
> scripts/Makefile.compiler | 5 +++--
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/rust/probe.rs b/rust/probe.rs
> new file mode 100644
> index 000000000000..bf024e394408
> --- /dev/null
> +++ b/rust/probe.rs
> @@ -0,0 +1,7 @@
> +//! Nearly empty file passed to rustc-option by Make.
> +//!
> +//! The no_core attribute is needed because rustc-option otherwise fails due to
> +//! not being able to find the core part of the standard library.
> +
> +#![feature(no_core)]
> +#![no_core]
> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> index 057305eae85c..08d5b7177ea8 100644
> --- a/scripts/Makefile.compiler
> +++ b/scripts/Makefile.compiler
> @@ -21,6 +21,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$
> # automatically cleaned up.
> try-run = $(shell set -e; \
> TMP=$(TMPOUT)/tmp; \
> + export RUSTC_BOOTSTRAP=1; \
> trap "rm -rf $(TMPOUT)" EXIT; \
> mkdir -p $(TMPOUT); \
> if ($(1)) >/dev/null 2>&1; \
> @@ -76,7 +77,7 @@ ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
> # __rustc-option
> # Usage: MY_RUSTFLAGS += $(call __rustc-option,$(RUSTC),$(MY_RUSTFLAGS),-Cinstrument-coverage,-Zinstrument-coverage)
> __rustc-option = $(call try-run,\
> - $(1) $(2) $(3) --crate-type=rlib /dev/null --out-dir=$$TMPOUT -o "$$TMP",$(3),$(4))
> + $(1) $(2) $(3) --crate-type=rlib $(srctree)/rust/probe.rs --out-dir=$$TMP,$(3),$(4))
>
> # rustc-option
> # Usage: rustflags-y += $(call rustc-option,-Cinstrument-coverage,-Zinstrument-coverage)
> @@ -86,4 +87,4 @@ rustc-option = $(call __rustc-option, $(RUSTC),\
> # rustc-option-yn
> # Usage: flag := $(call rustc-option-yn,-Cinstrument-coverage)
> rustc-option-yn = $(call try-run,\
> - $(RUSTC) $(KBUILD_RUSTFLAGS) $(1) --crate-type=rlib /dev/null --out-dir=$$TMPOUT -o "$$TMP",y,n)
> + $(RUSTC) $(KBUILD_RUSTFLAGS) $(1) --crate-type=rlib $(srctree)/rust/probe.rs --out-dir=$$TMP,y,n)
>
> ---
> base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
> change-id: 20241008-rustc-option-bootstrap-607e5bf3114c
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>
Err, slight amendment - I think you want `-o $$TMP`, and not
`--out-dir $$TMPOUT`
You definitely don't want `--out-dir $$TMP`, but the other two
settings above would be defensible.
On Tue, Oct 8, 2024 at 10:43 AM Matthew Maurer <mmaurer@google.com> wrote:
>
> Thanks for catching this - I had a specialized toolchain enabled which
> didn't exercise the `RUSTC_BOOTSTRAP` issue, and without an unexpected
> failure, didn't see the `--out-dir` flag conflict after I got the
> request to add it.
>
> Reviewed-By: Matthew Maurer <mmaurer@google.com>
>
>
> On Tue, Oct 8, 2024 at 10:32 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Fix a few different compiler errors that cause rustc-option to give
> > wrong results.
> >
> > If KBUILD_RUSTFLAGS or the flags being tested contain any -Z flags, then
> > the error below is generated. The RUSTC_BOOTSTRAP environment variable
> > is added to fix this error.
> >
> > error: the option `Z` is only accepted on the nightly compiler
> > help: consider switching to a nightly toolchain: `rustup default nightly`
> > note: selecting a toolchain with `+toolchain` arguments require a rustup proxy;
> > see <https://rust-lang.github.io/rustup/concepts/index.html>
> > note: for more information about Rust's stability policy, see
> > <https://doc.rust-lang.org/book/appendix-07-nightly-rust.html#unstable-features>
> > error: 1 nightly option were parsed
> >
> > The probe may also fail incorrectly with the below error message. To fix
> > it, the /dev/null argument is replaced with a new rust/probe.rs file
> > that doesn't need even the core part of the standard library.
> >
> > error[E0463]: can't find crate for `std`
> > |
> > = note: the `aarch64-unknown-none` target may not be installed
> > = help: consider downloading the target with `rustup target add aarch64-unknown-none`
> > = help: consider building the standard library from source with `cargo build -Zbuild-std`
> >
> > The -o and --out-dir parameters are altered to fix this warning:
> >
> > warning: ignoring --out-dir flag due to -o flag
> >
> > I verified that the Kconfig version of rustc-option doesn't have the
> > same issues.
> >
> > Fixes: c42297438aee ("kbuild: rust: Define probing macros for rustc")
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> > Changes in v2:
> > - Add `export` to RUSTC_BOOTSTRAP.
> > - Fix error about core being missing.
> > - Fix warning about -o flag.
> > - Link to v1: https://lore.kernel.org/r/20241008-rustc-option-bootstrap-v1-1-9eb06261d4f7@google.com
> > ---
> > rust/probe.rs | 7 +++++++
> > scripts/Makefile.compiler | 5 +++--
> > 2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/rust/probe.rs b/rust/probe.rs
> > new file mode 100644
> > index 000000000000..bf024e394408
> > --- /dev/null
> > +++ b/rust/probe.rs
> > @@ -0,0 +1,7 @@
> > +//! Nearly empty file passed to rustc-option by Make.
> > +//!
> > +//! The no_core attribute is needed because rustc-option otherwise fails due to
> > +//! not being able to find the core part of the standard library.
> > +
> > +#![feature(no_core)]
> > +#![no_core]
> > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> > index 057305eae85c..08d5b7177ea8 100644
> > --- a/scripts/Makefile.compiler
> > +++ b/scripts/Makefile.compiler
> > @@ -21,6 +21,7 @@ TMPOUT = $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/).tmp_$$$$
> > # automatically cleaned up.
> > try-run = $(shell set -e; \
> > TMP=$(TMPOUT)/tmp; \
> > + export RUSTC_BOOTSTRAP=1; \
> > trap "rm -rf $(TMPOUT)" EXIT; \
> > mkdir -p $(TMPOUT); \
> > if ($(1)) >/dev/null 2>&1; \
> > @@ -76,7 +77,7 @@ ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
> > # __rustc-option
> > # Usage: MY_RUSTFLAGS += $(call __rustc-option,$(RUSTC),$(MY_RUSTFLAGS),-Cinstrument-coverage,-Zinstrument-coverage)
> > __rustc-option = $(call try-run,\
> > - $(1) $(2) $(3) --crate-type=rlib /dev/null --out-dir=$$TMPOUT -o "$$TMP",$(3),$(4))
> > + $(1) $(2) $(3) --crate-type=rlib $(srctree)/rust/probe.rs --out-dir=$$TMP,$(3),$(4))
> >
> > # rustc-option
> > # Usage: rustflags-y += $(call rustc-option,-Cinstrument-coverage,-Zinstrument-coverage)
> > @@ -86,4 +87,4 @@ rustc-option = $(call __rustc-option, $(RUSTC),\
> > # rustc-option-yn
> > # Usage: flag := $(call rustc-option-yn,-Cinstrument-coverage)
> > rustc-option-yn = $(call try-run,\
> > - $(RUSTC) $(KBUILD_RUSTFLAGS) $(1) --crate-type=rlib /dev/null --out-dir=$$TMPOUT -o "$$TMP",y,n)
> > + $(RUSTC) $(KBUILD_RUSTFLAGS) $(1) --crate-type=rlib $(srctree)/rust/probe.rs --out-dir=$$TMP,y,n)
> >
> > ---
> > base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
> > change-id: 20241008-rustc-option-bootstrap-607e5bf3114c
> >
> > Best regards,
> > --
> > Alice Ryhl <aliceryhl@google.com>
> >
On Tue, Oct 8, 2024 at 8:25 PM Matthew Maurer <mmaurer@google.com> wrote: > > Err, slight amendment - I think you want `-o $$TMP`, and not > `--out-dir $$TMPOUT` We need the `--out-dir` to avoid temporary files being created in the current working directory. Cheers, Miguel
On Tue, Oct 8, 2024 at 1:22 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Tue, Oct 8, 2024 at 8:25 PM Matthew Maurer <mmaurer@google.com> wrote: > > > > Err, slight amendment - I think you want `-o $$TMP`, and not > > `--out-dir $$TMPOUT` > > We need the `--out-dir` to avoid temporary files being created in the > current working directory. In that case we should omit `-o $$TMP`, because `rustc` is emitting a warning that it's ignoring `--out-dir` because `-o` is set. > > Cheers, > Miguel
On Tue, Oct 8, 2024 at 10:23 PM Matthew Maurer <mmaurer@google.com> wrote:
>
> In that case we should omit `-o $$TMP`, because `rustc` is emitting a
> warning that it's ignoring `--out-dir` because `-o` is set.
Yes, we need to omit `-o` or we could use `--out-dir` together with
one of the `--emit=X=...` types to be more explicit and avoid building
the `.rlib`, which is what I did in the GitHub issue.
IIRC I used `obj` to run most of the compiler stages just in case that
mattered, but perhaps an even simpler one is good enough, e.g.
`metadata`.
I think just `--out-dir` should be fine and is simpler for this use
case. However, apparently outputting to stdout works too, so we could
do:
--emit=obj=- - >/dev/null
and avoid the output file altogether. We still most likely need the
`--out-dir` in case temporaries are created.
Cheers,
Miguel
On Tue, Oct 8, 2024 at 10:53 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Tue, Oct 8, 2024 at 10:23 PM Matthew Maurer <mmaurer@google.com> wrote:
> >
> > In that case we should omit `-o $$TMP`, because `rustc` is emitting a
> > warning that it's ignoring `--out-dir` because `-o` is set.
>
> Yes, we need to omit `-o` or we could use `--out-dir` together with
> one of the `--emit=X=...` types to be more explicit and avoid building
> the `.rlib`, which is what I did in the GitHub issue.
Miguel, can you link this issue? I don't think I saw it.
> IIRC I used `obj` to run most of the compiler stages just in case that
> mattered, but perhaps an even simpler one is good enough, e.g.
> `metadata`.
>
> I think just `--out-dir` should be fine and is simpler for this use
> case. However, apparently outputting to stdout works too, so we could
> do:
>
> --emit=obj=- - >/dev/null
>
> and avoid the output file altogether. We still most likely need the
> `--out-dir` in case temporaries are created.
Masahiro, are you able to clarify how to pass TMPOUT to rustc?
__rustc-option = $(call try-run2,\
$(1) $(2) $(3) --crate-type=rlib $(srctree)/rust/probe.rs
--out-dir=$(TMPOUT),$(3),$(4))
Should I use $(TMPOUT) or $$TMPOUT for this case? Right now, only TMP
is defined inside try-run. I am assuming that there is a reason for
having TMP be defined in try-run, rather than just using $(TMP)
everywhere. Does the same reason apply to TMPOUT? Should I add a
TMPOUT=$(TMPOUT) inside try-run?
Alice
On Wed, Oct 9, 2024 at 11:23 AM Alice Ryhl <aliceryhl@google.com> wrote: > > Miguel, can you link this issue? I don't think I saw it. https://github.com/Rust-for-Linux/linux/pull/1087#issuecomment-2218445303 (It was in the Lore message I linked yesterday, sorry, I should have been more explicit) > Masahiro, are you able to clarify how to pass TMPOUT to rustc? > > __rustc-option = $(call try-run2,\ > $(1) $(2) $(3) --crate-type=rlib $(srctree)/rust/probe.rs > --out-dir=$(TMPOUT),$(3),$(4)) > > Should I use $(TMPOUT) or $$TMPOUT for this case? Right now, only TMP > is defined inside try-run. I am assuming that there is a reason for > having TMP be defined in try-run, rather than just using $(TMP) > everywhere. Does the same reason apply to TMPOUT? Should I add a > TMPOUT=$(TMPOUT) inside try-run? `TMPOUT` is defined already in that `Makefile`, thus you can directly expand it. However, `TMP` is defined inside the `shell` function, and thus `$$TMP` is used so that that script (inside the `shell`) expands it instead. This is why Masahiro was saying that the `TMPOUT=$(TMPOUT)` was unnecessary, i.e. it would work, but we can just expand it directly. Something like this, combining everything [1] seems to work for me. i.e. passing the file inline, `RUSTC_BOOTSTRAP=1`, avoiding an output file, keeping `--out-dir` for intermediates files. I added using a null sysroot too (and skipping if already given, since that is an error). I will test it a bit more with KASAN etc. Cheers, Miguel [1] diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler index 057305eae85c..3ce6a808764a 100644 --- a/scripts/Makefile.compiler +++ b/scripts/Makefile.compiler @@ -76,7 +76,9 @@ ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3)) # __rustc-option # Usage: MY_RUSTFLAGS += $(call __rustc-option,$(RUSTC),$(MY_RUSTFLAGS),-Cinstrument-coverage,-Zinstrument-coverage) __rustc-option = $(call try-run,\ - $(1) $(2) $(3) --crate-type=rlib /dev/null --out-dir=$$TMPOUT -o "$$TMP",$(3),$(4)) + echo '//!\n#![feature(no_core)]#![no_core]' | RUSTC_BOOTSTRAP=1\ + $(1) --sysroot=/dev/null $(filter-out --sysroot=/dev/null,$(2)) $(3)\ + --crate-type=rlib --out-dir=$(TMPOUT) --emit=obj=- - >/dev/null,$(3),$(4)) # rustc-option # Usage: rustflags-y += $(call rustc-option,-Cinstrument-coverage,-Zinstrument-coverage)
On Wed, Oct 9, 2024 at 7:01 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Wed, Oct 9, 2024 at 11:23 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Miguel, can you link this issue? I don't think I saw it.
>
> https://github.com/Rust-for-Linux/linux/pull/1087#issuecomment-2218445303
>
> (It was in the Lore message I linked yesterday, sorry, I should have
> been more explicit)
>
> > Masahiro, are you able to clarify how to pass TMPOUT to rustc?
> >
> > __rustc-option = $(call try-run2,\
> > $(1) $(2) $(3) --crate-type=rlib $(srctree)/rust/probe.rs
> > --out-dir=$(TMPOUT),$(3),$(4))
> >
> > Should I use $(TMPOUT) or $$TMPOUT for this case? Right now, only TMP
> > is defined inside try-run. I am assuming that there is a reason for
> > having TMP be defined in try-run, rather than just using $(TMP)
> > everywhere. Does the same reason apply to TMPOUT? Should I add a
> > TMPOUT=$(TMPOUT) inside try-run?
>
> `TMPOUT` is defined already in that `Makefile`, thus you can directly
> expand it. However, `TMP` is defined inside the `shell` function, and
> thus `$$TMP` is used so that that script (inside the `shell`) expands
> it instead.
>
> This is why Masahiro was saying that the `TMPOUT=$(TMPOUT)` was
> unnecessary, i.e. it would work, but we can just expand it directly.
>
> Something like this, combining everything [1] seems to work for me.
>
> i.e. passing the file inline, `RUSTC_BOOTSTRAP=1`, avoiding an output
> file, keeping `--out-dir` for intermediates files. I added using a
> null sysroot too (and skipping if already given, since that is an
> error).
>
> I will test it a bit more with KASAN etc.
>
> Cheers,
> Miguel
>
> [1]
>
> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> index 057305eae85c..3ce6a808764a 100644
> --- a/scripts/Makefile.compiler
> +++ b/scripts/Makefile.compiler
> @@ -76,7 +76,9 @@ ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS)
> $(1) -v,$(1),$(2),$(3))
> # __rustc-option
> # Usage: MY_RUSTFLAGS += $(call
> __rustc-option,$(RUSTC),$(MY_RUSTFLAGS),-Cinstrument-coverage,-Zinstrument-coverage)
> __rustc-option = $(call try-run,\
> - $(1) $(2) $(3) --crate-type=rlib /dev/null --out-dir=$$TMPOUT
> -o "$$TMP",$(3),$(4))
> + echo '//!\n#![feature(no_core)]#![no_core]' | RUSTC_BOOTSTRAP=1\
> + $(1) --sysroot=/dev/null $(filter-out
> --sysroot=/dev/null,$(2)) $(3)\
> + --crate-type=rlib --out-dir=$(TMPOUT) --emit=obj=- -
> >/dev/null,$(3),$(4))
>
> # rustc-option
> # Usage: rustflags-y += $(call
> rustc-option,-Cinstrument-coverage,-Zinstrument-coverage)
>
Could you please add a comment line to remember the future clean-up?
e.g.
# TODO: remove RUSTC_BOOTSTRAP=1 when we raise the minimum GNU Make
version to 4.4
I also like the commit description to record that
RUSTC_BOOTSTRAP=1 is needed for GNU Make prier to commit 98da874c4303
("[SV 10593] Export variables to $(shell ...) commands"), i.e.,
GNU Make 4.3 or older.
--
Best Regards
Masahiro Yamada
On Wed, Oct 9, 2024 at 7:01 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Wed, Oct 9, 2024 at 11:23 AM Alice Ryhl <aliceryhl@google.com> wrote: > > > > Miguel, can you link this issue? I don't think I saw it. > > https://github.com/Rust-for-Linux/linux/pull/1087#issuecomment-2218445303 > > (It was in the Lore message I linked yesterday, sorry, I should have > been more explicit) > > > Masahiro, are you able to clarify how to pass TMPOUT to rustc? > > > > __rustc-option = $(call try-run2,\ > > $(1) $(2) $(3) --crate-type=rlib $(srctree)/rust/probe.rs > > --out-dir=$(TMPOUT),$(3),$(4)) > > > > Should I use $(TMPOUT) or $$TMPOUT for this case? Right now, only TMP > > is defined inside try-run. I am assuming that there is a reason for > > having TMP be defined in try-run, rather than just using $(TMP) > > everywhere. Does the same reason apply to TMPOUT? Should I add a > > TMPOUT=$(TMPOUT) inside try-run? > > `TMPOUT` is defined already in that `Makefile`, thus you can directly > expand it. However, `TMP` is defined inside the `shell` function, and > thus `$$TMP` is used so that that script (inside the `shell`) expands > it instead. > > This is why Masahiro was saying that the `TMPOUT=$(TMPOUT)` was > unnecessary, i.e. it would work, but we can just expand it directly. Yes. I like --out-dir=$(TMPOUT) -- Best Regards Masahiro Yamada
On Tue, Oct 8, 2024 at 8:25 PM Matthew Maurer <mmaurer@google.com> wrote: > > Err, slight amendment - I think you want `-o $$TMP`, and not > `--out-dir $$TMPOUT` > > You definitely don't want `--out-dir $$TMP`, but the other two > settings above would be defensible. Ah good point. It seems like the reason that `--out-dir $$TMP` still works is that it creates a sub-directory. But I agree that this is not what we want. Alice
© 2016 - 2024 Red Hat, Inc.