From 347ca50bc82734b45ed1834fd3a15ed978aba258 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 17 Mar 2024 13:28:31 +0100 Subject: [PATCH] mentioned items: also handle vtables --- compiler/rustc_middle/src/mir/mod.rs | 6 +- .../src/mentioned_items.rs | 26 ++++++- compiler/rustc_monomorphize/src/collector.rs | 75 +++++++++++-------- .../collect-in-dead-vtable.noopt.stderr | 10 +-- .../collect-in-dead-vtable.opt.stderr | 23 ++++++ .../required-consts/collect-in-dead-vtable.rs | 12 +-- 6 files changed, 107 insertions(+), 45 deletions(-) create mode 100644 tests/ui/consts/required-consts/collect-in-dead-vtable.opt.stderr diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index c4e3a12acef0c..f7451e4c9164e 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -319,7 +319,11 @@ impl<'tcx> CoroutineInfo<'tcx> { pub enum MentionedItem<'tcx> { Fn(DefId, GenericArgsRef<'tcx>), Drop(Ty<'tcx>), - // FIXME: add Vtable { source_ty: Ty<'tcx>, target_ty: Ty<'tcx> }, + /// Unsizing casts might require vtables, so we have to record them. + UnsizeCast { + source_ty: Ty<'tcx>, + target_ty: Ty<'tcx>, + }, // FIXME: do we have to add closures? } diff --git a/compiler/rustc_mir_transform/src/mentioned_items.rs b/compiler/rustc_mir_transform/src/mentioned_items.rs index ed363b4f2527a..0e0114c9d2c8c 100644 --- a/compiler/rustc_mir_transform/src/mentioned_items.rs +++ b/compiler/rustc_mir_transform/src/mentioned_items.rs @@ -1,6 +1,6 @@ use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::{self, ConstOperand, Location, MentionedItem, MirPass}; -use rustc_middle::ty::{self, TyCtxt}; +use rustc_middle::ty::{self, adjustment::PointerCoercion, TyCtxt}; use rustc_session::Session; use rustc_span::source_map::Spanned; @@ -54,4 +54,28 @@ impl<'tcx> Visitor<'tcx> for MentionedItemsVisitor<'_, 'tcx> { _ => {} } } + + fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) { + self.super_rvalue(rvalue, location); + match *rvalue { + // We need to detect unsizing casts that required vtables. + mir::Rvalue::Cast( + mir::CastKind::PointerCoercion(PointerCoercion::Unsize), + ref operand, + target_ty, + ) + | mir::Rvalue::Cast(mir::CastKind::DynStar, ref operand, target_ty) => { + let span = self.body.source_info(location).span; + self.mentioned_items.push(Spanned { + node: MentionedItem::UnsizeCast { + source_ty: operand.ty(self.body, self.tcx), + target_ty, + }, + span, + }); + } + // Function pointer casts are already handled by `visit_constant` above. + _ => {} + } + } } diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 84353a7c054ef..005a223350719 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -736,7 +736,7 @@ impl<'a, 'tcx> MirUsedCollector<'a, 'tcx> { where T: TypeFoldable>, { - debug!("monomorphize: self.instance={:?}", self.instance); + trace!("monomorphize: self.instance={:?}", self.instance); self.instance.instantiate_mir_and_normalize_erasing_regions( self.tcx, ty::ParamEnv::reveal_all(), @@ -1342,35 +1342,37 @@ fn create_mono_items_for_vtable_methods<'tcx>( ) { assert!(!trait_ty.has_escaping_bound_vars() && !impl_ty.has_escaping_bound_vars()); - if let ty::Dynamic(trait_ty, ..) = trait_ty.kind() { - if let Some(principal) = trait_ty.principal() { - let poly_trait_ref = principal.with_self_ty(tcx, impl_ty); - assert!(!poly_trait_ref.has_escaping_bound_vars()); - - // Walk all methods of the trait, including those of its supertraits - let entries = tcx.vtable_entries(poly_trait_ref); - let methods = entries - .iter() - .filter_map(|entry| match entry { - VtblEntry::MetadataDropInPlace - | VtblEntry::MetadataSize - | VtblEntry::MetadataAlign - | VtblEntry::Vacant => None, - VtblEntry::TraitVPtr(_) => { - // all super trait items already covered, so skip them. - None - } - VtblEntry::Method(instance) => { - Some(*instance).filter(|instance| should_codegen_locally(tcx, instance)) - } - }) - .map(|item| create_fn_mono_item(tcx, item, source)); - output.extend(methods); - } - - // Also add the destructor. - visit_drop_use(tcx, impl_ty, false, source, output); + let ty::Dynamic(trait_ty, ..) = trait_ty.kind() else { + bug!("create_mono_items_for_vtable_methods: {trait_ty:?} not a trait type"); + }; + if let Some(principal) = trait_ty.principal() { + let poly_trait_ref = principal.with_self_ty(tcx, impl_ty); + assert!(!poly_trait_ref.has_escaping_bound_vars()); + + // Walk all methods of the trait, including those of its supertraits + let entries = tcx.vtable_entries(poly_trait_ref); + debug!(?entries); + let methods = entries + .iter() + .filter_map(|entry| match entry { + VtblEntry::MetadataDropInPlace + | VtblEntry::MetadataSize + | VtblEntry::MetadataAlign + | VtblEntry::Vacant => None, + VtblEntry::TraitVPtr(_) => { + // all super trait items already covered, so skip them. + None + } + VtblEntry::Method(instance) => { + Some(*instance).filter(|instance| should_codegen_locally(tcx, instance)) + } + }) + .map(|item| create_fn_mono_item(tcx, item, source)); + output.extend(methods); } + + // Also add the destructor. + visit_drop_use(tcx, impl_ty, false, source, output); } //=----------------------------------------------------------------------------- @@ -1693,7 +1695,8 @@ fn collect_items_of_instance<'tcx>( } } -/// `item` must be already monomorphized +/// `item` must be already monomorphized. +#[instrument(skip(tcx, span, output), level = "debug")] fn visit_mentioned_item<'tcx>( tcx: TyCtxt<'tcx>, item: &MentionedItem<'tcx>, @@ -1712,6 +1715,18 @@ fn visit_mentioned_item<'tcx>( MentionedItem::Drop(ty) => { visit_drop_use(tcx, ty, /*is_direct_call*/ true, span, output); } + MentionedItem::UnsizeCast { source_ty, target_ty } => { + let (source_ty, target_ty) = + find_vtable_types_for_unsizing(tcx.at(span), source_ty, target_ty); + // This could also be a different Unsize instruction, like + // from a fixed sized array to a slice. But we are only + // interested in things that produce a vtable. + if (target_ty.is_trait() && !source_ty.is_trait()) + || (target_ty.is_dyn_star() && !source_ty.is_dyn_star()) + { + create_mono_items_for_vtable_methods(tcx, target_ty, source_ty, span, output); + } + } } } diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr b/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr index 2a835a4237eb8..53d8e0cb996b8 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.noopt.stderr @@ -1,21 +1,21 @@ error[E0080]: evaluation of `Fail::::C` failed - --> $DIR/collect-in-dead-vtable.rs:11:19 + --> $DIR/collect-in-dead-vtable.rs:8:19 | LL | const C: () = panic!(); - | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:11:19 + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:8:19 | = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) note: erroneous constant encountered - --> $DIR/collect-in-dead-vtable.rs:25:21 + --> $DIR/collect-in-dead-vtable.rs:21:21 | LL | let _ = Fail::::C; | ^^^^^^^^^^^^ note: the above error was encountered while instantiating `fn as MyTrait>::not_called` - --> $DIR/collect-in-dead-vtable.rs:34:40 + --> $DIR/collect-in-dead-vtable.rs:30:40 | -LL | let gen_vtable: &dyn MyTrait = &v; // vtable "appears" here +LL | let gen_vtable: &dyn MyTrait = &v; // vtable is "mentioned" here | ^^ error: aborting due to 1 previous error diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.opt.stderr b/tests/ui/consts/required-consts/collect-in-dead-vtable.opt.stderr new file mode 100644 index 0000000000000..53d8e0cb996b8 --- /dev/null +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.opt.stderr @@ -0,0 +1,23 @@ +error[E0080]: evaluation of `Fail::::C` failed + --> $DIR/collect-in-dead-vtable.rs:8:19 + | +LL | const C: () = panic!(); + | ^^^^^^^^ the evaluated program panicked at 'explicit panic', $DIR/collect-in-dead-vtable.rs:8:19 + | + = note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: erroneous constant encountered + --> $DIR/collect-in-dead-vtable.rs:21:21 + | +LL | let _ = Fail::::C; + | ^^^^^^^^^^^^ + +note: the above error was encountered while instantiating `fn as MyTrait>::not_called` + --> $DIR/collect-in-dead-vtable.rs:30:40 + | +LL | let gen_vtable: &dyn MyTrait = &v; // vtable is "mentioned" here + | ^^ + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/consts/required-consts/collect-in-dead-vtable.rs b/tests/ui/consts/required-consts/collect-in-dead-vtable.rs index 3098661fbd91c..39d51801c04f7 100644 --- a/tests/ui/consts/required-consts/collect-in-dead-vtable.rs +++ b/tests/ui/consts/required-consts/collect-in-dead-vtable.rs @@ -1,14 +1,11 @@ //@revisions: noopt opt -//@[noopt] build-fail +//@ build-fail //@[opt] compile-flags: -O -//FIXME: `opt` revision currently does not stop with an error due to -//. -//@[opt] build-pass //! This fails without optimizations, so it should also fail with optimizations. struct Fail(T); impl Fail { - const C: () = panic!(); //[noopt]~ERROR evaluation of `Fail::::C` failed + const C: () = panic!(); //~ERROR evaluation of `Fail::::C` failed } trait MyTrait { @@ -17,8 +14,7 @@ trait MyTrait { // This function is not actually called, but it is mentioned in a vtable in a function that is // called. Make sure we still find this error. -// This relies on mono-item collection checking `required_consts` in functions that are referenced -// in vtables that syntactically appear in collected functions (even inside dead code). +// This ensures that we are properly considering vtables when gathering "mentioned" items. impl MyTrait for Vec { fn not_called(&self) { if false { @@ -31,7 +27,7 @@ impl MyTrait for Vec { fn called() { if false { let v: Vec = Vec::new(); - let gen_vtable: &dyn MyTrait = &v; // vtable "appears" here + let gen_vtable: &dyn MyTrait = &v; // vtable is "mentioned" here } }