1
Fork 0

Treat safe target_feature functions as unsafe by default

This commit is contained in:
Oli Scherer 2024-12-13 12:19:46 +00:00
parent a907c56a77
commit 56178ddc90
20 changed files with 159 additions and 56 deletions

View file

@ -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 {
@ -198,7 +205,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
Asyncness::No => hir::IsAsync::NotAsync,
};
hir::FnHeader {
safety: sig.safety.into(),
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,

View file

@ -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() }
@ -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),
@ -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) },
@ -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)
}
@ -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)
}
@ -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))
@ -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)
@ -1352,6 +1356,7 @@ 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)
@ -1360,7 +1365,16 @@ impl<'hir> LoweringContext<'_, 'hir> {
};
let safety = self.lower_safety(h.safety, default_safety);
let safety = safety.into();
// 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()
&& !self.tcx.sess.target.is_like_wasm
{
hir::HeaderSafety::SafeTargetFeatures
} else {
safety.into()
};
hir::FnHeader {
safety,

View file

@ -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

View file

@ -3762,8 +3762,18 @@ 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),
}
@ -3800,6 +3810,7 @@ impl FnHeader {
pub fn safety(&self) -> Safety {
match self.safety {
HeaderSafety::SafeTargetFeatures => Safety::Unsafe,
HeaderSafety::Normal(safety) => safety,
}
}

View file

@ -2424,6 +2424,10 @@ impl<'a> State<'a> {
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,
};

View file

@ -932,9 +932,15 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
return Err(TypeError::ForceInlineCast);
}
// Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396),
// report a better error than a safety mismatch.
// FIXME(target_feature): do this inside `coerce_from_safe_fn`.
let fn_attrs = self.tcx.codegen_fn_attrs(def_id);
if matches!(fn_attrs.inline, InlineAttr::Force { .. }) {
return Err(TypeError::ForceInlineCast);
}
// FIXME(target_feature): Safe `#[target_feature]` functions could be cast to safe fn pointers (RFC 2396),
// as you can already write that "cast" in user code by wrapping a target_feature fn call in a closure,
// which is safe. This is sound because you already need to be executing code that is satisfying the target
// feature constraints..
if b_hdr.safety.is_safe()
&& self.tcx.codegen_fn_attrs(def_id).safe_target_features
{

View file

@ -1120,6 +1120,10 @@ pub(crate) fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
let hir_id = tcx.local_def_id_to_hir_id(def);
let safety_context = tcx.hir().fn_sig_by_hir_id(hir_id).map_or(SafetyContext::Safe, |fn_sig| {
match fn_sig.header.safety {
// We typeck the body as safe, but otherwise treat it as unsafe everywhere else.
// Call sites to other SafeTargetFeatures functions are checked explicitly and don't need
// to care about safety of the body.
hir::HeaderSafety::SafeTargetFeatures => SafetyContext::Safe,
hir::HeaderSafety::Normal(safety) => match safety {
hir::Safety::Unsafe => SafetyContext::UnsafeFn,
hir::Safety::Safe => SafetyContext::Safe,