From 9142cae7d5b86ac70f815c32c0f3b1223b62a947 Mon Sep 17 00:00:00 2001 From: Folkert Date: Thu, 8 Aug 2024 10:20:40 +0200 Subject: [PATCH 01/12] 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 02/12] 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 03/12] 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 04/12] 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 05/12] 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 06/12] 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 07/12] 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 08/12] 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 09/12] 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 9eb35a00baa0bb864c362eebd96e0447152887f6 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Sun, 1 Dec 2024 14:55:17 +0100 Subject: [PATCH 10/12] 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 | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/codegen/naked-asan.rs b/tests/codegen/naked-asan.rs index 639f069c0d9e5..8efedab6ea55d 100644 --- a/tests/codegen/naked-asan.rs +++ b/tests/codegen/naked-asan.rs @@ -12,14 +12,11 @@ pub fn caller() { page_fault_handler(1, 2); } -// CHECK: declare x86_intrcc void @page_fault_handler(ptr {{.*}}, i64 {{.*}}){{.*}}#[[ATTRS:[0-9]+]] -// CHECK-NOT: memcpy +// CHECK: declare x86_intrcc void @page_fault_handler(ptr {{.*}}, i64{{.*}}){{.*}}#[[ATTRS:[0-9]+]] #[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 10faf24a6144c849e7380b87f77465139facb8c2 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Mon, 9 Dec 2024 23:44:25 +0100 Subject: [PATCH 11/12] make naked function generics test stricter --- tests/codegen/naked-fn/generics.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/codegen/naked-fn/generics.rs b/tests/codegen/naked-fn/generics.rs index 23c7766203b9f..a33d213617a8b 100644 --- a/tests/codegen/naked-fn/generics.rs +++ b/tests/codegen/naked-fn/generics.rs @@ -55,12 +55,13 @@ impl Invert for i64 { } } -// CHECK-LABEL: generic_function // CHECK: .balign 4 +// CHECK-LABEL: generic_function: // CHECK: call // CHECK: ret #[naked] +#[no_mangle] pub extern "C" fn generic_function(x: i64) -> i64 { unsafe { naked_asm!( @@ -75,8 +76,8 @@ pub extern "C" fn generic_function(x: i64) -> i64 { #[repr(transparent)] struct Foo(u64); -// CHECK-LABEL: method // CHECK: .balign 4 +// CHECK-LABEL: method: // CHECK: mov rax, rdi impl Foo { @@ -87,8 +88,8 @@ impl Foo { } } -// CHECK-LABEL: trait_method // CHECK: .balign 4 +// CHECK-LABEL: trait_method: // CHECK: mov rax, rdi trait Bar { @@ -103,8 +104,8 @@ impl Bar for Foo { } } -// CHECK-LABEL: naked_with_args_and_return // CHECK: .balign 4 +// CHECK-LABEL: naked_with_args_and_return: // CHECK: lea rax, [rdi + rsi] // this previously ICE'd, see https://github.com/rust-lang/rust/issues/124375 From 4808cd0bde1502cccb55bd8c62e686c6ce12d270 Mon Sep 17 00:00:00 2001 From: Folkert de Vries Date: Tue, 10 Dec 2024 12:34:53 +0100 Subject: [PATCH 12/12] emit `.weak_definition` instead of `.weak` on macos --- compiler/rustc_codegen_ssa/src/mir/naked_asm.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs index 0767c002c5e06..0f1544e6ebf1d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs +++ b/compiler/rustc_codegen_ssa/src/mir/naked_asm.rs @@ -153,16 +153,23 @@ fn prefix_and_suffix<'tcx>( ("", "") }; + let asm_binary_format = AsmBinaryFormat::from_target(&tcx.sess.target); + + let weak_directive = match asm_binary_format { + AsmBinaryFormat::Elf | AsmBinaryFormat::Coff => ".weak", + AsmBinaryFormat::Macho => ".weak_definition", + }; + 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::LinkOnceAny => Some(weak_directive), + Linkage::LinkOnceODR => Some(weak_directive), + Linkage::WeakAny => Some(weak_directive), + Linkage::WeakODR => Some(weak_directive), Linkage::Internal => None, Linkage::Private => None, Linkage::Appending => emit_fatal("Only global variables can have appending linkage!"), @@ -179,7 +186,7 @@ fn prefix_and_suffix<'tcx>( let mut begin = String::new(); let mut end = String::new(); - match AsmBinaryFormat::from_target(&tcx.sess.target) { + match asm_binary_format { AsmBinaryFormat::Elf => { let section = link_section.unwrap_or(format!(".text.{asm_name}"));