From 058bec86f17fe35d614a7f5c6362d46897920d4c Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 26 Jun 2025 17:03:09 +0000 Subject: [PATCH] Support weak definitions When a symbol only has a weak definition, this definition will be picked. When a symbol has both a weak and a regular definition, the regular definition will be picked instead. --- src/shims/foreign_items.rs | 103 +++++++++++++----- .../function_calls/exported_symbol_weak.rs | 39 +++++++ 2 files changed, 114 insertions(+), 28 deletions(-) create mode 100644 tests/pass/function_calls/exported_symbol_weak.rs diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 21545b6802..1da97cae98 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -5,6 +5,7 @@ use std::path::Path; use rustc_abi::{Align, AlignFromBytesError, CanonAbi, Size}; use rustc_apfloat::Float; use rustc_ast::expand::allocator::alloc_error_handler_name; +use rustc_hir::attrs::Linkage; use rustc_hir::def::DefKind; use rustc_hir::def_id::CrateNum; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; @@ -138,7 +139,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => { // Find it if it was not cached. - let mut instance_and_crate: Option<(ty::Instance<'_>, CrateNum)> = None; + + struct SymbolTarget<'tcx> { + instance: ty::Instance<'tcx>, + cnum: CrateNum, + is_weak: bool, + } + let mut symbol_target: Option> = None; helpers::iter_exported_symbols(tcx, |cnum, def_id| { let attrs = tcx.codegen_fn_attrs(def_id); // Skip over imports of items. @@ -155,40 +162,80 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let instance = Instance::mono(tcx, def_id); let symbol_name = tcx.symbol_name(instance).name; + let is_weak = attrs.linkage == Some(Linkage::WeakAny); if symbol_name == link_name.as_str() { - if let Some((original_instance, original_cnum)) = instance_and_crate { - // Make sure we are consistent wrt what is 'first' and 'second'. - let original_span = tcx.def_span(original_instance.def_id()).data(); - let span = tcx.def_span(def_id).data(); - if original_span < span { - throw_machine_stop!(TerminationInfo::MultipleSymbolDefinitions { - link_name, - first: original_span, - first_crate: tcx.crate_name(original_cnum), - second: span, - second_crate: tcx.crate_name(cnum), - }); - } else { - throw_machine_stop!(TerminationInfo::MultipleSymbolDefinitions { - link_name, - first: span, - first_crate: tcx.crate_name(cnum), - second: original_span, - second_crate: tcx.crate_name(original_cnum), - }); + if let Some(original) = &symbol_target { + // There is more than one definition with this name. What we do now + // depends on whether one or both definitions are weak. + match (is_weak, original.is_weak) { + (false, true) => { + // Original definition is a weak definition. Override it. + + symbol_target = Some(SymbolTarget { + instance: ty::Instance::mono(tcx, def_id), + cnum, + is_weak, + }); + } + (true, false) => { + // Current definition is a weak definition. Keep the original one. + } + (true, true) | (false, false) => { + // Either both definitions are non-weak or both are weak. In + // either case return an error. For weak definitions we error + // because it is unspecified which definition would have been + // picked by the linker. + + // Make sure we are consistent wrt what is 'first' and 'second'. + let original_span = + tcx.def_span(original.instance.def_id()).data(); + let span = tcx.def_span(def_id).data(); + if original_span < span { + throw_machine_stop!( + TerminationInfo::MultipleSymbolDefinitions { + link_name, + first: original_span, + first_crate: tcx.crate_name(original.cnum), + second: span, + second_crate: tcx.crate_name(cnum), + } + ); + } else { + throw_machine_stop!( + TerminationInfo::MultipleSymbolDefinitions { + link_name, + first: span, + first_crate: tcx.crate_name(cnum), + second: original_span, + second_crate: tcx.crate_name(original.cnum), + } + ); + } + } } + } else { + symbol_target = Some(SymbolTarget { + instance: ty::Instance::mono(tcx, def_id), + cnum, + is_weak, + }); } - if !matches!(tcx.def_kind(def_id), DefKind::Fn | DefKind::AssocFn) { - throw_ub_format!( - "attempt to call an exported symbol that is not defined as a function" - ); - } - instance_and_crate = Some((ty::Instance::mono(tcx, def_id), cnum)); } interp_ok(()) })?; - e.insert(instance_and_crate.map(|ic| ic.0)) + // Once we identified the instance corresponding to the symbol, ensure + // it is a function. It is okay to encounter non-functions in the search above + // as long as the final instance we arrive at is a function. + if let Some(SymbolTarget { instance, .. }) = symbol_target { + if !matches!(tcx.def_kind(instance.def_id()), DefKind::Fn | DefKind::AssocFn) { + throw_ub_format!( + "attempt to call an exported symbol that is not defined as a function" + ); + } + } + + e.insert(symbol_target.map(|SymbolTarget { instance, .. }| instance)) } }; match instance { diff --git a/tests/pass/function_calls/exported_symbol_weak.rs b/tests/pass/function_calls/exported_symbol_weak.rs new file mode 100644 index 0000000000..abf4b6718a --- /dev/null +++ b/tests/pass/function_calls/exported_symbol_weak.rs @@ -0,0 +1,39 @@ +#![feature(linkage)] + +// FIXME move this module to a separate crate once aux-build is allowed +// This currently depends on the fact that miri skips the codegen check +// that denies multiple symbols with the same name. +mod first { + #[no_mangle] + #[linkage = "weak"] + extern "C" fn foo() -> i32 { + 1 + } + + #[no_mangle] + #[linkage = "weak"] + extern "C" fn bar() -> i32 { + 2 + } +} + +mod second { + #[no_mangle] + extern "C" fn bar() -> i32 { + 3 + } +} + +extern "C" { + fn foo() -> i32; + fn bar() -> i32; +} + +fn main() { + unsafe { + // If there is no non-weak definition, the weak definition will be used. + assert_eq!(foo(), 1); + // Non-weak definition takes presedence. + assert_eq!(bar(), 3); + } +}