Rollup merge of rust-lang#156370 - qaijuang:eii-dylib-default-override, r=bjorn3

Reject linked dylib EII default overrides

This PR rejects explicit EII implementations that would override a default implementation already selected through a linked dylib.

The check is intentionally split:
- rustc_passes keeps the early, format-independent checks for missing impls and duplicate explicit impls.
- rustc_codegen_ssa checks the default-vs-explicit conflict during linking, using the dependency formats selected for the final artifact.

Fixes rust-lang#156320.

Author: jhpratt

compiler/rustc_codegen_ssa/src/back/link.rs

@@ -30,6 +30,7 @@ use rustc_metadata::{
     walk_native_lib_search_dirs,
 };
 use rustc_middle::bug;
+use rustc_middle::error::DuplicateEiiImpls;
 use rustc_middle::lint::emit_lint_base;
 use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
 use rustc_middle::middle::dependency_format::Linkage;
@@ -76,6 +77,64 @@ pub fn ensure_removed(dcx: DiagCtxtHandle<'_>, path: &Path) {
     }
 }
 
+fn eii_impl_crate_name(crate_info: &CrateInfo, cnum: CrateNum) -> Symbol {
+    if cnum == LOCAL_CRATE { crate_info.local_crate_name } else { crate_info.crate_name[&cnum] }
+}
+
+fn check_externally_implementable_item_linkage(sess: &Session, crate_info: &CrateInfo) {
+    if crate_info.eii_linkage.is_empty() {
+        return;
+    }
+
+    // A crate can request multiple linked outputs with overlapping dependency
+    // formats, so report each underlying conflict once.
+    let mut emitted = FxHashSet::default();
+
+    // This needs the dependency formats selected for the final artifact. The
+    // earlier EII pass still handles missing impls and duplicate explicit impls.
+    for dependency_formats in crate_info.dependency_formats.values() {
+        for (eii_index, eii) in crate_info.eii_linkage.iter().enumerate() {
+            let Some(explicit_impl) = eii.impls.first() else {
+                continue;
+            };
+            // If the explicit impl is already coming from a dylib, that dylib
+            // has already resolved the default-vs-explicit choice.
+            if matches!(
+                dependency_formats.get(explicit_impl.impl_crate),
+                Some(Linkage::Dynamic | Linkage::IncludedFromDylib)
+            ) {
+                continue;
+            }
+
+            let Some(default_impl) = &eii.default_impl else {
+                continue;
+            };
+            if !matches!(
+                dependency_formats.get(default_impl.impl_crate),
+                Some(Linkage::Dynamic | Linkage::IncludedFromDylib)
+            ) {
+                continue;
+            }
+
+            if !emitted.insert(eii_index) {
+                continue;
+            }
+
+            sess.dcx().emit_err(DuplicateEiiImpls {
+                name: eii.name,
+                first_span: explicit_impl.span,
+                first_crate: eii_impl_crate_name(crate_info, explicit_impl.impl_crate),
+                second_span: default_impl.span,
+                second_crate: eii_impl_crate_name(crate_info, default_impl.impl_crate),
+                help: (),
+                additional_crates: None,
+                num_additional_crates: 0,
+                additional_crate_names: String::new(),
+            });
+        }
+    }
+}
+
 /// Performs the linkage portion of the compilation phase. This will generate all
 /// of the requested outputs for this compilation session.
 pub fn link_binary(
@@ -91,6 +150,14 @@ pub fn link_binary(
     let output_metadata = sess.opts.output_types.contains_key(&OutputType::Metadata);
     let mut tempfiles_for_stdout_output: Vec<PathBuf> = Vec::new();
     let mut rmeta_link_cache = RmetaLinkCache::default();
+
+    if outputs.outputs.should_link() {
+        sess.time("check_externally_implementable_item_linkage", || {
+            check_externally_implementable_item_linkage(sess, &crate_info);
+        });
+        sess.dcx().abort_if_errors();
+    }
+
     for &crate_type in &crate_info.crate_types {
         // Ignore executable crates if we have -Z no-codegen, as they will error.
         if (sess.opts.unstable_opts.no_codegen || !sess.opts.output_types.should_codegen())

compiler/rustc_codegen_ssa/src/base.rs

@@ -1,25 +1,25 @@
-use std::cmp;
 use std::collections::BTreeSet;
 use std::sync::Arc;
 use std::time::{Duration, Instant};
+use std::{cmp, iter};
 
 use itertools::Itertools;
 use rustc_abi::FIRST_VARIANT;
 use rustc_ast::expand::allocator::{
     ALLOC_ERROR_HANDLER, ALLOCATOR_METHODS, AllocatorKind, AllocatorMethod, AllocatorMethodInput,
     AllocatorTy,
 };
-use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
+use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet};
 use rustc_data_structures::profiling::{get_resident_set_size, print_time_passes_entry};
 use rustc_data_structures::sync::{IntoDynSyncSend, par_map};
 use rustc_data_structures::unord::UnordMap;
-use rustc_hir::attrs::{DebuggerVisualizerType, OptimizeAttr};
-use rustc_hir::def_id::{DefId, LOCAL_CRATE};
+use rustc_hir::attrs::{DebuggerVisualizerType, EiiDecl, EiiImpl, OptimizeAttr};
+use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
 use rustc_hir::lang_items::LangItem;
 use rustc_hir::{ItemId, Target, find_attr};
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
 use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
-use rustc_middle::middle::dependency_format::Dependencies;
+use rustc_middle::middle::dependency_format::{Dependencies, Linkage};
 use rustc_middle::middle::exported_symbols::{self, SymbolExportKind};
 use rustc_middle::middle::lang_items;
 use rustc_middle::mir::BinOp;
@@ -50,7 +50,8 @@ use crate::mir::operand::OperandValue;
 use crate::mir::place::PlaceRef;
 use crate::traits::*;
 use crate::{
-    CachedModuleCodegen, CodegenLintLevelSpecs, CrateInfo, ModuleCodegen, errors, meth, mir,
+    CachedModuleCodegen, CodegenLintLevelSpecs, CrateInfo, EiiLinkageImplInfo, EiiLinkageInfo,
+    ModuleCodegen, errors, meth, mir,
 };
 
 pub(crate) fn bin_op_to_icmp_predicate(op: BinOp, signed: bool) -> IntPredicate {
@@ -914,6 +915,71 @@ pub fn is_call_from_compiler_builtins_to_upstream_monomorphization<'tcx>(
         && !is_extern_call_to_local_crate(tcx, instance)
 }
 
+fn collect_eii_linkage(tcx: TyCtxt<'_>) -> Vec<EiiLinkageInfo> {
+    #[derive(Debug)]
+    struct FoundImpl {
+        imp: EiiImpl,
+        impl_crate: CrateNum,
+    }
+
+    #[derive(Debug)]
+    struct FoundEii {
+        decl: EiiDecl,
+        impls: FxIndexMap<DefId, FoundImpl>,
+    }
+
+    let mut eiis = FxIndexMap::<DefId, FoundEii>::default();
+
+    for &cnum in tcx.crates(()).iter().chain(iter::once(&LOCAL_CRATE)) {
+        for (&did, &(decl, ref impls)) in tcx.externally_implementable_items(cnum) {
+            eiis.entry(did)
+                .or_insert_with(|| FoundEii { decl, impls: Default::default() })
+                .impls
+                .extend(
+                    impls
+                        .into_iter()
+                        .map(|(&did, &imp)| (did, FoundImpl { imp, impl_crate: cnum })),
+                );
+        }
+    }
+
+    eiis.into_iter()
+        .filter_map(|(_, FoundEii { decl, impls })| {
+            let mut explicit_impls = Vec::new();
+            let mut default_impl = None;
+
+            for (impl_did, FoundImpl { imp, impl_crate }) in impls {
+                let impl_info = EiiLinkageImplInfo { span: tcx.def_span(impl_did), impl_crate };
+                if imp.is_default {
+                    default_impl = Some(impl_info);
+                } else {
+                    explicit_impls.push(impl_info);
+                }
+            }
+
+            // Link time check is only needed when there may be a default impl in a dylib.
+            // Other cases emit an error in `rustc_passes` already.
+            if let Some(default_impl) = default_impl {
+                Some(EiiLinkageInfo {
+                    name: decl.name.name,
+                    impls: explicit_impls,
+                    default_impl: Some(default_impl),
+                })
+            } else {
+                None
+            }
+        })
+        .collect()
+}
+
+fn eii_linkage_needed(dependency_formats: &Dependencies) -> bool {
+    dependency_formats.values().any(|formats| {
+        formats
+            .iter()
+            .any(|&linkage| matches!(linkage, Linkage::Dynamic | Linkage::IncludedFromDylib))
+    })
+}
+
 impl CrateInfo {
     pub fn new(tcx: TyCtxt<'_>, target_cpu: String) -> CrateInfo {
         let crate_types = tcx.crate_types().to_vec();
@@ -925,6 +991,12 @@ impl CrateInfo {
             crate_types.iter().map(|&c| (c, crate::back::linker::linked_symbols(tcx, c))).collect();
         let local_crate_name = tcx.crate_name(LOCAL_CRATE);
         let windows_subsystem = find_attr!(tcx, crate, WindowsSubsystem(kind) => *kind);
+        let dependency_formats = Arc::clone(tcx.dependency_formats(()));
+        let eii_linkage = if eii_linkage_needed(&dependency_formats) {
+            collect_eii_linkage(tcx)
+        } else {
+            Vec::new()
+        };
 
         // This list is used when generating the command line to pass through to
         // system linker. The linker expects undefined symbols on the left of the
@@ -969,7 +1041,8 @@ impl CrateInfo {
             crate_name: UnordMap::with_capacity(n_crates),
             used_crates,
             used_crate_source: UnordMap::with_capacity(n_crates),
-            dependency_formats: Arc::clone(tcx.dependency_formats(())),
+            dependency_formats,
+            eii_linkage,
             windows_subsystem,
             natvis_debugger_visualizers: Default::default(),
             lint_level_specs: CodegenLintLevelSpecs::from_tcx(tcx),

compiler/rustc_codegen_ssa/src/lib.rs

@@ -40,7 +40,7 @@ use rustc_session::Session;
 use rustc_session::config::{CrateType, OutputFilenames, OutputType};
 use rustc_session::cstore::{self, CrateSource};
 use rustc_session::lint::builtin::LINKER_MESSAGES;
-use rustc_span::Symbol;
+use rustc_span::{Span, Symbol};
 
 pub mod assert_module_sources;
 pub mod back;
@@ -256,6 +256,19 @@ impl SymbolExport {
     }
 }
 
+#[derive(Clone, Debug, Encodable, Decodable)]
+pub struct EiiLinkageImplInfo {
+    pub span: Span,
+    pub impl_crate: CrateNum,
+}
+
+#[derive(Clone, Debug, Encodable, Decodable)]
+pub struct EiiLinkageInfo {
+    pub name: Symbol,
+    pub impls: Vec<EiiLinkageImplInfo>,
+    pub default_impl: Option<EiiLinkageImplInfo>,
+}
+
 /// Misc info we load from metadata to persist beyond the tcx.
 ///
 /// Note: though `CrateNum` is only meaningful within the same tcx, information within `CrateInfo`
@@ -282,6 +295,9 @@ pub struct CrateInfo {
     pub used_crate_source: UnordMap<CrateNum, Arc<CrateSource>>,
     pub used_crates: Vec<CrateNum>,
     pub dependency_formats: Arc<Dependencies>,
+    /// EII implementations used by the link-time duplicate check, so `-Zno-link` can serialize the data needed by a
+    /// later `-Zlink-only` invocation.
+    pub eii_linkage: Vec<EiiLinkageInfo>,
     pub windows_subsystem: Option<WindowsSubsystemKind>,
     pub natvis_debugger_visualizers: BTreeSet<DebuggerVisualizerFile>,
     pub lint_level_specs: CodegenLintLevelSpecs,

compiler/rustc_middle/src/error.rs

@@ -169,3 +169,32 @@ pub(crate) struct IncrementCompilation {
     pub run_cmd: String,
     pub dep_node: String,
 }
+
+#[derive(Diagnostic)]
+#[diag("multiple implementations of `#[{$name}]`")]
+pub struct DuplicateEiiImpls {
+    pub name: Symbol,
+
+    #[primary_span]
+    #[label("first implemented here in crate `{$first_crate}`")]
+    pub first_span: Span,
+    pub first_crate: Symbol,
+
+    #[label("also implemented here in crate `{$second_crate}`")]
+    pub second_span: Span,
+    pub second_crate: Symbol,
+
+    #[note("in addition to these two, { $num_additional_crates ->
+        [one] another implementation was found in crate {$additional_crate_names}
+        *[other] more implementations were also found in the following crates: {$additional_crate_names}
+    }")]
+    pub additional_crates: Option<()>,
+
+    pub num_additional_crates: usize,
+    pub additional_crate_names: String,
+
+    #[help(
+        "an \"externally implementable item\" can only have a single implementation in the final artifact. When multiple implementations are found, also in different crates, they conflict"
+    )]
+    pub help: (),
+}

compiler/rustc_passes/src/diagnostics.rs

@@ -1104,35 +1104,6 @@ pub(crate) struct EiiWithoutImpl {
     pub help: (),
 }
 
-#[derive(Diagnostic)]
-#[diag("multiple implementations of `#[{$name}]`")]
-pub(crate) struct DuplicateEiiImpls {
-    pub name: Symbol,
-
-    #[primary_span]
-    #[label("first implemented here in crate `{$first_crate}`")]
-    pub first_span: Span,
-    pub first_crate: Symbol,
-
-    #[label("also implemented here in crate `{$second_crate}`")]
-    pub second_span: Span,
-    pub second_crate: Symbol,
-
-    #[note("in addition to these two, { $num_additional_crates ->
-        [one] another implementation was found in crate {$additional_crate_names}
-        *[other] more implementations were also found in the following crates: {$additional_crate_names}
-    }")]
-    pub additional_crates: Option<()>,
-
-    pub num_additional_crates: usize,
-    pub additional_crate_names: String,
-
-    #[help(
-        "an \"externally implementable item\" can only have a single implementation in the final artifact. When multiple implementations are found, also in different crates, they conflict"
-    )]
-    pub help: (),
-}
-
 #[derive(Diagnostic)]
 #[diag("function doesn't have a default implementation")]
 pub(crate) struct FunctionNotHaveDefaultImplementation {

compiler/rustc_passes/src/eii.rs

@@ -6,10 +6,11 @@ use std::iter;
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_hir::attrs::{EiiDecl, EiiImpl};
 use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
+use rustc_middle::error::DuplicateEiiImpls;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::config::CrateType;
 
-use crate::diagnostics::{DuplicateEiiImpls, EiiWithoutImpl};
+use crate::diagnostics::EiiWithoutImpl;
 
 #[derive(Clone, Copy, Debug)]
 enum CheckingMode {

tests/ui/eii/default/auxiliary/decl_with_default.rs

@@ -1,3 +1,4 @@
+//@ no-prefer-dynamic
 #![crate_type = "rlib"]
 #![feature(extern_item_impls)]
 

tests/ui/eii/default/auxiliary/impl1.rs

@@ -1,3 +1,4 @@
+//@ no-prefer-dynamic
 //@ aux-build: decl_with_default.rs
 #![crate_type = "rlib"]
 #![feature(extern_item_impls)]

tests/ui/eii/duplicate/auxiliary/dylib_default.rs

@@ -0,0 +1,5 @@
+#![crate_type = "dylib"]
+#![feature(extern_item_impls)]
+
+#[eii(eii1)]
+fn decl1(x: u64) {}

tests/ui/eii/duplicate/dylib_default_duplicate.rs

@@ -0,0 +1,20 @@
+//@ aux-build: dylib_default.rs
+//@ needs-crate-type: dylib
+//@ compile-flags: --emit link
+//@ ignore-backends: gcc
+// FIXME: linking on windows (specifically mingw) not yet supported, see tracking issue #125418
+//@ ignore-windows
+// Regression test for https://github.com/rust-lang/rust/issues/156320.
+// A default implementation from an upstream dylib has already been selected and
+// must not be overridden by a downstream explicit implementation.
+#![feature(extern_item_impls)]
+
+extern crate dylib_default;
+
+#[unsafe(dylib_default::eii1)]
+fn other(x: u64) {
+    //~^ ERROR multiple implementations of `#[eii1]`
+    println!("1{x}");
+}
+
+fn main() {}