Coerce safe-to-call target_feature functions to fn pointers.

Author: veluca93

Diff for: compiler/rustc_borrowck/src/type_check/mod.rs

@@ -19,6 +19,7 @@ use rustc_infer::infer::{
     BoundRegion, BoundRegionConversionTime, InferCtxt, NllRegionVariableOrigin,
 };
 use rustc_infer::traits::PredicateObligations;
+use rustc_middle::middle::codegen_fn_attrs::is_target_feature_call_safe;
 use rustc_middle::mir::tcx::PlaceTy;
 use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
 use rustc_middle::mir::*;
@@ -1654,7 +1655,31 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                 match *cast_kind {
                     CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer, coercion_source) => {
                         let is_implicit_coercion = coercion_source == CoercionSource::Implicit;
-                        let src_sig = op.ty(body, tcx).fn_sig(tcx);
+                        let src_ty = op.ty(body, tcx);
+                        let mut src_sig = src_ty.fn_sig(tcx);
+                        if let ty::FnDef(def_id, _) = src_ty.kind() {
+                            // If we are casting a safe target feature function to a safe fn
+                            // pointer, we need to mark the function as safe explicitly.
+                            if tcx.codegen_fn_attrs(def_id).safe_target_features
+                                && let ty::FnPtr(_, target_hdr) = *ty.kind()
+                                && target_hdr.safety.is_safe()
+                            {
+                                let coercee_features =
+                                    &tcx.codegen_fn_attrs(def_id).target_features;
+                                let body_features =
+                                    &tcx.codegen_fn_attrs(body.source.def_id()).target_features;
+                                if is_target_feature_call_safe(
+                                    tcx,
+                                    &coercee_features,
+                                    &body_features,
+                                ) {
+                                    src_sig = src_sig.map_bound(|sig| ty::FnSig {
+                                        safety: hir::Safety::Safe,
+                                        ..sig
+                                    })
+                                }
+                            }
+                        }
 
                         // HACK: This shouldn't be necessary... We can remove this when we actually
                         // get binders with where clauses, then elaborate implied bounds into that

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

@@ -51,6 +51,7 @@ use rustc_infer::traits::{
     PredicateObligations,
 };
 use rustc_middle::lint::in_external_macro;
+use rustc_middle::middle::codegen_fn_attrs::is_target_feature_call_safe;
 use rustc_middle::span_bug;
 use rustc_middle::traits::BuiltinImplSource;
 use rustc_middle::ty::adjustment::{
@@ -920,7 +921,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
 
         match b.kind() {
             ty::FnPtr(_, b_hdr) => {
-                let a_sig = a.fn_sig(self.tcx);
+                let mut a_sig = a.fn_sig(self.tcx);
                 if let ty::FnDef(def_id, _) = *a.kind() {
                     // Intrinsics are not coercible to function pointers
                     if self.tcx.intrinsic(def_id).is_some() {
@@ -932,19 +933,23 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
                         return Err(TypeError::ForceInlineCast);
                     }
 
-                    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
                     {
-                        return Err(TypeError::TargetFeatureCast(def_id));
+                        // Allow the coercion if the current function has all the features that would be
+                        // needed to call the coercee safely.
+                        let coercee_features = &self.tcx.codegen_fn_attrs(def_id).target_features;
+                        let body_features =
+                            &self.tcx.codegen_fn_attrs(self.fcx.body_id).target_features;
+                        if !is_target_feature_call_safe(self.tcx, &coercee_features, &body_features)
+                        {
+                            return Err(TypeError::TargetFeatureCast(def_id));
+                        } else {
+                            // The coercee behaves like a safe function, since it is a target_feature
+                            // function that would be callable safely in this context.
+                            a_sig = a_sig
+                                .map_bound(|sig| ty::FnSig { safety: hir::Safety::Safe, ..sig })
+                        }
                     }
                 }
 

Diff for: compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

@@ -5,6 +5,7 @@ use rustc_span::Symbol;
 use rustc_target::spec::SanitizerSet;
 
 use crate::mir::mono::Linkage;
+use crate::ty::TyCtxt;
 
 #[derive(Clone, TyEncodable, TyDecodable, HashStable, Debug)]
 pub struct CodegenFnAttrs {
@@ -179,3 +180,18 @@ impl CodegenFnAttrs {
             }
     }
 }
+
+pub fn is_target_feature_call_safe(
+    tcx: TyCtxt<'_>,
+    callee_features: &[TargetFeature],
+    body_features: &[TargetFeature],
+) -> bool {
+    // 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
+    tcx.sess.target.options.is_like_wasm
+        || callee_features
+            .iter()
+            .all(|feature| body_features.iter().any(|f| f.name == feature.name))
+}

Diff for: compiler/rustc_mir_build/src/check_unsafety.rs

@@ -5,7 +5,7 @@ use std::ops::Bound;
 use rustc_errors::DiagArgValue;
 use rustc_hir::def::DefKind;
 use rustc_hir::{self as hir, BindingMode, ByRef, HirId, Mutability};
-use rustc_middle::middle::codegen_fn_attrs::TargetFeature;
+use rustc_middle::middle::codegen_fn_attrs::{TargetFeature, is_target_feature_call_safe};
 use rustc_middle::mir::BorrowKind;
 use rustc_middle::span_bug;
 use rustc_middle::thir::visit::Visitor;
@@ -495,15 +495,11 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
                     };
                     self.requires_unsafe(expr.span, CallToUnsafeFunction(func_id));
                 } 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
-                    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)
-                        })
-                    {
+                    if !is_target_feature_call_safe(
+                        self.tcx,
+                        callee_features,
+                        self.body_target_features,
+                    ) {
                         let missing: Vec<_> = callee_features
                             .iter()
                             .copied()

Diff for: tests/ui/rfcs/rfc-2396-target_feature-11/fn-ptr.rs

@@ -2,9 +2,23 @@
 
 #![feature(target_feature_11)]
 
+#[target_feature(enable = "avx")]
+fn foo_avx() {}
+
 #[target_feature(enable = "sse2")]
 fn foo() {}
 
+#[target_feature(enable = "sse2")]
+fn bar() {
+    let foo: fn() = foo; // this is OK, as we have the necessary target features.
+    let foo: fn() = foo_avx; //~ ERROR mismatched types
+}
+
 fn main() {
+    if std::is_x86_feature_detected!("sse2") {
+        unsafe {
+            bar();
+        }
+    }
     let foo: fn() = foo; //~ ERROR mismatched types
 }

Diff for: tests/ui/rfcs/rfc-2396-target_feature-11/fn-ptr.stderr

@@ -1,5 +1,20 @@
 error[E0308]: mismatched types
-  --> $DIR/fn-ptr.rs:9:21
+  --> $DIR/fn-ptr.rs:14:21
+   |
+LL | #[target_feature(enable = "avx")]
+   | --------------------------------- `#[target_feature]` added here
+...
+LL |     let foo: fn() = foo_avx;
+   |              ----   ^^^^^^^ cannot coerce functions with `#[target_feature]` to safe function pointers
+   |              |
+   |              expected due to this
+   |
+   = note: expected fn pointer `fn()`
+                 found fn item `#[target_features] fn() {foo_avx}`
+   = note: functions with `#[target_feature]` can only be coerced to `unsafe` function pointers
+
+error[E0308]: mismatched types
+  --> $DIR/fn-ptr.rs:23:21
    |
 LL | #[target_feature(enable = "sse2")]
    | ---------------------------------- `#[target_feature]` added here
@@ -13,6 +28,6 @@ LL |     let foo: fn() = foo;
                  found fn item `#[target_features] fn() {foo}`
    = note: functions with `#[target_feature]` can only be coerced to `unsafe` function pointers
 
-error: aborting due to 1 previous error
+error: aborting due to 2 previous errors
 
 For more information about this error, try `rustc --explain E0308`.

Diff for: tests/ui/rfcs/rfc-2396-target_feature-11/return-fn-ptr.rs

@@ -0,0 +1,22 @@
+//@ only-x86_64
+//@ run-pass
+
+#![feature(target_feature_11)]
+
+#[target_feature(enable = "sse2")]
+fn foo() -> bool {
+    true
+}
+
+#[target_feature(enable = "sse2")]
+fn bar() -> fn() -> bool {
+    foo
+}
+
+fn main() {
+    if !std::is_x86_feature_detected!("sse2") {
+        return;
+    }
+    let f = unsafe { bar() };
+    assert!(f());
+}