From b92af94ced93299f3ae077dad8b4b09381421f46 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 12 Mar 2024 06:40:23 +0000 Subject: [PATCH 1/2] Stop walking the bodies of statics for reachability, and evaluate them instead --- compiler/rustc_passes/src/reachable.rs | 71 +++++++++++++++++++------- 1 file changed, 53 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_passes/src/reachable.rs b/compiler/rustc_passes/src/reachable.rs index f46f831ddd7c9..cd250ad901115 100644 --- a/compiler/rustc_passes/src/reachable.rs +++ b/compiler/rustc_passes/src/reachable.rs @@ -13,6 +13,7 @@ use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::Node; use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; use rustc_middle::middle::privacy::{self, Level}; +use rustc_middle::mir::interpret::{ConstAllocation, GlobalAlloc}; use rustc_middle::query::Providers; use rustc_middle::ty::{self, TyCtxt}; use rustc_session::config::CrateType; @@ -64,23 +65,8 @@ impl<'tcx> Visitor<'tcx> for ReachableContext<'tcx> { _ => None, }; - if let Some(res) = res - && let Some(def_id) = res.opt_def_id().and_then(|el| el.as_local()) - { - if self.def_id_represents_local_inlined_item(def_id.to_def_id()) { - self.worklist.push(def_id); - } else { - match res { - // Reachable constants and reachable statics can have their contents inlined - // into other crates. Mark them as reachable and recurse into their body. - Res::Def(DefKind::Const | DefKind::AssocConst | DefKind::Static(_), _) => { - self.worklist.push(def_id); - } - _ => { - self.reachable_symbols.insert(def_id); - } - } - } + if let Some(res) = res { + self.propagate_item(res); } intravisit::walk_expr(self, expr) @@ -197,9 +183,14 @@ impl<'tcx> ReachableContext<'tcx> { // Reachable constants will be inlined into other crates // unconditionally, so we need to make sure that their // contents are also reachable. - hir::ItemKind::Const(_, _, init) | hir::ItemKind::Static(_, _, init) => { + hir::ItemKind::Const(_, _, init) => { self.visit_nested_body(init); } + hir::ItemKind::Static(..) => { + if let Ok(alloc) = self.tcx.eval_static_initializer(item.owner_id.def_id) { + self.propagate_from_alloc(item.owner_id.def_id, alloc); + } + } // These are normal, nothing reachable about these // inherently and their children are already in the @@ -266,6 +257,50 @@ impl<'tcx> ReachableContext<'tcx> { } } } + + /// Finds things to add to `reachable_symbols` within allocations. + /// In contrast to visit_nested_body this ignores things that were only needed to evaluate + /// to the allocation. + fn propagate_from_alloc(&mut self, root: LocalDefId, alloc: ConstAllocation<'tcx>) { + if !self.any_library { + return; + } + for (_, prov) in alloc.0.provenance().ptrs().iter() { + match self.tcx.global_alloc(prov.alloc_id()) { + GlobalAlloc::Static(def_id) => { + self.propagate_item(Res::Def(self.tcx.def_kind(def_id), def_id)) + } + GlobalAlloc::Function(instance) => { + self.propagate_item(Res::Def( + self.tcx.def_kind(instance.def_id()), + instance.def_id(), + )) + // TODO: walk generic args + } + GlobalAlloc::VTable(ty, trait_ref) => todo!("{ty:?}, {trait_ref:?}"), + GlobalAlloc::Memory(alloc) => self.propagate_from_alloc(root, alloc), + } + } + } + + fn propagate_item(&mut self, res: Res) { + let Res::Def(kind, def_id) = res else { return }; + let Some(def_id) = def_id.as_local() else { return }; + if self.def_id_represents_local_inlined_item(def_id.to_def_id()) { + self.worklist.push(def_id); + } else { + match kind { + // Reachable constants and reachable statics can have their contents inlined + // into other crates. Mark them as reachable and recurse into their body. + DefKind::Const | DefKind::AssocConst | DefKind::Static(_) => { + self.worklist.push(def_id); + } + _ => { + self.reachable_symbols.insert(def_id); + } + } + } + } } fn check_item<'tcx>( From 7b7b6b53ee704996ee9038944991855353da73eb Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 12 Mar 2024 07:00:01 +0000 Subject: [PATCH 2/2] Test and implement reachability for trait objects and generic parameters of functions --- Cargo.lock | 1 + compiler/rustc_passes/Cargo.toml | 1 + compiler/rustc_passes/src/reachable.rs | 35 ++++++++++++++++--- compiler/rustc_privacy/src/lib.rs | 4 +-- .../cross-crate/auxiliary/static_init_aux.rs | 21 +++++++++++ tests/ui/cross-crate/static-init.rs | 4 +++ 6 files changed, 60 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 53bd088b55793..036c461e61c6c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4397,6 +4397,7 @@ dependencies = [ "rustc_lexer", "rustc_macros", "rustc_middle", + "rustc_privacy", "rustc_session", "rustc_span", "rustc_target", diff --git a/compiler/rustc_passes/Cargo.toml b/compiler/rustc_passes/Cargo.toml index 6abfa08c53080..b45a8dae277ed 100644 --- a/compiler/rustc_passes/Cargo.toml +++ b/compiler/rustc_passes/Cargo.toml @@ -18,6 +18,7 @@ rustc_index = { path = "../rustc_index" } rustc_lexer = { path = "../rustc_lexer" } rustc_macros = { path = "../rustc_macros" } rustc_middle = { path = "../rustc_middle" } +rustc_privacy = { path = "../rustc_privacy" } rustc_session = { path = "../rustc_session" } rustc_span = { path = "../rustc_span" } rustc_target = { path = "../rustc_target" } diff --git a/compiler/rustc_passes/src/reachable.rs b/compiler/rustc_passes/src/reachable.rs index cd250ad901115..21a5d6fec57e6 100644 --- a/compiler/rustc_passes/src/reachable.rs +++ b/compiler/rustc_passes/src/reachable.rs @@ -15,7 +15,8 @@ use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs} use rustc_middle::middle::privacy::{self, Level}; use rustc_middle::mir::interpret::{ConstAllocation, GlobalAlloc}; use rustc_middle::query::Providers; -use rustc_middle::ty::{self, TyCtxt}; +use rustc_middle::ty::{self, ExistentialTraitRef, TyCtxt}; +use rustc_privacy::DefIdVisitor; use rustc_session::config::CrateType; use rustc_target::spec::abi::Abi; @@ -271,13 +272,22 @@ impl<'tcx> ReachableContext<'tcx> { self.propagate_item(Res::Def(self.tcx.def_kind(def_id), def_id)) } GlobalAlloc::Function(instance) => { + // Manually visit to actually see the instance's `DefId`. Type visitors won't see it self.propagate_item(Res::Def( self.tcx.def_kind(instance.def_id()), instance.def_id(), - )) - // TODO: walk generic args + )); + self.visit(instance.args); + } + GlobalAlloc::VTable(ty, trait_ref) => { + self.visit(ty); + // Manually visit to actually see the trait's `DefId`. Type visitors won't see it + if let Some(trait_ref) = trait_ref { + let ExistentialTraitRef { def_id, args } = trait_ref.skip_binder(); + self.visit_def_id(def_id, "", &""); + self.visit(args); + } } - GlobalAlloc::VTable(ty, trait_ref) => todo!("{ty:?}, {trait_ref:?}"), GlobalAlloc::Memory(alloc) => self.propagate_from_alloc(root, alloc), } } @@ -303,6 +313,23 @@ impl<'tcx> ReachableContext<'tcx> { } } +impl<'tcx> DefIdVisitor<'tcx> for ReachableContext<'tcx> { + type Result = (); + + fn tcx(&self) -> TyCtxt<'tcx> { + self.tcx + } + + fn visit_def_id( + &mut self, + def_id: DefId, + _kind: &str, + _descr: &dyn std::fmt::Display, + ) -> Self::Result { + self.propagate_item(Res::Def(self.tcx.def_kind(def_id), def_id)) + } +} + fn check_item<'tcx>( tcx: TyCtxt<'tcx>, id: hir::ItemId, diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index fe3598616817b..6444e583e8b12 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -67,7 +67,7 @@ impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> { /// First, it doesn't have overridable `fn visit_trait_ref`, so we have to catch trait `DefId`s /// manually. Second, it doesn't visit some type components like signatures of fn types, or traits /// in `impl Trait`, see individual comments in `DefIdVisitorSkeleton::visit_ty`. -trait DefIdVisitor<'tcx> { +pub trait DefIdVisitor<'tcx> { type Result: VisitorResult = (); const SHALLOW: bool = false; const SKIP_ASSOC_TYS: bool = false; @@ -98,7 +98,7 @@ trait DefIdVisitor<'tcx> { } } -struct DefIdVisitorSkeleton<'v, 'tcx, V: ?Sized> { +pub struct DefIdVisitorSkeleton<'v, 'tcx, V: ?Sized> { def_id_visitor: &'v mut V, visited_opaque_tys: FxHashSet, dummy: PhantomData>, diff --git a/tests/ui/cross-crate/auxiliary/static_init_aux.rs b/tests/ui/cross-crate/auxiliary/static_init_aux.rs index 5e172ef3198ae..dca708733b923 100644 --- a/tests/ui/cross-crate/auxiliary/static_init_aux.rs +++ b/tests/ui/cross-crate/auxiliary/static_init_aux.rs @@ -1,6 +1,8 @@ pub static V: &u32 = &X; pub static F: fn() = f; pub static G: fn() = G0; +pub static H: &(dyn Fn() + Sync) = &h; +pub static I: fn() = Helper(j).mk(); static X: u32 = 42; static G0: fn() = g; @@ -12,3 +14,22 @@ pub fn v() -> *const u32 { fn f() {} fn g() {} + +fn h() {} + +#[derive(Copy, Clone)] +struct Helper(T); + +impl Helper { + const fn mk(self) -> fn() { + i:: + } +} + +fn i() { + assert_eq!(std::mem::size_of::(), 0); + // unsafe to work around the lack of a `Default` impl for function items + unsafe { (std::mem::transmute_copy::<(), T>(&()))() } +} + +fn j() {} diff --git a/tests/ui/cross-crate/static-init.rs b/tests/ui/cross-crate/static-init.rs index 090ad5f810e16..c4697a1d010c7 100644 --- a/tests/ui/cross-crate/static-init.rs +++ b/tests/ui/cross-crate/static-init.rs @@ -6,6 +6,8 @@ extern crate static_init_aux as aux; static V: &u32 = aux::V; static F: fn() = aux::F; static G: fn() = aux::G; +static H: &(dyn Fn() + Sync) = aux::H; +static I: fn() = aux::I; fn v() -> *const u32 { V @@ -15,4 +17,6 @@ fn main() { assert_eq!(aux::v(), crate::v()); F(); G(); + H(); + I(); }