Auto merge of #141061 - dpaoliello:shimasfn, r=bjorn3

Change __rust_no_alloc_shim_is_unstable to be a function

This fixes a long sequence of issues:

1. A customer reported that building for Arm64EC was broken: #138541
2. This was caused by a bug in my original implementation of Arm64EC support, namely that only functions on Arm64EC need to be decorated with `#` but Rust was decorating statics as well.
3. Once I corrected Rust to only decorate functions, I started linking failures where the linker couldn't find statics exported by dylib dependencies. This was caused by the compiler not marking exported statics in the generated DEF file with `DATA`, thus they were being exported as functions not data.
4. Once I corrected the way that the DEF files were being emitted, the linker started failing saying that it couldn't find `__rust_no_alloc_shim_is_unstable`. This is because the MSVC linker requires the declarations of statics imported from other dylibs to be marked with `dllimport` (whereas it will happily link to functions imported from other dylibs whether they are marked `dllimport` or not).
5. I then made a change to ensure that `__rust_no_alloc_shim_is_unstable` was marked as `dllimport`, but the MSVC linker started emitting warnings that `__rust_no_alloc_shim_is_unstable` was marked as `dllimport` but was declared in an obj file. This is a harmless warning which is a performance hint: anything that's marked `dllimport` must be indirected via an `__imp` symbol so I added a linker arg in the target to suppress the warning.
6. A customer then reported a similar warning when using `lld-link` (<https://github.com/rust-lang/rust/pull/140176#issuecomment-2872448443>). I don't think it was an implementation difference between the two linkers but rather that, depending on the obj that the declaration versus uses of `__rust_no_alloc_shim_is_unstable` landed in we would get different warnings, so I suppressed that warning as well: #140954.
7. Another customer reported that they weren't using the Rust compiler to invoke the linker, thus these warnings were breaking their build: <https://github.com/rust-lang/rust/pull/140176#issuecomment-2881867433>. At that point, my original change was reverted (#141024) leaving Arm64EC broken yet again.

Taking a step back, a lot of these linker issues arise from the fact that `__rust_no_alloc_shim_is_unstable` is marked as `extern "Rust"` in the standard library and, therefore, assumed to be a foreign item from a different crate BUT the Rust compiler may choose to generate it either in the current crate, some other crate that will be statically linked in OR some other crate that will by dynamically imported.

Worse yet, it is impossible while building a given crate to know if `__rust_no_alloc_shim_is_unstable` will statically linked or dynamically imported: it might be that one of its dependent crates is the one with an allocator kind set and thus that crate (which is compiled later) will decide depending if it has any dylib dependencies or not to import `__rust_no_alloc_shim_is_unstable` or generate it. Thus, there is no way to know if the declaration of `__rust_no_alloc_shim_is_unstable` should be marked with `dllimport` or not.

There is a simple fix for all this: there is no reason `__rust_no_alloc_shim_is_unstable` must be a static. It needs to be some symbol that must be linked in; thus, it could easily be a function instead. As a function, there is no need to mark it as `dllimport` when dynamically imported which avoids the entire mess above.

There may be a perf hit for changing the `volatile load` to be a `tail call`, so I'm happy to change that part back (although I question what the codegen of a `volatile load` would look like, and if the backend is going to try to use load-acquire semantics).

Build with this change applied BEFORE #140176 was reverted to demonstrate that there are no linking issues with either MSVC or MinGW: <https://github.com/rust-lang/rust/actions/runs/15078657205>

Incidentally, I fixed `tests/run-make/no-alloc-shim` to work with MSVC as I needed it to be able to test locally (FYI for #128602)

r? `@bjorn3`
cc `@jieyouxu`
This commit is contained in:
bors
2025-06-18 09:24:40 +00:00
18 changed files with 155 additions and 131 deletions
+1 -1
View File
@@ -22,7 +22,7 @@ pub fn alloc_error_handler_name(alloc_error_handler_kind: AllocatorKind) -> &'st
}
}
pub const NO_ALLOC_SHIM_IS_UNSTABLE: &str = "__rust_no_alloc_shim_is_unstable";
pub const NO_ALLOC_SHIM_IS_UNSTABLE: &str = "__rust_no_alloc_shim_is_unstable_v2";
pub enum AllocatorTy {
Layout,
@@ -1,6 +1,7 @@
//! Allocator shim
// Adapted from rustc
use cranelift_frontend::{FunctionBuilder, FunctionBuilderContext};
use rustc_ast::expand::allocator::{
ALLOCATOR_METHODS, AllocatorKind, AllocatorTy, NO_ALLOC_SHIM_IS_UNSTABLE,
alloc_error_handler_name, default_fn_name, global_fn_name,
@@ -97,16 +98,31 @@ fn codegen_inner(
data.define(Box::new([val]));
module.define_data(data_id, &data).unwrap();
let data_id = module
.declare_data(
&mangle_internal_symbol(tcx, NO_ALLOC_SHIM_IS_UNSTABLE),
Linkage::Export,
false,
false,
)
.unwrap();
let mut data = DataDescription::new();
data.set_align(1);
data.define(Box::new([0]));
module.define_data(data_id, &data).unwrap();
{
let sig = Signature {
call_conv: module.target_config().default_call_conv,
params: vec![],
returns: vec![],
};
let func_id = module
.declare_function(
&mangle_internal_symbol(tcx, NO_ALLOC_SHIM_IS_UNSTABLE),
Linkage::Export,
&sig,
)
.unwrap();
let mut ctx = Context::new();
ctx.func.signature = sig;
let mut func_ctx = FunctionBuilderContext::new();
let mut bcx = FunctionBuilder::new(&mut ctx.func, &mut func_ctx);
let block = bcx.create_block();
bcx.switch_to_block(block);
bcx.ins().return_(&[]);
bcx.seal_all_blocks();
bcx.finalize();
module.define_function(func_id, &mut ctx).unwrap();
}
}
+42 -36
View File
@@ -57,7 +57,7 @@ pub(crate) unsafe fn codegen(
let from_name = mangle_internal_symbol(tcx, &global_fn_name(method.name));
let to_name = mangle_internal_symbol(tcx, &default_fn_name(method.name));
create_wrapper_function(tcx, context, &from_name, &to_name, &types, output);
create_wrapper_function(tcx, context, &from_name, Some(&to_name), &types, output);
}
}
@@ -66,7 +66,7 @@ pub(crate) unsafe fn codegen(
tcx,
context,
&mangle_internal_symbol(tcx, "__rust_alloc_error_handler"),
&mangle_internal_symbol(tcx, alloc_error_handler_name(alloc_error_handler_kind)),
Some(&mangle_internal_symbol(tcx, alloc_error_handler_name(alloc_error_handler_kind))),
&[usize, usize],
None,
);
@@ -81,21 +81,21 @@ pub(crate) unsafe fn codegen(
let value = context.new_rvalue_from_int(i8, value as i32);
global.global_set_initializer_rvalue(value);
let name = mangle_internal_symbol(tcx, NO_ALLOC_SHIM_IS_UNSTABLE);
let global = context.new_global(None, GlobalKind::Exported, i8, name);
#[cfg(feature = "master")]
global.add_attribute(VarAttribute::Visibility(symbol_visibility_to_gcc(
tcx.sess.default_visibility(),
)));
let value = context.new_rvalue_from_int(i8, 0);
global.global_set_initializer_rvalue(value);
create_wrapper_function(
tcx,
context,
&mangle_internal_symbol(tcx, NO_ALLOC_SHIM_IS_UNSTABLE),
None,
&[],
None,
);
}
fn create_wrapper_function(
tcx: TyCtxt<'_>,
context: &Context<'_>,
from_name: &str,
to_name: &str,
to_name: Option<&str>,
types: &[Type<'_>],
output: Option<Type<'_>>,
) {
@@ -124,34 +124,40 @@ fn create_wrapper_function(
// TODO(antoyo): emit unwind tables.
}
let args: Vec<_> = types
.iter()
.enumerate()
.map(|(index, typ)| context.new_parameter(None, *typ, format!("param{}", index)))
.collect();
let callee = context.new_function(
None,
FunctionType::Extern,
output.unwrap_or(void),
&args,
to_name,
false,
);
#[cfg(feature = "master")]
callee.add_attribute(FnAttribute::Visibility(gccjit::Visibility::Hidden));
let block = func.new_block("entry");
let args = args
.iter()
.enumerate()
.map(|(i, _)| func.get_param(i as i32).to_rvalue())
.collect::<Vec<_>>();
let ret = context.new_call(None, callee, &args);
//llvm::LLVMSetTailCall(ret, True);
if output.is_some() {
block.end_with_return(None, ret);
if let Some(to_name) = to_name {
let args: Vec<_> = types
.iter()
.enumerate()
.map(|(index, typ)| context.new_parameter(None, *typ, format!("param{}", index)))
.collect();
let callee = context.new_function(
None,
FunctionType::Extern,
output.unwrap_or(void),
&args,
to_name,
false,
);
#[cfg(feature = "master")]
callee.add_attribute(FnAttribute::Visibility(gccjit::Visibility::Hidden));
let args = args
.iter()
.enumerate()
.map(|(i, _)| func.get_param(i as i32).to_rvalue())
.collect::<Vec<_>>();
let ret = context.new_call(None, callee, &args);
//llvm::LLVMSetTailCall(ret, True);
if output.is_some() {
block.end_with_return(None, ret);
} else {
block.add_eval(None, ret);
block.end_with_void_return(None);
}
} else {
assert!(output.is_none());
block.end_with_void_return(None);
}
+42 -32
View File
@@ -57,7 +57,7 @@ pub(crate) unsafe fn codegen(
let from_name = mangle_internal_symbol(tcx, &global_fn_name(method.name));
let to_name = mangle_internal_symbol(tcx, &default_fn_name(method.name));
create_wrapper_function(tcx, &cx, &from_name, &to_name, &args, output, false);
create_wrapper_function(tcx, &cx, &from_name, Some(&to_name), &args, output, false);
}
}
@@ -66,7 +66,7 @@ pub(crate) unsafe fn codegen(
tcx,
&cx,
&mangle_internal_symbol(tcx, "__rust_alloc_error_handler"),
&mangle_internal_symbol(tcx, alloc_error_handler_name(alloc_error_handler_kind)),
Some(&mangle_internal_symbol(tcx, alloc_error_handler_name(alloc_error_handler_kind))),
&[usize, usize], // size, align
None,
true,
@@ -81,11 +81,16 @@ pub(crate) unsafe fn codegen(
let llval = llvm::LLVMConstInt(i8, val as u64, False);
llvm::set_initializer(ll_g, llval);
let name = mangle_internal_symbol(tcx, NO_ALLOC_SHIM_IS_UNSTABLE);
let ll_g = cx.declare_global(&name, i8);
llvm::set_visibility(ll_g, llvm::Visibility::from_generic(tcx.sess.default_visibility()));
let llval = llvm::LLVMConstInt(i8, 0, False);
llvm::set_initializer(ll_g, llval);
// __rust_no_alloc_shim_is_unstable_v2
create_wrapper_function(
tcx,
&cx,
&mangle_internal_symbol(tcx, NO_ALLOC_SHIM_IS_UNSTABLE),
None,
&[],
None,
false,
);
}
if tcx.sess.opts.debuginfo != DebugInfo::None {
@@ -99,7 +104,7 @@ fn create_wrapper_function(
tcx: TyCtxt<'_>,
cx: &SimpleCx<'_>,
from_name: &str,
to_name: &str,
to_name: Option<&str>,
args: &[&Type],
output: Option<&Type>,
no_return: bool,
@@ -128,33 +133,38 @@ fn create_wrapper_function(
attributes::apply_to_llfn(llfn, llvm::AttributePlace::Function, &[uwtable]);
}
let callee = declare_simple_fn(
&cx,
to_name,
llvm::CallConv::CCallConv,
llvm::UnnamedAddr::Global,
llvm::Visibility::Hidden,
ty,
);
if let Some(no_return) = no_return {
// -> ! DIFlagNoReturn
attributes::apply_to_llfn(callee, llvm::AttributePlace::Function, &[no_return]);
}
llvm::set_visibility(callee, llvm::Visibility::Hidden);
let llbb = unsafe { llvm::LLVMAppendBasicBlockInContext(cx.llcx, llfn, c"entry".as_ptr()) };
let mut bx = SBuilder::build(&cx, llbb);
let args = args
.iter()
.enumerate()
.map(|(i, _)| llvm::get_param(llfn, i as c_uint))
.collect::<Vec<_>>();
let ret = bx.call(ty, callee, &args, None);
llvm::LLVMSetTailCall(ret, True);
if output.is_some() {
bx.ret(ret);
if let Some(to_name) = to_name {
let callee = declare_simple_fn(
&cx,
to_name,
llvm::CallConv::CCallConv,
llvm::UnnamedAddr::Global,
llvm::Visibility::Hidden,
ty,
);
if let Some(no_return) = no_return {
// -> ! DIFlagNoReturn
attributes::apply_to_llfn(callee, llvm::AttributePlace::Function, &[no_return]);
}
llvm::set_visibility(callee, llvm::Visibility::Hidden);
let args = args
.iter()
.enumerate()
.map(|(i, _)| llvm::get_param(llfn, i as c_uint))
.collect::<Vec<_>>();
let ret = bx.call(ty, callee, &args, None);
llvm::LLVMSetTailCall(ret, True);
if output.is_some() {
bx.ret(ret);
} else {
bx.ret_void()
}
} else {
assert!(output.is_none());
bx.ret_void()
}
}
@@ -219,6 +219,7 @@ fn exported_symbols_provider_local<'tcx>(
.chain([
mangle_internal_symbol(tcx, "__rust_alloc_error_handler"),
mangle_internal_symbol(tcx, OomStrategy::SYMBOL),
mangle_internal_symbol(tcx, NO_ALLOC_SHIM_IS_UNSTABLE),
])
{
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(tcx, &symbol_name));
@@ -232,19 +233,6 @@ fn exported_symbols_provider_local<'tcx>(
},
));
}
let exported_symbol = ExportedSymbol::NoDefId(SymbolName::new(
tcx,
&mangle_internal_symbol(tcx, NO_ALLOC_SHIM_IS_UNSTABLE),
));
symbols.push((
exported_symbol,
SymbolExportInfo {
level: SymbolExportLevel::Rust,
kind: SymbolExportKind::Data,
used: false,
},
))
}
if tcx.sess.instrument_coverage() || tcx.sess.opts.cg.profile_generate.enabled() {
-4
View File
@@ -85,10 +85,6 @@ pub fn mangle_internal_symbol<'tcx>(tcx: TyCtxt<'tcx>, item_name: &str) -> Strin
if item_name == "rust_eh_personality" {
// rust_eh_personality must not be renamed as LLVM hard-codes the name
return "rust_eh_personality".to_owned();
} else if item_name == "__rust_no_alloc_shim_is_unstable" {
// Temporary back compat hack to give people the chance to migrate to
// include #[rustc_std_internal_symbol].
return "__rust_no_alloc_shim_is_unstable".to_owned();
}
let prefix = "_R";
+4 -3
View File
@@ -31,8 +31,9 @@
#[rustc_std_internal_symbol]
fn __rust_alloc_zeroed(size: usize, align: usize) -> *mut u8;
#[rustc_nounwind]
#[rustc_std_internal_symbol]
static __rust_no_alloc_shim_is_unstable: u8;
fn __rust_no_alloc_shim_is_unstable_v2();
}
/// The global memory allocator.
@@ -88,7 +89,7 @@ pub unsafe fn alloc(layout: Layout) -> *mut u8 {
unsafe {
// Make sure we don't accidentally allow omitting the allocator shim in
// stable code until it is actually stabilized.
core::ptr::read_volatile(&__rust_no_alloc_shim_is_unstable);
__rust_no_alloc_shim_is_unstable_v2();
__rust_alloc(layout.size(), layout.align())
}
@@ -171,7 +172,7 @@ pub unsafe fn alloc_zeroed(layout: Layout) -> *mut u8 {
unsafe {
// Make sure we don't accidentally allow omitting the allocator shim in
// stable code until it is actually stabilized.
core::ptr::read_volatile(&__rust_no_alloc_shim_is_unstable);
__rust_no_alloc_shim_is_unstable_v2();
__rust_alloc_zeroed(layout.size(), layout.align())
}
@@ -45,10 +45,6 @@ fn weak_symbol_extern_statics(
/// Sets up the "extern statics" for this machine.
pub fn init_extern_statics(ecx: &mut MiriInterpCx<'tcx>) -> InterpResult<'tcx> {
// "__rust_no_alloc_shim_is_unstable"
let val = ImmTy::from_int(0, ecx.machine.layouts.u8); // always 0, value does not matter
Self::alloc_extern_static(ecx, "__rust_no_alloc_shim_is_unstable", val)?;
// "__rust_alloc_error_handler_should_panic"
let val = ecx.tcx.sess.opts.unstable_opts.oom.should_panic();
let val = ImmTy::from_int(val, ecx.machine.layouts.u8);
@@ -611,6 +611,10 @@ fn emulate_foreign_item_inner(
this.write_pointer(new_ptr, dest)
});
}
name if name == this.mangle_internal_symbol("__rust_no_alloc_shim_is_unstable_v2") => {
// This is a no-op shim that only exists to prevent making the allocator shims instantly stable.
let [] = this.check_shim(abi, CanonAbi::Rust, link_name, args)?;
}
// C memory handling functions
"memcmp" => {
@@ -1,7 +1,7 @@
#![no_std]
#![no_main]
//@compile-flags: -Zmiri-track-alloc-id=20 -Zmiri-track-alloc-accesses -Cpanic=abort
//@normalize-stderr-test: "id 20" -> "id $$ALLOC"
//@compile-flags: -Zmiri-track-alloc-id=19 -Zmiri-track-alloc-accesses -Cpanic=abort
//@normalize-stderr-test: "id 19" -> "id $$ALLOC"
//@only-target: linux # alloc IDs differ between OSes (due to extern static allocations)
extern "Rust" {
+2 -1
View File
@@ -5,7 +5,8 @@
pub fn alloc_test(data: u32) {
// CHECK-LABEL: @alloc_test
// CHECK-NEXT: start:
// CHECK-NEXT: {{.*}} load volatile i8, ptr @{{.*}}__rust_no_alloc_shim_is_unstable, align 1
// CHECK-NEXT: ; call __rustc::__rust_no_alloc_shim_is_unstable_v2
// CHECK-NEXT: tail call void @_R{{.+}}__rust_no_alloc_shim_is_unstable_v2()
// CHECK-NEXT: ret void
let x = Box::new(data);
drop(x);
+4 -2
View File
@@ -10,8 +10,10 @@
// See https://github.com/rust-lang/rust/issues/46515
// CHECK-LABEL: @check_no_escape_in_landingpad
// CHECK: start:
// CHECK-NEXT: __rust_no_alloc_shim_is_unstable
// CHECK-NEXT: __rust_no_alloc_shim_is_unstable
// CHECK-NEXT: ; call __rustc::__rust_no_alloc_shim_is_unstable_v2
// CHECK-NEXT: tail call void @[[NO_ALLOC_SHIM:_R.+__rust_no_alloc_shim_is_unstable_v2]]()
// CHECK-NEXT: ; call __rustc::__rust_no_alloc_shim_is_unstable_v2
// CHECK-NEXT: tail call void @[[NO_ALLOC_SHIM]]()
// CHECK-NEXT: ret void
#[no_mangle]
pub fn check_no_escape_in_landingpad(f: fn()) {
+4 -2
View File
@@ -4,7 +4,9 @@
#[no_mangle]
pub fn get_len() -> usize {
// CHECK-LABEL: @get_len
// CHECK-NOT: call
// CHECK-NOT: invoke
// CHECK-NEXT: start:
// CHECK-NEXT: ; call __rustc::__rust_no_alloc_shim_is_unstable_v2
// CHECK-NEXT: tail call void @_R{{.+}}__rust_no_alloc_shim_is_unstable_v2()
// CHECK-NEXT: ret i{{[0-9]+}} 3
[1, 2, 3].iter().collect::<Vec<_>>().len()
}
+2 -1
View File
@@ -5,7 +5,8 @@
pub fn sum_me() -> i32 {
// CHECK-LABEL: @sum_me
// CHECK-NEXT: {{^.*:$}}
// CHECK-NEXT: {{.*}} load volatile i8, ptr @{{.*}}__rust_no_alloc_shim_is_unstable, align 1
// CHECK-NEXT: ; call __rustc::__rust_no_alloc_shim_is_unstable_v2
// CHECK-NEXT: tail call void @_R{{.+}}__rust_no_alloc_shim_is_unstable_v2()
// CHECK-NEXT: ret i32 6
vec![1, 2, 3].iter().sum::<i32>()
}
+3 -3
View File
@@ -1,4 +1,4 @@
#![feature(default_alloc_error_handler)]
#![feature(rustc_attrs)]
#![no_std]
#![no_main]
@@ -31,8 +31,8 @@ unsafe fn dealloc(&self, _: *mut u8, _: Layout) {
}
#[cfg(not(check_feature_gate))]
#[no_mangle]
static __rust_no_alloc_shim_is_unstable: u8 = 0;
#[rustc_std_internal_symbol]
fn __rust_no_alloc_shim_is_unstable_v2() {}
#[no_mangle]
extern "C" fn main(_argc: core::ffi::c_int, _argv: *const *const i8) -> i32 {
+15 -7
View File
@@ -7,12 +7,6 @@
//@ ignore-cross-compile
// Reason: the compiled binary is executed
//@ ignore-msvc
//FIXME(Oneirical): Getting this to work on MSVC requires passing libcmt.lib to CC,
// which is not trivial to do.
// Tracking issue: https://github.com/rust-lang/rust/issues/128602
// Discussion: https://github.com/rust-lang/rust/pull/128407#discussion_r1702439172
use run_make_support::{cc, has_extension, has_prefix, run, rustc, shallow_find_files};
fn main() {
@@ -30,15 +24,28 @@ fn main() {
has_prefix(path, "libcompiler_builtins") && has_extension(path, "rlib")
});
#[allow(unused_mut)]
let mut platform_args = Vec::<String>::new();
#[cfg(target_env = "msvc")]
{
platform_args.push("-MD".to_string());
// `/link` tells MSVC that the remaining arguments are linker options.
platform_args.push("/link".to_string());
platform_args.push("vcruntime.lib".to_string());
platform_args.push("msvcrt.lib".to_string());
}
cc().input("foo.o")
.out_exe("foo")
.args(&platform_args)
.args(&alloc_libs)
.args(&core_libs)
.args(&compiler_builtins_libs)
.run();
run("foo");
// Check that linking without __rust_no_alloc_shim_is_unstable defined fails
// Check that linking without __rust_no_alloc_shim_is_unstable_v2 defined fails
rustc()
.input("foo.rs")
.crate_type("bin")
@@ -48,6 +55,7 @@ fn main() {
.run();
cc().input("foo.o")
.out_exe("foo")
.args(&platform_args)
.args(&alloc_libs)
.args(&core_libs)
.args(&compiler_builtins_libs)
@@ -35,10 +35,6 @@ fn symbols_check_archive(path: &str) {
continue; // All compiler-builtins symbols must remain unmangled
}
if name == "__rust_no_alloc_shim_is_unstable" {
continue; // FIXME remove exception once we mangle this symbol
}
if name.contains("rust_eh_personality") {
continue; // Unfortunately LLVM doesn't allow us to mangle this symbol
}
@@ -75,10 +71,6 @@ fn symbols_check(path: &str) {
continue;
}
if name == "__rust_no_alloc_shim_is_unstable" {
continue; // FIXME remove exception once we mangle this symbol
}
if name.contains("rust_eh_personality") {
continue; // Unfortunately LLVM doesn't allow us to mangle this symbol
}
+1
View File
@@ -503,3 +503,4 @@ fun:__rust_realloc=uninstrumented
fun:_ZN4core*=uninstrumented
fun:_ZN3std*=uninstrumented
fun:rust_eh_personality=uninstrumented
fun:_R*__rustc*=uninstrumented