Skip to content

fix -Zsanitizer=kcfi on #[naked] functions #143293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_cranelift/src/driver/aot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,8 @@ fn codegen_cgu_content(
for (mono_item, item_data) in mono_items {
match mono_item {
MonoItem::Fn(instance) => {
if tcx.codegen_fn_attrs(instance.def_id()).flags.contains(CodegenFnAttrFlags::NAKED)
{
let flags = tcx.codegen_instance_attrs(instance.def).flags;
if flags.contains(CodegenFnAttrFlags::NAKED) {
rustc_codegen_ssa::mir::naked_asm::codegen_naked_asm(
&mut GlobalAsmContext { tcx, global_asm: &mut cx.global_asm },
instance,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/driver/jit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ fn codegen_and_compile_fn<'tcx>(
module: &mut dyn Module,
instance: Instance<'tcx>,
) {
if tcx.codegen_fn_attrs(instance.def_id()).flags.contains(CodegenFnAttrFlags::NAKED) {
if tcx.codegen_instance_attrs(instance.def).flags.contains(CodegenFnAttrFlags::NAKED) {
tcx.dcx()
.span_fatal(tcx.def_span(instance.def_id()), "Naked asm is not supported in JIT mode");
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/driver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn predefine_mono_items<'tcx>(
is_compiler_builtins,
);
let is_naked = tcx
.codegen_fn_attrs(instance.def_id())
.codegen_instance_attrs(instance.def)
.flags
.contains(CodegenFnAttrFlags::NAKED);
module
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub fn from_fn_attrs<'gcc, 'tcx>(
#[cfg_attr(not(feature = "master"), allow(unused_variables))] func: Function<'gcc>,
instance: ty::Instance<'tcx>,
) {
let codegen_fn_attrs = cx.tcx.codegen_fn_attrs(instance.def_id());
let codegen_fn_attrs = cx.tcx.codegen_instance_attrs(instance.def);

#[cfg(feature = "master")]
{
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub fn get_fn<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, instance: Instance<'tcx>)
let is_hidden = if is_generic {
// This is a monomorphization of a generic function.
if !(cx.tcx.sess.opts.share_generics()
|| tcx.codegen_fn_attrs(instance_def_id).inline
|| tcx.codegen_instance_attrs(instance.def).inline
== rustc_attr_data_structures::InlineAttr::Never)
{
// When not sharing generics, all instances are in the same
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/mono_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl<'gcc, 'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty());
self.linkage.set(base::linkage_to_gcc(linkage));
let decl = self.declare_fn(symbol_name, fn_abi);
//let attrs = self.tcx.codegen_fn_attrs(instance.def_id());
//let attrs = self.tcx.codegen_instance_attrs(instance.def);

attributes::from_fn_attrs(self, decl, instance);

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ pub(crate) fn llfn_attrs_from_instance<'ll, 'tcx>(
llfn: &'ll Value,
instance: ty::Instance<'tcx>,
) {
let codegen_fn_attrs = cx.tcx.codegen_fn_attrs(instance.def_id());
let codegen_fn_attrs = cx.tcx.codegen_instance_attrs(instance.def);

let mut to_add = SmallVec::<[_; 16]>::new();

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub(crate) fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'t
let is_hidden = if is_generic {
// This is a monomorphization of a generic function.
if !(cx.tcx.sess.opts.share_generics()
|| tcx.codegen_fn_attrs(instance_def_id).inline
|| tcx.codegen_instance_attrs(instance.def).inline
== rustc_attr_data_structures::InlineAttr::Never)
{
// When not sharing generics, all instances are in the same
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/mono_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,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));
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);
let attrs = self.tcx.codegen_instance_attrs(instance.def);
base::set_link_section(lldecl, &attrs);
if (linkage == Linkage::LinkOnceODR || linkage == Linkage::WeakODR)
&& self.tcx.sess.target.supports_comdat()
{
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
codegen_fn_attrs.export_name = Some(*name);
}
AttributeKind::Naked(_) => codegen_fn_attrs.flags |= CodegenFnAttrFlags::NAKED,

AttributeKind::Align { align, .. } => codegen_fn_attrs.alignment = Some(*align),
AttributeKind::LinkName { name, .. } => codegen_fn_attrs.link_name = Some(*name),
AttributeKind::LinkSection { name, .. } => {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
LocalRef::Operand(operand) => {
// Don't spill operands onto the stack in naked functions.
// See: https://github.com/rust-lang/rust/issues/42779
let attrs = bx.tcx().codegen_fn_attrs(self.instance.def_id());
let attrs = bx.tcx().codegen_instance_attrs(self.instance.def);
if attrs.flags.contains(CodegenFnAttrFlags::NAKED) {
return;
}
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,9 +385,8 @@ fn arg_local_refs<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

let mut num_untupled = None;

let codegen_fn_attrs = bx.tcx().codegen_fn_attrs(fx.instance.def_id());
let naked = codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED);
if naked {
let codegen_fn_attrs = bx.tcx().codegen_instance_attrs(fx.instance.def);
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) {
return vec![];
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/naked_asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn prefix_and_suffix<'tcx>(
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 attrs = tcx.codegen_instance_attrs(instance.def);
let link_section = attrs.link_section.map(|symbol| symbol.as_str().to_string());

// If no alignment is specified, an alignment of 4 bytes is used.
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -688,11 +688,19 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let fn_abi = bx.fn_abi_of_instance(instance, ty::List::empty());
let fn_ty = bx.fn_decl_backend_type(fn_abi);
let fn_attrs = if bx.tcx().def_kind(instance.def_id()).has_codegen_attrs() {
Some(bx.tcx().codegen_fn_attrs(instance.def_id()))
Some(bx.tcx().codegen_instance_attrs(instance.def))
} else {
None
};
bx.call(fn_ty, fn_attrs, Some(fn_abi), fn_ptr, &[], None, Some(instance))
bx.call(
fn_ty,
fn_attrs.as_deref(),
Some(fn_abi),
fn_ptr,
&[],
None,
Some(instance),
)
} else {
bx.get_static(def_id)
};
Expand Down
10 changes: 3 additions & 7 deletions compiler/rustc_codegen_ssa/src/mono_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,8 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> {
base::codegen_global_asm(cx, item_id);
}
MonoItem::Fn(instance) => {
if cx
.tcx()
.codegen_fn_attrs(instance.def_id())
.flags
.contains(CodegenFnAttrFlags::NAKED)
{
let flags = cx.tcx().codegen_instance_attrs(instance.def).flags;
if flags.contains(CodegenFnAttrFlags::NAKED) {
naked_asm::codegen_naked_asm::<Bx::CodegenCx>(cx, instance, item_data);
} else {
base::codegen_instance::<Bx>(cx, instance);
Expand Down Expand Up @@ -75,7 +71,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> {
cx.predefine_static(def_id, linkage, visibility, symbol_name);
}
MonoItem::Fn(instance) => {
let attrs = cx.tcx().codegen_fn_attrs(instance.def_id());
let attrs = cx.tcx().codegen_instance_attrs(instance.def);

if attrs.flags.contains(CodegenFnAttrFlags::NAKED) {
// do not define this function; it will become a global assembly block
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
if let Some(fn_val) = self.get_fn_alloc(id) {
let align = match fn_val {
FnVal::Instance(instance) => {
self.tcx.codegen_fn_attrs(instance.def_id()).alignment.unwrap_or(Align::ONE)
self.tcx.codegen_instance_attrs(instance.def).alignment.unwrap_or(Align::ONE)
}
// Machine-specific extra functions currently do not support alignment restrictions.
FnVal::Other(_) => Align::ONE,
Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;

use rustc_abi::Align;
use rustc_ast::expand::autodiff_attrs::AutoDiffAttrs;
use rustc_attr_data_structures::{InlineAttr, InstructionSetAttr, OptimizeAttr};
Expand All @@ -6,6 +8,24 @@ use rustc_span::Symbol;
use rustc_target::spec::SanitizerSet;

use crate::mir::mono::Linkage;
use crate::ty::{InstanceKind, TyCtxt};

impl<'tcx> TyCtxt<'tcx> {
pub fn codegen_instance_attrs(
self,
instance_kind: InstanceKind<'_>,
) -> Cow<'tcx, CodegenFnAttrs> {
let mut attrs = Cow::Borrowed(self.codegen_fn_attrs(instance_kind.def_id()));

// Drop the `#[naked]` attribute on non-item `InstanceKind`s, like the shims that
// are generated for indirect function calls.
if !matches!(instance_kind, InstanceKind::Item(_)) {
attrs.to_mut().flags.remove(CodegenFnAttrFlags::NAKED);
}

attrs
}
}

#[derive(Clone, TyEncodable, TyDecodable, HashStable, Debug)]
pub struct CodegenFnAttrs {
Expand Down
13 changes: 6 additions & 7 deletions compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ impl<'tcx> MonoItem<'tcx> {
// If the function is #[naked] or contains any other attribute that requires exactly-once
// instantiation:
// We emit an unused_attributes lint for this case, which should be kept in sync if possible.
let codegen_fn_attrs = tcx.codegen_fn_attrs(instance.def_id());
let codegen_fn_attrs = tcx.codegen_instance_attrs(instance.def);
if codegen_fn_attrs.contains_extern_indicator()
|| codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED)
{
Expand Down Expand Up @@ -219,7 +219,7 @@ impl<'tcx> MonoItem<'tcx> {
// functions the same as those that unconditionally get LocalCopy codegen. It's only when
// we get here that we can at least not codegen a #[inline(never)] generic function in all
// of our CGUs.
if let InlineAttr::Never = tcx.codegen_fn_attrs(instance.def_id()).inline
if let InlineAttr::Never = codegen_fn_attrs.inline
&& self.is_generic_fn()
{
return InstantiationMode::GloballyShared { may_conflict: true };
Expand All @@ -234,14 +234,13 @@ impl<'tcx> MonoItem<'tcx> {
}

pub fn explicit_linkage(&self, tcx: TyCtxt<'tcx>) -> Option<Linkage> {
let def_id = match *self {
MonoItem::Fn(ref instance) => instance.def_id(),
MonoItem::Static(def_id) => def_id,
let instance_kind = match *self {
MonoItem::Fn(ref instance) => instance.def,
MonoItem::Static(def_id) => InstanceKind::Item(def_id),
MonoItem::GlobalAsm(..) => return None,
};

let codegen_fn_attrs = tcx.codegen_fn_attrs(def_id);
codegen_fn_attrs.linkage
tcx.codegen_instance_attrs(instance_kind).linkage
}

/// Returns `true` if this instance is instantiable - whether it has no unsatisfied
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_symbol_mangling/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ fn compute_symbol_name<'tcx>(

// FIXME(eddyb) Precompute a custom symbol name based on attributes.
let attrs = if tcx.def_kind(def_id).has_codegen_attrs() {
tcx.codegen_fn_attrs(def_id)
&tcx.codegen_instance_attrs(instance.def)
} else {
CodegenFnAttrs::EMPTY
};
Expand Down
4 changes: 2 additions & 2 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
ecx: &MiriInterpCx<'tcx>,
instance: ty::Instance<'tcx>,
) -> InterpResult<'tcx> {
let attrs = ecx.tcx.codegen_fn_attrs(instance.def_id());
let attrs = ecx.tcx.codegen_instance_attrs(instance.def);
if attrs
.target_features
.iter()
Expand Down Expand Up @@ -1795,7 +1795,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
ecx.tcx.sess.opts.unstable_opts.cross_crate_inline_threshold,
InliningThreshold::Always
) || !matches!(
ecx.tcx.codegen_fn_attrs(instance.def_id()).inline,
ecx.tcx.codegen_instance_attrs(instance.def).inline,
InlineAttr::Never
);
!is_generic && !can_be_inlined
Expand Down
47 changes: 47 additions & 0 deletions tests/codegen/sanitizer/kcfi/naked-function.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//@ add-core-stubs
//@ revisions: aarch64 x86_64
//@ [aarch64] compile-flags: --target aarch64-unknown-none
//@ [aarch64] needs-llvm-components: aarch64
//@ [x86_64] compile-flags: --target x86_64-unknown-none
//@ [x86_64] needs-llvm-components: x86
//@ compile-flags: -Ctarget-feature=-crt-static -Zsanitizer=kcfi -Cno-prepopulate-passes -Copt-level=0

#![feature(no_core, lang_items)]
#![crate_type = "lib"]
#![no_core]

extern crate minicore;
use minicore::*;

struct Thing;
trait MyTrait {
#[unsafe(naked)]
extern "C" fn my_naked_function() {
// the real function is defined
// CHECK: .globl
// CHECK-SAME: my_naked_function
naked_asm!("ret")
}
}
impl MyTrait for Thing {}

// the shim calls the real function
// CHECK-LABEL: define
// CHECK-SAME: my_naked_function
// CHECK-SAME: reify.shim.fnptr

// CHECK-LABEL: main
#[unsafe(no_mangle)]
pub fn main() {
// Trick the compiler into generating an indirect call.
const F: extern "C" fn() = Thing::my_naked_function;

// main calls the shim function
// CHECK: call void
// CHECK-SAME: my_naked_function
// CHECK-SAME: reify.shim.fnptr
(F)();
}

// CHECK: declare !kcfi_type
// CHECK-SAME: my_naked_function
31 changes: 31 additions & 0 deletions tests/ui/asm/naked-function-shim.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// The indirect call will generate a shim that then calls the actual function. Test that
// this is handled correctly. See also https://github.com/rust-lang/rust/issues/143266.

//@ build-pass
//@ add-core-stubs
//@ revisions: aarch64 x86_64
//@ [aarch64] compile-flags: --target aarch64-unknown-none
//@ [aarch64] needs-llvm-components: aarch64
//@ [x86_64] compile-flags: --target x86_64-unknown-none
//@ [x86_64] needs-llvm-components: x86

#![feature(no_core, lang_items)]
#![crate_type = "lib"]
#![no_core]

extern crate minicore;
use minicore::*;

trait MyTrait {
#[unsafe(naked)]
extern "C" fn foo(&self) {
naked_asm!("ret")
}
}

impl MyTrait for i32 {}

fn main() {
let x: extern "C" fn(&_) = <dyn MyTrait as MyTrait>::foo;
x(&1);
}
Loading