declare imported functions with a nondescript signature

This means when the function is imported with multiple different signatures, they don't clash

Author: RalfJung

compiler/rustc_ast_passes/src/feature_gate.rs

@@ -324,7 +324,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> {
                         &self,
                         link_llvm_intrinsics,
                         i.span,
-                        "linking to LLVM intrinsics is experimental"
+                        "linking to LLVM intrinsics is an internal feature reserved for the standard library"
                     );
                 }
             }

compiler/rustc_codegen_llvm/src/abi.rs

@@ -321,7 +321,12 @@ pub trait FnAbiLlvmExt<'ll, 'tcx> {
     );
 
     /// Apply attributes to a function call.
-    fn apply_attrs_callsite(&self, bx: &mut Builder<'_, 'll, 'tcx>, callsite: &'ll Value);
+    fn apply_attrs_callsite(
+        &self,
+        bx: &mut Builder<'_, 'll, 'tcx>,
+        callsite: &'ll Value,
+        instance: Option<ty::Instance<'tcx>>,
+    );
 }
 
 impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
@@ -527,11 +532,18 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
 
         // If the declaration has an associated instance, compute extra attributes based on that.
         if let Some(instance) = instance {
-            llfn_attrs_from_instance(cx, llfn, instance);
+            llfn_attrs_from_instance(cx, instance, Some(llfn), |place, attrs| {
+                attributes::apply_to_llfn(llfn, place, attrs)
+            });
         }
     }
 
-    fn apply_attrs_callsite(&self, bx: &mut Builder<'_, 'll, 'tcx>, callsite: &'ll Value) {
+    fn apply_attrs_callsite(
+        &self,
+        bx: &mut Builder<'_, 'll, 'tcx>,
+        callsite: &'ll Value,
+        instance: Option<ty::Instance<'tcx>>,
+    ) {
         let mut func_attrs = SmallVec::<[_; 2]>::new();
         if self.ret.layout.abi.is_uninhabited() {
             func_attrs.push(llvm::AttributeKind::NoReturn.create_attr(bx.cx.llcx));
@@ -649,6 +661,13 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
                 &[element_type_attr],
             );
         }
+
+        // If the call site has an associated instance, compute extra attributes based on that.
+        if let Some(instance) = instance {
+            llfn_attrs_from_instance(bx.cx, instance, None, |place, attrs| {
+                attributes::apply_to_callsite(callsite, place, attrs)
+            });
+        }
     }
 }
 

compiler/rustc_codegen_llvm/src/attributes.rs

@@ -14,8 +14,8 @@ use crate::context::CodegenCx;
 use crate::errors::{MissingFeatures, SanitizerMemtagRequiresMte, TargetFeatureDisableOrEnable};
 use crate::llvm::AttributePlace::Function;
 use crate::llvm::{self, AllocKindFlags, Attribute, AttributeKind, AttributePlace, MemoryEffects};
+use crate::llvm_util;
 use crate::value::Value;
-use crate::{attributes, llvm_util};
 
 pub fn apply_to_llfn(llfn: &Value, idx: AttributePlace, attrs: &[&Attribute]) {
     if !attrs.is_empty() {
@@ -324,13 +324,18 @@ fn create_alloc_family_attr(llcx: &llvm::Context) -> &llvm::Attribute {
     llvm::CreateAttrStringValue(llcx, "alloc-family", "__rust_alloc")
 }
 
-/// Helper for `FnAbi::apply_attrs_llfn`:
+/// Helper for `FnAbi::apply_attrs_*`:
 /// Composite function which sets LLVM attributes for function depending on its AST (`#[attribute]`)
 /// attributes.
+///
+/// `apply_attrs` is called to apply the attributes, so this can be used both for declarations and calls.
+/// However, some things are not represented as attributes and and only be set on declarations, so
+/// `declare_llfn` should be `Some` if this is a declaration.
 pub fn llfn_attrs_from_instance<'ll, 'tcx>(
     cx: &CodegenCx<'ll, 'tcx>,
-    llfn: &'ll Value,
     instance: ty::Instance<'tcx>,
+    declare_llfn: Option<&'ll Value>,
+    apply_attrs: impl Fn(AttributePlace, &[&Attribute]),
 ) {
     let codegen_fn_attrs = cx.tcx.codegen_fn_attrs(instance.def_id());
 
@@ -440,7 +445,7 @@ pub fn llfn_attrs_from_instance<'ll, 'tcx>(
         to_add.push(create_alloc_family_attr(cx.llcx));
         // apply to argument place instead of function
         let alloc_align = AttributeKind::AllocAlign.create_attr(cx.llcx);
-        attributes::apply_to_llfn(llfn, AttributePlace::Argument(1), &[alloc_align]);
+        apply_attrs(AttributePlace::Argument(1), &[alloc_align]);
         to_add.push(llvm::CreateAllocSizeAttr(cx.llcx, 0));
         let mut flags = AllocKindFlags::Alloc | AllocKindFlags::Aligned;
         if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::ALLOCATOR) {
@@ -451,7 +456,7 @@ pub fn llfn_attrs_from_instance<'ll, 'tcx>(
         to_add.push(llvm::CreateAllocKindAttr(cx.llcx, flags));
         // apply to return place instead of function (unlike all other attributes applied in this function)
         let no_alias = AttributeKind::NoAlias.create_attr(cx.llcx);
-        attributes::apply_to_llfn(llfn, AttributePlace::ReturnValue, &[no_alias]);
+        apply_attrs(AttributePlace::ReturnValue, &[no_alias]);
     }
     if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::REALLOCATOR) {
         to_add.push(create_alloc_family_attr(cx.llcx));
@@ -461,26 +466,28 @@ pub fn llfn_attrs_from_instance<'ll, 'tcx>(
         ));
         // applies to argument place instead of function place
         let allocated_pointer = AttributeKind::AllocatedPointer.create_attr(cx.llcx);
-        attributes::apply_to_llfn(llfn, AttributePlace::Argument(0), &[allocated_pointer]);
+        apply_attrs(AttributePlace::Argument(0), &[allocated_pointer]);
         // apply to argument place instead of function
         let alloc_align = AttributeKind::AllocAlign.create_attr(cx.llcx);
-        attributes::apply_to_llfn(llfn, AttributePlace::Argument(2), &[alloc_align]);
+        apply_attrs(AttributePlace::Argument(2), &[alloc_align]);
         to_add.push(llvm::CreateAllocSizeAttr(cx.llcx, 3));
         let no_alias = AttributeKind::NoAlias.create_attr(cx.llcx);
-        attributes::apply_to_llfn(llfn, AttributePlace::ReturnValue, &[no_alias]);
+        apply_attrs(AttributePlace::ReturnValue, &[no_alias]);
     }
     if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::DEALLOCATOR) {
         to_add.push(create_alloc_family_attr(cx.llcx));
         to_add.push(llvm::CreateAllocKindAttr(cx.llcx, AllocKindFlags::Free));
         // applies to argument place instead of function place
         let allocated_pointer = AttributeKind::AllocatedPointer.create_attr(cx.llcx);
-        attributes::apply_to_llfn(llfn, AttributePlace::Argument(0), &[allocated_pointer]);
+        apply_attrs(AttributePlace::Argument(0), &[allocated_pointer]);
     }
     if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::CMSE_NONSECURE_ENTRY) {
         to_add.push(llvm::CreateAttrString(cx.llcx, "cmse_nonsecure_entry"));
     }
     if let Some(align) = codegen_fn_attrs.alignment {
-        llvm::set_alignment(llfn, align);
+        if let Some(llfn) = declare_llfn {
+            llvm::set_alignment(llfn, align);
+        }
     }
     if let Some(backchain) = backchain_attr(cx) {
         to_add.push(backchain);
@@ -551,7 +558,7 @@ pub fn llfn_attrs_from_instance<'ll, 'tcx>(
         to_add.push(llvm::CreateAttrStringValue(cx.llcx, "target-features", &target_features));
     }
 
-    attributes::apply_to_llfn(llfn, Function, &to_add);
+    apply_attrs(Function, &to_add);
 }
 
 fn wasm_import_module(tcx: TyCtxt<'_>, id: DefId) -> Option<&String> {

compiler/rustc_codegen_llvm/src/builder.rs

@@ -262,7 +262,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
             )
         };
         if let Some(fn_abi) = fn_abi {
-            fn_abi.apply_attrs_callsite(self, invoke);
+            fn_abi.apply_attrs_callsite(self, invoke, instance);
         }
         invoke
     }
@@ -1307,7 +1307,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
             )
         };
         if let Some(fn_abi) = fn_abi {
-            fn_abi.apply_attrs_callsite(self, call);
+            fn_abi.apply_attrs_callsite(self, call, instance);
         }
         call
     }
@@ -1645,7 +1645,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
             )
         };
         if let Some(fn_abi) = fn_abi {
-            fn_abi.apply_attrs_callsite(self, callbr);
+            fn_abi.apply_attrs_callsite(self, callbr, instance);
         }
         callbr
     }

compiler/rustc_codegen_llvm/src/declare.rs

@@ -12,8 +12,9 @@
 //! * When in doubt, define.
 
 use itertools::Itertools;
-use rustc_codegen_ssa::traits::TypeMembershipMethods;
+use rustc_codegen_ssa::traits::{BaseTypeMethods, TypeMembershipMethods};
 use rustc_data_structures::fx::FxIndexSet;
+use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc_middle::ty::{Instance, Ty};
 use rustc_sanitizers::{cfi, kcfi};
 use smallvec::SmallVec;
@@ -127,6 +128,47 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
     ) -> &'ll Value {
         debug!("declare_rust_fn(name={:?}, fn_abi={:?})", name, fn_abi);
 
+        // FFI imports need to be treated specially: in Rust one can import the same function
+        // with different signatures, but LLVM can only import each function once. Within a
+        // codegen unit, whatever declaration we set up last will "win"; across codegen units,
+        // LLVM is doing some sort of unspecified merging as part of LTO.
+        // This is partially unsound due to LLVM bugs (https://github.com/llvm/llvm-project/issues/58976),
+        // but on the Rust side we try to ensure soundness by making the least amount of promises
+        // for these imports: no matter the signatures, we declare all functions as `void fn()`.
+        // If the same symbol is declared both as a function and a static, we just hope that
+        // doesn't lead to unsoundness (or LLVM crashes)...
+        let ffi_import = instance.is_some_and(|i| {
+            self.tcx.is_foreign_item(i.def_id()) && {
+                // If this is a Rust native allocator function, we want its attributes, so we do not
+                // treat it like the other FFI imports. If a user declares `__rust_alloc` we're going to have
+                // one import with the attributes and one with the default signature; that should hopefully be fine
+                // even if LLVM only sees the default one.
+                let codegen_fn_attrs = self.tcx.codegen_fn_attrs(i.def_id());
+                let is_alloc_fn = codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::ALLOCATOR)
+                    || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::ALLOCATOR_ZEROED)
+                    || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::REALLOCATOR)
+                    || codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::DEALLOCATOR);
+                !is_alloc_fn
+            }
+        }) && {
+            // If this is calling an LLVM intrinsic, we can't use the `global` trick.
+            // These are also unstable to call so nobody but us can screw up their signature.
+            let is_llvm_intrinsic = name.starts_with("llvm.");
+            !is_llvm_intrinsic
+        };
+        if ffi_import {
+            // This signatures is essentially irrelevant since we set all the attributes at the call
+            // site.
+            return declare_raw_fn(
+                self,
+                name,
+                llvm::CallConv::CCallConv,
+                llvm::UnnamedAddr::Global,
+                llvm::Visibility::Default,
+                self.type_func(&[], self.type_void()),
+            );
+        }
+
         // Function addresses in Rust are never significant, allowing functions to
         // be merged.
         let llfn = declare_raw_fn(

tests/codegen/box-uninit-bytes.rs

@@ -8,7 +8,7 @@ use std::mem::MaybeUninit;
 pub fn box_uninitialized() -> Box<MaybeUninit<usize>> {
     // CHECK-LABEL: @box_uninitialized
     // CHECK-NOT: store
-    // CHECK-NOT: alloca
+    // CHECK-NOT: "alloca "
     // CHECK-NOT: memcpy
     // CHECK-NOT: memset
     Box::new(MaybeUninit::uninit())
@@ -19,7 +19,7 @@ pub fn box_uninitialized() -> Box<MaybeUninit<usize>> {
 pub fn box_uninitialized2() -> Box<MaybeUninit<[usize; 1024 * 1024]>> {
     // CHECK-LABEL: @box_uninitialized2
     // CHECK-NOT: store
-    // CHECK-NOT: alloca
+    // CHECK-NOT: "alloca ""
     // CHECK-NOT: memcpy
     // CHECK-NOT: memset
     Box::new(MaybeUninit::uninit())
@@ -32,7 +32,7 @@ pub struct LotsaPadding(usize);
 #[no_mangle]
 pub fn box_lotsa_padding() -> Box<LotsaPadding> {
     // CHECK-LABEL: @box_lotsa_padding
-    // CHECK-NOT: alloca
+    // CHECK-NOT: "alloca "
     // CHECK-NOT: getelementptr
     // CHECK-NOT: memcpy
     // CHECK-NOT: memset

tests/codegen/iter-repeat-n-trivial-drop.rs

@@ -47,7 +47,7 @@ pub fn iter_repeat_n_next(it: &mut std::iter::RepeatN<NotCopy>) -> Option<NotCop
 #[no_mangle]
 // CHECK-LABEL: @vec_extend_via_iter_repeat_n
 pub fn vec_extend_via_iter_repeat_n() -> Vec<u8> {
-    // CHECK: %[[ADDR:.+]] = tail call {{(noalias )?}}noundef dereferenceable_or_null(1234) ptr @__rust_alloc(i64 noundef 1234, i64 noundef 1)
+    // CHECK: %[[ADDR:.+]] = tail call {{(noalias )?}}noundef dereferenceable_or_null(1234) ptr @__rust_alloc(i64 noundef 1234, i64 {{(allocalign )?}}noundef 1)
     // CHECK: tail call void @llvm.memset.p0.i64(ptr noundef nonnull align 1 dereferenceable(1234) %[[ADDR]], i8 42, i64 1234,
 
     let n = 1234_usize;

tests/codegen/no_builtins-at-crate.rs

@@ -10,15 +10,13 @@
 pub unsafe extern "C" fn __aeabi_memcpy(dest: *mut u8, src: *const u8, size: usize) {
     // CHECK: call
     // CHECK-SAME: @memcpy(
+    // CHECK-SAME: #2
     memcpy(dest, src, size);
 }
 
-// CHECK: declare
-// CHECK-SAME: @memcpy
-// CHECK-SAME: #0
 extern "C" {
     pub fn memcpy(dest: *mut u8, src: *const u8, n: usize) -> *mut u8;
 }
 
-// CHECK: attributes #0
+// CHECK: attributes #2
 // CHECK-SAME: "no-builtins"

tests/codegen/pic-relocation-model.rs

@@ -8,10 +8,7 @@ pub fn call_foreign_fn() -> u8 {
     unsafe { foreign_fn() }
 }
 
-// (Allow but do not require `zeroext` here, because it is not worth effort to
-// spell out which targets have it and which ones do not; see rust#97800.)
-
-// CHECK: declare{{( zeroext)?}} i8 @foreign_fn()
+// CHECK: declare void @foreign_fn()
 extern "C" {
     fn foreign_fn() -> u8;
 }

tests/codegen/pie-relocation-model.rs

@@ -13,7 +13,7 @@ pub fn call_foreign_fn() -> u8 {
 
 // External functions are still marked as non-dso_local, since we don't know if the symbol
 // is defined in the binary or in the shared library.
-// CHECK: declare zeroext i8 @foreign_fn()
+// CHECK: declare void @foreign_fn()
 extern "C" {
     fn foreign_fn() -> u8;
 }

tests/codegen/unwind-extern-imports.rs

@@ -3,9 +3,21 @@
 
 #![crate_type = "lib"]
 
+// The flag are only at the call sites, not at the declarations.
+// CHECK-LABEL: @force_declare
+#[no_mangle]
+pub unsafe fn force_declare() {
+    // CHECK: call void @extern_fn() [[NOUNWIND_ATTR:#[0-9]+]]
+    extern_fn();
+    // Call without attributes.
+    // FIXME: Make sure that there truly are no attributes! How can we match "end of line" with FileCheck?
+    // CHECK: call void @c_unwind_extern_fn()
+    c_unwind_extern_fn();
+}
+
 extern "C" {
-    // CHECK: Function Attrs:{{.*}}nounwind
-    // CHECK-NEXT: declare{{.*}}void @extern_fn
+    // CHECK-NOT: nounwind
+    // CHECK: declare{{.*}}void @extern_fn
     fn extern_fn();
 }
 
@@ -15,7 +27,4 @@ extern "C-unwind" {
     fn c_unwind_extern_fn();
 }
 
-pub unsafe fn force_declare() {
-    extern_fn();
-    c_unwind_extern_fn();
-}
+// CHECK: attributes [[NOUNWIND_ATTR]] = { {{.*}}nounwind{{.*}} }

tests/codegen/unwind-landingpad-cold.rs

@@ -8,7 +8,8 @@
 
 // CHECK-LABEL: @check_cold
 // CHECK: {{(call|invoke) void .+}}drop_in_place{{.+}} [[ATTRIBUTES:#[0-9]+]]
-// CHECK: attributes [[ATTRIBUTES]] = { cold }
+// CHECK: attributes [[ATTRIBUTES]]
+// CHECK-SAME: cold
 #[no_mangle]
 pub fn check_cold(f: fn(), x: Box<u32>) {
     // this may unwind