Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow reifying intrinsics to fn pointers. (rebase of #86699) #126595

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_span::DUMMY_SP;
use rustc_target::abi::{FieldIdx, FIRST_VARIANT};
use rustc_target::spec::abi::Abi;
use rustc_trait_selection::traits::query::type_op::custom::scrape_region_constraints;
use rustc_trait_selection::traits::query::type_op::custom::CustomTypeOp;
use rustc_trait_selection::traits::query::type_op::{TypeOp, TypeOpOutput};
Expand Down Expand Up @@ -1994,6 +1995,16 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
// and hence may contain unnormalized results.
let fn_sig = self.normalize(fn_sig, location);

// NOTE(eddyb) see comment on `prepare_fn_sig_for_reify`
// in `rustc_typeck::check::coercion`.
let fn_sig = fn_sig.map_bound(|mut sig| {
if matches!(sig.abi, Abi::RustIntrinsic) {
sig.abi = Abi::Rust;
}

sig
});

let ty_fn_ptr_from = Ty::new_fn_ptr(tcx, fn_sig);

if let Err(terr) = self.eq_types(
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ use rustc_middle::bug;
use rustc_middle::mir::Mutability;
use rustc_middle::ty::adjustment::AllowTwoPhase;
use rustc_middle::ty::cast::{CastKind, CastTy};
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::TyCtxt;
use rustc_middle::ty::{self, Ty, TypeAndMut, TypeVisitableExt, VariantDef};
use rustc_session::lint;
Expand Down Expand Up @@ -692,9 +691,6 @@ impl<'a, 'tcx> CastCheck<'tcx> {
AllowTwoPhase::No,
None,
);
if let Err(TypeError::IntrinsicCast) = res {
return Err(CastError::IllegalCast);
}
if res.is_err() {
return Err(CastError::NonScalar);
}
Expand Down
31 changes: 22 additions & 9 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,23 @@ impl<'a, 'tcx> Deref for Coerce<'a, 'tcx> {

type CoerceResult<'tcx> = InferResult<'tcx, (Vec<Adjustment<'tcx>>, Ty<'tcx>)>;

/// Make any adjustments necessary for a function signature to be compatible
/// with reification to a `fn` pointer. In particular, intrinsics are imported
/// using pseudo-ABIs (`extern "rust-intrinsic" {...}`) currently, but that's
/// an implementation detail and any `fn` pointers that may be taken to them
/// should be indistinguishable from those to regular Rust functions, in order
/// to allow e.g. libcore public APIs to be replaced with intrinsics, without
/// breaking code that was, explicitly or implicitly, creating `fn` pointers.
// FIXME(eddyb) intrinsics shouldn't use pseudo-ABIs, but rather the Rust ABI
// and some other way to indicate that they are intrinsics (e.g. new attributes).
fn prepare_fn_sig_for_reify<'tcx>(mut sig: ty::FnSig<'tcx>) -> ty::FnSig<'tcx> {
if matches!(sig.abi, Abi::RustIntrinsic) {
sig.abi = Abi::Rust;
}

sig
}

/// Coercing a mutable reference to an immutable works, while
/// coercing `&T` to `&mut T` should be forbidden.
fn coerce_mutbls<'tcx>(
Expand Down Expand Up @@ -843,12 +860,9 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
match b.kind() {
ty::FnPtr(b_sig) => {
let a_sig = a.fn_sig(self.tcx);
// NOTE(eddyb) see comment on `prepare_fn_sig_for_reify`.
let a_sig = a_sig.map_bound(prepare_fn_sig_for_reify);
if let ty::FnDef(def_id, _) = *a.kind() {
// Intrinsics are not coercible to function pointers
if self.tcx.intrinsic(def_id).is_some() {
return Err(TypeError::IntrinsicCast);
}

// Safe `#[target_feature]` functions are not assignable to safe fn pointers (RFC 2396).

if b_sig.safety() == hir::Safety::Safe
Expand Down Expand Up @@ -1150,10 +1164,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
};
if let (Some(a_sig), Some(b_sig)) = (a_sig, b_sig) {
// Intrinsics are not coercible to function pointers.
if a_sig.abi() == Abi::RustIntrinsic || b_sig.abi() == Abi::RustIntrinsic {
return Err(TypeError::IntrinsicCast);
}
// NOTE(eddyb) see comment on `prepare_fn_sig_for_reify`.
let a_sig = a_sig.map_bound(prepare_fn_sig_for_reify);
let b_sig = b_sig.map_bound(prepare_fn_sig_for_reify);
// The signature must match.
let (a_sig, b_sig) = self.normalize(new.span, (a_sig, b_sig));
let sig = self
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2039,7 +2039,6 @@ impl<'tcx> ObligationCause<'tcx> {
{
FailureCode::Error0644
}
TypeError::IntrinsicCast => FailureCode::Error0308,
_ => FailureCode::Error0308,
},
}
Expand Down Expand Up @@ -2105,9 +2104,6 @@ impl<'tcx> ObligationCause<'tcx> {
{
ObligationCauseFailureCode::ClosureSelfref { span }
}
TypeError::IntrinsicCast => {
ObligationCauseFailureCode::CantCoerce { span, subdiags }
}
_ => ObligationCauseFailureCode::Generic { span, subdiags },
},
}
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ impl<'tcx> TypeError<'tcx> {
TypeError::ConstMismatch(ref values) => {
format!("expected `{}`, found `{}`", values.expected, values.found).into()
}
TypeError::IntrinsicCast => "cannot coerce intrinsics to function pointers".into(),
TypeError::TargetFeatureCast(_) => {
"cannot coerce functions with `#[target_feature]` to safe function pointers".into()
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,10 @@ impl<'tcx> Instance<'tcx> {
// unresolved instance.
resolved = Instance { def: InstanceKind::ReifyShim(def_id, reason), args }
}
InstanceKind::Intrinsic(def_id) => {
debug!(" => fn pointer created for intrinsic call");
resolved.def = InstanceKind::ReifyShim(def_id, reason);
}
_ => {}
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_transform/src/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::iter;

use crate::{
abort_unwinding_calls, add_call_guards, add_moves_for_packed_drops, deref_separator,
mentioned_items, pass_manager as pm, remove_noop_landing_pads, simplify,
lower_intrinsics, mentioned_items, pass_manager as pm, remove_noop_landing_pads, simplify,
};
use rustc_middle::mir::patch::MirPatch;
use rustc_mir_dataflow::elaborate_drops::{self, DropElaborator, DropFlagMode, DropStyle};
Expand Down Expand Up @@ -153,6 +153,7 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceKind<'tcx>) -> Body<
&add_moves_for_packed_drops::AddMovesForPackedDrops,
&deref_separator::Derefer,
&remove_noop_landing_pads::RemoveNoopLandingPads,
&lower_intrinsics::LowerIntrinsics,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this added?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I know why. I wonder if it's better to place this in build_call_shim, or even make our own function like build_reified_intrinsic.

&simplify::SimplifyCfg::MakeShim,
&abort_unwinding_calls::AbortUnwindingCalls,
&add_call_guards::CriticalCallEdges,
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_type_ir/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ pub enum TypeError<I: Interner> {
ExistentialMismatch(ExpectedFound<I::BoundExistentialPredicates>),
ConstMismatch(ExpectedFound<I::Const>),

IntrinsicCast,
/// Safe `#[target_feature]` functions are not assignable to safe function pointers.
TargetFeatureCast(I::DefId),
}
Expand Down Expand Up @@ -93,8 +92,7 @@ impl<I: Interner> TypeError<I> {
| Traits(_)
| ProjectionMismatched(_)
| ExistentialMismatch(_)
| ConstMismatch(_)
| IntrinsicCast => true,
| ConstMismatch(_) => true,
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
- // MIR for `discriminant_value` before LowerIntrinsics
+ // MIR for `discriminant_value` after LowerIntrinsics

fn discriminant_value(_1: &T) -> <T as DiscriminantKind>::Discriminant {
let mut _0: <T as std::marker::DiscriminantKind>::Discriminant; // return place in scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL

bb0: {
- _0 = discriminant_value::<T>(move _1) -> bb1; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // mir::Constant
- // + span: $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // + literal: Const { ty: for<'r> extern "rust-intrinsic" fn(&'r T) -> <T as std::marker::DiscriminantKind>::Discriminant {std::intrinsics::discriminant_value::<T>}, val: Value(Scalar(<ZST>)) }
+ _0 = discriminant((*_1)); // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
+ goto -> bb1; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb1: {
return; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb2 (cleanup): {
resume; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
- // MIR for `std::intrinsics::forget` before LowerIntrinsics
+ // MIR for `std::intrinsics::forget` after LowerIntrinsics

fn std::intrinsics::forget(_1: T) -> () {
let mut _0: (); // return place in scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL

bb0: {
- _0 = std::intrinsics::forget::<T>(move _1) -> bb1; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // mir::Constant
- // + span: $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // + literal: Const { ty: extern "rust-intrinsic" fn(T) {std::intrinsics::forget::<T>}, val: Value(Scalar(<ZST>)) }
+ _0 = const (); // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
+ goto -> bb1; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb1: {
return; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb2 (cleanup): {
resume; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
- // MIR for `std::intrinsics::size_of` before LowerIntrinsics
+ // MIR for `std::intrinsics::size_of` after LowerIntrinsics

fn std::intrinsics::size_of() -> usize {
let mut _0: usize; // return place in scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL

bb0: {
- _0 = std::intrinsics::size_of::<T>() -> bb1; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // mir::Constant
- // + span: $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // + literal: Const { ty: extern "rust-intrinsic" fn() -> usize {std::intrinsics::size_of::<T>}, val: Value(Scalar(<ZST>)) }
+ _0 = SizeOf(T); // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
+ goto -> bb1; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb1: {
return; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb2 (cleanup): {
resume; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
- // MIR for `std::intrinsics::unreachable` before LowerIntrinsics
+ // MIR for `std::intrinsics::unreachable` after LowerIntrinsics

fn std::intrinsics::unreachable() -> ! {
let mut _0: !; // return place in scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL

bb0: {
- _0 = std::intrinsics::unreachable() -> bb1; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // mir::Constant
- // + span: $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn() -> ! {std::intrinsics::unreachable}, val: Value(Scalar(<ZST>)) }
+ unreachable; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb1: {
return; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb2 (cleanup): {
resume; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
- // MIR for `wrapping_add` before LowerIntrinsics
+ // MIR for `wrapping_add` after LowerIntrinsics

fn wrapping_add(_1: T, _2: T) -> T {
let mut _0: T; // return place in scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL

bb0: {
- _0 = wrapping_add::<T>(move _1, move _2) -> bb1; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // mir::Constant
- // + span: $SRC_DIR/core/src/intrinsics.rs:LL:COL
- // + literal: Const { ty: extern "rust-intrinsic" fn(T, T) -> T {std::intrinsics::wrapping_add::<T>}, val: Value(Scalar(<ZST>)) }
+ _0 = Add(move _1, move _2); // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
+ goto -> bb1; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb1: {
return; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}

bb2 (cleanup): {
resume; // scope 0 at $SRC_DIR/core/src/intrinsics.rs:LL:COL
}
}
17 changes: 17 additions & 0 deletions tests/mir-opt/lower_intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,20 @@ pub fn get_metadata(a: *const i32, b: *const [u8], c: *const dyn std::fmt::Debug
let _usize = ptr_metadata(b);
let _vtable = ptr_metadata(c);
}

// Check that the MIR shims used for reifying intrinsics to `fn` pointers,
// also go through the lowering pass.
pub fn reify_intrinsics() -> impl Copy {
(
// EMIT_MIR core.intrinsics-#1-wrapping_add.LowerIntrinsics.diff
core::intrinsics::wrapping_add::<u32> as unsafe fn(_, _) -> _,
// EMIT_MIR core.intrinsics-#1-size_of.LowerIntrinsics.diff
core::intrinsics::size_of::<u8> as unsafe fn() -> _,
// EMIT_MIR core.intrinsics-#1-unreachable.LowerIntrinsics.diff
core::intrinsics::unreachable as unsafe fn() -> !,
// EMIT_MIR core.intrinsics-#1-forget.LowerIntrinsics.diff
core::intrinsics::forget::<E> as unsafe fn(_),
// EMIT_MIR core.intrinsics-#1-discriminant_value.LowerIntrinsics.diff
core::intrinsics::discriminant_value::<E> as unsafe fn(_) -> _,
)
}
38 changes: 27 additions & 11 deletions tests/ui/intrinsics/reify-intrinsic.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,38 @@
//@ check-fail
//@ run-pass

#![feature(core_intrinsics, intrinsics)]

fn a() {
let _: unsafe extern "rust-intrinsic" fn(isize) -> usize = std::mem::transmute;
//~^ ERROR cannot coerce
// NOTE(eddyb) `#[inline(never)]` and returning `fn` pointers from functions is
// done to force codegen (of the reification-to-`fn`-ptr shims around intrinsics).

#[inline(never)]
fn a() -> unsafe fn(isize) -> usize {
let f: unsafe fn(isize) -> usize = std::mem::transmute;
f
}

fn b() {
let _ = std::mem::transmute as unsafe extern "rust-intrinsic" fn(isize) -> usize;
//~^ ERROR casting
#[inline(never)]
fn b() -> unsafe fn(isize) -> usize {
let f = std::mem::transmute as unsafe fn(isize) -> usize;
f
}

fn c() {
let _: [unsafe extern "rust-intrinsic" fn(bool) -> bool; 2] = [
std::intrinsics::likely, //~ ERROR cannot coerce
#[inline(never)]
fn c() -> [fn(bool) -> bool; 2] {
let fs = [
std::intrinsics::likely,
std::intrinsics::unlikely,
];
fs
}

fn main() {}
fn main() {
unsafe {
assert_eq!(a()(-1), !0);
assert_eq!(b()(-1), !0);
}

let [likely_ptr, unlikely_ptr] = c();
assert!(likely_ptr(true));
assert!(unlikely_ptr(true));
}
30 changes: 0 additions & 30 deletions tests/ui/intrinsics/reify-intrinsic.stderr

This file was deleted.

Loading
Loading