Add `-Zfunction-return={keep,thunk-extern}` option by ojeda · Pull Request #116892 · rust-lang/rust

ojeda · 2023-10-18T15:10:49Z

This is intended to be used for Linux kernel RETHUNK builds.

With this commit (optionally backported to Rust 1.73.0), plus a patched Linux kernel to pass the flag, I get a RETHUNK build with Rust enabled that is objtool-warning-free and is able to boot in QEMU and load a sample Rust kernel module.

Issue: #116853.

rustbot · 2023-10-18T15:11:00Z

r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

ojeda · 2023-10-18T15:24:26Z

Note that LLVM ignores the attribute in non-x86, but I have not replicated Clang's check that this only works in x86 for the CLI flag for the moment (should it be part of the target spec?).

Also, I linked the feature request issue as the tracking one in the docs -- we can reuse it changing the OP message, I assume, but please let me know if it is best to create a new one.

compiler/rustc_interface/src/tests.rs

compiler/rustc_session/src/config.rs

compiler/rustc_codegen_llvm/src/attributes.rs

src/doc/unstable-book/src/compiler-flags/function-return.md

ojeda · 2023-10-19T22:25:43Z

compiler/rustc_session/src/session.rs

+            // FIXME: In principle, the inherited base LLVM target code model could be large,
+            // but this only checks whether we were passed one explicitly (like Clang does).


If there is a way to get the effective code model, it would be nice to use it.

Maybe check sess.target.options.code_model ?

That covers even less, no? i.e. code_model() covers codegen and target options.

ojeda · 2023-10-19T22:43:51Z

I added a folder but that still counts against the tidy limit in tests/ui. Should I move the folder to e.g. tests/ui/invalid-compile-flags or increase the limit by 1?

ojeda · 2023-10-20T11:51:01Z

One more note: GCC and Clang give an error if one tries to use function-return at all, i.e. even with keep, on non-x86. However, in this PR keep is allowed for all architectures.

I did it like this because it seemed simpler (no Option) and more flexible, especially since (in the case of this flag) other architectures could support particular modes, e.g. s390 expolines (https://github.com/llvm/llvm-project/blob/49af6502c6dcb4a7f7520178bd14df396f78240c/clang/lib/Frontend/CompilerInvocation.cpp#L1942).

But perhaps consistency with GCC/Clang is better?

ojeda · 2023-10-20T13:00:57Z

I added a folder but that still counts against the tidy limit in tests/ui. Should I move the folder to e.g. tests/ui/invalid-compile-flags or increase the limit by 1?

I moved them inside the other folder, and noticed one in invalid/ should be in that invalid-compile-flags/ folder: #116975.

The Rust compiler does not support the equivalent of `-mfunction-return=thunk-extern` yet [1]. Thus, currently, `objtool` warns about it, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build The support in `rustc` for `-Zfunction-return` has been submitted and is being reviewed [2]. It adds the needed LLVM function attributes and, with it, I got a RETHUNK kernel build with Rust enabled that does not print the `objtool` related warnings, boots in QEMU and can load a kernel loadable module. In any case, until proper/complete support is added to `rustc`, make it a hard restriction until the mitigation is in place. This may have an impact for developers that may not need/care about the mitigation in the Rust side (e.g. Ubuntu offers Rust as a "technology preview" [3]), but given we are getting closer to having the first actual in-tree Rust kernel users, it seems like the right time to disallow it. This should also avoid confusion [4]. Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Link: https://lore.kernel.org/rust-for-linux/ZSQXqX2%2Flhf5ICZP@gpd/ [3] Link: https://lore.kernel.org/rust-for-linux/CANiq72n6DMeXQrgOzS_+3VdgNYAmpcnneAHJnZERUQhMExg+0A@mail.gmail.com/ [4] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

When support for `-Zfunction-return` lands in Rust [1], this patch may be used to enable RETHUNK support on top of the previous patch. Link: rust-lang/rust#116892 [1] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The Rust compiler does not support the equivalent of `-mfunction-return=thunk-extern` yet [1]. Thus, currently, `objtool` warns about it, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build The support in `rustc` for `-Zfunction-return` has been submitted and is being reviewed [2]. It adds the needed LLVM function attributes and, with it, I got a RETHUNK kernel build with Rust enabled that does not print the `objtool` related warnings, boots in QEMU and can load a kernel loadable module. In any case, until proper/complete support is added to `rustc`, make it a hard restriction until the mitigation is in place. This may have an impact for developers that may not need/care about the mitigation in the Rust side (e.g. Ubuntu offers Rust as a "technology preview" [3]), but given we are getting closer to having the first actual in-tree Rust kernel users, it seems like the right time to disallow it. This should also avoid confusion [4]. Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Link: https://lore.kernel.org/rust-for-linux/ZSQXqX2%2Flhf5ICZP@gpd/ [3] Link: https://lore.kernel.org/rust-for-linux/CANiq72n6DMeXQrgOzS_+3VdgNYAmpcnneAHJnZERUQhMExg+0A@mail.gmail.com/ [4] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

When support for `-Zfunction-return` lands in Rust [1], this patch may be used to enable RETHUNK support on top of the previous patch. Link: rust-lang/rust#116892 [1] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

bors · 2023-10-30T21:39:27Z

☔ The latest upstream changes (presumably #117405) made this pull request unmergeable. Please resolve the merge conflicts.

ojeda · 2023-10-31T07:51:57Z

Resolved trivial formatting conflict.

wesleywiser · 2023-11-01T21:18:31Z

Thanks for already getting the tracking issue set up @ojeda!

@bors r+

ojeda · 2023-12-01T12:01:54Z

@wesleywiser Thanks for the very quick review and have a nice holiday (I saw the other PR by chance when looking for this one :)

wesleywiser · 2023-12-01T12:12:04Z

Thanks! ☺️

The Rust compiler does not support the equivalent of `-mfunction-return=thunk-extern` yet [1]. Thus, currently, `objtool` warns about it, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build The support in `rustc` for `-Zfunction-return` has been submitted and is being reviewed [2]. It adds the needed LLVM function attributes and, with it, I got a RETHUNK kernel build with Rust enabled that does not print the `objtool` related warnings, boots in QEMU and can load a kernel loadable module. In any case, until proper/complete support is added to `rustc`, make it a hard restriction until the mitigation is in place. This may have an impact for developers that may not need/care about the mitigation in the Rust side (e.g. Ubuntu offers Rust as a "technology preview" [3]), but given we are getting closer to having the first actual in-tree Rust kernel users, it seems like the right time to disallow it. This should also avoid confusion [4]. Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Link: https://lore.kernel.org/rust-for-linux/ZSQXqX2%2Flhf5ICZP@gpd/ [3] Link: https://lore.kernel.org/rust-for-linux/CANiq72n6DMeXQrgOzS_+3VdgNYAmpcnneAHJnZERUQhMExg+0A@mail.gmail.com/ [4] Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Acked-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

When support for `-Zfunction-return` lands in Rust [1], this patch may be used to enable RETHUNK support on top of the previous patch. Link: rust-lang/rust#116892 [1] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The Rust compiler does not support the equivalent of `-mfunction-return=thunk-extern` yet [1]. Thus, currently, `objtool` warns about it, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build The support in `rustc` for `-Zfunction-return` has been submitted and is being reviewed [2]. It adds the needed LLVM function attributes and, with it, I got a RETHUNK kernel build with Rust enabled that does not print the `objtool` related warnings, boots in QEMU and can load a kernel loadable module. In any case, until proper/complete support is added to `rustc`, make it a hard restriction until the mitigation is in place. This may have an impact for developers that may not need/care about the mitigation in the Rust side (e.g. Ubuntu offers Rust as a "technology preview" [3]), but given we are getting closer to having the first actual in-tree Rust kernel users, it seems like the right time to disallow it. This should also avoid confusion [4]. Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Link: https://lore.kernel.org/rust-for-linux/ZSQXqX2%2Flhf5ICZP@gpd/ [3] Link: https://lore.kernel.org/rust-for-linux/CANiq72n6DMeXQrgOzS_+3VdgNYAmpcnneAHJnZERUQhMExg+0A@mail.gmail.com/ [4] Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Acked-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>

When support for `-Zfunction-return` lands in Rust [1], this patch may be used to enable RETHUNK support on top of the previous patch. Link: rust-lang/rust#116892 [1] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The equivalent of `-mfunction-return=thunk-extern` is not passed for Rust. Thus, currently, `objtool` warns about it, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2]. Thus add support for `MITIGATION_RETHUNK`. Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`. Thus add support for `MITIGATION_RETHUNK`. Without this, `objtool` would warn if enabled for Rust and already warns under IBT builds, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`. Thus add support for `MITIGATION_RETHUNK`. Without this, `objtool` would warn if enabled for Rust and already warns under IBT builds, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Reviewed-by: Gary Guo <gary@garyguo.net> Tested-by: Alice Ryhl <aliceryhl@google.com> Tested-by: Benno Lossin <benno.lossin@proton.me> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`. Thus add support for `MITIGATION_RETHUNK`. Without this, `objtool` would warn if enabled for Rust and already warns under IBT builds, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Reviewed-by: Gary Guo <gary@garyguo.net> Tested-by: Alice Ryhl <aliceryhl@google.com> Tested-by: Benno Lossin <benno.lossin@proton.me> Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`. Thus add support for `MITIGATION_RETHUNK`. Without this, `objtool` would warn if enabled for Rust and already warns under IBT builds, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Reviewed-by: Gary Guo <gary@garyguo.net> Tested-by: Alice Ryhl <aliceryhl@google.com> Tested-by: Benno Lossin <benno.lossin@proton.me> Change-Id: If366c0b0334ca372d91711a70e177290da092635 Signed-off-by: Miguel Ojeda <ojeda@kernel.org> Link: https://lore.kernel.org/r/20240725183325.122827-4-ojeda@kernel.org Signed-off-by: Alice Ryhl <aliceryhl@google.com>

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`. Thus add support for `MITIGATION_RETHUNK`. Without this, `objtool` would warn if enabled for Rust and already warns under IBT builds, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Reviewed-by: Gary Guo <gary@garyguo.net> Tested-by: Alice Ryhl <aliceryhl@google.com> Tested-by: Benno Lossin <benno.lossin@proton.me> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`. Thus add support for `MITIGATION_RETHUNK`. Without this, `objtool` would warn if enabled for Rust and already warns under IBT builds, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Reviewed-by: Gary Guo <gary@garyguo.net> Tested-by: Alice Ryhl <aliceryhl@google.com> Tested-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Link: https://lore.kernel.org/r/20240725183325.122827-4-ojeda@kernel.org Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`. Thus add support for `MITIGATION_RETHUNK`. Without this, `objtool` would warn if enabled for Rust and already warns under IBT builds, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Reviewed-by: Gary Guo <gary@garyguo.net> Tested-by: Alice Ryhl <aliceryhl@google.com> Tested-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Link: Rust-for-Linux#945 Link: https://lore.kernel.org/r/20240725183325.122827-4-ojeda@kernel.org Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`. Thus add support for `MITIGATION_RETHUNK`. Without this, `objtool` would warn if enabled for Rust and already warns under IBT builds, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Reviewed-by: Gary Guo <gary@garyguo.net> Tested-by: Alice Ryhl <aliceryhl@google.com> Tested-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Link: #945 Link: https://lore.kernel.org/r/20240725183325.122827-4-ojeda@kernel.org Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1] in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`. Thus add support for `MITIGATION_RETHUNK`. Without this, `objtool` would warn if enabled for Rust and already warns under IBT builds, e.g.: samples/rust/rust_print.o: warning: objtool: _R...init+0xa5c: 'naked' return found in RETHUNK build Link: rust-lang/rust#116853 [1] Link: rust-lang/rust#116892 [2] Reviewed-by: Gary Guo <gary@garyguo.net> Tested-by: Alice Ryhl <aliceryhl@google.com> Tested-by: Benno Lossin <benno.lossin@proton.me> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Link: Rust-for-Linux#945 Link: https://lore.kernel.org/r/20240725183325.122827-4-ojeda@kernel.org Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

…sleywiser Add missing module flags for `-Zfunction-return=thunk-extern` This fixes a bug in the `-Zfunction-return=thunk-extern` flag. The flag needs to be passed onto LLVM to ensure that functions such as `asan.module_ctor` and `asan.module_dtor` that are created internally in LLVM have the mitigation applied to them. This was originally discovered [in the Linux kernel](https://lore.kernel.org/all/CANiq72myZL4_poCMuNFevtpYYc0V0embjSuKb7y=C+m3vVA_8g@mail.gmail.com/). Original flag PR: rust-lang#116892 PR for similar issue: rust-lang#129373 Tracking issue: rust-lang#116853 cc `@ojeda` r? `@wesleywiser`

…sleywiser Add missing module flags for `-Zfunction-return=thunk-extern` This fixes a bug in the `-Zfunction-return=thunk-extern` flag. The flag needs to be passed onto LLVM to ensure that functions such as `asan.module_ctor` and `asan.module_dtor` that are created internally in LLVM have the mitigation applied to them. This was originally discovered [in the Linux kernel](https://lore.kernel.org/all/CANiq72myZL4_poCMuNFevtpYYc0V0embjSuKb7y=C+m3vVA_8g@mail.gmail.com/). Original flag PR: rust-lang#116892 PR for similar issue: rust-lang#129373 Tracking issue: rust-lang#116853 cc ``@ojeda`` r? ``@wesleywiser``

Rollup merge of rust-lang#130824 - Darksonn:fix-function-return, r=wesleywiser Add missing module flags for `-Zfunction-return=thunk-extern` This fixes a bug in the `-Zfunction-return=thunk-extern` flag. The flag needs to be passed onto LLVM to ensure that functions such as `asan.module_ctor` and `asan.module_dtor` that are created internally in LLVM have the mitigation applied to them. This was originally discovered [in the Linux kernel](https://lore.kernel.org/all/CANiq72myZL4_poCMuNFevtpYYc0V0embjSuKb7y=C+m3vVA_8g@mail.gmail.com/). Original flag PR: rust-lang#116892 PR for similar issue: rust-lang#129373 Tracking issue: rust-lang#116853 cc ``@ojeda`` r? ``@wesleywiser``

rustbot assigned wesleywiser Oct 18, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 18, 2023

This comment has been minimized.

Sign in to view

ojeda force-pushed the rethunk branch from 3cf45d6 to a53de9b Compare October 18, 2023 15:19

ojeda mentioned this pull request Oct 18, 2023

rustc wanted features & bugfixes Rust-for-Linux/linux#355

Open

65 tasks

ojeda mentioned this pull request Oct 18, 2023

Rust unstable features needed for the kernel Rust-for-Linux/linux#2

Open

Noratrieb reviewed Oct 18, 2023

View reviewed changes

compiler/rustc_interface/src/tests.rs Outdated Show resolved Hide resolved

compiler/rustc_session/src/config.rs Show resolved Hide resolved

klensy reviewed Oct 18, 2023

View reviewed changes

compiler/rustc_codegen_llvm/src/attributes.rs Outdated Show resolved Hide resolved

ojeda force-pushed the rethunk branch from a53de9b to 6a0ae7d Compare October 18, 2023 15:40

bjorn3 reviewed Oct 18, 2023

View reviewed changes

src/doc/unstable-book/src/compiler-flags/function-return.md Outdated Show resolved Hide resolved

ojeda force-pushed the rethunk branch 2 times, most recently from 8df5004 to 8b6016a Compare October 19, 2023 22:25

ojeda commented Oct 19, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

ojeda force-pushed the rethunk branch from 8b6016a to 605760e Compare October 20, 2023 12:49

ojeda force-pushed the rethunk branch from 605760e to e4f42de Compare October 31, 2023 07:39

wesleywiser approved these changes Nov 1, 2023

View reviewed changes

ojeda deleted the rethunk branch December 1, 2023 11:59

ojeda mentioned this pull request Aug 10, 2024

Tracking Issue for -Zfunction-return #116853

Open

3 tasks

Darksonn mentioned this pull request Sep 25, 2024

Add missing module flags for -Zfunction-return=thunk-extern #130824

Merged

		// FIXME: In principle, the inherited base LLVM target code model could be large,
		// but this only checks whether we were passed one explicitly (like Clang does).

Uh oh!

Conversation

ojeda commented Oct 18, 2023

Uh oh!

rustbot commented Oct 18, 2023

Uh oh!

This comment has been minimized.

ojeda commented Oct 18, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ojeda Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

Urgau Oct 19, 2023

Choose a reason for hiding this comment

Uh oh!

ojeda Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

ojeda commented Oct 19, 2023

Uh oh!

ojeda commented Oct 20, 2023

Uh oh!

ojeda commented Oct 20, 2023

Uh oh!

bors commented Oct 30, 2023

Uh oh!

ojeda commented Oct 31, 2023

Uh oh!

wesleywiser commented Nov 1, 2023

Uh oh!

ojeda commented Dec 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wesleywiser commented Dec 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

ojeda commented Dec 1, 2023 •

edited

Loading