Treat safe target_feature functions as unsafe by default

Author: oli-obk

compiler/rustc_ast_lowering/src/delegation.rs

@@ -189,7 +189,11 @@ 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,
+                    if sig.target_feature { hir::Safety::Unsafe } else { hir::Safety::Safe },
+                    &[],
+                ),
                 None => self.generate_header_error(),
             }
         } else {
@@ -199,7 +203,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 Asyncness::No => hir::IsAsync::NotAsync,
             };
             hir::FnHeader {
-                safety: sig.safety,
+                safety: (!self.tcx.codegen_fn_attrs(sig_id).safe_target_features)
+                    .then_some(sig.safety),
                 constness: self.tcx.constness(sig_id),
                 asyncness,
                 abi: sig.abi,
@@ -385,7 +390,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
     fn generate_header_error(&self) -> hir::FnHeader {
         hir::FnHeader {
-            safety: hir::Safety::Safe,
+            safety: Some(hir::Safety::Safe),
             constness: hir::Constness::NotConst,
             asyncness: hir::IsAsync::NotAsync,
             abi: abi::Abi::Rust,

compiler/rustc_ast_lowering/src/item.rs

@@ -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_id)
@@ -609,7 +609,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),
@@ -633,7 +633,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) },
@@ -748,7 +748,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
     fn lower_trait_item(&mut self, i: &AssocItem) -> &'hir hir::TraitItem<'hir> {
         let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
-        self.lower_attrs(hir_id, &i.attrs);
+        let attrs = self.lower_attrs(hir_id, &i.attrs);
         let trait_item_def_id = hir_id.expect_owner();
 
         let (generics, kind, has_default) = match &i.kind {
@@ -775,6 +775,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                     i.id,
                     FnDeclKind::Trait,
                     sig.header.coroutine_kind,
+                    attrs,
                 );
                 (generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Required(names)), false)
             }
@@ -793,6 +794,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)
             }
@@ -878,7 +880,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         let has_value = true;
         let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value);
         let hir_id = hir::HirId::make_owner(self.current_hir_id_owner.def_id);
-        self.lower_attrs(hir_id, &i.attrs);
+        let attrs = self.lower_attrs(hir_id, &i.attrs);
 
         let (generics, kind) = match &i.kind {
             AssocItemKind::Const(box ConstItem { generics, ty, expr, .. }) => self.lower_generics(
@@ -908,6 +910,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))
@@ -1322,8 +1325,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
         id: NodeId,
         kind: FnDeclKind,
         coroutine_kind: Option<CoroutineKind>,
+        attrs: &[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)
@@ -1335,14 +1339,23 @@ impl<'hir> LoweringContext<'_, 'hir> {
         &mut self,
         h: FnHeader,
         default_safety: hir::Safety,
+        attrs: &[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 target_feature =
+            attrs.iter().any(|attr| attr.has_name(sym::target_feature)) && safety.is_safe();
+        let safety = (!target_feature).then_some(safety);
+
         hir::FnHeader {
-            safety: self.lower_safety(h.safety, default_safety),
+            safety,
             asyncness,
             constness: self.lower_constness(h.constness),
             abi: self.lower_extern(h.ext),

compiler/rustc_codegen_ssa/src/codegen_attrs.rs

@@ -249,16 +249,19 @@ 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 = sig.header.safety.is_none();
+                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
                         // ones. Other targets require that `#[target_feature]` is
                         // only applied to unsafe functions (pending the
-                        // `target_feature_11` feature) because on most targets
+                        // `target_feature_11` feature) because on most target.is_s
                         // execution of instructions that are not supported is
                         // considered undefined behavior. For WebAssembly which is a
                         // 100% safe target at execution time it's not possible to

compiler/rustc_hir/src/hir.rs

@@ -3475,7 +3475,7 @@ impl fmt::Display for Constness {
 
 #[derive(Copy, Clone, Debug, HashStable_Generic)]
 pub struct FnHeader {
-    pub safety: Safety,
+    pub safety: Option<Safety>,
     pub constness: Constness,
     pub asyncness: IsAsync,
     pub abi: ExternAbi,
@@ -3491,7 +3491,15 @@ 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 {
+        self.safety.unwrap_or(Safety::Unsafe)
     }
 }
 

compiler/rustc_hir_analysis/src/collect.rs

@@ -1350,7 +1350,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),
@@ -1365,13 +1365,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() => {
@@ -1419,7 +1424,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),

compiler/rustc_hir_pretty/src/lib.rs

@@ -2280,7 +2280,7 @@ impl<'a> State<'a> {
         self.print_fn(
             decl,
             hir::FnHeader {
-                safety,
+                safety: Some(safety),
                 abi,
                 constness: hir::Constness::NotConst,
                 asyncness: hir::IsAsync::NotAsync,
@@ -2296,12 +2296,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 {
+            Some(safety) => safety,
+            None => {
+                self.word_nbsp("#[target_feature]");
+                hir::Safety::Safe
+            }
+        };
+
         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");

compiler/rustc_hir_typeck/src/coercion.rs

@@ -924,10 +924,12 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
                         return Err(TypeError::IntrinsicCast);
                     }
 
-                    // Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396).
+                    // 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`
 
                     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));
                     }

compiler/rustc_hir_typeck/src/lib.rs

@@ -151,7 +151,7 @@ fn typeck_with_fallback<'tcx>(
 
     if let Some(hir::FnSig { header, decl, .. }) = node.fn_sig() {
         let fn_sig = if decl.output.get_infer_ret_ty().is_some() {
-            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()
         };

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

@@ -29,6 +29,8 @@ pub struct CodegenFnAttrs {
     /// The `#[target_feature(enable = "...")]` attribute and the enabled
     /// features (only enabled features are supported right now).
     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.
@@ -149,6 +151,7 @@ impl CodegenFnAttrs {
             link_name: None,
             link_ordinal: None,
             target_features: vec![],
+            safe_target_features: false,
             linkage: None,
             import_linkage: None,
             link_section: None,

compiler/rustc_middle/src/ty/mod.rs

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

compiler/rustc_mir_build/src/check_unsafety.rs

@@ -479,19 +479,27 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
                 return; // don't visit the whole expression
             }
             ExprKind::Call { fun, ty: _, args: _, from_hir_call: _, fn_span: _ } => {
-                if self.thir[fun].ty.fn_sig(self.tcx).safety().is_unsafe() {
-                    let func_id = if let ty::FnDef(func_id, _) = self.thir[fun].ty.kind() {
+                let fn_ty = self.thir[fun].ty;
+                let sig = fn_ty.fn_sig(self.tcx);
+                let (callee_features, safe_target_features): (&[_], _) = match fn_ty.kind() {
+                    ty::FnDef(func_id, ..) => {
+                        let cg_attrs = self.tcx.codegen_fn_attrs(func_id);
+                        (&cg_attrs.target_features, cg_attrs.safe_target_features)
+                    }
+                    _ => (&[], false),
+                };
+                if sig.safety().is_unsafe() && !safe_target_features {
+                    let func_id = if let ty::FnDef(func_id, _) = fn_ty.kind() {
                         Some(*func_id)
                     } else {
                         None
                     };
                     self.requires_unsafe(expr.span, CallToUnsafeFunction(func_id));
-                } else if let &ty::FnDef(func_did, _) = self.thir[fun].ty.kind() {
+                } else if let &ty::FnDef(func_did, _) = fn_ty.kind() {
                     // If the called function has target features the calling function hasn't,
                     // the call requires `unsafe`. Don't check this on wasm
                     // targets, though. For more information on wasm see the
                     // is_like_wasm check in hir_analysis/src/collect.rs
-                    let callee_features = &self.tcx.codegen_fn_attrs(func_did).target_features;
                     if !self.tcx.sess.target.options.is_like_wasm
                         && !callee_features.iter().all(|feature| {
                             self.body_target_features.iter().any(|f| f.name == feature.name)
@@ -1112,7 +1120,11 @@ 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| {
-        if fn_sig.header.safety.is_unsafe() { SafetyContext::UnsafeFn } else { SafetyContext::Safe }
+        if matches!(fn_sig.header.safety, Some(hir::Safety::Unsafe)) {
+            SafetyContext::UnsafeFn
+        } else {
+            SafetyContext::Safe
+        }
     });
     let body_target_features = &tcx.body_codegen_attrs(def.to_def_id()).target_features;
     let mut warnings = Vec::new();

compiler/rustc_resolve/src/late.rs

@@ -4988,12 +4988,13 @@ struct ItemInfoCollector<'a, 'ra, 'tcx> {
 }
 
 impl ItemInfoCollector<'_, '_, '_> {
-    fn collect_fn_info(&mut self, sig: &FnSig, id: NodeId) {
+    fn collect_fn_info(&mut self, sig: &FnSig, id: NodeId, attrs: &[Attribute]) {
         let sig = DelegationFnSig {
             header: sig.header,
             param_count: sig.decl.inputs.len(),
             has_self: sig.decl.has_self(),
             c_variadic: sig.decl.c_variadic(),
+            target_feature: attrs.iter().any(|attr| attr.has_name(sym::target_feature)),
         };
         self.r.delegation_fn_sigs.insert(self.r.local_def_id(id), sig);
     }
@@ -5012,7 +5013,7 @@ impl<'ast> Visitor<'ast> for ItemInfoCollector<'_, '_, '_> {
             | ItemKind::Trait(box Trait { ref generics, .. })
             | ItemKind::TraitAlias(ref generics, _) => {
                 if let ItemKind::Fn(box Fn { ref sig, .. }) = &item.kind {
-                    self.collect_fn_info(sig, item.id);
+                    self.collect_fn_info(sig, item.id, &item.attrs);
                 }
 
                 let def_id = self.r.local_def_id(item.id);
@@ -5045,7 +5046,7 @@ impl<'ast> Visitor<'ast> for ItemInfoCollector<'_, '_, '_> {
 
     fn visit_assoc_item(&mut self, item: &'ast AssocItem, ctxt: AssocCtxt) {
         if let AssocItemKind::Fn(box Fn { ref sig, .. }) = &item.kind {
-            self.collect_fn_info(sig, item.id);
+            self.collect_fn_info(sig, item.id, &item.attrs);
         }
         visit::walk_assoc_item(self, item, ctxt);
     }

compiler/rustc_smir/src/rustc_smir/convert/mod.rs

@@ -15,7 +15,7 @@ impl<'tcx> Stable<'tcx> for rustc_hir::Safety {
     type T = stable_mir::mir::Safety;
     fn stable(&self, _: &mut Tables<'_>) -> Self::T {
         match self {
-            rustc_hir::Safety::Unsafe => stable_mir::mir::Safety::Unsafe,
+            rustc_hir::Safety::Unsafe { .. } => stable_mir::mir::Safety::Unsafe,
             rustc_hir::Safety::Safe => stable_mir::mir::Safety::Safe,
         }
     }