Skip to content

Commit 26c08d5

Browse files
oli-obkoli-obk
committed
Treat safe target_feature functions as unsafe by default
1 parent 3ea7daf commit 26c08d5

File tree

20 files changed

+147
-59
lines changed

20 files changed

+147
-59
lines changed

‎compiler/rustc_ast_lowering/src/delegation.rs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
188188
) -> hir::FnSig<'hir> {
189189
let header = if let Some(local_sig_id) = sig_id.as_local() {
190190
match self.resolver.delegation_fn_sigs.get(&local_sig_id) {
191-
Some(sig) => self.lower_fn_header(sig.header, hir::Safety::Safe),
191+
Some(sig) => self.lower_fn_header(
192+
sig.header,
193+
// HACK: we override the default safety instead of generating attributes from the ether.
194+
// We are not forwarding the attributes, as the delegation fn sigs are collected on the ast,
195+
// and here we need the hir attributes.
196+
if sig.target_feature { hir::Safety::Unsafe } else { hir::Safety::Safe },
197+
&[],
198+
),
192199
None => self.generate_header_error(),
193200
}
194201
} else {
@@ -198,7 +205,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
198205
Asyncness::No => hir::IsAsync::NotAsync,
199206
};
200207
hir::FnHeader {
201-
safety: sig.safety.into(),
208+
safety: if self.tcx.codegen_fn_attrs(sig_id).safe_target_features {
209+
hir::HeaderSafety::SafeTargetFeatures
210+
} else {
211+
hir::HeaderSafety::Normal(sig.safety)
212+
},
202213
constness: self.tcx.constness(sig_id),
203214
asyncness,
204215
abi: sig.abi,

‎compiler/rustc_ast_lowering/src/item.rs‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
230230
});
231231
let sig = hir::FnSig {
232232
decl,
233-
header: this.lower_fn_header(*header, hir::Safety::Safe),
233+
header: this.lower_fn_header(*header, hir::Safety::Safe, attrs),
234234
span: this.lower_span(*fn_sig_span),
235235
};
236236
hir::ItemKind::Fn(sig, generics, body_id)
@@ -608,7 +608,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
608608
fn lower_foreign_item(&mut self, i: &ForeignItem) -> &'hir hir::ForeignItem<'hir> {
609609
let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
610610
let owner_id = hir_id.expect_owner();
611-
self.lower_attrs(hir_id, &i.attrs);
611+
let attrs = self.lower_attrs(hir_id, &i.attrs);
612612
let item = hir::ForeignItem {
613613
owner_id,
614614
ident: self.lower_ident(i.ident),
@@ -632,7 +632,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
632632
});
633633

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

637637
hir::ForeignItemKind::Fn(
638638
hir::FnSig { header, decl, span: self.lower_span(sig.span) },
@@ -747,7 +747,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
747747

748748
fn lower_trait_item(&mut self, i: &AssocItem) -> &'hir hir::TraitItem<'hir> {
749749
let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
750-
self.lower_attrs(hir_id, &i.attrs);
750+
let attrs = self.lower_attrs(hir_id, &i.attrs);
751751
let trait_item_def_id = hir_id.expect_owner();
752752

753753
let (generics, kind, has_default) = match &i.kind {
@@ -774,6 +774,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
774774
i.id,
775775
FnDeclKind::Trait,
776776
sig.header.coroutine_kind,
777+
attrs,
777778
);
778779
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Required(names)), false)
779780
}
@@ -792,6 +793,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
792793
i.id,
793794
FnDeclKind::Trait,
794795
sig.header.coroutine_kind,
796+
attrs,
795797
);
796798
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id)), true)
797799
}
@@ -877,7 +879,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
877879
let has_value = true;
878880
let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
879881
let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
880-
self.lower_attrs(hir_id, &i.attrs);
882+
let attrs = self.lower_attrs(hir_id, &i.attrs);
881883

882884
let (generics, kind) = match &i.kind {
883885
AssocItemKind::Const(box ConstItem { generics, ty, expr, .. }) => self.lower_generics(
@@ -907,6 +909,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
907909
i.id,
908910
if self.is_in_trait_impl { FnDeclKind::Impl } else { FnDeclKind::Inherent },
909911
sig.header.coroutine_kind,
912+
attrs,
910913
);
911914

912915
(generics, hir::ImplItemKind::Fn(sig, body_id))
@@ -1319,8 +1322,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
13191322
id: NodeId,
13201323
kind: FnDeclKind,
13211324
coroutine_kind: Option<CoroutineKind>,
1325+
attrs: &[hir::Attribute],
13221326
) -> (&'hir hir::Generics<'hir>, hir::FnSig<'hir>) {
1323-
let header = self.lower_fn_header(sig.header, hir::Safety::Safe);
1327+
let header = self.lower_fn_header(sig.header, hir::Safety::Safe, attrs);
13241328
let itctx = ImplTraitContext::Universal;
13251329
let (generics, decl) = self.lower_generics(generics, id, itctx, |this| {
13261330
this.lower_fn_decl(&sig.decl, id, sig.span, kind, coroutine_kind)
@@ -1332,6 +1336,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
13321336
&mut self,
13331337
h: FnHeader,
13341338
default_safety: hir::Safety,
1339+
attrs: &[hir::Attribute],
13351340
) -> hir::FnHeader {
13361341
let asyncness = if let Some(CoroutineKind::Async { span, .. }) = h.coroutine_kind {
13371342
hir::IsAsync::Async(span)
@@ -1340,7 +1345,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
13401345
};
13411346

13421347
let safety = self.lower_safety(h.safety, default_safety);
1343-
let safety = safety.into();
1348+
1349+
// Treat safe `#[target_feature]` functions as unsafe, but also remember that we did so.
1350+
let safety =
1351+
if attrs.iter().any(|attr| attr.has_name(sym::target_feature)) && safety.is_safe() {
1352+
hir::HeaderSafety::SafeTargetFeatures
1353+
} else {
1354+
safety.into()
1355+
};
13441356

13451357
hir::FnHeader {
13461358
safety,

‎compiler/rustc_codegen_ssa/src/codegen_attrs.rs‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,16 +249,20 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
249249
}
250250
}
251251
sym::target_feature => {
252-
if !tcx.is_closure_like(did.to_def_id())
253-
&& let Some(fn_sig) = fn_sig()
254-
&& fn_sig.skip_binder().safety().is_safe()
255-
{
252+
let Some(sig) = tcx.hir_node_by_def_id(did).fn_sig() else {
253+
tcx.dcx().span_delayed_bug(attr.span, "target_feature applied to non-fn");
254+
continue;
255+
};
256+
let safe_target_features =
257+
matches!(sig.header.safety, hir::HeaderSafety::SafeTargetFeatures);
258+
codegen_fn_attrs.safe_target_features = safe_target_features;
259+
if safe_target_features {
256260
if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
257261
// The `#[target_feature]` attribute is allowed on
258262
// WebAssembly targets on all functions, including safe
259263
// ones. Other targets require that `#[target_feature]` is
260264
// only applied to unsafe functions (pending the
261-
// `target_feature_11` feature) because on most targets
265+
// `target_feature_11` feature) because on most target.is_s
262266
// execution of instructions that are not supported is
263267
// considered undefined behavior. For WebAssembly which is a
264268
// 100% safe target at execution time it's not possible to

‎compiler/rustc_hir/src/hir.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3722,6 +3722,8 @@ impl fmt::Display for Constness {
37223722

37233723
#[derive(Copy, Clone, Debug, HashStable_Generic, PartialEq, Eq)]
37243724
pub enum HeaderSafety {
3725+
/// A safe function annotated with `#[target_features]`
3726+
SafeTargetFeatures,
37253727
Normal(Safety),
37263728
}
37273729

@@ -3758,6 +3760,7 @@ impl FnHeader {
37583760

37593761
pub fn safety(&self) -> Safety {
37603762
match self.safety {
3763+
HeaderSafety::SafeTargetFeatures => Safety::Unsafe,
37613764
HeaderSafety::Normal(safety) => safety,
37623765
}
37633766
}

‎compiler/rustc_hir_pretty/src/lib.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2402,6 +2402,10 @@ impl<'a> State<'a> {
24022402
self.print_constness(header.constness);
24032403

24042404
let safety = match header.safety {
2405+
hir::HeaderSafety::SafeTargetFeatures => {
2406+
self.word_nbsp("#[target_feature]");
2407+
hir::Safety::Safe
2408+
}
24052409
hir::HeaderSafety::Normal(safety) => safety,
24062410
};
24072411

‎compiler/rustc_hir_typeck/src/coercion.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -923,9 +923,10 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
923923
return Err(TypeError::IntrinsicCast);
924924
}
925925

926-
// Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396),
927-
// report a better error than a safety mismatch.
928-
// FIXME(target_feature): do this inside `coerce_from_safe_fn`
926+
// FIXME(target_feature): Safe `#[target_feature]` functions can be cast to safe fn pointers (RFC 2396),
927+
// as you can already write that "cast" in user code by wrapping a target_feature fn call in a closure,
928+
// which is safe. This is sound because you already need to be executing code that is satisfying the target
929+
// feature constraints.
929930

930931
if b_hdr.safety.is_safe()
931932
&& self.tcx.codegen_fn_attrs(def_id).safe_target_features

‎compiler/rustc_mir_build/src/check_unsafety.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,10 @@ pub(crate) fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
11201120
let hir_id = tcx.local_def_id_to_hir_id(def);
11211121
let safety_context = tcx.hir().fn_sig_by_hir_id(hir_id).map_or(SafetyContext::Safe, |fn_sig| {
11221122
match fn_sig.header.safety {
1123+
// We typeck the body as safe, but otherwise treat it as unsafe everywhere else.
1124+
// Call sites to other SafeTargetFeatures functions are checked explicitly and don't need
1125+
// to care about safety of the body.
1126+
hir::HeaderSafety::SafeTargetFeatures => SafetyContext::Safe,
11231127
hir::HeaderSafety::Normal(safety) => match safety {
11241128
hir::Safety::Unsafe => SafetyContext::UnsafeFn,
11251129
hir::Safety::Safe => SafetyContext::Safe,

‎src/librustdoc/clean/types.rs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -648,14 +648,25 @@ impl Item {
648648
ty::Asyncness::Yes => hir::IsAsync::Async(DUMMY_SP),
649649
ty::Asyncness::No => hir::IsAsync::NotAsync,
650650
};
651-
hir::FnHeader { safety: sig.safety().into(), abi: sig.abi(), constness, asyncness }
651+
hir::FnHeader {
652+
safety: if tcx.codegen_fn_attrs(def_id).safe_target_features {
653+
hir::HeaderSafety::SafeTargetFeatures
654+
} else {
655+
sig.safety().into()
656+
},
657+
abi: sig.abi(),
658+
constness,
659+
asyncness,
660+
}
652661
}
653662
let header = match self.kind {
654663
ItemKind::ForeignFunctionItem(_, safety) => {
655664
let def_id = self.def_id().unwrap();
656665
let abi = tcx.fn_sig(def_id).skip_binder().abi();
657666
hir::FnHeader {
658-
safety: if abi == ExternAbi::RustIntrinsic {
667+
safety: if tcx.codegen_fn_attrs(def_id).safe_target_features {
668+
hir::HeaderSafety::SafeTargetFeatures
669+
} else if abi == ExternAbi::RustIntrinsic {
659670
intrinsic_operation_unsafety(tcx, def_id.expect_local()).into()
660671
} else {
661672
safety.into()

‎src/librustdoc/html/format.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1633,6 +1633,7 @@ impl PrintWithSpace for hir::Safety {
16331633
impl PrintWithSpace for hir::HeaderSafety {
16341634
fn print_with_space(&self) -> &str {
16351635
match self {
1636+
hir::HeaderSafety::SafeTargetFeatures => "",
16361637
hir::HeaderSafety::Normal(safety) => safety.print_with_space(),
16371638
}
16381639
}

‎tests/ui/async-await/async-closures/fn-exception-target-features.stderr‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error[E0277]: the trait bound `fn() -> Pin<Box<(dyn Future<Output = ()> + 'static)>> {target_feature}: AsyncFn()` is not satisfied
1+
error[E0277]: the trait bound `unsafe fn() -> Pin<Box<(dyn Future<Output = ()> + 'static)>> {target_feature}: AsyncFn()` is not satisfied
22
--> $DIR/fn-exception-target-features.rs:16:10
33
|
44
LL | test(target_feature);
5-
| ---- ^^^^^^^^^^^^^^ the trait `AsyncFn()` is not implemented for fn item `fn() -> Pin<Box<(dyn Future<Output = ()> + 'static)>> {target_feature}`
5+
| ---- ^^^^^^^^^^^^^^ the trait `AsyncFn()` is not implemented for fn item `unsafe fn() -> Pin<Box<(dyn Future<Output = ()> + 'static)>> {target_feature}`
66
| |
77
| required by a bound introduced by this call
88
|

0 commit comments

Comments
 (0)