Skip to content

Commit b1a7dfb

Browse files
borsbors
committed
Auto merge of rust-lang#134082 - davidtwco:forced-inlining, r=saethlin
mir_transform: implement `#[rustc_force_inline]` Adds `#[rustc_force_inline]` which is similar to always inlining but reports an error if the inlining was not possible. - `#[rustc_force_inline]` can only be applied to free functions to guarantee that the MIR inliner will be able to resolve calls. - `rustc_mir_transform::inline::Inline` is refactored into two passes (`Inline` and `ForceInline`), sharing the vast majority of the implementation. - `rustc_mir_transform::inline::ForceInline` can't be disabled so annotated items are always inlined. - `rustc_mir_transform::inline::ForceInline` runs regardless of optimisation level. - `#[rustc_force_inline]` won't inline unless target features match, as with normal inlining. - MIR validation will ICE if a `#[rustc_force_inline]` isn't inlined, to guarantee that it will never be codegened independently. As a further guarantee, monomorphisation collection will always decide that `#[rustc_force_inline]` functions cannot be codegened locally. - Like other intrinsics, `#[rustc_force_inline]` annotated functions cannot be cast to function pointers. - As with other rustc attrs, this cannot be used by users, just within the compiler and standard library. - This is only implemented within rustc, so should avoid any limitations of LLVM's inlining. It is intended that this attribute be used with intrinsics that must be inlined for security reasons. For example, pointer authentication intrinsics would allow Rust users to make use of pointer authentication instructions, but if these intrinsic functions were in the binary then they could be used as gadgets with ROP attacks, defeating the point of introducing them. We don't have any intrinsics like this today, but I expect to upstream some once a force inlining mechanism such as this is available. cc rust-lang#131687 rust-lang/rfcs#3711 - this approach should resolve the concerns from these previous attempts r? `@saethlin`
2 parents 336209e + cc9a9ec commit b1a7dfb

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+2402
-661
lines changed

Diff for: ‎Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -4158,6 +4158,7 @@ dependencies = [
41584158
"rustc_apfloat",
41594159
"rustc_arena",
41604160
"rustc_ast",
4161+
"rustc_attr_parsing",
41614162
"rustc_data_structures",
41624163
"rustc_errors",
41634164
"rustc_fluent_macro",

Diff for: ‎compiler/rustc_attr_data_structures/src/attributes.rs

+16
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,22 @@ pub enum InlineAttr {
1111
Hint,
1212
Always,
1313
Never,
14+
/// `#[rustc_force_inline]` forces inlining to happen in the MIR inliner - it reports an error
15+
/// if the inlining cannot happen. It is limited to only free functions so that the calls
16+
/// can always be resolved.
17+
Force {
18+
attr_span: Span,
19+
reason: Option<Symbol>,
20+
},
21+
}
22+
23+
impl InlineAttr {
24+
pub fn always(&self) -> bool {
25+
match self {
26+
InlineAttr::Always | InlineAttr::Force { .. } => true,
27+
InlineAttr::None | InlineAttr::Hint | InlineAttr::Never => false,
28+
}
29+
}
1430
}
1531

1632
#[derive(Clone, Encodable, Decodable, Debug, PartialEq, Eq, HashStable_Generic)]

Diff for: ‎compiler/rustc_codegen_gcc/src/attributes.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ fn inline_attr<'gcc, 'tcx>(
2020
) -> Option<FnAttribute<'gcc>> {
2121
match inline {
2222
InlineAttr::Hint => Some(FnAttribute::Inline),
23-
InlineAttr::Always => Some(FnAttribute::AlwaysInline),
23+
InlineAttr::Always | InlineAttr::Force { .. } => Some(FnAttribute::AlwaysInline),
2424
InlineAttr::Never => {
2525
if cx.sess().target.arch != "amdgpu" {
2626
Some(FnAttribute::NoInline)

Diff for: ‎compiler/rustc_codegen_llvm/src/attributes.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ fn inline_attr<'ll>(cx: &CodegenCx<'ll, '_>, inline: InlineAttr) -> Option<&'ll
3737
}
3838
match inline {
3939
InlineAttr::Hint => Some(AttributeKind::InlineHint.create_attr(cx.llcx)),
40-
InlineAttr::Always => Some(AttributeKind::AlwaysInline.create_attr(cx.llcx)),
40+
InlineAttr::Always | InlineAttr::Force { .. } => {
41+
Some(AttributeKind::AlwaysInline.create_attr(cx.llcx))
42+
}
4143
InlineAttr::Never => {
4244
if cx.sess().target.arch != "amdgpu" {
4345
Some(AttributeKind::NoInline.create_attr(cx.llcx))

Diff for: ‎compiler/rustc_codegen_ssa/src/codegen_attrs.rs

+32-10
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use rustc_session::parse::feature_err;
1818
use rustc_session::{Session, lint};
1919
use rustc_span::{Ident, Span, sym};
2020
use rustc_target::spec::{SanitizerSet, abi};
21+
use tracing::debug;
2122

2223
use crate::errors;
2324
use crate::target_features::{check_target_feature_trait_unsafe, from_target_feature_attr};
@@ -525,6 +526,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
525526
if !attr.has_name(sym::inline) {
526527
return ia;
527528
}
529+
528530
if attr.is_word() {
529531
InlineAttr::Hint
530532
} else if let Some(ref items) = attr.meta_item_list() {
@@ -547,6 +549,20 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
547549
ia
548550
}
549551
});
552+
codegen_fn_attrs.inline = attrs.iter().fold(codegen_fn_attrs.inline, |ia, attr| {
553+
if !attr.has_name(sym::rustc_force_inline) || !tcx.features().rustc_attrs() {
554+
return ia;
555+
}
556+
557+
if attr.is_word() {
558+
InlineAttr::Force { attr_span: attr.span, reason: None }
559+
} else if let Some(val) = attr.value_str() {
560+
InlineAttr::Force { attr_span: attr.span, reason: Some(val) }
561+
} else {
562+
debug!("`rustc_force_inline` not checked by attribute validation");
563+
ia
564+
}
565+
});
550566

551567
// naked function MUST NOT be inlined! This attribute is required for the rust compiler itself,
552568
// but not for the code generation backend because at that point the naked function will just be
@@ -596,7 +612,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
596612
// is probably a poor usage of `#[inline(always)]` and easily avoided by not using the attribute.
597613
if tcx.features().target_feature_11()
598614
&& tcx.is_closure_like(did.to_def_id())
599-
&& codegen_fn_attrs.inline != InlineAttr::Always
615+
&& !codegen_fn_attrs.inline.always()
600616
{
601617
let owner_id = tcx.parent(did.to_def_id());
602618
if tcx.def_kind(owner_id).has_codegen_attrs() {
@@ -606,22 +622,28 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
606622
}
607623
}
608624

609-
// If a function uses #[target_feature] it can't be inlined into general
625+
// If a function uses `#[target_feature]` it can't be inlined into general
610626
// purpose functions as they wouldn't have the right target features
611-
// enabled. For that reason we also forbid #[inline(always)] as it can't be
627+
// enabled. For that reason we also forbid `#[inline(always)]` as it can't be
612628
// respected.
613-
if !codegen_fn_attrs.target_features.is_empty() && codegen_fn_attrs.inline == InlineAttr::Always
629+
//
630+
// `#[rustc_force_inline]` doesn't need to be prohibited here, only
631+
// `#[inline(always)]`, as forced inlining is implemented entirely within
632+
// rustc (and so the MIR inliner can do any necessary checks for compatible target
633+
// features).
634+
//
635+
// This sidesteps the LLVM blockers in enabling `target_features` +
636+
// `inline(always)` to be used together (see rust-lang/rust#116573 and
637+
// llvm/llvm-project#70563).
638+
if !codegen_fn_attrs.target_features.is_empty()
639+
&& matches!(codegen_fn_attrs.inline, InlineAttr::Always)
614640
{
615641
if let Some(span) = inline_span {
616-
tcx.dcx().span_err(
617-
span,
618-
"cannot use `#[inline(always)]` with \
619-
`#[target_feature]`",
620-
);
642+
tcx.dcx().span_err(span, "cannot use `#[inline(always)]` with `#[target_feature]`");
621643
}
622644
}
623645

624-
if !codegen_fn_attrs.no_sanitize.is_empty() && codegen_fn_attrs.inline == InlineAttr::Always {
646+
if !codegen_fn_attrs.no_sanitize.is_empty() && codegen_fn_attrs.inline.always() {
625647
if let (Some(no_sanitize_span), Some(inline_span)) = (no_sanitize_span, inline_span) {
626648
let hir_id = tcx.local_def_id_to_hir_id(did);
627649
tcx.node_span_lint(

Diff for: ‎compiler/rustc_feature/src/builtin_attrs.rs

+4
Original file line numberDiff line numberDiff line change
@@ -1019,6 +1019,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
10191019
rustc_no_mir_inline, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::Yes,
10201020
"#[rustc_no_mir_inline] prevents the MIR inliner from inlining a function while not affecting codegen"
10211021
),
1022+
rustc_attr!(
1023+
rustc_force_inline, Normal, template!(Word, NameValueStr: "reason"), WarnFollowing, EncodeCrossCrate::Yes,
1024+
"#![rustc_force_inline] forces a free function to be inlined"
1025+
),
10221026

10231027
// ==========================================================================
10241028
// Internal attributes, Testing:

Diff for: ‎compiler/rustc_hir_typeck/src/coercion.rs

+18-1
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
use std::ops::Deref;
3939

4040
use rustc_abi::ExternAbi;
41+
use rustc_attr_parsing::InlineAttr;
4142
use rustc_errors::codes::*;
4243
use rustc_errors::{Applicability, Diag, struct_span_code_err};
4344
use rustc_hir as hir;
@@ -926,8 +927,13 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
926927
return Err(TypeError::IntrinsicCast);
927928
}
928929

929-
// Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396).
930+
let fn_attrs = self.tcx.codegen_fn_attrs(def_id);
931+
if matches!(fn_attrs.inline, InlineAttr::Force { .. }) {
932+
return Err(TypeError::ForceInlineCast);
933+
}
930934

935+
// Safe `#[target_feature]` functions are not assignable to safe fn pointers
936+
// (RFC 2396).
931937
if b_hdr.safety.is_safe()
932938
&& !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty()
933939
{
@@ -1197,6 +1203,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
11971203
return Ok(prev_ty);
11981204
}
11991205

1206+
let is_force_inline = |ty: Ty<'tcx>| {
1207+
if let ty::FnDef(did, _) = ty.kind() {
1208+
matches!(self.tcx.codegen_fn_attrs(did).inline, InlineAttr::Force { .. })
1209+
} else {
1210+
false
1211+
}
1212+
};
1213+
if is_force_inline(prev_ty) || is_force_inline(new_ty) {
1214+
return Err(TypeError::ForceInlineCast);
1215+
}
1216+
12001217
// Special-case that coercion alone cannot handle:
12011218
// Function items or non-capturing closures of differing IDs or GenericArgs.
12021219
let (a_sig, b_sig) = {

Diff for: ‎compiler/rustc_middle/src/mir/mono.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,10 @@ impl<'tcx> MonoItem<'tcx> {
132132
// creating one copy of this `#[inline]` function which may
133133
// conflict with upstream crates as it could be an exported
134134
// symbol.
135-
match tcx.codegen_fn_attrs(instance.def_id()).inline {
136-
InlineAttr::Always => InstantiationMode::LocalCopy,
137-
_ => InstantiationMode::GloballyShared { may_conflict: true },
135+
if tcx.codegen_fn_attrs(instance.def_id()).inline.always() {
136+
InstantiationMode::LocalCopy
137+
} else {
138+
InstantiationMode::GloballyShared { may_conflict: true }
138139
}
139140
}
140141
MonoItem::Static(..) | MonoItem::GlobalAsm(..) => {

Diff for: ‎compiler/rustc_middle/src/ty/error.rs

+3
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ impl<'tcx> TypeError<'tcx> {
109109
TypeError::ConstMismatch(ref values) => {
110110
format!("expected `{}`, found `{}`", values.expected, values.found).into()
111111
}
112+
TypeError::ForceInlineCast => {
113+
"cannot coerce functions which must be inlined to function pointers".into()
114+
}
112115
TypeError::IntrinsicCast => "cannot coerce intrinsics to function pointers".into(),
113116
TypeError::TargetFeatureCast(_) => {
114117
"cannot coerce functions with `#[target_feature]` to safe function pointers".into()

Diff for: ‎compiler/rustc_mir_build/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ rustc_abi = { path = "../rustc_abi" }
1212
rustc_apfloat = "0.2.0"
1313
rustc_arena = { path = "../rustc_arena" }
1414
rustc_ast = { path = "../rustc_ast" }
15+
rustc_attr_parsing = { path = "../rustc_attr_parsing" }
1516
rustc_data_structures = { path = "../rustc_data_structures" }
1617
rustc_errors = { path = "../rustc_errors" }
1718
rustc_fluent_macro = { path = "../rustc_fluent_macro" }

Diff for: ‎compiler/rustc_mir_build/messages.ftl

+6
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,12 @@ mir_build_extern_static_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
118118
.note = extern statics are not controlled by the Rust type system: invalid data, aliasing violations or data races will cause undefined behavior
119119
.label = use of extern static
120120
121+
mir_build_force_inline =
122+
`{$callee}` is incompatible with `#[rustc_force_inline]`
123+
.attr = annotation here
124+
.callee = `{$callee}` defined here
125+
.note = incompatible due to: {$reason}
126+
121127
mir_build_inform_irrefutable = `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
122128
123129
mir_build_initializing_type_with_requires_unsafe =

Diff for: ‎compiler/rustc_mir_build/src/builder/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use rustc_span::{Span, Symbol, sym};
2929
use super::lints;
3030
use crate::builder::expr::as_place::PlaceBuilder;
3131
use crate::builder::scope::DropKind;
32+
use crate::check_inline;
3233

3334
pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
3435
tcx: TyCtxt<'tcx>,
@@ -80,6 +81,7 @@ pub(crate) fn mir_build<'tcx>(tcx: TyCtxtAt<'tcx>, def: LocalDefId) -> Body<'tcx
8081
};
8182

8283
lints::check(tcx, &body);
84+
check_inline::check_force_inline(tcx, &body);
8385

8486
// The borrow checker will replace all the regions here with its own
8587
// inference variables. There's no point having non-erased regions here.

Diff for: ‎compiler/rustc_mir_build/src/check_inline.rs

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
use rustc_attr_parsing::InlineAttr;
2+
use rustc_hir::def_id::DefId;
3+
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
4+
use rustc_middle::mir::{Body, TerminatorKind};
5+
use rustc_middle::ty;
6+
use rustc_middle::ty::TyCtxt;
7+
use rustc_span::sym;
8+
9+
/// Check that a body annotated with `#[rustc_force_inline]` will not fail to inline based on its
10+
/// definition alone (irrespective of any specific caller).
11+
pub(crate) fn check_force_inline<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
12+
let def_id = body.source.def_id();
13+
if !tcx.hir().body_owner_kind(def_id).is_fn_or_closure() || !def_id.is_local() {
14+
return;
15+
}
16+
let InlineAttr::Force { attr_span, .. } = tcx.codegen_fn_attrs(def_id).inline else {
17+
return;
18+
};
19+
20+
if let Err(reason) =
21+
is_inline_valid_on_fn(tcx, def_id).and_then(|_| is_inline_valid_on_body(tcx, body))
22+
{
23+
tcx.dcx().emit_err(crate::errors::InvalidForceInline {
24+
attr_span,
25+
callee_span: tcx.def_span(def_id),
26+
callee: tcx.def_path_str(def_id),
27+
reason,
28+
});
29+
}
30+
}
31+
32+
pub fn is_inline_valid_on_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Result<(), &'static str> {
33+
let codegen_attrs = tcx.codegen_fn_attrs(def_id);
34+
if tcx.has_attr(def_id, sym::rustc_no_mir_inline) {
35+
return Err("#[rustc_no_mir_inline]");
36+
}
37+
38+
// FIXME(#127234): Coverage instrumentation currently doesn't handle inlined
39+
// MIR correctly when Modified Condition/Decision Coverage is enabled.
40+
if tcx.sess.instrument_coverage_mcdc() {
41+
return Err("incompatible with MC/DC coverage");
42+
}
43+
44+
let ty = tcx.type_of(def_id);
45+
if match ty.instantiate_identity().kind() {
46+
ty::FnDef(..) => tcx.fn_sig(def_id).instantiate_identity().c_variadic(),
47+
ty::Closure(_, args) => args.as_closure().sig().c_variadic(),
48+
_ => false,
49+
} {
50+
return Err("C variadic");
51+
}
52+
53+
if codegen_attrs.flags.contains(CodegenFnAttrFlags::COLD) {
54+
return Err("cold");
55+
}
56+
57+
// Intrinsic fallback bodies are automatically made cross-crate inlineable,
58+
// but at this stage we don't know whether codegen knows the intrinsic,
59+
// so just conservatively don't inline it. This also ensures that we do not
60+
// accidentally inline the body of an intrinsic that *must* be overridden.
61+
if tcx.has_attr(def_id, sym::rustc_intrinsic) {
62+
return Err("callee is an intrinsic");
63+
}
64+
65+
Ok(())
66+
}
67+
68+
pub fn is_inline_valid_on_body<'tcx>(
69+
_: TyCtxt<'tcx>,
70+
body: &Body<'tcx>,
71+
) -> Result<(), &'static str> {
72+
if body
73+
.basic_blocks
74+
.iter()
75+
.any(|bb| matches!(bb.terminator().kind, TerminatorKind::TailCall { .. }))
76+
{
77+
return Err("can't inline functions with tail calls");
78+
}
79+
80+
Ok(())
81+
}

Diff for: ‎compiler/rustc_mir_build/src/errors.rs

+12
Original file line numberDiff line numberDiff line change
@@ -1107,3 +1107,15 @@ impl<'a> Subdiagnostic for Rust2024IncompatiblePatSugg<'a> {
11071107
);
11081108
}
11091109
}
1110+
1111+
#[derive(Diagnostic)]
1112+
#[diag(mir_build_force_inline)]
1113+
#[note]
1114+
pub(crate) struct InvalidForceInline {
1115+
#[primary_span]
1116+
pub attr_span: Span,
1117+
#[label(mir_build_callee)]
1118+
pub callee_span: Span,
1119+
pub callee: String,
1120+
pub reason: &'static str,
1121+
}

Diff for: ‎compiler/rustc_mir_build/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
// "Go to file" feature to silently ignore all files in the module, probably
1616
// because it assumes that "build" is a build-output directory. See #134365.
1717
mod builder;
18+
pub mod check_inline;
1819
mod check_tail_calls;
1920
mod check_unsafety;
2021
mod errors;

Diff for: ‎compiler/rustc_mir_transform/messages.ftl

+11
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@ mir_transform_ffi_unwind_call = call to {$foreign ->
1919
mir_transform_fn_item_ref = taking a reference to a function item does not give a function pointer
2020
.suggestion = cast `{$ident}` to obtain a function pointer
2121
22+
mir_transform_force_inline =
23+
`{$callee}` could not be inlined into `{$caller}` but is required to be inlined
24+
.call = ...`{$callee}` called here
25+
.attr = inlining due to this annotation
26+
.caller = within `{$caller}`...
27+
.callee = `{$callee}` defined here
28+
.note = could not be inlined due to: {$reason}
29+
30+
mir_transform_force_inline_justification =
31+
`{$callee}` is required to be inlined to: {$sym}
32+
2233
mir_transform_must_not_suspend = {$pre}`{$def_path}`{$post} held across a suspend point, but should not be
2334
.label = the value is held across this suspend point
2435
.note = {$reason}

Diff for: ‎compiler/rustc_mir_transform/src/cross_crate_inline.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ fn cross_crate_inlinable(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
4646
// #[inline(never)] to force code generation.
4747
match codegen_fn_attrs.inline {
4848
InlineAttr::Never => return false,
49-
InlineAttr::Hint | InlineAttr::Always => return true,
49+
InlineAttr::Hint | InlineAttr::Always | InlineAttr::Force { .. } => return true,
5050
_ => {}
5151
}
5252

@@ -69,8 +69,9 @@ fn cross_crate_inlinable(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
6969
// Don't do any inference if codegen optimizations are disabled and also MIR inlining is not
7070
// enabled. This ensures that we do inference even if someone only passes -Zinline-mir,
7171
// which is less confusing than having to also enable -Copt-level=1.
72-
if matches!(tcx.sess.opts.optimize, OptLevel::No) && !pm::should_run_pass(tcx, &inline::Inline)
73-
{
72+
let inliner_will_run = pm::should_run_pass(tcx, &inline::Inline)
73+
|| inline::ForceInline::should_run_pass_for_callee(tcx, def_id.to_def_id());
74+
if matches!(tcx.sess.opts.optimize, OptLevel::No) && !inliner_will_run {
7475
return false;
7576
}
7677

0 commit comments

Comments
 (0)