Skip to content

Commit 2800108

Browse files
oli-obkoli-obk
committed
Treat safe target_feature functions as unsafe by default
1 parent 8a4e5d7 commit 2800108

File tree

33 files changed

+197
-87
lines changed

33 files changed

+197
-87
lines changed

Diff for: ‎compiler/rustc_ast_lowering/src/delegation.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,15 @@ impl<'hir> LoweringContext<'_, 'hir> {
189189
) -> hir::FnSig<'hir> {
190190
let header = if let Some(local_sig_id) = sig_id.as_local() {
191191
match self.resolver.delegation_fn_sigs.get(&local_sig_id) {
192-
Some(sig) => self.lower_fn_header(sig.header, hir::Safety::Safe),
192+
Some(sig) => self.lower_fn_header(
193+
sig.header,
194+
if sig.target_feature && !matches!(sig.header.safety, Safety::Unsafe(_)) {
195+
hir::Safety::Unsafe { target_feature: true }
196+
} else {
197+
hir::Safety::Safe
198+
},
199+
&[],
200+
),
193201
None => self.generate_header_error(),
194202
}
195203
} else {

Diff for: ‎compiler/rustc_ast_lowering/src/item.rs

+28-8
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
231231
});
232232
let sig = hir::FnSig {
233233
decl,
234-
header: this.lower_fn_header(*header, hir::Safety::Safe),
234+
header: this.lower_fn_header(*header, hir::Safety::Safe, attrs),
235235
span: this.lower_span(*fn_sig_span),
236236
};
237237
hir::ItemKind::Fn(sig, generics, body_id)
@@ -609,7 +609,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
609609
fn lower_foreign_item(&mut self, i: &ForeignItem) -> &'hir hir::ForeignItem<'hir> {
610610
let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
611611
let owner_id = hir_id.expect_owner();
612-
self.lower_attrs(hir_id, &i.attrs);
612+
let attrs = self.lower_attrs(hir_id, &i.attrs);
613613
let item = hir::ForeignItem {
614614
owner_id,
615615
ident: self.lower_ident(i.ident),
@@ -633,7 +633,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
633633
});
634634

635635
// Unmarked safety in unsafe block defaults to unsafe.
636-
let header = self.lower_fn_header(sig.header, hir::Safety::Unsafe);
636+
let header = self.lower_fn_header(
637+
sig.header,
638+
hir::Safety::Unsafe { target_feature: false },
639+
attrs,
640+
);
637641

638642
hir::ForeignItemKind::Fn(
639643
hir::FnSig { header, decl, span: self.lower_span(sig.span) },
@@ -644,7 +648,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
644648
ForeignItemKind::Static(box StaticItem { ty, mutability, expr: _, safety }) => {
645649
let ty = self
646650
.lower_ty(ty, ImplTraitContext::Disallowed(ImplTraitPosition::StaticTy));
647-
let safety = self.lower_safety(*safety, hir::Safety::Unsafe);
651+
let safety =
652+
self.lower_safety(*safety, hir::Safety::Unsafe { target_feature: false });
648653

649654
hir::ForeignItemKind::Static(ty, *mutability, safety)
650655
}
@@ -748,7 +753,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
748753

749754
fn lower_trait_item(&mut self, i: &AssocItem) -> &'hir hir::TraitItem<'hir> {
750755
let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
751-
self.lower_attrs(hir_id, &i.attrs);
756+
let attrs = self.lower_attrs(hir_id, &i.attrs);
752757
let trait_item_def_id = hir_id.expect_owner();
753758

754759
let (generics, kind, has_default) = match &i.kind {
@@ -775,6 +780,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
775780
i.id,
776781
FnDeclKind::Trait,
777782
sig.header.coroutine_kind,
783+
attrs,
778784
);
779785
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Required(names)), false)
780786
}
@@ -793,6 +799,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
793799
i.id,
794800
FnDeclKind::Trait,
795801
sig.header.coroutine_kind,
802+
attrs,
796803
);
797804
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id)), true)
798805
}
@@ -878,7 +885,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
878885
let has_value = true;
879886
let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
880887
let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
881-
self.lower_attrs(hir_id, &i.attrs);
888+
let attrs = self.lower_attrs(hir_id, &i.attrs);
882889

883890
let (generics, kind) = match &i.kind {
884891
AssocItemKind::Const(box ConstItem { generics, ty, expr, .. }) => self.lower_generics(
@@ -908,6 +915,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
908915
i.id,
909916
if self.is_in_trait_impl { FnDeclKind::Impl } else { FnDeclKind::Inherent },
910917
sig.header.coroutine_kind,
918+
attrs,
911919
);
912920

913921
(generics, hir::ImplItemKind::Fn(sig, body_id))
@@ -1322,8 +1330,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
13221330
id: NodeId,
13231331
kind: FnDeclKind,
13241332
coroutine_kind: Option<CoroutineKind>,
1333+
attrs: &[Attribute],
13251334
) -> (&'hir hir::Generics<'hir>, hir::FnSig<'hir>) {
1326-
let header = self.lower_fn_header(sig.header, hir::Safety::Safe);
1335+
let header = self.lower_fn_header(sig.header, hir::Safety::Safe, attrs);
13271336
let itctx = ImplTraitContext::Universal;
13281337
let (generics, decl) = self.lower_generics(generics, id, itctx, |this| {
13291338
this.lower_fn_decl(&sig.decl, id, sig.span, kind, coroutine_kind)
@@ -1335,12 +1344,23 @@ impl<'hir> LoweringContext<'_, 'hir> {
13351344
&mut self,
13361345
h: FnHeader,
13371346
default_safety: hir::Safety,
1347+
attrs: &[Attribute],
13381348
) -> hir::FnHeader {
13391349
let asyncness = if let Some(CoroutineKind::Async { span, .. }) = h.coroutine_kind {
13401350
hir::IsAsync::Async(span)
13411351
} else {
13421352
hir::IsAsync::NotAsync
13431353
};
1354+
1355+
let default_safety = if attrs.iter().any(|attr| attr.has_name(sym::target_feature))
1356+
&& !matches!(h.safety, Safety::Unsafe(_))
1357+
&& default_safety.is_safe()
1358+
{
1359+
hir::Safety::Unsafe { target_feature: true }
1360+
} else {
1361+
default_safety
1362+
};
1363+
13441364
hir::FnHeader {
13451365
safety: self.lower_safety(h.safety, default_safety),
13461366
asyncness,
@@ -1394,7 +1414,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
13941414

13951415
pub(super) fn lower_safety(&mut self, s: Safety, default: hir::Safety) -> hir::Safety {
13961416
match s {
1397-
Safety::Unsafe(_) => hir::Safety::Unsafe,
1417+
Safety::Unsafe(_) => hir::Safety::Unsafe { target_feature: false },
13981418
Safety::Default => default,
13991419
Safety::Safe(_) => hir::Safety::Safe,
14001420
}

Diff for: ‎compiler/rustc_codegen_gcc/src/intrinsic/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1222,7 +1222,7 @@ fn get_rust_try_fn<'a, 'gcc, 'tcx>(
12221222
iter::once(i8p),
12231223
tcx.types.unit,
12241224
false,
1225-
rustc_hir::Safety::Unsafe,
1225+
rustc_hir::Safety::Unsafe { target_feature: false },
12261226
Abi::Rust,
12271227
)),
12281228
);
@@ -1233,7 +1233,7 @@ fn get_rust_try_fn<'a, 'gcc, 'tcx>(
12331233
[i8p, i8p].iter().cloned(),
12341234
tcx.types.unit,
12351235
false,
1236-
rustc_hir::Safety::Unsafe,
1236+
rustc_hir::Safety::Unsafe { target_feature: false },
12371237
Abi::Rust,
12381238
)),
12391239
);
@@ -1242,7 +1242,7 @@ fn get_rust_try_fn<'a, 'gcc, 'tcx>(
12421242
[try_fn_ty, i8p, catch_fn_ty],
12431243
tcx.types.i32,
12441244
false,
1245-
rustc_hir::Safety::Unsafe,
1245+
rustc_hir::Safety::Unsafe { target_feature: false },
12461246
Abi::Rust,
12471247
));
12481248
let rust_try = gen_fn(cx, "__rust_try", rust_fn_sig, codegen);

Diff for: ‎compiler/rustc_codegen_llvm/src/intrinsic.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1091,7 +1091,7 @@ fn get_rust_try_fn<'ll, 'tcx>(
10911091
[i8p],
10921092
tcx.types.unit,
10931093
false,
1094-
hir::Safety::Unsafe,
1094+
hir::Safety::Unsafe { target_feature: false },
10951095
ExternAbi::Rust,
10961096
)),
10971097
);
@@ -1102,7 +1102,7 @@ fn get_rust_try_fn<'ll, 'tcx>(
11021102
[i8p, i8p],
11031103
tcx.types.unit,
11041104
false,
1105-
hir::Safety::Unsafe,
1105+
hir::Safety::Unsafe { target_feature: false },
11061106
ExternAbi::Rust,
11071107
)),
11081108
);
@@ -1111,7 +1111,7 @@ fn get_rust_try_fn<'ll, 'tcx>(
11111111
[try_fn_ty, i8p, catch_fn_ty],
11121112
tcx.types.i32,
11131113
false,
1114-
hir::Safety::Unsafe,
1114+
hir::Safety::Unsafe { target_feature: false },
11151115
ExternAbi::Rust,
11161116
));
11171117
let rust_try = gen_fn(cx, "__rust_try", rust_fn_sig, codegen);

Diff for: ‎compiler/rustc_codegen_ssa/src/codegen_attrs.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc_errors::{DiagMessage, SubdiagMessage, struct_span_code_err};
66
use rustc_hir::def::DefKind;
77
use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId};
88
use rustc_hir::weak_lang_items::WEAK_LANG_ITEMS;
9-
use rustc_hir::{HirId, LangItem, lang_items};
9+
use rustc_hir::{self as hir, HirId, LangItem, lang_items};
1010
use rustc_middle::middle::codegen_fn_attrs::{
1111
CodegenFnAttrFlags, CodegenFnAttrs, PatchableFunctionEntry,
1212
};
@@ -251,7 +251,9 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs {
251251
sym::target_feature => {
252252
if !tcx.is_closure_like(did.to_def_id())
253253
&& let Some(fn_sig) = fn_sig()
254-
&& fn_sig.skip_binder().safety().is_safe()
254+
&& matches!(fn_sig.skip_binder().safety(), hir::Safety::Unsafe {
255+
target_feature: true
256+
})
255257
{
256258
if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
257259
// The `#[target_feature]` attribute is allowed on

Diff for: ‎compiler/rustc_hir/src/hir.rs

+12-4
Original file line numberDiff line numberDiff line change
@@ -3390,14 +3390,21 @@ impl<'hir> Item<'hir> {
33903390
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
33913391
#[derive(Encodable, Decodable, HashStable_Generic)]
33923392
pub enum Safety {
3393-
Unsafe,
3393+
Unsafe {
3394+
/// Whether this is a safe target_feature function.
3395+
/// We treat those as unsafe almost everywhere, but in unsafeck,
3396+
/// which allows calling them from other target_feature functions
3397+
/// without an `unsafe` block.
3398+
target_feature: bool,
3399+
},
33943400
Safe,
33953401
}
33963402

33973403
impl Safety {
33983404
pub fn prefix_str(self) -> &'static str {
33993405
match self {
3400-
Self::Unsafe => "unsafe ",
3406+
Self::Unsafe { target_feature: false } => "unsafe ",
3407+
Self::Unsafe { target_feature: true } => "#[target_feature] ",
34013408
Self::Safe => "",
34023409
}
34033410
}
@@ -3410,7 +3417,7 @@ impl Safety {
34103417
#[inline]
34113418
pub fn is_safe(self) -> bool {
34123419
match self {
3413-
Self::Unsafe => false,
3420+
Self::Unsafe { .. } => false,
34143421
Self::Safe => true,
34153422
}
34163423
}
@@ -3419,7 +3426,8 @@ impl Safety {
34193426
impl fmt::Display for Safety {
34203427
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
34213428
f.write_str(match *self {
3422-
Self::Unsafe => "unsafe",
3429+
Self::Unsafe { target_feature: false } => "unsafe",
3430+
Self::Unsafe { target_feature: true } => "#[target_feature] safe",
34233431
Self::Safe => "safe",
34243432
})
34253433
}

Diff for: ‎compiler/rustc_hir_analysis/src/check/intrinsic.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -
7676
tcx.fn_sig(intrinsic_id).skip_binder().safety()
7777
} else {
7878
// Old-style intrinsics are never safe
79-
Safety::Unsafe
79+
Safety::Unsafe { target_feature: false }
8080
};
8181
let is_in_list = match tcx.item_name(intrinsic_id.into()) {
8282
// When adding a new intrinsic to this list,
@@ -137,7 +137,7 @@ pub fn intrinsic_operation_unsafety(tcx: TyCtxt<'_>, intrinsic_id: LocalDefId) -
137137
| sym::fdiv_algebraic
138138
| sym::frem_algebraic
139139
| sym::const_eval_select => hir::Safety::Safe,
140-
_ => hir::Safety::Unsafe,
140+
_ => hir::Safety::Unsafe { target_feature: false },
141141
};
142142

143143
if has_safe_attr != is_in_list {
@@ -216,7 +216,7 @@ pub fn check_intrinsic_type(
216216
return;
217217
}
218218
};
219-
(n_tps, 0, 0, inputs, output, hir::Safety::Unsafe)
219+
(n_tps, 0, 0, inputs, output, hir::Safety::Unsafe { target_feature: false })
220220
} else {
221221
let safety = intrinsic_operation_unsafety(tcx, intrinsic_id);
222222
let (n_tps, n_cts, inputs, output) = match intrinsic_name {

Diff for: ‎compiler/rustc_hir_analysis/src/coherence/unsafety.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ pub(super) fn check_item(
2424
let trait_def_safety = if is_copy {
2525
// If `Self` has unsafe fields, `Copy` is unsafe to implement.
2626
if trait_header.trait_ref.skip_binder().self_ty().has_unsafe_fields() {
27-
rustc_hir::Safety::Unsafe
27+
rustc_hir::Safety::Unsafe { target_feature: false }
2828
} else {
2929
rustc_hir::Safety::Safe
3030
}
@@ -33,7 +33,7 @@ pub(super) fn check_item(
3333
};
3434

3535
match (trait_def_safety, unsafe_attr, trait_header.safety, trait_header.polarity) {
36-
(Safety::Safe, None, Safety::Unsafe, Positive | Reservation) => {
36+
(Safety::Safe, None, Safety::Unsafe { .. }, Positive | Reservation) => {
3737
let span = tcx.def_span(def_id);
3838
return Err(struct_span_code_err!(
3939
tcx.dcx(),
@@ -51,7 +51,7 @@ pub(super) fn check_item(
5151
.emit());
5252
}
5353

54-
(Safety::Unsafe, _, Safety::Safe, Positive | Reservation) => {
54+
(Safety::Unsafe { .. }, _, Safety::Safe, Positive | Reservation) => {
5555
let span = tcx.def_span(def_id);
5656
return Err(struct_span_code_err!(
5757
tcx.dcx(),
@@ -109,14 +109,14 @@ pub(super) fn check_item(
109109
.emit());
110110
}
111111

112-
(_, _, Safety::Unsafe, Negative) => {
112+
(_, _, Safety::Unsafe { .. }, Negative) => {
113113
// Reported in AST validation
114114
assert!(tcx.dcx().has_errors().is_some(), "unsafe negative impl");
115115
Ok(())
116116
}
117117
(_, _, Safety::Safe, Negative)
118-
| (Safety::Unsafe, _, Safety::Unsafe, Positive | Reservation)
119-
| (Safety::Safe, Some(_), Safety::Unsafe, Positive | Reservation)
118+
| (Safety::Unsafe { .. }, _, Safety::Unsafe { .. }, Positive | Reservation)
119+
| (Safety::Safe, Some(_), Safety::Unsafe { .. }, Positive | Reservation)
120120
| (Safety::Safe, None, Safety::Safe, _) => Ok(()),
121121
}
122122
}

Diff for: ‎compiler/rustc_hir_analysis/src/collect.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1371,7 +1371,7 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_, ty::PolyFn
13711371
// constructors for structs with `layout_scalar_valid_range` are unsafe to call
13721372
let safety = match tcx.layout_scalar_valid_range(adt_def_id) {
13731373
(Bound::Unbounded, Bound::Unbounded) => hir::Safety::Safe,
1374-
_ => hir::Safety::Unsafe,
1374+
_ => hir::Safety::Unsafe { target_feature: false },
13751375
};
13761376
ty::Binder::dummy(tcx.mk_fn_sig(inputs, ty, false, safety, ExternAbi::Rust))
13771377
}

Diff for: ‎compiler/rustc_hir_pretty/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2293,8 +2293,8 @@ impl<'a> State<'a> {
22932293

22942294
fn print_safety(&mut self, s: hir::Safety) {
22952295
match s {
2296-
hir::Safety::Safe => {}
2297-
hir::Safety::Unsafe => self.word_nbsp("unsafe"),
2296+
hir::Safety::Unsafe { target_feature: true } | hir::Safety::Safe => {}
2297+
hir::Safety::Unsafe { target_feature: false } => self.word_nbsp("unsafe"),
22982298
}
22992299
}
23002300

Diff for: ‎compiler/rustc_hir_typeck/src/coercion.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -863,7 +863,10 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
863863
let outer_universe = self.infcx.universe();
864864

865865
let result = if let ty::FnPtr(_, hdr_b) = b.kind()
866-
&& fn_ty_a.safety().is_safe()
866+
&& matches!(
867+
fn_ty_a.safety(),
868+
hir::Safety::Safe | hir::Safety::Unsafe { target_feature: true }
869+
)
867870
&& hdr_b.safety.is_unsafe()
868871
{
869872
let unsafe_a = self.tcx.safe_to_unsafe_fn_ty(fn_ty_a);
@@ -924,10 +927,12 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
924927
return Err(TypeError::IntrinsicCast);
925928
}
926929

927-
// Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396).
930+
// Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396),
931+
// report a better error than a safety mismatch.
932+
// FIXME(target_feature): do this inside `coerce_from_safe_fn`
928933

929934
if b_hdr.safety.is_safe()
930-
&& !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty()
935+
&& matches!(a_sig.safety(), hir::Safety::Unsafe { target_feature: true })
931936
{
932937
return Err(TypeError::TargetFeatureCast(def_id));
933938
}

0 commit comments

Comments
 (0)