Unrolled build for #159740

Rollup merge of #159740 - RalfJung:miri-dedup-exported-symbols, r=bjorn3

reuse regular exported_non_generic_symbols logic in Miri

This is some gnarly code we have duplicated in Miri, let's try to reuse the version from rustc.

r? @bjorn3

Author: rust-timer

compiler/rustc_codegen_ssa/src/back/mod.rs

@@ -15,6 +15,8 @@ mod symbol_edit;
 pub mod symbol_export;
 pub mod write;
 
+pub use symbol_export::{exported_non_generic_symbols_helper, reachable_non_generics_helper};
+
 /// The target triple depends on the deployment target, and is required to
 /// enable features such as cross-language LTO, and for picking the right
 /// Mach-O commands.

compiler/rustc_codegen_ssa/src/back/symbol_export.rs

@@ -54,6 +54,11 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S
         return Default::default();
     }
 
+    reachable_non_generics_helper(tcx)
+}
+
+/// Exposed separately *without* the "should codegen" check so Miri can access it.
+pub fn reachable_non_generics_helper(tcx: TyCtxt<'_>) -> DefIdMap<SymbolExportInfo> {
     let is_compiler_builtins = tcx.is_compiler_builtins(LOCAL_CRATE);
 
     let mut reachable_non_generics: DefIdMap<_> = tcx
@@ -176,6 +181,13 @@ fn exported_non_generic_symbols_provider_local<'tcx>(
         return &[];
     }
 
+    exported_non_generic_symbols_helper(tcx)
+}
+
+/// Exposed separately *without* the "should codegen" check so Miri can access it.
+pub fn exported_non_generic_symbols_helper<'tcx>(
+    tcx: TyCtxt<'tcx>,
+) -> &'tcx [(ExportedSymbol<'tcx>, SymbolExportInfo)] {
     // FIXME: Sorting this is unnecessary since we are sorting later anyway.
     //        Can we skip the later sorting?
     let sorted = tcx.with_stable_hashing_context(|mut hcx| {

compiler/rustc_passes/src/reachable.rs

@@ -444,6 +444,8 @@ fn has_custom_linkage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
         // FIXME(nbdd0121): `#[used]` are marked as reachable here so it's picked up by
         // `linked_symbols` in cg_ssa. They won't be exported in binary or cdylib due to their
         // `SymbolExportLevel::Rust` export level but may end up being exported in dylibs.
+        // Also note that Miri is relying on this to be able to find private `link_section` statics
+        // across all crates.
         || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER)
         || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)
         // Right now, the only way to get "foreign item symbol aliases" is by being an EII-implementation.

src/bootstrap/src/core/build_steps/test.rs

@@ -839,6 +839,10 @@ impl Step for CargoMiri {
             SourceType::Submodule,
             &[],
         );
+        // Run subcrate tests as well.
+        cargo.arg("--workspace");
+        // Some tests need isolation disabled.
+        cargo.env("MIRIFLAGS", "-Zmiri-disable-isolation");
 
         // If we are testing stage 2+ cargo miri, make sure that it works with the in-tree cargo.
         // We want to do this *somewhere* to ensure that Miri + nightly cargo actually works.

src/tools/miri/src/bin/miri.rs

@@ -11,7 +11,6 @@
 extern crate rustc_codegen_ssa;
 extern crate rustc_data_structures;
 extern crate rustc_driver;
-extern crate rustc_hir;
 extern crate rustc_interface;
 extern crate rustc_log;
 extern crate rustc_middle;
@@ -48,14 +47,9 @@ use miri::{
 use rustc_codegen_ssa::traits::CodegenBackend;
 use rustc_data_structures::sync::{self, DynSync};
 use rustc_driver::Compilation;
-use rustc_hir::{self as hir, Node};
 use rustc_interface::interface::Config;
 use rustc_interface::util::DummyCodegenBackend;
 use rustc_log::tracing::debug;
-use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
-use rustc_middle::middle::exported_symbols::{
-    ExportedSymbol, SymbolExportInfo, SymbolExportKind, SymbolExportLevel,
-};
 use rustc_middle::query::LocalCrate;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::config::{CrateType, ErrorOutputType, OptLevel};
@@ -258,58 +252,14 @@ impl rustc_driver::Callbacks for MiriDepCompilerCalls {
         // Queries overridden here affect the data stored in `rmeta` files of dependencies,
         // which will be used later in non-`MIRI_BE_RUSTC` mode.
         config.override_queries = Some(|_, local_providers| {
-            // We need to add #[used] symbols to exported_symbols for `lookup_link_section`.
-            // FIXME handle this somehow in rustc itself to avoid this hack.
-            local_providers.queries.exported_non_generic_symbols = |tcx, LocalCrate| {
-                let reachable_set = tcx.with_stable_hashing_context(|mut hcx| {
-                    tcx.reachable_set(()).to_sorted(&mut hcx, true)
-                });
-                tcx.arena.alloc_from_iter(
-                    // This is based on:
-                    // https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L62-L63
-                    // https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L174
-                    reachable_set.into_iter().filter_map(|&local_def_id| {
-                        // Do the same filtering that rustc does:
-                        // https://github.com/rust-lang/rust/blob/2962e7c0089d5c136f4e9600b7abccfbbde4973d/compiler/rustc_codegen_ssa/src/back/symbol_export.rs#L84-L102
-                        // Otherwise it may cause unexpected behaviours and ICEs
-                        // (https://github.com/rust-lang/rust/issues/86261).
-                        let is_reachable_non_generic = matches!(
-                            tcx.hir_node_by_def_id(local_def_id),
-                            Node::Item(&hir::Item {
-                                kind: hir::ItemKind::Static(..) | hir::ItemKind::Fn{ .. },
-                                ..
-                            }) | Node::ImplItem(&hir::ImplItem {
-                                kind: hir::ImplItemKind::Fn(..),
-                                ..
-                            })
-                            if !tcx.generics_of(local_def_id).requires_monomorphization(tcx)
-                        );
-                        if !is_reachable_non_generic {
-                            return None;
-                        }
-                        let codegen_fn_attrs = tcx.codegen_fn_attrs(local_def_id);
-                        if codegen_fn_attrs.contains_extern_indicator()
-                            || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER)
-                            || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)
-                        {
-                            Some((
-                                ExportedSymbol::NonGeneric(local_def_id.to_def_id()),
-                                // Some dummy `SymbolExportInfo` here. We only use
-                                // `exported_symbols` in shims/foreign_items.rs and the export info
-                                // is ignored.
-                                SymbolExportInfo {
-                                    level: SymbolExportLevel::C,
-                                    kind: SymbolExportKind::Text,
-                                    used: false,
-                                    rustc_std_internal_symbol: false,
-                                },
-                            ))
-                        } else {
-                            None
-                        }
-                    }),
-                )
-            }
+            // `exported_non_generic_symbols` is usually empty because we don't codegen anything.
+            // However, we need it for `lookup_link_section`.
+            // So overwrite the query with a version that dooes something even without codegen.
+            local_providers.queries.exported_non_generic_symbols =
+                |tcx, LocalCrate| rustc_codegen_ssa::back::exported_non_generic_symbols_helper(tcx);
+            // `exported_non_generic_symbols_helper` calls `reachable_non_generics`.
+            local_providers.queries.reachable_non_generics =
+                |tcx, LocalCrate| rustc_codegen_ssa::back::reachable_non_generics_helper(tcx);
         });
 
         // Register our custom extra symbols.

src/tools/miri/src/helpers.rs

@@ -116,34 +116,36 @@ pub fn path_ty_layout<'tcx>(cx: &impl LayoutOf<'tcx>, path: &[&str]) -> TyAndLay
 /// Call `f` for each exported symbol.
 pub fn iter_exported_symbols<'tcx>(
     tcx: TyCtxt<'tcx>,
-    mut f: impl FnMut(CrateNum, DefId) -> InterpResult<'tcx>,
+    mut f: impl FnMut(CrateNum, DefId, /* used */ bool) -> InterpResult<'tcx>,
 ) -> InterpResult<'tcx> {
-    // First, the symbols in the local crate. We can't use `exported_symbols` here as that
-    // skips `#[used]` statics (since `reachable_set` skips them in binary crates).
-    // So we walk all HIR items ourselves instead.
+    // First, the symbols in the local crate. We can't use `exported_symbols` here as that skips
+    // `#[used]` statics (since `reachable_set` does not specifically include them in binary crates,
+    // only in library crates). So we walk all HIR items ourselves instead.
     let crate_items = tcx.hir_crate_items(());
     for def_id in crate_items.definitions() {
-        let exported = tcx.def_kind(def_id).has_codegen_attrs() && {
-            let codegen_attrs = tcx.codegen_fn_attrs(def_id);
-            codegen_attrs.contains_extern_indicator()
-                || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER)
-                || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)
-        };
+        if !tcx.def_kind(def_id).has_codegen_attrs() || tcx.is_foreign_item(def_id) {
+            continue;
+        }
+        let codegen_attrs = tcx.codegen_fn_attrs(def_id);
+        let used = codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER)
+            || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER);
+        if !(used || codegen_attrs.contains_extern_indicator()) {
+            continue;
+        }
         // FIXME: `#[no_mangle]` makes no sense on a generic item, but still causes it to be
         // considered "extern". Remove this once `no_mangle_generic_items` is a hard error.
-        let exported_mono = exported && {
+        let mono = {
             let generics = tcx.generics_of(def_id);
             !generics.requires_monomorphization(tcx)
         };
-        if exported_mono {
-            f(LOCAL_CRATE, def_id.into())?;
+        if mono {
+            f(LOCAL_CRATE, def_id.into(), used)?;
         }
     }
 
     // Next, all our dependencies.
-    // `dependency_formats` includes all the transitive information needed to link a crate,
-    // which is what we need here since we need to dig out `exported_symbols` from all transitive
-    // dependencies.
+    // `dependency_formats` includes all the transitive information needed to link a crate, which is
+    // what we need to dig out `exported_symbols` from all transitive dependencies.
     let dependency_formats = tcx.dependency_formats(());
     // Find the dependencies of the executable we are running.
     let dependency_format = dependency_formats
@@ -157,11 +159,12 @@ pub fn iter_exported_symbols<'tcx>(
             continue; // Already handled above
         }
 
-        // We can ignore `_export_info` here: we are a Rust crate, and everything is exported
-        // from a Rust crate.
-        for &(symbol, _export_info) in tcx.exported_non_generic_symbols(cnum) {
-            if let ExportedSymbol::NonGeneric(def_id) = symbol {
-                f(cnum, def_id)?;
+        for &(symbol, export_info) in tcx.exported_non_generic_symbols(cnum) {
+            if let ExportedSymbol::NonGeneric(def_id) = symbol
+                // Sometimes Rust has to re-export FFI imports; skip those.
+                && !tcx.is_foreign_item(def_id)
+            {
+                f(cnum, def_id, export_info.used)?;
             }
         }
     }
@@ -961,8 +964,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         let mut array = vec![];
 
-        iter_exported_symbols(tcx, |_cnum, def_id| {
+        iter_exported_symbols(tcx, |_cnum, def_id, used| {
             let attrs = tcx.codegen_fn_attrs(def_id);
+            if !used {
+                // We don't know if the symbol is actually going to be in the final binary,
+                // so we conservatively skip it.
+                return interp_ok(());
+            }
             let Some(link_section) = attrs.link_section else {
                 return interp_ok(());
             };

src/tools/miri/src/shims/foreign_items.rs

@@ -132,12 +132,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
                     is_weak: bool,
                 }
                 let mut symbol_target: Option<SymbolTarget<'tcx>> = None;
-                helpers::iter_exported_symbols(tcx, |cnum, def_id| {
+                helpers::iter_exported_symbols(tcx, |cnum, def_id, _used| {
                     let attrs = tcx.codegen_fn_attrs(def_id);
-                    // Skip over imports of items.
-                    if tcx.is_foreign_item(def_id) {
-                        return interp_ok(());
-                    }
                     // Skip over items without an explicitly defined symbol name.
                     if !(attrs.symbol_name.is_some()
                         || attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE)

src/tools/miri/test-cargo-miri/exported-symbol-dep/src/lib.rs

@@ -11,3 +11,49 @@ impl AssocFn {
         -123456
     }
 }
+
+// Also check static constructors in dependencies are run.
+
+#[rustfmt::skip]
+#[macro_export]
+macro_rules! ctor {
+    ($ident:ident = $ctor:ident) => {
+        #[cfg_attr(
+            all(any(
+                target_os = "linux",
+                target_os = "android",
+                target_os = "dragonfly",
+                target_os = "freebsd",
+                target_os = "haiku",
+                target_os = "illumos",
+                target_os = "netbsd",
+                target_os = "openbsd",
+                target_os = "solaris",
+                target_os = "none",
+                target_family = "wasm",
+            )),
+            link_section = ".init_array"
+        )]
+        #[cfg_attr(windows, link_section = ".CRT$XCU")]
+        #[cfg_attr(
+            any(target_os = "macos", target_os = "ios"),
+            // We do not set the `mod_init_funcs` flag here since ctor/inventory also do not do
+            // that. See <https://github.com/rust-lang/miri/pull/4459#discussion_r2200115629>.
+            link_section = "__DATA,__mod_init_func"
+        )]
+        #[used]
+        static $ident: unsafe extern "C" fn() = $ctor;
+    };
+}
+
+static mut INITIALIZED: bool = false;
+
+unsafe extern "C" fn ctor() {
+    unsafe { INITIALIZED = true };
+}
+
+pub fn check_initialized() {
+    assert!(unsafe { INITIALIZED });
+}
+
+ctor! { CTOR = ctor }

src/tools/miri/test-cargo-miri/exported-symbol/src/lib.rs

@@ -1 +1,2 @@
 extern crate exported_symbol_dep;
+pub use exported_symbol_dep::check_initialized;

src/tools/miri/test-cargo-miri/src/main.rs

@@ -67,6 +67,10 @@ fn main() {
 mod test {
     use byteorder_2::{BigEndian, ByteOrder};
 
+    extern crate cargo_miri_test;
+    extern crate exported_symbol;
+    extern crate issue_rust_86261;
+
     // Make sure in-crate tests with dev-dependencies work
     #[test]
     fn dev_dependency() {
@@ -75,9 +79,6 @@ mod test {
 
     #[test]
     fn exported_symbol() {
-        extern crate cargo_miri_test;
-        extern crate exported_symbol;
-        extern crate issue_rust_86261;
         // Test calling exported symbols in (transitive) dependencies.
         // Repeat calls to make sure the `Instance` cache is not broken.
         for _ in 0..3 {
@@ -95,4 +96,9 @@ mod test {
             unsafe { no_mangle_generic() }
         }
     }
+
+    #[test]
+    fn static_initializer_in_dep() {
+        exported_symbol::check_initialized();
+    }
 }