From cfb78419cd865a19dac54d234d2a6abde0f14628 Mon Sep 17 00:00:00 2001 From: Maybe Waffle Date: Thu, 11 May 2023 11:43:09 +0000 Subject: [PATCH 01/27] implement checks for tail calls this implements checks necessary to guarantee that we can actually perform a tail call. while extremely restrictive, this is what is documented in the RFC, and all these checks are needed for one reason or another. --- compiler/rustc_middle/src/query/mod.rs | 6 + compiler/rustc_mir_build/src/build/mod.rs | 4 + .../rustc_mir_build/src/check_tail_calls.rs | 387 ++++++++++++++++++ compiler/rustc_mir_build/src/lib.rs | 2 + tests/ui/explicit-tail-calls/become-macro.rs | 13 + .../explicit-tail-calls/become-operator.fixed | 42 ++ .../ui/explicit-tail-calls/become-operator.rs | 42 ++ .../become-operator.stderr | 75 ++++ .../become-uncallable.fixed | 18 + .../explicit-tail-calls/become-uncallable.rs | 18 + .../become-uncallable.stderr | 44 ++ tests/ui/explicit-tail-calls/closure.fixed | 31 ++ tests/ui/explicit-tail-calls/closure.rs | 31 ++ tests/ui/explicit-tail-calls/closure.stderr | 46 +++ tests/ui/explicit-tail-calls/in-closure.rs | 8 + .../ui/explicit-tail-calls/in-closure.stderr | 8 + .../explicit-tail-calls/signature-mismatch.rs | 33 ++ .../signature-mismatch.stderr | 40 ++ 18 files changed, 848 insertions(+) create mode 100644 compiler/rustc_mir_build/src/check_tail_calls.rs create mode 100644 tests/ui/explicit-tail-calls/become-macro.rs create mode 100644 tests/ui/explicit-tail-calls/become-operator.fixed create mode 100644 tests/ui/explicit-tail-calls/become-operator.rs create mode 100644 tests/ui/explicit-tail-calls/become-operator.stderr create mode 100644 tests/ui/explicit-tail-calls/become-uncallable.fixed create mode 100644 tests/ui/explicit-tail-calls/become-uncallable.rs create mode 100644 tests/ui/explicit-tail-calls/become-uncallable.stderr create mode 100644 tests/ui/explicit-tail-calls/closure.fixed create mode 100644 tests/ui/explicit-tail-calls/closure.rs create mode 100644 tests/ui/explicit-tail-calls/closure.stderr create mode 100644 tests/ui/explicit-tail-calls/in-closure.rs create mode 100644 tests/ui/explicit-tail-calls/in-closure.stderr create mode 100644 tests/ui/explicit-tail-calls/signature-mismatch.rs create mode 100644 tests/ui/explicit-tail-calls/signature-mismatch.stderr diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 0f2a6d598a0fd..d3748af40777b 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -916,6 +916,12 @@ rustc_queries! { cache_on_disk_if { true } } + /// Checks well-formedness of tail calls (`become f()`). + query check_tail_calls(key: LocalDefId) -> Result<(), rustc_errors::ErrorGuaranteed> { + desc { |tcx| "tail-call-checking `{}`", tcx.def_path_str(key) } + cache_on_disk_if { true } + } + /// Returns the types assumed to be well formed while "inside" of the given item. /// /// Note that we've liberated the late bound regions of function signatures, so diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 3317f3b7f8acb..f43c29d8f5d68 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -50,6 +50,10 @@ pub(crate) fn mir_build<'tcx>(tcx: TyCtxtAt<'tcx>, def: LocalDefId) -> Body<'tcx return construct_error(tcx, def, e); } + if let Err(err) = tcx.check_tail_calls(def) { + return construct_error(tcx, def, err); + } + let body = match tcx.thir_body(def) { Err(error_reported) => construct_error(tcx, def, error_reported), Ok((thir, expr)) => { diff --git a/compiler/rustc_mir_build/src/check_tail_calls.rs b/compiler/rustc_mir_build/src/check_tail_calls.rs new file mode 100644 index 0000000000000..911a6cb7de606 --- /dev/null +++ b/compiler/rustc_mir_build/src/check_tail_calls.rs @@ -0,0 +1,387 @@ +use rustc_abi::ExternAbi; +use rustc_errors::Applicability; +use rustc_hir::LangItem; +use rustc_hir::def::DefKind; +use rustc_middle::span_bug; +use rustc_middle::thir::visit::{self, Visitor}; +use rustc_middle::thir::{BodyTy, Expr, ExprId, ExprKind, Thir}; +use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_span::def_id::{DefId, LocalDefId}; +use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span}; + +pub(crate) fn check_tail_calls(tcx: TyCtxt<'_>, def: LocalDefId) -> Result<(), ErrorGuaranteed> { + let (thir, expr) = tcx.thir_body(def)?; + let thir = &thir.borrow(); + + // If `thir` is empty, a type error occurred, skip this body. + if thir.exprs.is_empty() { + return Ok(()); + } + + let is_closure = matches!(tcx.def_kind(def), DefKind::Closure); + let caller_ty = tcx.type_of(def).skip_binder(); + + let mut visitor = TailCallCkVisitor { + tcx, + thir, + found_errors: Ok(()), + // FIXME(#132279): we're clearly in a body here. + typing_env: ty::TypingEnv::non_body_analysis(tcx, def), + is_closure, + caller_ty, + }; + + visitor.visit_expr(&thir[expr]); + + visitor.found_errors +} + +struct TailCallCkVisitor<'a, 'tcx> { + tcx: TyCtxt<'tcx>, + thir: &'a Thir<'tcx>, + typing_env: ty::TypingEnv<'tcx>, + /// Whatever the currently checked body is one of a closure + is_closure: bool, + /// The result of the checks, `Err(_)` if there was a problem with some + /// tail call, `Ok(())` if all of them were fine. + found_errors: Result<(), ErrorGuaranteed>, + /// Type of the caller function. + caller_ty: Ty<'tcx>, +} + +impl<'tcx> TailCallCkVisitor<'_, 'tcx> { + fn check_tail_call(&mut self, call: &Expr<'_>, expr: &Expr<'_>) { + if self.is_closure { + self.report_in_closure(expr); + return; + } + + let BodyTy::Fn(caller_sig) = self.thir.body_type else { + span_bug!( + call.span, + "`become` outside of functions should have been disallowed by hit_typeck" + ) + }; + + let ExprKind::Scope { value, .. } = call.kind else { + span_bug!(call.span, "expected scope, found: {call:?}") + }; + let value = &self.thir[value]; + + if matches!( + value.kind, + ExprKind::Binary { .. } + | ExprKind::Unary { .. } + | ExprKind::AssignOp { .. } + | ExprKind::Index { .. } + ) { + self.report_builtin_op(call, expr); + return; + } + + let ExprKind::Call { ty, fun, ref args, from_hir_call, fn_span } = value.kind else { + self.report_non_call(value, expr); + return; + }; + + if !from_hir_call { + self.report_op(ty, args, fn_span, expr); + } + + // Closures in thir look something akin to + // `for<'a> extern "rust-call" fn(&'a [closure@...], ()) -> <[closure@...] as FnOnce<()>>::Output {<[closure@...] as Fn<()>>::call}` + // So we have to check for them in this weird way... + if let &ty::FnDef(did, substs) = ty.kind() { + let parent = self.tcx.parent(did); + let fn_ = self.tcx.require_lang_item(LangItem::Fn, Some(expr.span)); + let fn_once = self.tcx.require_lang_item(LangItem::FnOnce, Some(expr.span)); + let fn_mut = self.tcx.require_lang_item(LangItem::FnMut, Some(expr.span)); + if [fn_, fn_once, fn_mut].contains(&parent) { + if substs.first().and_then(|arg| arg.as_type()).is_some_and(|t| t.is_closure()) { + self.report_calling_closure( + &self.thir[fun], + substs[1].as_type().unwrap(), + expr, + ); + + // Tail calling is likely to cause unrelated errors (ABI, argument mismatches) + return; + } + }; + } + + // Erase regions since tail calls don't care about lifetimes + let callee_sig = + self.tcx.normalize_erasing_late_bound_regions(self.typing_env, ty.fn_sig(self.tcx)); + + if caller_sig.abi != callee_sig.abi { + self.report_abi_mismatch(expr.span, caller_sig.abi, callee_sig.abi); + } + + if caller_sig.inputs_and_output != callee_sig.inputs_and_output { + if caller_sig.inputs() != callee_sig.inputs() { + self.report_arguments_mismatch(expr.span, caller_sig, callee_sig); + } + + if caller_sig.output() != callee_sig.output() { + span_bug!(expr.span, "hir typeck should have checked the return type already"); + } + } + + { + let caller_needs_location = self.needs_location(self.caller_ty); + let callee_needs_location = self.needs_location(ty); + + if caller_needs_location != callee_needs_location { + self.report_track_caller_mismatch(expr.span, caller_needs_location); + } + } + + if caller_sig.c_variadic { + self.report_c_variadic_caller(expr.span); + } + + if callee_sig.c_variadic { + self.report_c_variadic_callee(expr.span); + } + } + + /// Returns true if function of type `ty` needs location argument + /// (i.e. if a function is marked as `#[track_caller]`) + fn needs_location(&self, ty: Ty<'tcx>) -> bool { + if let &ty::FnDef(did, substs) = ty.kind() { + let instance = + ty::Instance::expect_resolve(self.tcx, self.typing_env, did, substs, DUMMY_SP) + .polymorphize(self.tcx); + + instance.def.requires_caller_location(self.tcx) + } else { + false + } + } + + fn report_in_closure(&mut self, expr: &Expr<'_>) { + let err = self.tcx.dcx().span_err(expr.span, "`become` is not allowed in closures"); + self.found_errors = Err(err); + } + + fn report_builtin_op(&mut self, value: &Expr<'_>, expr: &Expr<'_>) { + let err = self + .tcx + .dcx() + .struct_span_err(value.span, "`become` does not support operators") + .with_note("using `become` on a builtin operator is not useful") + .with_span_suggestion( + value.span.until(expr.span), + "try using `return` instead", + "return ", + Applicability::MachineApplicable, + ) + .emit(); + self.found_errors = Err(err); + } + + fn report_op(&mut self, fun_ty: Ty<'_>, args: &[ExprId], fn_span: Span, expr: &Expr<'_>) { + let mut err = + self.tcx.dcx().struct_span_err(fn_span, "`become` does not support operators"); + + if let &ty::FnDef(did, _substs) = fun_ty.kind() + && let parent = self.tcx.parent(did) + && matches!(self.tcx.def_kind(parent), DefKind::Trait) + && let Some(method) = op_trait_as_method_name(self.tcx, parent) + { + match args { + &[arg] => { + let arg = &self.thir[arg]; + + err.multipart_suggestion( + "try using the method directly", + vec![ + (fn_span.shrink_to_lo().until(arg.span), "(".to_owned()), + (arg.span.shrink_to_hi(), format!(").{method}()")), + ], + Applicability::MaybeIncorrect, + ); + } + &[lhs, rhs] => { + let lhs = &self.thir[lhs]; + let rhs = &self.thir[rhs]; + + err.multipart_suggestion( + "try using the method directly", + vec![ + (lhs.span.shrink_to_lo(), format!("(")), + (lhs.span.between(rhs.span), format!(").{method}(")), + (rhs.span.between(expr.span.shrink_to_hi()), ")".to_owned()), + ], + Applicability::MaybeIncorrect, + ); + } + _ => span_bug!(expr.span, "operator with more than 2 args? {args:?}"), + } + } + + self.found_errors = Err(err.emit()); + } + + fn report_non_call(&mut self, value: &Expr<'_>, expr: &Expr<'_>) { + let err = self + .tcx + .dcx() + .struct_span_err(value.span, "`become` requires a function call") + .with_span_note(value.span, "not a function call") + .with_span_suggestion( + value.span.until(expr.span), + "try using `return` instead", + "return ", + Applicability::MaybeIncorrect, + ) + .emit(); + self.found_errors = Err(err); + } + + fn report_calling_closure(&mut self, fun: &Expr<'_>, tupled_args: Ty<'_>, expr: &Expr<'_>) { + let underscored_args = match tupled_args.kind() { + ty::Tuple(tys) if tys.is_empty() => "".to_owned(), + ty::Tuple(tys) => std::iter::repeat("_, ").take(tys.len() - 1).chain(["_"]).collect(), + _ => "_".to_owned(), + }; + + let err = self + .tcx + .dcx() + .struct_span_err(expr.span, "tail calling closures directly is not allowed") + .with_multipart_suggestion( + "try casting the closure to a function pointer type", + vec![ + (fun.span.shrink_to_lo(), "(".to_owned()), + (fun.span.shrink_to_hi(), format!(" as fn({underscored_args}) -> _)")), + ], + Applicability::MaybeIncorrect, + ) + .emit(); + self.found_errors = Err(err); + } + + fn report_abi_mismatch(&mut self, sp: Span, caller_abi: ExternAbi, callee_abi: ExternAbi) { + let err = self + .tcx + .dcx() + .struct_span_err(sp, "mismatched function ABIs") + .with_note("`become` requires caller and callee to have the same ABI") + .with_note(format!("caller ABI is `{caller_abi}`, while callee ABI is `{callee_abi}`")) + .emit(); + self.found_errors = Err(err); + } + + fn report_arguments_mismatch( + &mut self, + sp: Span, + caller_sig: ty::FnSig<'_>, + callee_sig: ty::FnSig<'_>, + ) { + let err = self + .tcx + .dcx() + .struct_span_err(sp, "mismatched signatures") + .with_note("`become` requires caller and callee to have matching signatures") + .with_note(format!("caller signature: `{caller_sig}`")) + .with_note(format!("callee signature: `{callee_sig}`")) + .emit(); + self.found_errors = Err(err); + } + + fn report_track_caller_mismatch(&mut self, sp: Span, caller_needs_location: bool) { + let err = match caller_needs_location { + true => self + .tcx + .dcx() + .struct_span_err( + sp, + "a function marked with `#[track_caller]` cannot tail-call one that is not", + ) + .emit(), + false => self + .tcx + .dcx() + .struct_span_err( + sp, + "a function mot marked with `#[track_caller]` cannot tail-call one that is", + ) + .emit(), + }; + + self.found_errors = Err(err); + } + + fn report_c_variadic_caller(&mut self, sp: Span) { + let err = self + .tcx + .dcx() + // FIXME(explicit_tail_calls): highlight the `...` + .struct_span_err(sp, "tail-calls are not allowed in c-variadic functions") + .emit(); + + self.found_errors = Err(err); + } + + fn report_c_variadic_callee(&mut self, sp: Span) { + let err = self + .tcx + .dcx() + // FIXME(explicit_tail_calls): highlight the function or something... + .struct_span_err(sp, "c-variadic functions can't be tail-called") + .emit(); + + self.found_errors = Err(err); + } +} + +impl<'a, 'tcx> Visitor<'a, 'tcx> for TailCallCkVisitor<'a, 'tcx> { + fn thir(&self) -> &'a Thir<'tcx> { + &self.thir + } + + fn visit_expr(&mut self, expr: &'a Expr<'tcx>) { + if let ExprKind::Become { value } = expr.kind { + let call = &self.thir[value]; + self.check_tail_call(call, expr); + } + + visit::walk_expr(self, expr); + } +} + +fn op_trait_as_method_name(tcx: TyCtxt<'_>, trait_did: DefId) -> Option<&'static str> { + let trait_did = Some(trait_did); + let items = tcx.lang_items(); + let m = match () { + _ if trait_did == items.get(LangItem::Add) => "add", + _ if trait_did == items.get(LangItem::Sub) => "sub", + _ if trait_did == items.get(LangItem::Mul) => "mul", + _ if trait_did == items.get(LangItem::Div) => "div", + _ if trait_did == items.get(LangItem::Rem) => "rem", + _ if trait_did == items.get(LangItem::Neg) => "neg", + _ if trait_did == items.get(LangItem::Not) => "not", + _ if trait_did == items.get(LangItem::BitXor) => "bitxor", + _ if trait_did == items.get(LangItem::BitAnd) => "bitand", + _ if trait_did == items.get(LangItem::BitOr) => "bitor", + _ if trait_did == items.get(LangItem::Shl) => "shl", + _ if trait_did == items.get(LangItem::Shr) => "shr", + _ if trait_did == items.get(LangItem::AddAssign) => "add_assign", + _ if trait_did == items.get(LangItem::SubAssign) => "sub_assign", + _ if trait_did == items.get(LangItem::MulAssign) => "mul_assign", + _ if trait_did == items.get(LangItem::DivAssign) => "div_assign", + _ if trait_did == items.get(LangItem::RemAssign) => "rem_assign", + _ if trait_did == items.get(LangItem::BitXorAssign) => "bitxor_assign", + _ if trait_did == items.get(LangItem::BitAndAssign) => "bitand_assign", + _ if trait_did == items.get(LangItem::BitOrAssign) => "bitor_assign", + _ if trait_did == items.get(LangItem::ShlAssign) => "shl_assign", + _ if trait_did == items.get(LangItem::ShrAssign) => "shr_assign", + _ if trait_did == items.get(LangItem::Index) => "index", + _ if trait_did == items.get(LangItem::IndexMut) => "index_mut", + _ => return None, + }; + + Some(m) +} diff --git a/compiler/rustc_mir_build/src/lib.rs b/compiler/rustc_mir_build/src/lib.rs index 3dbb552cdbbef..833e5019865e8 100644 --- a/compiler/rustc_mir_build/src/lib.rs +++ b/compiler/rustc_mir_build/src/lib.rs @@ -12,6 +12,7 @@ // tidy-alphabetical-end mod build; +mod check_tail_calls; mod check_unsafety; mod errors; pub mod lints; @@ -28,6 +29,7 @@ pub fn provide(providers: &mut Providers) { providers.closure_saved_names_of_captured_variables = build::closure_saved_names_of_captured_variables; providers.check_unsafety = check_unsafety::check_unsafety; + providers.check_tail_calls = check_tail_calls::check_tail_calls; providers.thir_body = thir::cx::thir_body; providers.hooks.thir_tree = thir::print::thir_tree; providers.hooks.thir_flat = thir::print::thir_flat; diff --git a/tests/ui/explicit-tail-calls/become-macro.rs b/tests/ui/explicit-tail-calls/become-macro.rs new file mode 100644 index 0000000000000..0bbc483b2c703 --- /dev/null +++ b/tests/ui/explicit-tail-calls/become-macro.rs @@ -0,0 +1,13 @@ +//@ check-pass +#![allow(incomplete_features)] +#![feature(explicit_tail_calls, decl_macro)] + +macro call($f:expr $(, $args:expr)* $(,)?) { + ($f)($($args),*) +} + +fn main() { + become call!(f); +} + +fn f() {} diff --git a/tests/ui/explicit-tail-calls/become-operator.fixed b/tests/ui/explicit-tail-calls/become-operator.fixed new file mode 100644 index 0000000000000..36925d21b7a9c --- /dev/null +++ b/tests/ui/explicit-tail-calls/become-operator.fixed @@ -0,0 +1,42 @@ +//@ run-rustfix +#![allow(incomplete_features)] +#![feature(explicit_tail_calls)] +#![allow(unused)] +use std::num::Wrapping; +use std::ops::{Not, Add, BitXorAssign}; + +// built-ins and overloaded operators are handled differently + +fn f(a: u64, b: u64) -> u64 { + return a + b; //~ error: `become` does not support operators +} + +fn g(a: String, b: &str) -> String { + become (a).add(b); //~ error: `become` does not support operators +} + +fn h(x: u64) -> u64 { + return !x; //~ error: `become` does not support operators +} + +fn i_do_not_know_any_more_letters(x: Wrapping) -> Wrapping { + become (x).not(); //~ error: `become` does not support operators +} + +fn builtin_index(x: &[u8], i: usize) -> u8 { + return x[i] //~ error: `become` does not support operators +} + +// FIXME(explicit_tail_calls): overloaded index is represented like `[&]*x.index(i)`, +// and so need additional handling + +fn a(a: &mut u8, _: u8) { + return *a ^= 1; //~ error: `become` does not support operators +} + +fn b(b: &mut Wrapping, _: u8) { + become (*b).bitxor_assign(1); //~ error: `become` does not support operators +} + + +fn main() {} diff --git a/tests/ui/explicit-tail-calls/become-operator.rs b/tests/ui/explicit-tail-calls/become-operator.rs new file mode 100644 index 0000000000000..b1af3f59df7f0 --- /dev/null +++ b/tests/ui/explicit-tail-calls/become-operator.rs @@ -0,0 +1,42 @@ +//@ run-rustfix +#![allow(incomplete_features)] +#![feature(explicit_tail_calls)] +#![allow(unused)] +use std::num::Wrapping; +use std::ops::{Not, Add, BitXorAssign}; + +// built-ins and overloaded operators are handled differently + +fn f(a: u64, b: u64) -> u64 { + become a + b; //~ error: `become` does not support operators +} + +fn g(a: String, b: &str) -> String { + become a + b; //~ error: `become` does not support operators +} + +fn h(x: u64) -> u64 { + become !x; //~ error: `become` does not support operators +} + +fn i_do_not_know_any_more_letters(x: Wrapping) -> Wrapping { + become !x; //~ error: `become` does not support operators +} + +fn builtin_index(x: &[u8], i: usize) -> u8 { + become x[i] //~ error: `become` does not support operators +} + +// FIXME(explicit_tail_calls): overloaded index is represented like `[&]*x.index(i)`, +// and so need additional handling + +fn a(a: &mut u8, _: u8) { + become *a ^= 1; //~ error: `become` does not support operators +} + +fn b(b: &mut Wrapping, _: u8) { + become *b ^= 1; //~ error: `become` does not support operators +} + + +fn main() {} diff --git a/tests/ui/explicit-tail-calls/become-operator.stderr b/tests/ui/explicit-tail-calls/become-operator.stderr new file mode 100644 index 0000000000000..26e4343faaea7 --- /dev/null +++ b/tests/ui/explicit-tail-calls/become-operator.stderr @@ -0,0 +1,75 @@ +error: `become` does not support operators + --> $DIR/become-operator.rs:11:12 + | +LL | become a + b; + | -------^^^^^ + | | + | help: try using `return` instead: `return` + | + = note: using `become` on a builtin operator is not useful + +error: `become` does not support operators + --> $DIR/become-operator.rs:15:12 + | +LL | become a + b; + | ^^^^^ + | +help: try using the method directly + | +LL | become (a).add(b); + | + ~~~~~~ + + +error: `become` does not support operators + --> $DIR/become-operator.rs:19:12 + | +LL | become !x; + | -------^^ + | | + | help: try using `return` instead: `return` + | + = note: using `become` on a builtin operator is not useful + +error: `become` does not support operators + --> $DIR/become-operator.rs:23:12 + | +LL | become !x; + | ^^ + | +help: try using the method directly + | +LL | become (x).not(); + | ~ +++++++ + +error: `become` does not support operators + --> $DIR/become-operator.rs:27:12 + | +LL | become x[i] + | -------^^^^ + | | + | help: try using `return` instead: `return` + | + = note: using `become` on a builtin operator is not useful + +error: `become` does not support operators + --> $DIR/become-operator.rs:34:12 + | +LL | become *a ^= 1; + | -------^^^^^^^ + | | + | help: try using `return` instead: `return` + | + = note: using `become` on a builtin operator is not useful + +error: `become` does not support operators + --> $DIR/become-operator.rs:38:12 + | +LL | become *b ^= 1; + | ^^^^^^^ + | +help: try using the method directly + | +LL | become (*b).bitxor_assign(1); + | + ~~~~~~~~~~~~~~~~ + + +error: aborting due to 7 previous errors + diff --git a/tests/ui/explicit-tail-calls/become-uncallable.fixed b/tests/ui/explicit-tail-calls/become-uncallable.fixed new file mode 100644 index 0000000000000..af6785e08fe69 --- /dev/null +++ b/tests/ui/explicit-tail-calls/become-uncallable.fixed @@ -0,0 +1,18 @@ +//@ run-rustfix +#![allow(incomplete_features)] +#![feature(explicit_tail_calls)] +#![allow(unused)] + +fn f() -> u64 { + return 1; //~ error: `become` requires a function call +} + +fn g() { + return { h() }; //~ error: `become` requires a function call +} + +fn h() { + return *&g(); //~ error: `become` requires a function call +} + +fn main() {} diff --git a/tests/ui/explicit-tail-calls/become-uncallable.rs b/tests/ui/explicit-tail-calls/become-uncallable.rs new file mode 100644 index 0000000000000..60026b0d5d6fb --- /dev/null +++ b/tests/ui/explicit-tail-calls/become-uncallable.rs @@ -0,0 +1,18 @@ +//@ run-rustfix +#![allow(incomplete_features)] +#![feature(explicit_tail_calls)] +#![allow(unused)] + +fn f() -> u64 { + become 1; //~ error: `become` requires a function call +} + +fn g() { + become { h() }; //~ error: `become` requires a function call +} + +fn h() { + become *&g(); //~ error: `become` requires a function call +} + +fn main() {} diff --git a/tests/ui/explicit-tail-calls/become-uncallable.stderr b/tests/ui/explicit-tail-calls/become-uncallable.stderr new file mode 100644 index 0000000000000..90f10b05d2afc --- /dev/null +++ b/tests/ui/explicit-tail-calls/become-uncallable.stderr @@ -0,0 +1,44 @@ +error: `become` requires a function call + --> $DIR/become-uncallable.rs:7:12 + | +LL | become 1; + | -------^ + | | + | help: try using `return` instead: `return` + | +note: not a function call + --> $DIR/become-uncallable.rs:7:12 + | +LL | become 1; + | ^ + +error: `become` requires a function call + --> $DIR/become-uncallable.rs:11:12 + | +LL | become { h() }; + | -------^^^^^^^ + | | + | help: try using `return` instead: `return` + | +note: not a function call + --> $DIR/become-uncallable.rs:11:12 + | +LL | become { h() }; + | ^^^^^^^ + +error: `become` requires a function call + --> $DIR/become-uncallable.rs:15:12 + | +LL | become *&g(); + | -------^^^^^ + | | + | help: try using `return` instead: `return` + | +note: not a function call + --> $DIR/become-uncallable.rs:15:12 + | +LL | become *&g(); + | ^^^^^ + +error: aborting due to 3 previous errors + diff --git a/tests/ui/explicit-tail-calls/closure.fixed b/tests/ui/explicit-tail-calls/closure.fixed new file mode 100644 index 0000000000000..18384d91e0ffe --- /dev/null +++ b/tests/ui/explicit-tail-calls/closure.fixed @@ -0,0 +1,31 @@ +//@ run-rustfix +#![allow(incomplete_features)] +#![feature(explicit_tail_calls)] + +fn a() { + become ((|| ()) as fn() -> _)(); + //~^ ERROR: tail calling closures directly is not allowed +} + +fn aa((): ()) { + become ((|()| ()) as fn(_) -> _)(()); + //~^ ERROR: tail calling closures directly is not allowed +} + +fn aaa((): (), _: i32) { + become ((|(), _| ()) as fn(_, _) -> _)((), 1); + //~^ ERROR: tail calling closures directly is not allowed +} + +fn v((): (), ((), ()): ((), ())) -> (((), ()), ()) { + let f = |(), ((), ())| (((), ()), ()); + become (f as fn(_, _) -> _)((), ((), ())); + //~^ ERROR: tail calling closures directly is not allowed +} + +fn main() { + a(); + aa(()); + aaa((), 1); + v((), ((), ())); +} diff --git a/tests/ui/explicit-tail-calls/closure.rs b/tests/ui/explicit-tail-calls/closure.rs new file mode 100644 index 0000000000000..b65ebed594bfd --- /dev/null +++ b/tests/ui/explicit-tail-calls/closure.rs @@ -0,0 +1,31 @@ +//@ run-rustfix +#![allow(incomplete_features)] +#![feature(explicit_tail_calls)] + +fn a() { + become (|| ())(); + //~^ ERROR: tail calling closures directly is not allowed +} + +fn aa((): ()) { + become (|()| ())(()); + //~^ ERROR: tail calling closures directly is not allowed +} + +fn aaa((): (), _: i32) { + become (|(), _| ())((), 1); + //~^ ERROR: tail calling closures directly is not allowed +} + +fn v((): (), ((), ()): ((), ())) -> (((), ()), ()) { + let f = |(), ((), ())| (((), ()), ()); + become f((), ((), ())); + //~^ ERROR: tail calling closures directly is not allowed +} + +fn main() { + a(); + aa(()); + aaa((), 1); + v((), ((), ())); +} diff --git a/tests/ui/explicit-tail-calls/closure.stderr b/tests/ui/explicit-tail-calls/closure.stderr new file mode 100644 index 0000000000000..5d57bece68e2c --- /dev/null +++ b/tests/ui/explicit-tail-calls/closure.stderr @@ -0,0 +1,46 @@ +error: tail calling closures directly is not allowed + --> $DIR/closure.rs:6:5 + | +LL | become (|| ())(); + | ^^^^^^^^^^^^^^^^ + | +help: try casting the closure to a function pointer type + | +LL | become ((|| ()) as fn() -> _)(); + | + +++++++++++++ + +error: tail calling closures directly is not allowed + --> $DIR/closure.rs:11:5 + | +LL | become (|()| ())(()); + | ^^^^^^^^^^^^^^^^^^^^ + | +help: try casting the closure to a function pointer type + | +LL | become ((|()| ()) as fn(_) -> _)(()); + | + ++++++++++++++ + +error: tail calling closures directly is not allowed + --> $DIR/closure.rs:16:5 + | +LL | become (|(), _| ())((), 1); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try casting the closure to a function pointer type + | +LL | become ((|(), _| ()) as fn(_, _) -> _)((), 1); + | + +++++++++++++++++ + +error: tail calling closures directly is not allowed + --> $DIR/closure.rs:22:5 + | +LL | become f((), ((), ())); + | ^^^^^^^^^^^^^^^^^^^^^^ + | +help: try casting the closure to a function pointer type + | +LL | become (f as fn(_, _) -> _)((), ((), ())); + | + +++++++++++++++++ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/explicit-tail-calls/in-closure.rs b/tests/ui/explicit-tail-calls/in-closure.rs new file mode 100644 index 0000000000000..225bc0a7fd762 --- /dev/null +++ b/tests/ui/explicit-tail-calls/in-closure.rs @@ -0,0 +1,8 @@ +#![allow(incomplete_features)] +#![feature(explicit_tail_calls)] + +fn main() { + || become f(); //~ error: `become` is not allowed in closures +} + +fn f() {} diff --git a/tests/ui/explicit-tail-calls/in-closure.stderr b/tests/ui/explicit-tail-calls/in-closure.stderr new file mode 100644 index 0000000000000..d221f732e938b --- /dev/null +++ b/tests/ui/explicit-tail-calls/in-closure.stderr @@ -0,0 +1,8 @@ +error: `become` is not allowed in closures + --> $DIR/in-closure.rs:5:8 + | +LL | || become f(); + | ^^^^^^^^^^ + +error: aborting due to 1 previous error + diff --git a/tests/ui/explicit-tail-calls/signature-mismatch.rs b/tests/ui/explicit-tail-calls/signature-mismatch.rs new file mode 100644 index 0000000000000..3a01cc1ca2fd2 --- /dev/null +++ b/tests/ui/explicit-tail-calls/signature-mismatch.rs @@ -0,0 +1,33 @@ +#![allow(incomplete_features)] +#![feature(explicit_tail_calls)] +#![feature(c_variadic)] + +fn _f0((): ()) { + become _g0(); //~ error: mismatched signatures +} + +fn _g0() {} + + +fn _f1() { + become _g1(()); //~ error: mismatched signatures +} + +fn _g1((): ()) {} + + +extern "C" fn _f2() { + become _g2(); //~ error: mismatched function ABIs +} + +fn _g2() {} + + +fn _f3() { + become _g3(); //~ error: mismatched function ABIs +} + +extern "C" fn _g3() {} + + +fn main() {} diff --git a/tests/ui/explicit-tail-calls/signature-mismatch.stderr b/tests/ui/explicit-tail-calls/signature-mismatch.stderr new file mode 100644 index 0000000000000..ba9e9dcb98483 --- /dev/null +++ b/tests/ui/explicit-tail-calls/signature-mismatch.stderr @@ -0,0 +1,40 @@ +error: mismatched signatures + --> $DIR/signature-mismatch.rs:6:5 + | +LL | become _g0(); + | ^^^^^^^^^^^^ + | + = note: `become` requires caller and callee to have matching signatures + = note: caller signature: `fn(())` + = note: callee signature: `fn()` + +error: mismatched signatures + --> $DIR/signature-mismatch.rs:13:5 + | +LL | become _g1(()); + | ^^^^^^^^^^^^^^ + | + = note: `become` requires caller and callee to have matching signatures + = note: caller signature: `fn()` + = note: callee signature: `fn(())` + +error: mismatched function ABIs + --> $DIR/signature-mismatch.rs:20:5 + | +LL | become _g2(); + | ^^^^^^^^^^^^ + | + = note: `become` requires caller and callee to have the same ABI + = note: caller ABI is `"C"`, while callee ABI is `"Rust"` + +error: mismatched function ABIs + --> $DIR/signature-mismatch.rs:27:5 + | +LL | become _g3(); + | ^^^^^^^^^^^^ + | + = note: `become` requires caller and callee to have the same ABI + = note: caller ABI is `"Rust"`, while callee ABI is `"C"` + +error: aborting due to 4 previous errors + From 3208b8649e2e117b08e5bbcae46061d44bf72203 Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Fri, 29 Nov 2024 04:28:02 +0100 Subject: [PATCH 02/27] use `expect(incomplete_feature)` instead of `allow` in tail call tests --- tests/ui/explicit-tail-calls/become-macro.rs | 2 +- tests/ui/explicit-tail-calls/become-operator.fixed | 2 +- tests/ui/explicit-tail-calls/become-operator.rs | 2 +- tests/ui/explicit-tail-calls/become-outside.rs | 2 +- tests/ui/explicit-tail-calls/become-uncallable.fixed | 2 +- tests/ui/explicit-tail-calls/become-uncallable.rs | 2 +- tests/ui/explicit-tail-calls/closure.fixed | 2 +- tests/ui/explicit-tail-calls/closure.rs | 2 +- tests/ui/explicit-tail-calls/constck.rs | 2 +- tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.rs | 2 +- tests/ui/explicit-tail-calls/ctfe-arg-good-borrow.rs | 2 +- tests/ui/explicit-tail-calls/ctfe-arg-move.rs | 2 +- tests/ui/explicit-tail-calls/ctfe-collatz-multi-rec.rs | 2 +- tests/ui/explicit-tail-calls/ctfe-id-unlimited.rs | 2 +- tests/ui/explicit-tail-calls/ctfe-tail-call-panic.rs | 2 +- tests/ui/explicit-tail-calls/drop-order.rs | 2 +- tests/ui/explicit-tail-calls/in-closure.rs | 2 +- tests/ui/explicit-tail-calls/return-lifetime-sub.rs | 2 +- tests/ui/explicit-tail-calls/return-mismatches.rs | 2 +- tests/ui/explicit-tail-calls/signature-mismatch.rs | 2 +- tests/ui/explicit-tail-calls/unsafeck.rs | 2 +- 21 files changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/ui/explicit-tail-calls/become-macro.rs b/tests/ui/explicit-tail-calls/become-macro.rs index 0bbc483b2c703..0a9d069ceb3cb 100644 --- a/tests/ui/explicit-tail-calls/become-macro.rs +++ b/tests/ui/explicit-tail-calls/become-macro.rs @@ -1,5 +1,5 @@ //@ check-pass -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls, decl_macro)] macro call($f:expr $(, $args:expr)* $(,)?) { diff --git a/tests/ui/explicit-tail-calls/become-operator.fixed b/tests/ui/explicit-tail-calls/become-operator.fixed index 36925d21b7a9c..24baef42d69a3 100644 --- a/tests/ui/explicit-tail-calls/become-operator.fixed +++ b/tests/ui/explicit-tail-calls/become-operator.fixed @@ -1,5 +1,5 @@ //@ run-rustfix -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] #![allow(unused)] use std::num::Wrapping; diff --git a/tests/ui/explicit-tail-calls/become-operator.rs b/tests/ui/explicit-tail-calls/become-operator.rs index b1af3f59df7f0..e547e979b6485 100644 --- a/tests/ui/explicit-tail-calls/become-operator.rs +++ b/tests/ui/explicit-tail-calls/become-operator.rs @@ -1,5 +1,5 @@ //@ run-rustfix -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] #![allow(unused)] use std::num::Wrapping; diff --git a/tests/ui/explicit-tail-calls/become-outside.rs b/tests/ui/explicit-tail-calls/become-outside.rs index 9c90d929111e1..29738bb9a299f 100644 --- a/tests/ui/explicit-tail-calls/become-outside.rs +++ b/tests/ui/explicit-tail-calls/become-outside.rs @@ -1,5 +1,5 @@ //@ revisions: constant array -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] #[cfg(constant)] diff --git a/tests/ui/explicit-tail-calls/become-uncallable.fixed b/tests/ui/explicit-tail-calls/become-uncallable.fixed index af6785e08fe69..b77c46ea43549 100644 --- a/tests/ui/explicit-tail-calls/become-uncallable.fixed +++ b/tests/ui/explicit-tail-calls/become-uncallable.fixed @@ -1,5 +1,5 @@ //@ run-rustfix -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] #![allow(unused)] diff --git a/tests/ui/explicit-tail-calls/become-uncallable.rs b/tests/ui/explicit-tail-calls/become-uncallable.rs index 60026b0d5d6fb..a73b9258aaabb 100644 --- a/tests/ui/explicit-tail-calls/become-uncallable.rs +++ b/tests/ui/explicit-tail-calls/become-uncallable.rs @@ -1,5 +1,5 @@ //@ run-rustfix -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] #![allow(unused)] diff --git a/tests/ui/explicit-tail-calls/closure.fixed b/tests/ui/explicit-tail-calls/closure.fixed index 18384d91e0ffe..4af71c5d4cc95 100644 --- a/tests/ui/explicit-tail-calls/closure.fixed +++ b/tests/ui/explicit-tail-calls/closure.fixed @@ -1,5 +1,5 @@ //@ run-rustfix -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] fn a() { diff --git a/tests/ui/explicit-tail-calls/closure.rs b/tests/ui/explicit-tail-calls/closure.rs index b65ebed594bfd..79a1b5296b0ae 100644 --- a/tests/ui/explicit-tail-calls/closure.rs +++ b/tests/ui/explicit-tail-calls/closure.rs @@ -1,5 +1,5 @@ //@ run-rustfix -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] fn a() { diff --git a/tests/ui/explicit-tail-calls/constck.rs b/tests/ui/explicit-tail-calls/constck.rs index 938f15f12c091..36fc3ef6f995b 100644 --- a/tests/ui/explicit-tail-calls/constck.rs +++ b/tests/ui/explicit-tail-calls/constck.rs @@ -1,4 +1,4 @@ -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] const fn f() { diff --git a/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.rs b/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.rs index 5a105ee4eb59d..0a61c90bd00a7 100644 --- a/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.rs +++ b/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.rs @@ -1,4 +1,4 @@ -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] pub const fn test(_: &Type) { diff --git a/tests/ui/explicit-tail-calls/ctfe-arg-good-borrow.rs b/tests/ui/explicit-tail-calls/ctfe-arg-good-borrow.rs index 50bf6c946ca8f..a34482a352725 100644 --- a/tests/ui/explicit-tail-calls/ctfe-arg-good-borrow.rs +++ b/tests/ui/explicit-tail-calls/ctfe-arg-good-borrow.rs @@ -1,5 +1,5 @@ //@ check-pass -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] pub const fn test(x: &Type) { diff --git a/tests/ui/explicit-tail-calls/ctfe-arg-move.rs b/tests/ui/explicit-tail-calls/ctfe-arg-move.rs index 88ff3a4a5ad13..610eb74a91e37 100644 --- a/tests/ui/explicit-tail-calls/ctfe-arg-move.rs +++ b/tests/ui/explicit-tail-calls/ctfe-arg-move.rs @@ -1,5 +1,5 @@ //@ check-pass -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] pub const fn test(s: String) -> String { diff --git a/tests/ui/explicit-tail-calls/ctfe-collatz-multi-rec.rs b/tests/ui/explicit-tail-calls/ctfe-collatz-multi-rec.rs index 86041b669601d..53d46d92bd7b7 100644 --- a/tests/ui/explicit-tail-calls/ctfe-collatz-multi-rec.rs +++ b/tests/ui/explicit-tail-calls/ctfe-collatz-multi-rec.rs @@ -1,5 +1,5 @@ //@ run-pass -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] /// A very unnecessarily complicated "implementation" of the Collatz conjecture. diff --git a/tests/ui/explicit-tail-calls/ctfe-id-unlimited.rs b/tests/ui/explicit-tail-calls/ctfe-id-unlimited.rs index 54e68b2b7f793..2a04d4893e68a 100644 --- a/tests/ui/explicit-tail-calls/ctfe-id-unlimited.rs +++ b/tests/ui/explicit-tail-calls/ctfe-id-unlimited.rs @@ -1,6 +1,6 @@ //@ revisions: become return //@ [become] run-pass -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] // This is an identity function (`|x| x`), but implemented using recursion. diff --git a/tests/ui/explicit-tail-calls/ctfe-tail-call-panic.rs b/tests/ui/explicit-tail-calls/ctfe-tail-call-panic.rs index 3d69cde29895a..fba4a2692afe3 100644 --- a/tests/ui/explicit-tail-calls/ctfe-tail-call-panic.rs +++ b/tests/ui/explicit-tail-calls/ctfe-tail-call-panic.rs @@ -1,4 +1,4 @@ -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] pub const fn f() { diff --git a/tests/ui/explicit-tail-calls/drop-order.rs b/tests/ui/explicit-tail-calls/drop-order.rs index e20730446ec55..242336be4845e 100644 --- a/tests/ui/explicit-tail-calls/drop-order.rs +++ b/tests/ui/explicit-tail-calls/drop-order.rs @@ -1,7 +1,7 @@ // FIXME(explicit_tail_calls): enable this test once rustc_codegen_ssa supports tail calls //@ ignore-test: tail calls are not implemented in rustc_codegen_ssa yet, so this causes 🧊 //@ run-pass -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] use std::cell::RefCell; diff --git a/tests/ui/explicit-tail-calls/in-closure.rs b/tests/ui/explicit-tail-calls/in-closure.rs index 225bc0a7fd762..b23ff5a1e05b4 100644 --- a/tests/ui/explicit-tail-calls/in-closure.rs +++ b/tests/ui/explicit-tail-calls/in-closure.rs @@ -1,4 +1,4 @@ -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] fn main() { diff --git a/tests/ui/explicit-tail-calls/return-lifetime-sub.rs b/tests/ui/explicit-tail-calls/return-lifetime-sub.rs index 1243fba9b5883..df0982cc02bdc 100644 --- a/tests/ui/explicit-tail-calls/return-lifetime-sub.rs +++ b/tests/ui/explicit-tail-calls/return-lifetime-sub.rs @@ -1,5 +1,5 @@ //@ check-pass -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] fn _f<'a>() -> &'a [u8] { diff --git a/tests/ui/explicit-tail-calls/return-mismatches.rs b/tests/ui/explicit-tail-calls/return-mismatches.rs index 935a1a1d28b02..e48766b89b00c 100644 --- a/tests/ui/explicit-tail-calls/return-mismatches.rs +++ b/tests/ui/explicit-tail-calls/return-mismatches.rs @@ -1,4 +1,4 @@ -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] fn _f0<'a>() -> &'static [u8] { diff --git a/tests/ui/explicit-tail-calls/signature-mismatch.rs b/tests/ui/explicit-tail-calls/signature-mismatch.rs index 3a01cc1ca2fd2..a32ac9d8bfee3 100644 --- a/tests/ui/explicit-tail-calls/signature-mismatch.rs +++ b/tests/ui/explicit-tail-calls/signature-mismatch.rs @@ -1,4 +1,4 @@ -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] #![feature(c_variadic)] diff --git a/tests/ui/explicit-tail-calls/unsafeck.rs b/tests/ui/explicit-tail-calls/unsafeck.rs index 872a70ca3a0a7..f8a8140cedc33 100644 --- a/tests/ui/explicit-tail-calls/unsafeck.rs +++ b/tests/ui/explicit-tail-calls/unsafeck.rs @@ -1,4 +1,4 @@ -#![allow(incomplete_features)] +#![expect(incomplete_features)] #![feature(explicit_tail_calls)] const fn f() { From d93ea6bc79a06bcf6c8cbbd3af24f8724acd5009 Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Fri, 29 Nov 2024 20:25:57 +0100 Subject: [PATCH 03/27] simplify things by using `tcx.fn_trait_kind_from_def_id` --- .../rustc_mir_build/src/check_tail_calls.rs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_mir_build/src/check_tail_calls.rs b/compiler/rustc_mir_build/src/check_tail_calls.rs index 911a6cb7de606..f7264341c0a8b 100644 --- a/compiler/rustc_mir_build/src/check_tail_calls.rs +++ b/compiler/rustc_mir_build/src/check_tail_calls.rs @@ -91,22 +91,16 @@ impl<'tcx> TailCallCkVisitor<'_, 'tcx> { // Closures in thir look something akin to // `for<'a> extern "rust-call" fn(&'a [closure@...], ()) -> <[closure@...] as FnOnce<()>>::Output {<[closure@...] as Fn<()>>::call}` // So we have to check for them in this weird way... - if let &ty::FnDef(did, substs) = ty.kind() { + if let &ty::FnDef(did, args) = ty.kind() { let parent = self.tcx.parent(did); - let fn_ = self.tcx.require_lang_item(LangItem::Fn, Some(expr.span)); - let fn_once = self.tcx.require_lang_item(LangItem::FnOnce, Some(expr.span)); - let fn_mut = self.tcx.require_lang_item(LangItem::FnMut, Some(expr.span)); - if [fn_, fn_once, fn_mut].contains(&parent) { - if substs.first().and_then(|arg| arg.as_type()).is_some_and(|t| t.is_closure()) { - self.report_calling_closure( - &self.thir[fun], - substs[1].as_type().unwrap(), - expr, - ); - - // Tail calling is likely to cause unrelated errors (ABI, argument mismatches) - return; - } + if self.tcx.fn_trait_kind_from_def_id(parent).is_some() + && args.first().and_then(|arg| arg.as_type()).is_some_and(Ty::is_closure) + { + self.report_calling_closure(&self.thir[fun], args[1].as_type().unwrap(), expr); + + // Tail calling is likely to cause unrelated errors (ABI, argument mismatches), + // skip them, producing an error about calling a closure is enough. + return; }; } From ef5808a03596385b32bdb74de55ac10c6e74f0ba Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Fri, 29 Nov 2024 20:26:30 +0100 Subject: [PATCH 04/27] add a fixme for tailcalls with opaque types --- compiler/rustc_mir_build/src/check_tail_calls.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/compiler/rustc_mir_build/src/check_tail_calls.rs b/compiler/rustc_mir_build/src/check_tail_calls.rs index f7264341c0a8b..e6a25634e29d3 100644 --- a/compiler/rustc_mir_build/src/check_tail_calls.rs +++ b/compiler/rustc_mir_build/src/check_tail_calls.rs @@ -117,6 +117,14 @@ impl<'tcx> TailCallCkVisitor<'_, 'tcx> { self.report_arguments_mismatch(expr.span, caller_sig, callee_sig); } + // FIXME(explicit_tail_calls): this currenly fails for cases where opaques are used. + // e.g. + // ``` + // fn a() -> impl Sized { become b() } // ICE + // fn b() -> u8 { 0 } + // ``` + // we should think what is the expected behavior here. + // (we should probably just accept this by revealing opaques?) if caller_sig.output() != callee_sig.output() { span_bug!(expr.span, "hir typeck should have checked the return type already"); } From c6454dd582c66a53bbf91947098af59689f61c1e Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Fri, 29 Nov 2024 20:27:24 +0100 Subject: [PATCH 05/27] don't polymorphize without a reason to --- compiler/rustc_mir_build/src/check_tail_calls.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_mir_build/src/check_tail_calls.rs b/compiler/rustc_mir_build/src/check_tail_calls.rs index e6a25634e29d3..6eade2049702e 100644 --- a/compiler/rustc_mir_build/src/check_tail_calls.rs +++ b/compiler/rustc_mir_build/src/check_tail_calls.rs @@ -153,8 +153,7 @@ impl<'tcx> TailCallCkVisitor<'_, 'tcx> { fn needs_location(&self, ty: Ty<'tcx>) -> bool { if let &ty::FnDef(did, substs) = ty.kind() { let instance = - ty::Instance::expect_resolve(self.tcx, self.typing_env, did, substs, DUMMY_SP) - .polymorphize(self.tcx); + ty::Instance::expect_resolve(self.tcx, self.typing_env, did, substs, DUMMY_SP); instance.def.requires_caller_location(self.tcx) } else { From 144d6cc65ba5d3ebdf1e20cc1a27bd3964402921 Mon Sep 17 00:00:00 2001 From: Maybe Lapkin Date: Fri, 29 Nov 2024 20:28:02 +0100 Subject: [PATCH 06/27] simplify things using `tcx.as_lang_item` --- .../rustc_mir_build/src/check_tail_calls.rs | 52 +++++++++---------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_mir_build/src/check_tail_calls.rs b/compiler/rustc_mir_build/src/check_tail_calls.rs index 6eade2049702e..b1f46d37d5062 100644 --- a/compiler/rustc_mir_build/src/check_tail_calls.rs +++ b/compiler/rustc_mir_build/src/check_tail_calls.rs @@ -354,33 +354,31 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for TailCallCkVisitor<'a, 'tcx> { } fn op_trait_as_method_name(tcx: TyCtxt<'_>, trait_did: DefId) -> Option<&'static str> { - let trait_did = Some(trait_did); - let items = tcx.lang_items(); - let m = match () { - _ if trait_did == items.get(LangItem::Add) => "add", - _ if trait_did == items.get(LangItem::Sub) => "sub", - _ if trait_did == items.get(LangItem::Mul) => "mul", - _ if trait_did == items.get(LangItem::Div) => "div", - _ if trait_did == items.get(LangItem::Rem) => "rem", - _ if trait_did == items.get(LangItem::Neg) => "neg", - _ if trait_did == items.get(LangItem::Not) => "not", - _ if trait_did == items.get(LangItem::BitXor) => "bitxor", - _ if trait_did == items.get(LangItem::BitAnd) => "bitand", - _ if trait_did == items.get(LangItem::BitOr) => "bitor", - _ if trait_did == items.get(LangItem::Shl) => "shl", - _ if trait_did == items.get(LangItem::Shr) => "shr", - _ if trait_did == items.get(LangItem::AddAssign) => "add_assign", - _ if trait_did == items.get(LangItem::SubAssign) => "sub_assign", - _ if trait_did == items.get(LangItem::MulAssign) => "mul_assign", - _ if trait_did == items.get(LangItem::DivAssign) => "div_assign", - _ if trait_did == items.get(LangItem::RemAssign) => "rem_assign", - _ if trait_did == items.get(LangItem::BitXorAssign) => "bitxor_assign", - _ if trait_did == items.get(LangItem::BitAndAssign) => "bitand_assign", - _ if trait_did == items.get(LangItem::BitOrAssign) => "bitor_assign", - _ if trait_did == items.get(LangItem::ShlAssign) => "shl_assign", - _ if trait_did == items.get(LangItem::ShrAssign) => "shr_assign", - _ if trait_did == items.get(LangItem::Index) => "index", - _ if trait_did == items.get(LangItem::IndexMut) => "index_mut", + let m = match tcx.as_lang_item(trait_did)? { + LangItem::Add => "add", + LangItem::Sub => "sub", + LangItem::Mul => "mul", + LangItem::Div => "div", + LangItem::Rem => "rem", + LangItem::Neg => "neg", + LangItem::Not => "not", + LangItem::BitXor => "bitxor", + LangItem::BitAnd => "bitand", + LangItem::BitOr => "bitor", + LangItem::Shl => "shl", + LangItem::Shr => "shr", + LangItem::AddAssign => "add_assign", + LangItem::SubAssign => "sub_assign", + LangItem::MulAssign => "mul_assign", + LangItem::DivAssign => "div_assign", + LangItem::RemAssign => "rem_assign", + LangItem::BitXorAssign => "bitxor_assign", + LangItem::BitAndAssign => "bitand_assign", + LangItem::BitOrAssign => "bitor_assign", + LangItem::ShlAssign => "shl_assign", + LangItem::ShrAssign => "shr_assign", + LangItem::Index => "index", + LangItem::IndexMut => "index_mut", _ => return None, }; From 9142cae7d5b86ac70f815c32c0f3b1223b62a947 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 8 Aug 2024 10:20:40 +0200 Subject: [PATCH 07/27] add `LinkageInfo` to keep track of how we figured out the linkage --- .../rustc_codegen_cranelift/src/driver/mod.rs | 2 +- compiler/rustc_codegen_gcc/src/base.rs | 2 +- compiler/rustc_codegen_llvm/src/base.rs | 2 +- .../src/back/symbol_export.rs | 4 +- compiler/rustc_codegen_ssa/src/mono_item.rs | 12 ++--- compiler/rustc_middle/src/mir/mono.rs | 46 ++++++++++++++++- .../rustc_monomorphize/src/partitioning.rs | 49 ++++++++++--------- 7 files changed, 82 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/driver/mod.rs b/compiler/rustc_codegen_cranelift/src/driver/mod.rs index fb0eed07c1971..dbc913528f44a 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/mod.rs @@ -30,7 +30,7 @@ fn predefine_mono_items<'tcx>( get_function_sig(tcx, module.target_config().default_call_conv, instance); let linkage = crate::linkage::get_clif_linkage( mono_item, - data.linkage, + data.linkage_info.into_linkage(), data.visibility, is_compiler_builtins, ); diff --git a/compiler/rustc_codegen_gcc/src/base.rs b/compiler/rustc_codegen_gcc/src/base.rs index 18aa32754e1d9..844871a714671 100644 --- a/compiler/rustc_codegen_gcc/src/base.rs +++ b/compiler/rustc_codegen_gcc/src/base.rs @@ -213,7 +213,7 @@ pub fn compile_codegen_unit( let mono_items = cgu.items_in_deterministic_order(tcx); for &(mono_item, data) in &mono_items { - mono_item.predefine::>(&cx, data.linkage, data.visibility); + mono_item.predefine::>(&cx, data.linkage_info, data.visibility); } // ... and now that we have everything pre-defined, fill out those definitions. diff --git a/compiler/rustc_codegen_llvm/src/base.rs b/compiler/rustc_codegen_llvm/src/base.rs index f62310bd94808..82c3cfe9aa04a 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -86,7 +86,7 @@ pub(crate) fn compile_codegen_unit( let cx = CodegenCx::new(tcx, cgu, &llvm_module); let mono_items = cx.codegen_unit.items_in_deterministic_order(cx.tcx); for &(mono_item, data) in &mono_items { - mono_item.predefine::>(&cx, data.linkage, data.visibility); + mono_item.predefine::>(&cx, data.linkage_info, data.visibility); } // ... and now that we have everything pre-defined, fill out those definitions. diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs index 788a8a13b3ee4..0321a244f8683 100644 --- a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs +++ b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs @@ -283,7 +283,7 @@ fn exported_symbols_provider_local( } if tcx.local_crate_exports_generics() { - use rustc_middle::mir::mono::{Linkage, MonoItem, Visibility}; + use rustc_middle::mir::mono::{MonoItem, Visibility}; use rustc_middle::ty::InstanceKind; // Normally, we require that shared monomorphizations are not hidden, @@ -298,7 +298,7 @@ fn exported_symbols_provider_local( // The symbols created in this loop are sorted below it #[allow(rustc::potential_query_instability)] for (mono_item, data) in cgus.iter().flat_map(|cgu| cgu.items().iter()) { - if data.linkage != Linkage::External { + if !data.linkage_info.is_external() { // We can only re-use things with external linkage, otherwise // we'll get a linker error continue; diff --git a/compiler/rustc_codegen_ssa/src/mono_item.rs b/compiler/rustc_codegen_ssa/src/mono_item.rs index 038c5857face1..04159422e3cb2 100644 --- a/compiler/rustc_codegen_ssa/src/mono_item.rs +++ b/compiler/rustc_codegen_ssa/src/mono_item.rs @@ -1,8 +1,8 @@ use rustc_hir as hir; use rustc_middle::mir::interpret::ErrorHandled; -use rustc_middle::mir::mono::{Linkage, MonoItem, Visibility}; -use rustc_middle::ty::Instance; +use rustc_middle::mir::mono::{LinkageInfo, MonoItem, Visibility}; use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf}; +use rustc_middle::ty::Instance; use rustc_middle::{span_bug, ty}; use tracing::debug; @@ -14,7 +14,7 @@ pub trait MonoItemExt<'a, 'tcx> { fn predefine>( &self, cx: &'a Bx::CodegenCx, - linkage: Linkage, + linkage_info: LinkageInfo, visibility: Visibility, ); fn to_raw_string(&self) -> String; @@ -116,7 +116,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> { fn predefine>( &self, cx: &'a Bx::CodegenCx, - linkage: Linkage, + linkage_info: LinkageInfo, visibility: Visibility, ) { debug!( @@ -132,10 +132,10 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> { match *self { MonoItem::Static(def_id) => { - cx.predefine_static(def_id, linkage, visibility, symbol_name); + cx.predefine_static(def_id, linkage_info.into_linkage(), visibility, symbol_name); } MonoItem::Fn(instance) => { - cx.predefine_fn(instance, linkage, visibility, symbol_name); + cx.predefine_fn(instance, linkage_info.into_linkage(), visibility, symbol_name); } MonoItem::GlobalAsm(..) => {} } diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index 161716610fe63..ca230d7fa8584 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/compiler/rustc_middle/src/mir/mono.rs @@ -266,13 +266,57 @@ pub struct MonoItemData { /// `GloballyShared` maps to `false` and `LocalCopy` maps to `true`. pub inlined: bool, - pub linkage: Linkage, + pub linkage_info: LinkageInfo, pub visibility: Visibility, /// A cached copy of the result of `MonoItem::size_estimate`. pub size_estimate: usize, } +/// Stores how we know what linkage to use +#[derive(Copy, Clone, PartialEq, Debug, TyEncodable, TyDecodable, HashStable)] +pub enum LinkageInfo { + /// The linkage was specified explicitly (e.g. using #[linkage = "..."]) + Explicit(Linkage), + /// Assume the symbol may be used from other CGUs, a safe default + ImplicitExternal, + /// We did not find any uses from other CGUs, so it's fine to make this internal + ImplicitInternal, +} + +impl LinkageInfo { + pub const fn is_external(self) -> bool { + matches!(self.into_linkage(), Linkage::External) + } + + pub const fn into_linkage(self) -> Linkage { + match self { + Self::Explicit(linkage) => linkage, + Self::ImplicitExternal => Linkage::External, + Self::ImplicitInternal => Linkage::Internal, + } + } + + /// Linkage when the MonoItem is a naked function + /// + /// Naked functions are generated as a separate declaration (effectively an extern fn) and + /// definition (using global assembly). To link them together, some flavor of external linkage + /// must be used. + pub const fn into_naked_linkage(self) -> Linkage { + match self { + // promote Weak linkage to ExternalWeak + Self::Explicit(Linkage::WeakAny | Linkage::WeakODR) => Linkage::ExternalWeak, + Self::Explicit(linkage) => linkage, + + // the "implicit" means that linkage is picked by the partitioning algorithm. + // picking external should always be valid (given that we are in fact linking + // to the global assembly in the same CGU) + Self::ImplicitExternal => Linkage::External, + Self::ImplicitInternal => Linkage::External, + } + } +} + /// Specifies the linkage type for a `MonoItem`. /// /// See for more details about these variants. diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 7ea4ded2b052c..2fd89e389e547 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -109,8 +109,8 @@ use rustc_middle::bug; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::middle::exported_symbols::{SymbolExportInfo, SymbolExportLevel}; use rustc_middle::mir::mono::{ - CodegenUnit, CodegenUnitNameBuilder, InstantiationMode, Linkage, MonoItem, MonoItemData, - Visibility, + CodegenUnit, CodegenUnitNameBuilder, InstantiationMode, Linkage, LinkageInfo, MonoItem, + MonoItemData, Visibility, }; use rustc_middle::ty::print::{characteristic_def_id_of_type, with_no_trimmed_paths}; use rustc_middle::ty::visit::TypeVisitableExt; @@ -245,7 +245,7 @@ where let cgu = codegen_units.entry(cgu_name).or_insert_with(|| CodegenUnit::new(cgu_name)); let mut can_be_internalized = true; - let (linkage, visibility) = mono_item_linkage_and_visibility( + let (linkage_info, visibility) = mono_item_linkage_info_and_visibility( cx.tcx, &mono_item, &mut can_be_internalized, @@ -259,7 +259,7 @@ where cgu.items_mut().insert(mono_item, MonoItemData { inlined: false, - linkage, + linkage_info, visibility, size_estimate, }); @@ -278,7 +278,7 @@ where // This is a CGU-private copy. cgu.items_mut().entry(inlined_item).or_insert_with(|| MonoItemData { inlined: true, - linkage: Linkage::Internal, + linkage_info: LinkageInfo::ImplicitInternal, visibility: Visibility::Default, size_estimate: inlined_item.size_estimate(cx.tcx), }); @@ -589,7 +589,8 @@ fn internalize_symbols<'tcx>( // If we got here, we did not find any uses from other CGUs, so // it's fine to make this monomorphization internal. - data.linkage = Linkage::Internal; + debug_assert_eq!(data.linkage_info, LinkageInfo::ImplicitExternal); + data.linkage_info = LinkageInfo::ImplicitInternal; data.visibility = Visibility::Default; } } @@ -607,7 +608,7 @@ fn mark_code_coverage_dead_code_cgu<'tcx>(codegen_units: &mut [CodegenUnit<'tcx> // function symbols to be included via `-u` or `/include` linker args. let dead_code_cgu = codegen_units .iter_mut() - .filter(|cgu| cgu.items().iter().any(|(_, data)| data.linkage == Linkage::External)) + .filter(|cgu| cgu.items().iter().any(|(_, data)| data.linkage_info.is_external())) .min_by_key(|cgu| cgu.size_estimate()); // If there are no CGUs that have externally linked items, then we just @@ -736,24 +737,26 @@ fn fallback_cgu_name(name_builder: &mut CodegenUnitNameBuilder<'_>) -> Symbol { name_builder.build_cgu_name(LOCAL_CRATE, &["fallback"], Some("cgu")) } -fn mono_item_linkage_and_visibility<'tcx>( +fn mono_item_linkage_info_and_visibility<'tcx>( tcx: TyCtxt<'tcx>, mono_item: &MonoItem<'tcx>, can_be_internalized: &mut bool, can_export_generics: bool, always_export_generics: bool, -) -> (Linkage, Visibility) { +) -> (LinkageInfo, Visibility) { if let Some(explicit_linkage) = mono_item.explicit_linkage(tcx) { - return (explicit_linkage, Visibility::Default); + (LinkageInfo::Explicit(explicit_linkage), Visibility::Default) + } else { + let vis = mono_item_visibility( + tcx, + mono_item, + can_be_internalized, + can_export_generics, + always_export_generics, + ); + + (LinkageInfo::ImplicitExternal, vis) } - let vis = mono_item_visibility( - tcx, - mono_item, - can_be_internalized, - can_export_generics, - always_export_generics, - ); - (Linkage::External, vis) } type CguNameCache = UnordMap<(DefId, bool), Symbol>; @@ -1033,7 +1036,7 @@ fn debug_dump<'a, 'tcx: 'a>(tcx: TyCtxt<'tcx>, label: &str, cgus: &[CodegenUnit< writeln!(s, " - items: {num_items}, mean size: {mean_size:.1}, sizes: {sizes}",); for (item, data) in cgu.items_in_deterministic_order(tcx) { - let linkage = data.linkage; + let linkage_info = data.linkage_info; let symbol_name = item.symbol_name(tcx).name; let symbol_hash_start = symbol_name.rfind('h'); let symbol_hash = symbol_hash_start.map_or("", |i| &symbol_name[i..]); @@ -1041,7 +1044,7 @@ fn debug_dump<'a, 'tcx: 'a>(tcx: TyCtxt<'tcx>, label: &str, cgus: &[CodegenUnit< let size = data.size_estimate; let _ = with_no_trimmed_paths!(writeln!( s, - " - {item} [{linkage:?}] [{symbol_hash}] ({kind}, size: {size})" + " - {item} [{linkage_info:?}] [{symbol_hash}] ({kind}, size: {size})" )); } @@ -1194,7 +1197,7 @@ fn collect_and_partition_mono_items(tcx: TyCtxt<'_>, (): ()) -> (&DefIdSet, &[Co for cgu in codegen_units { for (&mono_item, &data) in cgu.items() { - item_to_cgus.entry(mono_item).or_default().push((cgu.name(), data.linkage)); + item_to_cgus.entry(mono_item).or_default().push((cgu.name(), data.linkage_info)); } } @@ -1207,11 +1210,11 @@ fn collect_and_partition_mono_items(tcx: TyCtxt<'_>, (): ()) -> (&DefIdSet, &[Co let cgus = item_to_cgus.get_mut(i).unwrap_or(&mut empty); cgus.sort_by_key(|(name, _)| *name); cgus.dedup(); - for &(ref cgu_name, linkage) in cgus.iter() { + for &(ref cgu_name, linkage_info) in cgus.iter() { output.push(' '); output.push_str(cgu_name.as_str()); - let linkage_abbrev = match linkage { + let linkage_abbrev = match linkage_info.into_linkage() { Linkage::External => "External", Linkage::AvailableExternally => "Available", Linkage::LinkOnceAny => "OnceAny", From bdf64e134d2d55919c567c891ddc00b757fcd27c Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 19 Jul 2024 23:04:32 +0200 Subject: [PATCH 08/27] squashed changes to inlining and const eval first steps of codegen for `#[naked]` functions using `global_asm!` configure external linkage if no linkage is explicitly set create a `FunctionCx` and properly evaluate consts inline attribute is no longer relevant for naked functions the naked attribute no longer needs to be set by llvm/... we're generating global asm now, so this attribute is meaningless for the codegen backend --- .../rustc_codegen_ssa/src/codegen_attrs.rs | 11 +- compiler/rustc_codegen_ssa/src/mir/mod.rs | 27 +++ .../rustc_codegen_ssa/src/mir/naked_asm.rs | 212 ++++++++++++++++++ tests/codegen/naked-fn/naked-noinline.rs | 31 --- 4 files changed, 248 insertions(+), 33 deletions(-) create mode 100644 compiler/rustc_codegen_ssa/src/mir/naked_asm.rs delete mode 100644 tests/codegen/naked-fn/naked-noinline.rs diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index a5acd8170ab81..0768b01cb15e5 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -113,7 +113,10 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { sym::rustc_allocator_zeroed => { codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR_ZEROED } - sym::naked => codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED, + sym::naked => { + // this attribute is ignored during codegen, because a function marked as naked is + // turned into a global asm block. + } sym::no_mangle => { if tcx.opt_item_name(did.to_def_id()).is_some() { codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE @@ -627,7 +630,11 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { - codegen_fn_attrs.inline = InlineAttr::Never; + // naked functions are generated using an extern function and global assembly. To + // make sure that these can be linked together by LLVM, the linkage should be external, + // unless the user explicitly configured something else (in which case any linker errors + // they encounter are their responsibility). + codegen_fn_attrs.linkage = codegen_fn_attrs.linkage.or(Some(Linkage::External)); } // Weak lang items have the same semantics as "std internal" symbols in the diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index 0cbc5c45736e8..b14e43acb6078 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -20,6 +20,7 @@ mod coverageinfo; pub mod debuginfo; mod intrinsic; mod locals; +mod naked_asm; pub mod operand; pub mod place; mod rvalue; @@ -176,6 +177,32 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( let fn_abi = cx.fn_abi_of_instance(instance, ty::List::empty()); debug!("fn_abi: {:?}", fn_abi); + if cx.tcx().has_attr(instance.def.def_id(), rustc_span::sym::naked) { + let cached_llbbs = IndexVec::new(); + + let fx: FunctionCx<'_, '_, Bx> = FunctionCx { + instance, + mir, + llfn, + fn_abi, + cx, + personality_slot: None, + cached_llbbs, + unreachable_block: None, + terminate_block: None, + cleanup_kinds: None, + landing_pads: IndexVec::from_elem(None, &mir.basic_blocks), + funclets: IndexVec::from_fn_n(|_| None, mir.basic_blocks.len()), + locals: locals::Locals::empty(), + debug_context: None, + per_local_var_debug_info: None, + caller_location: None, + }; + + fx.codegen_naked_asm(instance); + return; + } + let debug_context = cx.create_function_debug_context(instance, fn_abi, llfn, mir); let start_llbb = Bx::append_block(cx, llfn, "start"); diff --git a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs new file mode 100644 index 0000000000000..1f41a9aefeb73 --- /dev/null +++ b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs @@ -0,0 +1,212 @@ +use crate::common; +use crate::mir::FunctionCx; +use crate::traits::{AsmMethods, BuilderMethods, GlobalAsmOperandRef}; +use rustc_middle::bug; +use rustc_middle::mir::InlineAsmOperand; +use rustc_middle::ty; +use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf}; +use rustc_middle::ty::{Instance, TyCtxt}; + +use rustc_span::sym; +use rustc_target::asm::InlineAsmArch; + +impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { + pub fn codegen_naked_asm(&self, instance: Instance<'tcx>) { + let cx = &self.cx; + + let rustc_middle::mir::TerminatorKind::InlineAsm { + asm_macro: _, + template, + ref operands, + options, + line_spans, + targets: _, + unwind: _, + } = self.mir.basic_blocks.iter().next().unwrap().terminator().kind + else { + bug!("#[naked] functions should always terminate with an asm! block") + }; + + let operands: Vec<_> = + operands.iter().map(|op| self.inline_to_global_operand(op)).collect(); + + let (begin, end) = crate::mir::naked_asm::prefix_and_suffix(cx.tcx(), instance); + + let mut template_vec = Vec::new(); + template_vec.push(rustc_ast::ast::InlineAsmTemplatePiece::String(begin)); + template_vec.extend(template.iter().cloned()); + template_vec.push(rustc_ast::ast::InlineAsmTemplatePiece::String(end)); + + cx.codegen_global_asm(&template_vec, &operands, options, line_spans); + } + + fn inline_to_global_operand(&self, op: &InlineAsmOperand<'tcx>) -> GlobalAsmOperandRef<'tcx> { + match op { + InlineAsmOperand::Const { value } => { + let const_value = self.eval_mir_constant(value); + let string = common::asm_const_to_str( + self.cx.tcx(), + value.span, + const_value, + self.cx.layout_of(value.ty()), + ); + GlobalAsmOperandRef::Const { string } + } + InlineAsmOperand::SymFn { value } => { + let instance = match value.ty().kind() { + &ty::FnDef(def_id, args) => Instance::new(def_id, args), + _ => bug!("asm sym is not a function"), + }; + + GlobalAsmOperandRef::SymFn { instance } + } + InlineAsmOperand::SymStatic { def_id } => { + GlobalAsmOperandRef::SymStatic { def_id: *def_id } + } + InlineAsmOperand::In { .. } + | InlineAsmOperand::Out { .. } + | InlineAsmOperand::InOut { .. } + | InlineAsmOperand::Label { .. } => { + bug!("invalid operand type for naked_asm!") + } + } + } +} + +enum AsmBinaryFormat { + Elf, + Macho, + Coff, +} + +impl AsmBinaryFormat { + fn from_target(target: &rustc_target::spec::Target) -> Self { + if target.is_like_windows { + Self::Coff + } else if target.options.vendor == "apple" { + Self::Macho + } else { + Self::Elf + } + } +} + +fn prefix_and_suffix<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> (String, String) { + use std::fmt::Write; + + let target = &tcx.sess.target; + let target_arch = tcx.sess.asm_arch; + + let is_arm = target.arch == "arm"; + let is_thumb = is_arm && target.llvm_target.contains("thumb"); + + let mangle = (target.is_like_windows && matches!(target_arch, Some(InlineAsmArch::X86))) + || target.options.vendor == "apple"; + + let asm_name = format!("{}{}", if mangle { "_" } else { "" }, tcx.symbol_name(instance).name); + + let opt_section = tcx + .get_attr(instance.def.def_id(), sym::link_section) + .and_then(|attr| attr.value_str()) + .map(|attr| attr.as_str().to_string()); + + let instruction_set = + tcx.get_attr(instance.def.def_id(), sym::instruction_set).and_then(|attr| attr.value_str()); + + let (arch_prefix, arch_suffix) = if is_arm { + ( + match instruction_set { + None => match is_thumb { + true => ".thumb\n.thumb_func", + false => ".arm", + }, + Some(sym::a32) => ".arm", + Some(sym::t32) => ".thumb\n.thumb_func", + Some(other) => bug!("invalid instruction set: {other}"), + }, + match is_thumb { + true => ".thumb", + false => ".arm", + }, + ) + } else { + ("", "") + }; + + let mut begin = String::new(); + let mut end = String::new(); + match AsmBinaryFormat::from_target(&tcx.sess.target) { + AsmBinaryFormat::Elf => { + let section = opt_section.unwrap_or(format!(".text.{asm_name}")); + + let progbits = match is_arm { + true => "%progbits", + false => "@progbits", + }; + + let function = match is_arm { + true => "%function", + false => "@function", + }; + + writeln!(begin, ".pushsection {section},\"ax\", {progbits}").unwrap(); + writeln!(begin, ".balign 4").unwrap(); + writeln!(begin, ".globl {asm_name}").unwrap(); + writeln!(begin, ".hidden {asm_name}").unwrap(); + writeln!(begin, ".type {asm_name}, {function}").unwrap(); + if let Some(instruction_set) = instruction_set { + writeln!(begin, "{}", instruction_set.as_str()).unwrap(); + } + if !arch_prefix.is_empty() { + writeln!(begin, "{}", arch_prefix).unwrap(); + } + writeln!(begin, "{asm_name}:").unwrap(); + + writeln!(end).unwrap(); + writeln!(end, ".size {asm_name}, . - {asm_name}").unwrap(); + writeln!(end, ".popsection").unwrap(); + if !arch_suffix.is_empty() { + writeln!(end, "{}", arch_suffix).unwrap(); + } + } + AsmBinaryFormat::Macho => { + let section = opt_section.unwrap_or("__TEXT,__text".to_string()); + writeln!(begin, ".pushsection {},regular,pure_instructions", section).unwrap(); + writeln!(begin, ".balign 4").unwrap(); + writeln!(begin, ".globl {asm_name}").unwrap(); + writeln!(begin, ".private_extern {asm_name}").unwrap(); + if let Some(instruction_set) = instruction_set { + writeln!(begin, "{}", instruction_set.as_str()).unwrap(); + } + writeln!(begin, "{asm_name}:").unwrap(); + + writeln!(end).unwrap(); + writeln!(end, ".popsection").unwrap(); + if !arch_suffix.is_empty() { + writeln!(end, "{}", arch_suffix).unwrap(); + } + } + AsmBinaryFormat::Coff => { + let section = opt_section.unwrap_or(format!(".text.{asm_name}")); + writeln!(begin, ".pushsection {},\"xr\"", section).unwrap(); + writeln!(begin, ".balign 4").unwrap(); + writeln!(begin, ".globl {asm_name}").unwrap(); + writeln!(begin, ".def {asm_name}").unwrap(); + writeln!(begin, ".scl 2").unwrap(); + writeln!(begin, ".type 32").unwrap(); + writeln!(begin, ".endef {asm_name}").unwrap(); + if let Some(instruction_set) = instruction_set { + writeln!(begin, "{}", instruction_set.as_str()).unwrap(); + } + writeln!(begin, "{asm_name}:").unwrap(); + + writeln!(end).unwrap(); + writeln!(end, ".popsection").unwrap(); + if !arch_suffix.is_empty() { + writeln!(end, "{}", arch_suffix).unwrap(); + } + } + } + + (begin, end) +} diff --git a/tests/codegen/naked-fn/naked-noinline.rs b/tests/codegen/naked-fn/naked-noinline.rs deleted file mode 100644 index 6ea36d9678315..0000000000000 --- a/tests/codegen/naked-fn/naked-noinline.rs +++ /dev/null @@ -1,31 +0,0 @@ -// Checks that naked functions are never inlined. -//@ compile-flags: -O -Zmir-opt-level=3 -//@ needs-asm-support -//@ ignore-wasm32 -#![crate_type = "lib"] -#![feature(naked_functions)] - -use std::arch::naked_asm; - -#[naked] -#[no_mangle] -pub unsafe extern "C" fn f() { - // Check that f has naked and noinline attributes. - // - // CHECK: define {{(dso_local )?}}void @f() unnamed_addr [[ATTR:#[0-9]+]] - // CHECK-NEXT: start: - // CHECK-NEXT: call void asm - naked_asm!(""); -} - -#[no_mangle] -pub unsafe fn g() { - // Check that call to f is not inlined. - // - // CHECK-LABEL: define {{(dso_local )?}}void @g() - // CHECK-NEXT: start: - // CHECK-NEXT: call void @f() - f(); -} - -// CHECK: attributes [[ATTR]] = { naked{{.*}}noinline{{.*}} } From d67a5c0b25e8fd02f2397d06c5857f96314e6e90 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 20 Jul 2024 01:16:49 +0200 Subject: [PATCH 09/27] test the new global asm output of naked functions --- tests/codegen/naked-fn/naked-functions.rs | 28 +++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/tests/codegen/naked-fn/naked-functions.rs b/tests/codegen/naked-fn/naked-functions.rs index 3f7447af599ab..00d64089ad8bc 100644 --- a/tests/codegen/naked-fn/naked-functions.rs +++ b/tests/codegen/naked-fn/naked-functions.rs @@ -6,8 +6,17 @@ #![feature(naked_functions)] use std::arch::naked_asm; -// CHECK: Function Attrs: naked -// CHECK-NEXT: define{{.*}}void @naked_empty() +// CHECK: module asm ".intel_syntax" +// CHECK: .pushsection .text.naked_empty,\22ax\22, @progbits +// CHECK: .balign 4" +// CHECK: .globl naked_empty" +// CHECK: .hidden naked_empty" +// CHECK: .type naked_empty, @function" +// CHECK-LABEL: naked_empty: +// CHECK: ret +// CHECK: .popsection +// CHECK: .att_syntax + #[no_mangle] #[naked] pub unsafe extern "C" fn naked_empty() { @@ -17,8 +26,19 @@ pub unsafe extern "C" fn naked_empty() { naked_asm!("ret"); } -// CHECK: Function Attrs: naked -// CHECK-NEXT: define{{.*}}i{{[0-9]+}} @naked_with_args_and_return(i64 %0, i64 %1) +// CHECK: .intel_syntax +// CHECK: .pushsection .text.naked_with_args_and_return,\22ax\22, @progbits +// CHECK: .balign 4 +// CHECK: .globl naked_with_args_and_return +// CHECK: .hidden naked_with_args_and_return +// CHECK: .type naked_with_args_and_return, @function +// CHECK-LABEL: naked_with_args_and_return: +// CHECK: lea rax, [rdi + rsi] +// CHECK: ret +// CHECK: .size naked_with_args_and_return, . - naked_with_args_and_return +// CHECK: .popsection +// CHECK: .att_syntax + #[no_mangle] #[naked] pub unsafe extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize { From a93192ca85072a41a4f970275d118bde0b3117d5 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 20 Jul 2024 13:17:31 +0200 Subject: [PATCH 10/27] add `InstructionSetAttr::as_str` and make the type `Copy` --- compiler/rustc_attr/src/builtin.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_attr/src/builtin.rs b/compiler/rustc_attr/src/builtin.rs index 94f9727eb7fbe..c8928097eafda 100644 --- a/compiler/rustc_attr/src/builtin.rs +++ b/compiler/rustc_attr/src/builtin.rs @@ -49,12 +49,21 @@ pub enum InlineAttr { Never, } -#[derive(Clone, Encodable, Decodable, Debug, PartialEq, Eq, HashStable_Generic)] +#[derive(Copy, Clone, Encodable, Decodable, Debug, PartialEq, Eq, HashStable_Generic)] pub enum InstructionSetAttr { ArmA32, ArmT32, } +impl InstructionSetAttr { + pub fn as_str(self) -> &'static str { + match self { + Self::ArmA32 => sym::a32.as_str(), + Self::ArmT32 => sym::t32.as_str(), + } + } +} + #[derive(Clone, Encodable, Decodable, Debug, HashStable_Generic)] pub enum OptimizeAttr { None, From 49a297ab211affaf782cc7603bc413cb8e1ab715 Mon Sep 17 00:00:00 2001 From: Folkert Date: Sat, 20 Jul 2024 13:23:34 +0200 Subject: [PATCH 11/27] squashed changes to tests correctly emit `.hidden` this test was added in https://github.com/rust-lang/rust/pull/105193 but actually NO_COVERAGE is no longer a thing in the compiler. Sadly, the link to the issue is broken, so I don't know what the problem was originally, but I don't think this is relevant any more with the global asm approach rename test file because it now specifically checks for directives only used by non-macos, non-windows x86_64 add codegen tests for 4 interesting platforms add codegen test for the `#[instruction_set]` attribute add test for `#[link_section]` use `tcx.codegen_fn_attrs` to get attribute info Fix #124375 inline const monomorphization/evaluation getting rid of FunctionCx mark naked functions as `InstantiatedMode::GloballyShared` this makes sure that the function prototype is defined correctly, and we don't see LLVM complaining about a global value with invalid linkage monomorphize type given to `SymFn` remove hack that always emits `.globl` monomorphize type given to `Const` remove `linkage_directive` make naked functions always have external linkage mark naked functions as `#[inline(never)]` add test file for functional generics/const/impl/trait usage of naked functions --- compiler/rustc_codegen_llvm/src/attributes.rs | 14 +- compiler/rustc_codegen_llvm/src/mono_item.rs | 10 +- .../rustc_codegen_ssa/src/codegen_attrs.rs | 20 +- compiler/rustc_codegen_ssa/src/mir/mod.rs | 25 +-- .../rustc_codegen_ssa/src/mir/naked_asm.rs | 200 ++++++++++-------- compiler/rustc_codegen_ssa/src/mono_item.rs | 12 +- compiler/rustc_middle/src/mir/mono.rs | 8 +- tests/codegen/naked-fn/generics.rs | 117 ++++++++++ tests/codegen/naked-fn/instruction-set.rs | 43 ++++ tests/codegen/naked-fn/naked-functions.rs | 158 +++++++++++--- tests/codegen/naked-fn/naked-nocoverage.rs | 19 -- tests/crashes/124375.rs | 11 - .../ui/asm/naked-functions-instruction-set.rs | 2 +- tests/ui/asm/naked-functions.rs | 6 + 14 files changed, 443 insertions(+), 202 deletions(-) create mode 100644 tests/codegen/naked-fn/generics.rs create mode 100644 tests/codegen/naked-fn/instruction-set.rs delete mode 100644 tests/codegen/naked-fn/naked-nocoverage.rs delete mode 100644 tests/crashes/124375.rs diff --git a/compiler/rustc_codegen_llvm/src/attributes.rs b/compiler/rustc_codegen_llvm/src/attributes.rs index cb958c1d4d771..5552a2410601d 100644 --- a/compiler/rustc_codegen_llvm/src/attributes.rs +++ b/compiler/rustc_codegen_llvm/src/attributes.rs @@ -395,17 +395,9 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>( to_add.push(MemoryEffects::None.create_attr(cx.llcx)); } if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { - to_add.push(AttributeKind::Naked.create_attr(cx.llcx)); - // HACK(jubilee): "indirect branch tracking" works by attaching prologues to functions. - // And it is a module-level attribute, so the alternative is pulling naked functions into - // new LLVM modules. Otherwise LLVM's "naked" functions come with endbr prefixes per - // https://github.com/rust-lang/rust/issues/98768 - to_add.push(AttributeKind::NoCfCheck.create_attr(cx.llcx)); - if llvm_util::get_version() < (19, 0, 0) { - // Prior to LLVM 19, branch-target-enforcement was disabled by setting the attribute to - // the string "false". Now it is disabled by absence of the attribute. - to_add.push(llvm::CreateAttrStringValue(cx.llcx, "branch-target-enforcement", "false")); - } + // do nothing; a naked function is converted into an extern function + // and a global assembly block. LLVM's support for naked functions is + // not used. } else { // Do not set sanitizer attributes for naked functions. to_add.extend(sanitize_attrs(cx, codegen_fn_attrs.no_sanitize)); diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index 33789c6261f10..4d1704f2c998f 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -2,6 +2,7 @@ use rustc_codegen_ssa::traits::*; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_middle::bug; +use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::mono::{Linkage, Visibility}; use rustc_middle::ty::layout::{FnAbiOf, HasTypingEnv, LayoutOf}; use rustc_middle::ty::{self, Instance, TypeVisitableExt}; @@ -58,8 +59,15 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty()); let lldecl = self.declare_fn(symbol_name, fn_abi, Some(instance)); - llvm::set_linkage(lldecl, base::linkage_to_llvm(linkage)); let attrs = self.tcx.codegen_fn_attrs(instance.def_id()); + let llvm_linkage = + if attrs.flags.contains(CodegenFnAttrFlags::NAKED) && linkage == Linkage::Internal { + // this is effectively an extern fn, and must have external linkage + llvm::Linkage::ExternalLinkage + } else { + base::linkage_to_llvm(linkage) + }; + llvm::set_linkage(lldecl, llvm_linkage); base::set_link_section(lldecl, attrs); if (linkage == Linkage::LinkOnceODR || linkage == Linkage::WeakODR) && self.tcx.sess.target.supports_comdat() diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 0768b01cb15e5..01dd75c5ee85e 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -113,10 +113,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { sym::rustc_allocator_zeroed => { codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR_ZEROED } - sym::naked => { - // this attribute is ignored during codegen, because a function marked as naked is - // turned into a global asm block. - } + sym::naked => codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED, sym::no_mangle => { if tcx.opt_item_name(did.to_def_id()).is_some() { codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE @@ -545,6 +542,13 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } }); + // naked function MUST NOT be inlined! This attribute is required for the rust compiler itself, + // but not for the code generation backend because at that point the naked function will just be + // a declaration, with a definition provided in global assembly. + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { + codegen_fn_attrs.inline = InlineAttr::Never; + } + codegen_fn_attrs.optimize = attrs.iter().fold(OptimizeAttr::None, |ia, attr| { if !attr.has_name(sym::optimize) { return ia; @@ -629,14 +633,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { } } - if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { - // naked functions are generated using an extern function and global assembly. To - // make sure that these can be linked together by LLVM, the linkage should be external, - // unless the user explicitly configured something else (in which case any linker errors - // they encounter are their responsibility). - codegen_fn_attrs.linkage = codegen_fn_attrs.linkage.or(Some(Linkage::External)); - } - // Weak lang items have the same semantics as "std internal" symbols in the // sense that they're preserved through all our LTO passes and only // strippable by the linker. diff --git a/compiler/rustc_codegen_ssa/src/mir/mod.rs b/compiler/rustc_codegen_ssa/src/mir/mod.rs index b14e43acb6078..62f69af3f2f75 100644 --- a/compiler/rustc_codegen_ssa/src/mir/mod.rs +++ b/compiler/rustc_codegen_ssa/src/mir/mod.rs @@ -177,29 +177,8 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( let fn_abi = cx.fn_abi_of_instance(instance, ty::List::empty()); debug!("fn_abi: {:?}", fn_abi); - if cx.tcx().has_attr(instance.def.def_id(), rustc_span::sym::naked) { - let cached_llbbs = IndexVec::new(); - - let fx: FunctionCx<'_, '_, Bx> = FunctionCx { - instance, - mir, - llfn, - fn_abi, - cx, - personality_slot: None, - cached_llbbs, - unreachable_block: None, - terminate_block: None, - cleanup_kinds: None, - landing_pads: IndexVec::from_elem(None, &mir.basic_blocks), - funclets: IndexVec::from_fn_n(|_| None, mir.basic_blocks.len()), - locals: locals::Locals::empty(), - debug_context: None, - per_local_var_debug_info: None, - caller_location: None, - }; - - fx.codegen_naked_asm(instance); + if cx.tcx().codegen_fn_attrs(instance.def_id()).flags.contains(CodegenFnAttrFlags::NAKED) { + crate::mir::naked_asm::codegen_naked_asm::(cx, &mir, instance); return; } diff --git a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs index 1f41a9aefeb73..b4eb6110da8ca 100644 --- a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs +++ b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs @@ -1,74 +1,99 @@ -use crate::common; -use crate::mir::FunctionCx; -use crate::traits::{AsmMethods, BuilderMethods, GlobalAsmOperandRef}; -use rustc_middle::bug; -use rustc_middle::mir::InlineAsmOperand; -use rustc_middle::ty; -use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf}; +use rustc_attr::InstructionSetAttr; +use rustc_middle::mir::mono::{MonoItem, MonoItemData, Visibility}; +use rustc_middle::mir::{Body, InlineAsmOperand}; +use rustc_middle::ty::layout::{HasTyCtxt, HasTypingEnv, LayoutOf}; use rustc_middle::ty::{Instance, TyCtxt}; - -use rustc_span::sym; +use rustc_middle::{bug, ty}; use rustc_target::asm::InlineAsmArch; -impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { - pub fn codegen_naked_asm(&self, instance: Instance<'tcx>) { - let cx = &self.cx; - - let rustc_middle::mir::TerminatorKind::InlineAsm { - asm_macro: _, - template, - ref operands, - options, - line_spans, - targets: _, - unwind: _, - } = self.mir.basic_blocks.iter().next().unwrap().terminator().kind - else { - bug!("#[naked] functions should always terminate with an asm! block") - }; - - let operands: Vec<_> = - operands.iter().map(|op| self.inline_to_global_operand(op)).collect(); - - let (begin, end) = crate::mir::naked_asm::prefix_and_suffix(cx.tcx(), instance); - - let mut template_vec = Vec::new(); - template_vec.push(rustc_ast::ast::InlineAsmTemplatePiece::String(begin)); - template_vec.extend(template.iter().cloned()); - template_vec.push(rustc_ast::ast::InlineAsmTemplatePiece::String(end)); - - cx.codegen_global_asm(&template_vec, &operands, options, line_spans); - } +use crate::common; +use crate::traits::{AsmCodegenMethods, BuilderMethods, GlobalAsmOperandRef, MiscCodegenMethods}; + +pub(crate) fn codegen_naked_asm<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( + cx: &'a Bx::CodegenCx, + mir: &Body<'tcx>, + instance: Instance<'tcx>, +) { + let rustc_middle::mir::TerminatorKind::InlineAsm { + asm_macro: _, + template, + ref operands, + options, + line_spans, + targets: _, + unwind: _, + } = mir.basic_blocks.iter().next().unwrap().terminator().kind + else { + bug!("#[naked] functions should always terminate with an asm! block") + }; - fn inline_to_global_operand(&self, op: &InlineAsmOperand<'tcx>) -> GlobalAsmOperandRef<'tcx> { - match op { - InlineAsmOperand::Const { value } => { - let const_value = self.eval_mir_constant(value); - let string = common::asm_const_to_str( - self.cx.tcx(), - value.span, - const_value, - self.cx.layout_of(value.ty()), - ); - GlobalAsmOperandRef::Const { string } - } - InlineAsmOperand::SymFn { value } => { - let instance = match value.ty().kind() { - &ty::FnDef(def_id, args) => Instance::new(def_id, args), - _ => bug!("asm sym is not a function"), - }; + let operands: Vec<_> = + operands.iter().map(|op| inline_to_global_operand::(cx, instance, op)).collect(); - GlobalAsmOperandRef::SymFn { instance } - } - InlineAsmOperand::SymStatic { def_id } => { - GlobalAsmOperandRef::SymStatic { def_id: *def_id } - } - InlineAsmOperand::In { .. } - | InlineAsmOperand::Out { .. } - | InlineAsmOperand::InOut { .. } - | InlineAsmOperand::Label { .. } => { - bug!("invalid operand type for naked_asm!") - } + let item_data = cx.codegen_unit().items().get(&MonoItem::Fn(instance)).unwrap(); + let (begin, end) = crate::mir::naked_asm::prefix_and_suffix(cx.tcx(), instance, item_data); + + let mut template_vec = Vec::new(); + template_vec.push(rustc_ast::ast::InlineAsmTemplatePiece::String(begin.into())); + template_vec.extend(template.iter().cloned()); + template_vec.push(rustc_ast::ast::InlineAsmTemplatePiece::String(end.into())); + + cx.codegen_global_asm(&template_vec, &operands, options, line_spans); +} + +fn inline_to_global_operand<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( + cx: &'a Bx::CodegenCx, + instance: Instance<'tcx>, + op: &InlineAsmOperand<'tcx>, +) -> GlobalAsmOperandRef<'tcx> { + match op { + InlineAsmOperand::Const { value } => { + let const_value = instance + .instantiate_mir_and_normalize_erasing_regions( + cx.tcx(), + cx.typing_env(), + ty::EarlyBinder::bind(value.const_), + ) + .eval(cx.tcx(), cx.typing_env(), value.span) + .expect("erroneous constant missed by mono item collection"); + + let mono_type = instance.instantiate_mir_and_normalize_erasing_regions( + cx.tcx(), + cx.typing_env(), + ty::EarlyBinder::bind(value.ty()), + ); + + let string = common::asm_const_to_str( + cx.tcx(), + value.span, + const_value, + cx.layout_of(mono_type), + ); + + GlobalAsmOperandRef::Const { string } + } + InlineAsmOperand::SymFn { value } => { + let mono_type = instance.instantiate_mir_and_normalize_erasing_regions( + cx.tcx(), + cx.typing_env(), + ty::EarlyBinder::bind(value.ty()), + ); + + let instance = match mono_type.kind() { + &ty::FnDef(def_id, args) => Instance::new(def_id, args), + _ => bug!("asm sym is not a function"), + }; + + GlobalAsmOperandRef::SymFn { instance } + } + InlineAsmOperand::SymStatic { def_id } => { + GlobalAsmOperandRef::SymStatic { def_id: *def_id } + } + InlineAsmOperand::In { .. } + | InlineAsmOperand::Out { .. } + | InlineAsmOperand::InOut { .. } + | InlineAsmOperand::Label { .. } => { + bug!("invalid operand type for naked_asm!") } } } @@ -91,7 +116,11 @@ impl AsmBinaryFormat { } } -fn prefix_and_suffix<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> (String, String) { +fn prefix_and_suffix<'tcx>( + tcx: TyCtxt<'tcx>, + instance: Instance<'tcx>, + item_data: &MonoItemData, +) -> (String, String) { use std::fmt::Write; let target = &tcx.sess.target; @@ -105,24 +134,18 @@ fn prefix_and_suffix<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> (Stri let asm_name = format!("{}{}", if mangle { "_" } else { "" }, tcx.symbol_name(instance).name); - let opt_section = tcx - .get_attr(instance.def.def_id(), sym::link_section) - .and_then(|attr| attr.value_str()) - .map(|attr| attr.as_str().to_string()); - - let instruction_set = - tcx.get_attr(instance.def.def_id(), sym::instruction_set).and_then(|attr| attr.value_str()); + let attrs = tcx.codegen_fn_attrs(instance.def_id()); + let link_section = attrs.link_section.map(|symbol| symbol.as_str().to_string()); let (arch_prefix, arch_suffix) = if is_arm { ( - match instruction_set { + match attrs.instruction_set { None => match is_thumb { true => ".thumb\n.thumb_func", false => ".arm", }, - Some(sym::a32) => ".arm", - Some(sym::t32) => ".thumb\n.thumb_func", - Some(other) => bug!("invalid instruction set: {other}"), + Some(InstructionSetAttr::ArmA32) => ".arm", + Some(InstructionSetAttr::ArmT32) => ".thumb\n.thumb_func", }, match is_thumb { true => ".thumb", @@ -137,7 +160,7 @@ fn prefix_and_suffix<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> (Stri let mut end = String::new(); match AsmBinaryFormat::from_target(&tcx.sess.target) { AsmBinaryFormat::Elf => { - let section = opt_section.unwrap_or(format!(".text.{asm_name}")); + let section = link_section.unwrap_or(format!(".text.{asm_name}")); let progbits = match is_arm { true => "%progbits", @@ -152,11 +175,10 @@ fn prefix_and_suffix<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> (Stri writeln!(begin, ".pushsection {section},\"ax\", {progbits}").unwrap(); writeln!(begin, ".balign 4").unwrap(); writeln!(begin, ".globl {asm_name}").unwrap(); - writeln!(begin, ".hidden {asm_name}").unwrap(); - writeln!(begin, ".type {asm_name}, {function}").unwrap(); - if let Some(instruction_set) = instruction_set { - writeln!(begin, "{}", instruction_set.as_str()).unwrap(); + if let Visibility::Hidden = item_data.visibility { + writeln!(begin, ".hidden {asm_name}").unwrap(); } + writeln!(begin, ".type {asm_name}, {function}").unwrap(); if !arch_prefix.is_empty() { writeln!(begin, "{}", arch_prefix).unwrap(); } @@ -170,13 +192,12 @@ fn prefix_and_suffix<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> (Stri } } AsmBinaryFormat::Macho => { - let section = opt_section.unwrap_or("__TEXT,__text".to_string()); + let section = link_section.unwrap_or("__TEXT,__text".to_string()); writeln!(begin, ".pushsection {},regular,pure_instructions", section).unwrap(); writeln!(begin, ".balign 4").unwrap(); writeln!(begin, ".globl {asm_name}").unwrap(); - writeln!(begin, ".private_extern {asm_name}").unwrap(); - if let Some(instruction_set) = instruction_set { - writeln!(begin, "{}", instruction_set.as_str()).unwrap(); + if let Visibility::Hidden = item_data.visibility { + writeln!(begin, ".private_extern {asm_name}").unwrap(); } writeln!(begin, "{asm_name}:").unwrap(); @@ -187,7 +208,7 @@ fn prefix_and_suffix<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> (Stri } } AsmBinaryFormat::Coff => { - let section = opt_section.unwrap_or(format!(".text.{asm_name}")); + let section = link_section.unwrap_or(format!(".text.{asm_name}")); writeln!(begin, ".pushsection {},\"xr\"", section).unwrap(); writeln!(begin, ".balign 4").unwrap(); writeln!(begin, ".globl {asm_name}").unwrap(); @@ -195,9 +216,6 @@ fn prefix_and_suffix<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> (Stri writeln!(begin, ".scl 2").unwrap(); writeln!(begin, ".type 32").unwrap(); writeln!(begin, ".endef {asm_name}").unwrap(); - if let Some(instruction_set) = instruction_set { - writeln!(begin, "{}", instruction_set.as_str()).unwrap(); - } writeln!(begin, "{asm_name}:").unwrap(); writeln!(end).unwrap(); diff --git a/compiler/rustc_codegen_ssa/src/mono_item.rs b/compiler/rustc_codegen_ssa/src/mono_item.rs index 04159422e3cb2..085ef1cd8ea36 100644 --- a/compiler/rustc_codegen_ssa/src/mono_item.rs +++ b/compiler/rustc_codegen_ssa/src/mono_item.rs @@ -1,8 +1,9 @@ use rustc_hir as hir; +use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::mir::mono::{LinkageInfo, MonoItem, Visibility}; -use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf}; use rustc_middle::ty::Instance; +use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf}; use rustc_middle::{span_bug, ty}; use tracing::debug; @@ -135,7 +136,14 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> { cx.predefine_static(def_id, linkage_info.into_linkage(), visibility, symbol_name); } MonoItem::Fn(instance) => { - cx.predefine_fn(instance, linkage_info.into_linkage(), visibility, symbol_name); + let attrs = cx.tcx().codegen_fn_attrs(instance.def_id()); + let linkage = if attrs.flags.contains(CodegenFnAttrFlags::NAKED) { + linkage_info.into_naked_linkage() + } else { + linkage_info.into_linkage() + }; + + cx.predefine_fn(instance, linkage, visibility, symbol_name); } MonoItem::GlobalAsm(..) => {} } diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index ca230d7fa8584..4287b2c574ffe 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/compiler/rustc_middle/src/mir/mono.rs @@ -19,6 +19,7 @@ use rustc_target::spec::SymbolVisibility; use tracing::debug; use crate::dep_graph::{DepNode, WorkProduct, WorkProductId}; +use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags; use crate::ty::{GenericArgs, Instance, InstanceKind, SymbolName, TyCtxt}; /// Describes how a monomorphization will be instantiated in object files. @@ -104,7 +105,9 @@ impl<'tcx> MonoItem<'tcx> { let entry_def_id = tcx.entry_fn(()).map(|(id, _)| id); // If this function isn't inlined or otherwise has an extern // indicator, then we'll be creating a globally shared version. - if tcx.codegen_fn_attrs(instance.def_id()).contains_extern_indicator() + let codegen_fn_attrs = tcx.codegen_fn_attrs(instance.def_id()); + if codegen_fn_attrs.contains_extern_indicator() + || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) || !instance.def.generates_cgu_internal_copy(tcx) || Some(instance.def_id()) == entry_def_id { @@ -302,6 +305,9 @@ impl LinkageInfo { /// Naked functions are generated as a separate declaration (effectively an extern fn) and /// definition (using global assembly). To link them together, some flavor of external linkage /// must be used. + /// + /// This should be just an implementation detail of the backend, which is why this translation + /// is made at the final moment. pub const fn into_naked_linkage(self) -> Linkage { match self { // promote Weak linkage to ExternalWeak diff --git a/tests/codegen/naked-fn/generics.rs b/tests/codegen/naked-fn/generics.rs new file mode 100644 index 0000000000000..5bb40cddea56c --- /dev/null +++ b/tests/codegen/naked-fn/generics.rs @@ -0,0 +1,117 @@ +//@ compile-flags: -O +//@ only-x86_64 + +#![crate_type = "lib"] +#![feature(naked_functions, asm_const)] + +use std::arch::asm; + +#[no_mangle] +fn test(x: u64) { + // just making sure these symbols get used + using_const_generics::<1>(x); + using_const_generics::<2>(x); + + generic_function::(x as i64); + + let foo = Foo(x); + + foo.method(); + foo.trait_method(); +} + +// CHECK: .balign 4 +// CHECK: add rax, 2 +// CHECK: add rax, 42 + +// CHECK: .balign 4 +// CHECK: add rax, 1 +// CHECK: add rax, 42 + +#[naked] +pub extern "C" fn using_const_generics(x: u64) -> u64 { + const M: u64 = 42; + + unsafe { + asm!( + "xor rax, rax", + "add rax, rdi", + "add rax, {}", + "add rax, {}", + "ret", + const N, + const M, + options(noreturn), + ) + } +} + +trait Invert { + fn invert(self) -> Self; +} + +impl Invert for i64 { + fn invert(self) -> Self { + -1 * self + } +} + +// CHECK-LABEL: generic_function +// CHECK: .balign 4 +// CHECK: call +// CHECK: ret + +#[naked] +pub extern "C" fn generic_function(x: i64) -> i64 { + unsafe { + asm!( + "call {}", + "ret", + sym ::invert, + options(noreturn), + ) + } +} + +#[derive(Copy, Clone)] +#[repr(transparent)] +struct Foo(u64); + +// CHECK-LABEL: method +// CHECK: .balign 4 +// CHECK: mov rax, rdi + +impl Foo { + #[naked] + #[no_mangle] + extern "C" fn method(self) -> u64 { + unsafe { asm!("mov rax, rdi", "ret", options(noreturn)) } + } +} + +// CHECK-LABEL: trait_method +// CHECK: .balign 4 +// CHECK: mov rax, rdi + +trait Bar { + extern "C" fn trait_method(self) -> u64; +} + +impl Bar for Foo { + #[naked] + #[no_mangle] + extern "C" fn trait_method(self) -> u64 { + unsafe { asm!("mov rax, rdi", "ret", options(noreturn)) } + } +} + +// CHECK-LABEL: naked_with_args_and_return +// CHECK: .balign 4 +// CHECK: lea rax, [rdi + rsi] + +// this previously ICE'd, see https://github.com/rust-lang/rust/issues/124375 +#[naked] +#[no_mangle] +pub unsafe extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize { + asm!("lea rax, [rdi + rsi]", "ret", options(noreturn)); +} diff --git a/tests/codegen/naked-fn/instruction-set.rs b/tests/codegen/naked-fn/instruction-set.rs new file mode 100644 index 0000000000000..509c24f9457cf --- /dev/null +++ b/tests/codegen/naked-fn/instruction-set.rs @@ -0,0 +1,43 @@ +//@ compile-flags: --target armv5te-none-eabi +//@ needs-llvm-components: arm + +#![crate_type = "lib"] +#![feature(no_core, lang_items, rustc_attrs, naked_functions)] +#![no_core] + +#[rustc_builtin_macro] +macro_rules! asm { + () => {}; +} + +#[lang = "sized"] +trait Sized {} +#[lang = "copy"] +trait Copy {} + +// CHECK-LABEL: test_unspecified: +// CHECK: .arm +#[no_mangle] +#[naked] +unsafe extern "C" fn test_unspecified() { + asm!("bx lr", options(noreturn)); +} + +// CHECK-LABEL: test_thumb: +// CHECK: .thumb +// CHECK: .thumb_func +#[no_mangle] +#[naked] +#[instruction_set(arm::t32)] +unsafe extern "C" fn test_thumb() { + asm!("bx lr", options(noreturn)); +} + +// CHECK-LABEL: test_arm: +// CHECK: .arm +#[no_mangle] +#[naked] +#[instruction_set(arm::t32)] +unsafe extern "C" fn test_arm() { + asm!("bx lr", options(noreturn)); +} diff --git a/tests/codegen/naked-fn/naked-functions.rs b/tests/codegen/naked-fn/naked-functions.rs index 00d64089ad8bc..8b12693da8f66 100644 --- a/tests/codegen/naked-fn/naked-functions.rs +++ b/tests/codegen/naked-fn/naked-functions.rs @@ -1,49 +1,147 @@ -//@ compile-flags: -C no-prepopulate-passes -Copt-level=0 -//@ needs-asm-support -//@ only-x86_64 +//@ revisions: linux windows macos thumb +// +//@[linux] compile-flags: --target x86_64-unknown-linux-gnu +//@[linux] needs-llvm-components: x86 +//@[windows] compile-flags: --target x86_64-pc-windows-gnu +//@[windows] needs-llvm-components: x86 +//@[macos] compile-flags: --target aarch64-apple-darwin +//@[macos] needs-llvm-components: arm +//@[thumb] compile-flags: --target thumbv7em-none-eabi +//@[thumb] needs-llvm-components: arm #![crate_type = "lib"] -#![feature(naked_functions)] -use std::arch::naked_asm; - -// CHECK: module asm ".intel_syntax" -// CHECK: .pushsection .text.naked_empty,\22ax\22, @progbits -// CHECK: .balign 4" -// CHECK: .globl naked_empty" -// CHECK: .hidden naked_empty" -// CHECK: .type naked_empty, @function" +#![feature(no_core, lang_items, rustc_attrs, naked_functions)] +#![no_core] + +#[rustc_builtin_macro] +macro_rules! naked_asm { + () => {}; +} + +#[lang = "sized"] +trait Sized {} +#[lang = "copy"] +trait Copy {} + +// linux,windows: .intel_syntax +// +// linux: .pushsection .text.naked_empty,\22ax\22, @progbits +// macos: .pushsection __TEXT,__text,regular,pure_instructions +// windows: .pushsection .text.naked_empty,\22xr\22 +// thumb: .pushsection .text.naked_empty,\22ax\22, %progbits +// +// CHECK: .balign 4 +// +// linux,windows,thumb: .globl naked_empty +// macos: .globl _naked_empty +// +// CHECK-NOT: .private_extern +// CHECK-NOT: .hidden +// +// linux: .type naked_empty, @function +// +// windows: .def naked_empty +// windows: .scl 2 +// windows: .type 32 +// windows: .endef naked_empty +// +// thumb: .type naked_empty, %function +// thumb: .thumb +// thumb: .thumb_func +// // CHECK-LABEL: naked_empty: -// CHECK: ret +// +// linux,macos,windows: ret +// thumb: bx lr +// // CHECK: .popsection -// CHECK: .att_syntax +// +// thumb: .thumb +// +// linux,windows: .att_syntax #[no_mangle] #[naked] pub unsafe extern "C" fn naked_empty() { - // CHECK-NEXT: {{.+}}: - // CHECK-NEXT: call void asm - // CHECK-NEXT: unreachable + #[cfg(not(all(target_arch = "arm", target_feature = "thumb-mode")))] naked_asm!("ret"); + + #[cfg(all(target_arch = "arm", target_feature = "thumb-mode"))] + naked_asm!("bx lr"); } -// CHECK: .intel_syntax -// CHECK: .pushsection .text.naked_with_args_and_return,\22ax\22, @progbits +// linux,windows: .intel_syntax +// +// linux: .pushsection .text.naked_with_args_and_return,\22ax\22, @progbits +// macos: .pushsection __TEXT,__text,regular,pure_instructions +// windows: .pushsection .text.naked_with_args_and_return,\22xr\22 +// thumb: .pushsection .text.naked_with_args_and_return,\22ax\22, %progbits +// // CHECK: .balign 4 -// CHECK: .globl naked_with_args_and_return -// CHECK: .hidden naked_with_args_and_return -// CHECK: .type naked_with_args_and_return, @function +// +// linux,windows,thumb: .globl naked_with_args_and_return +// macos: .globl _naked_with_args_and_return +// +// CHECK-NOT: .private_extern +// CHECK-NOT: .hidden +// +// linux: .type naked_with_args_and_return, @function +// +// windows: .def naked_with_args_and_return +// windows: .scl 2 +// windows: .type 32 +// windows: .endef naked_with_args_and_return +// +// thumb: .type naked_with_args_and_return, %function +// thumb: .thumb +// thumb: .thumb_func +// // CHECK-LABEL: naked_with_args_and_return: -// CHECK: lea rax, [rdi + rsi] -// CHECK: ret -// CHECK: .size naked_with_args_and_return, . - naked_with_args_and_return +// +// linux, windows: lea rax, [rdi + rsi] +// macos: add x0, x0, x1 +// thumb: adds r0, r0, r1 +// +// linux,macos,windows: ret +// thumb: bx lr +// // CHECK: .popsection -// CHECK: .att_syntax +// +// thumb: .thumb +// +// linux,windows: .att_syntax #[no_mangle] #[naked] pub unsafe extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize { - // CHECK-NEXT: {{.+}}: - // CHECK-NEXT: call void asm - // CHECK-NEXT: unreachable - naked_asm!("lea rax, [rdi + rsi]", "ret"); + #[cfg(any(target_os = "windows", target_os = "linux"))] + { + naked_asm!("lea rax, [rdi + rsi]", "ret") + } + + #[cfg(target_os = "macos")] + { + naked_asm!("add x0, x0, x1", "ret") + } + + #[cfg(all(target_arch = "arm", target_feature = "thumb-mode"))] + { + naked_asm!("adds r0, r0, r1", "bx lr") + } +} + +// linux: .pushsection .text.some_different_name,\22ax\22, @progbits +// macos: .pushsection .text.some_different_name,regular,pure_instructions +// windows: .pushsection .text.some_different_name,\22xr\22 +// thumb: .pushsection .text.some_different_name,\22ax\22, %progbits +// CHECK-LABEL: test_link_section: +#[no_mangle] +#[naked] +#[link_section = ".text.some_different_name"] +pub unsafe extern "C" fn test_link_section() { + #[cfg(not(all(target_arch = "arm", target_feature = "thumb-mode")))] + naked_asm!("ret", options(noreturn)); + + #[cfg(all(target_arch = "arm", target_feature = "thumb-mode"))] + naked_asm!("bx lr", options(noreturn)); } diff --git a/tests/codegen/naked-fn/naked-nocoverage.rs b/tests/codegen/naked-fn/naked-nocoverage.rs deleted file mode 100644 index f63661bcd3a7a..0000000000000 --- a/tests/codegen/naked-fn/naked-nocoverage.rs +++ /dev/null @@ -1,19 +0,0 @@ -// Checks that naked functions are not instrumented by -Cinstrument-coverage. -// Regression test for issue #105170. -// -//@ needs-asm-support -//@ compile-flags: -Zno-profiler-runtime -//@ compile-flags: -Cinstrument-coverage -#![crate_type = "lib"] -#![feature(naked_functions)] -use std::arch::naked_asm; - -#[naked] -#[no_mangle] -pub unsafe extern "C" fn f() { - // CHECK: define {{(dso_local )?}}void @f() - // CHECK-NEXT: start: - // CHECK-NEXT: call void asm - // CHECK-NEXT: unreachable - naked_asm!(""); -} diff --git a/tests/crashes/124375.rs b/tests/crashes/124375.rs deleted file mode 100644 index 1d877caeb8bc1..0000000000000 --- a/tests/crashes/124375.rs +++ /dev/null @@ -1,11 +0,0 @@ -//@ known-bug: #124375 -//@ compile-flags: -Zmir-opt-level=0 -//@ only-x86_64 -#![crate_type = "lib"] -#![feature(naked_functions)] -use std::arch::naked_asm; - -#[naked] -pub unsafe extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize { - naked_asm!("lea rax, [rdi + rsi]", "ret"); -} diff --git a/tests/ui/asm/naked-functions-instruction-set.rs b/tests/ui/asm/naked-functions-instruction-set.rs index 37c7b52c191cd..3a6e7a46ce5a7 100644 --- a/tests/ui/asm/naked-functions-instruction-set.rs +++ b/tests/ui/asm/naked-functions-instruction-set.rs @@ -24,7 +24,7 @@ unsafe extern "C" fn test_thumb() { #[no_mangle] #[naked] -#[instruction_set(arm::t32)] +#[instruction_set(arm::a32)] unsafe extern "C" fn test_arm() { naked_asm!("bx lr"); } diff --git a/tests/ui/asm/naked-functions.rs b/tests/ui/asm/naked-functions.rs index 5c58f1498cc97..0939b5b57e5e1 100644 --- a/tests/ui/asm/naked-functions.rs +++ b/tests/ui/asm/naked-functions.rs @@ -183,6 +183,12 @@ pub unsafe extern "C" fn invalid_asm_syntax(a: u32) -> u32 { //~^ ERROR asm template must be a string literal } +// this previously ICE'd, see https://github.com/rust-lang/rust/issues/124375 +#[naked] +pub unsafe extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize { + asm!("lea rax, [rdi + rsi]", "ret", options(noreturn)); +} + #[cfg(target_arch = "x86_64")] #[cfg_attr(target_pointer_width = "64", no_mangle)] #[naked] From 589ebb815d5e078de200155f207ab7fcc550cf9c Mon Sep 17 00:00:00 2001 From: Folkert Date: Fri, 26 Jul 2024 16:54:15 +0200 Subject: [PATCH 12/27] make naked functions always have external linkage *in LLVM*. If we do it earlier, then some other logic causes invalid visibility for the item (exporting when it shouldn't). --- compiler/rustc_codegen_llvm/src/mono_item.rs | 10 +--------- tests/ui/asm/naked-functions.rs | 2 +- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index 4d1704f2c998f..b91941396ed81 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -2,7 +2,6 @@ use rustc_codegen_ssa::traits::*; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_middle::bug; -use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::mono::{Linkage, Visibility}; use rustc_middle::ty::layout::{FnAbiOf, HasTypingEnv, LayoutOf}; use rustc_middle::ty::{self, Instance, TypeVisitableExt}; @@ -60,14 +59,7 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty()); let lldecl = self.declare_fn(symbol_name, fn_abi, Some(instance)); let attrs = self.tcx.codegen_fn_attrs(instance.def_id()); - let llvm_linkage = - if attrs.flags.contains(CodegenFnAttrFlags::NAKED) && linkage == Linkage::Internal { - // this is effectively an extern fn, and must have external linkage - llvm::Linkage::ExternalLinkage - } else { - base::linkage_to_llvm(linkage) - }; - llvm::set_linkage(lldecl, llvm_linkage); + llvm::set_linkage(lldecl, base::linkage_to_llvm(linkage)); base::set_link_section(lldecl, attrs); if (linkage == Linkage::LinkOnceODR || linkage == Linkage::WeakODR) && self.tcx.sess.target.supports_comdat() diff --git a/tests/ui/asm/naked-functions.rs b/tests/ui/asm/naked-functions.rs index 0939b5b57e5e1..586645edac969 100644 --- a/tests/ui/asm/naked-functions.rs +++ b/tests/ui/asm/naked-functions.rs @@ -186,7 +186,7 @@ pub unsafe extern "C" fn invalid_asm_syntax(a: u32) -> u32 { // this previously ICE'd, see https://github.com/rust-lang/rust/issues/124375 #[naked] pub unsafe extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize { - asm!("lea rax, [rdi + rsi]", "ret", options(noreturn)); + naked_asm!("lea rax, [rdi + rsi]", "ret"); } #[cfg(target_arch = "x86_64")] From 2e24cdbf30ca0ae5473cf0aa06140e044b805f1a Mon Sep 17 00:00:00 2001 From: Folkert Date: Sun, 11 Aug 2024 12:21:42 +0200 Subject: [PATCH 13/27] squashed changes: - codegen tests: change `windows` to `win` - cleanup - fix review comments - better way of checking for thumb - get the mangled name from the codegen backend - propagate function alignment - fix gcc backend - fix asan test - check that assembler mode restored --- compiler/rustc_codegen_gcc/src/asm.rs | 7 +++ compiler/rustc_codegen_llvm/src/asm.rs | 8 +++ .../rustc_codegen_ssa/src/mir/naked_asm.rs | 31 ++++++------ compiler/rustc_codegen_ssa/src/traits/asm.rs | 7 +++ tests/codegen/naked-asan.rs | 6 ++- tests/codegen/naked-fn/aligned.rs | 9 ++-- tests/codegen/naked-fn/generics.rs | 14 +++--- tests/codegen/naked-fn/instruction-set.rs | 36 +++++++++---- tests/codegen/naked-fn/naked-functions.rs | 50 +++++++++---------- 9 files changed, 101 insertions(+), 67 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/asm.rs b/compiler/rustc_codegen_gcc/src/asm.rs index 341d1b9c179b7..199d22e02e287 100644 --- a/compiler/rustc_codegen_gcc/src/asm.rs +++ b/compiler/rustc_codegen_gcc/src/asm.rs @@ -865,6 +865,13 @@ impl<'gcc, 'tcx> AsmCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> { template_str.push_str("\n.popsection"); self.context.add_top_level_asm(None, &template_str); } + + fn mangled_name(&self, instance: Instance<'tcx>) -> String { + // TODO(@Amanieu): Additional mangling is needed on + // some targets to add a leading underscore (Mach-O) + // or byte count suffixes (x86 Windows). + self.tcx.symbol_name(instance).name.to_string() + } } fn modifier_to_gcc( diff --git a/compiler/rustc_codegen_llvm/src/asm.rs b/compiler/rustc_codegen_llvm/src/asm.rs index 9aa01bd1b956c..a6fa1fc3e19a4 100644 --- a/compiler/rustc_codegen_llvm/src/asm.rs +++ b/compiler/rustc_codegen_llvm/src/asm.rs @@ -442,6 +442,14 @@ impl<'tcx> AsmCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { ); } } + + fn mangled_name(&self, instance: Instance<'tcx>) -> String { + let llval = self.get_fn(instance); + llvm::build_string(|s| unsafe { + llvm::LLVMRustGetMangledName(llval, s); + }) + .expect("symbol is not valid UTF-8") + } } pub(crate) fn inline_asm_call<'ll>( diff --git a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs index b4eb6110da8ca..65470dd6d5371 100644 --- a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs +++ b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs @@ -4,7 +4,7 @@ use rustc_middle::mir::{Body, InlineAsmOperand}; use rustc_middle::ty::layout::{HasTyCtxt, HasTypingEnv, LayoutOf}; use rustc_middle::ty::{Instance, TyCtxt}; use rustc_middle::{bug, ty}; -use rustc_target::asm::InlineAsmArch; +use rustc_span::sym; use crate::common; use crate::traits::{AsmCodegenMethods, BuilderMethods, GlobalAsmOperandRef, MiscCodegenMethods}; @@ -31,7 +31,8 @@ pub(crate) fn codegen_naked_asm<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>( operands.iter().map(|op| inline_to_global_operand::(cx, instance, op)).collect(); let item_data = cx.codegen_unit().items().get(&MonoItem::Fn(instance)).unwrap(); - let (begin, end) = crate::mir::naked_asm::prefix_and_suffix(cx.tcx(), instance, item_data); + let name = cx.mangled_name(instance); + let (begin, end) = prefix_and_suffix(cx.tcx(), instance, &name, item_data); let mut template_vec = Vec::new(); template_vec.push(rustc_ast::ast::InlineAsmTemplatePiece::String(begin.into())); @@ -108,7 +109,7 @@ impl AsmBinaryFormat { fn from_target(target: &rustc_target::spec::Target) -> Self { if target.is_like_windows { Self::Coff - } else if target.options.vendor == "apple" { + } else if target.is_like_osx { Self::Macho } else { Self::Elf @@ -119,24 +120,20 @@ impl AsmBinaryFormat { fn prefix_and_suffix<'tcx>( tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, + asm_name: &str, item_data: &MonoItemData, ) -> (String, String) { use std::fmt::Write; - let target = &tcx.sess.target; - let target_arch = tcx.sess.asm_arch; - - let is_arm = target.arch == "arm"; - let is_thumb = is_arm && target.llvm_target.contains("thumb"); - - let mangle = (target.is_like_windows && matches!(target_arch, Some(InlineAsmArch::X86))) - || target.options.vendor == "apple"; - - let asm_name = format!("{}{}", if mangle { "_" } else { "" }, tcx.symbol_name(instance).name); + let is_arm = tcx.sess.target.arch == "arm"; + let is_thumb = tcx.sess.unstable_target_features.contains(&sym::thumb_mode); let attrs = tcx.codegen_fn_attrs(instance.def_id()); let link_section = attrs.link_section.map(|symbol| symbol.as_str().to_string()); + let align = attrs.alignment.map(|a| a.bytes()).unwrap_or(4); + // See https://sourceware.org/binutils/docs/as/ARM-Directives.html for info on these directives. + // In particular, `.arm` can also be written `.code 32` and `.thumb` as `.code 16`. let (arch_prefix, arch_suffix) = if is_arm { ( match attrs.instruction_set { @@ -144,8 +141,8 @@ fn prefix_and_suffix<'tcx>( true => ".thumb\n.thumb_func", false => ".arm", }, - Some(InstructionSetAttr::ArmA32) => ".arm", Some(InstructionSetAttr::ArmT32) => ".thumb\n.thumb_func", + Some(InstructionSetAttr::ArmA32) => ".arm", }, match is_thumb { true => ".thumb", @@ -173,7 +170,7 @@ fn prefix_and_suffix<'tcx>( }; writeln!(begin, ".pushsection {section},\"ax\", {progbits}").unwrap(); - writeln!(begin, ".balign 4").unwrap(); + writeln!(begin, ".balign {align}").unwrap(); writeln!(begin, ".globl {asm_name}").unwrap(); if let Visibility::Hidden = item_data.visibility { writeln!(begin, ".hidden {asm_name}").unwrap(); @@ -194,7 +191,7 @@ fn prefix_and_suffix<'tcx>( AsmBinaryFormat::Macho => { let section = link_section.unwrap_or("__TEXT,__text".to_string()); writeln!(begin, ".pushsection {},regular,pure_instructions", section).unwrap(); - writeln!(begin, ".balign 4").unwrap(); + writeln!(begin, ".balign {align}").unwrap(); writeln!(begin, ".globl {asm_name}").unwrap(); if let Visibility::Hidden = item_data.visibility { writeln!(begin, ".private_extern {asm_name}").unwrap(); @@ -210,7 +207,7 @@ fn prefix_and_suffix<'tcx>( AsmBinaryFormat::Coff => { let section = link_section.unwrap_or(format!(".text.{asm_name}")); writeln!(begin, ".pushsection {},\"xr\"", section).unwrap(); - writeln!(begin, ".balign 4").unwrap(); + writeln!(begin, ".balign {align}").unwrap(); writeln!(begin, ".globl {asm_name}").unwrap(); writeln!(begin, ".def {asm_name}").unwrap(); writeln!(begin, ".scl 2").unwrap(); diff --git a/compiler/rustc_codegen_ssa/src/traits/asm.rs b/compiler/rustc_codegen_ssa/src/traits/asm.rs index f4853da115648..7767bffbfbfd6 100644 --- a/compiler/rustc_codegen_ssa/src/traits/asm.rs +++ b/compiler/rustc_codegen_ssa/src/traits/asm.rs @@ -68,4 +68,11 @@ pub trait AsmCodegenMethods<'tcx> { options: InlineAsmOptions, line_spans: &[Span], ); + + /// The mangled name of this instance + /// + /// Additional mangling is used on + /// some targets to add a leading underscore (Mach-O) + /// or byte count suffixes (x86 Windows). + fn mangled_name(&self, instance: Instance<'tcx>) -> String; } diff --git a/tests/codegen/naked-asan.rs b/tests/codegen/naked-asan.rs index bcaa60baeffd8..639f069c0d9e5 100644 --- a/tests/codegen/naked-asan.rs +++ b/tests/codegen/naked-asan.rs @@ -8,7 +8,11 @@ #![no_std] #![feature(abi_x86_interrupt, naked_functions)] -// CHECK: define x86_intrcc void @page_fault_handler(ptr {{.*}}%0, i64 {{.*}}%1){{.*}}#[[ATTRS:[0-9]+]] { +pub fn caller() { + page_fault_handler(1, 2); +} + +// CHECK: declare x86_intrcc void @page_fault_handler(ptr {{.*}}, i64 {{.*}}){{.*}}#[[ATTRS:[0-9]+]] // CHECK-NOT: memcpy #[naked] #[no_mangle] diff --git a/tests/codegen/naked-fn/aligned.rs b/tests/codegen/naked-fn/aligned.rs index 3bbd67981e5bf..d9dcd7f6c3ef7 100644 --- a/tests/codegen/naked-fn/aligned.rs +++ b/tests/codegen/naked-fn/aligned.rs @@ -6,15 +6,12 @@ #![feature(naked_functions, fn_align)] use std::arch::naked_asm; -// CHECK: Function Attrs: naked -// CHECK-NEXT: define{{.*}}void @naked_empty() -// CHECK: align 16 +// CHECK: .balign 16 +// CHECK-LABEL: naked_empty: #[repr(align(16))] #[no_mangle] #[naked] pub unsafe extern "C" fn naked_empty() { - // CHECK-NEXT: start: - // CHECK-NEXT: call void asm - // CHECK-NEXT: unreachable + // CHECK: ret naked_asm!("ret"); } diff --git a/tests/codegen/naked-fn/generics.rs b/tests/codegen/naked-fn/generics.rs index 5bb40cddea56c..23c7766203b9f 100644 --- a/tests/codegen/naked-fn/generics.rs +++ b/tests/codegen/naked-fn/generics.rs @@ -4,7 +4,7 @@ #![crate_type = "lib"] #![feature(naked_functions, asm_const)] -use std::arch::asm; +use std::arch::naked_asm; #[no_mangle] fn test(x: u64) { @@ -33,7 +33,7 @@ pub extern "C" fn using_const_generics(x: u64) -> u64 { const M: u64 = 42; unsafe { - asm!( + naked_asm!( "xor rax, rax", "add rax, rdi", "add rax, {}", @@ -41,7 +41,6 @@ pub extern "C" fn using_const_generics(x: u64) -> u64 { "ret", const N, const M, - options(noreturn), ) } } @@ -64,11 +63,10 @@ impl Invert for i64 { #[naked] pub extern "C" fn generic_function(x: i64) -> i64 { unsafe { - asm!( + naked_asm!( "call {}", "ret", sym ::invert, - options(noreturn), ) } } @@ -85,7 +83,7 @@ impl Foo { #[naked] #[no_mangle] extern "C" fn method(self) -> u64 { - unsafe { asm!("mov rax, rdi", "ret", options(noreturn)) } + unsafe { naked_asm!("mov rax, rdi", "ret") } } } @@ -101,7 +99,7 @@ impl Bar for Foo { #[naked] #[no_mangle] extern "C" fn trait_method(self) -> u64 { - unsafe { asm!("mov rax, rdi", "ret", options(noreturn)) } + unsafe { naked_asm!("mov rax, rdi", "ret") } } } @@ -113,5 +111,5 @@ impl Bar for Foo { #[naked] #[no_mangle] pub unsafe extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize { - asm!("lea rax, [rdi + rsi]", "ret", options(noreturn)); + naked_asm!("lea rax, [rdi + rsi]", "ret"); } diff --git a/tests/codegen/naked-fn/instruction-set.rs b/tests/codegen/naked-fn/instruction-set.rs index 509c24f9457cf..5b790b2034c09 100644 --- a/tests/codegen/naked-fn/instruction-set.rs +++ b/tests/codegen/naked-fn/instruction-set.rs @@ -1,12 +1,15 @@ -//@ compile-flags: --target armv5te-none-eabi -//@ needs-llvm-components: arm +//@ revisions: arm-mode thumb-mode +//@ [arm-mode] compile-flags: --target armv5te-none-eabi +//@ [thumb-mode] compile-flags: --target thumbv5te-none-eabi +//@ [arm-mode] needs-llvm-components: arm +//@ [thumb-mode] needs-llvm-components: arm #![crate_type = "lib"] #![feature(no_core, lang_items, rustc_attrs, naked_functions)] #![no_core] #[rustc_builtin_macro] -macro_rules! asm { +macro_rules! naked_asm { () => {}; } @@ -15,29 +18,42 @@ trait Sized {} #[lang = "copy"] trait Copy {} +// arm-mode: .arm +// thumb-mode: .thumb // CHECK-LABEL: test_unspecified: -// CHECK: .arm +// CHECK: bx lr +// CHECK: .popsection +// arm-mode: .arm +// thumb-mode: .thumb #[no_mangle] #[naked] unsafe extern "C" fn test_unspecified() { - asm!("bx lr", options(noreturn)); + naked_asm!("bx lr"); } -// CHECK-LABEL: test_thumb: // CHECK: .thumb // CHECK: .thumb_func +// CHECK-LABEL: test_thumb: +// CHECK: bx lr +// CHECK: .popsection +// arm-mode: .arm +// thumb-mode: .thumb #[no_mangle] #[naked] #[instruction_set(arm::t32)] unsafe extern "C" fn test_thumb() { - asm!("bx lr", options(noreturn)); + naked_asm!("bx lr"); } -// CHECK-LABEL: test_arm: // CHECK: .arm +// CHECK-LABEL: test_arm: +// CHECK: bx lr +// CHECK: .popsection +// arm-mode: .arm +// thumb-mode: .thumb #[no_mangle] #[naked] -#[instruction_set(arm::t32)] +#[instruction_set(arm::a32)] unsafe extern "C" fn test_arm() { - asm!("bx lr", options(noreturn)); + naked_asm!("bx lr"); } diff --git a/tests/codegen/naked-fn/naked-functions.rs b/tests/codegen/naked-fn/naked-functions.rs index 8b12693da8f66..f505d27d48c62 100644 --- a/tests/codegen/naked-fn/naked-functions.rs +++ b/tests/codegen/naked-fn/naked-functions.rs @@ -1,9 +1,9 @@ -//@ revisions: linux windows macos thumb +//@ revisions: linux win macos thumb // //@[linux] compile-flags: --target x86_64-unknown-linux-gnu //@[linux] needs-llvm-components: x86 -//@[windows] compile-flags: --target x86_64-pc-windows-gnu -//@[windows] needs-llvm-components: x86 +//@[win] compile-flags: --target x86_64-pc-windows-gnu +//@[win] needs-llvm-components: x86 //@[macos] compile-flags: --target aarch64-apple-darwin //@[macos] needs-llvm-components: arm //@[thumb] compile-flags: --target thumbv7em-none-eabi @@ -23,16 +23,16 @@ trait Sized {} #[lang = "copy"] trait Copy {} -// linux,windows: .intel_syntax +// linux,win: .intel_syntax // // linux: .pushsection .text.naked_empty,\22ax\22, @progbits // macos: .pushsection __TEXT,__text,regular,pure_instructions -// windows: .pushsection .text.naked_empty,\22xr\22 +// win: .pushsection .text.naked_empty,\22xr\22 // thumb: .pushsection .text.naked_empty,\22ax\22, %progbits // // CHECK: .balign 4 // -// linux,windows,thumb: .globl naked_empty +// linux,win,thumb: .globl naked_empty // macos: .globl _naked_empty // // CHECK-NOT: .private_extern @@ -40,10 +40,10 @@ trait Copy {} // // linux: .type naked_empty, @function // -// windows: .def naked_empty -// windows: .scl 2 -// windows: .type 32 -// windows: .endef naked_empty +// win: .def naked_empty +// win: .scl 2 +// win: .type 32 +// win: .endef naked_empty // // thumb: .type naked_empty, %function // thumb: .thumb @@ -51,14 +51,14 @@ trait Copy {} // // CHECK-LABEL: naked_empty: // -// linux,macos,windows: ret +// linux,macos,win: ret // thumb: bx lr // // CHECK: .popsection // // thumb: .thumb // -// linux,windows: .att_syntax +// linux,win: .att_syntax #[no_mangle] #[naked] @@ -70,16 +70,16 @@ pub unsafe extern "C" fn naked_empty() { naked_asm!("bx lr"); } -// linux,windows: .intel_syntax +// linux,win: .intel_syntax // // linux: .pushsection .text.naked_with_args_and_return,\22ax\22, @progbits // macos: .pushsection __TEXT,__text,regular,pure_instructions -// windows: .pushsection .text.naked_with_args_and_return,\22xr\22 +// win: .pushsection .text.naked_with_args_and_return,\22xr\22 // thumb: .pushsection .text.naked_with_args_and_return,\22ax\22, %progbits // // CHECK: .balign 4 // -// linux,windows,thumb: .globl naked_with_args_and_return +// linux,win,thumb: .globl naked_with_args_and_return // macos: .globl _naked_with_args_and_return // // CHECK-NOT: .private_extern @@ -87,10 +87,10 @@ pub unsafe extern "C" fn naked_empty() { // // linux: .type naked_with_args_and_return, @function // -// windows: .def naked_with_args_and_return -// windows: .scl 2 -// windows: .type 32 -// windows: .endef naked_with_args_and_return +// win: .def naked_with_args_and_return +// win: .scl 2 +// win: .type 32 +// win: .endef naked_with_args_and_return // // thumb: .type naked_with_args_and_return, %function // thumb: .thumb @@ -98,18 +98,18 @@ pub unsafe extern "C" fn naked_empty() { // // CHECK-LABEL: naked_with_args_and_return: // -// linux, windows: lea rax, [rdi + rsi] +// linux, win: lea rax, [rdi + rsi] // macos: add x0, x0, x1 // thumb: adds r0, r0, r1 // -// linux,macos,windows: ret +// linux,macos,win: ret // thumb: bx lr // // CHECK: .popsection // // thumb: .thumb // -// linux,windows: .att_syntax +// linux,win: .att_syntax #[no_mangle] #[naked] @@ -132,7 +132,7 @@ pub unsafe extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize // linux: .pushsection .text.some_different_name,\22ax\22, @progbits // macos: .pushsection .text.some_different_name,regular,pure_instructions -// windows: .pushsection .text.some_different_name,\22xr\22 +// win: .pushsection .text.some_different_name,\22xr\22 // thumb: .pushsection .text.some_different_name,\22ax\22, %progbits // CHECK-LABEL: test_link_section: #[no_mangle] @@ -140,8 +140,8 @@ pub unsafe extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize #[link_section = ".text.some_different_name"] pub unsafe extern "C" fn test_link_section() { #[cfg(not(all(target_arch = "arm", target_feature = "thumb-mode")))] - naked_asm!("ret", options(noreturn)); + naked_asm!("ret"); #[cfg(all(target_arch = "arm", target_feature = "thumb-mode"))] - naked_asm!("bx lr", options(noreturn)); + naked_asm!("bx lr"); } From 64420b6ea81d9f7065421add9843ba22e876cd6c Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Sat, 30 Nov 2024 15:36:03 +0100 Subject: [PATCH 14/27] skip `predefine_fn` for naked functions --- compiler/rustc_codegen_llvm/src/mono_item.rs | 2 +- .../rustc_codegen_ssa/src/mir/naked_asm.rs | 40 +++++++++++++++++-- compiler/rustc_codegen_ssa/src/mono_item.rs | 9 ++--- compiler/rustc_middle/src/mir/mono.rs | 22 ---------- 4 files changed, 41 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index b91941396ed81..33789c6261f10 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -58,8 +58,8 @@ impl<'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'_, 'tcx> { let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty()); let lldecl = self.declare_fn(symbol_name, fn_abi, Some(instance)); - let attrs = self.tcx.codegen_fn_attrs(instance.def_id()); llvm::set_linkage(lldecl, base::linkage_to_llvm(linkage)); + let attrs = self.tcx.codegen_fn_attrs(instance.def_id()); base::set_link_section(lldecl, attrs); if (linkage == Linkage::LinkOnceODR || linkage == Linkage::WeakODR) && self.tcx.sess.target.supports_comdat() diff --git a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs index 65470dd6d5371..359cda19b8449 100644 --- a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs +++ b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs @@ -1,5 +1,5 @@ use rustc_attr::InstructionSetAttr; -use rustc_middle::mir::mono::{MonoItem, MonoItemData, Visibility}; +use rustc_middle::mir::mono::{Linkage, MonoItem, MonoItemData, Visibility}; use rustc_middle::mir::{Body, InlineAsmOperand}; use rustc_middle::ty::layout::{HasTyCtxt, HasTypingEnv, LayoutOf}; use rustc_middle::ty::{Instance, TyCtxt}; @@ -153,6 +153,30 @@ fn prefix_and_suffix<'tcx>( ("", "") }; + let emit_fatal = |msg| tcx.dcx().span_fatal(tcx.def_span(instance.def_id()), msg); + + // see https://godbolt.org/z/cPK4sxKor. + // None means the default, which corresponds to internal linkage + let linkage = match item_data.linkage { + Linkage::External => Some(".globl"), + Linkage::LinkOnceAny => Some(".weak"), + Linkage::LinkOnceODR => Some(".weak"), + Linkage::WeakAny => Some(".weak"), + Linkage::WeakODR => Some(".weak"), + Linkage::Internal => None, + Linkage::Private => None, + Linkage::Appending => emit_fatal("Only global variables can have appending linkage!"), + Linkage::Common => emit_fatal("Functions may not have common linkage"), + Linkage::AvailableExternally => { + // this would make the function equal an extern definition + emit_fatal("Functions may not have available_externally linkage") + } + Linkage::ExternalWeak => { + // FIXME: actually this causes a SIGILL in LLVM + emit_fatal("Functions may not have external weak linkage") + } + }; + let mut begin = String::new(); let mut end = String::new(); match AsmBinaryFormat::from_target(&tcx.sess.target) { @@ -171,7 +195,9 @@ fn prefix_and_suffix<'tcx>( writeln!(begin, ".pushsection {section},\"ax\", {progbits}").unwrap(); writeln!(begin, ".balign {align}").unwrap(); - writeln!(begin, ".globl {asm_name}").unwrap(); + if let Some(linkage) = linkage { + writeln!(begin, "{linkage} {asm_name}").unwrap(); + } if let Visibility::Hidden = item_data.visibility { writeln!(begin, ".hidden {asm_name}").unwrap(); } @@ -181,6 +207,8 @@ fn prefix_and_suffix<'tcx>( } writeln!(begin, "{asm_name}:").unwrap(); + eprintln!("{}", &begin); + writeln!(end).unwrap(); writeln!(end, ".size {asm_name}, . - {asm_name}").unwrap(); writeln!(end, ".popsection").unwrap(); @@ -192,7 +220,9 @@ fn prefix_and_suffix<'tcx>( let section = link_section.unwrap_or("__TEXT,__text".to_string()); writeln!(begin, ".pushsection {},regular,pure_instructions", section).unwrap(); writeln!(begin, ".balign {align}").unwrap(); - writeln!(begin, ".globl {asm_name}").unwrap(); + if let Some(linkage) = linkage { + writeln!(begin, "{linkage} {asm_name}").unwrap(); + } if let Visibility::Hidden = item_data.visibility { writeln!(begin, ".private_extern {asm_name}").unwrap(); } @@ -208,7 +238,9 @@ fn prefix_and_suffix<'tcx>( let section = link_section.unwrap_or(format!(".text.{asm_name}")); writeln!(begin, ".pushsection {},\"xr\"", section).unwrap(); writeln!(begin, ".balign {align}").unwrap(); - writeln!(begin, ".globl {asm_name}").unwrap(); + if let Some(linkage) = linkage { + writeln!(begin, "{linkage} {asm_name}").unwrap(); + } writeln!(begin, ".def {asm_name}").unwrap(); writeln!(begin, ".scl 2").unwrap(); writeln!(begin, ".type 32").unwrap(); diff --git a/compiler/rustc_codegen_ssa/src/mono_item.rs b/compiler/rustc_codegen_ssa/src/mono_item.rs index 085ef1cd8ea36..70ad5bee3e09c 100644 --- a/compiler/rustc_codegen_ssa/src/mono_item.rs +++ b/compiler/rustc_codegen_ssa/src/mono_item.rs @@ -137,13 +137,12 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> { } MonoItem::Fn(instance) => { let attrs = cx.tcx().codegen_fn_attrs(instance.def_id()); - let linkage = if attrs.flags.contains(CodegenFnAttrFlags::NAKED) { - linkage_info.into_naked_linkage() + + if attrs.flags.contains(CodegenFnAttrFlags::NAKED) { + // do not define this function; it will become a global assembly block } else { - linkage_info.into_linkage() + cx.predefine_fn(instance, linkage_info.into_linkage(), visibility, symbol_name); }; - - cx.predefine_fn(instance, linkage, visibility, symbol_name); } MonoItem::GlobalAsm(..) => {} } diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index 4287b2c574ffe..01c66ecb570cb 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/compiler/rustc_middle/src/mir/mono.rs @@ -299,28 +299,6 @@ impl LinkageInfo { Self::ImplicitInternal => Linkage::Internal, } } - - /// Linkage when the MonoItem is a naked function - /// - /// Naked functions are generated as a separate declaration (effectively an extern fn) and - /// definition (using global assembly). To link them together, some flavor of external linkage - /// must be used. - /// - /// This should be just an implementation detail of the backend, which is why this translation - /// is made at the final moment. - pub const fn into_naked_linkage(self) -> Linkage { - match self { - // promote Weak linkage to ExternalWeak - Self::Explicit(Linkage::WeakAny | Linkage::WeakODR) => Linkage::ExternalWeak, - Self::Explicit(linkage) => linkage, - - // the "implicit" means that linkage is picked by the partitioning algorithm. - // picking external should always be valid (given that we are in fact linking - // to the global assembly in the same CGU) - Self::ImplicitExternal => Linkage::External, - Self::ImplicitInternal => Linkage::External, - } - } } /// Specifies the linkage type for a `MonoItem`. From cf738f6bfba45aa65e96eb2c9cce3d0ecd25f66c Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Sat, 30 Nov 2024 15:42:04 +0100 Subject: [PATCH 15/27] Revert "add `LinkageInfo` to keep track of how we figured out the linkage" This reverts commit 9142cae7d5b86ac70f815c32c0f3b1223b62a947. --- .../rustc_codegen_cranelift/src/driver/mod.rs | 2 +- compiler/rustc_codegen_gcc/src/base.rs | 2 +- compiler/rustc_codegen_llvm/src/base.rs | 2 +- .../src/back/symbol_export.rs | 4 +- .../rustc_codegen_ssa/src/mir/naked_asm.rs | 2 - compiler/rustc_codegen_ssa/src/mono_item.rs | 10 ++-- compiler/rustc_middle/src/mir/mono.rs | 27 +--------- .../rustc_monomorphize/src/partitioning.rs | 49 +++++++++---------- 8 files changed, 34 insertions(+), 64 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/driver/mod.rs b/compiler/rustc_codegen_cranelift/src/driver/mod.rs index dbc913528f44a..fb0eed07c1971 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/mod.rs @@ -30,7 +30,7 @@ fn predefine_mono_items<'tcx>( get_function_sig(tcx, module.target_config().default_call_conv, instance); let linkage = crate::linkage::get_clif_linkage( mono_item, - data.linkage_info.into_linkage(), + data.linkage, data.visibility, is_compiler_builtins, ); diff --git a/compiler/rustc_codegen_gcc/src/base.rs b/compiler/rustc_codegen_gcc/src/base.rs index 844871a714671..18aa32754e1d9 100644 --- a/compiler/rustc_codegen_gcc/src/base.rs +++ b/compiler/rustc_codegen_gcc/src/base.rs @@ -213,7 +213,7 @@ pub fn compile_codegen_unit( let mono_items = cgu.items_in_deterministic_order(tcx); for &(mono_item, data) in &mono_items { - mono_item.predefine::>(&cx, data.linkage_info, data.visibility); + mono_item.predefine::>(&cx, data.linkage, data.visibility); } // ... and now that we have everything pre-defined, fill out those definitions. diff --git a/compiler/rustc_codegen_llvm/src/base.rs b/compiler/rustc_codegen_llvm/src/base.rs index 82c3cfe9aa04a..f62310bd94808 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -86,7 +86,7 @@ pub(crate) fn compile_codegen_unit( let cx = CodegenCx::new(tcx, cgu, &llvm_module); let mono_items = cx.codegen_unit.items_in_deterministic_order(cx.tcx); for &(mono_item, data) in &mono_items { - mono_item.predefine::>(&cx, data.linkage_info, data.visibility); + mono_item.predefine::>(&cx, data.linkage, data.visibility); } // ... and now that we have everything pre-defined, fill out those definitions. diff --git a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs index 0321a244f8683..788a8a13b3ee4 100644 --- a/compiler/rustc_codegen_ssa/src/back/symbol_export.rs +++ b/compiler/rustc_codegen_ssa/src/back/symbol_export.rs @@ -283,7 +283,7 @@ fn exported_symbols_provider_local( } if tcx.local_crate_exports_generics() { - use rustc_middle::mir::mono::{MonoItem, Visibility}; + use rustc_middle::mir::mono::{Linkage, MonoItem, Visibility}; use rustc_middle::ty::InstanceKind; // Normally, we require that shared monomorphizations are not hidden, @@ -298,7 +298,7 @@ fn exported_symbols_provider_local( // The symbols created in this loop are sorted below it #[allow(rustc::potential_query_instability)] for (mono_item, data) in cgus.iter().flat_map(|cgu| cgu.items().iter()) { - if !data.linkage_info.is_external() { + if data.linkage != Linkage::External { // We can only re-use things with external linkage, otherwise // we'll get a linker error continue; diff --git a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs index 359cda19b8449..0767c002c5e06 100644 --- a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs +++ b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs @@ -207,8 +207,6 @@ fn prefix_and_suffix<'tcx>( } writeln!(begin, "{asm_name}:").unwrap(); - eprintln!("{}", &begin); - writeln!(end).unwrap(); writeln!(end, ".size {asm_name}, . - {asm_name}").unwrap(); writeln!(end, ".popsection").unwrap(); diff --git a/compiler/rustc_codegen_ssa/src/mono_item.rs b/compiler/rustc_codegen_ssa/src/mono_item.rs index 70ad5bee3e09c..6749bc63327e4 100644 --- a/compiler/rustc_codegen_ssa/src/mono_item.rs +++ b/compiler/rustc_codegen_ssa/src/mono_item.rs @@ -1,7 +1,7 @@ use rustc_hir as hir; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::interpret::ErrorHandled; -use rustc_middle::mir::mono::{LinkageInfo, MonoItem, Visibility}; +use rustc_middle::mir::mono::{Linkage, MonoItem, Visibility}; use rustc_middle::ty::Instance; use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf}; use rustc_middle::{span_bug, ty}; @@ -15,7 +15,7 @@ pub trait MonoItemExt<'a, 'tcx> { fn predefine>( &self, cx: &'a Bx::CodegenCx, - linkage_info: LinkageInfo, + linkage: Linkage, visibility: Visibility, ); fn to_raw_string(&self) -> String; @@ -117,7 +117,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> { fn predefine>( &self, cx: &'a Bx::CodegenCx, - linkage_info: LinkageInfo, + linkage: Linkage, visibility: Visibility, ) { debug!( @@ -133,7 +133,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> { match *self { MonoItem::Static(def_id) => { - cx.predefine_static(def_id, linkage_info.into_linkage(), visibility, symbol_name); + cx.predefine_static(def_id, linkage, visibility, symbol_name); } MonoItem::Fn(instance) => { let attrs = cx.tcx().codegen_fn_attrs(instance.def_id()); @@ -141,7 +141,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> { if attrs.flags.contains(CodegenFnAttrFlags::NAKED) { // do not define this function; it will become a global assembly block } else { - cx.predefine_fn(instance, linkage_info.into_linkage(), visibility, symbol_name); + cx.predefine_fn(instance, linkage, visibility, symbol_name); }; } MonoItem::GlobalAsm(..) => {} diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index 01c66ecb570cb..1f50b67cb50d5 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/compiler/rustc_middle/src/mir/mono.rs @@ -269,38 +269,13 @@ pub struct MonoItemData { /// `GloballyShared` maps to `false` and `LocalCopy` maps to `true`. pub inlined: bool, - pub linkage_info: LinkageInfo, + pub linkage: Linkage, pub visibility: Visibility, /// A cached copy of the result of `MonoItem::size_estimate`. pub size_estimate: usize, } -/// Stores how we know what linkage to use -#[derive(Copy, Clone, PartialEq, Debug, TyEncodable, TyDecodable, HashStable)] -pub enum LinkageInfo { - /// The linkage was specified explicitly (e.g. using #[linkage = "..."]) - Explicit(Linkage), - /// Assume the symbol may be used from other CGUs, a safe default - ImplicitExternal, - /// We did not find any uses from other CGUs, so it's fine to make this internal - ImplicitInternal, -} - -impl LinkageInfo { - pub const fn is_external(self) -> bool { - matches!(self.into_linkage(), Linkage::External) - } - - pub const fn into_linkage(self) -> Linkage { - match self { - Self::Explicit(linkage) => linkage, - Self::ImplicitExternal => Linkage::External, - Self::ImplicitInternal => Linkage::Internal, - } - } -} - /// Specifies the linkage type for a `MonoItem`. /// /// See for more details about these variants. diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 2fd89e389e547..7ea4ded2b052c 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -109,8 +109,8 @@ use rustc_middle::bug; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::middle::exported_symbols::{SymbolExportInfo, SymbolExportLevel}; use rustc_middle::mir::mono::{ - CodegenUnit, CodegenUnitNameBuilder, InstantiationMode, Linkage, LinkageInfo, MonoItem, - MonoItemData, Visibility, + CodegenUnit, CodegenUnitNameBuilder, InstantiationMode, Linkage, MonoItem, MonoItemData, + Visibility, }; use rustc_middle::ty::print::{characteristic_def_id_of_type, with_no_trimmed_paths}; use rustc_middle::ty::visit::TypeVisitableExt; @@ -245,7 +245,7 @@ where let cgu = codegen_units.entry(cgu_name).or_insert_with(|| CodegenUnit::new(cgu_name)); let mut can_be_internalized = true; - let (linkage_info, visibility) = mono_item_linkage_info_and_visibility( + let (linkage, visibility) = mono_item_linkage_and_visibility( cx.tcx, &mono_item, &mut can_be_internalized, @@ -259,7 +259,7 @@ where cgu.items_mut().insert(mono_item, MonoItemData { inlined: false, - linkage_info, + linkage, visibility, size_estimate, }); @@ -278,7 +278,7 @@ where // This is a CGU-private copy. cgu.items_mut().entry(inlined_item).or_insert_with(|| MonoItemData { inlined: true, - linkage_info: LinkageInfo::ImplicitInternal, + linkage: Linkage::Internal, visibility: Visibility::Default, size_estimate: inlined_item.size_estimate(cx.tcx), }); @@ -589,8 +589,7 @@ fn internalize_symbols<'tcx>( // If we got here, we did not find any uses from other CGUs, so // it's fine to make this monomorphization internal. - debug_assert_eq!(data.linkage_info, LinkageInfo::ImplicitExternal); - data.linkage_info = LinkageInfo::ImplicitInternal; + data.linkage = Linkage::Internal; data.visibility = Visibility::Default; } } @@ -608,7 +607,7 @@ fn mark_code_coverage_dead_code_cgu<'tcx>(codegen_units: &mut [CodegenUnit<'tcx> // function symbols to be included via `-u` or `/include` linker args. let dead_code_cgu = codegen_units .iter_mut() - .filter(|cgu| cgu.items().iter().any(|(_, data)| data.linkage_info.is_external())) + .filter(|cgu| cgu.items().iter().any(|(_, data)| data.linkage == Linkage::External)) .min_by_key(|cgu| cgu.size_estimate()); // If there are no CGUs that have externally linked items, then we just @@ -737,26 +736,24 @@ fn fallback_cgu_name(name_builder: &mut CodegenUnitNameBuilder<'_>) -> Symbol { name_builder.build_cgu_name(LOCAL_CRATE, &["fallback"], Some("cgu")) } -fn mono_item_linkage_info_and_visibility<'tcx>( +fn mono_item_linkage_and_visibility<'tcx>( tcx: TyCtxt<'tcx>, mono_item: &MonoItem<'tcx>, can_be_internalized: &mut bool, can_export_generics: bool, always_export_generics: bool, -) -> (LinkageInfo, Visibility) { +) -> (Linkage, Visibility) { if let Some(explicit_linkage) = mono_item.explicit_linkage(tcx) { - (LinkageInfo::Explicit(explicit_linkage), Visibility::Default) - } else { - let vis = mono_item_visibility( - tcx, - mono_item, - can_be_internalized, - can_export_generics, - always_export_generics, - ); - - (LinkageInfo::ImplicitExternal, vis) + return (explicit_linkage, Visibility::Default); } + let vis = mono_item_visibility( + tcx, + mono_item, + can_be_internalized, + can_export_generics, + always_export_generics, + ); + (Linkage::External, vis) } type CguNameCache = UnordMap<(DefId, bool), Symbol>; @@ -1036,7 +1033,7 @@ fn debug_dump<'a, 'tcx: 'a>(tcx: TyCtxt<'tcx>, label: &str, cgus: &[CodegenUnit< writeln!(s, " - items: {num_items}, mean size: {mean_size:.1}, sizes: {sizes}",); for (item, data) in cgu.items_in_deterministic_order(tcx) { - let linkage_info = data.linkage_info; + let linkage = data.linkage; let symbol_name = item.symbol_name(tcx).name; let symbol_hash_start = symbol_name.rfind('h'); let symbol_hash = symbol_hash_start.map_or("", |i| &symbol_name[i..]); @@ -1044,7 +1041,7 @@ fn debug_dump<'a, 'tcx: 'a>(tcx: TyCtxt<'tcx>, label: &str, cgus: &[CodegenUnit< let size = data.size_estimate; let _ = with_no_trimmed_paths!(writeln!( s, - " - {item} [{linkage_info:?}] [{symbol_hash}] ({kind}, size: {size})" + " - {item} [{linkage:?}] [{symbol_hash}] ({kind}, size: {size})" )); } @@ -1197,7 +1194,7 @@ fn collect_and_partition_mono_items(tcx: TyCtxt<'_>, (): ()) -> (&DefIdSet, &[Co for cgu in codegen_units { for (&mono_item, &data) in cgu.items() { - item_to_cgus.entry(mono_item).or_default().push((cgu.name(), data.linkage_info)); + item_to_cgus.entry(mono_item).or_default().push((cgu.name(), data.linkage)); } } @@ -1210,11 +1207,11 @@ fn collect_and_partition_mono_items(tcx: TyCtxt<'_>, (): ()) -> (&DefIdSet, &[Co let cgus = item_to_cgus.get_mut(i).unwrap_or(&mut empty); cgus.sort_by_key(|(name, _)| *name); cgus.dedup(); - for &(ref cgu_name, linkage_info) in cgus.iter() { + for &(ref cgu_name, linkage) in cgus.iter() { output.push(' '); output.push_str(cgu_name.as_str()); - let linkage_abbrev = match linkage_info.into_linkage() { + let linkage_abbrev = match linkage { Linkage::External => "External", Linkage::AvailableExternally => "Available", Linkage::LinkOnceAny => "OnceAny", From a8f8c65d0089f0ec54e7dfe51e730f6030f2087b Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Sun, 1 Dec 2024 14:55:17 +0100 Subject: [PATCH 16/27] fix the `naked-asan` test we get these declarations ``` ; opt level 0 declare x86_intrcc void @page_fault_handler(ptr byval([8 x i8]) align 8, i64) unnamed_addr #1 ; opt level > 0 declare x86_intrcc void @page_fault_handler(ptr noalias nocapture noundef byval([8 x i8]) align 8 dereferenceable(8), i64 noundef) unnamed_addr #1 ``` The space after `i64` in the original regex made the regex not match for opt level 0. Removing the space fixes the issue. ``` declare x86_intrcc void @page_fault_handler(ptr {{.*}}, i64 {{.*}}){{.*}}#[[ATTRS:[0-9]+]] ``` --- tests/codegen/naked-asan.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/codegen/naked-asan.rs b/tests/codegen/naked-asan.rs index 639f069c0d9e5..ac6b0ffec74c3 100644 --- a/tests/codegen/naked-asan.rs +++ b/tests/codegen/naked-asan.rs @@ -12,14 +12,12 @@ pub fn caller() { page_fault_handler(1, 2); } -// CHECK: declare x86_intrcc void @page_fault_handler(ptr {{.*}}, i64 {{.*}}){{.*}}#[[ATTRS:[0-9]+]] +// CHECK: declare x86_intrcc void @page_fault_handler(ptr {{.*}}, i64{{.*}}){{.*}}#[[ATTRS:[0-9]+]] // CHECK-NOT: memcpy #[naked] #[no_mangle] pub extern "x86-interrupt" fn page_fault_handler(_: u64, _: u64) { - unsafe { - core::arch::naked_asm!("ud2"); - } + unsafe { core::arch::naked_asm!("ud2") }; } // CHECK: #[[ATTRS]] = From 70ab84d72351624c8ae38d982d04be98c396d66f Mon Sep 17 00:00:00 2001 From: MarcoIeni <11428655+MarcoIeni@users.noreply.github.com> Date: Mon, 2 Dec 2024 12:55:08 +0100 Subject: [PATCH 17/27] CI: split x86_64-msvc job --- src/bootstrap/mk/Makefile.in | 13 +++++++++---- src/ci/github-actions/jobs.yml | 13 ++++++++++--- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/bootstrap/mk/Makefile.in b/src/bootstrap/mk/Makefile.in index a1f38b9ac147f..a29c96ff78ceb 100644 --- a/src/bootstrap/mk/Makefile.in +++ b/src/bootstrap/mk/Makefile.in @@ -97,13 +97,18 @@ tidy: prepare: $(Q)$(BOOTSTRAP) build --stage 2 --dry-run +STAGE_2_TEST_SET1 := test --stage 2 --skip=compiler --skip=src +STAGE_2_TEST_SET2 := test --stage 2 --skip=tests --skip=coverage-map --skip=coverage-run --skip=library --skip=tidyselftest + ## MSVC native builders +# Set of tests that should represent half of the time of the test suite. +# Used to split tests across multiple CI runners. # this intentionally doesn't use `$(BOOTSTRAP)` so we can test the shebang on Windows ci-msvc-py: - $(Q)$(CFG_SRC_DIR)/x.py test --stage 2 tidy + $(Q)$(CFG_SRC_DIR)/x.py $(STAGE_2_TEST_SET1) ci-msvc-ps1: - $(Q)$(CFG_SRC_DIR)/x.ps1 test --stage 2 --skip tidy + $(Q)$(CFG_SRC_DIR)/x.ps1 $(STAGE_2_TEST_SET2) ci-msvc: ci-msvc-py ci-msvc-ps1 ## MingW native builders @@ -112,9 +117,9 @@ ci-msvc: ci-msvc-py ci-msvc-ps1 # Used to split tests across multiple CI runners. # Test both x and bootstrap entrypoints. ci-mingw-x: - $(Q)$(CFG_SRC_DIR)/x test --stage 2 --skip=compiler --skip=src + $(Q)$(CFG_SRC_DIR)/x $(STAGE_2_TEST_SET1) ci-mingw-bootstrap: - $(Q)$(BOOTSTRAP) test --stage 2 --skip=tests --skip=coverage-map --skip=coverage-run --skip=library --skip=tidyselftest + $(Q)$(BOOTSTRAP) $(STAGE_2_TEST_SET2) ci-mingw: ci-mingw-x ci-mingw-bootstrap .PHONY: dist diff --git a/src/ci/github-actions/jobs.yml b/src/ci/github-actions/jobs.yml index 454c855c75d25..a669efc76a6d7 100644 --- a/src/ci/github-actions/jobs.yml +++ b/src/ci/github-actions/jobs.yml @@ -372,11 +372,18 @@ auto: # Windows Builders # ###################### - - image: x86_64-msvc + # x86_64-msvc is split into two jobs to run tests in parallel. + - image: x86_64-msvc-1 env: RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-msvc --enable-profiler - SCRIPT: make ci-msvc - <<: *job-windows-8c + SCRIPT: make ci-msvc-py + <<: *job-windows + + - image: x86_64-msvc-2 + env: + RUST_CONFIGURE_ARGS: --build=x86_64-pc-windows-msvc --enable-profiler + SCRIPT: make ci-msvc-ps1 + <<: *job-windows - image: i686-msvc env: From b81391ec76f8f18b92b0aee7369c7bda2864e0d9 Mon Sep 17 00:00:00 2001 From: MarcoIeni <11428655+MarcoIeni@users.noreply.github.com> Date: Tue, 3 Dec 2024 08:17:33 +0100 Subject: [PATCH 18/27] CI: use free runners for i686-gnu jobs --- .../host-x86_64/i686-gnu-nopt/Dockerfile | 4 +- src/ci/docker/host-x86_64/i686-gnu/Dockerfile | 9 +-- src/ci/docker/run.sh | 35 +++++++----- src/ci/github-actions/jobs.yml | 55 +++++++++++++++++-- 4 files changed, 77 insertions(+), 26 deletions(-) diff --git a/src/ci/docker/host-x86_64/i686-gnu-nopt/Dockerfile b/src/ci/docker/host-x86_64/i686-gnu-nopt/Dockerfile index e2b66c2cff1d8..e273672060732 100644 --- a/src/ci/docker/host-x86_64/i686-gnu-nopt/Dockerfile +++ b/src/ci/docker/host-x86_64/i686-gnu-nopt/Dockerfile @@ -27,5 +27,5 @@ RUN echo "[rust]" > /config/nopt-std-config.toml RUN echo "optimize = false" >> /config/nopt-std-config.toml ENV RUST_CONFIGURE_ARGS --build=i686-unknown-linux-gnu --disable-optimize-tests -ENV SCRIPT python3 ../x.py test --stage 0 --config /config/nopt-std-config.toml library/std \ - && python3 ../x.py --stage 2 test +ARG SCRIPT_ARG +ENV SCRIPT=${SCRIPT_ARG} diff --git a/src/ci/docker/host-x86_64/i686-gnu/Dockerfile b/src/ci/docker/host-x86_64/i686-gnu/Dockerfile index 61811c41904c4..dec25461bb4e8 100644 --- a/src/ci/docker/host-x86_64/i686-gnu/Dockerfile +++ b/src/ci/docker/host-x86_64/i686-gnu/Dockerfile @@ -24,10 +24,5 @@ COPY scripts/sccache.sh /scripts/ RUN sh /scripts/sccache.sh ENV RUST_CONFIGURE_ARGS --build=i686-unknown-linux-gnu -# Skip some tests that are unlikely to be platform specific, to speed up -# this slow job. -ENV SCRIPT python3 ../x.py --stage 2 test \ - --skip src/bootstrap \ - --skip tests/rustdoc-js \ - --skip src/tools/error_index_generator \ - --skip src/tools/linkchecker +ARG SCRIPT_ARG +ENV SCRIPT=${SCRIPT_ARG} diff --git a/src/ci/docker/run.sh b/src/ci/docker/run.sh index d554186df4cfe..a0adf60b6b2c9 100755 --- a/src/ci/docker/run.sh +++ b/src/ci/docker/run.sh @@ -105,6 +105,23 @@ if [ -f "$docker_dir/$image/Dockerfile" ]; then # It seems that it cannot be the same as $IMAGE_TAG, otherwise it overwrites the cache CACHE_IMAGE_TAG=${REGISTRY}/${REGISTRY_USERNAME}/rust-ci-cache:${cksum} + # Docker build arguments. + build_args=( + "build" + "--rm" + "-t" "rust-ci" + "-f" "$dockerfile" + "$context" + ) + + # If the environment variable DOCKER_SCRIPT is defined, + # set the build argument SCRIPT_ARG to DOCKER_SCRIPT. + # In this way, we run the script defined in CI, + # instead of the one defined in the Dockerfile. + if [ -n "${DOCKER_SCRIPT+x}" ]; then + build_args+=("--build-arg" "SCRIPT_ARG=${DOCKER_SCRIPT}") + fi + # On non-CI jobs, we try to download a pre-built image from the rust-lang-ci # ghcr.io registry. If it is not possible, we fall back to building the image # locally. @@ -115,7 +132,7 @@ if [ -f "$docker_dir/$image/Dockerfile" ]; then docker tag "${IMAGE_TAG}" rust-ci else echo "Building local Docker image" - retry docker build --rm -t rust-ci -f "$dockerfile" "$context" + retry docker "${build_args[@]}" fi # On PR CI jobs, we don't have permissions to write to the registry cache, # but we can still read from it. @@ -127,13 +144,9 @@ if [ -f "$docker_dir/$image/Dockerfile" ]; then # Build the image using registry caching backend retry docker \ buildx \ - build \ - --rm \ - -t rust-ci \ - -f "$dockerfile" \ + "${build_args[@]}" \ --cache-from type=registry,ref=${CACHE_IMAGE_TAG} \ - --output=type=docker \ - "$context" + --output=type=docker # On auto/try builds, we can also write to the cache. else # Log into the Docker registry, so that we can read/write cache and the final image @@ -147,14 +160,10 @@ if [ -f "$docker_dir/$image/Dockerfile" ]; then # Build the image using registry caching backend retry docker \ buildx \ - build \ - --rm \ - -t rust-ci \ - -f "$dockerfile" \ + "${build_args[@]}" \ --cache-from type=registry,ref=${CACHE_IMAGE_TAG} \ --cache-to type=registry,ref=${CACHE_IMAGE_TAG},compression=zstd \ - --output=type=docker \ - "$context" + --output=type=docker # Print images for debugging purposes docker images diff --git a/src/ci/github-actions/jobs.yml b/src/ci/github-actions/jobs.yml index 454c855c75d25..e74e4875453e4 100644 --- a/src/ci/github-actions/jobs.yml +++ b/src/ci/github-actions/jobs.yml @@ -58,6 +58,22 @@ envs: NO_DEBUG_ASSERTIONS: 1 NO_OVERFLOW_CHECKS: 1 + # Different set of tests to run tests in parallel in multiple jobs. + stage_2_test_set1: &stage_2_test_set1 + DOCKER_SCRIPT: >- + python3 ../x.py --stage 2 test + --skip compiler + --skip src + + stage_2_test_set2: &stage_2_test_set2 + DOCKER_SCRIPT: >- + python3 ../x.py --stage 2 test + --skip tests + --skip coverage-map + --skip coverage-run + --skip library + --skip tidyselftest + production: &production DEPLOY_BUCKET: rust-lang-ci2 @@ -212,11 +228,42 @@ auto: - image: dist-x86_64-netbsd <<: *job-linux-4c - - image: i686-gnu - <<: *job-linux-8c + # The i686-gnu job is split into multiple jobs to run tests in parallel. + # i686-gnu-1 skips tests that run in i686-gnu-2. + - image: i686-gnu-1 + env: + IMAGE: i686-gnu + <<: *stage_2_test_set1 + <<: *job-linux-4c - - image: i686-gnu-nopt - <<: *job-linux-8c + # Skip tests that run in i686-gnu-1 + - image: i686-gnu-2 + env: + IMAGE: i686-gnu + <<: *stage_2_test_set2 + <<: *job-linux-4c + + # The i686-gnu-nopt job is split into multiple jobs to run tests in parallel. + # i686-gnu-nopt-1 skips tests that run in i686-gnu-nopt-2 + - image: i686-gnu-nopt-1 + env: + IMAGE: i686-gnu-nopt + <<: *stage_2_test_set1 + <<: *job-linux-4c + + # Skip tests that run in i686-gnu-nopt-1 + - image: i686-gnu-nopt-2 + env: + IMAGE: i686-gnu-nopt + DOCKER_SCRIPT: >- + python3 ../x.py test --stage 0 --config /config/nopt-std-config.toml library/std && + python3 ../x.py --stage 2 test + --skip tests + --skip coverage-map + --skip coverage-run + --skip library + --skip tidyselftest + <<: *job-linux-4c - image: mingw-check <<: *job-linux-4c From 059f6272c346654dfce7a68a6bb0c0aeb1ed756a Mon Sep 17 00:00:00 2001 From: "Brian J. Tarricone" Date: Mon, 15 Feb 2021 23:32:16 -0800 Subject: [PATCH 19/27] Teach rust core about Xtensa VaListImpl and add a custom lowering of vaarg for xtensa. LLVM does not include an implementation of the va_arg instruction for Xtensa. From what I understand, this is a conscious decision and instead language frontends are encouraged to implement it themselves. The rationale seems to be that loading values correctly requires language and ABI-specific knowledge that LLVM lacks. This is true of most architectures, and rustc already provides implementation for a number of them. This commit extends the support to include Xtensa. See https://lists.llvm.org/pipermail/llvm-dev/2017-August/116337.html for some discussion on the topic. Unfortunately there does not seem to be a reference document for the semantics of the va_list and va_arg on Xtensa. The most reliable source is the GCC implementation, which this commit tries to follow. Clang also provides its own compatible implementation. This was tested for all the types that rustc allows in variadics. Co-authored-by: Brian Tarricone Co-authored-by: Jonathan Bastien-Filiatrault Co-authored-by: Paul Lietar --- compiler/rustc_codegen_llvm/src/va_arg.rs | 113 +++++++++++++++++++++- library/core/Cargo.toml | 1 + library/core/src/ffi/va_list.rs | 19 ++++ 3 files changed, 131 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/va_arg.rs b/compiler/rustc_codegen_llvm/src/va_arg.rs index e4c3e748cb5df..8baa69cefe1e2 100644 --- a/compiler/rustc_codegen_llvm/src/va_arg.rs +++ b/compiler/rustc_codegen_llvm/src/va_arg.rs @@ -10,6 +10,15 @@ use crate::type_::Type; use crate::type_of::LayoutLlvmExt; use crate::value::Value; +fn round_up_to_alignment<'ll>( + bx: &mut Builder<'_, 'll, '_>, + mut value: &'ll Value, + align: Align, +) -> &'ll Value { + value = bx.add(value, bx.cx().const_i32(align.bytes() as i32 - 1)); + return bx.and(value, bx.cx().const_i32(-(align.bytes() as i32))); +} + fn round_pointer_up_to_alignment<'ll>( bx: &mut Builder<'_, 'll, '_>, addr: &'ll Value, @@ -17,8 +26,7 @@ fn round_pointer_up_to_alignment<'ll>( ptr_ty: &'ll Type, ) -> &'ll Value { let mut ptr_as_int = bx.ptrtoint(addr, bx.cx().type_isize()); - ptr_as_int = bx.add(ptr_as_int, bx.cx().const_i32(align.bytes() as i32 - 1)); - ptr_as_int = bx.and(ptr_as_int, bx.cx().const_i32(-(align.bytes() as i32))); + ptr_as_int = round_up_to_alignment(bx, ptr_as_int, align); bx.inttoptr(ptr_as_int, ptr_ty) } @@ -270,6 +278,106 @@ fn emit_s390x_va_arg<'ll, 'tcx>( bx.load(val_type, val_addr, layout.align.abi) } +fn emit_xtensa_va_arg<'ll, 'tcx>( + bx: &mut Builder<'_, 'll, 'tcx>, + list: OperandRef<'tcx, &'ll Value>, + target_ty: Ty<'tcx>, +) -> &'ll Value { + // Implementation of va_arg for Xtensa. There doesn't seem to be an authoritative source for + // this, other than "what GCC does". + // + // The va_list type has three fields: + // struct __va_list_tag { + // int32_t *va_stk; // Arguments passed on the stack + // int32_t *va_reg; // Arguments passed in registers, saved to memory by the prologue. + // int32_t va_ndx; // Offset into the arguments, in bytes + // }; + // + // The first 24 bytes (equivalent to 6 registers) come from va_reg, the rest from va_stk. + // Thus if va_ndx is less than 24, the next va_arg *may* read from va_reg, + // otherwise it must come from va_stk. + // + // Primitive arguments are never split between registers and the stack. For example, if loading an 8 byte + // primitive value and va_ndx = 20, we instead bump the offset and read everything from va_stk. + let va_list_addr = list.immediate(); + // FIXME: handle multi-field structs that split across regsave/stack? + let layout = bx.cx.layout_of(target_ty); + let from_stack = bx.append_sibling_block("va_arg.from_stack"); + let from_regsave = bx.append_sibling_block("va_arg.from_regsave"); + let end = bx.append_sibling_block("va_arg.end"); + + // (*va).va_ndx + let va_reg_offset = 4; + let va_ndx_offset = va_reg_offset + 4; + let offset_ptr = + bx.inbounds_gep(bx.type_i8(), va_list_addr, &[bx.cx.const_usize(va_ndx_offset)]); + + let offset = bx.load(bx.type_i32(), offset_ptr, bx.tcx().data_layout.i32_align.abi); + let offset = round_up_to_alignment(bx, offset, layout.align.abi); + + let slot_size = layout.size.align_to(Align::from_bytes(4).unwrap()).bytes() as i32; + + // Update the offset in va_list, by adding the slot's size. + let offset_next = bx.add(offset, bx.const_i32(slot_size)); + + // Figure out where to look for our value. We do that by checking the end of our slot (offset_next). + // If that is within the regsave area, then load from there. Otherwise load from the stack area. + let regsave_size = bx.const_i32(24); + let use_regsave = bx.icmp(IntPredicate::IntULE, offset_next, regsave_size); + bx.cond_br(use_regsave, from_regsave, from_stack); + + bx.switch_to_block(from_regsave); + // update va_ndx + bx.store(offset_next, offset_ptr, bx.tcx().data_layout.pointer_align.abi); + + // (*va).va_reg + let regsave_area_ptr = + bx.inbounds_gep(bx.type_i8(), va_list_addr, &[bx.cx.const_usize(va_reg_offset)]); + let regsave_area = + bx.load(bx.type_ptr(), regsave_area_ptr, bx.tcx().data_layout.pointer_align.abi); + let regsave_value_ptr = bx.inbounds_gep(bx.type_i8(), regsave_area, &[offset]); + bx.br(end); + + bx.switch_to_block(from_stack); + + // The first time we switch from regsave to stack we needs to adjust our offsets a bit. + // va_stk is set up such that the first stack argument is always at va_stk + 32. + // The corrected offset is written back into the va_list struct. + + // let offset_corrected = cmp::max(offset, 32); + let stack_offset_start = bx.const_i32(32); + let needs_correction = bx.icmp(IntPredicate::IntULE, offset, stack_offset_start); + let offset_corrected = bx.select(needs_correction, stack_offset_start, offset); + + // let offset_next_corrected = offset_corrected + slot_size; + // va_ndx = offset_next_corrected; + let offset_next_corrected = bx.add(offset_next, bx.const_i32(slot_size)); + // update va_ndx + bx.store(offset_next_corrected, offset_ptr, bx.tcx().data_layout.pointer_align.abi); + + // let stack_value_ptr = unsafe { (*va).va_stk.byte_add(offset_corrected) }; + let stack_area_ptr = bx.inbounds_gep(bx.type_i8(), va_list_addr, &[bx.cx.const_usize(0)]); + let stack_area = bx.load(bx.type_ptr(), stack_area_ptr, bx.tcx().data_layout.pointer_align.abi); + let stack_value_ptr = bx.inbounds_gep(bx.type_i8(), stack_area, &[offset_corrected]); + bx.br(end); + + bx.switch_to_block(end); + + // On big-endian, for values smaller than the slot size we'd have to align the read to the end + // of the slot rather than the start. While the ISA and GCC support big-endian, all the Xtensa + // targets supported by rustc are litte-endian so don't worry about it. + + // if from_regsave { + // unsafe { *regsave_value_ptr } + // } else { + // unsafe { *stack_value_ptr } + // } + assert!(bx.tcx().sess.target.endian == Endian::Little); + let value_ptr = + bx.phi(bx.type_ptr(), &[regsave_value_ptr, stack_value_ptr], &[from_regsave, from_stack]); + return bx.load(layout.llvm_type(bx), value_ptr, layout.align.abi); +} + pub(super) fn emit_va_arg<'ll, 'tcx>( bx: &mut Builder<'_, 'll, 'tcx>, addr: OperandRef<'tcx, &'ll Value>, @@ -302,6 +410,7 @@ pub(super) fn emit_va_arg<'ll, 'tcx>( let indirect: bool = target_ty_size > 8 || !target_ty_size.is_power_of_two(); emit_ptr_va_arg(bx, addr, target_ty, indirect, Align::from_bytes(8).unwrap(), false) } + "xtensa" => emit_xtensa_va_arg(bx, addr, target_ty), // For all other architecture/OS combinations fall back to using // the LLVM va_arg instruction. // https://llvm.org/docs/LangRef.html#va-arg-instruction diff --git a/library/core/Cargo.toml b/library/core/Cargo.toml index cace4582b489a..46c55c437cce5 100644 --- a/library/core/Cargo.toml +++ b/library/core/Cargo.toml @@ -43,6 +43,7 @@ check-cfg = [ 'cfg(bootstrap)', 'cfg(no_fp_fmt_parse)', 'cfg(stdarch_intel_sde)', + 'cfg(target_arch, values("xtensa"))', # core use #[path] imports to portable-simd `core_simd` crate # and to stdarch `core_arch` crate which messes-up with Cargo list # of declared features, we therefor expect any feature cfg diff --git a/library/core/src/ffi/va_list.rs b/library/core/src/ffi/va_list.rs index 3a224e4d8fe5f..f67c592d8d8f7 100644 --- a/library/core/src/ffi/va_list.rs +++ b/library/core/src/ffi/va_list.rs @@ -15,6 +15,7 @@ use crate::ops::{Deref, DerefMut}; not(target_arch = "aarch64"), not(target_arch = "powerpc"), not(target_arch = "s390x"), + not(target_arch = "xtensa"), not(target_arch = "x86_64") ), all(target_arch = "aarch64", target_vendor = "apple"), @@ -37,6 +38,7 @@ pub struct VaListImpl<'f> { not(target_arch = "aarch64"), not(target_arch = "powerpc"), not(target_arch = "s390x"), + not(target_arch = "xtensa"), not(target_arch = "x86_64") ), all(target_arch = "aarch64", target_vendor = "apple"), @@ -113,6 +115,18 @@ pub struct VaListImpl<'f> { _marker: PhantomData<&'f mut &'f c_void>, } +/// Xtensa ABI implementation of a `va_list`. +#[cfg(target_arch = "xtensa")] +#[repr(C)] +#[derive(Debug)] +#[lang = "va_list"] +pub struct VaListImpl<'f> { + stk: *mut i32, + reg: *mut i32, + ndx: i32, + _marker: PhantomData<&'f mut &'f c_void>, +} + /// A wrapper for a `va_list` #[repr(transparent)] #[derive(Debug)] @@ -124,6 +138,7 @@ pub struct VaList<'a, 'f: 'a> { not(target_arch = "s390x"), not(target_arch = "x86_64") ), + target_arch = "xtensa", all(target_arch = "aarch64", target_vendor = "apple"), target_family = "wasm", target_os = "uefi", @@ -138,6 +153,7 @@ pub struct VaList<'a, 'f: 'a> { target_arch = "s390x", target_arch = "x86_64" ), + not(target_arch = "xtensa"), any(not(target_arch = "aarch64"), not(target_vendor = "apple")), not(target_family = "wasm"), not(target_os = "uefi"), @@ -155,6 +171,7 @@ pub struct VaList<'a, 'f: 'a> { not(target_arch = "s390x"), not(target_arch = "x86_64") ), + target_arch = "xtensa", all(target_arch = "aarch64", target_vendor = "apple"), target_family = "wasm", target_os = "uefi", @@ -173,8 +190,10 @@ impl<'f> VaListImpl<'f> { target_arch = "aarch64", target_arch = "powerpc", target_arch = "s390x", + target_arch = "xtensa", target_arch = "x86_64" ), + not(target_arch = "xtensa"), any(not(target_arch = "aarch64"), not(target_vendor = "apple")), not(target_family = "wasm"), not(target_os = "uefi"), From 7cc6f4da881b4947f0132defdd97444772c2165b Mon Sep 17 00:00:00 2001 From: Miguel Ojeda Date: Tue, 3 Dec 2024 23:24:59 +0100 Subject: [PATCH 20/27] CI: rfl: move job forward to Linux v6.13-rc1 Linux v6.13-rc1 contains commit 28e848386b92 ("rust: block: fix formatting of `kernel::block::mq::request` module"), which in turn contains commit c95bbb59a9b2 ("rust: enable arbitrary_self_types and remove `Receiver`"), which is why we had a hash rather than a tag. Signed-off-by: Miguel Ojeda --- src/ci/docker/scripts/rfl-build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ci/docker/scripts/rfl-build.sh b/src/ci/docker/scripts/rfl-build.sh index f07515f7784f4..8776e0f0be901 100755 --- a/src/ci/docker/scripts/rfl-build.sh +++ b/src/ci/docker/scripts/rfl-build.sh @@ -2,7 +2,7 @@ set -euo pipefail -LINUX_VERSION=28e848386b92645f93b9f2fdba5882c3ca7fb3e2 +LINUX_VERSION=v6.13-rc1 # Build rustc, rustdoc, cargo, clippy-driver and rustfmt ../x.py build --stage 2 library rustdoc clippy rustfmt @@ -64,7 +64,7 @@ make -C linux LLVM=1 -j$(($(nproc) + 1)) \ BUILD_TARGETS=" samples/rust/rust_minimal.o - samples/rust/rust_print.o + samples/rust/rust_print_main.o drivers/net/phy/ax88796b_rust.o rust/doctests_kernel_generated.o " From f4217f42967621c53dbeb1ecdd698c1d17e97b7a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 4 Dec 2024 08:37:23 +0100 Subject: [PATCH 21/27] clarify simd_relaxed_fma non-determinism --- library/core/src/intrinsics/simd.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/core/src/intrinsics/simd.rs b/library/core/src/intrinsics/simd.rs index 0d24b0558c594..f80a60d471c04 100644 --- a/library/core/src/intrinsics/simd.rs +++ b/library/core/src/intrinsics/simd.rs @@ -619,7 +619,8 @@ extern "rust-intrinsic" { /// set has support for a fused operation, and that the fused operation is more efficient /// than the equivalent, separate pair of mul and add instructions. It is unspecified /// whether or not a fused operation is selected, and that may depend on optimization - /// level and context, for example. + /// level and context, for example. It may even be the case that some SIMD lanes get fused + /// and others do not. /// /// `T` must be a vector of floats. #[cfg(not(bootstrap))] From 76f9aa1d7edb579621f0ab50d18665461bc6ae48 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Thu, 5 Dec 2024 04:23:57 +0900 Subject: [PATCH 22/27] Fix "std" support status of some tier 3 targets --- .../spec/targets/powerpc64le_unknown_freebsd.rs | 4 ++-- .../src/spec/targets/powerpc_unknown_freebsd.rs | 2 +- .../spec/targets/riscv32gc_unknown_linux_gnu.rs | 2 +- .../spec/targets/riscv32gc_unknown_linux_musl.rs | 2 +- .../src/spec/targets/riscv64_linux_android.rs | 2 +- .../spec/targets/riscv64gc_unknown_freebsd.rs | 2 +- .../spec/targets/riscv64gc_unknown_fuchsia.rs | 2 +- .../spec/targets/riscv64gc_unknown_linux_musl.rs | 2 +- .../src/spec/targets/s390x_unknown_linux_musl.rs | 2 +- src/doc/rustc/src/platform-support.md | 16 ++++++++-------- 10 files changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_freebsd.rs b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_freebsd.rs index e194f5f8a36f3..0c1218bd05939 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_freebsd.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_freebsd.rs @@ -12,8 +12,8 @@ pub(crate) fn target() -> Target { metadata: crate::spec::TargetMetadata { description: Some("PPC64LE FreeBSD".into()), tier: Some(3), - host_tools: Some(false), - std: Some(false), + host_tools: Some(true), + std: Some(true), }, pointer_width: 64, data_layout: "e-m:e-Fn32-i64:64-n32:64".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc_unknown_freebsd.rs b/compiler/rustc_target/src/spec/targets/powerpc_unknown_freebsd.rs index 393946980f8b3..b72f8e8acf8db 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc_unknown_freebsd.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc_unknown_freebsd.rs @@ -17,7 +17,7 @@ pub(crate) fn target() -> Target { description: Some("PowerPC FreeBSD".into()), tier: Some(3), host_tools: Some(false), - std: Some(false), + std: None, }, pointer_width: 32, data_layout: "E-m:e-p:32:32-Fn32-i64:64-n32".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_gnu.rs index 8b0a140c9ade5..e9c57b99b9279 100644 --- a/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_gnu.rs @@ -9,7 +9,7 @@ pub(crate) fn target() -> Target { description: Some("RISC-V Linux (kernel 5.4, glibc 2.33)".into()), tier: Some(3), host_tools: Some(false), - std: Some(false), + std: Some(true), }, pointer_width: 32, data_layout: "e-m:e-p:32:32-i64:64-n32-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_musl.rs index 212de791e499a..db8482110eb67 100644 --- a/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_musl.rs @@ -11,7 +11,7 @@ pub(crate) fn target() -> Target { ), tier: Some(3), host_tools: Some(false), - std: Some(false), + std: None, }, pointer_width: 32, data_layout: "e-m:e-p:32:32-i64:64-n32-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv64_linux_android.rs b/compiler/rustc_target/src/spec/targets/riscv64_linux_android.rs index 8b5d4751a5733..38bf4b006eb74 100644 --- a/compiler/rustc_target/src/spec/targets/riscv64_linux_android.rs +++ b/compiler/rustc_target/src/spec/targets/riscv64_linux_android.rs @@ -9,7 +9,7 @@ pub(crate) fn target() -> Target { description: Some("RISC-V 64-bit Android".into()), tier: Some(3), host_tools: Some(false), - std: Some(false), + std: None, }, pointer_width: 64, data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_freebsd.rs b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_freebsd.rs index d1b0d801fd60b..db56e264e943a 100644 --- a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_freebsd.rs +++ b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_freebsd.rs @@ -7,7 +7,7 @@ pub(crate) fn target() -> Target { description: Some("RISC-V FreeBSD".into()), tier: Some(3), host_tools: Some(false), - std: Some(false), + std: None, }, pointer_width: 64, data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_fuchsia.rs b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_fuchsia.rs index 6de51f9b07e2a..75b7dfcbb0922 100644 --- a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_fuchsia.rs +++ b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_fuchsia.rs @@ -7,7 +7,7 @@ pub(crate) fn target() -> Target { description: Some("RISC-V Fuchsia".into()), tier: Some(3), host_tools: Some(false), - std: Some(false), + std: None, }, pointer_width: 64, data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_linux_musl.rs index 2e6fce91d4c75..397c202ec1a57 100644 --- a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_linux_musl.rs @@ -9,7 +9,7 @@ pub(crate) fn target() -> Target { description: Some("RISC-V Linux (kernel 4.20, musl 1.2.3)".into()), tier: Some(3), host_tools: Some(false), - std: Some(false), + std: Some(true), }, pointer_width: 64, data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/s390x_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/s390x_unknown_linux_musl.rs index 4bde0fb729c75..7a78004927b10 100644 --- a/compiler/rustc_target/src/spec/targets/s390x_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/s390x_unknown_linux_musl.rs @@ -19,7 +19,7 @@ pub(crate) fn target() -> Target { description: Some("S390x Linux (kernel 3.2, musl 1.2.3)".into()), tier: Some(3), host_tools: Some(false), - std: Some(false), + std: Some(true), }, pointer_width: 64, data_layout: "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64".into(), diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index 243cb3b2fc832..3b82d123276d9 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -344,15 +344,15 @@ target | std | host | notes [`powerpc-wrs-vxworks-spe`](platform-support/vxworks.md) | ✓ | | [`powerpc-wrs-vxworks`](platform-support/vxworks.md) | ✓ | | `powerpc64-unknown-freebsd` | ✓ | ✓ | PPC64 FreeBSD (ELFv1 and ELFv2) -`powerpc64le-unknown-freebsd` | | | PPC64LE FreeBSD -`powerpc-unknown-freebsd` | | | PowerPC FreeBSD +`powerpc64le-unknown-freebsd` | ✓ | ✓ | PPC64LE FreeBSD +`powerpc-unknown-freebsd` | ? | | PowerPC FreeBSD `powerpc64-unknown-linux-musl` | ? | | 64-bit PowerPC Linux with musl 1.2.3 [`powerpc64-wrs-vxworks`](platform-support/vxworks.md) | ✓ | | `powerpc64le-unknown-linux-musl` | ? | | 64-bit PowerPC Linux with musl 1.2.3, Little Endian [`powerpc64-unknown-openbsd`](platform-support/openbsd.md) | ✓ | ✓ | OpenBSD/powerpc64 [`powerpc64-ibm-aix`](platform-support/aix.md) | ? | | 64-bit AIX (7.2 and newer) -`riscv32gc-unknown-linux-gnu` | | | RISC-V Linux (kernel 5.4, glibc 2.33) -`riscv32gc-unknown-linux-musl` | | | RISC-V Linux (kernel 5.4, musl 1.2.3 + RISCV32 support patches) +`riscv32gc-unknown-linux-gnu` | ✓ | | RISC-V Linux (kernel 5.4, glibc 2.33) +`riscv32gc-unknown-linux-musl` | ? | | RISC-V Linux (kernel 5.4, musl 1.2.3 + RISCV32 support patches) [`riscv32im-risc0-zkvm-elf`](platform-support/riscv32im-risc0-zkvm-elf.md) | ? | | RISC Zero's zero-knowledge Virtual Machine (RV32IM ISA) [`riscv32ima-unknown-none-elf`](platform-support/riscv32-unknown-none-elf.md) | * | | Bare RISC-V (RV32IMA ISA) [`riscv32imac-unknown-xous-elf`](platform-support/riscv32imac-unknown-xous-elf.md) | ? | | RISC-V Xous (RV32IMAC ISA) @@ -361,13 +361,13 @@ target | std | host | notes [`riscv32imafc-esp-espidf`](platform-support/esp-idf.md) | ✓ | | RISC-V ESP-IDF [`riscv32-wrs-vxworks`](platform-support/vxworks.md) | ✓ | | [`riscv64gc-unknown-hermit`](platform-support/hermit.md) | ✓ | | RISC-V Hermit -`riscv64gc-unknown-freebsd` | | | RISC-V FreeBSD -`riscv64gc-unknown-fuchsia` | | | RISC-V Fuchsia +`riscv64gc-unknown-freebsd` | ? | | RISC-V FreeBSD +`riscv64gc-unknown-fuchsia` | ? | | RISC-V Fuchsia [`riscv64gc-unknown-netbsd`](platform-support/netbsd.md) | ✓ | ✓ | RISC-V NetBSD [`riscv64gc-unknown-openbsd`](platform-support/openbsd.md) | ✓ | ✓ | OpenBSD/riscv64 -[`riscv64-linux-android`](platform-support/android.md) | | | RISC-V 64-bit Android +[`riscv64-linux-android`](platform-support/android.md) | ? | | RISC-V 64-bit Android [`riscv64-wrs-vxworks`](platform-support/vxworks.md) | ✓ | | -[`s390x-unknown-linux-musl`](platform-support/s390x-unknown-linux-musl.md) | | | S390x Linux (kernel 3.2, musl 1.2.3) +[`s390x-unknown-linux-musl`](platform-support/s390x-unknown-linux-musl.md) | ✓ | | S390x Linux (kernel 3.2, musl 1.2.3) `sparc-unknown-linux-gnu` | ✓ | | 32-bit SPARC Linux [`sparc-unknown-none-elf`](./platform-support/sparc-unknown-none-elf.md) | * | | Bare 32-bit SPARC V7+ [`sparc64-unknown-netbsd`](platform-support/netbsd.md) | ✓ | ✓ | NetBSD/sparc64 From 4ba701977d97ca1001c4daca4053a36ec160e0cb Mon Sep 17 00:00:00 2001 From: jyn Date: Sun, 1 Dec 2024 23:57:09 -0500 Subject: [PATCH 23/27] compiletest: show the difference between the normalized output and the actual output for lines which didn't match example output: ``` failures: ---- [ui] tests/ui/layout/enum.rs stdout ---- diff of stderr: - error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIGN } + error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIN } 2 --> $DIR/enum.rs:9:1 3 | 4 LL | enum UninhabitedVariantAlign { Note: some mismatched output was normalized before being compared - error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: Align(8 bytes) } - --> /home/jyn/src/rust2/tests/ui/layout/enum.rs:9:1 + error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIN } ``` --- src/tools/compiletest/src/runtest.rs | 136 ++++++++++++++---- src/tools/compiletest/src/runtest/coverage.rs | 16 ++- src/tools/compiletest/src/runtest/ui.rs | 2 +- 3 files changed, 119 insertions(+), 35 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 8c747a1d93f61..1cf6b55accd26 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -22,7 +22,7 @@ use crate::common::{ UI_STDERR, UI_STDOUT, UI_SVG, UI_WINDOWS_SVG, Ui, expected_output_path, incremental_dir, output_base_dir, output_base_name, output_testname_unique, }; -use crate::compute_diff::{write_diff, write_filtered_diff}; +use crate::compute_diff::{DiffLine, make_diff, write_diff, write_filtered_diff}; use crate::errors::{self, Error, ErrorKind}; use crate::header::TestProps; use crate::read2::{Truncated, read2_abbreviated}; @@ -2295,17 +2295,31 @@ impl<'test> TestCx<'test> { match output_kind { TestOutput::Compile => { if !self.props.dont_check_compiler_stdout { - errors += - self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout); + errors += self.compare_output( + stdout_kind, + &normalized_stdout, + &proc_res.stdout, + &expected_stdout, + ); } if !self.props.dont_check_compiler_stderr { - errors += - self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr); + errors += self.compare_output( + stderr_kind, + &normalized_stderr, + &stderr, + &expected_stderr, + ); } } TestOutput::Run => { - errors += self.compare_output(stdout_kind, &normalized_stdout, &expected_stdout); - errors += self.compare_output(stderr_kind, &normalized_stderr, &expected_stderr); + errors += self.compare_output( + stdout_kind, + &normalized_stdout, + &proc_res.stdout, + &expected_stdout, + ); + errors += + self.compare_output(stderr_kind, &normalized_stderr, &stderr, &expected_stderr); } } errors @@ -2533,7 +2547,13 @@ impl<'test> TestCx<'test> { } } - fn compare_output(&self, stream: &str, actual: &str, expected: &str) -> usize { + fn compare_output( + &self, + stream: &str, + actual: &str, + actual_unnormalized: &str, + expected: &str, + ) -> usize { let are_different = match (self.force_color_svg(), expected.find('\n'), actual.find('\n')) { // FIXME: We ignore the first line of SVG files // because the width parameter is non-deterministic. @@ -2590,28 +2610,14 @@ impl<'test> TestCx<'test> { if expected.is_empty() { eprintln!("normalized {}:\n{}\n", stream, actual); } else { - eprintln!("diff of {stream}:\n"); - if let Some(diff_command) = self.config.diff_command.as_deref() { - let mut args = diff_command.split_whitespace(); - let name = args.next().unwrap(); - match Command::new(name) - .args(args) - .args([&expected_path, &actual_path]) - .output() - { - Err(err) => { - self.fatal(&format!( - "failed to call custom diff command `{diff_command}`: {err}" - )); - } - Ok(output) => { - let output = String::from_utf8_lossy(&output.stdout); - print!("{output}"); - } - } - } else { - print!("{}", write_diff(expected, actual, 3)); - } + self.show_diff( + stream, + &expected_path, + &actual_path, + expected, + actual, + actual_unnormalized, + ); } } else { // Delete non-revision .stderr/.stdout file if revisions are used. @@ -2633,6 +2639,76 @@ impl<'test> TestCx<'test> { if self.config.bless { 0 } else { 1 } } + /// Returns whether to show the full stderr/stdout. + fn show_diff( + &self, + stream: &str, + expected_path: &Path, + actual_path: &Path, + expected: &str, + actual: &str, + actual_unnormalized: &str, + ) { + eprintln!("diff of {stream}:\n"); + if let Some(diff_command) = self.config.diff_command.as_deref() { + let mut args = diff_command.split_whitespace(); + let name = args.next().unwrap(); + match Command::new(name).args(args).args([expected_path, actual_path]).output() { + Err(err) => { + self.fatal(&format!( + "failed to call custom diff command `{diff_command}`: {err}" + )); + } + Ok(output) => { + let output = String::from_utf8_lossy(&output.stdout); + eprint!("{output}"); + } + } + } else { + eprint!("{}", write_diff(expected, actual, 3)); + } + + // NOTE: argument order is important, we need `actual` to be on the left so the line number match up when we compare it to `actual_unnormalized` below. + let diff_results = make_diff(actual, expected, 0); + + let (mut mismatches_normalized, mut mismatch_line_nos) = (String::new(), vec![]); + for hunk in diff_results { + let mut line_no = hunk.line_number; + for line in hunk.lines { + // NOTE: `Expected` is actually correct here, the argument order is reversed so our line numbers match up + if let DiffLine::Expected(normalized) = line { + mismatches_normalized += &normalized; + mismatches_normalized += "\n"; + mismatch_line_nos.push(line_no); + line_no += 1; + } + } + } + let mut mismatches_unnormalized = String::new(); + let diff_normalized = make_diff(actual, actual_unnormalized, 0); + for hunk in diff_normalized { + if mismatch_line_nos.contains(&hunk.line_number) { + for line in hunk.lines { + if let DiffLine::Resulting(unnormalized) = line { + mismatches_unnormalized += &unnormalized; + mismatches_unnormalized += "\n"; + } + } + } + } + + let normalized_diff = make_diff(&mismatches_normalized, &mismatches_unnormalized, 0); + // HACK: instead of checking if each hunk is empty, this only checks if the whole input is empty. we should be smarter about this so we don't treat added or removed output as normalized. + if !normalized_diff.is_empty() + && !mismatches_unnormalized.is_empty() + && !mismatches_normalized.is_empty() + { + eprintln!("Note: some mismatched output was normalized before being compared"); + // FIXME: respect diff_command + eprint!("{}", write_diff(&mismatches_unnormalized, &mismatches_normalized, 0)); + } + } + fn check_and_prune_duplicate_outputs( &self, proc_res: &ProcRes, diff --git a/src/tools/compiletest/src/runtest/coverage.rs b/src/tools/compiletest/src/runtest/coverage.rs index 961a160298631..030ca5ebb2474 100644 --- a/src/tools/compiletest/src/runtest/coverage.rs +++ b/src/tools/compiletest/src/runtest/coverage.rs @@ -39,8 +39,12 @@ impl<'test> TestCx<'test> { let expected_coverage_dump = self.load_expected_output(kind); let actual_coverage_dump = self.normalize_output(&proc_res.stdout, &[]); - let coverage_dump_errors = - self.compare_output(kind, &actual_coverage_dump, &expected_coverage_dump); + let coverage_dump_errors = self.compare_output( + kind, + &actual_coverage_dump, + &proc_res.stdout, + &expected_coverage_dump, + ); if coverage_dump_errors > 0 { self.fatal_proc_rec( @@ -135,8 +139,12 @@ impl<'test> TestCx<'test> { self.fatal_proc_rec(&err, &proc_res); }); - let coverage_errors = - self.compare_output(kind, &normalized_actual_coverage, &expected_coverage); + let coverage_errors = self.compare_output( + kind, + &normalized_actual_coverage, + &proc_res.stdout, + &expected_coverage, + ); if coverage_errors > 0 { self.fatal_proc_rec( diff --git a/src/tools/compiletest/src/runtest/ui.rs b/src/tools/compiletest/src/runtest/ui.rs index 414f1a6adb33d..bc860bf18c741 100644 --- a/src/tools/compiletest/src/runtest/ui.rs +++ b/src/tools/compiletest/src/runtest/ui.rs @@ -100,7 +100,7 @@ impl TestCx<'_> { ) }); - errors += self.compare_output("fixed", &fixed_code, &expected_fixed); + errors += self.compare_output("fixed", &fixed_code, &fixed_code, &expected_fixed); } else if !expected_fixed.is_empty() { panic!( "the `//@ run-rustfix` directive wasn't found but a `*.fixed` \ From e3638e24215e11a5f2081c470fe76c9651857d63 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Thu, 5 Dec 2024 10:49:36 +0900 Subject: [PATCH 24/27] Mark all Linux/Android/BSD/Fuchsia as supporting "std" in target-spec metadata --- .../src/spec/targets/armv4t_unknown_linux_gnueabi.rs | 2 +- .../src/spec/targets/armv5te_unknown_linux_uclibceabi.rs | 2 +- .../rustc_target/src/spec/targets/m68k_unknown_linux_gnu.rs | 2 +- .../rustc_target/src/spec/targets/mips64_openwrt_linux_musl.rs | 2 +- .../src/spec/targets/mipsisa32r6_unknown_linux_gnu.rs | 2 +- .../src/spec/targets/mipsisa32r6el_unknown_linux_gnu.rs | 2 +- .../src/spec/targets/mipsisa64r6_unknown_linux_gnuabi64.rs | 2 +- .../src/spec/targets/powerpc64_unknown_linux_musl.rs | 2 +- .../src/spec/targets/powerpc64le_unknown_linux_musl.rs | 2 +- .../rustc_target/src/spec/targets/powerpc_unknown_freebsd.rs | 2 +- .../rustc_target/src/spec/targets/powerpc_unknown_linux_musl.rs | 2 +- .../rustc_target/src/spec/targets/powerpc_unknown_openbsd.rs | 2 +- .../src/spec/targets/riscv32gc_unknown_linux_musl.rs | 2 +- compiler/rustc_target/src/spec/targets/riscv64_linux_android.rs | 2 +- .../rustc_target/src/spec/targets/riscv64gc_unknown_freebsd.rs | 2 +- .../rustc_target/src/spec/targets/riscv64gc_unknown_fuchsia.rs | 2 +- .../src/spec/targets/thumbv7neon_unknown_linux_musleabihf.rs | 2 +- .../rustc_target/src/spec/targets/x86_64_unknown_linux_none.rs | 2 +- 18 files changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_target/src/spec/targets/armv4t_unknown_linux_gnueabi.rs b/compiler/rustc_target/src/spec/targets/armv4t_unknown_linux_gnueabi.rs index 3a2fb753ba147..7ca2babffe9fc 100644 --- a/compiler/rustc_target/src/spec/targets/armv4t_unknown_linux_gnueabi.rs +++ b/compiler/rustc_target/src/spec/targets/armv4t_unknown_linux_gnueabi.rs @@ -7,7 +7,7 @@ pub(crate) fn target() -> Target { description: Some("Armv4T Linux".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 32, data_layout: "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64".into(), diff --git a/compiler/rustc_target/src/spec/targets/armv5te_unknown_linux_uclibceabi.rs b/compiler/rustc_target/src/spec/targets/armv5te_unknown_linux_uclibceabi.rs index e0bc1936e221c..154103c36f79c 100644 --- a/compiler/rustc_target/src/spec/targets/armv5te_unknown_linux_uclibceabi.rs +++ b/compiler/rustc_target/src/spec/targets/armv5te_unknown_linux_uclibceabi.rs @@ -7,7 +7,7 @@ pub(crate) fn target() -> Target { description: Some("Armv5TE Linux with uClibc".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 32, data_layout: "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64".into(), diff --git a/compiler/rustc_target/src/spec/targets/m68k_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/targets/m68k_unknown_linux_gnu.rs index 1515d8fe00abc..47c6d80548359 100644 --- a/compiler/rustc_target/src/spec/targets/m68k_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/m68k_unknown_linux_gnu.rs @@ -12,7 +12,7 @@ pub(crate) fn target() -> Target { description: Some("Motorola 680x0 Linux".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 32, data_layout: "E-m:e-p:32:16:32-i8:8:8-i16:16:16-i32:16:32-n8:16:32-a:0:16-S16".into(), diff --git a/compiler/rustc_target/src/spec/targets/mips64_openwrt_linux_musl.rs b/compiler/rustc_target/src/spec/targets/mips64_openwrt_linux_musl.rs index b7415bf683de2..cc288e042d987 100644 --- a/compiler/rustc_target/src/spec/targets/mips64_openwrt_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/mips64_openwrt_linux_musl.rs @@ -17,7 +17,7 @@ pub(crate) fn target() -> Target { description: Some("MIPS64 for OpenWrt Linux musl 1.2.3".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 64, data_layout: "E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/mipsisa32r6_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/targets/mipsisa32r6_unknown_linux_gnu.rs index 2ff06dc6669bb..3ef0988d23570 100644 --- a/compiler/rustc_target/src/spec/targets/mipsisa32r6_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/mipsisa32r6_unknown_linux_gnu.rs @@ -8,7 +8,7 @@ pub(crate) fn target() -> Target { description: Some("32-bit MIPS Release 6 Big Endian".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 32, data_layout: "E-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64".into(), diff --git a/compiler/rustc_target/src/spec/targets/mipsisa32r6el_unknown_linux_gnu.rs b/compiler/rustc_target/src/spec/targets/mipsisa32r6el_unknown_linux_gnu.rs index 05b246698482a..c75023385c220 100644 --- a/compiler/rustc_target/src/spec/targets/mipsisa32r6el_unknown_linux_gnu.rs +++ b/compiler/rustc_target/src/spec/targets/mipsisa32r6el_unknown_linux_gnu.rs @@ -7,7 +7,7 @@ pub(crate) fn target() -> Target { description: Some("32-bit MIPS Release 6 Little Endian".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 32, data_layout: "e-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64".into(), diff --git a/compiler/rustc_target/src/spec/targets/mipsisa64r6_unknown_linux_gnuabi64.rs b/compiler/rustc_target/src/spec/targets/mipsisa64r6_unknown_linux_gnuabi64.rs index c5948b745ff59..d0d0ed1acc3f0 100644 --- a/compiler/rustc_target/src/spec/targets/mipsisa64r6_unknown_linux_gnuabi64.rs +++ b/compiler/rustc_target/src/spec/targets/mipsisa64r6_unknown_linux_gnuabi64.rs @@ -8,7 +8,7 @@ pub(crate) fn target() -> Target { description: Some("64-bit MIPS Release 6 Big Endian".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 64, data_layout: "E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_musl.rs index 5da891cc13f6d..62c30aebc518e 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64_unknown_linux_musl.rs @@ -14,7 +14,7 @@ pub(crate) fn target() -> Target { description: Some("64-bit PowerPC Linux with musl 1.2.3".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 64, data_layout: "E-m:e-Fn32-i64:64-n32:64-S128-v256:256:256-v512:512:512".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_musl.rs index e2921aa17fe64..50946ae4ce66d 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc64le_unknown_linux_musl.rs @@ -13,7 +13,7 @@ pub(crate) fn target() -> Target { description: Some("64-bit PowerPC Linux with musl 1.2.3, Little Endian".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 64, data_layout: "e-m:e-Fn32-i64:64-n32:64-S128-v256:256:256-v512:512:512".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc_unknown_freebsd.rs b/compiler/rustc_target/src/spec/targets/powerpc_unknown_freebsd.rs index b72f8e8acf8db..9cfc2153ac170 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc_unknown_freebsd.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc_unknown_freebsd.rs @@ -17,7 +17,7 @@ pub(crate) fn target() -> Target { description: Some("PowerPC FreeBSD".into()), tier: Some(3), host_tools: Some(false), - std: None, + std: Some(true), }, pointer_width: 32, data_layout: "E-m:e-p:32:32-Fn32-i64:64-n32".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/powerpc_unknown_linux_musl.rs index 9859e8ced4f46..5372a83e29a22 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc_unknown_linux_musl.rs @@ -13,7 +13,7 @@ pub(crate) fn target() -> Target { description: Some("PowerPC Linux with musl 1.2.3".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 32, data_layout: "E-m:e-p:32:32-Fn32-i64:64-n32".into(), diff --git a/compiler/rustc_target/src/spec/targets/powerpc_unknown_openbsd.rs b/compiler/rustc_target/src/spec/targets/powerpc_unknown_openbsd.rs index a8254af9736c0..7aedc2d0665cb 100644 --- a/compiler/rustc_target/src/spec/targets/powerpc_unknown_openbsd.rs +++ b/compiler/rustc_target/src/spec/targets/powerpc_unknown_openbsd.rs @@ -13,7 +13,7 @@ pub(crate) fn target() -> Target { description: None, tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 32, data_layout: "E-m:e-p:32:32-Fn32-i64:64-n32".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_musl.rs b/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_musl.rs index db8482110eb67..08871d9d72bfb 100644 --- a/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_musl.rs +++ b/compiler/rustc_target/src/spec/targets/riscv32gc_unknown_linux_musl.rs @@ -11,7 +11,7 @@ pub(crate) fn target() -> Target { ), tier: Some(3), host_tools: Some(false), - std: None, + std: Some(true), }, pointer_width: 32, data_layout: "e-m:e-p:32:32-i64:64-n32-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv64_linux_android.rs b/compiler/rustc_target/src/spec/targets/riscv64_linux_android.rs index 38bf4b006eb74..f694a1cb60dd2 100644 --- a/compiler/rustc_target/src/spec/targets/riscv64_linux_android.rs +++ b/compiler/rustc_target/src/spec/targets/riscv64_linux_android.rs @@ -9,7 +9,7 @@ pub(crate) fn target() -> Target { description: Some("RISC-V 64-bit Android".into()), tier: Some(3), host_tools: Some(false), - std: None, + std: Some(true), }, pointer_width: 64, data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_freebsd.rs b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_freebsd.rs index db56e264e943a..905bed76db4d0 100644 --- a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_freebsd.rs +++ b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_freebsd.rs @@ -7,7 +7,7 @@ pub(crate) fn target() -> Target { description: Some("RISC-V FreeBSD".into()), tier: Some(3), host_tools: Some(false), - std: None, + std: Some(true), }, pointer_width: 64, data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_fuchsia.rs b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_fuchsia.rs index 75b7dfcbb0922..7a887b604c507 100644 --- a/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_fuchsia.rs +++ b/compiler/rustc_target/src/spec/targets/riscv64gc_unknown_fuchsia.rs @@ -7,7 +7,7 @@ pub(crate) fn target() -> Target { description: Some("RISC-V Fuchsia".into()), tier: Some(3), host_tools: Some(false), - std: None, + std: Some(true), }, pointer_width: 64, data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), diff --git a/compiler/rustc_target/src/spec/targets/thumbv7neon_unknown_linux_musleabihf.rs b/compiler/rustc_target/src/spec/targets/thumbv7neon_unknown_linux_musleabihf.rs index 5c43ca69bd852..2f539f4f6004d 100644 --- a/compiler/rustc_target/src/spec/targets/thumbv7neon_unknown_linux_musleabihf.rs +++ b/compiler/rustc_target/src/spec/targets/thumbv7neon_unknown_linux_musleabihf.rs @@ -16,7 +16,7 @@ pub(crate) fn target() -> Target { description: Some("Thumb2-mode ARMv7-A Linux with NEON, musl 1.2.3".into()), tier: Some(3), host_tools: Some(false), - std: None, // ? + std: Some(true), }, pointer_width: 32, data_layout: "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64".into(), diff --git a/compiler/rustc_target/src/spec/targets/x86_64_unknown_linux_none.rs b/compiler/rustc_target/src/spec/targets/x86_64_unknown_linux_none.rs index 91eb7b53fea8a..900dbed205c8e 100644 --- a/compiler/rustc_target/src/spec/targets/x86_64_unknown_linux_none.rs +++ b/compiler/rustc_target/src/spec/targets/x86_64_unknown_linux_none.rs @@ -14,7 +14,7 @@ pub(crate) fn target() -> Target { description: None, tier: None, host_tools: None, - std: None, + std: Some(true), }, pointer_width: 64, data_layout: From 01442e70a775556a62d7f3678b3c558400094861 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 5 Dec 2024 03:13:47 +0000 Subject: [PATCH 25/27] Never close a job after the process is assigned --- src/bootstrap/src/utils/job.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/bootstrap/src/utils/job.rs b/src/bootstrap/src/utils/job.rs index c5e892450c4c5..b8c97f3d22308 100644 --- a/src/bootstrap/src/utils/job.rs +++ b/src/bootstrap/src/utils/job.rs @@ -124,14 +124,19 @@ mod for_windows { // (only when wrongly setting the environmental variable), // it might be better to improve the experience of the second case // when users have interrupted the parent process and we haven't finish - // duplicating the handle yet. We just need close the job object if that occurs. - CloseHandle(job).ok(); + // duplicating the handle yet. return; } }; let mut parent_handle = HANDLE::default(); - let r = DuplicateHandle( + // If this fails, well at least we tried! An example of DuplicateHandle + // failing in the past has been when the wrong python2 package spawned this + // build system (e.g., the `python2` package in MSYS instead of + // `mingw-w64-x86_64-python2`). Not sure why it failed, but the "failure + // mode" here is that we only clean everything up when the build system + // dies, not when the python parent does, so not too bad. + let _ = DuplicateHandle( GetCurrentProcess(), job, parent, @@ -140,15 +145,6 @@ mod for_windows { false, DUPLICATE_SAME_ACCESS, ); - - // If this failed, well at least we tried! An example of DuplicateHandle - // failing in the past has been when the wrong python2 package spawned this - // build system (e.g., the `python2` package in MSYS instead of - // `mingw-w64-x86_64-python2`). Not sure why it failed, but the "failure - // mode" here is that we only clean everything up when the build system - // dies, not when the python parent does, so not too bad. - if r.is_err() { - CloseHandle(job).ok(); - } + CloseHandle(parent).ok(); } } From a89f340efed5112eda108150ccc47f329aacc0e4 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 5 Dec 2024 03:18:53 +0000 Subject: [PATCH 26/27] Nested job objects are now supported in CI Nested job objects aren't supported on Windows 7 but we've long since moved on from that. --- src/bootstrap/src/utils/job.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/bootstrap/src/utils/job.rs b/src/bootstrap/src/utils/job.rs index b8c97f3d22308..bd2d86498c515 100644 --- a/src/bootstrap/src/utils/job.rs +++ b/src/bootstrap/src/utils/job.rs @@ -87,15 +87,7 @@ mod for_windows { ); assert!(r.is_ok(), "{}", io::Error::last_os_error()); - // Assign our process to this job object. Note that if this fails, one very - // likely reason is that we are ourselves already in a job object! This can - // happen on the build bots that we've got for Windows, or if just anyone - // else is instrumenting the build. In this case we just bail out - // immediately and assume that they take care of it. - // - // Also note that nested jobs (why this might fail) are supported in recent - // versions of Windows, but the version of Windows that our bots are running - // at least don't support nested job objects. + // Assign our process to this job object. let r = AssignProcessToJobObject(job, GetCurrentProcess()); if r.is_err() { CloseHandle(job).ok(); From 898d75122f9998205516759022b740e873e30dcc Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Thu, 5 Dec 2024 03:45:05 +0000 Subject: [PATCH 27/27] Update comments on Windows job objects --- src/bootstrap/bootstrap.py | 2 ++ src/bootstrap/src/utils/job.rs | 9 +++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/bootstrap/bootstrap.py b/src/bootstrap/bootstrap.py index 762f4e653e91b..9ef9ccec1937f 100644 --- a/src/bootstrap/bootstrap.py +++ b/src/bootstrap/bootstrap.py @@ -1184,6 +1184,8 @@ def bootstrap(args): args = [build.bootstrap_binary()] args.extend(sys.argv[1:]) env = os.environ.copy() + # The Python process ID is used when creating a Windows job object + # (see src\bootstrap\src\utils\job.rs) env["BOOTSTRAP_PARENT_ID"] = str(os.getpid()) env["BOOTSTRAP_PYTHON"] = sys.executable run(args, env=env, verbose=build.verbose, is_bootstrap=True) diff --git a/src/bootstrap/src/utils/job.rs b/src/bootstrap/src/utils/job.rs index bd2d86498c515..fb69d331d27e1 100644 --- a/src/bootstrap/src/utils/job.rs +++ b/src/bootstrap/src/utils/job.rs @@ -15,11 +15,12 @@ pub unsafe fn setup(build: &mut crate::Build) { /// /// Most of the time when you're running a build system (e.g., make) you expect /// Ctrl-C or abnormal termination to actually terminate the entire tree of -/// process in play, not just the one at the top. This currently works "by +/// processes in play. This currently works "by /// default" on Unix platforms because Ctrl-C actually sends a signal to the -/// *process group* rather than the parent process, so everything will get torn -/// down. On Windows, however, this does not happen and Ctrl-C just kills the -/// parent process. +/// *process group* so everything will get torn +/// down. On Windows, however, Ctrl-C is only sent to processes in the same console. +/// If a process is detached or attached to another console, it won't receive the +/// signal. /// /// To achieve the same semantics on Windows we use Job Objects to ensure that /// all processes die at the same time. Job objects have a mode of operation