Skip to content

Commit

Permalink
mir_build: check annotated functions w/out callers
Browse files Browse the repository at this point in the history
  • Loading branch information
davidtwco committed Jan 10, 2025
1 parent ce602ac commit cc9a9ec
Show file tree
Hide file tree
Showing 16 changed files with 223 additions and 98 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4158,6 +4158,7 @@ dependencies = [
"rustc_apfloat",
"rustc_arena",
"rustc_ast",
"rustc_attr_parsing",
"rustc_data_structures",
"rustc_errors",
"rustc_fluent_macro",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ rustc_abi = { path = "../rustc_abi" }
rustc_apfloat = "0.2.0"
rustc_arena = { path = "../rustc_arena" }
rustc_ast = { path = "../rustc_ast" }
rustc_attr_parsing = { path = "../rustc_attr_parsing" }
rustc_data_structures = { path = "../rustc_data_structures" }
rustc_errors = { path = "../rustc_errors" }
rustc_fluent_macro = { path = "../rustc_fluent_macro" }
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,12 @@ mir_build_extern_static_requires_unsafe_unsafe_op_in_unsafe_fn_allowed =
.note = extern statics are not controlled by the Rust type system: invalid data, aliasing violations or data races will cause undefined behavior
.label = use of extern static
mir_build_force_inline =
`{$callee}` is incompatible with `#[rustc_force_inline]`
.attr = annotation here
.callee = `{$callee}` defined here
.note = incompatible due to: {$reason}
mir_build_inform_irrefutable = `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
mir_build_initializing_type_with_requires_unsafe =
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_mir_build/src/builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use rustc_span::{Span, Symbol, sym};
use super::lints;
use crate::builder::expr::as_place::PlaceBuilder;
use crate::builder::scope::DropKind;
use crate::check_inline;

pub(crate) fn closure_saved_names_of_captured_variables<'tcx>(
tcx: TyCtxt<'tcx>,
Expand Down Expand Up @@ -80,6 +81,7 @@ pub(crate) fn mir_build<'tcx>(tcx: TyCtxtAt<'tcx>, def: LocalDefId) -> Body<'tcx
};

lints::check(tcx, &body);
check_inline::check_force_inline(tcx, &body);

// The borrow checker will replace all the regions here with its own
// inference variables. There's no point having non-erased regions here.
Expand Down
81 changes: 81 additions & 0 deletions compiler/rustc_mir_build/src/check_inline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
use rustc_attr_parsing::InlineAttr;
use rustc_hir::def_id::DefId;
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::{Body, TerminatorKind};
use rustc_middle::ty;
use rustc_middle::ty::TyCtxt;
use rustc_span::sym;

/// Check that a body annotated with `#[rustc_force_inline]` will not fail to inline based on its
/// definition alone (irrespective of any specific caller).
pub(crate) fn check_force_inline<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) {
let def_id = body.source.def_id();
if !tcx.hir().body_owner_kind(def_id).is_fn_or_closure() || !def_id.is_local() {
return;
}
let InlineAttr::Force { attr_span, .. } = tcx.codegen_fn_attrs(def_id).inline else {
return;
};

if let Err(reason) =
is_inline_valid_on_fn(tcx, def_id).and_then(|_| is_inline_valid_on_body(tcx, body))
{
tcx.dcx().emit_err(crate::errors::InvalidForceInline {
attr_span,
callee_span: tcx.def_span(def_id),
callee: tcx.def_path_str(def_id),
reason,
});
}
}

pub fn is_inline_valid_on_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Result<(), &'static str> {
let codegen_attrs = tcx.codegen_fn_attrs(def_id);
if tcx.has_attr(def_id, sym::rustc_no_mir_inline) {
return Err("#[rustc_no_mir_inline]");
}

// FIXME(#127234): Coverage instrumentation currently doesn't handle inlined
// MIR correctly when Modified Condition/Decision Coverage is enabled.
if tcx.sess.instrument_coverage_mcdc() {
return Err("incompatible with MC/DC coverage");
}

let ty = tcx.type_of(def_id);
if match ty.instantiate_identity().kind() {
ty::FnDef(..) => tcx.fn_sig(def_id).instantiate_identity().c_variadic(),
ty::Closure(_, args) => args.as_closure().sig().c_variadic(),
_ => false,
} {
return Err("C variadic");
}

if codegen_attrs.flags.contains(CodegenFnAttrFlags::COLD) {
return Err("cold");
}

// Intrinsic fallback bodies are automatically made cross-crate inlineable,
// but at this stage we don't know whether codegen knows the intrinsic,
// so just conservatively don't inline it. This also ensures that we do not
// accidentally inline the body of an intrinsic that *must* be overridden.
if tcx.has_attr(def_id, sym::rustc_intrinsic) {
return Err("callee is an intrinsic");
}

Ok(())
}

pub fn is_inline_valid_on_body<'tcx>(
_: TyCtxt<'tcx>,
body: &Body<'tcx>,
) -> Result<(), &'static str> {
if body
.basic_blocks
.iter()
.any(|bb| matches!(bb.terminator().kind, TerminatorKind::TailCall { .. }))
{
return Err("can't inline functions with tail calls");
}

Ok(())
}
12 changes: 12 additions & 0 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1107,3 +1107,15 @@ impl<'a> Subdiagnostic for Rust2024IncompatiblePatSugg<'a> {
);
}
}

#[derive(Diagnostic)]
#[diag(mir_build_force_inline)]
#[note]
pub(crate) struct InvalidForceInline {
#[primary_span]
pub attr_span: Span,
#[label(mir_build_callee)]
pub callee_span: Span,
pub callee: String,
pub reason: &'static str,
}
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
// "Go to file" feature to silently ignore all files in the module, probably
// because it assumes that "build" is a build-output directory. See #134365.
mod builder;
pub mod check_inline;
mod check_tail_calls;
mod check_unsafety;
mod errors;
Expand Down
52 changes: 7 additions & 45 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ use rustc_hir::def_id::DefId;
use rustc_index::Idx;
use rustc_index::bit_set::BitSet;
use rustc_middle::bug;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, Instance, InstanceKind, Ty, TyCtxt, TypeFlags, TypeVisitableExt};
use rustc_session::config::{DebugInfo, OptLevel};
use rustc_span::source_map::Spanned;
use rustc_span::sym;
use tracing::{debug, instrument, trace, trace_span};

use crate::cost_checker::CostChecker;
Expand Down Expand Up @@ -120,7 +119,6 @@ trait Inliner<'tcx> {
callsite: &CallSite<'tcx>,
callee_body: &Body<'tcx>,
callee_attrs: &CodegenFnAttrs,
cross_crate_inlinable: bool,
) -> Result<(), &'static str>;

// How many callsites in a body are we allowed to inline? We need to limit this in order
Expand Down Expand Up @@ -196,7 +194,6 @@ impl<'tcx> Inliner<'tcx> for ForceInliner<'tcx> {
_: &CallSite<'tcx>,
callee_body: &Body<'tcx>,
callee_attrs: &CodegenFnAttrs,
_: bool,
) -> Result<(), &'static str> {
if callee_body.tainted_by_errors.is_some() {
return Err("body has errors");
Expand All @@ -215,14 +212,6 @@ impl<'tcx> Inliner<'tcx> for ForceInliner<'tcx> {
// inline-asm is detected. LLVM will still possibly do an inline later on
// if the no-attribute function ends up with the same instruction set anyway.
Err("cannot move inline-asm across instruction sets")
} else if callee_body
.basic_blocks
.iter()
.any(|bb| matches!(bb.terminator().kind, TerminatorKind::TailCall { .. }))
{
// FIXME(explicit_tail_calls): figure out how exactly functions containing tail
// calls can be inlined (and if they even should)
Err("can't inline functions with tail calls")
} else {
Ok(())
}
Expand Down Expand Up @@ -348,7 +337,6 @@ impl<'tcx> Inliner<'tcx> for NormalInliner<'tcx> {
callsite: &CallSite<'tcx>,
callee_body: &Body<'tcx>,
callee_attrs: &CodegenFnAttrs,
cross_crate_inlinable: bool,
) -> Result<(), &'static str> {
let tcx = self.tcx();

Expand All @@ -358,7 +346,7 @@ impl<'tcx> Inliner<'tcx> for NormalInliner<'tcx> {

let mut threshold = if self.caller_is_inline_forwarder {
tcx.sess.opts.unstable_opts.inline_mir_forwarder_threshold.unwrap_or(30)
} else if cross_crate_inlinable {
} else if tcx.cross_crate_inlinable(callsite.callee.def_id()) {
tcx.sess.opts.unstable_opts.inline_mir_hint_threshold.unwrap_or(100)
} else {
tcx.sess.opts.unstable_opts.inline_mir_threshold.unwrap_or(50)
Expand Down Expand Up @@ -587,16 +575,8 @@ fn try_inlining<'tcx, I: Inliner<'tcx>>(
check_mir_is_available(inliner, caller_body, callsite.callee)?;

let callee_attrs = tcx.codegen_fn_attrs(callsite.callee.def_id());
let cross_crate_inlinable = tcx.cross_crate_inlinable(callsite.callee.def_id());
check_codegen_attributes(inliner, callsite, callee_attrs, cross_crate_inlinable)?;

// Intrinsic fallback bodies are automatically made cross-crate inlineable,
// but at this stage we don't know whether codegen knows the intrinsic,
// so just conservatively don't inline it. This also ensures that we do not
// accidentally inline the body of an intrinsic that *must* be overridden.
if tcx.has_attr(callsite.callee.def_id(), sym::rustc_intrinsic) {
return Err("callee is an intrinsic");
}
rustc_mir_build::check_inline::is_inline_valid_on_fn(tcx, callsite.callee.def_id())?;
check_codegen_attributes(inliner, callsite, callee_attrs)?;

let terminator = caller_body[callsite.block].terminator.as_ref().unwrap();
let TerminatorKind::Call { args, destination, .. } = &terminator.kind else { bug!() };
Expand All @@ -610,7 +590,8 @@ fn try_inlining<'tcx, I: Inliner<'tcx>>(
}

let callee_body = try_instance_mir(tcx, callsite.callee.def)?;
inliner.check_callee_mir_body(callsite, callee_body, callee_attrs, cross_crate_inlinable)?;
rustc_mir_build::check_inline::is_inline_valid_on_body(tcx, callee_body)?;
inliner.check_callee_mir_body(callsite, callee_body, callee_attrs)?;

let Ok(callee_body) = callsite.callee.try_instantiate_mir_and_normalize_erasing_regions(
tcx,
Expand Down Expand Up @@ -775,38 +756,19 @@ fn check_codegen_attributes<'tcx, I: Inliner<'tcx>>(
inliner: &I,
callsite: &CallSite<'tcx>,
callee_attrs: &CodegenFnAttrs,
cross_crate_inlinable: bool,
) -> Result<(), &'static str> {
let tcx = inliner.tcx();
if tcx.has_attr(callsite.callee.def_id(), sym::rustc_no_mir_inline) {
return Err("#[rustc_no_mir_inline]");
}

if let InlineAttr::Never = callee_attrs.inline {
return Err("never inline attribute");
}

// FIXME(#127234): Coverage instrumentation currently doesn't handle inlined
// MIR correctly when Modified Condition/Decision Coverage is enabled.
if tcx.sess.instrument_coverage_mcdc() {
return Err("incompatible with MC/DC coverage");
}

// Reachability pass defines which functions are eligible for inlining. Generally inlining
// other functions is incorrect because they could reference symbols that aren't exported.
let is_generic = callsite.callee.args.non_erasable_generics().next().is_some();
if !is_generic && !cross_crate_inlinable {
if !is_generic && !tcx.cross_crate_inlinable(callsite.callee.def_id()) {
return Err("not exported");
}

if callsite.fn_sig.c_variadic() {
return Err("C variadic");
}

if callee_attrs.flags.contains(CodegenFnAttrFlags::COLD) {
return Err("cold");
}

let codegen_fn_attrs = tcx.codegen_fn_attrs(inliner.caller_def_id());
if callee_attrs.no_sanitize != codegen_fn_attrs.no_sanitize {
return Err("incompatible sanitizer set");
Expand Down
5 changes: 2 additions & 3 deletions tests/ui/force-inlining/deny-async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,17 @@

#[rustc_no_mir_inline]
#[rustc_force_inline]
//~^ ERROR `callee` is incompatible with `#[rustc_force_inline]`
pub fn callee() {
}

#[rustc_no_mir_inline]
#[rustc_force_inline = "the test requires it"]
//~^ ERROR `callee_justified` is incompatible with `#[rustc_force_inline]`
pub fn callee_justified() {
}

async fn async_caller() {
callee();
//~^ ERROR `callee` could not be inlined into `async_caller::{closure#0}` but is required to be inlined

callee_justified();
//~^ ERROR `callee_justified` could not be inlined into `async_caller::{closure#0}` but is required to be inlined
}
27 changes: 16 additions & 11 deletions tests/ui/force-inlining/deny-async.stderr
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
error: `callee` could not be inlined into `async_caller::{closure#0}` but is required to be inlined
--> $DIR/deny-async.rs:20:5
error: `callee` is incompatible with `#[rustc_force_inline]`
--> $DIR/deny-async.rs:10:1
|
LL | callee();
| ^^^^^^^^ ...`callee` called here
LL | #[rustc_force_inline]
| ^^^^^^^^^^^^^^^^^^^^^
LL |
LL | pub fn callee() {
| --------------- `callee` defined here
|
= note: could not be inlined due to: #[rustc_no_mir_inline]
= note: incompatible due to: #[rustc_no_mir_inline]

error: `callee_justified` could not be inlined into `async_caller::{closure#0}` but is required to be inlined
--> $DIR/deny-async.rs:23:5
error: `callee_justified` is incompatible with `#[rustc_force_inline]`
--> $DIR/deny-async.rs:16:1
|
LL | callee_justified();
| ^^^^^^^^^^^^^^^^^^ ...`callee_justified` called here
LL | #[rustc_force_inline = "the test requires it"]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL |
LL | pub fn callee_justified() {
| ------------------------- `callee_justified` defined here
|
= note: could not be inlined due to: #[rustc_no_mir_inline]
= note: `callee_justified` is required to be inlined to: the test requires it
= note: incompatible due to: #[rustc_no_mir_inline]

error: aborting due to 2 previous errors

7 changes: 3 additions & 4 deletions tests/ui/force-inlining/deny-closure.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@ build-fail
//@ check-fail
//@ compile-flags: --crate-type=lib
#![allow(internal_features)]
#![feature(rustc_attrs)]
Expand All @@ -7,20 +7,19 @@

#[rustc_no_mir_inline]
#[rustc_force_inline]
//~^ ERROR `callee` is incompatible with `#[rustc_force_inline]`
pub fn callee() {
}

#[rustc_no_mir_inline]
#[rustc_force_inline = "the test requires it"]
//~^ ERROR `callee_justified` is incompatible with `#[rustc_force_inline]`
pub fn callee_justified() {
}

pub fn caller() {
(|| {
callee();
//~^ ERROR `callee` could not be inlined into `caller::{closure#0}` but is required to be inlined

callee_justified();
//~^ ERROR `callee_justified` could not be inlined into `caller::{closure#0}` but is required to be inlined
})();
}
Loading

0 comments on commit cc9a9ec

Please sign in to comment.