Skip to content
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

Add -Zfunction-return={keep,thunk-extern} option #116892

Merged
merged 2 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
ojeda marked this conversation as resolved.
Show resolved Hide resolved
#[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).
Comment on lines +1694 to +1695
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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
ojeda marked this conversation as resolved.
Show resolved Hide resolved
// unset-NOT: jmp __x86_return_thunk
// keep: ret
ojeda marked this conversation as resolved.
Show resolved Hide resolved
// 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
ojeda marked this conversation as resolved.
Show resolved Hide resolved
// 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]
Loading