Skip to content

Commit d2e7717

Browse files
oli-obkoli-obk
committed
Treat safe target_feature functions as unsafe by default
1 parent 2088059 commit d2e7717

File tree

35 files changed

+206
-94
lines changed

35 files changed

+206
-94
lines changed

‎compiler/rustc_ast_lowering/src/delegation.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,11 @@ 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 { hir::Safety::Unsafe } else { hir::Safety::Safe },
195+
&[],
196+
),
193197
None => self.generate_header_error(),
194198
}
195199
} else {
@@ -199,7 +203,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
199203
Asyncness::No => hir::IsAsync::NotAsync,
200204
};
201205
hir::FnHeader {
202-
safety: sig.safety,
206+
safety: (!self.tcx.codegen_fn_attrs(sig_id).safe_target_features)
207+
.then_some(sig.safety),
203208
constness: self.tcx.constness(sig_id),
204209
asyncness,
205210
abi: sig.abi,
@@ -385,7 +390,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
385390

386391
fn generate_header_error(&self) -> hir::FnHeader {
387392
hir::FnHeader {
388-
safety: hir::Safety::Safe,
393+
safety: Some(hir::Safety::Safe),
389394
constness: hir::Constness::NotConst,
390395
asyncness: hir::IsAsync::NotAsync,
391396
abi: abi::Abi::Rust,

‎compiler/rustc_ast_lowering/src/item.rs

Lines changed: 20 additions & 7 deletions
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,7 @@ 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(sig.header, hir::Safety::Unsafe, attrs);
637637

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

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

754754
let (generics, kind, has_default) = match &i.kind {
@@ -775,6 +775,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
775775
i.id,
776776
FnDeclKind::Trait,
777777
sig.header.coroutine_kind,
778+
attrs,
778779
);
779780
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Required(names)), false)
780781
}
@@ -793,6 +794,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
793794
i.id,
794795
FnDeclKind::Trait,
795796
sig.header.coroutine_kind,
797+
attrs,
796798
);
797799
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Provided(body_id)), true)
798800
}
@@ -878,7 +880,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
878880
let has_value = true;
879881
let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
880882
let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
881-
self.lower_attrs(hir_id, &i.attrs);
883+
let attrs = self.lower_attrs(hir_id, &i.attrs);
882884

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

913916
(generics, hir::ImplItemKind::Fn(sig, body_id))
@@ -1322,8 +1325,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
13221325
id: NodeId,
13231326
kind: FnDeclKind,
13241327
coroutine_kind: Option<CoroutineKind>,
1328+
attrs: &[Attribute],
13251329
) -> (&'hir hir::Generics<'hir>, hir::FnSig<'hir>) {
1326-
let header = self.lower_fn_header(sig.header, hir::Safety::Safe);
1330+
let header = self.lower_fn_header(sig.header, hir::Safety::Safe, attrs);
13271331
let itctx = ImplTraitContext::Universal;
13281332
let (generics, decl) = self.lower_generics(generics, id, itctx, |this| {
13291333
this.lower_fn_decl(&sig.decl, id, sig.span, kind, coroutine_kind)
@@ -1335,14 +1339,23 @@ impl<'hir> LoweringContext<'_, 'hir> {
13351339
&mut self,
13361340
h: FnHeader,
13371341
default_safety: hir::Safety,
1342+
attrs: &[Attribute],
13381343
) -> hir::FnHeader {
13391344
let asyncness = if let Some(CoroutineKind::Async { span, .. }) = h.coroutine_kind {
13401345
hir::IsAsync::Async(span)
13411346
} else {
13421347
hir::IsAsync::NotAsync
13431348
};
1349+
1350+
let safety = self.lower_safety(h.safety, default_safety);
1351+
1352+
// Treat safe `#[target_feature]` functions as unsafe, but also remember that we did so.
1353+
let target_feature =
1354+
attrs.iter().any(|attr| attr.has_name(sym::target_feature)) && safety.is_safe();
1355+
let safety = (!target_feature).then_some(safety);
1356+
13441357
hir::FnHeader {
1345-
safety: self.lower_safety(h.safety, default_safety),
1358+
safety,
13461359
asyncness,
13471360
constness: self.lower_constness(h.constness),
13481361
abi: self.lower_extern(h.ext),

‎compiler/rustc_codegen_ssa/src/codegen_attrs.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,16 +249,19 @@ 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 = sig.header.safety.is_none();
257+
codegen_fn_attrs.safe_target_features = safe_target_features;
258+
if safe_target_features {
256259
if tcx.sess.target.is_like_wasm || tcx.sess.opts.actually_rustdoc {
257260
// The `#[target_feature]` attribute is allowed on
258261
// WebAssembly targets on all functions, including safe
259262
// ones. Other targets require that `#[target_feature]` is
260263
// only applied to unsafe functions (pending the
261-
// `target_feature_11` feature) because on most targets
264+
// `target_feature_11` feature) because on most target.is_s
262265
// execution of instructions that are not supported is
263266
// considered undefined behavior. For WebAssembly which is a
264267
// 100% safe target at execution time it's not possible to

‎compiler/rustc_hir/src/hir.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3723,7 +3723,7 @@ impl fmt::Display for Constness {
37233723

37243724
#[derive(Copy, Clone, Debug, HashStable_Generic)]
37253725
pub struct FnHeader {
3726-
pub safety: Safety,
3726+
pub safety: Option<Safety>,
37273727
pub constness: Constness,
37283728
pub asyncness: IsAsync,
37293729
pub abi: ExternAbi,
@@ -3739,7 +3739,15 @@ impl FnHeader {
37393739
}
37403740

37413741
pub fn is_unsafe(&self) -> bool {
3742-
self.safety.is_unsafe()
3742+
self.safety().is_unsafe()
3743+
}
3744+
3745+
pub fn is_safe(&self) -> bool {
3746+
self.safety().is_safe()
3747+
}
3748+
3749+
pub fn safety(&self) -> Safety {
3750+
self.safety.unwrap_or(Safety::Unsafe)
37433751
}
37443752
}
37453753

‎compiler/rustc_hir_analysis/src/collect.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,7 +1350,7 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_, ty::PolyFn
13501350
{
13511351
icx.lowerer().lower_fn_ty(
13521352
hir_id,
1353-
sig.header.safety,
1353+
sig.header.safety(),
13541354
sig.header.abi,
13551355
sig.decl,
13561356
Some(generics),
@@ -1365,13 +1365,18 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_, ty::PolyFn
13651365
kind: TraitItemKind::Fn(FnSig { header, decl, span: _ }, _),
13661366
generics,
13671367
..
1368-
}) => {
1369-
icx.lowerer().lower_fn_ty(hir_id, header.safety, header.abi, decl, Some(generics), None)
1370-
}
1368+
}) => icx.lowerer().lower_fn_ty(
1369+
hir_id,
1370+
header.safety(),
1371+
header.abi,
1372+
decl,
1373+
Some(generics),
1374+
None,
1375+
),
13711376

13721377
ForeignItem(&hir::ForeignItem { kind: ForeignItemKind::Fn(sig, _, _), .. }) => {
13731378
let abi = tcx.hir().get_foreign_abi(hir_id);
1374-
compute_sig_of_foreign_fn_decl(tcx, def_id, sig.decl, abi, sig.header.safety)
1379+
compute_sig_of_foreign_fn_decl(tcx, def_id, sig.decl, abi, sig.header.safety())
13751380
}
13761381

13771382
Ctor(data) | Variant(hir::Variant { data, .. }) if data.ctor().is_some() => {
@@ -1419,7 +1424,7 @@ fn lower_fn_sig_recovering_infer_ret_ty<'tcx>(
14191424

14201425
icx.lowerer().lower_fn_ty(
14211426
icx.tcx().local_def_id_to_hir_id(def_id),
1422-
sig.header.safety,
1427+
sig.header.safety(),
14231428
sig.header.abi,
14241429
sig.decl,
14251430
Some(generics),

‎compiler/rustc_hir_pretty/src/lib.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2386,7 +2386,7 @@ impl<'a> State<'a> {
23862386
self.print_fn(
23872387
decl,
23882388
hir::FnHeader {
2389-
safety,
2389+
safety: Some(safety),
23902390
abi,
23912391
constness: hir::Constness::NotConst,
23922392
asyncness: hir::IsAsync::NotAsync,
@@ -2402,12 +2402,20 @@ impl<'a> State<'a> {
24022402
fn print_fn_header_info(&mut self, header: hir::FnHeader) {
24032403
self.print_constness(header.constness);
24042404

2405+
let safety = match header.safety {
2406+
Some(safety) => safety,
2407+
None => {
2408+
self.word_nbsp("#[target_feature]");
2409+
hir::Safety::Safe
2410+
}
2411+
};
2412+
24052413
match header.asyncness {
24062414
hir::IsAsync::NotAsync => {}
24072415
hir::IsAsync::Async(_) => self.word_nbsp("async"),
24082416
}
24092417

2410-
self.print_safety(header.safety);
2418+
self.print_safety(safety);
24112419

24122420
if header.abi != ExternAbi::Rust {
24132421
self.word_nbsp("extern");

‎compiler/rustc_hir_typeck/src/coercion.rs

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

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

929931
if b_hdr.safety.is_safe()
930-
&& !self.tcx.codegen_fn_attrs(def_id).target_features.is_empty()
932+
&& self.tcx.codegen_fn_attrs(def_id).safe_target_features
931933
{
932934
return Err(TypeError::TargetFeatureCast(def_id));
933935
}

‎compiler/rustc_hir_typeck/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ fn typeck_with_fallback<'tcx>(
151151

152152
if let Some(hir::FnSig { header, decl, .. }) = node.fn_sig() {
153153
let fn_sig = if decl.output.get_infer_ret_ty().is_some() {
154-
fcx.lowerer().lower_fn_ty(id, header.safety, header.abi, decl, None, None)
154+
fcx.lowerer().lower_fn_ty(id, header.safety(), header.abi, decl, None, None)
155155
} else {
156156
tcx.fn_sig(def_id).instantiate_identity()
157157
};

‎compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ pub struct CodegenFnAttrs {
2929
/// The `#[target_feature(enable = "...")]` attribute and the enabled
3030
/// features (only enabled features are supported right now).
3131
pub target_features: Vec<TargetFeature>,
32+
/// Whether the function was declared safe, but has target features
33+
pub safe_target_features: bool,
3234
/// The `#[linkage = "..."]` attribute on Rust-defined items and the value we found.
3335
pub linkage: Option<Linkage>,
3436
/// The `#[linkage = "..."]` attribute on foreign items and the value we found.
@@ -149,6 +151,7 @@ impl CodegenFnAttrs {
149151
link_name: None,
150152
link_ordinal: None,
151153
target_features: vec![],
154+
safe_target_features: false,
152155
linkage: None,
153156
import_linkage: None,
154157
link_section: None,

‎compiler/rustc_middle/src/ty/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ pub struct DelegationFnSig {
222222
pub param_count: usize,
223223
pub has_self: bool,
224224
pub c_variadic: bool,
225+
pub target_feature: bool,
225226
}
226227

227228
#[derive(Clone, Copy, Debug)]

‎compiler/rustc_mir_build/src/check_unsafety.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -479,19 +479,27 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
479479
return; // don't visit the whole expression
480480
}
481481
ExprKind::Call { fun, ty: _, args: _, from_hir_call: _, fn_span: _ } => {
482-
if self.thir[fun].ty.fn_sig(self.tcx).safety().is_unsafe() {
483-
let func_id = if let ty::FnDef(func_id, _) = self.thir[fun].ty.kind() {
482+
let fn_ty = self.thir[fun].ty;
483+
let sig = fn_ty.fn_sig(self.tcx);
484+
let (callee_features, safe_target_features): (&[_], _) = match fn_ty.kind() {
485+
ty::FnDef(func_id, ..) => {
486+
let cg_attrs = self.tcx.codegen_fn_attrs(func_id);
487+
(&cg_attrs.target_features, cg_attrs.safe_target_features)
488+
}
489+
_ => (&[], false),
490+
};
491+
if sig.safety().is_unsafe() && !safe_target_features {
492+
let func_id = if let ty::FnDef(func_id, _) = fn_ty.kind() {
484493
Some(*func_id)
485494
} else {
486495
None
487496
};
488497
self.requires_unsafe(expr.span, CallToUnsafeFunction(func_id));
489-
} else if let &ty::FnDef(func_did, _) = self.thir[fun].ty.kind() {
498+
} else if let &ty::FnDef(func_did, _) = fn_ty.kind() {
490499
// If the called function has target features the calling function hasn't,
491500
// the call requires `unsafe`. Don't check this on wasm
492501
// targets, though. For more information on wasm see the
493502
// is_like_wasm check in hir_analysis/src/collect.rs
494-
let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features;
495503
if !self.tcx.sess.target.options.is_like_wasm
496504
&& !callee_features.iter().all(|feature| {
497505
self.body_target_features.iter().any(|f| f.name == feature.name)
@@ -1112,7 +1120,11 @@ pub(crate) fn check_unsafety(tcx: TyCtxt<'_>, def: LocalDefId) {
11121120

11131121
let hir_id = tcx.local_def_id_to_hir_id(def);
11141122
let safety_context = tcx.hir().fn_sig_by_hir_id(hir_id).map_or(SafetyContext::Safe, |fn_sig| {
1115-
if fn_sig.header.safety.is_unsafe() { SafetyContext::UnsafeFn } else { SafetyContext::Safe }
1123+
if matches!(fn_sig.header.safety, Some(hir::Safety::Unsafe)) {
1124+
SafetyContext::UnsafeFn
1125+
} else {
1126+
SafetyContext::Safe
1127+
}
11161128
});
11171129
let body_target_features = &tcx.body_codegen_attrs(def.to_def_id()).target_features;
11181130
let mut warnings = Vec::new();

‎compiler/rustc_resolve/src/late.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4988,12 +4988,13 @@ struct ItemInfoCollector<'a, 'ra, 'tcx> {
49884988
}
49894989

49904990
impl ItemInfoCollector<'_, '_, '_> {
4991-
fn collect_fn_info(&mut self, sig: &FnSig, id: NodeId) {
4991+
fn collect_fn_info(&mut self, sig: &FnSig, id: NodeId, attrs: &[Attribute]) {
49924992
let sig = DelegationFnSig {
49934993
header: sig.header,
49944994
param_count: sig.decl.inputs.len(),
49954995
has_self: sig.decl.has_self(),
49964996
c_variadic: sig.decl.c_variadic(),
4997+
target_feature: attrs.iter().any(|attr| attr.has_name(sym::target_feature)),
49974998
};
49984999
self.r.delegation_fn_sigs.insert(self.r.local_def_id(id), sig);
49995000
}
@@ -5012,7 +5013,7 @@ impl<'ast> Visitor<'ast> for ItemInfoCollector<'_, '_, '_> {
50125013
| ItemKind::Trait(box Trait { ref generics, .. })
50135014
| ItemKind::TraitAlias(ref generics, _) => {
50145015
if let ItemKind::Fn(box Fn { ref sig, .. }) = &item.kind {
5015-
self.collect_fn_info(sig, item.id);
5016+
self.collect_fn_info(sig, item.id, &item.attrs);
50165017
}
50175018

50185019
let def_id = self.r.local_def_id(item.id);
@@ -5045,7 +5046,7 @@ impl<'ast> Visitor<'ast> for ItemInfoCollector<'_, '_, '_> {
50455046

50465047
fn visit_assoc_item(&mut self, item: &'ast AssocItem, ctxt: AssocCtxt) {
50475048
if let AssocItemKind::Fn(box Fn { ref sig, .. }) = &item.kind {
5048-
self.collect_fn_info(sig, item.id);
5049+
self.collect_fn_info(sig, item.id, &item.attrs);
50495050
}
50505051
visit::walk_assoc_item(self, item, ctxt);
50515052
}

‎src/librustdoc/clean/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3068,7 +3068,7 @@ fn clean_maybe_renamed_foreign_item<'tcx>(
30683068
let kind = match item.kind {
30693069
hir::ForeignItemKind::Fn(sig, names, generics) => ForeignFunctionItem(
30703070
clean_function(cx, &sig, generics, FunctionArgs::Names(names)),
3071-
sig.header.safety,
3071+
sig.header.safety(),
30723072
),
30733073
hir::ForeignItemKind::Static(ty, mutability, safety) => ForeignStaticItem(
30743074
Static { type_: Box::new(clean_ty(ty, cx)), mutability, expr: None },

0 commit comments

Comments
 (0)