Skip to content

Commit

Permalink
Auto merge of #116892 - ojeda:rethunk, r=wesleywiser
Browse files Browse the repository at this point in the history
Add `-Zfunction-return={keep,thunk-extern}` option

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.
  • Loading branch information
bors committed Nov 30, 2023
2 parents 87e1447 + 2d47622 commit f45631b
Show file tree
Hide file tree
Showing 17 changed files with 216 additions and 10 deletions.
14 changes: 12 additions & 2 deletions compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_codegen_ssa::traits::*;
use rustc_hir::def_id::DefId;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::ty::{self, TyCtxt};
use rustc_session::config::OptLevel;
use rustc_session::config::{FunctionReturn, OptLevel};
use rustc_span::symbol::sym;
use rustc_target::spec::abi::Abi;
use rustc_target::spec::{FramePointer, SanitizerSet, StackProbeType, StackProtector};
Expand Down Expand Up @@ -118,6 +118,15 @@ pub fn frame_pointer_type_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> Option<&'ll Attr
Some(llvm::CreateAttrStringValue(cx.llcx, "frame-pointer", attr_value))
}

fn function_return_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> Option<&'ll Attribute> {
let function_return_attr = match cx.sess().opts.unstable_opts.function_return {
FunctionReturn::Keep => return None,
FunctionReturn::ThunkExtern => AttributeKind::FnRetThunkExtern,
};

Some(function_return_attr.create_attr(cx.llcx))
}

/// Tell LLVM what instrument function to insert.
#[inline]
fn instrument_function_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> SmallVec<[&'ll Attribute; 4]> {
Expand Down Expand Up @@ -331,8 +340,9 @@ pub fn from_fn_attrs<'ll, 'tcx>(
to_add.push(llvm::CreateAttrString(cx.llcx, "use-sample-profile"));
}

// FIXME: none of these three functions interact with source level attributes.
// FIXME: none of these functions interact with source level attributes.
to_add.extend(frame_pointer_type_attr(cx));
to_add.extend(function_return_attr(cx));
to_add.extend(instrument_function_attr(cx));
to_add.extend(nojumptables_attr(cx));
to_add.extend(probestack_attr(cx));
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ pub enum AttributeKind {
AllocatedPointer = 38,
AllocAlign = 39,
SanitizeSafeStack = 40,
FnRetThunkExtern = 41,
}

/// LLVMIntPredicate
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@ use rustc_errors::{emitter::HumanReadableErrorType, registry, ColorConfig};
use rustc_session::config::{
build_configuration, build_session_options, rustc_optgroups, BranchProtection, CFGuard, Cfg,
DebugInfo, DumpMonoStatsFormat, ErrorOutputType, ExternEntry, ExternLocation, Externs,
InliningThreshold, Input, InstrumentCoverage, InstrumentXRay, LinkSelfContained,
LinkerPluginLto, LocationDetail, LtoCli, MirSpanview, OomStrategy, Options, OutFileName,
OutputType, OutputTypes, PAuthKey, PacRet, Passes, Polonius, ProcMacroExecutionStrategy, Strip,
SwitchWithOptPath, SymbolManglingVersion, TraitSolver, WasiExecModel,
FunctionReturn, InliningThreshold, Input, InstrumentCoverage, InstrumentXRay,
LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli, MirSpanview, OomStrategy, Options,
OutFileName, OutputType, OutputTypes, PAuthKey, PacRet, Passes, Polonius,
ProcMacroExecutionStrategy, Strip, SwitchWithOptPath, SymbolManglingVersion, TraitSolver,
WasiExecModel,
};
use rustc_session::lint::Level;
use rustc_session::search_paths::SearchPath;
Expand Down Expand Up @@ -758,6 +759,7 @@ fn test_unstable_options_tracking_hash() {
tracked!(flatten_format_args, false);
tracked!(force_unstable_if_unmarked, true);
tracked!(fuel, Some(("abc".to_string(), 99)));
tracked!(function_return, FunctionReturn::ThunkExtern);
tracked!(function_sections, Some(false));
tracked!(human_readable_cgu_names, true);
tracked!(incremental_ignore_spans, true);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_llvm/llvm-wrapper/LLVMWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ enum LLVMRustAttribute {
AllocatedPointer = 38,
AllocAlign = 39,
SanitizeSafeStack = 40,
FnRetThunkExtern = 41,
};

typedef struct OpaqueRustString *RustStringRef;
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,8 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) {
return Attribute::AllocAlign;
case SanitizeSafeStack:
return Attribute::SafeStack;
case FnRetThunkExtern:
return Attribute::FnRetThunkExtern;
}
report_fatal_error("bad AttributeKind");
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_session/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ session_file_is_not_writeable = output file {$file} is not writeable -- check it
session_file_write_fail = failed to write `{$path}` due to error `{$err}`
session_function_return_requires_x86_or_x86_64 = `-Zfunction-return` (except `keep`) is only supported on x86 and x86_64
session_function_return_thunk_extern_requires_non_large_code_model = `-Zfunction-return=thunk-extern` is only supported on non-large code models
session_hexadecimal_float_literal_not_supported = hexadecimal float literal is not supported
session_incompatible_linker_flavor = linker flavor `{$flavor}` is incompatible with the current target
Expand Down
18 changes: 15 additions & 3 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3164,9 +3164,9 @@ impl PpMode {
pub(crate) mod dep_tracking {
use super::{
BranchProtection, CFGuard, CFProtection, CrateType, DebugInfo, DebugInfoCompression,
ErrorOutputType, InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto,
LocationDetail, LtoCli, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes,
Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm,
ErrorOutputType, FunctionReturn, InliningThreshold, InstrumentCoverage, InstrumentXRay,
LinkerPluginLto, LocationDetail, LtoCli, OomStrategy, OptLevel, OutFileName, OutputType,
OutputTypes, Polonius, RemapPathScopeComponents, ResolveDocLinks, SourceFileHashAlgorithm,
SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion, TraitSolver, TrimmedDefPaths,
};
use crate::lint;
Expand Down Expand Up @@ -3273,6 +3273,7 @@ pub(crate) mod dep_tracking {
TraitSolver,
Polonius,
InliningThreshold,
FunctionReturn,
);

impl<T1, T2> DepTrackingHash for (T1, T2)
Expand Down Expand Up @@ -3451,3 +3452,14 @@ impl Default for InliningThreshold {
Self::Sometimes(100)
}
}

/// The different settings that the `-Zfunction-return` flag can have.
#[derive(Clone, Copy, PartialEq, Hash, Debug, Default)]
pub enum FunctionReturn {
/// Keep the function return unmodified.
#[default]
Keep,

/// Replace returns with jumps to thunk, without emitting the thunk.
ThunkExtern,
}
8 changes: 8 additions & 0 deletions compiler/rustc_session/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,3 +436,11 @@ pub struct IncompatibleLinkerFlavor {
pub flavor: &'static str,
pub compatible_list: String,
}

#[derive(Diagnostic)]
#[diag(session_function_return_requires_x86_or_x86_64)]
pub(crate) struct FunctionReturnRequiresX86OrX8664;

#[derive(Diagnostic)]
#[diag(session_function_return_thunk_extern_requires_non_large_code_model)]
pub(crate) struct FunctionReturnThunkExternRequiresNonLargeCodeModel;
12 changes: 12 additions & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@ mod desc {
pub const parse_inlining_threshold: &str =
"either a boolean (`yes`, `no`, `on`, `off`, etc), or a non-negative number";
pub const parse_llvm_module_flag: &str = "<key>:<type>:<value>:<behavior>. Type must currently be `u32`. Behavior should be one of (`error`, `warning`, `require`, `override`, `append`, `appendunique`, `max`, `min`)";
pub const parse_function_return: &str = "`keep` or `thunk-extern`";
}

mod parse {
Expand Down Expand Up @@ -1359,6 +1360,15 @@ mod parse {
slot.push((key.to_string(), value, behavior));
true
}

pub(crate) fn parse_function_return(slot: &mut FunctionReturn, v: Option<&str>) -> bool {
match v {
Some("keep") => *slot = FunctionReturn::Keep,
Some("thunk-extern") => *slot = FunctionReturn::ThunkExtern,
_ => return false,
}
true
}
}

options! {
Expand Down Expand Up @@ -1603,6 +1613,8 @@ options! {
"force all crates to be `rustc_private` unstable (default: no)"),
fuel: Option<(String, u64)> = (None, parse_optimization_fuel, [TRACKED],
"set the optimization fuel quota for a crate"),
function_return: FunctionReturn = (FunctionReturn::default(), parse_function_return, [TRACKED],
"replace returns with jumps to `__x86_return_thunk` (default: `keep`)"),
function_sections: Option<bool> = (None, parse_opt_bool, [TRACKED],
"whether each function should go in its own section"),
future_incompat_test: bool = (false, parse_bool, [UNTRACKED],
Expand Down
24 changes: 23 additions & 1 deletion compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::code_stats::CodeStats;
pub use crate::code_stats::{DataTypeKind, FieldInfo, FieldKind, SizeKind, VariantInfo};
use crate::config::{
self, CrateType, InstrumentCoverage, OptLevel, OutFileName, OutputType,
self, CrateType, FunctionReturn, InstrumentCoverage, OptLevel, OutFileName, OutputType,
RemapPathScopeComponents, SwitchWithOptPath,
};
use crate::config::{ErrorOutputType, Input};
Expand Down Expand Up @@ -1678,6 +1678,28 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
sess.emit_err(errors::IncompatibleLinkerFlavor { flavor, compatible_list });
}
}

if sess.opts.unstable_opts.function_return != FunctionReturn::default() {
if sess.target.arch != "x86" && sess.target.arch != "x86_64" {
sess.emit_err(errors::FunctionReturnRequiresX86OrX8664);
}
}

// The code model check applies to `thunk` and `thunk-extern`, but not `thunk-inline`, so it is
// kept as a `match` to force a change if new ones are added, even if we currently only support
// `thunk-extern` like Clang.
match sess.opts.unstable_opts.function_return {
FunctionReturn::Keep => (),
FunctionReturn::ThunkExtern => {
// 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 let Some(code_model) = sess.code_model()
&& code_model == CodeModel::Large
{
sess.emit_err(errors::FunctionReturnThunkExternRequiresNonLargeCodeModel);
}
}
}
}

/// Holds data on the current incremental compilation session, if there is one.
Expand Down
25 changes: 25 additions & 0 deletions src/doc/unstable-book/src/compiler-flags/function-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# `function-return`

The tracking issue for this feature is: https://github.com/rust-lang/rust/issues/116853.

------------------------

Option `-Zfunction-return` controls how function returns are converted.

It is equivalent to [Clang]'s and [GCC]'s `-mfunction-return`. The Linux kernel
uses it for RETHUNK builds. For details, see [LLVM commit 2240d72f15f3] ("[X86]
initial -mfunction-return=thunk-extern support") which introduces the feature.

Supported values for this option are:

- `keep`: do not convert function returns.
- `thunk-extern`: convert function returns (`ret`) to jumps (`jmp`)
to an external symbol called `__x86_return_thunk`.

Like in Clang, GCC's values `thunk` and `thunk-inline` are not supported.

Only x86 and non-large code models are supported.

[Clang]: https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-mfunction-return
[GCC]: https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html#index-mfunction-return
[LLVM commit 2240d72f15f3]: https://github.com/llvm/llvm-project/commit/2240d72f15f3b7b9d9fb65450f9bf635fd310f6f
30 changes: 30 additions & 0 deletions tests/assembly/x86_64-function-return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Test that the function return is (not) converted into a jump to the thunk
// when the `-Zfunction-return={keep,thunk-extern}` flag is (not) set.

// revisions: unset keep thunk-extern keep-thunk-extern thunk-extern-keep
// assembly-output: emit-asm
// compile-flags: -O
// [keep] compile-flags: -Zfunction-return=keep
// [thunk-extern] compile-flags: -Zfunction-return=thunk-extern
// [keep-thunk-extern] compile-flags: -Zfunction-return=keep -Zfunction-return=thunk-extern
// [thunk-extern-keep] compile-flags: -Zfunction-return=thunk-extern -Zfunction-return=keep
// only-x86_64
// ignore-x86_64-apple-darwin Symbol is called `___x86_return_thunk` (Darwin's extra underscore)
// ignore-sgx Tests incompatible with LVI mitigations

#![crate_type = "lib"]

// CHECK-LABEL: foo:
#[no_mangle]
pub unsafe fn foo() {
// unset: ret
// unset-NOT: jmp __x86_return_thunk
// keep: ret
// keep-NOT: jmp __x86_return_thunk
// thunk-extern: jmp __x86_return_thunk
// thunk-extern-NOT: ret
// keep-thunk-extern: jmp __x86_return_thunk
// keep-thunk-extern-NOT: ret
// thunk-extern-keep: ret
// thunk-extern-keep-NOT: jmp __x86_return_thunk
}
28 changes: 28 additions & 0 deletions tests/codegen/function-return.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Test that the `fn_ret_thunk_extern` function attribute is (not) emitted when
// the `-Zfunction-return={keep,thunk-extern}` flag is (not) set.

// revisions: unset keep thunk-extern keep-thunk-extern thunk-extern-keep
// needs-llvm-components: x86
// compile-flags: --target x86_64-unknown-linux-gnu
// [keep] compile-flags: -Zfunction-return=keep
// [thunk-extern] compile-flags: -Zfunction-return=thunk-extern
// [keep-thunk-extern] compile-flags: -Zfunction-return=keep -Zfunction-return=thunk-extern
// [thunk-extern-keep] compile-flags: -Zfunction-return=thunk-extern -Zfunction-return=keep

#![crate_type = "lib"]
#![feature(no_core, lang_items)]
#![no_core]

#[lang = "sized"]
trait Sized {}

#[no_mangle]
pub fn foo() {
// CHECK: @foo() unnamed_addr #0

// unset-NOT: fn_ret_thunk_extern
// keep-NOT: fn_ret_thunk_extern
// thunk-extern: attributes #0 = { {{.*}}fn_ret_thunk_extern{{.*}} }
// keep-thunk-extern: attributes #0 = { {{.*}}fn_ret_thunk_extern{{.*}} }
// thunk-extern-keep-NOT: fn_ret_thunk_extern
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: `-Zfunction-return` (except `keep`) is only supported on x86 and x86_64

error: aborting due to 1 previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// revisions: x86 x86_64 aarch64

// compile-flags: -Zfunction-return=thunk-extern

//[x86] check-pass
//[x86] needs-llvm-components: x86
//[x86] compile-flags: --target i686-unknown-linux-gnu

//[x86_64] check-pass
//[x86_64] needs-llvm-components: x86
//[x86_64] compile-flags: --target x86_64-unknown-linux-gnu

//[aarch64] check-fail
//[aarch64] needs-llvm-components: aarch64
//[aarch64] compile-flags: --target aarch64-unknown-linux-gnu
//[aarch64] error-pattern: `-Zfunction-return` (except `keep`) is only supported on x86 and x86_64

#![feature(no_core)]
#![no_core]
#![no_main]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
error: `-Zfunction-return=thunk-extern` is only supported on non-large code models

error: aborting due to 1 previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// revisions: small kernel medium large

// needs-llvm-components: x86
// compile-flags: --target x86_64-unknown-linux-gnu -Zfunction-return=thunk-extern

//[small] check-pass
//[small] compile-flags: -Ccode-model=small

//[kernel] check-pass
//[kernel] compile-flags: -Ccode-model=kernel

//[medium] check-pass
//[medium] compile-flags: -Ccode-model=medium

//[large] check-fail
//[large] compile-flags: -Ccode-model=large
//[large] error-pattern: `-Zfunction-return=thunk-extern` is only supported on non-large code models

#![feature(no_core)]
#![no_core]
#![no_main]

0 comments on commit f45631b

Please sign in to comment.