Skip to content
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

Allow coercing safe-to-call target_feature functions to safe fn pointers #135504

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
17 changes: 14 additions & 3 deletions compiler/rustc_ast_lowering/src/delegation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
) -> hir::FnSig<'hir> {
let header = if let Some(local_sig_id) = sig_id.as_local() {
match self.resolver.delegation_fn_sigs.get(&local_sig_id) {
Some(sig) => self.lower_fn_header(sig.header, hir::Safety::Safe),
Some(sig) => self.lower_fn_header(
sig.header,
// HACK: we override the default safety instead of generating attributes from the ether.
// We are not forwarding the attributes, as the delegation fn sigs are collected on the ast,
// and here we need the hir attributes.
if sig.target_feature { hir::Safety::Unsafe } else { hir::Safety::Safe },
&[],
),
None => self.generate_header_error(),
}
} else {
Expand All @@ -198,7 +205,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
Asyncness::No => hir::IsAsync::NotAsync,
};
hir::FnHeader {
safety: sig.safety,
safety: if self.tcx.codegen_fn_attrs(sig_id).safe_target_features {
hir::HeaderSafety::SafeTargetFeatures
} else {
hir::HeaderSafety::Normal(sig.safety)
},
constness: self.tcx.constness(sig_id),
asyncness,
abi: sig.abi,
Expand Down Expand Up @@ -384,7 +395,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

fn generate_header_error(&self) -> hir::FnHeader {
hir::FnHeader {
safety: hir::Safety::Safe,
safety: hir::Safety::Safe.into(),
constness: hir::Constness::NotConst,
asyncness: hir::IsAsync::NotAsync,
abi: abi::Abi::Rust,
Expand Down
26 changes: 21 additions & 5 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
});
let sig = hir::FnSig {
decl,
header: this.lower_fn_header(*header, hir::Safety::Safe),
header: this.lower_fn_header(*header, hir::Safety::Safe, attrs),
span: this.lower_span(*fn_sig_span),
};
hir::ItemKind::Fn { sig, generics, body: body_id, has_body: body.is_some() }
Expand Down Expand Up @@ -610,7 +610,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
fn lower_foreign_item(&mut self, i: &ForeignItem) -> &'hir hir::ForeignItem<'hir> {
let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
let owner_id = hir_id.expect_owner();
self.lower_attrs(hir_id, &i.attrs);
let attrs = self.lower_attrs(hir_id, &i.attrs);
let item = hir::ForeignItem {
owner_id,
ident: self.lower_ident(i.ident),
Expand All @@ -634,7 +634,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
});

// Unmarked safety in unsafe block defaults to unsafe.
let header = self.lower_fn_header(sig.header, hir::Safety::Unsafe);
let header = self.lower_fn_header(sig.header, hir::Safety::Unsafe, attrs);

hir::ForeignItemKind::Fn(
hir::FnSig { header, decl, span: self.lower_span(sig.span) },
Expand Down Expand Up @@ -776,6 +776,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
i.id,
FnDeclKind::Trait,
sig.header.coroutine_kind,
attrs,
);
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Required(names)), false)
}
Expand All @@ -795,6 +796,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
i.id,
FnDeclKind::Trait,
sig.header.coroutine_kind,
attrs,
);
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id)), true)
}
Expand Down Expand Up @@ -911,6 +913,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
i.id,
if self.is_in_trait_impl { FnDeclKind::Impl } else { FnDeclKind::Inherent },
sig.header.coroutine_kind,
attrs,
);

(generics, hir::ImplItemKind::Fn(sig, body_id))
Expand Down Expand Up @@ -1339,8 +1342,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
id: NodeId,
kind: FnDeclKind,
coroutine_kind: Option<CoroutineKind>,
attrs: &[hir::Attribute],
) -> (&'hir hir::Generics<'hir>, hir::FnSig<'hir>) {
let header = self.lower_fn_header(sig.header, hir::Safety::Safe);
let header = self.lower_fn_header(sig.header, hir::Safety::Safe, attrs);
let itctx = ImplTraitContext::Universal;
let (generics, decl) = self.lower_generics(generics, id, itctx, |this| {
this.lower_fn_decl(&sig.decl, id, sig.span, kind, coroutine_kind)
Expand All @@ -1352,14 +1356,26 @@ impl<'hir> LoweringContext<'_, 'hir> {
&mut self,
h: FnHeader,
default_safety: hir::Safety,
attrs: &[hir::Attribute],
) -> hir::FnHeader {
let asyncness = if let Some(CoroutineKind::Async { span, .. }) = h.coroutine_kind {
hir::IsAsync::Async(span)
} else {
hir::IsAsync::NotAsync
};

let safety = self.lower_safety(h.safety, default_safety);

// Treat safe `#[target_feature]` functions as unsafe, but also remember that we did so.
let safety =
if attrs.iter().any(|attr| attr.has_name(sym::target_feature)) && safety.is_safe() {
hir::HeaderSafety::SafeTargetFeatures
} else {
safety.into()
};

hir::FnHeader {
safety: self.lower_safety(h.safety, default_safety),
safety,
asyncness,
constness: self.lower_constness(h.constness),
abi: self.lower_extern(h.ext),
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,14 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
}
}
sym::target_feature => {
if !tcx.is_closure_like(did.to_def_id())
&& let Some(fn_sig) = fn_sig()
&& fn_sig.skip_binder().safety().is_safe()
{
let Some(sig) = tcx.hir_node_by_def_id(did).fn_sig() else {
tcx.dcx().span_delayed_bug(attr.span, "target_feature applied to non-fn");
continue;
};
let safe_target_features =
matches!(sig.header.safety, hir::HeaderSafety::SafeTargetFeatures);
codegen_fn_attrs.safe_target_features = safe_target_features;
if safe_target_features {
if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
// The `#[target_feature]` attribute is allowed on
// WebAssembly targets on all functions, including safe
Expand Down
36 changes: 34 additions & 2 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3762,9 +3762,30 @@ impl fmt::Display for Constness {
}
}

/// The actualy safety specified in syntax. We may treat
/// its safety different within the type system to create a
/// "sound by default" system that needs checking this enum
/// explicitly to allow unsafe operations.
#[derive(Copy, Clone, Debug, HashStable_Generic, PartialEq, Eq)]
pub enum HeaderSafety {
/// A safe function annotated with `#[target_features]`.
/// The type system treats this function as an unsafe function,
/// but safety checking will check this enum to treat it as safe
/// and allowing calling other safe target feature functions with
/// the same features without requiring an additional unsafe block.
SafeTargetFeatures,
Normal(Safety),
}

impl From<Safety> for HeaderSafety {
fn from(v: Safety) -> Self {
Self::Normal(v)
}
}

#[derive(Copy, Clone, Debug, HashStable_Generic)]
pub struct FnHeader {
pub safety: Safety,
pub safety: HeaderSafety,
pub constness: Constness,
pub asyncness: IsAsync,
pub abi: ExternAbi,
Expand All @@ -3780,7 +3801,18 @@ impl FnHeader {
}

pub fn is_unsafe(&self) -> bool {
self.safety.is_unsafe()
self.safety().is_unsafe()
}

pub fn is_safe(&self) -> bool {
self.safety().is_safe()
}

pub fn safety(&self) -> Safety {
match self.safety {
HeaderSafety::SafeTargetFeatures => Safety::Unsafe,
HeaderSafety::Normal(safety) => safety,
}
}
}

Expand Down
17 changes: 11 additions & 6 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1336,7 +1336,7 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_, ty::PolyFn
{
icx.lowerer().lower_fn_ty(
hir_id,
sig.header.safety,
sig.header.safety(),
sig.header.abi,
sig.decl,
Some(generics),
Expand All @@ -1351,13 +1351,18 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_, ty::PolyFn
kind: TraitItemKind::Fn(FnSig { header, decl, span: _ }, _),
generics,
..
}) => {
icx.lowerer().lower_fn_ty(hir_id, header.safety, header.abi, decl, Some(generics), None)
}
}) => icx.lowerer().lower_fn_ty(
hir_id,
header.safety(),
header.abi,
decl,
Some(generics),
None,
),

ForeignItem(&hir::ForeignItem { kind: ForeignItemKind::Fn(sig, _, _), .. }) => {
let abi = tcx.hir().get_foreign_abi(hir_id);
compute_sig_of_foreign_fn_decl(tcx, def_id, sig.decl, abi, sig.header.safety)
compute_sig_of_foreign_fn_decl(tcx, def_id, sig.decl, abi, sig.header.safety())
}

Ctor(data) | Variant(hir::Variant { data, .. }) if data.ctor().is_some() => {
Expand Down Expand Up @@ -1405,7 +1410,7 @@ fn lower_fn_sig_recovering_infer_ret_ty<'tcx>(

icx.lowerer().lower_fn_ty(
icx.tcx().local_def_id_to_hir_id(def_id),
sig.header.safety,
sig.header.safety(),
sig.header.abi,
sig.decl,
Some(generics),
Expand Down
12 changes: 10 additions & 2 deletions compiler/rustc_hir_pretty/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2407,7 +2407,7 @@ impl<'a> State<'a> {
self.print_fn(
decl,
hir::FnHeader {
safety,
safety: safety.into(),
abi,
constness: hir::Constness::NotConst,
asyncness: hir::IsAsync::NotAsync,
Expand All @@ -2423,12 +2423,20 @@ impl<'a> State<'a> {
fn print_fn_header_info(&mut self, header: hir::FnHeader) {
self.print_constness(header.constness);

let safety = match header.safety {
hir::HeaderSafety::SafeTargetFeatures => {
self.word_nbsp("#[target_feature]");
hir::Safety::Safe
}
hir::HeaderSafety::Normal(safety) => safety,
};

match header.asyncness {
hir::IsAsync::NotAsync => {}
hir::IsAsync::Async(_) => self.word_nbsp("async"),
}

self.print_safety(header.safety);
self.print_safety(safety);

if header.abi != ExternAbi::Rust {
self.word_nbsp("extern");
Expand Down
28 changes: 24 additions & 4 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
match b.kind() {
ty::FnPtr(_, b_hdr) => {
let a_sig = a.fn_sig(self.tcx);
let mut allow_unsafe_to_safe_coercion = false;
if let ty::FnDef(def_id, _) = *a.kind() {
// Intrinsics are not coercible to function pointers
if self.tcx.intrinsic(def_id).is_some() {
Expand All @@ -932,19 +933,38 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
return Err(TypeError::ForceInlineCast);
}

// Safe `#[target_feature]` functions are not assignable to safe fn pointers
// (RFC 2396).
if b_hdr.safety.is_safe()
&& !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty()
&& self.tcx.codegen_fn_attrs(def_id).safe_target_features
{
return Err(TypeError::TargetFeatureCast(def_id));
// Allow the coercion if the current function has all the features that would be
// needed to call the coercee safely.
let coercee_features = &self.tcx.codegen_fn_attrs(def_id).target_features;
let body_features =
&self.tcx.codegen_fn_attrs(self.fcx.body_id).target_features;
if !self.tcx.sess.target.options.is_like_wasm
&& !coercee_features
.iter()
.all(|feature| body_features.iter().any(|f| f.name == feature.name))
{
return Err(TypeError::TargetFeatureCast(def_id));
} else {
allow_unsafe_to_safe_coercion = true;
}
}
}

let InferOk { value: a_sig, obligations: o1 } =
self.at(&self.cause, self.param_env).normalize(a_sig);
obligations.extend(o1);

let a_sig = if allow_unsafe_to_safe_coercion {
// The coercee behaves like a safe function, since it is a target_feature
// function that would be callable safely in this context.
a_sig.map_bound(|sig| ty::FnSig { safety: hir::Safety::Safe, ..sig })
} else {
a_sig
};

let a_fn_pointer = Ty::new_fn_ptr(self.tcx, a_sig);
let InferOk { value, obligations: o2 } = self.coerce_from_safe_fn(
a_fn_pointer,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ fn typeck_with_fallback<'tcx>(
// type that has an infer in it, lower the type directly so that it'll
// be correctly filled with infer. We'll use this inference to provide
// a suggestion later on.
fcx.lowerer().lower_fn_ty(id, header.safety, header.abi, decl, None, None)
fcx.lowerer().lower_fn_ty(id, header.safety(), header.abi, decl, None, None)
} else {
tcx.fn_sig(def_id).instantiate_identity()
};
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ pub struct CodegenFnAttrs {
/// features (only enabled features are supported right now).
/// Implied target features have already been applied.
pub target_features: Vec<TargetFeature>,
/// Whether the function was declared safe, but has target features
pub safe_target_features: bool,
/// The `#[linkage = "..."]` attribute on Rust-defined items and the value we found.
pub linkage: Option<Linkage>,
/// The `#[linkage = "..."]` attribute on foreign items and the value we found.
Expand Down Expand Up @@ -150,6 +152,7 @@ impl CodegenFnAttrs {
link_name: None,
link_ordinal: None,
target_features: vec![],
safe_target_features: false,
linkage: None,
import_linkage: None,
link_section: None,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ pub struct DelegationFnSig {
pub param_count: usize,
pub has_self: bool,
pub c_variadic: bool,
pub target_feature: bool,
}

#[derive(Clone, Copy, Debug)]
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,14 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
if with_reduced_queries() {
p!(print_def_path(def_id, args));
} else {
let sig = self.tcx().fn_sig(def_id).instantiate(self.tcx(), args);
let mut sig = self.tcx().fn_sig(def_id).instantiate(self.tcx(), args);
if self.tcx().codegen_fn_attrs(def_id).safe_target_features {
p!("#[target_features] ");
sig = sig.map_bound(|mut sig| {
sig.safety = hir::Safety::Safe;
sig
});
}
p!(print(sig), " {{", print_value_path(def_id, args), "}}");
}
}
Expand Down
Loading
Loading