From 3674032eb21d145992f1b8374e4ab201d606fbd9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 5 Apr 2024 15:12:34 -0400 Subject: [PATCH 01/14] Rework the ByMoveBody shim to actually work correctly --- .../src/coroutine/by_move_body.rs | 128 ++++++++++---- .../precise-captures.call.run.stdout | 29 ++++ .../precise-captures.call_once.run.stdout | 29 ++++ .../precise-captures.force_once.run.stdout | 29 ++++ .../async-closures/precise-captures.rs | 157 ++++++++++++++++++ 5 files changed, 336 insertions(+), 36 deletions(-) create mode 100644 tests/ui/async-await/async-closures/precise-captures.call.run.stdout create mode 100644 tests/ui/async-await/async-closures/precise-captures.call_once.run.stdout create mode 100644 tests/ui/async-await/async-closures/precise-captures.force_once.run.stdout create mode 100644 tests/ui/async-await/async-closures/precise-captures.rs diff --git a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs index de43f9faff9..1fb2c80dd40 100644 --- a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs +++ b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs @@ -60,14 +60,13 @@ //! with it in lockstep. When we need to resolve a body for `FnOnce` or `AsyncFnOnce`, //! we use this "by move" body instead. -use itertools::Itertools; - -use rustc_data_structures::unord::UnordSet; +use rustc_data_structures::unord::UnordMap; use rustc_hir as hir; +use rustc_middle::hir::place::{Projection, ProjectionKind}; use rustc_middle::mir::visit::MutVisitor; use rustc_middle::mir::{self, dump_mir, MirPass}; use rustc_middle::ty::{self, InstanceDef, Ty, TyCtxt, TypeVisitableExt}; -use rustc_target::abi::FieldIdx; +use rustc_target::abi::{FieldIdx, VariantIdx}; pub struct ByMoveBody; @@ -116,32 +115,76 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody { .tuple_fields() .len(); - let mut by_ref_fields = UnordSet::default(); - for (idx, (coroutine_capture, parent_capture)) in tcx + let mut field_remapping = UnordMap::default(); + + let mut parent_captures = + tcx.closure_captures(parent_def_id).iter().copied().enumerate().peekable(); + + for (child_field_idx, child_capture) in tcx .closure_captures(coroutine_def_id) .iter() + .copied() // By construction we capture all the args first. .skip(num_args) - .zip_eq(tcx.closure_captures(parent_def_id)) .enumerate() { - // This upvar is captured by-move from the parent closure, but by-ref - // from the inner async block. That means that it's being borrowed from - // the outer closure body -- we need to change the coroutine to take the - // upvar by value. - if coroutine_capture.is_by_ref() && !parent_capture.is_by_ref() { - assert_ne!( - coroutine_kind, - ty::ClosureKind::FnOnce, - "`FnOnce` coroutine-closures return coroutines that capture from \ - their body; it will always result in a borrowck error!" - ); - by_ref_fields.insert(FieldIdx::from_usize(num_args + idx)); - } + loop { + let Some(&(parent_field_idx, parent_capture)) = parent_captures.peek() else { + bug!("we ran out of parent captures!") + }; - // Make sure we're actually talking about the same capture. - // FIXME(async_closures): We could look at the `hir::Upvar` instead? - assert_eq!(coroutine_capture.place.ty(), parent_capture.place.ty()); + if !std::iter::zip( + &child_capture.place.projections, + &parent_capture.place.projections, + ) + .all(|(child, parent)| child.kind == parent.kind) + { + // Skip this field. + let _ = parent_captures.next().unwrap(); + continue; + } + + let child_precise_captures = + &child_capture.place.projections[parent_capture.place.projections.len()..]; + + let needs_deref = child_capture.is_by_ref() && !parent_capture.is_by_ref(); + if needs_deref { + assert_ne!( + coroutine_kind, + ty::ClosureKind::FnOnce, + "`FnOnce` coroutine-closures return coroutines that capture from \ + their body; it will always result in a borrowck error!" + ); + } + + let mut parent_capture_ty = parent_capture.place.ty(); + parent_capture_ty = match parent_capture.info.capture_kind { + ty::UpvarCapture::ByValue => parent_capture_ty, + ty::UpvarCapture::ByRef(kind) => Ty::new_ref( + tcx, + tcx.lifetimes.re_erased, + parent_capture_ty, + kind.to_mutbl_lossy(), + ), + }; + + field_remapping.insert( + FieldIdx::from_usize(child_field_idx + num_args), + ( + FieldIdx::from_usize(parent_field_idx + num_args), + parent_capture_ty, + needs_deref, + child_precise_captures, + ), + ); + + break; + } + } + + if coroutine_kind == ty::ClosureKind::FnOnce { + assert_eq!(field_remapping.len(), tcx.closure_captures(parent_def_id).len()); + return; } let by_move_coroutine_ty = tcx @@ -157,7 +200,7 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody { ); let mut by_move_body = body.clone(); - MakeByMoveBody { tcx, by_ref_fields, by_move_coroutine_ty }.visit_body(&mut by_move_body); + MakeByMoveBody { tcx, field_remapping, by_move_coroutine_ty }.visit_body(&mut by_move_body); dump_mir(tcx, false, "coroutine_by_move", &0, &by_move_body, |_, _| Ok(())); by_move_body.source = mir::MirSource::from_instance(InstanceDef::CoroutineKindShim { coroutine_def_id: coroutine_def_id.to_def_id(), @@ -168,7 +211,7 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody { struct MakeByMoveBody<'tcx> { tcx: TyCtxt<'tcx>, - by_ref_fields: UnordSet, + field_remapping: UnordMap, bool, &'tcx [Projection<'tcx>])>, by_move_coroutine_ty: Ty<'tcx>, } @@ -184,23 +227,36 @@ impl<'tcx> MutVisitor<'tcx> for MakeByMoveBody<'tcx> { location: mir::Location, ) { if place.local == ty::CAPTURE_STRUCT_LOCAL - && let Some((&mir::ProjectionElem::Field(idx, ty), projection)) = + && let Some((&mir::ProjectionElem::Field(idx, _), projection)) = place.projection.split_first() - && self.by_ref_fields.contains(&idx) + && let Some(&(remapped_idx, remapped_ty, needs_deref, additional_projections)) = + self.field_remapping.get(&idx) { - let (begin, end) = projection.split_first().unwrap(); - // FIXME(async_closures): I'm actually a bit surprised to see that we always - // initially deref the by-ref upvars. If this is not actually true, then we - // will at least get an ICE that explains why this isn't true :^) - assert_eq!(*begin, mir::ProjectionElem::Deref); - // Peel one ref off of the ty. - let peeled_ty = ty.builtin_deref(true).unwrap().ty; + let final_deref = if needs_deref { + let Some((mir::ProjectionElem::Deref, rest)) = projection.split_first() else { + bug!(); + }; + rest + } else { + projection + }; + + let additional_projections = + additional_projections.iter().map(|elem| match elem.kind { + ProjectionKind::Deref => mir::ProjectionElem::Deref, + ProjectionKind::Field(idx, VariantIdx::ZERO) => { + mir::ProjectionElem::Field(idx, elem.ty) + } + _ => unreachable!("precise captures only through fields and derefs"), + }); + *place = mir::Place { local: place.local, projection: self.tcx.mk_place_elems_from_iter( - [mir::ProjectionElem::Field(idx, peeled_ty)] + [mir::ProjectionElem::Field(remapped_idx, remapped_ty)] .into_iter() - .chain(end.iter().copied()), + .chain(additional_projections) + .chain(final_deref.iter().copied()), ), }; } diff --git a/tests/ui/async-await/async-closures/precise-captures.call.run.stdout b/tests/ui/async-await/async-closures/precise-captures.call.run.stdout new file mode 100644 index 00000000000..6062556837c --- /dev/null +++ b/tests/ui/async-await/async-closures/precise-captures.call.run.stdout @@ -0,0 +1,29 @@ +after call +after await +fixed +uncaptured + +after call +after await +fixed +uncaptured + +after call +after await +fixed +uncaptured + +after call +after await +fixed +untouched + +after call +drop first +after await +uncaptured + +after call +drop first +after await +uncaptured diff --git a/tests/ui/async-await/async-closures/precise-captures.call_once.run.stdout b/tests/ui/async-await/async-closures/precise-captures.call_once.run.stdout new file mode 100644 index 00000000000..ddb02d47600 --- /dev/null +++ b/tests/ui/async-await/async-closures/precise-captures.call_once.run.stdout @@ -0,0 +1,29 @@ +after call +after await +fixed +uncaptured + +after call +after await +fixed +uncaptured + +after call +fixed +after await +uncaptured + +after call +after await +fixed +untouched + +after call +drop first +after await +uncaptured + +after call +drop first +after await +uncaptured diff --git a/tests/ui/async-await/async-closures/precise-captures.force_once.run.stdout b/tests/ui/async-await/async-closures/precise-captures.force_once.run.stdout new file mode 100644 index 00000000000..ddb02d47600 --- /dev/null +++ b/tests/ui/async-await/async-closures/precise-captures.force_once.run.stdout @@ -0,0 +1,29 @@ +after call +after await +fixed +uncaptured + +after call +after await +fixed +uncaptured + +after call +fixed +after await +uncaptured + +after call +after await +fixed +untouched + +after call +drop first +after await +uncaptured + +after call +drop first +after await +uncaptured diff --git a/tests/ui/async-await/async-closures/precise-captures.rs b/tests/ui/async-await/async-closures/precise-captures.rs new file mode 100644 index 00000000000..e82dd1dbaf0 --- /dev/null +++ b/tests/ui/async-await/async-closures/precise-captures.rs @@ -0,0 +1,157 @@ +//@ aux-build:block-on.rs +//@ edition:2021 +//@ run-pass +//@ check-run-results +//@ revisions: call call_once force_once + +// call - Call the closure regularly. +// call_once - Call the closure w/ `async FnOnce`, so exercising the by_move shim. +// force_once - Force the closure mode to `FnOnce`, so exercising what was fixed +// in . + +#![feature(async_closure)] +#![allow(unused_mut)] + +extern crate block_on; + +#[cfg(any(call, force_once))] +macro_rules! call { + ($c:expr) => { ($c)() } +} + +#[cfg(call_once)] +async fn call_once(f: impl async FnOnce()) { + f().await +} + +#[cfg(call_once)] +macro_rules! call { + ($c:expr) => { call_once($c) } +} + +#[cfg(not(force_once))] +macro_rules! guidance { + ($c:expr) => { $c } +} + +#[cfg(force_once)] +fn infer_fnonce(c: impl async FnOnce()) -> impl async FnOnce() { c } + +#[cfg(force_once)] +macro_rules! guidance { + ($c:expr) => { infer_fnonce($c) } +} + +#[derive(Debug)] +struct Drop(&'static str); + +impl std::ops::Drop for Drop { + fn drop(&mut self) { + println!("{}", self.0); + } +} + +struct S { + a: i32, + b: Drop, + c: Drop, +} + +async fn async_main() { + // Precise capture struct + { + let mut s = S { a: 1, b: Drop("fix me up"), c: Drop("untouched") }; + let mut c = guidance!(async || { + s.a = 2; + let w = &mut s.b; + w.0 = "fixed"; + }); + s.c.0 = "uncaptured"; + let fut = call!(c); + println!("after call"); + fut.await; + println!("after await"); + } + println!(); + + // Precise capture &mut struct + { + let s = &mut S { a: 1, b: Drop("fix me up"), c: Drop("untouched") }; + let mut c = guidance!(async || { + s.a = 2; + let w = &mut s.b; + w.0 = "fixed"; + }); + s.c.0 = "uncaptured"; + let fut = call!(c); + println!("after call"); + fut.await; + println!("after await"); + } + println!(); + + // Precise capture struct by move + { + let mut s = S { a: 1, b: Drop("fix me up"), c: Drop("untouched") }; + let mut c = guidance!(async move || { + s.a = 2; + let w = &mut s.b; + w.0 = "fixed"; + }); + s.c.0 = "uncaptured"; + let fut = call!(c); + println!("after call"); + fut.await; + println!("after await"); + } + println!(); + + // Precise capture &mut struct by move + { + let s = &mut S { a: 1, b: Drop("fix me up"), c: Drop("untouched") }; + let mut c = guidance!(async move || { + s.a = 2; + let w = &mut s.b; + w.0 = "fixed"; + }); + // `s` is still captured fully as `&mut S`. + let fut = call!(c); + println!("after call"); + fut.await; + println!("after await"); + } + println!(); + + // Precise capture struct, consume field + { + let mut s = S { a: 1, b: Drop("drop first"), c: Drop("untouched") }; + let c = guidance!(async move || { + // s.a = 2; // FIXME(async_closures): Figure out why this fails + drop(s.b); + }); + s.c.0 = "uncaptured"; + let fut = call!(c); + println!("after call"); + fut.await; + println!("after await"); + } + println!(); + + // Precise capture struct by move, consume field + { + let mut s = S { a: 1, b: Drop("drop first"), c: Drop("untouched") }; + let c = guidance!(async move || { + // s.a = 2; // FIXME(async_closures): Figure out why this fails + drop(s.b); + }); + s.c.0 = "uncaptured"; + let fut = call!(c); + println!("after call"); + fut.await; + println!("after await"); + } +} + +fn main() { + block_on::block_on(async_main()); +} From 0f13bd436b8168efd5055d66b2a1aa6cae37fabb Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 5 Apr 2024 16:29:49 -0400 Subject: [PATCH 02/14] Add some helpful comments --- .../src/coroutine/by_move_body.rs | 60 +++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs index 1fb2c80dd40..a62fe4af810 100644 --- a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs +++ b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs @@ -58,7 +58,16 @@ //! borrowing from the outer closure, and we simply peel off a `deref` projection //! from them. This second body is stored alongside the first body, and optimized //! with it in lockstep. When we need to resolve a body for `FnOnce` or `AsyncFnOnce`, -//! we use this "by move" body instead. +//! we use this "by-move" body instead. +//! +//! ## How does this work? +//! +//! This pass essentially remaps the body of the (child) closure of the coroutine-closure +//! to take the set of upvars of the parent closure by value. This at least requires +//! changing a by-ref upvar to be by-value in the case that the outer coroutine-closure +//! captures something by value; however, it may also require renumbering field indices +//! in case precise captures (edition 2021 closure capture rules) caused the inner coroutine +//! to split one field capture into two. use rustc_data_structures::unord::UnordMap; use rustc_hir as hir; @@ -117,8 +126,15 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody { let mut field_remapping = UnordMap::default(); + // One parent capture may correspond to several child captures if we end up + // refining the set of captures via edition-2021 precise captures. We want to + // match up any number of child captures with one parent capture, so we keep + // peeking off this `Peekable` until the child doesn't match anymore. let mut parent_captures = tcx.closure_captures(parent_def_id).iter().copied().enumerate().peekable(); + // Make sure we use every field at least once, b/c why are we capturing something + // if it's not used in the inner coroutine. + let mut field_used_at_least_once = false; for (child_field_idx, child_capture) in tcx .closure_captures(coroutine_def_id) @@ -133,20 +149,36 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody { bug!("we ran out of parent captures!") }; + assert!( + child_capture.place.projections.len() >= parent_capture.place.projections.len() + ); + // A parent matches a child they share the same prefix of projections. + // The child may have more, if it is capturing sub-fields out of + // something that is captured by-move in the parent closure. if !std::iter::zip( &child_capture.place.projections, &parent_capture.place.projections, ) .all(|(child, parent)| child.kind == parent.kind) { + // Make sure the field was used at least once. + assert!( + field_used_at_least_once, + "we captured {parent_capture:#?} but it was not used in the child coroutine?" + ); + field_used_at_least_once = false; // Skip this field. let _ = parent_captures.next().unwrap(); continue; } + // Store this set of additional projections (fields and derefs). + // We need to re-apply them later. let child_precise_captures = &child_capture.place.projections[parent_capture.place.projections.len()..]; + // If the parent captures by-move, and the child captures by-ref, then we + // need to peel an additional `deref` off of the body of the child. let needs_deref = child_capture.is_by_ref() && !parent_capture.is_by_ref(); if needs_deref { assert_ne!( @@ -157,6 +189,8 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody { ); } + // Finally, store the type of the parent's captured place. We need + // this when building the field projection in the MIR body later on. let mut parent_capture_ty = parent_capture.place.ty(); parent_capture_ty = match parent_capture.info.capture_kind { ty::UpvarCapture::ByValue => parent_capture_ty, @@ -178,6 +212,7 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody { ), ); + field_used_at_least_once = true; break; } } @@ -226,21 +261,34 @@ impl<'tcx> MutVisitor<'tcx> for MakeByMoveBody<'tcx> { context: mir::visit::PlaceContext, location: mir::Location, ) { + // Initializing an upvar local always starts with `CAPTURE_STRUCT_LOCAL` and a + // field projection. If this is in `field_remapping`, then it must not be an + // arg from calling the closure, but instead an upvar. if place.local == ty::CAPTURE_STRUCT_LOCAL && let Some((&mir::ProjectionElem::Field(idx, _), projection)) = place.projection.split_first() && let Some(&(remapped_idx, remapped_ty, needs_deref, additional_projections)) = self.field_remapping.get(&idx) { + // As noted before, if the parent closure captures a field by value, and + // the child captures a field by ref, then for the by-move body we're + // generating, we also are taking that field by value. Peel off a deref, + // since a layer of reffing has now become redundant. let final_deref = if needs_deref { - let Some((mir::ProjectionElem::Deref, rest)) = projection.split_first() else { - bug!(); + let [mir::ProjectionElem::Deref] = projection else { + bug!("There should only be a single deref for an upvar local initialization"); }; - rest + &[] } else { projection }; + // The only thing that should be left is a deref, if the parent captured + // an upvar by-ref. + std::assert_matches::assert_matches!(final_deref, [] | [mir::ProjectionElem::Deref]); + + // For all of the additional projections that come out of precise capturing, + // re-apply these projections. let additional_projections = additional_projections.iter().map(|elem| match elem.kind { ProjectionKind::Deref => mir::ProjectionElem::Deref, @@ -250,6 +298,10 @@ impl<'tcx> MutVisitor<'tcx> for MakeByMoveBody<'tcx> { _ => unreachable!("precise captures only through fields and derefs"), }); + // We start out with an adjusted field index (and ty), representing the + // upvar that we get from our parent closure. We apply any of the additional + // projections to make sure that to the rest of the body of the closure, the + // place looks the same, and then apply that final deref if necessary. *place = mir::Place { local: place.local, projection: self.tcx.mk_place_elems_from_iter( From 49c4ebcc409b1537fb0cf99134f5166481096c5f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 5 Apr 2024 16:48:12 -0400 Subject: [PATCH 03/14] Check the base of the place too! --- .../src/coroutine/by_move_body.rs | 26 +++++++++++++----- .../async-closures/overlapping-projs.rs | 27 +++++++++++++++++++ .../overlapping-projs.run.stdout | 1 + 3 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 tests/ui/async-await/async-closures/overlapping-projs.rs create mode 100644 tests/ui/async-await/async-closures/overlapping-projs.run.stdout diff --git a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs index a62fe4af810..d94441b1413 100644 --- a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs +++ b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs @@ -71,7 +71,7 @@ use rustc_data_structures::unord::UnordMap; use rustc_hir as hir; -use rustc_middle::hir::place::{Projection, ProjectionKind}; +use rustc_middle::hir::place::{PlaceBase, Projection, ProjectionKind}; use rustc_middle::mir::visit::MutVisitor; use rustc_middle::mir::{self, dump_mir, MirPass}; use rustc_middle::ty::{self, InstanceDef, Ty, TyCtxt, TypeVisitableExt}; @@ -149,17 +149,25 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody { bug!("we ran out of parent captures!") }; + let PlaceBase::Upvar(parent_base) = parent_capture.place.base else { + bug!("expected capture to be an upvar"); + }; + let PlaceBase::Upvar(child_base) = child_capture.place.base else { + bug!("expected capture to be an upvar"); + }; + assert!( child_capture.place.projections.len() >= parent_capture.place.projections.len() ); // A parent matches a child they share the same prefix of projections. // The child may have more, if it is capturing sub-fields out of // something that is captured by-move in the parent closure. - if !std::iter::zip( - &child_capture.place.projections, - &parent_capture.place.projections, - ) - .all(|(child, parent)| child.kind == parent.kind) + if parent_base.var_path.hir_id != child_base.var_path.hir_id + || !std::iter::zip( + &child_capture.place.projections, + &parent_capture.place.projections, + ) + .all(|(child, parent)| child.kind == parent.kind) { // Make sure the field was used at least once. assert!( @@ -217,6 +225,12 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody { } } + // Pop the last parent capture + if field_used_at_least_once { + let _ = parent_captures.next().unwrap(); + } + assert_eq!(parent_captures.next(), None, "leftover parent captures?"); + if coroutine_kind == ty::ClosureKind::FnOnce { assert_eq!(field_remapping.len(), tcx.closure_captures(parent_def_id).len()); return; diff --git a/tests/ui/async-await/async-closures/overlapping-projs.rs b/tests/ui/async-await/async-closures/overlapping-projs.rs new file mode 100644 index 00000000000..6dd00b16103 --- /dev/null +++ b/tests/ui/async-await/async-closures/overlapping-projs.rs @@ -0,0 +1,27 @@ +//@ aux-build:block-on.rs +//@ edition:2021 +//@ run-pass +//@ check-run-results + +#![feature(async_closure)] + +extern crate block_on; + +async fn call_once(f: impl async FnOnce()) { + f().await; +} + +async fn async_main() { + let x = &mut 0; + let y = &mut 0; + let c = async || { + *x = 1; + *y = 2; + }; + call_once(c).await; + println!("{x} {y}"); +} + +fn main() { + block_on::block_on(async_main()); +} diff --git a/tests/ui/async-await/async-closures/overlapping-projs.run.stdout b/tests/ui/async-await/async-closures/overlapping-projs.run.stdout new file mode 100644 index 00000000000..8d04f961a03 --- /dev/null +++ b/tests/ui/async-await/async-closures/overlapping-projs.run.stdout @@ -0,0 +1 @@ +1 2 From ad0fcac72b4fbd5a6558fa1d440882156eafae33 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 5 Apr 2024 17:34:57 -0400 Subject: [PATCH 04/14] Account for an additional reborrow inserted by UniqueImmBorrow and MutBorrow --- .../src/coroutine/by_move_body.rs | 11 +++++--- .../async-closures/mut-ref-reborrow.rs | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 tests/ui/async-await/async-closures/mut-ref-reborrow.rs diff --git a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs index d94441b1413..320d8fd3977 100644 --- a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs +++ b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs @@ -289,10 +289,15 @@ impl<'tcx> MutVisitor<'tcx> for MakeByMoveBody<'tcx> { // generating, we also are taking that field by value. Peel off a deref, // since a layer of reffing has now become redundant. let final_deref = if needs_deref { - let [mir::ProjectionElem::Deref] = projection else { - bug!("There should only be a single deref for an upvar local initialization"); + let Some((mir::ProjectionElem::Deref, projection)) = projection.split_first() + else { + bug!( + "There should be at least a single deref for an upvar local initialization, found {projection:#?}" + ); }; - &[] + // There may be more derefs, since we may also implicitly reborrow + // a captured mut pointer. + projection } else { projection }; diff --git a/tests/ui/async-await/async-closures/mut-ref-reborrow.rs b/tests/ui/async-await/async-closures/mut-ref-reborrow.rs new file mode 100644 index 00000000000..9f2cbd7ce1c --- /dev/null +++ b/tests/ui/async-await/async-closures/mut-ref-reborrow.rs @@ -0,0 +1,27 @@ +//@ aux-build:block-on.rs +//@ run-pass +//@ check-run-results +//@ revisions: e2021 e2018 +//@[e2018] edition:2018 +//@[e2021] edition:2021 + +#![feature(async_closure)] + +extern crate block_on; + +async fn call_once(f: impl async FnOnce()) { f().await; } + +pub async fn async_closure(x: &mut i32) { + let c = async move || { + *x += 1; + }; + call_once(c).await; +} + +fn main() { + block_on::block_on(async { + let mut x = 0; + async_closure(&mut x).await; + assert_eq!(x, 1); + }); +} From e0af5eae3615c0aa9dd4b33ae6f9e1fd8e4e03b3 Mon Sep 17 00:00:00 2001 From: klensy Date: Sat, 6 Apr 2024 13:58:52 +0300 Subject: [PATCH 05/14] bootstrap: remove unused pub fns --- src/bootstrap/src/core/builder.rs | 21 --------------------- src/bootstrap/src/lib.rs | 9 --------- 2 files changed, 30 deletions(-) diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs index 9555be481e6..40f58b25e6f 100644 --- a/src/bootstrap/src/core/builder.rs +++ b/src/bootstrap/src/core/builder.rs @@ -645,27 +645,6 @@ pub enum Kind { } impl Kind { - pub fn parse(string: &str) -> Option { - // these strings, including the one-letter aliases, must match the x.py help text - Some(match string { - "build" | "b" => Kind::Build, - "check" | "c" => Kind::Check, - "clippy" => Kind::Clippy, - "fix" => Kind::Fix, - "fmt" => Kind::Format, - "test" | "t" => Kind::Test, - "bench" => Kind::Bench, - "doc" | "d" => Kind::Doc, - "clean" => Kind::Clean, - "dist" => Kind::Dist, - "install" => Kind::Install, - "run" | "r" => Kind::Run, - "setup" => Kind::Setup, - "suggest" => Kind::Suggest, - _ => return None, - }) - } - pub fn as_str(&self) -> &'static str { match self { Kind::Build => "build", diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index bcb8260b15a..1a8322c0dfd 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -1911,15 +1911,6 @@ impl Compiler { pub fn is_snapshot(&self, build: &Build) -> bool { self.stage == 0 && self.host == build.build } - - /// Returns if this compiler should be treated as a final stage one in the - /// current build session. - /// This takes into account whether we're performing a full bootstrap or - /// not; don't directly compare the stage with `2`! - pub fn is_final_stage(&self, build: &Build) -> bool { - let final_stage = if build.config.full_bootstrap { 2 } else { 1 }; - self.stage >= final_stage - } } fn envify(s: &str) -> String { From 00bd24766ff31f4234c72677f978e198ff8cac82 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 6 Apr 2024 11:37:57 -0700 Subject: [PATCH 06/14] Don't emit divide-by-zero panic paths in `StepBy::len` I happened to notice today that there's actually two such calls emitted in the assembly: Since they're impossible, hopefully telling LLVM that will also help optimizations elsewhere. --- library/core/src/iter/adapters/step_by.rs | 80 ++++++++++++++--------- tests/codegen/step_by-overflow-checks.rs | 26 ++++++++ 2 files changed, 75 insertions(+), 31 deletions(-) create mode 100644 tests/codegen/step_by-overflow-checks.rs diff --git a/library/core/src/iter/adapters/step_by.rs b/library/core/src/iter/adapters/step_by.rs index b8b96417d13..616dd0afc51 100644 --- a/library/core/src/iter/adapters/step_by.rs +++ b/library/core/src/iter/adapters/step_by.rs @@ -1,6 +1,7 @@ use crate::{ intrinsics, iter::{from_fn, TrustedLen, TrustedRandomAccess}, + num::NonZeroUsize, ops::{Range, Try}, }; @@ -22,7 +23,11 @@ pub struct StepBy { /// Additionally this type-dependent preprocessing means specialized implementations /// cannot be used interchangeably. iter: I, - step: usize, + /// This field is `step - 1`, aka the correct amount to pass to `nth` when iterating. + /// It MUST NOT be `usize::MAX`, as `unsafe` code depends on being able to add one + /// without the risk of overflow. (This is important so that length calculations + /// don't need to check for division-by-zero, for example.) + step_minus_one: usize, first_take: bool, } @@ -31,7 +36,16 @@ impl StepBy { pub(in crate::iter) fn new(iter: I, step: usize) -> StepBy { assert!(step != 0); let iter = >::setup(iter, step); - StepBy { iter, step: step - 1, first_take: true } + StepBy { iter, step_minus_one: step - 1, first_take: true } + } + + /// The `step` that was originally passed to `Iterator::step_by(step)`, + /// aka `self.step_minus_one + 1`. + #[inline] + fn original_step(&self) -> NonZeroUsize { + // SAFETY: By type invariant, `step_minus_one` cannot be `MAX`, which + // means the addition cannot overflow and the result cannot be zero. + unsafe { NonZeroUsize::new_unchecked(intrinsics::unchecked_add(self.step_minus_one, 1)) } } } @@ -81,8 +95,8 @@ where // The zero-based index starting from the end of the iterator of the // last element. Used in the `DoubleEndedIterator` implementation. fn next_back_index(&self) -> usize { - let rem = self.iter.len() % (self.step + 1); - if self.first_take { if rem == 0 { self.step } else { rem - 1 } } else { rem } + let rem = self.iter.len() % self.original_step(); + if self.first_take { if rem == 0 { self.step_minus_one } else { rem - 1 } } else { rem } } } @@ -209,7 +223,7 @@ unsafe impl StepByImpl for StepBy { #[inline] default fn spec_next(&mut self) -> Option { - let step_size = if self.first_take { 0 } else { self.step }; + let step_size = if self.first_take { 0 } else { self.step_minus_one }; self.first_take = false; self.iter.nth(step_size) } @@ -217,22 +231,22 @@ unsafe impl StepByImpl for StepBy { #[inline] default fn spec_size_hint(&self) -> (usize, Option) { #[inline] - fn first_size(step: usize) -> impl Fn(usize) -> usize { - move |n| if n == 0 { 0 } else { 1 + (n - 1) / (step + 1) } + fn first_size(step: NonZeroUsize) -> impl Fn(usize) -> usize { + move |n| if n == 0 { 0 } else { 1 + (n - 1) / step } } #[inline] - fn other_size(step: usize) -> impl Fn(usize) -> usize { - move |n| n / (step + 1) + fn other_size(step: NonZeroUsize) -> impl Fn(usize) -> usize { + move |n| n / step } let (low, high) = self.iter.size_hint(); if self.first_take { - let f = first_size(self.step); + let f = first_size(self.original_step()); (f(low), high.map(f)) } else { - let f = other_size(self.step); + let f = other_size(self.original_step()); (f(low), high.map(f)) } } @@ -247,10 +261,9 @@ unsafe impl StepByImpl for StepBy { } n -= 1; } - // n and self.step are indices, we need to add 1 to get the amount of elements + // n and self.step_minus_one are indices, we need to add 1 to get the amount of elements // When calling `.nth`, we need to subtract 1 again to convert back to an index - // step + 1 can't overflow because `.step_by` sets `self.step` to `step - 1` - let mut step = self.step + 1; + let mut step = self.original_step().get(); // n + 1 could overflow // thus, if n is usize::MAX, instead of adding one, we call .nth(step) if n == usize::MAX { @@ -288,8 +301,11 @@ unsafe impl StepByImpl for StepBy { R: Try, { #[inline] - fn nth(iter: &mut I, step: usize) -> impl FnMut() -> Option + '_ { - move || iter.nth(step) + fn nth( + iter: &mut I, + step_minus_one: usize, + ) -> impl FnMut() -> Option + '_ { + move || iter.nth(step_minus_one) } if self.first_take { @@ -299,7 +315,7 @@ unsafe impl StepByImpl for StepBy { Some(x) => acc = f(acc, x)?, } } - from_fn(nth(&mut self.iter, self.step)).try_fold(acc, f) + from_fn(nth(&mut self.iter, self.step_minus_one)).try_fold(acc, f) } default fn spec_fold(mut self, mut acc: Acc, mut f: F) -> Acc @@ -307,8 +323,11 @@ unsafe impl StepByImpl for StepBy { F: FnMut(Acc, Self::Item) -> Acc, { #[inline] - fn nth(iter: &mut I, step: usize) -> impl FnMut() -> Option + '_ { - move || iter.nth(step) + fn nth( + iter: &mut I, + step_minus_one: usize, + ) -> impl FnMut() -> Option + '_ { + move || iter.nth(step_minus_one) } if self.first_take { @@ -318,7 +337,7 @@ unsafe impl StepByImpl for StepBy { Some(x) => acc = f(acc, x), } } - from_fn(nth(&mut self.iter, self.step)).fold(acc, f) + from_fn(nth(&mut self.iter, self.step_minus_one)).fold(acc, f) } } @@ -336,7 +355,7 @@ unsafe impl StepByBackImpl for St // is out of bounds because the length of `self.iter` does not exceed // `usize::MAX` (because `I: ExactSizeIterator`) and `nth_back` is // zero-indexed - let n = n.saturating_mul(self.step + 1).saturating_add(self.next_back_index()); + let n = n.saturating_mul(self.original_step().get()).saturating_add(self.next_back_index()); self.iter.nth_back(n) } @@ -348,16 +367,16 @@ unsafe impl StepByBackImpl for St #[inline] fn nth_back( iter: &mut I, - step: usize, + step_minus_one: usize, ) -> impl FnMut() -> Option + '_ { - move || iter.nth_back(step) + move || iter.nth_back(step_minus_one) } match self.next_back() { None => try { init }, Some(x) => { let acc = f(init, x)?; - from_fn(nth_back(&mut self.iter, self.step)).try_fold(acc, f) + from_fn(nth_back(&mut self.iter, self.step_minus_one)).try_fold(acc, f) } } } @@ -371,16 +390,16 @@ unsafe impl StepByBackImpl for St #[inline] fn nth_back( iter: &mut I, - step: usize, + step_minus_one: usize, ) -> impl FnMut() -> Option + '_ { - move || iter.nth_back(step) + move || iter.nth_back(step_minus_one) } match self.next_back() { None => init, Some(x) => { let acc = f(init, x); - from_fn(nth_back(&mut self.iter, self.step)).fold(acc, f) + from_fn(nth_back(&mut self.iter, self.step_minus_one)).fold(acc, f) } } } @@ -424,8 +443,7 @@ macro_rules! spec_int_ranges { fn spec_next(&mut self) -> Option<$t> { // if a step size larger than the type has been specified fall back to // t::MAX, in which case remaining will be at most 1. - // The `+ 1` can't overflow since the constructor substracted 1 from the original value. - let step = <$t>::try_from(self.step + 1).unwrap_or(<$t>::MAX); + let step = <$t>::try_from(self.original_step().get()).unwrap_or(<$t>::MAX); let remaining = self.iter.end; if remaining > 0 { let val = self.iter.start; @@ -474,7 +492,7 @@ macro_rules! spec_int_ranges { { // if a step size larger than the type has been specified fall back to // t::MAX, in which case remaining will be at most 1. - let step = <$t>::try_from(self.step + 1).unwrap_or(<$t>::MAX); + let step = <$t>::try_from(self.original_step().get()).unwrap_or(<$t>::MAX); let remaining = self.iter.end; let mut acc = init; let mut val = self.iter.start; @@ -500,7 +518,7 @@ macro_rules! spec_int_ranges_r { fn spec_next_back(&mut self) -> Option where Range<$t>: DoubleEndedIterator + ExactSizeIterator, { - let step = (self.step + 1) as $t; + let step = self.original_step().get() as $t; let remaining = self.iter.end; if remaining > 0 { let start = self.iter.start; diff --git a/tests/codegen/step_by-overflow-checks.rs b/tests/codegen/step_by-overflow-checks.rs new file mode 100644 index 00000000000..43e8514a8b7 --- /dev/null +++ b/tests/codegen/step_by-overflow-checks.rs @@ -0,0 +1,26 @@ +//@ compile-flags: -O + +#![crate_type = "lib"] + +use std::iter::StepBy; +use std::slice::Iter; + +// The constructor for `StepBy` ensures we can never end up needing to do zero +// checks on denominators, so check that the code isn't emitting panic paths. + +// CHECK-LABEL: @step_by_len_std +#[no_mangle] +pub fn step_by_len_std(x: &StepBy>) -> usize { + // CHECK-NOT: div_by_zero + // CHECK: udiv + // CHECK-NOT: div_by_zero + x.len() +} + +// CHECK-LABEL: @step_by_len_naive +#[no_mangle] +pub fn step_by_len_naive(x: Iter, step_minus_one: usize) -> usize { + // CHECK: udiv + // CHECK: call{{.+}}div_by_zero + x.len() / (step_minus_one + 1) +} From 54f8db84329116889fce5d58292b5cc6b83642e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 6 Apr 2024 23:25:58 +0000 Subject: [PATCH 07/14] add non-regression test for issue 123275 --- ...own-to-hold-modulo-regions-unsized-tail.rs | 244 ++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 tests/ui/traits/pred-known-to-hold-modulo-regions-unsized-tail.rs diff --git a/tests/ui/traits/pred-known-to-hold-modulo-regions-unsized-tail.rs b/tests/ui/traits/pred-known-to-hold-modulo-regions-unsized-tail.rs new file mode 100644 index 00000000000..4e8c19d600d --- /dev/null +++ b/tests/ui/traits/pred-known-to-hold-modulo-regions-unsized-tail.rs @@ -0,0 +1,244 @@ +// This is a non-regression test for issues #108721 and its duplicate #123275 (hopefully, because +// the test is still convoluted and the ICE is fiddly). +// +// `pred_known_to_hold_modulo_regions` prevented "unexpected unsized tail" ICEs with warp/hyper but +// was unknowingly removed in #120463. + +//@ build-pass: the ICE happened in codegen + +use std::future::Future; +trait TryFuture: Future { + type Ok; +} +impl TryFuture for F +where + F: ?Sized + Future>, +{ + type Ok = T; +} +trait Executor {} +struct Exec {} +trait HttpBody { + type Data; +} +trait ConnStreamExec {} +impl ConnStreamExec for Exec where H2Stream: Send {} +impl ConnStreamExec for E where E: Executor {} +struct H2Stream { + _fut: F, +} +trait NewSvcExec> { + fn execute_new_svc(&mut self, _fut: NewSvcTask) { + unimplemented!() + } +} +impl NewSvcExec for Exec where W: Watcher {} +trait Watcher { + type Future; +} +struct NoopWatcher; +impl Watcher for NoopWatcher +where + S: HttpService, + E: ConnStreamExec, +{ + type Future = Option<<::ResBody as HttpBody>::Data>; +} +trait Service { + type Response; + type Future; +} +trait HttpService { + type ResBody: HttpBody; + type Future; +} +struct Body {} +impl HttpBody for Body { + type Data = String; +} +impl HttpService for S +where + S: Service<(), Response = ()>, +{ + type ResBody = Body; + type Future = S::Future; +} +trait MakeServiceRef { + type ResBody; + type Service: HttpService; +} +impl MakeServiceRef for T +where + T: for<'a> Service<&'a Target, Response = S, Future = F>, + S: HttpService, +{ + type Service = S; + type ResBody = S::ResBody; +} +fn make_service_fn(_f: F) -> MakeServiceFn +where + F: FnMut(&Target) -> Ret, + Ret: Future, +{ + unimplemented!() +} +struct MakeServiceFn { + _func: F, +} +impl<'t, F, Ret, Target, Svc> Service<&'t Target> for MakeServiceFn +where + F: FnMut(&Target) -> Ret, + Ret: Future>, +{ + type Response = Svc; + type Future = Option<()>; +} +struct AddrIncoming {} +struct Server { + _incoming: I, + _make_service: S, + _protocol: E, +} +impl Server +where + S: MakeServiceRef<(), ResBody = B>, + B: HttpBody, + E: ConnStreamExec<::Future>, + E: NewSvcExec, +{ + fn serve(&mut self) { + let fut = NewSvcTask::new(); + self._protocol.execute_new_svc(fut); + } +} +fn serve(_make_service: S) -> Server { + unimplemented!() +} +struct NewSvcTask> { + _state: State, +} +struct State> { + _fut: W::Future, +} +impl> NewSvcTask { + fn new() -> Self { + unimplemented!() + } +} +trait Filter { + type Extract; + type Future; + fn map(self, _fun: F) -> MapFilter + where + Self: Sized, + { + unimplemented!() + } + fn wrap_with(self, _wrapper: W) -> W::Wrapped + where + Self: Sized, + W: Wrap, + { + unimplemented!() + } +} +fn service(_filter: F) -> FilteredService +where + F: Filter, +{ + unimplemented!() +} +struct FilteredService { + _filter: F, +} +impl Service<()> for FilteredService +where + F: Filter, +{ + type Response = (); + type Future = FilteredFuture; +} +struct FilteredFuture { + _fut: F, +} +struct MapFilter { + _filter: T, + _func: F, +} +impl Filter for MapFilter +where + T: Filter, + F: Func, +{ + type Extract = F::Output; + type Future = MapFilterFuture; +} +struct MapFilterFuture { + _extract: T::Future, + _func: F, +} +trait Wrap { + type Wrapped; +} +fn make_filter_fn(_func: F) -> FilterFn +where + F: Fn() -> U, +{ + unimplemented!() +} +struct FilterFn { + _func: F, +} +impl Filter for FilterFn +where + F: Fn() -> U, + U: TryFuture, + U::Ok: Send, +{ + type Extract = U::Ok; + type Future = Option; +} +fn trace(_func: F) -> Trace +where + F: Fn(), +{ + unimplemented!() +} +struct Trace { + _func: F, +} +impl Wrap for Trace { + type Wrapped = WithTrace; +} +struct WithTrace { + _filter: F, + _trace: FN, +} +impl Filter for WithTrace +where + F: Filter, +{ + type Extract = (); + type Future = (F::Future, fn(F::Extract)); +} +trait Func { + type Output; +} +impl Func<()> for F +where + F: Fn() -> R, +{ + type Output = R; +} +fn main() { + let make_service = make_service_fn(|_| { + let tracer = trace(|| unimplemented!()); + let filter = make_filter_fn(|| std::future::ready(Some(()))) + .map(|| "Hello, world") + .wrap_with(tracer); + let svc = service(filter); + std::future::ready(Some(svc)) + }); + let mut server = serve(make_service); + server.serve(); +} From 68b4257ccf0c94f855a46b48e48c4c73559eff84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 6 Apr 2024 23:29:59 +0000 Subject: [PATCH 08/14] Revert "remove `pred_known_to_hold_modulo_regions`" This reverts commit 399a258f46074740862568b124c02f7b7d04638c. --- .../rustc_trait_selection/src/traits/mod.rs | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index 2c8116b779b..98d5b466cd0 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -119,7 +119,9 @@ pub fn predicates_for_generics<'tcx>( /// Determines whether the type `ty` is known to meet `bound` and /// returns true if so. Returns false if `ty` either does not meet -/// `bound` or is not known to meet bound. +/// `bound` or is not known to meet bound (note that this is +/// conservative towards *no impl*, which is the opposite of the +/// `evaluate` methods). pub fn type_known_to_meet_bound_modulo_regions<'tcx>( infcx: &InferCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, @@ -127,8 +129,50 @@ pub fn type_known_to_meet_bound_modulo_regions<'tcx>( def_id: DefId, ) -> bool { let trait_ref = ty::TraitRef::new(infcx.tcx, def_id, [ty]); - let obligation = Obligation::new(infcx.tcx, ObligationCause::dummy(), param_env, trait_ref); - infcx.predicate_must_hold_modulo_regions(&obligation) + pred_known_to_hold_modulo_regions(infcx, param_env, trait_ref) +} + +/// FIXME(@lcnr): this function doesn't seem right and shouldn't exist? +/// +/// Ping me on zulip if you want to use this method and need help with finding +/// an appropriate replacement. +#[instrument(level = "debug", skip(infcx, param_env, pred), ret)] +fn pred_known_to_hold_modulo_regions<'tcx>( + infcx: &InferCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, + pred: impl ToPredicate<'tcx>, +) -> bool { + let obligation = Obligation::new(infcx.tcx, ObligationCause::dummy(), param_env, pred); + + let result = infcx.evaluate_obligation_no_overflow(&obligation); + debug!(?result); + + if result.must_apply_modulo_regions() { + true + } else if result.may_apply() { + // Sometimes obligations are ambiguous because the recursive evaluator + // is not smart enough, so we fall back to fulfillment when we're not certain + // that an obligation holds or not. Even still, we must make sure that + // the we do no inference in the process of checking this obligation. + let goal = infcx.resolve_vars_if_possible((obligation.predicate, obligation.param_env)); + infcx.probe(|_| { + let ocx = ObligationCtxt::new(infcx); + ocx.register_obligation(obligation); + + let errors = ocx.select_all_or_error(); + match errors.as_slice() { + // Only known to hold if we did no inference. + [] => infcx.shallow_resolve(goal) == goal, + + errors => { + debug!(?errors); + false + } + } + }) + } else { + false + } } #[instrument(level = "debug", skip(tcx, elaborated_env))] From e38dfc49617086f31086f9d49275785d0ecc3d53 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 7 Apr 2024 21:14:01 +1000 Subject: [PATCH 09/14] Remove unnecessary cast from `LLVMRustGetInstrProfIncrementIntrinsic` This particular cast appears to have been copied over from clang, but there are plenty of other call sites in clang that don't bother with a cast here, and it works fine without one. For context, `llvm::Intrinsic::ID` is a typedef for `unsigned`, and `llvm::Intrinsic::instrprof_increment` is a member of `enum IndependentIntrinsics : unsigned`. --- compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 8ec1f5a99e7..a6894a7e089 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1524,8 +1524,8 @@ extern "C" LLVMValueRef LLVMRustBuildCall(LLVMBuilderRef B, LLVMTypeRef Ty, LLVM } extern "C" LLVMValueRef LLVMRustGetInstrProfIncrementIntrinsic(LLVMModuleRef M) { - return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), - (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_increment)); + return wrap(llvm::Intrinsic::getDeclaration( + unwrap(M), llvm::Intrinsic::instrprof_increment)); } extern "C" LLVMValueRef LLVMRustBuildMemCpy(LLVMBuilderRef B, From 009280c5e312bdf11cd0e0e1b336bf374eed7b00 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 20 Mar 2024 14:38:32 +0100 Subject: [PATCH 10/14] Fix argument ABI for overaligned structs on ppc64le When passing a 16 (or higher) aligned struct by value on ppc64le, it needs to be passed as an array of `i128` rather than an array of `i64`. This will force the use of an even starting register. For the case of a 16 byte struct with alignment 16 it is important that `[1 x i128]` is used instead of `i128` -- apparently, the latter will get treated similarly to `[2 x i64]`, not exhibiting the correct ABI. Add a `force_array` flag to `Uniform` to support this. The relevant clang code can be found here: https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L878-L884 https://github.com/llvm/llvm-project/blob/fe2119a7b08b6e468b2a67768904ea85b1bf0a45/clang/lib/CodeGen/Targets/PPC.cpp#L780-L784 I think the corresponding psABI wording is this: > Fixed size aggregates and unions passed by value are mapped to as > many doublewords of the parameter save area as the value uses in > memory. Aggregrates and unions are aligned according to their > alignment requirements. This may result in doublewords being > skipped for alignment. In particular the last sentence. Fixes https://github.com/rust-lang/rust/issues/122767. --- compiler/rustc_codegen_llvm/src/abi.rs | 2 +- compiler/rustc_target/src/abi/call/aarch64.rs | 8 +- compiler/rustc_target/src/abi/call/arm.rs | 10 +- compiler/rustc_target/src/abi/call/csky.rs | 4 +- .../rustc_target/src/abi/call/loongarch.rs | 7 +- compiler/rustc_target/src/abi/call/mips.rs | 5 +- compiler/rustc_target/src/abi/call/mips64.rs | 4 +- compiler/rustc_target/src/abi/call/mod.rs | 5 +- compiler/rustc_target/src/abi/call/nvptx64.rs | 2 +- .../rustc_target/src/abi/call/powerpc64.rs | 25 ++--- compiler/rustc_target/src/abi/call/riscv.rs | 7 +- compiler/rustc_target/src/abi/call/sparc.rs | 5 +- compiler/rustc_target/src/abi/call/sparc64.rs | 4 +- compiler/rustc_target/src/abi/call/wasm.rs | 4 +- tests/codegen/powerpc64le-struct-align-128.rs | 93 +++++++++++++++++++ 15 files changed, 152 insertions(+), 33 deletions(-) create mode 100644 tests/codegen/powerpc64le-struct-align-128.rs diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index d2828669d43..8b2330471f7 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -150,7 +150,7 @@ impl LlvmType for CastTarget { // Simplify to a single unit or an array if there's no prefix. // This produces the same layout, but using a simpler type. if self.prefix.iter().all(|x| x.is_none()) { - if rest_count == 1 { + if rest_count == 1 && !self.rest.force_array { return rest_ll_unit; } diff --git a/compiler/rustc_target/src/abi/call/aarch64.rs b/compiler/rustc_target/src/abi/call/aarch64.rs index f99f6a3b721..bf9cf232d81 100644 --- a/compiler/rustc_target/src/abi/call/aarch64.rs +++ b/compiler/rustc_target/src/abi/call/aarch64.rs @@ -31,7 +31,7 @@ where RegKind::Vector => size.bits() == 64 || size.bits() == 128, }; - valid_unit.then_some(Uniform { unit, total: size }) + valid_unit.then_some(Uniform { unit, total: size, force_array: false }) }) } @@ -60,7 +60,7 @@ where let size = ret.layout.size; let bits = size.bits(); if bits <= 128 { - ret.cast_to(Uniform { unit: Reg::i64(), total: size }); + ret.cast_to(Uniform { unit: Reg::i64(), total: size, force_array: false }); return; } ret.make_indirect(); @@ -100,9 +100,9 @@ where }; if size.bits() <= 128 { if align.bits() == 128 { - arg.cast_to(Uniform { unit: Reg::i128(), total: size }); + arg.cast_to(Uniform { unit: Reg::i128(), total: size, force_array: false }); } else { - arg.cast_to(Uniform { unit: Reg::i64(), total: size }); + arg.cast_to(Uniform { unit: Reg::i64(), total: size, force_array: false }); } return; } diff --git a/compiler/rustc_target/src/abi/call/arm.rs b/compiler/rustc_target/src/abi/call/arm.rs index 95f6691d42a..177a16bafc8 100644 --- a/compiler/rustc_target/src/abi/call/arm.rs +++ b/compiler/rustc_target/src/abi/call/arm.rs @@ -21,7 +21,7 @@ where RegKind::Vector => size.bits() == 64 || size.bits() == 128, }; - valid_unit.then_some(Uniform { unit, total: size }) + valid_unit.then_some(Uniform { unit, total: size, force_array: false }) }) } @@ -49,7 +49,7 @@ where let size = ret.layout.size; let bits = size.bits(); if bits <= 32 { - ret.cast_to(Uniform { unit: Reg::i32(), total: size }); + ret.cast_to(Uniform { unit: Reg::i32(), total: size, force_array: false }); return; } ret.make_indirect(); @@ -78,7 +78,11 @@ where let align = arg.layout.align.abi.bytes(); let total = arg.layout.size; - arg.cast_to(Uniform { unit: if align <= 4 { Reg::i32() } else { Reg::i64() }, total }); + arg.cast_to(Uniform { + unit: if align <= 4 { Reg::i32() } else { Reg::i64() }, + total, + force_array: false, + }); } pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>) diff --git a/compiler/rustc_target/src/abi/call/csky.rs b/compiler/rustc_target/src/abi/call/csky.rs index 8b4328db52e..dae428e3538 100644 --- a/compiler/rustc_target/src/abi/call/csky.rs +++ b/compiler/rustc_target/src/abi/call/csky.rs @@ -18,7 +18,7 @@ fn classify_ret(arg: &mut ArgAbi<'_, Ty>) { if total.bits() > 64 { arg.make_indirect(); } else if total.bits() > 32 { - arg.cast_to(Uniform { unit: Reg::i32(), total }); + arg.cast_to(Uniform { unit: Reg::i32(), total, force_array: false }); } else { arg.cast_to(Reg::i32()); } @@ -38,7 +38,7 @@ fn classify_arg(arg: &mut ArgAbi<'_, Ty>) { if arg.layout.is_aggregate() { let total = arg.layout.size; if total.bits() > 32 { - arg.cast_to(Uniform { unit: Reg::i32(), total }); + arg.cast_to(Uniform { unit: Reg::i32(), total, force_array: false }); } else { arg.cast_to(Reg::i32()); } diff --git a/compiler/rustc_target/src/abi/call/loongarch.rs b/compiler/rustc_target/src/abi/call/loongarch.rs index 35d4b331cb4..3b8a3ba66ba 100644 --- a/compiler/rustc_target/src/abi/call/loongarch.rs +++ b/compiler/rustc_target/src/abi/call/loongarch.rs @@ -195,7 +195,11 @@ where if total.bits() <= xlen { arg.cast_to(xlen_reg); } else { - arg.cast_to(Uniform { unit: xlen_reg, total: Size::from_bits(xlen * 2) }); + arg.cast_to(Uniform { + unit: xlen_reg, + total: Size::from_bits(xlen * 2), + force_array: false, + }); } return false; } @@ -281,6 +285,7 @@ fn classify_arg<'a, Ty, C>( arg.cast_to(Uniform { unit: if align_regs { double_xlen_reg } else { xlen_reg }, total: Size::from_bits(xlen * 2), + force_array: false, }); } if align_regs && is_vararg { diff --git a/compiler/rustc_target/src/abi/call/mips.rs b/compiler/rustc_target/src/abi/call/mips.rs index 57ccfe2152b..054c127181a 100644 --- a/compiler/rustc_target/src/abi/call/mips.rs +++ b/compiler/rustc_target/src/abi/call/mips.rs @@ -27,7 +27,10 @@ where if arg.layout.is_aggregate() { let pad_i32 = !offset.is_aligned(align); - arg.cast_to_and_pad_i32(Uniform { unit: Reg::i32(), total: size }, pad_i32); + arg.cast_to_and_pad_i32( + Uniform { unit: Reg::i32(), total: size, force_array: false }, + pad_i32, + ); } else { arg.extend_integer_width_to(32); } diff --git a/compiler/rustc_target/src/abi/call/mips64.rs b/compiler/rustc_target/src/abi/call/mips64.rs index 2700f67b209..69e73beca9a 100644 --- a/compiler/rustc_target/src/abi/call/mips64.rs +++ b/compiler/rustc_target/src/abi/call/mips64.rs @@ -68,7 +68,7 @@ where } // Cast to a uniform int structure - ret.cast_to(Uniform { unit: Reg::i64(), total: size }); + ret.cast_to(Uniform { unit: Reg::i64(), total: size, force_array: false }); } else { ret.make_indirect(); } @@ -139,7 +139,7 @@ where let rest_size = size - Size::from_bytes(8) * prefix_index as u64; arg.cast_to(CastTarget { prefix, - rest: Uniform { unit: Reg::i64(), total: rest_size }, + rest: Uniform { unit: Reg::i64(), total: rest_size, force_array: false }, attrs: ArgAttributes { regular: ArgAttribute::default(), arg_ext: ArgExtension::None, diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index cdd3f0afd79..022ea44cc93 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -255,11 +255,14 @@ pub struct Uniform { /// for 64-bit integers with a total size of 20 bytes. When the argument is actually passed, /// this size will be rounded up to the nearest multiple of `unit.size`. pub total: Size, + + /// Force the use of an array, even if there is only a single element. + pub force_array: bool, } impl From for Uniform { fn from(unit: Reg) -> Uniform { - Uniform { unit, total: unit.size } + Uniform { unit, total: unit.size, force_array: false } } } diff --git a/compiler/rustc_target/src/abi/call/nvptx64.rs b/compiler/rustc_target/src/abi/call/nvptx64.rs index 5c040ce9c3b..e7525c76015 100644 --- a/compiler/rustc_target/src/abi/call/nvptx64.rs +++ b/compiler/rustc_target/src/abi/call/nvptx64.rs @@ -35,7 +35,7 @@ where 16 => Reg::i128(), _ => unreachable!("Align is given as power of 2 no larger than 16 bytes"), }; - arg.cast_to(Uniform { unit, total: Size::from_bytes(2 * align_bytes) }); + arg.cast_to(Uniform { unit, total: Size::from_bytes(2 * align_bytes), force_array: false }); } else { // FIXME: find a better way to do this. See https://github.com/rust-lang/rust/issues/117271. arg.make_direct_deprecated(); diff --git a/compiler/rustc_target/src/abi/call/powerpc64.rs b/compiler/rustc_target/src/abi/call/powerpc64.rs index 2d41f77e50e..0b2bd8c90d7 100644 --- a/compiler/rustc_target/src/abi/call/powerpc64.rs +++ b/compiler/rustc_target/src/abi/call/powerpc64.rs @@ -2,7 +2,7 @@ // Alignment of 128 bit types is not currently handled, this will // need to be fixed when PowerPC vector support is added. -use crate::abi::call::{ArgAbi, FnAbi, Reg, RegKind, Uniform}; +use crate::abi::call::{Align, ArgAbi, FnAbi, Reg, RegKind, Uniform}; use crate::abi::{Endian, HasDataLayout, TyAbiInterface}; use crate::spec::HasTargetSpec; @@ -37,7 +37,7 @@ where RegKind::Vector => arg.layout.size.bits() == 128, }; - valid_unit.then_some(Uniform { unit, total: arg.layout.size }) + valid_unit.then_some(Uniform { unit, total: arg.layout.size, force_array: false }) }) } @@ -81,7 +81,7 @@ where Reg::i64() }; - ret.cast_to(Uniform { unit, total: size }); + ret.cast_to(Uniform { unit, total: size, force_array: false }); return; } @@ -108,18 +108,21 @@ where } let size = arg.layout.size; - let (unit, total) = if size.bits() <= 64 { + if size.bits() <= 64 { // Aggregates smaller than a doubleword should appear in // the least-significant bits of the parameter doubleword. - (Reg { kind: RegKind::Integer, size }, size) + arg.cast_to(Reg { kind: RegKind::Integer, size }) } else { - // Aggregates larger than a doubleword should be padded - // at the tail to fill out a whole number of doublewords. - let reg_i64 = Reg::i64(); - (reg_i64, size.align_to(reg_i64.align(cx))) + // Aggregates larger than i64 should be padded at the tail to fill out a whole number + // of i64s or i128s, depending on the aggregate alignment. Always use an array for + // this, even if there is only a single element. + let reg = if arg.layout.align.abi.bytes() > 8 { Reg::i128() } else { Reg::i64() }; + arg.cast_to(Uniform { + unit: reg, + total: size.align_to(Align::from_bytes(reg.size.bytes()).unwrap()), + force_array: true, + }) }; - - arg.cast_to(Uniform { unit, total }); } pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>) diff --git a/compiler/rustc_target/src/abi/call/riscv.rs b/compiler/rustc_target/src/abi/call/riscv.rs index 6a38496dc57..0dad35c8aab 100644 --- a/compiler/rustc_target/src/abi/call/riscv.rs +++ b/compiler/rustc_target/src/abi/call/riscv.rs @@ -201,7 +201,11 @@ where if total.bits() <= xlen { arg.cast_to(xlen_reg); } else { - arg.cast_to(Uniform { unit: xlen_reg, total: Size::from_bits(xlen * 2) }); + arg.cast_to(Uniform { + unit: xlen_reg, + total: Size::from_bits(xlen * 2), + force_array: false, + }); } return false; } @@ -287,6 +291,7 @@ fn classify_arg<'a, Ty, C>( arg.cast_to(Uniform { unit: if align_regs { double_xlen_reg } else { xlen_reg }, total: Size::from_bits(xlen * 2), + force_array: false, }); } if align_regs && is_vararg { diff --git a/compiler/rustc_target/src/abi/call/sparc.rs b/compiler/rustc_target/src/abi/call/sparc.rs index 57ccfe2152b..054c127181a 100644 --- a/compiler/rustc_target/src/abi/call/sparc.rs +++ b/compiler/rustc_target/src/abi/call/sparc.rs @@ -27,7 +27,10 @@ where if arg.layout.is_aggregate() { let pad_i32 = !offset.is_aligned(align); - arg.cast_to_and_pad_i32(Uniform { unit: Reg::i32(), total: size }, pad_i32); + arg.cast_to_and_pad_i32( + Uniform { unit: Reg::i32(), total: size, force_array: false }, + pad_i32, + ); } else { arg.extend_integer_width_to(32); } diff --git a/compiler/rustc_target/src/abi/call/sparc64.rs b/compiler/rustc_target/src/abi/call/sparc64.rs index cbed5b4afc1..a36b691468e 100644 --- a/compiler/rustc_target/src/abi/call/sparc64.rs +++ b/compiler/rustc_target/src/abi/call/sparc64.rs @@ -192,7 +192,7 @@ where arg.cast_to(CastTarget { prefix: data.prefix, - rest: Uniform { unit: Reg::i64(), total: rest_size }, + rest: Uniform { unit: Reg::i64(), total: rest_size, force_array: false }, attrs: ArgAttributes { regular: data.arg_attribute, arg_ext: ArgExtension::None, @@ -205,7 +205,7 @@ where } } - arg.cast_to(Uniform { unit: Reg::i64(), total }); + arg.cast_to(Uniform { unit: Reg::i64(), total, force_array: false }); } pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>) diff --git a/compiler/rustc_target/src/abi/call/wasm.rs b/compiler/rustc_target/src/abi/call/wasm.rs index a7a2b314a94..a773fb1e814 100644 --- a/compiler/rustc_target/src/abi/call/wasm.rs +++ b/compiler/rustc_target/src/abi/call/wasm.rs @@ -1,4 +1,4 @@ -use crate::abi::call::{ArgAbi, FnAbi, Uniform}; +use crate::abi::call::{ArgAbi, FnAbi}; use crate::abi::{HasDataLayout, TyAbiInterface}; fn unwrap_trivial_aggregate<'a, Ty, C>(cx: &C, val: &mut ArgAbi<'a, Ty>) -> bool @@ -10,7 +10,7 @@ where if let Some(unit) = val.layout.homogeneous_aggregate(cx).ok().and_then(|ha| ha.unit()) { let size = val.layout.size; if unit.size == size { - val.cast_to(Uniform { unit, total: size }); + val.cast_to(unit); return true; } } diff --git a/tests/codegen/powerpc64le-struct-align-128.rs b/tests/codegen/powerpc64le-struct-align-128.rs new file mode 100644 index 00000000000..0096c6d3138 --- /dev/null +++ b/tests/codegen/powerpc64le-struct-align-128.rs @@ -0,0 +1,93 @@ +// Test that structs aligned to 128 bits are passed with the correct ABI on powerpc64le. +// This is similar to aarch64-struct-align-128.rs, but for ppc. + +//@ compile-flags: --target powerpc64le-unknown-linux-gnu +//@ needs-llvm-components: powerpc + +#![feature(no_core, lang_items)] +#![crate_type = "lib"] +#![no_core] + +#[lang="sized"] +trait Sized { } +#[lang="freeze"] +trait Freeze { } +#[lang="copy"] +trait Copy { } + +#[repr(C)] +pub struct Align8 { + pub a: u64, + pub b: u64, +} + +#[repr(transparent)] +pub struct Transparent8 { + a: Align8 +} + +#[repr(C)] +pub struct Wrapped8 { + a: Align8, +} + +extern "C" { + // CHECK: declare void @test_8([2 x i64], [2 x i64], [2 x i64]) + fn test_8(a: Align8, b: Transparent8, c: Wrapped8); +} + +#[repr(C)] +#[repr(align(16))] +pub struct Align16 { + pub a: u64, + pub b: u64, +} + +#[repr(transparent)] +pub struct Transparent16 { + a: Align16 +} + +#[repr(C)] +pub struct Wrapped16 { + pub a: Align16, +} + +extern "C" { + // It's important that this produces [1 x i128] rather than just i128! + // CHECK: declare void @test_16([1 x i128], [1 x i128], [1 x i128]) + fn test_16(a: Align16, b: Transparent16, c: Wrapped16); +} + +#[repr(C)] +#[repr(align(32))] +pub struct Align32 { + pub a: u64, + pub b: u64, + pub c: u64, +} + +#[repr(transparent)] +pub struct Transparent32 { + a: Align32 +} + +#[repr(C)] +pub struct Wrapped32 { + pub a: Align32, +} + +extern "C" { + // CHECK: declare void @test_32([2 x i128], [2 x i128], [2 x i128]) + fn test_32(a: Align32, b: Transparent32, c: Wrapped32); +} + +pub unsafe fn main( + a1: Align8, a2: Transparent8, a3: Wrapped8, + b1: Align16, b2: Transparent16, b3: Wrapped16, + c1: Align32, c2: Transparent32, c3: Wrapped32, +) { + test_8(a1, a2, a3); + test_16(b1, b2, b3); + test_32(c1, c2, c3); +} From 1b7342b4117e321012cbe49df5ae859d52031f2b Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 21 Mar 2024 16:10:23 +0100 Subject: [PATCH 11/14] force_array -> is_consecutive The actual ABI implication here is that in some cases the values are required to be "consecutive", i.e. must either all be passed in registers or all on stack (without padding). Adjust the code to either use Uniform::new() or Uniform::consecutive() depending on which behavior is needed. Then, when lowering this in LLVM, skip the [1 x i128] to i128 simplification if is_consecutive is set. i128 is the only case I'm aware of where this is problematic right now. If we find other cases, we can extend this (either based on target information or possibly just by not simplifying for is_consecutive entirely). --- compiler/rustc_codegen_llvm/src/abi.rs | 5 ++++- compiler/rustc_target/src/abi/call/aarch64.rs | 8 ++++---- compiler/rustc_target/src/abi/call/arm.rs | 10 +++------- compiler/rustc_target/src/abi/call/csky.rs | 4 ++-- .../rustc_target/src/abi/call/loongarch.rs | 15 +++++--------- compiler/rustc_target/src/abi/call/mips.rs | 5 +---- compiler/rustc_target/src/abi/call/mips64.rs | 4 ++-- compiler/rustc_target/src/abi/call/mod.rs | 20 ++++++++++++++++--- compiler/rustc_target/src/abi/call/nvptx64.rs | 2 +- .../rustc_target/src/abi/call/powerpc64.rs | 13 ++++++------ compiler/rustc_target/src/abi/call/riscv.rs | 15 +++++--------- compiler/rustc_target/src/abi/call/sparc.rs | 5 +---- compiler/rustc_target/src/abi/call/sparc64.rs | 4 ++-- 13 files changed, 53 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index 8b2330471f7..f918facc86d 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -150,7 +150,10 @@ impl LlvmType for CastTarget { // Simplify to a single unit or an array if there's no prefix. // This produces the same layout, but using a simpler type. if self.prefix.iter().all(|x| x.is_none()) { - if rest_count == 1 && !self.rest.force_array { + // We can't do this if is_consecutive is set and the unit would get + // split on the target. Currently, this is only relevant for i128 + // registers. + if rest_count == 1 && (!self.rest.is_consecutive || self.rest.unit != Reg::i128()) { return rest_ll_unit; } diff --git a/compiler/rustc_target/src/abi/call/aarch64.rs b/compiler/rustc_target/src/abi/call/aarch64.rs index bf9cf232d81..04020d13f22 100644 --- a/compiler/rustc_target/src/abi/call/aarch64.rs +++ b/compiler/rustc_target/src/abi/call/aarch64.rs @@ -31,7 +31,7 @@ where RegKind::Vector => size.bits() == 64 || size.bits() == 128, }; - valid_unit.then_some(Uniform { unit, total: size, force_array: false }) + valid_unit.then_some(Uniform::consecutive(unit, size)) }) } @@ -60,7 +60,7 @@ where let size = ret.layout.size; let bits = size.bits(); if bits <= 128 { - ret.cast_to(Uniform { unit: Reg::i64(), total: size, force_array: false }); + ret.cast_to(Uniform::new(Reg::i64(), size)); return; } ret.make_indirect(); @@ -100,9 +100,9 @@ where }; if size.bits() <= 128 { if align.bits() == 128 { - arg.cast_to(Uniform { unit: Reg::i128(), total: size, force_array: false }); + arg.cast_to(Uniform::new(Reg::i128(), size)); } else { - arg.cast_to(Uniform { unit: Reg::i64(), total: size, force_array: false }); + arg.cast_to(Uniform::new(Reg::i64(), size)); } return; } diff --git a/compiler/rustc_target/src/abi/call/arm.rs b/compiler/rustc_target/src/abi/call/arm.rs index 177a16bafc8..9371e1b3958 100644 --- a/compiler/rustc_target/src/abi/call/arm.rs +++ b/compiler/rustc_target/src/abi/call/arm.rs @@ -21,7 +21,7 @@ where RegKind::Vector => size.bits() == 64 || size.bits() == 128, }; - valid_unit.then_some(Uniform { unit, total: size, force_array: false }) + valid_unit.then_some(Uniform::consecutive(unit, size)) }) } @@ -49,7 +49,7 @@ where let size = ret.layout.size; let bits = size.bits(); if bits <= 32 { - ret.cast_to(Uniform { unit: Reg::i32(), total: size, force_array: false }); + ret.cast_to(Uniform::new(Reg::i32(), size)); return; } ret.make_indirect(); @@ -78,11 +78,7 @@ where let align = arg.layout.align.abi.bytes(); let total = arg.layout.size; - arg.cast_to(Uniform { - unit: if align <= 4 { Reg::i32() } else { Reg::i64() }, - total, - force_array: false, - }); + arg.cast_to(Uniform::consecutive(if align <= 4 { Reg::i32() } else { Reg::i64() }, total)); } pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>) diff --git a/compiler/rustc_target/src/abi/call/csky.rs b/compiler/rustc_target/src/abi/call/csky.rs index dae428e3538..7951f28beea 100644 --- a/compiler/rustc_target/src/abi/call/csky.rs +++ b/compiler/rustc_target/src/abi/call/csky.rs @@ -18,7 +18,7 @@ fn classify_ret(arg: &mut ArgAbi<'_, Ty>) { if total.bits() > 64 { arg.make_indirect(); } else if total.bits() > 32 { - arg.cast_to(Uniform { unit: Reg::i32(), total, force_array: false }); + arg.cast_to(Uniform::new(Reg::i32(), total)); } else { arg.cast_to(Reg::i32()); } @@ -38,7 +38,7 @@ fn classify_arg(arg: &mut ArgAbi<'_, Ty>) { if arg.layout.is_aggregate() { let total = arg.layout.size; if total.bits() > 32 { - arg.cast_to(Uniform { unit: Reg::i32(), total, force_array: false }); + arg.cast_to(Uniform::new(Reg::i32(), total)); } else { arg.cast_to(Reg::i32()); } diff --git a/compiler/rustc_target/src/abi/call/loongarch.rs b/compiler/rustc_target/src/abi/call/loongarch.rs index 3b8a3ba66ba..943b12a9fbf 100644 --- a/compiler/rustc_target/src/abi/call/loongarch.rs +++ b/compiler/rustc_target/src/abi/call/loongarch.rs @@ -195,11 +195,7 @@ where if total.bits() <= xlen { arg.cast_to(xlen_reg); } else { - arg.cast_to(Uniform { - unit: xlen_reg, - total: Size::from_bits(xlen * 2), - force_array: false, - }); + arg.cast_to(Uniform::new(xlen_reg, Size::from_bits(xlen * 2))); } return false; } @@ -282,11 +278,10 @@ fn classify_arg<'a, Ty, C>( if total.bits() > xlen { let align_regs = align > xlen; if is_loongarch_aggregate(arg) { - arg.cast_to(Uniform { - unit: if align_regs { double_xlen_reg } else { xlen_reg }, - total: Size::from_bits(xlen * 2), - force_array: false, - }); + arg.cast_to(Uniform::new( + if align_regs { double_xlen_reg } else { xlen_reg }, + Size::from_bits(xlen * 2), + )); } if align_regs && is_vararg { *avail_gprs -= *avail_gprs % 2; diff --git a/compiler/rustc_target/src/abi/call/mips.rs b/compiler/rustc_target/src/abi/call/mips.rs index 054c127181a..0e5a7f37a09 100644 --- a/compiler/rustc_target/src/abi/call/mips.rs +++ b/compiler/rustc_target/src/abi/call/mips.rs @@ -27,10 +27,7 @@ where if arg.layout.is_aggregate() { let pad_i32 = !offset.is_aligned(align); - arg.cast_to_and_pad_i32( - Uniform { unit: Reg::i32(), total: size, force_array: false }, - pad_i32, - ); + arg.cast_to_and_pad_i32(Uniform::new(Reg::i32(), size), pad_i32); } else { arg.extend_integer_width_to(32); } diff --git a/compiler/rustc_target/src/abi/call/mips64.rs b/compiler/rustc_target/src/abi/call/mips64.rs index 69e73beca9a..b2a2c34b980 100644 --- a/compiler/rustc_target/src/abi/call/mips64.rs +++ b/compiler/rustc_target/src/abi/call/mips64.rs @@ -68,7 +68,7 @@ where } // Cast to a uniform int structure - ret.cast_to(Uniform { unit: Reg::i64(), total: size, force_array: false }); + ret.cast_to(Uniform::new(Reg::i64(), size)); } else { ret.make_indirect(); } @@ -139,7 +139,7 @@ where let rest_size = size - Size::from_bytes(8) * prefix_index as u64; arg.cast_to(CastTarget { prefix, - rest: Uniform { unit: Reg::i64(), total: rest_size, force_array: false }, + rest: Uniform::new(Reg::i64(), rest_size), attrs: ArgAttributes { regular: ArgAttribute::default(), arg_ext: ArgExtension::None, diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 022ea44cc93..4502df339d1 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -256,13 +256,15 @@ pub struct Uniform { /// this size will be rounded up to the nearest multiple of `unit.size`. pub total: Size, - /// Force the use of an array, even if there is only a single element. - pub force_array: bool, + /// Indicate that the argument is consecutive, in the sense that either all values need to be + /// passed in register, or all on the stack. If they are passed on the stack, there should be + /// no additional padding between elements. + pub is_consecutive: bool, } impl From for Uniform { fn from(unit: Reg) -> Uniform { - Uniform { unit, total: unit.size, force_array: false } + Uniform { unit, total: unit.size, is_consecutive: false } } } @@ -270,6 +272,18 @@ impl Uniform { pub fn align(&self, cx: &C) -> Align { self.unit.align(cx) } + + /// Pass using one or more values of the given type, without requiring them to be consecutive. + /// That is, some values may be passed in register and some on the stack. + pub fn new(unit: Reg, total: Size) -> Self { + Uniform { unit, total, is_consecutive: false } + } + + /// Pass using one or more consecutive values of the given type. Either all values will be + /// passed in registers, or all on the stack. + pub fn consecutive(unit: Reg, total: Size) -> Self { + Uniform { unit, total, is_consecutive: true } + } } /// Describes the type used for `PassMode::Cast`. diff --git a/compiler/rustc_target/src/abi/call/nvptx64.rs b/compiler/rustc_target/src/abi/call/nvptx64.rs index e7525c76015..f85fa2419f0 100644 --- a/compiler/rustc_target/src/abi/call/nvptx64.rs +++ b/compiler/rustc_target/src/abi/call/nvptx64.rs @@ -35,7 +35,7 @@ where 16 => Reg::i128(), _ => unreachable!("Align is given as power of 2 no larger than 16 bytes"), }; - arg.cast_to(Uniform { unit, total: Size::from_bytes(2 * align_bytes), force_array: false }); + arg.cast_to(Uniform::new(unit, Size::from_bytes(2 * align_bytes))); } else { // FIXME: find a better way to do this. See https://github.com/rust-lang/rust/issues/117271. arg.make_direct_deprecated(); diff --git a/compiler/rustc_target/src/abi/call/powerpc64.rs b/compiler/rustc_target/src/abi/call/powerpc64.rs index 0b2bd8c90d7..11a6cb52bab 100644 --- a/compiler/rustc_target/src/abi/call/powerpc64.rs +++ b/compiler/rustc_target/src/abi/call/powerpc64.rs @@ -37,7 +37,7 @@ where RegKind::Vector => arg.layout.size.bits() == 128, }; - valid_unit.then_some(Uniform { unit, total: arg.layout.size, force_array: false }) + valid_unit.then_some(Uniform::consecutive(unit, arg.layout.size)) }) } @@ -81,7 +81,7 @@ where Reg::i64() }; - ret.cast_to(Uniform { unit, total: size, force_array: false }); + ret.cast_to(Uniform::new(unit, size)); return; } @@ -117,11 +117,10 @@ where // of i64s or i128s, depending on the aggregate alignment. Always use an array for // this, even if there is only a single element. let reg = if arg.layout.align.abi.bytes() > 8 { Reg::i128() } else { Reg::i64() }; - arg.cast_to(Uniform { - unit: reg, - total: size.align_to(Align::from_bytes(reg.size.bytes()).unwrap()), - force_array: true, - }) + arg.cast_to(Uniform::consecutive( + reg, + size.align_to(Align::from_bytes(reg.size.bytes()).unwrap()), + )) }; } diff --git a/compiler/rustc_target/src/abi/call/riscv.rs b/compiler/rustc_target/src/abi/call/riscv.rs index 0dad35c8aab..5d4b3a9d245 100644 --- a/compiler/rustc_target/src/abi/call/riscv.rs +++ b/compiler/rustc_target/src/abi/call/riscv.rs @@ -201,11 +201,7 @@ where if total.bits() <= xlen { arg.cast_to(xlen_reg); } else { - arg.cast_to(Uniform { - unit: xlen_reg, - total: Size::from_bits(xlen * 2), - force_array: false, - }); + arg.cast_to(Uniform::new(xlen_reg, Size::from_bits(xlen * 2))); } return false; } @@ -288,11 +284,10 @@ fn classify_arg<'a, Ty, C>( if total.bits() > xlen { let align_regs = align > xlen; if is_riscv_aggregate(arg) { - arg.cast_to(Uniform { - unit: if align_regs { double_xlen_reg } else { xlen_reg }, - total: Size::from_bits(xlen * 2), - force_array: false, - }); + arg.cast_to(Uniform::new( + if align_regs { double_xlen_reg } else { xlen_reg }, + Size::from_bits(xlen * 2), + )); } if align_regs && is_vararg { *avail_gprs -= *avail_gprs % 2; diff --git a/compiler/rustc_target/src/abi/call/sparc.rs b/compiler/rustc_target/src/abi/call/sparc.rs index 054c127181a..0e5a7f37a09 100644 --- a/compiler/rustc_target/src/abi/call/sparc.rs +++ b/compiler/rustc_target/src/abi/call/sparc.rs @@ -27,10 +27,7 @@ where if arg.layout.is_aggregate() { let pad_i32 = !offset.is_aligned(align); - arg.cast_to_and_pad_i32( - Uniform { unit: Reg::i32(), total: size, force_array: false }, - pad_i32, - ); + arg.cast_to_and_pad_i32(Uniform::new(Reg::i32(), size), pad_i32); } else { arg.extend_integer_width_to(32); } diff --git a/compiler/rustc_target/src/abi/call/sparc64.rs b/compiler/rustc_target/src/abi/call/sparc64.rs index a36b691468e..acdcd5cc0d4 100644 --- a/compiler/rustc_target/src/abi/call/sparc64.rs +++ b/compiler/rustc_target/src/abi/call/sparc64.rs @@ -192,7 +192,7 @@ where arg.cast_to(CastTarget { prefix: data.prefix, - rest: Uniform { unit: Reg::i64(), total: rest_size, force_array: false }, + rest: Uniform::new(Reg::i64(), rest_size), attrs: ArgAttributes { regular: data.arg_attribute, arg_ext: ArgExtension::None, @@ -205,7 +205,7 @@ where } } - arg.cast_to(Uniform { unit: Reg::i64(), total, force_array: false }); + arg.cast_to(Uniform::new(Reg::i64(), total)); } pub fn compute_abi_info<'a, Ty, C>(cx: &C, fn_abi: &mut FnAbi<'a, Ty>) From 3a0d8d8afc1c4bc88089fa0c3d045abb9d0afb25 Mon Sep 17 00:00:00 2001 From: Yutaro Ohno Date: Mon, 8 Apr 2024 00:47:31 +0900 Subject: [PATCH 12/14] parser: reduce visibility of unnecessary public `UnmatchedDelim` `lexer::UnmatchedDelim` struct in `rustc_parse` is unnecessary public outside of the crate. This commit reduces the visibility to `pub(crate)`. Beside, this removes unnecessary field `expected_delim` that causes warnings after changing the visibility. --- compiler/rustc_parse/src/lexer/mod.rs | 3 +-- compiler/rustc_parse/src/lexer/tokentrees.rs | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_parse/src/lexer/mod.rs b/compiler/rustc_parse/src/lexer/mod.rs index 69b48bf0aff..353c3f41ed8 100644 --- a/compiler/rustc_parse/src/lexer/mod.rs +++ b/compiler/rustc_parse/src/lexer/mod.rs @@ -34,8 +34,7 @@ use unescape_error_reporting::{emit_unescape_error, escaped_char}; rustc_data_structures::static_assert_size!(rustc_lexer::Token, 12); #[derive(Clone, Debug)] -pub struct UnmatchedDelim { - pub expected_delim: Delimiter, +pub(crate) struct UnmatchedDelim { pub found_delim: Option, pub found_span: Span, pub unclosed_span: Option, diff --git a/compiler/rustc_parse/src/lexer/tokentrees.rs b/compiler/rustc_parse/src/lexer/tokentrees.rs index a506f98bf3a..b5a5a2a90ee 100644 --- a/compiler/rustc_parse/src/lexer/tokentrees.rs +++ b/compiler/rustc_parse/src/lexer/tokentrees.rs @@ -77,7 +77,6 @@ impl<'psess, 'src> TokenTreesReader<'psess, 'src> { for &(_, sp) in &self.diag_info.open_braces { err.span_label(sp, "unclosed delimiter"); self.diag_info.unmatched_delims.push(UnmatchedDelim { - expected_delim: Delimiter::Brace, found_delim: None, found_span: self.token.span, unclosed_span: Some(sp), @@ -163,9 +162,8 @@ impl<'psess, 'src> TokenTreesReader<'psess, 'src> { candidate = Some(*brace_span); } } - let (tok, _) = self.diag_info.open_braces.pop().unwrap(); + let (_, _) = self.diag_info.open_braces.pop().unwrap(); self.diag_info.unmatched_delims.push(UnmatchedDelim { - expected_delim: tok, found_delim: Some(close_delim), found_span: self.token.span, unclosed_span: unclosed_delimiter, From 3aa14e3b2e61739e8d0ec7883f9c185821ca5da2 Mon Sep 17 00:00:00 2001 From: Jack Wrenn Date: Tue, 19 Mar 2024 14:49:13 +0000 Subject: [PATCH 13/14] Compute transmutability from `rustc_target::abi::Layout` In its first step of computing transmutability, `rustc_transmutability` constructs a byte-level representation of type layout (`Tree`). Previously, this representation was computed for ADTs by inspecting the ADT definition and performing our own layout computations. This process was error-prone, verbose, and limited our ability to analyze many types (particularly default-repr types). In this PR, we instead construct `Tree`s from `rustc_target::abi::Layout`s. This helps ensure that layout optimizations are reflected our analyses, and increases the kinds of types we can now analyze, including: - default repr ADTs - transparent unions - `UnsafeCell`-containing types Overall, this PR expands the expressvity of `rustc_transmutability` to be much closer to the transmutability analysis performed by miri. Future PRs will work to close the remaining gaps (e.g., support for `Box`, raw pointers, `NonZero*`, coroutines, etc.). --- .../src/traits/error_reporting/mod.rs | 2 +- .../error_reporting/type_err_ctxt_ext.rs | 30 +- .../src/traits/select/confirmation.rs | 42 +- compiler/rustc_transmute/src/layout/dfa.rs | 2 + compiler/rustc_transmute/src/layout/nfa.rs | 1 + compiler/rustc_transmute/src/layout/tree.rs | 453 ++++++++---------- compiler/rustc_transmute/src/lib.rs | 4 +- .../src/maybe_transmutable/mod.rs | 19 +- .../src/maybe_transmutable/query_context.rs | 10 - .../ui/transmutability/arrays/huge-len.stderr | 4 +- .../should_require_well_defined_layout.stderr | 12 +- .../enums/niche_optimization.rs | 156 ++++++ .../enums/repr/padding_differences.rs | 80 ++++ ...defined_layout.rs => should_handle_all.rs} | 21 +- .../should_require_well_defined_layout.stderr | 135 ------ .../unknown_src_field.stderr | 2 +- tests/ui/transmutability/maybeuninit.rs | 43 ++ tests/ui/transmutability/maybeuninit.stderr | 18 + .../transmutability/references/unsafecell.rs | 47 ++ .../references/unsafecell.stderr | 33 ++ ...defined_layout.rs => should_handle_all.rs} | 29 +- .../should_require_well_defined_layout.stderr | 267 ----------- .../transmute_infinitely_recursive_type.rs | 1 + ...transmute_infinitely_recursive_type.stderr | 19 +- .../transmutability/transmute-padding-ice.rs | 9 +- .../transmute-padding-ice.stderr | 22 - tests/ui/transmutability/uninhabited.rs | 71 +++ tests/ui/transmutability/uninhabited.stderr | 86 ++++ .../unions/repr/should_handle_align.rs | 4 +- ...defined_layout.rs => should_handle_all.rs} | 24 +- .../unions/repr/should_handle_packed.rs | 1 - .../should_require_well_defined_layout.stderr | 47 -- 32 files changed, 905 insertions(+), 789 deletions(-) create mode 100644 tests/ui/transmutability/enums/niche_optimization.rs create mode 100644 tests/ui/transmutability/enums/repr/padding_differences.rs rename tests/ui/transmutability/enums/repr/{should_require_well_defined_layout.rs => should_handle_all.rs} (80%) delete mode 100644 tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr create mode 100644 tests/ui/transmutability/maybeuninit.rs create mode 100644 tests/ui/transmutability/maybeuninit.stderr create mode 100644 tests/ui/transmutability/references/unsafecell.rs create mode 100644 tests/ui/transmutability/references/unsafecell.stderr rename tests/ui/transmutability/structs/repr/{should_require_well_defined_layout.rs => should_handle_all.rs} (54%) delete mode 100644 tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr delete mode 100644 tests/ui/transmutability/transmute-padding-ice.stderr create mode 100644 tests/ui/transmutability/uninhabited.rs create mode 100644 tests/ui/transmutability/uninhabited.stderr rename tests/ui/transmutability/unions/repr/{should_require_well_defined_layout.rs => should_handle_all.rs} (58%) delete mode 100644 tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 0515b09ae46..cd3383dbfc2 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -40,7 +40,7 @@ pub struct ImplCandidate<'tcx> { enum GetSafeTransmuteErrorAndReason { Silent, - Error { err_msg: String, safe_transmute_explanation: String }, + Error { err_msg: String, safe_transmute_explanation: Option }, } struct UnsatisfiedConst(pub bool); diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 01f9c8bb5d1..3a989c46fc1 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -557,7 +557,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { GetSafeTransmuteErrorAndReason::Error { err_msg, safe_transmute_explanation, - } => (err_msg, Some(safe_transmute_explanation)), + } => (err_msg, safe_transmute_explanation), } } else { (err_msg, None) @@ -3061,28 +3061,33 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { return GetSafeTransmuteErrorAndReason::Silent; }; + let dst = trait_ref.args.type_at(0); + let src = trait_ref.args.type_at(1); + let err_msg = format!("`{src}` cannot be safely transmuted into `{dst}`"); + match rustc_transmute::TransmuteTypeEnv::new(self.infcx).is_transmutable( obligation.cause, src_and_dst, assume, ) { Answer::No(reason) => { - let dst = trait_ref.args.type_at(0); - let src = trait_ref.args.type_at(1); - let err_msg = format!("`{src}` cannot be safely transmuted into `{dst}`"); let safe_transmute_explanation = match reason { rustc_transmute::Reason::SrcIsNotYetSupported => { - format!("analyzing the transmutability of `{src}` is not yet supported.") + format!("analyzing the transmutability of `{src}` is not yet supported") } rustc_transmute::Reason::DstIsNotYetSupported => { - format!("analyzing the transmutability of `{dst}` is not yet supported.") + format!("analyzing the transmutability of `{dst}` is not yet supported") } rustc_transmute::Reason::DstIsBitIncompatible => { format!("at least one value of `{src}` isn't a bit-valid value of `{dst}`") } + rustc_transmute::Reason::DstUninhabited => { + format!("`{dst}` is uninhabited") + } + rustc_transmute::Reason::DstMayHaveSafetyInvariants => { format!("`{dst}` may carry safety invariants") } @@ -3128,14 +3133,23 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { format!("`{dst}` has an unknown layout") } }; - GetSafeTransmuteErrorAndReason::Error { err_msg, safe_transmute_explanation } + GetSafeTransmuteErrorAndReason::Error { + err_msg, + safe_transmute_explanation: Some(safe_transmute_explanation), + } } // Should never get a Yes at this point! We already ran it before, and did not get a Yes. Answer::Yes => span_bug!( span, "Inconsistent rustc_transmute::is_transmutable(...) result, got Yes", ), - other => span_bug!(span, "Unsupported rustc_transmute::Answer variant: `{other:?}`"), + // Reached when a different obligation (namely `Freeze`) causes the + // transmutability analysis to fail. In this case, silence the + // transmutability error message in favor of that more specific + // error. + Answer::If(_) => { + GetSafeTransmuteErrorAndReason::Error { err_msg, safe_transmute_explanation: None } + } } } diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index 6f512a1173f..a7cfcf6b366 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -314,12 +314,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { .flat_map(|cond| flatten_answer_tree(tcx, obligation, predicate, cond)) .collect(), Condition::IfTransmutable { src, dst } => { - let trait_def_id = obligation.predicate.def_id(); + let transmute_trait = obligation.predicate.def_id(); let assume_const = predicate.trait_ref.args.const_at(2); - let make_obl = |from_ty, to_ty| { - let trait_ref1 = ty::TraitRef::new( + let make_transmute_obl = |from_ty, to_ty| { + let trait_ref = ty::TraitRef::new( tcx, - trait_def_id, + transmute_trait, [ ty::GenericArg::from(to_ty), ty::GenericArg::from(from_ty), @@ -331,17 +331,45 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { obligation.cause.clone(), obligation.recursion_depth + 1, obligation.param_env, - trait_ref1, + trait_ref, ) }; + let make_freeze_obl = |ty| { + let trait_ref = ty::TraitRef::new( + tcx, + tcx.lang_items().freeze_trait().unwrap(), + [ty::GenericArg::from(ty)], + ); + Obligation::with_depth( + tcx, + obligation.cause.clone(), + obligation.recursion_depth + 1, + obligation.param_env, + trait_ref, + ) + }; + + let mut obls = vec![]; + + // If the source is a shared reference, it must be `Freeze`; + // otherwise, transmuting could lead to data races. + if src.mutability == Mutability::Not { + obls.extend([make_freeze_obl(src.ty), make_freeze_obl(dst.ty)]) + } + // If Dst is mutable, check bidirectionally. // For example, transmuting bool -> u8 is OK as long as you can't update that u8 // to be > 1, because you could later transmute the u8 back to a bool and get UB. match dst.mutability { - Mutability::Not => vec![make_obl(src.ty, dst.ty)], - Mutability::Mut => vec![make_obl(src.ty, dst.ty), make_obl(dst.ty, src.ty)], + Mutability::Not => obls.push(make_transmute_obl(src.ty, dst.ty)), + Mutability::Mut => obls.extend([ + make_transmute_obl(src.ty, dst.ty), + make_transmute_obl(dst.ty, src.ty), + ]), } + + obls } } } diff --git a/compiler/rustc_transmute/src/layout/dfa.rs b/compiler/rustc_transmute/src/layout/dfa.rs index b8922696e30..77d5a48f158 100644 --- a/compiler/rustc_transmute/src/layout/dfa.rs +++ b/compiler/rustc_transmute/src/layout/dfa.rs @@ -35,6 +35,7 @@ impl Transitions where R: Ref, { + #[allow(dead_code)] fn insert(&mut self, transition: Transition, state: State) { match transition { Transition::Byte(b) => { @@ -82,6 +83,7 @@ impl Dfa where R: Ref, { + #[allow(dead_code)] pub(crate) fn unit() -> Self { let transitions: Map> = Map::default(); let start = State::new(); diff --git a/compiler/rustc_transmute/src/layout/nfa.rs b/compiler/rustc_transmute/src/layout/nfa.rs index 78fcceb5f2c..3c5963202c6 100644 --- a/compiler/rustc_transmute/src/layout/nfa.rs +++ b/compiler/rustc_transmute/src/layout/nfa.rs @@ -160,6 +160,7 @@ where Self { transitions, start, accepting } } + #[allow(dead_code)] pub(crate) fn edges_from(&self, start: State) -> Option<&Map, Set>> { self.transitions.get(&start) } diff --git a/compiler/rustc_transmute/src/layout/tree.rs b/compiler/rustc_transmute/src/layout/tree.rs index f6bc224c7e7..12c984f1603 100644 --- a/compiler/rustc_transmute/src/layout/tree.rs +++ b/compiler/rustc_transmute/src/layout/tree.rs @@ -173,16 +173,20 @@ pub(crate) mod rustc { use super::Tree; use crate::layout::rustc::{Def, Ref}; + use rustc_middle::ty::layout::HasTyCtxt; + use rustc_middle::ty::layout::LayoutCx; use rustc_middle::ty::layout::LayoutError; + use rustc_middle::ty::layout::LayoutOf; use rustc_middle::ty::AdtDef; - use rustc_middle::ty::GenericArgsRef; - use rustc_middle::ty::ParamEnv; + use rustc_middle::ty::AdtKind; + use rustc_middle::ty::List; use rustc_middle::ty::ScalarInt; - use rustc_middle::ty::VariantDef; use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt}; use rustc_span::ErrorGuaranteed; - use rustc_target::abi::Align; - use std::alloc; + use rustc_target::abi::FieldsShape; + use rustc_target::abi::Size; + use rustc_target::abi::TyAndLayout; + use rustc_target::abi::Variants; #[derive(Debug, Copy, Clone)] pub(crate) enum Err { @@ -206,176 +210,64 @@ pub(crate) mod rustc { } } - trait LayoutExt { - fn clamp_align(&self, min_align: Align, max_align: Align) -> Self; - } - - impl LayoutExt for alloc::Layout { - fn clamp_align(&self, min_align: Align, max_align: Align) -> Self { - let min_align = min_align.bytes().try_into().unwrap(); - let max_align = max_align.bytes().try_into().unwrap(); - Self::from_size_align(self.size(), self.align().clamp(min_align, max_align)).unwrap() - } - } - - struct LayoutSummary { - total_align: Align, - total_size: usize, - discriminant_size: usize, - discriminant_align: Align, - } - - impl LayoutSummary { - fn from_ty<'tcx>(ty: Ty<'tcx>, ctx: TyCtxt<'tcx>) -> Result> { - use rustc_middle::ty::ParamEnvAnd; - use rustc_target::abi::{TyAndLayout, Variants}; - - let param_env = ParamEnv::reveal_all(); - let param_env_and_type = ParamEnvAnd { param_env, value: ty }; - let TyAndLayout { layout, .. } = ctx.layout_of(param_env_and_type)?; - - let total_size: usize = layout.size().bytes_usize(); - let total_align: Align = layout.align().abi; - let discriminant_align: Align; - let discriminant_size: usize; - - if let Variants::Multiple { tag, .. } = layout.variants() { - discriminant_align = tag.align(&ctx).abi; - discriminant_size = tag.size(&ctx).bytes_usize(); - } else { - discriminant_align = Align::ONE; - discriminant_size = 0; - }; - - Ok(Self { total_align, total_size, discriminant_align, discriminant_size }) - } - - fn into(&self) -> alloc::Layout { - alloc::Layout::from_size_align( - self.total_size, - self.total_align.bytes().try_into().unwrap(), - ) - .unwrap() - } - } - impl<'tcx> Tree, Ref<'tcx>> { - pub fn from_ty(ty: Ty<'tcx>, tcx: TyCtxt<'tcx>) -> Result { - use rustc_middle::ty::FloatTy::*; - use rustc_middle::ty::IntTy::*; - use rustc_middle::ty::UintTy::*; + pub fn from_ty( + ty_and_layout: TyAndLayout<'tcx, Ty<'tcx>>, + cx: LayoutCx<'tcx, TyCtxt<'tcx>>, + ) -> Result { use rustc_target::abi::HasDataLayout; - if let Err(e) = ty.error_reported() { + if let Err(e) = ty_and_layout.ty.error_reported() { return Err(Err::TypeError(e)); } - let target = tcx.data_layout(); + let target = cx.tcx.data_layout(); + let pointer_size = target.pointer_size; - match ty.kind() { + match ty_and_layout.ty.kind() { ty::Bool => Ok(Self::bool()), - ty::Int(I8) | ty::Uint(U8) => Ok(Self::u8()), - ty::Int(I16) | ty::Uint(U16) => Ok(Self::number(2)), - ty::Int(I32) | ty::Uint(U32) | ty::Float(F32) => Ok(Self::number(4)), - ty::Int(I64) | ty::Uint(U64) | ty::Float(F64) => Ok(Self::number(8)), - ty::Int(I128) | ty::Uint(U128) => Ok(Self::number(16)), - ty::Int(Isize) | ty::Uint(Usize) => { - Ok(Self::number(target.pointer_size.bytes_usize())) + ty::Float(nty) => { + let width = nty.bit_width() / 8; + Ok(Self::number(width as _)) } - ty::Tuple(members) => { - if members.len() == 0 { - Ok(Tree::unit()) - } else { - Err(Err::NotYetSupported) - } + ty::Int(nty) => { + let width = nty.normalize(pointer_size.bits() as _).bit_width().unwrap() / 8; + Ok(Self::number(width as _)) } - ty::Array(ty, len) => { - let len = len - .try_eval_target_usize(tcx, ParamEnv::reveal_all()) - .ok_or(Err::NotYetSupported)?; - let elt = Tree::from_ty(*ty, tcx)?; + ty::Uint(nty) => { + let width = nty.normalize(pointer_size.bits() as _).bit_width().unwrap() / 8; + Ok(Self::number(width as _)) + } + + ty::Tuple(members) => Self::from_tuple(ty_and_layout, members, cx), + + ty::Array(inner_ty, len) => { + let FieldsShape::Array { stride, count } = &ty_and_layout.fields else { + return Err(Err::NotYetSupported); + }; + let inner_ty_and_layout = cx.layout_of(*inner_ty)?; + assert_eq!(*stride, inner_ty_and_layout.size); + let elt = Tree::from_ty(inner_ty_and_layout, cx)?; Ok(std::iter::repeat(elt) - .take(len as usize) + .take(*count as usize) .fold(Tree::unit(), |tree, elt| tree.then(elt))) } - ty::Adt(adt_def, args_ref) => { - use rustc_middle::ty::AdtKind; - - // If the layout is ill-specified, halt. - if !(adt_def.repr().c() || adt_def.repr().int.is_some()) { - return Err(Err::NotYetSupported); + ty::Adt(adt_def, _args_ref) if !ty_and_layout.ty.is_box() => { + match adt_def.adt_kind() { + AdtKind::Struct => Self::from_struct(ty_and_layout, *adt_def, cx), + AdtKind::Enum => Self::from_enum(ty_and_layout, *adt_def, cx), + AdtKind::Union => Self::from_union(ty_and_layout, *adt_def, cx), } - - // Compute a summary of the type's layout. - let layout_summary = LayoutSummary::from_ty(ty, tcx)?; - - // The layout begins with this adt's visibility. - let vis = Self::def(Def::Adt(*adt_def)); - - // And is followed the layout(s) of its variants - Ok(vis.then(match adt_def.adt_kind() { - AdtKind::Struct => Self::from_repr_c_variant( - ty, - *adt_def, - args_ref, - &layout_summary, - None, - adt_def.non_enum_variant(), - tcx, - )?, - AdtKind::Enum => { - trace!(?adt_def, "treeifying enum"); - let mut tree = Tree::uninhabited(); - - for (idx, variant) in adt_def.variants().iter_enumerated() { - let tag = tcx.tag_for_variant((ty, idx)); - tree = tree.or(Self::from_repr_c_variant( - ty, - *adt_def, - args_ref, - &layout_summary, - tag, - variant, - tcx, - )?); - } - - tree - } - AdtKind::Union => { - // is the layout well-defined? - if !adt_def.repr().c() { - return Err(Err::NotYetSupported); - } - - let ty_layout = layout_of(tcx, ty)?; - - let mut tree = Tree::uninhabited(); - - for field in adt_def.all_fields() { - let variant_ty = field.ty(tcx, args_ref); - let variant_layout = layout_of(tcx, variant_ty)?; - let padding_needed = ty_layout.size() - variant_layout.size(); - let variant = Self::def(Def::Field(field)) - .then(Self::from_ty(variant_ty, tcx)?) - .then(Self::padding(padding_needed)); - - tree = tree.or(variant); - } - - tree - } - })) } ty::Ref(lifetime, ty, mutability) => { - let layout = layout_of(tcx, *ty)?; - let align = layout.align(); - let size = layout.size(); + let ty_and_layout = cx.layout_of(*ty)?; + let align = ty_and_layout.align.abi.bytes() as usize; + let size = ty_and_layout.size.bytes_usize(); Ok(Tree::Ref(Ref { lifetime: *lifetime, ty: *ty, @@ -389,80 +281,143 @@ pub(crate) mod rustc { } } - fn from_repr_c_variant( - ty: Ty<'tcx>, - adt_def: AdtDef<'tcx>, - args_ref: GenericArgsRef<'tcx>, - layout_summary: &LayoutSummary, - tag: Option, - variant_def: &'tcx VariantDef, - tcx: TyCtxt<'tcx>, + /// Constructs a `Tree` from a tuple. + fn from_tuple( + ty_and_layout: TyAndLayout<'tcx, Ty<'tcx>>, + members: &'tcx List>, + cx: LayoutCx<'tcx, TyCtxt<'tcx>>, ) -> Result { - let mut tree = Tree::unit(); - - let repr = adt_def.repr(); - let min_align = repr.align.unwrap_or(Align::ONE); - let max_align = repr.pack.unwrap_or(Align::MAX); - - let variant_span = trace_span!( - "treeifying variant", - min_align = ?min_align, - max_align = ?max_align, - ) - .entered(); - - let mut variant_layout = alloc::Layout::from_size_align( - 0, - layout_summary.total_align.bytes().try_into().unwrap(), - ) - .unwrap(); - - // The layout of the variant is prefixed by the tag, if any. - if let Some(tag) = tag { - let tag_layout = - alloc::Layout::from_size_align(tag.size().bytes_usize(), 1).unwrap(); - tree = tree.then(Self::from_tag(tag, tcx)); - variant_layout = variant_layout.extend(tag_layout).unwrap().0; - } - - // Next come fields. - let fields_span = trace_span!("treeifying fields").entered(); - for field_def in variant_def.fields.iter() { - let field_ty = field_def.ty(tcx, args_ref); - let _span = trace_span!("treeifying field", field = ?field_ty).entered(); - - // begin with the field's visibility - tree = tree.then(Self::def(Def::Field(field_def))); - - // compute the field's layout characteristics - let field_layout = layout_of(tcx, field_ty)?.clamp_align(min_align, max_align); - - // next comes the field's padding - let padding_needed = variant_layout.padding_needed_for(field_layout.align()); - if padding_needed > 0 { - tree = tree.then(Self::padding(padding_needed)); + match &ty_and_layout.fields { + FieldsShape::Primitive => { + assert_eq!(members.len(), 1); + let inner_ty = members[0]; + let inner_ty_and_layout = cx.layout_of(inner_ty)?; + assert_eq!(ty_and_layout.layout, inner_ty_and_layout.layout); + Self::from_ty(inner_ty_and_layout, cx) } - - // finally, the field's layout - tree = tree.then(Self::from_ty(field_ty, tcx)?); - - // extend the variant layout with the field layout - variant_layout = variant_layout.extend(field_layout).unwrap().0; + FieldsShape::Arbitrary { offsets, .. } => { + assert_eq!(offsets.len(), members.len()); + Self::from_variant(Def::Primitive, None, ty_and_layout, ty_and_layout.size, cx) + } + FieldsShape::Array { .. } | FieldsShape::Union(_) => Err(Err::NotYetSupported), } - drop(fields_span); - - // finally: padding - let padding_span = trace_span!("adding trailing padding").entered(); - if layout_summary.total_size > variant_layout.size() { - let padding_needed = layout_summary.total_size - variant_layout.size(); - tree = tree.then(Self::padding(padding_needed)); - }; - drop(padding_span); - drop(variant_span); - Ok(tree) } - pub fn from_tag(tag: ScalarInt, tcx: TyCtxt<'tcx>) -> Self { + /// Constructs a `Tree` from a struct. + /// + /// # Panics + /// + /// Panics if `def` is not a struct definition. + fn from_struct( + ty_and_layout: TyAndLayout<'tcx, Ty<'tcx>>, + def: AdtDef<'tcx>, + cx: LayoutCx<'tcx, TyCtxt<'tcx>>, + ) -> Result { + assert!(def.is_struct()); + let def = Def::Adt(def); + Self::from_variant(def, None, ty_and_layout, ty_and_layout.size, cx) + } + + /// Constructs a `Tree` from an enum. + /// + /// # Panics + /// + /// Panics if `def` is not an enum definition. + fn from_enum( + ty_and_layout: TyAndLayout<'tcx, Ty<'tcx>>, + def: AdtDef<'tcx>, + cx: LayoutCx<'tcx, TyCtxt<'tcx>>, + ) -> Result { + assert!(def.is_enum()); + let layout = ty_and_layout.layout; + + if let Variants::Multiple { tag_field, .. } = layout.variants() { + // For enums (but not coroutines), the tag field is + // currently always the first field of the layout. + assert_eq!(*tag_field, 0); + } + + let variants = def.discriminants(cx.tcx()).try_fold( + Self::uninhabited(), + |variants, (idx, ref discriminant)| { + let tag = cx.tcx.tag_for_variant((ty_and_layout.ty, idx)); + let variant_def = Def::Variant(def.variant(idx)); + let variant_ty_and_layout = ty_and_layout.for_variant(&cx, idx); + let variant = Self::from_variant( + variant_def, + tag, + variant_ty_and_layout, + layout.size, + cx, + )?; + Result::::Ok(variants.or(variant)) + }, + )?; + + return Ok(Self::def(Def::Adt(def)).then(variants)); + } + + /// Constructs a `Tree` from a 'variant-like' layout. + /// + /// A 'variant-like' layout includes those of structs and, of course, + /// enum variants. Pragmatically speaking, this method supports anything + /// with `FieldsShape::Arbitrary`. + /// + /// Note: This routine assumes that the optional `tag` is the first + /// field, and enum callers should check that `tag_field` is, in fact, + /// `0`. + fn from_variant( + def: Def<'tcx>, + tag: Option, + ty_and_layout: TyAndLayout<'tcx, Ty<'tcx>>, + total_size: Size, + cx: LayoutCx<'tcx, TyCtxt<'tcx>>, + ) -> Result { + // This constructor does not support non-`FieldsShape::Arbitrary` + // layouts. + let FieldsShape::Arbitrary { offsets, memory_index } = ty_and_layout.layout.fields() + else { + return Err(Err::NotYetSupported); + }; + + // When this function is invoked with enum variants, + // `ty_and_layout.size` does not encompass the entire size of the + // enum. We rely on `total_size` for this. + assert!(ty_and_layout.size <= total_size); + + let mut size = Size::ZERO; + let mut struct_tree = Self::def(def); + + // If a `tag` is provided, place it at the start of the layout. + if let Some(tag) = tag { + size += tag.size(); + struct_tree = struct_tree.then(Self::from_tag(tag, cx.tcx)); + } + + // Append the fields, in memory order, to the layout. + let inverse_memory_index = memory_index.invert_bijective_mapping(); + for (memory_idx, field_idx) in inverse_memory_index.iter_enumerated() { + // Add interfield padding. + let padding_needed = offsets[*field_idx] - size; + let padding = Self::padding(padding_needed.bytes_usize()); + + let field_ty_and_layout = ty_and_layout.field(&cx, field_idx.as_usize()); + let field_tree = Self::from_ty(field_ty_and_layout, cx)?; + + struct_tree = struct_tree.then(padding).then(field_tree); + + size += padding_needed + field_ty_and_layout.size; + } + + // Add trailing padding. + let padding_needed = total_size - size; + let trailing_padding = Self::padding(padding_needed.bytes_usize()); + + Ok(struct_tree.then(trailing_padding)) + } + + /// Constructs a `Tree` representing the value of a enum tag. + fn from_tag(tag: ScalarInt, tcx: TyCtxt<'tcx>) -> Self { use rustc_target::abi::Endian; let size = tag.size(); let bits = tag.to_bits(size).unwrap(); @@ -479,24 +434,42 @@ pub(crate) mod rustc { }; Self::Seq(bytes.iter().map(|&b| Self::from_bits(b)).collect()) } - } - fn layout_of<'tcx>( - ctx: TyCtxt<'tcx>, - ty: Ty<'tcx>, - ) -> Result> { - use rustc_middle::ty::ParamEnvAnd; - use rustc_target::abi::TyAndLayout; + /// Constructs a `Tree` from a union. + /// + /// # Panics + /// + /// Panics if `def` is not a union definition. + fn from_union( + ty_and_layout: TyAndLayout<'tcx, Ty<'tcx>>, + def: AdtDef<'tcx>, + cx: LayoutCx<'tcx, TyCtxt<'tcx>>, + ) -> Result { + assert!(def.is_union()); - let param_env = ParamEnv::reveal_all(); - let param_env_and_type = ParamEnvAnd { param_env, value: ty }; - let TyAndLayout { layout, .. } = ctx.layout_of(param_env_and_type)?; - let layout = alloc::Layout::from_size_align( - layout.size().bytes_usize(), - layout.align().abi.bytes().try_into().unwrap(), - ) - .unwrap(); - trace!(?ty, ?layout, "computed layout for type"); - Ok(layout) + let union_layout = ty_and_layout.layout; + + // This constructor does not support non-`FieldsShape::Union` + // layouts. Fields of this shape are all placed at offset 0. + let FieldsShape::Union(fields) = union_layout.fields() else { + return Err(Err::NotYetSupported); + }; + + let fields = &def.non_enum_variant().fields; + let fields = fields.iter_enumerated().try_fold( + Self::uninhabited(), + |fields, (idx, ref field_def)| { + let field_def = Def::Field(field_def); + let field_ty_and_layout = ty_and_layout.field(&cx, idx.as_usize()); + let field = Self::from_ty(field_ty_and_layout, cx)?; + let trailing_padding_needed = union_layout.size - field_ty_and_layout.size; + let trailing_padding = Self::padding(trailing_padding_needed.bytes_usize()); + let field_and_padding = field.then(trailing_padding); + Result::::Ok(fields.or(field_and_padding)) + }, + )?; + + Ok(Self::def(Def::Adt(def)).then(fields)) + } } } diff --git a/compiler/rustc_transmute/src/lib.rs b/compiler/rustc_transmute/src/lib.rs index b6f937ebe47..12312271646 100644 --- a/compiler/rustc_transmute/src/lib.rs +++ b/compiler/rustc_transmute/src/lib.rs @@ -1,6 +1,6 @@ #![feature(alloc_layout_extra)] #![feature(never_type)] -#![allow(dead_code, unused_variables)] +#![allow(unused_variables)] #[macro_use] extern crate tracing; @@ -49,6 +49,8 @@ pub enum Reason { DstIsNotYetSupported, /// The layout of the destination type is bit-incompatible with the source type. DstIsBitIncompatible, + /// The destination type is uninhabited. + DstUninhabited, /// The destination type may carry safety invariants. DstMayHaveSafetyInvariants, /// `Dst` is larger than `Src`, and the excess bytes were not exclusively uninitialized. diff --git a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs index 16d15580a05..2789fe8f6b1 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/mod.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/mod.rs @@ -33,6 +33,9 @@ mod rustc { use super::*; use crate::layout::tree::rustc::Err; + use rustc_middle::ty::layout::LayoutCx; + use rustc_middle::ty::layout::LayoutOf; + use rustc_middle::ty::ParamEnv; use rustc_middle::ty::Ty; use rustc_middle::ty::TyCtxt; @@ -43,12 +46,20 @@ mod rustc { pub fn answer(self) -> Answer< as QueryContext>::Ref> { let Self { src, dst, assume, context } = self; + let layout_cx = LayoutCx { tcx: context, param_env: ParamEnv::reveal_all() }; + let layout_of = |ty| { + layout_cx + .layout_of(ty) + .map_err(|_| Err::NotYetSupported) + .and_then(|tl| Tree::from_ty(tl, layout_cx)) + }; + // Convert `src` and `dst` from their rustc representations, to `Tree`-based // representations. If these conversions fail, conclude that the transmutation is // unacceptable; the layouts of both the source and destination types must be // well-defined. - let src = Tree::from_ty(src, context); - let dst = Tree::from_ty(dst, context); + let src = layout_of(src); + let dst = layout_of(dst); match (src, dst) { (Err(Err::TypeError(_)), _) | (_, Err(Err::TypeError(_))) => { @@ -86,6 +97,10 @@ where // references. let src = src.prune(&|def| false); + if src.is_inhabited() && !dst.is_inhabited() { + return Answer::No(Reason::DstUninhabited); + } + trace!(?src, "pruned src"); // Remove all `Def` nodes from `dst`, additionally... diff --git a/compiler/rustc_transmute/src/maybe_transmutable/query_context.rs b/compiler/rustc_transmute/src/maybe_transmutable/query_context.rs index 54ed03d44e6..1ccb6f36c8e 100644 --- a/compiler/rustc_transmute/src/maybe_transmutable/query_context.rs +++ b/compiler/rustc_transmute/src/maybe_transmutable/query_context.rs @@ -5,8 +5,6 @@ pub(crate) trait QueryContext { type Def: layout::Def; type Ref: layout::Ref; type Scope: Copy; - - fn min_align(&self, reference: Self::Ref) -> usize; } #[cfg(test)] @@ -31,10 +29,6 @@ pub(crate) mod test { type Def = Def; type Ref = !; type Scope = (); - - fn min_align(&self, reference: !) -> usize { - unimplemented!() - } } } @@ -48,9 +42,5 @@ mod rustc { type Ref = layout::rustc::Ref<'tcx>; type Scope = Ty<'tcx>; - - fn min_align(&self, reference: Self::Ref) -> usize { - unimplemented!() - } } } diff --git a/tests/ui/transmutability/arrays/huge-len.stderr b/tests/ui/transmutability/arrays/huge-len.stderr index 37160c5c959..3fc652f47c1 100644 --- a/tests/ui/transmutability/arrays/huge-len.stderr +++ b/tests/ui/transmutability/arrays/huge-len.stderr @@ -2,7 +2,7 @@ error[E0277]: `()` cannot be safely transmuted into `ExplicitlyPadded` --> $DIR/huge-len.rs:21:41 | LL | assert::is_maybe_transmutable::<(), ExplicitlyPadded>(); - | ^^^^^^^^^^^^^^^^ values of the type `ExplicitlyPadded` are too big for the current architecture + | ^^^^^^^^^^^^^^^^ analyzing the transmutability of `ExplicitlyPadded` is not yet supported | note: required by a bound in `is_maybe_transmutable` --> $DIR/huge-len.rs:8:14 @@ -17,7 +17,7 @@ error[E0277]: `ExplicitlyPadded` cannot be safely transmuted into `()` --> $DIR/huge-len.rs:24:55 | LL | assert::is_maybe_transmutable::(); - | ^^ values of the type `ExplicitlyPadded` are too big for the current architecture + | ^^ analyzing the transmutability of `ExplicitlyPadded` is not yet supported | note: required by a bound in `is_maybe_transmutable` --> $DIR/huge-len.rs:8:14 diff --git a/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr b/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr index e486928a445..b4cd70142c4 100644 --- a/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr +++ b/tests/ui/transmutability/arrays/should_require_well_defined_layout.stderr @@ -2,7 +2,7 @@ error[E0277]: `[String; 0]` cannot be safely transmuted into `()` --> $DIR/should_require_well_defined_layout.rs:25:52 | LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `[String; 0]` is not yet supported. + | ^^ analyzing the transmutability of `[String; 0]` is not yet supported | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:12:14 @@ -23,7 +23,7 @@ error[E0277]: `u128` cannot be safely transmuted into `[String; 0]` --> $DIR/should_require_well_defined_layout.rs:26:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `[String; 0]` is not yet supported. + | ^^^^^^^^^ analyzing the transmutability of `[String; 0]` is not yet supported | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:12:14 @@ -44,7 +44,7 @@ error[E0277]: `[String; 1]` cannot be safely transmuted into `()` --> $DIR/should_require_well_defined_layout.rs:31:52 | LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `[String; 1]` is not yet supported. + | ^^ analyzing the transmutability of `[String; 1]` is not yet supported | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:12:14 @@ -65,7 +65,7 @@ error[E0277]: `u128` cannot be safely transmuted into `[String; 1]` --> $DIR/should_require_well_defined_layout.rs:32:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `[String; 1]` is not yet supported. + | ^^^^^^^^^ analyzing the transmutability of `[String; 1]` is not yet supported | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:12:14 @@ -86,7 +86,7 @@ error[E0277]: `[String; 2]` cannot be safely transmuted into `()` --> $DIR/should_require_well_defined_layout.rs:37:52 | LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `[String; 2]` is not yet supported. + | ^^ analyzing the transmutability of `[String; 2]` is not yet supported | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:12:14 @@ -107,7 +107,7 @@ error[E0277]: `u128` cannot be safely transmuted into `[String; 2]` --> $DIR/should_require_well_defined_layout.rs:38:47 | LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `[String; 2]` is not yet supported. + | ^^^^^^^^^ analyzing the transmutability of `[String; 2]` is not yet supported | note: required by a bound in `is_maybe_transmutable` --> $DIR/should_require_well_defined_layout.rs:12:14 diff --git a/tests/ui/transmutability/enums/niche_optimization.rs b/tests/ui/transmutability/enums/niche_optimization.rs new file mode 100644 index 00000000000..23f57ecad75 --- /dev/null +++ b/tests/ui/transmutability/enums/niche_optimization.rs @@ -0,0 +1,156 @@ +//@ check-pass +//! Checks that niche optimizations are encoded correctly. +#![crate_type = "lib"] +#![feature(transmutability)] +#![allow(dead_code, incomplete_features, non_camel_case_types)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + + pub fn is_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +#[repr(u8)] enum V0 { V = 0 } +#[repr(u8)] enum V1 { V = 1 } +#[repr(u8)] enum V2 { V = 2 } +#[repr(u8)] enum V253 { V = 253 } +#[repr(u8)] enum V254 { V = 254 } +#[repr(u8)] enum V255 { V = 255 } + +fn bool() { + enum OptionLike { + A(bool), + B, + } + + const _: () = { + assert!(std::mem::size_of::() == 1); + }; + + assert::is_transmutable::(); + + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::(); +} + +fn one_niche() { + #[repr(u8)] + enum N1 { + S = 0, + E = 255 - 1, + } + + enum OptionLike { + A(N1), + B, + } + + const _: () = { + assert!(std::mem::size_of::() == 1); + }; + + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::(); +} + +fn one_niche_alt() { + #[repr(u8)] + enum N1 { + S = 1, + E = 255 - 1, + } + + enum OptionLike { + A(N1), + B, + C, + } + + const _: () = { + assert!(std::mem::size_of::() == 1); + }; + + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::(); +} + +fn two_niche() { + #[repr(u8)] + enum Niche { + S = 0, + E = 255 - 2, + } + + enum OptionLike { + A(Niche), + B, + C, + } + + const _: () = { + assert!(std::mem::size_of::() == 1); + }; + + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::(); +} + +fn no_niche() { + use std::mem::MaybeUninit; + + #[repr(u8)] + enum Niche { + S = 0, + E = 255 - 1, + } + + enum OptionLike { + A(Niche), + B, + C, + } + + const _: () = { + assert!(std::mem::size_of::() == 2); + }; + + #[repr(C)] + struct Pair(T, U); + + assert::is_transmutable::(); + assert::is_transmutable::(); + assert::is_transmutable::, OptionLike>(); + assert::is_transmutable::>, OptionLike>(); + assert::is_transmutable::>, OptionLike>(); +} diff --git a/tests/ui/transmutability/enums/repr/padding_differences.rs b/tests/ui/transmutability/enums/repr/padding_differences.rs new file mode 100644 index 00000000000..d0e1502b5e2 --- /dev/null +++ b/tests/ui/transmutability/enums/repr/padding_differences.rs @@ -0,0 +1,80 @@ +//@ check-pass +//! Adapted from https://rust-lang.github.io/unsafe-code-guidelines/layout/enums.html#explicit-repr-annotation-without-c-compatibility +#![crate_type = "lib"] +#![feature(transmutability)] +#![allow(dead_code, incomplete_features, non_camel_case_types)] + +use std::mem::MaybeUninit; + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + + pub fn is_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +#[repr(u8)] enum V0 { V = 0 } +#[repr(u8)] enum V1 { V = 1 } + +fn repr_u8() { + #[repr(u8)] + enum TwoCases { + A(u8, u16), // 0x00 INIT INIT INIT + B(u16), // 0x01 PADD INIT INIT + } + + const _: () = { + assert!(std::mem::size_of::() == 4); + }; + + #[repr(C)] struct TwoCasesA(V0, u8, u8, u8); + #[repr(C)] struct TwoCasesB(V1, MaybeUninit, u8, u8); + + assert::is_transmutable::(); + assert::is_transmutable::(); + + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); +} + +fn repr_c_u8() { + #[repr(C, u8)] + enum TwoCases { + A(u8, u16), // 0x00 PADD INIT PADD INIT INIT + B(u16), // 0x01 PADD INIT INIT PADD PADD + } + + const _: () = { + assert!(std::mem::size_of::() == 6); + }; + + #[repr(C)] struct TwoCasesA(V0, MaybeUninit, u8, MaybeUninit, u8, u8); + #[repr(C)] struct TwoCasesB(V1, MaybeUninit, u8, u8, MaybeUninit, MaybeUninit); + + assert::is_transmutable::(); + assert::is_transmutable::(); + + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); +} diff --git a/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.rs b/tests/ui/transmutability/enums/repr/should_handle_all.rs similarity index 80% rename from tests/ui/transmutability/enums/repr/should_require_well_defined_layout.rs rename to tests/ui/transmutability/enums/repr/should_handle_all.rs index 630e662b926..a8ec86fa40d 100644 --- a/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.rs +++ b/tests/ui/transmutability/enums/repr/should_handle_all.rs @@ -1,5 +1,4 @@ -//! An enum must have a well-defined layout to participate in a transmutation. - +//@ check-pass #![crate_type = "lib"] #![feature(repr128)] #![feature(transmutability)] @@ -21,23 +20,17 @@ mod assert { {} } -fn should_reject_repr_rust() { - fn void() { - enum repr_rust {} - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - } - +fn should_accept_repr_rust() { fn singleton() { enum repr_rust { V } - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); } fn duplex() { enum repr_rust { A, B } - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); } } @@ -116,7 +109,7 @@ fn should_accept_primitive_reprs() } } -fn should_accept_repr_C() { +fn should_accept_repr_c() { #[repr(C)] enum repr_c { V } assert::is_maybe_transmutable::(); assert::is_maybe_transmutable::(); diff --git a/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr b/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr deleted file mode 100644 index 2a683de6a65..00000000000 --- a/tests/ui/transmutability/enums/repr/should_require_well_defined_layout.stderr +++ /dev/null @@ -1,135 +0,0 @@ -error[E0277]: `void::repr_rust` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:27:52 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `void::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:13:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `void::repr_rust` - --> $DIR/should_require_well_defined_layout.rs:28:47 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `void::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:13:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `singleton::repr_rust` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:33:52 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `singleton::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:13:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `singleton::repr_rust` - --> $DIR/should_require_well_defined_layout.rs:34:47 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `singleton::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:13:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `duplex::repr_rust` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:39:52 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `duplex::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:13:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `duplex::repr_rust` - --> $DIR/should_require_well_defined_layout.rs:40:47 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `duplex::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:13:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error: aborting due to 6 previous errors - -For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.stderr b/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.stderr index eeed8a62a2a..cabc7bcfef7 100644 --- a/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.stderr +++ b/tests/ui/transmutability/malformed-program-gracefulness/unknown_src_field.stderr @@ -8,7 +8,7 @@ error[E0277]: `Src` cannot be safely transmuted into `Dst` --> $DIR/unknown_src_field.rs:19:36 | LL | assert::is_transmutable::(); - | ^^^ `Dst` has an unknown layout + | ^^^ analyzing the transmutability of `Dst` is not yet supported | note: required by a bound in `is_transmutable` --> $DIR/unknown_src_field.rs:12:14 diff --git a/tests/ui/transmutability/maybeuninit.rs b/tests/ui/transmutability/maybeuninit.rs new file mode 100644 index 00000000000..77c3381c774 --- /dev/null +++ b/tests/ui/transmutability/maybeuninit.rs @@ -0,0 +1,43 @@ +#![crate_type = "lib"] +#![feature(transmutability)] +#![allow(dead_code, incomplete_features, non_camel_case_types)] + +use std::mem::MaybeUninit; + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn validity() { + // An initialized byte is a valid uninitialized byte. + assert::is_maybe_transmutable::>(); + + // An uninitialized byte is never a valid initialized byte. + assert::is_maybe_transmutable::, u8>(); //~ ERROR: cannot be safely transmuted +} + +fn padding() { + #[repr(align(8))] + struct Align8; + + #[repr(u8)] + enum ImplicitlyPadded { + A(Align8), + } + + #[repr(u8)] + enum V0 { + V0 = 0, + } + + #[repr(C)] + struct ExplicitlyPadded(V0, MaybeUninit<[u8; 7]>); + + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); +} diff --git a/tests/ui/transmutability/maybeuninit.stderr b/tests/ui/transmutability/maybeuninit.stderr new file mode 100644 index 00000000000..be7dcaf35ea --- /dev/null +++ b/tests/ui/transmutability/maybeuninit.stderr @@ -0,0 +1,18 @@ +error[E0277]: `MaybeUninit` cannot be safely transmuted into `u8` + --> $DIR/maybeuninit.rs:21:54 + | +LL | assert::is_maybe_transmutable::, u8>(); + | ^^ at least one value of `MaybeUninit` isn't a bit-valid value of `u8` + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/maybeuninit.rs:12:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `is_maybe_transmutable` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/references/unsafecell.rs b/tests/ui/transmutability/references/unsafecell.rs new file mode 100644 index 00000000000..a8a1f969fb4 --- /dev/null +++ b/tests/ui/transmutability/references/unsafecell.rs @@ -0,0 +1,47 @@ +#![crate_type = "lib"] +#![feature(transmutability)] +#![allow(dead_code, incomplete_features, non_camel_case_types)] + +use std::cell::UnsafeCell; + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn value_to_value() { + // We accept value-to-value transmutations of `UnsafeCell`-containing types, + // because owning a value implies exclusive access. + assert::is_maybe_transmutable::, u8>(); + assert::is_maybe_transmutable::>(); + assert::is_maybe_transmutable::, UnsafeCell>(); +} + +fn ref_to_ref() { + // We forbid `UnsafeCell`-containing ref-to-ref transmutations, because the + // two types may use different, incompatible synchronization strategies. + assert::is_maybe_transmutable::<&'static u8, &'static UnsafeCell>(); //~ ERROR: cannot be safely transmuted + + assert::is_maybe_transmutable::<&'static UnsafeCell, &'static UnsafeCell>(); //~ ERROR: cannot be safely transmuted +} + +fn mut_to_mut() { + // `UnsafeCell` does't matter for `&mut T` to `&mut U`, since exclusive + // borrows can't be used for shared access. + assert::is_maybe_transmutable::<&'static mut u8, &'static mut UnsafeCell>(); + assert::is_maybe_transmutable::<&'static mut UnsafeCell, &'static mut u8>(); + assert::is_maybe_transmutable::<&'static mut UnsafeCell, &'static mut UnsafeCell>(); +} + +fn mut_to_ref() { + // We don't care about `UnsafeCell` for transmutations in the form `&mut T + // -> &U`, because downgrading a `&mut T` to a `&U` deactivates `&mut T` for + // the lifetime of `&U`. + assert::is_maybe_transmutable::<&'static mut u8, &'static UnsafeCell>(); + assert::is_maybe_transmutable::<&'static mut UnsafeCell, &'static u8>(); + assert::is_maybe_transmutable::<&'static mut UnsafeCell, &'static UnsafeCell>(); +} diff --git a/tests/ui/transmutability/references/unsafecell.stderr b/tests/ui/transmutability/references/unsafecell.stderr new file mode 100644 index 00000000000..651eb8ceb26 --- /dev/null +++ b/tests/ui/transmutability/references/unsafecell.stderr @@ -0,0 +1,33 @@ +error[E0277]: `&u8` cannot be safely transmuted into `&UnsafeCell` + --> $DIR/unsafecell.rs:27:50 + | +LL | assert::is_maybe_transmutable::<&'static u8, &'static UnsafeCell>(); + | ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Freeze` is not implemented for `&'static UnsafeCell` + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/unsafecell.rs:12:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `is_maybe_transmutable` + +error[E0277]: `&UnsafeCell` cannot be safely transmuted into `&UnsafeCell` + --> $DIR/unsafecell.rs:29:62 + | +LL | assert::is_maybe_transmutable::<&'static UnsafeCell, &'static UnsafeCell>(); + | ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Freeze` is not implemented for `&'static UnsafeCell` + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/unsafecell.rs:12:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `is_maybe_transmutable` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.rs b/tests/ui/transmutability/structs/repr/should_handle_all.rs similarity index 54% rename from tests/ui/transmutability/structs/repr/should_require_well_defined_layout.rs rename to tests/ui/transmutability/structs/repr/should_handle_all.rs index 2e673601baf..52c24eecf12 100644 --- a/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.rs +++ b/tests/ui/transmutability/structs/repr/should_handle_all.rs @@ -1,3 +1,4 @@ +//@ check-pass //! A struct must have a well-defined layout to participate in a transmutation. #![crate_type = "lib"] @@ -20,47 +21,47 @@ mod assert { {} } -fn should_reject_repr_rust() +fn should_accept_repr_rust() { fn unit() { struct repr_rust; - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); } fn tuple() { struct repr_rust(); - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); } fn braces() { struct repr_rust{} - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); } fn aligned() { #[repr(align(1))] struct repr_rust{} - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); } fn packed() { #[repr(packed)] struct repr_rust{} - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); } fn nested() { struct repr_rust; #[repr(C)] struct repr_c(repr_rust); - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); } } -fn should_accept_repr_C() +fn should_accept_repr_c() { fn unit() { #[repr(C)] struct repr_c; diff --git a/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr b/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr deleted file mode 100644 index 77788f72c21..00000000000 --- a/tests/ui/transmutability/structs/repr/should_require_well_defined_layout.stderr +++ /dev/null @@ -1,267 +0,0 @@ -error[E0277]: `should_reject_repr_rust::unit::repr_rust` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:27:52 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `should_reject_repr_rust::unit::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust::unit::repr_rust` - --> $DIR/should_require_well_defined_layout.rs:28:47 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `should_reject_repr_rust::unit::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `should_reject_repr_rust::tuple::repr_rust` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:33:52 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `should_reject_repr_rust::tuple::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust::tuple::repr_rust` - --> $DIR/should_require_well_defined_layout.rs:34:47 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `should_reject_repr_rust::tuple::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `should_reject_repr_rust::braces::repr_rust` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:39:52 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `should_reject_repr_rust::braces::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust::braces::repr_rust` - --> $DIR/should_require_well_defined_layout.rs:40:47 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `should_reject_repr_rust::braces::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `aligned::repr_rust` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:45:52 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `aligned::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `aligned::repr_rust` - --> $DIR/should_require_well_defined_layout.rs:46:47 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `aligned::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `packed::repr_rust` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:51:52 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `packed::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `packed::repr_rust` - --> $DIR/should_require_well_defined_layout.rs:52:47 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `packed::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `nested::repr_c` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:58:49 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `nested::repr_c` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `nested::repr_c` - --> $DIR/should_require_well_defined_layout.rs:59:47 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^ analyzing the transmutability of `nested::repr_c` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error: aborting due to 12 previous errors - -For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/structs/repr/transmute_infinitely_recursive_type.rs b/tests/ui/transmutability/structs/repr/transmute_infinitely_recursive_type.rs index 4c285a616b3..64110753832 100644 --- a/tests/ui/transmutability/structs/repr/transmute_infinitely_recursive_type.rs +++ b/tests/ui/transmutability/structs/repr/transmute_infinitely_recursive_type.rs @@ -22,4 +22,5 @@ fn should_pad_explicitly_packed_field() { //~^ ERROR: recursive type assert::is_maybe_transmutable::(); + //~^ ERROR: cannot be safely transmuted } diff --git a/tests/ui/transmutability/structs/repr/transmute_infinitely_recursive_type.stderr b/tests/ui/transmutability/structs/repr/transmute_infinitely_recursive_type.stderr index 7fb051f6625..ebfb5361143 100644 --- a/tests/ui/transmutability/structs/repr/transmute_infinitely_recursive_type.stderr +++ b/tests/ui/transmutability/structs/repr/transmute_infinitely_recursive_type.stderr @@ -15,7 +15,22 @@ error[E0391]: cycle detected when computing layout of `should_pad_explicitly_pac = note: cycle used when evaluating trait selection obligation `(): core::mem::transmutability::BikeshedIntrinsicFrom` = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information -error: aborting due to 2 previous errors +error[E0277]: `ExplicitlyPadded` cannot be safely transmuted into `()` + --> $DIR/transmute_infinitely_recursive_type.rs:24:55 + | +LL | assert::is_maybe_transmutable::(); + | ^^ analyzing the transmutability of `ExplicitlyPadded` is not yet supported + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/transmute_infinitely_recursive_type.rs:14:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `is_maybe_transmutable` -Some errors have detailed explanations: E0072, E0391. +error: aborting due to 3 previous errors + +Some errors have detailed explanations: E0072, E0277, E0391. For more information about an error, try `rustc --explain E0072`. diff --git a/tests/ui/transmutability/transmute-padding-ice.rs b/tests/ui/transmutability/transmute-padding-ice.rs index 3f3f75bc086..f5935a0009e 100644 --- a/tests/ui/transmutability/transmute-padding-ice.rs +++ b/tests/ui/transmutability/transmute-padding-ice.rs @@ -1,7 +1,13 @@ +//@ check-pass +//! This UI test was introduced as check-fail by a buggy bug-fix for an ICE. In +//! fact, this transmutation should be valid. + #![crate_type = "lib"] #![feature(transmutability)] #![allow(dead_code)] +use std::mem::size_of; + mod assert { use std::mem::{Assume, BikeshedIntrinsicFrom}; @@ -22,6 +28,7 @@ fn test() { #[repr(C)] struct B(u8, u8); + assert_eq!(size_of::(), size_of::()); + assert::is_maybe_transmutable::(); - //~^ ERROR cannot be safely transmuted } diff --git a/tests/ui/transmutability/transmute-padding-ice.stderr b/tests/ui/transmutability/transmute-padding-ice.stderr deleted file mode 100644 index 4c121d463c6..00000000000 --- a/tests/ui/transmutability/transmute-padding-ice.stderr +++ /dev/null @@ -1,22 +0,0 @@ -error[E0277]: `B` cannot be safely transmuted into `A` - --> $DIR/transmute-padding-ice.rs:25:40 - | -LL | assert::is_maybe_transmutable::(); - | ^ the size of `B` is smaller than the size of `A` - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/transmute-padding-ice.rs:10:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom< - | ______________^ -LL | | Src, -LL | | { Assume { alignment: true, lifetimes: true, safety: true, validity: true } }, -LL | | >, - | |_________^ required by this bound in `is_maybe_transmutable` - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0277`. diff --git a/tests/ui/transmutability/uninhabited.rs b/tests/ui/transmutability/uninhabited.rs new file mode 100644 index 00000000000..b61b110f6a1 --- /dev/null +++ b/tests/ui/transmutability/uninhabited.rs @@ -0,0 +1,71 @@ +#![crate_type = "lib"] +#![feature(transmutability)] +#![allow(dead_code, incomplete_features, non_camel_case_types)] + +mod assert { + use std::mem::{Assume, BikeshedIntrinsicFrom}; + + pub fn is_maybe_transmutable() + where + Dst: BikeshedIntrinsicFrom + {} +} + +fn void() { + enum Void {} + + // This transmutation is vacuously acceptable; since one cannot construct a + // `Void`, unsoundness cannot directly arise from transmuting a void into + // anything else. + assert::is_maybe_transmutable::(); + + assert::is_maybe_transmutable::<(), Void>(); //~ ERROR: cannot be safely transmuted +} + +// Non-ZST uninhabited types are, nonetheless, uninhabited. +fn yawning_void() { + enum Void {} + + struct YawningVoid(Void, u128); + + const _: () = { + assert!(std::mem::size_of::() == std::mem::size_of::()); + // Just to be sure the above constant actually evaluated: + assert!(false); //~ ERROR: evaluation of constant value failed + }; + + // This transmutation is vacuously acceptable; since one cannot construct a + // `Void`, unsoundness cannot directly arise from transmuting a void into + // anything else. + assert::is_maybe_transmutable::(); + + assert::is_maybe_transmutable::<(), Void>(); //~ ERROR: cannot be safely transmuted +} + +// References to uninhabited types are, logically, uninhabited, but for layout +// purposes are not ZSTs, and aren't treated as uninhabited when they appear in +// enum variants. +fn distant_void() { + enum Void {} + + enum DistantVoid { + A(&'static Void) + } + + const _: () = { + assert!(std::mem::size_of::() == std::mem::size_of::()); + // Just to be sure the above constant actually evaluated: + assert!(false); //~ ERROR: evaluation of constant value failed + }; + + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); //~ ERROR: cannot be safely transmuted +} diff --git a/tests/ui/transmutability/uninhabited.stderr b/tests/ui/transmutability/uninhabited.stderr new file mode 100644 index 00000000000..60219b0f263 --- /dev/null +++ b/tests/ui/transmutability/uninhabited.stderr @@ -0,0 +1,86 @@ +error[E0080]: evaluation of constant value failed + --> $DIR/uninhabited.rs:41:9 + | +LL | assert!(false); + | ^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: false', $DIR/uninhabited.rs:41:9 + | + = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0080]: evaluation of constant value failed + --> $DIR/uninhabited.rs:65:9 + | +LL | assert!(false); + | ^^^^^^^^^^^^^^ the evaluated program panicked at 'assertion failed: false', $DIR/uninhabited.rs:65:9 + | + = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: `()` cannot be safely transmuted into `void::Void` + --> $DIR/uninhabited.rs:29:41 + | +LL | assert::is_maybe_transmutable::<(), Void>(); + | ^^^^ `void::Void` is uninhabited + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/uninhabited.rs:10:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | |__________^ required by this bound in `is_maybe_transmutable` + +error[E0277]: `()` cannot be safely transmuted into `yawning_void::Void` + --> $DIR/uninhabited.rs:49:41 + | +LL | assert::is_maybe_transmutable::<(), Void>(); + | ^^^^ `yawning_void::Void` is uninhabited + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/uninhabited.rs:10:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | |__________^ required by this bound in `is_maybe_transmutable` + +error[E0277]: `u128` cannot be safely transmuted into `DistantVoid` + --> $DIR/uninhabited.rs:70:43 + | +LL | assert::is_maybe_transmutable::(); + | ^^^^^^^^^^^ at least one value of `u128` isn't a bit-valid value of `DistantVoid` + | +note: required by a bound in `is_maybe_transmutable` + --> $DIR/uninhabited.rs:10:14 + | +LL | pub fn is_maybe_transmutable() + | --------------------- required by a bound in this function +LL | where +LL | Dst: BikeshedIntrinsicFrom + | |__________^ required by this bound in `is_maybe_transmutable` + +error: aborting due to 5 previous errors + +Some errors have detailed explanations: E0080, E0277. +For more information about an error, try `rustc --explain E0080`. diff --git a/tests/ui/transmutability/unions/repr/should_handle_align.rs b/tests/ui/transmutability/unions/repr/should_handle_align.rs index 8668cca3cb5..ba4e904e161 100644 --- a/tests/ui/transmutability/unions/repr/should_handle_align.rs +++ b/tests/ui/transmutability/unions/repr/should_handle_align.rs @@ -25,13 +25,13 @@ fn should_pad_explicitly_aligned_field() { #[derive(Clone, Copy)] #[repr(u8)] enum V0u8 { V = 0 } #[derive(Clone, Copy)] #[repr(u8)] enum V1u8 { V = 1 } - #[repr(C)] + #[repr(align(1))] pub union Uninit { a: (), b: V1u8, } - #[repr(C, align(2))] + #[repr(align(2))] pub union align_2 { a: V0u8, } diff --git a/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.rs b/tests/ui/transmutability/unions/repr/should_handle_all.rs similarity index 58% rename from tests/ui/transmutability/unions/repr/should_require_well_defined_layout.rs rename to tests/ui/transmutability/unions/repr/should_handle_all.rs index 8495b0ea88f..85d48dd9b7f 100644 --- a/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.rs +++ b/tests/ui/transmutability/unions/repr/should_handle_all.rs @@ -1,7 +1,7 @@ -//! A struct must have a well-defined layout to participate in a transmutation. +//@ check-pass #![crate_type = "lib"] -#![feature(transmutability)] +#![feature(transmutability, transparent_unions)] #![allow(dead_code, incomplete_features, non_camel_case_types)] mod assert { @@ -20,17 +20,17 @@ mod assert { {} } -fn should_reject_repr_rust() +fn should_accept_repr_rust() { union repr_rust { a: u8 } - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted - assert::is_maybe_transmutable::(); //~ ERROR cannot be safely transmuted + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); } -fn should_accept_repr_C() +fn should_accept_repr_c() { #[repr(C)] union repr_c { @@ -41,3 +41,15 @@ fn should_accept_repr_C() assert::is_maybe_transmutable::(); assert::is_maybe_transmutable::(); } + + +fn should_accept_transparent() +{ + #[repr(transparent)] + union repr_transparent { + a: u8 + } + + assert::is_maybe_transmutable::(); + assert::is_maybe_transmutable::(); +} diff --git a/tests/ui/transmutability/unions/repr/should_handle_packed.rs b/tests/ui/transmutability/unions/repr/should_handle_packed.rs index 4af6c1d3a61..fc06eba4353 100644 --- a/tests/ui/transmutability/unions/repr/should_handle_packed.rs +++ b/tests/ui/transmutability/unions/repr/should_handle_packed.rs @@ -27,7 +27,6 @@ fn should_pad_explicitly_packed_field() { #[derive(Clone, Copy)] #[repr(u8)] enum V2u8 { V = 2 } #[derive(Clone, Copy)] #[repr(u32)] enum V3u32 { V = 3 } - #[repr(C)] pub union Uninit { a: (), b: V1u8, diff --git a/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr b/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr deleted file mode 100644 index bec07f13103..00000000000 --- a/tests/ui/transmutability/unions/repr/should_require_well_defined_layout.stderr +++ /dev/null @@ -1,47 +0,0 @@ -error[E0277]: `should_reject_repr_rust::repr_rust` cannot be safely transmuted into `()` - --> $DIR/should_require_well_defined_layout.rs:29:48 - | -LL | assert::is_maybe_transmutable::(); - | ^^ analyzing the transmutability of `should_reject_repr_rust::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error[E0277]: `u128` cannot be safely transmuted into `should_reject_repr_rust::repr_rust` - --> $DIR/should_require_well_defined_layout.rs:30:43 - | -LL | assert::is_maybe_transmutable::(); - | ^^^^^^^^^ analyzing the transmutability of `should_reject_repr_rust::repr_rust` is not yet supported. - | -note: required by a bound in `is_maybe_transmutable` - --> $DIR/should_require_well_defined_layout.rs:12:14 - | -LL | pub fn is_maybe_transmutable() - | --------------------- required by a bound in this function -LL | where -LL | Dst: BikeshedIntrinsicFrom - | |__________^ required by this bound in `is_maybe_transmutable` - -error: aborting due to 2 previous errors - -For more information about this error, try `rustc --explain E0277`. From 284da5d6b44ffaf22db50f4c30f5d941a471b26c Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Mon, 8 Apr 2024 16:37:06 +0000 Subject: [PATCH 14/14] CFI: Fix ICE in KCFI non-associated function pointers We oddly weren't testing the more usual case of casting non-methods to function pointers. The KCFI shim insertion logic would ICE on these due to asking for an irrefutable associated item if we cast a function to a function pointer without needing a traditional shim. --- compiler/rustc_middle/src/ty/instance.rs | 5 ++++- .../{cfi-method-fn-ptr-cast.rs => cfi-fn-ptr.rs} | 8 ++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) rename tests/ui/sanitizer/{cfi-method-fn-ptr-cast.rs => cfi-fn-ptr.rs} (85%) diff --git a/compiler/rustc_middle/src/ty/instance.rs b/compiler/rustc_middle/src/ty/instance.rs index 4a7720b38f8..f8f59fbeab4 100644 --- a/compiler/rustc_middle/src/ty/instance.rs +++ b/compiler/rustc_middle/src/ty/instance.rs @@ -520,7 +520,10 @@ impl<'tcx> Instance<'tcx> { // Reify `Trait::method` implementations if KCFI is enabled // FIXME(maurer) only reify it if it is a vtable-safe function _ if tcx.sess.is_sanitizer_kcfi_enabled() - && tcx.associated_item(def_id).trait_item_def_id.is_some() => + && tcx + .opt_associated_item(def_id) + .and_then(|assoc| assoc.trait_item_def_id) + .is_some() => { // If this function could also go in a vtable, we need to `ReifyShim` it with // KCFI because it can only attach one type per function. diff --git a/tests/ui/sanitizer/cfi-method-fn-ptr-cast.rs b/tests/ui/sanitizer/cfi-fn-ptr.rs similarity index 85% rename from tests/ui/sanitizer/cfi-method-fn-ptr-cast.rs rename to tests/ui/sanitizer/cfi-fn-ptr.rs index 8f79de11748..505b4b8e7f0 100644 --- a/tests/ui/sanitizer/cfi-method-fn-ptr-cast.rs +++ b/tests/ui/sanitizer/cfi-fn-ptr.rs @@ -1,4 +1,4 @@ -// Verifies that casting a method to a function pointer works. +// Verifies that casting to a function pointer works. //@ revisions: cfi kcfi // FIXME(#122848) Remove only-linux once OSX CFI binaries work @@ -46,6 +46,8 @@ impl Trait1 for Type1 { fn foo(&self) {} } +fn foo(_: &T) {} + fn main() { let type1 = Type1 {}; let f = ::foo; @@ -53,5 +55,7 @@ fn main() { // Check again with different optimization barriers S2 { f: ::foo }.foo(&S); // Check mismatched #[track_caller] - S2 { f: ::bar }.foo(&S) + S2 { f: ::bar }.foo(&S); + // Check non-method functions + S2 { f: foo }.foo(&S) }