Rollup merge of rust-lang#145724 - folkertdev:track-caller-drop-no-mangle, r=fee1-dead

the `#[track_caller]` shim should not inherit `#[no_mangle]`

fixes rust-lang#143162

builds on rust-lang#143293 which introduced a mechanism to strip attributes from shims.

cc `@Jules-Bertholet` `@workingjubilee` `@bjorn3`

---

Summary:

This PR fixes an interaction between `#[track_caller]`, `#[no_mangle]`, and casting to a function pointer.

A function annotated with `#[track_caller]` internally has a hidden extra argument for the panic location. The `#[track_caller]` attribute is only allowed on `extern "Rust"` functions. When a function is annotated with both `#[no_mangle]` and `#[track_caller]`, the exported symbol has the signature that includes the extra panic location argument. This works on stable rust today:

```rust
extern "Rust" {
    #[track_caller]
    fn rust_track_caller_ffi_test_tracked() -> &'static Location<'static>;
}

mod provides {
    use std::panic::Location;
    #[track_caller] // UB if we did not have this!
    #[no_mangle]
    fn rust_track_caller_ffi_test_tracked() -> &'static Location<'static> {
        Location::caller()
    }
}
```

When a `#[track_caller]` function is converted to a function pointer, a shim is added to drop the additional argument. So this is a valid program:

```rust
#[track_caller]
fn foo() {}

fn main() {
    let f = foo as fn();
    f();
}
```

The issue arises when `foo` is additionally annotated with `#[no_mangle]`, the generated shim currently inherits this attribute, also exporting a symbol named `foo`, but one without the hidden panic location argument. The linker rightfully complains about a duplicate symbol.

The solution of this PR is to have the generated shim drop the `#[no_mangle]` attribute.

Author: matthiaskrgr

compiler/rustc_codegen_cranelift/src/abi/mod.rs

@@ -467,7 +467,7 @@ pub(crate) fn codegen_terminator_call<'tcx>(
         true
     } else {
         instance.is_some_and(|inst| {
-            fx.tcx.codegen_fn_attrs(inst.def_id()).flags.contains(CodegenFnAttrFlags::COLD)
+            fx.tcx.codegen_instance_attrs(inst.def).flags.contains(CodegenFnAttrFlags::COLD)
         })
     };
     if is_cold {

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -200,10 +200,11 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
         let fn_ty = bx.fn_decl_backend_type(fn_abi);
 
         let fn_attrs = if bx.tcx().def_kind(fx.instance.def_id()).has_codegen_attrs() {
-            Some(bx.tcx().codegen_fn_attrs(fx.instance.def_id()))
+            Some(bx.tcx().codegen_instance_attrs(fx.instance.def))
         } else {
             None
         };
+        let fn_attrs = fn_attrs.as_deref();
 
         if !fn_abi.can_unwind {
             unwind = mir::UnwindAction::Unreachable;

compiler/rustc_middle/src/middle/codegen_fn_attrs.rs

@@ -13,6 +13,8 @@ impl<'tcx> TyCtxt<'tcx> {
         self,
         instance_kind: InstanceKind<'_>,
     ) -> Cow<'tcx, CodegenFnAttrs> {
+        // NOTE: we try to not clone the `CodegenFnAttrs` when that is not needed.
+        // The `to_mut` method used below clones the inner value.
         let mut attrs = Cow::Borrowed(self.codegen_fn_attrs(instance_kind.def_id()));
 
         // Drop the `#[naked]` attribute on non-item `InstanceKind`s, like the shims that
@@ -23,6 +25,28 @@ impl<'tcx> TyCtxt<'tcx> {
             }
         }
 
+        // A shim created by `#[track_caller]` should not inherit any attributes
+        // that modify the symbol name. Failing to remove these attributes from
+        // the shim leads to errors like `symbol `foo` is already defined`.
+        //
+        // A `ClosureOnceShim` with the track_caller attribute does not have a symbol,
+        // and therefore can be skipped here.
+        if let InstanceKind::ReifyShim(_, _) = instance_kind
+            && attrs.flags.contains(CodegenFnAttrFlags::TRACK_CALLER)
+        {
+            if attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE) {
+                attrs.to_mut().flags.remove(CodegenFnAttrFlags::NO_MANGLE);
+            }
+
+            if attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) {
+                attrs.to_mut().flags.remove(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL);
+            }
+
+            if attrs.symbol_name.is_some() {
+                attrs.to_mut().symbol_name = None;
+            }
+        }
+
         attrs
     }
 }

compiler/rustc_mir_transform/src/inline.rs

@@ -245,7 +245,7 @@ impl<'tcx> Inliner<'tcx> for ForceInliner<'tcx> {
     fn on_inline_failure(&self, callsite: &CallSite<'tcx>, reason: &'static str) {
         let tcx = self.tcx();
         let InlineAttr::Force { attr_span, reason: justification } =
-            tcx.codegen_fn_attrs(callsite.callee.def_id()).inline
+            tcx.codegen_instance_attrs(callsite.callee.def).inline
         else {
             bug!("called on item without required inlining");
         };
@@ -603,7 +603,8 @@ fn try_inlining<'tcx, I: Inliner<'tcx>>(
     let tcx = inliner.tcx();
     check_mir_is_available(inliner, caller_body, callsite.callee)?;
 
-    let callee_attrs = tcx.codegen_fn_attrs(callsite.callee.def_id());
+    let callee_attrs = tcx.codegen_instance_attrs(callsite.callee.def);
+    let callee_attrs = callee_attrs.as_ref();
     check_inline::is_inline_valid_on_fn(tcx, callsite.callee.def_id())?;
     check_codegen_attributes(inliner, callsite, callee_attrs)?;
     inliner.check_codegen_attributes_extra(callee_attrs)?;

tests/ui/rfcs/rfc-2091-track-caller/shim-does-not-modify-symbol.rs

@@ -0,0 +1,26 @@
+//@ run-pass
+#![feature(rustc_attrs)]
+
+// The shim that is generated for a function annotated with `#[track_caller]` should not inherit
+// attributes that modify its symbol name. Failing to remove these attributes from the shim
+// leads to errors like `symbol `foo` is already defined`.
+//
+// See also https://github.com/rust-lang/rust/issues/143162.
+
+#[unsafe(no_mangle)]
+#[track_caller]
+pub fn foo() {}
+
+#[unsafe(export_name = "bar")]
+#[track_caller]
+pub fn bar() {}
+
+#[rustc_std_internal_symbol]
+#[track_caller]
+pub fn baz() {}
+
+fn main() {
+    let _a = foo as fn();
+    let _b = bar as fn();
+    let _c = baz as fn();
+}

tests/ui/rfcs/rfc-2091-track-caller/track-caller-ffi.rs

@@ -2,14 +2,14 @@
 
 use std::panic::Location;
 
-extern "Rust" {
+unsafe extern "Rust" {
     #[track_caller]
-    fn rust_track_caller_ffi_test_tracked() -> &'static Location<'static>;
-    fn rust_track_caller_ffi_test_untracked() -> &'static Location<'static>;
+    safe fn rust_track_caller_ffi_test_tracked() -> &'static Location<'static>;
+    safe fn rust_track_caller_ffi_test_untracked() -> &'static Location<'static>;
 }
 
 fn rust_track_caller_ffi_test_nested_tracked() -> &'static Location<'static> {
-    unsafe { rust_track_caller_ffi_test_tracked() }
+    rust_track_caller_ffi_test_tracked()
 }
 
 mod provides {
@@ -31,18 +31,23 @@ fn main() {
     assert_eq!(location.line(), 29);
     assert_eq!(location.column(), 20);
 
-    let tracked = unsafe { rust_track_caller_ffi_test_tracked() };
+    let tracked = rust_track_caller_ffi_test_tracked();
     assert_eq!(tracked.file(), file!());
     assert_eq!(tracked.line(), 34);
-    assert_eq!(tracked.column(), 28);
+    assert_eq!(tracked.column(), 19);
 
-    let untracked = unsafe { rust_track_caller_ffi_test_untracked() };
+    let untracked = rust_track_caller_ffi_test_untracked();
     assert_eq!(untracked.file(), file!());
     assert_eq!(untracked.line(), 24);
     assert_eq!(untracked.column(), 9);
 
     let contained = rust_track_caller_ffi_test_nested_tracked();
     assert_eq!(contained.file(), file!());
     assert_eq!(contained.line(), 12);
-    assert_eq!(contained.column(), 14);
+    assert_eq!(contained.column(), 5);
+
+    let indirect = (rust_track_caller_ffi_test_tracked as fn() -> &'static Location<'static>)();
+    assert_eq!(indirect.file(), file!());
+    assert_eq!(indirect.line(), 7);
+    assert_eq!(indirect.column(), 5);
 }