Fix linkage

src/attributes.rs

@@ -15,6 +15,8 @@ use rustc_target::callconv::FnAbi;
 #[cfg(feature = "master")]
 use rustc_target::spec::Arch;
 
+#[cfg(feature = "master")]
+use crate::base;
 use crate::context::CodegenCx;
 use crate::gcc_util::to_gcc_features;
 
@@ -116,6 +118,17 @@ pub fn from_fn_attrs<'gcc, 'tcx>(
         } else {
             codegen_fn_attrs.inline
         };
+        // GCC warns that `inline` and `weak` conflict, and cg_gcc turns libgccjit warnings into
+        // errors. The linkage is what has to survive: rustc lints `#[inline]` as ignored on a
+        // function with an explicit `#[linkage]` anyway. `inline(never)` does not conflict.
+        let inline = match inline {
+            InlineAttr::Always | InlineAttr::Hint | InlineAttr::Force { .. }
+                if codegen_fn_attrs.linkage.is_some_and(base::linkage_needs_weak_attribute) =>
+            {
+                InlineAttr::None
+            }
+            inline => inline,
+        };
         if let Some(attr) = inline_attr(cx, inline, instance) {
             if let FnAttribute::AlwaysInline = attr {
                 func.add_attribute(FnAttribute::Inline);

src/base.rs

@@ -39,32 +39,59 @@ pub fn symbol_visibility_to_gcc(visibility: SymbolVisibility) -> gccjit::Visibil
     }
 }
 
+/// The kind of a global *definition* with an explicit `#[linkage]`.
+///
+/// The flavours that another object file is allowed to override also need
+/// `linkage_needs_weak_attribute` from the caller: `GlobalKind` alone cannot express weakness.
 pub fn global_linkage_to_gcc(linkage: Linkage) -> GlobalKind {
     match linkage {
-        Linkage::External => GlobalKind::Imported,
-        Linkage::AvailableExternally => GlobalKind::Imported,
-        Linkage::LinkOnceAny => unimplemented!(),
-        Linkage::LinkOnceODR => unimplemented!(),
-        Linkage::WeakAny => unimplemented!(),
-        Linkage::WeakODR => unimplemented!(),
-        Linkage::Internal => GlobalKind::Internal,
-        Linkage::ExternalWeak => GlobalKind::Imported, // FIXME(antoyo): should be weak linkage.
-        Linkage::Common => unimplemented!(),
+        Linkage::External => GlobalKind::Exported,
+        // libgccjit cannot emit a definition that the linker discards in favour of the one in
+        // another object file, so emit a private copy of it instead.
+        Linkage::AvailableExternally | Linkage::Internal => GlobalKind::Internal,
+        // libgccjit exposes neither comdat nor common storage, so `weak` stands in for every
+        // overridable flavour.
+        Linkage::LinkOnceAny
+        | Linkage::LinkOnceODR
+        | Linkage::WeakAny
+        | Linkage::WeakODR
+        | Linkage::ExternalWeak
+        | Linkage::Common => GlobalKind::Exported,
     }
 }
 
+/// The type of a function *definition* with an explicit `#[linkage]`.
+///
+/// The flavours that another object file is allowed to override also need
+/// `linkage_needs_weak_attribute` from the caller: `FunctionType` alone cannot express weakness.
 pub fn linkage_to_gcc(linkage: Linkage) -> FunctionType {
     match linkage {
         Linkage::External => FunctionType::Exported,
-        // FIXME(antoyo): set the attribute externally_visible.
-        Linkage::AvailableExternally => FunctionType::Extern,
-        Linkage::LinkOnceAny => unimplemented!(),
-        Linkage::LinkOnceODR => unimplemented!(),
-        Linkage::WeakAny => FunctionType::Exported, // FIXME(antoyo): should be similar to linkonce.
-        Linkage::WeakODR => unimplemented!(),
-        Linkage::Internal => FunctionType::Internal,
-        Linkage::ExternalWeak => unimplemented!(),
-        Linkage::Common => unimplemented!(),
+        // libgccjit cannot emit a definition that the linker discards in favour of the one in
+        // another object file, so emit a private copy of it instead.
+        Linkage::AvailableExternally | Linkage::Internal => FunctionType::Internal,
+        // libgccjit exposes no comdat, so `weak` stands in for every overridable flavour.
+        Linkage::LinkOnceAny
+        | Linkage::LinkOnceODR
+        | Linkage::WeakAny
+        | Linkage::WeakODR
+        | Linkage::ExternalWeak
+        | Linkage::Common => FunctionType::Exported,
+    }
+}
+
+/// Whether a definition with this linkage must carry the `weak` attribute, so that a strong
+/// definition in another object file wins over it instead of clashing with it.
+#[cfg(feature = "master")]
+pub fn linkage_needs_weak_attribute(linkage: Linkage) -> bool {
+    match linkage {
+        Linkage::LinkOnceAny
+        | Linkage::LinkOnceODR
+        | Linkage::WeakAny
+        | Linkage::WeakODR
+        | Linkage::ExternalWeak
+        | Linkage::Common => true,
+        Linkage::External | Linkage::AvailableExternally | Linkage::Internal => false,
     }
 }
 

src/consts.rs

@@ -21,7 +21,6 @@ use rustc_middle::ty::{self, Instance};
 use rustc_middle::{bug, span_bug};
 use rustc_span::def_id::DefId;
 
-use crate::base;
 use crate::common::bytes_type_in_context;
 use crate::context::CodegenCx;
 use crate::type_::struct_attributes;
@@ -469,10 +468,10 @@ fn check_and_apply_linkage<'gcc, 'tcx>(
 ) -> LValue<'gcc> {
     let is_tls = attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL);
     if let Some(linkage) = attrs.import_linkage {
-        // Declare a symbol `foo` with the desired linkage.
-        let global1 =
-            cx.declare_global_with_linkage(sym, cx.type_i8(), base::global_linkage_to_gcc(linkage));
+        // Whatever the flavour, an import is an undefined reference to a symbol defined elsewhere.
+        let global1 = cx.declare_global_with_linkage(sym, cx.type_i8(), GlobalKind::Imported);
 
+        // Only `extern_weak` lets the symbol stay unresolved, in which case it reads as null.
         if linkage == Linkage::ExternalWeak {
             #[cfg(feature = "master")]
             global1.add_attribute(VarAttribute::Weak);
@@ -486,8 +485,13 @@ fn check_and_apply_linkage<'gcc, 'tcx>(
         // zero.
         let real_name =
             format!("_rust_extern_with_linkage_{:016x}_{sym}", cx.tcx.stable_crate_id(LOCAL_CRATE));
-        let global2 = cx.define_global(&real_name, gcc_type, is_tls, attrs.link_section);
-        // FIXME(antoyo): set linkage.
+        let global2 = cx.define_global(
+            &real_name,
+            gcc_type,
+            GlobalKind::Internal,
+            is_tls,
+            attrs.link_section,
+        );
         let value = cx.const_ptrcast(global1.get_address(None), gcc_type);
         global2.global_set_initializer_rvalue(value);
         global2

src/declare.rs

@@ -14,6 +14,7 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
         &self,
         name: &str,
         ty: Type<'gcc>,
+        global_kind: GlobalKind,
         is_tls: bool,
         link_section: Option<Symbol>,
     ) -> LValue<'gcc> {
@@ -31,7 +32,7 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
             }
             global
         } else {
-            self.declare_global(name, ty, GlobalKind::Exported, is_tls, link_section)
+            self.declare_global(name, ty, global_kind, is_tls, link_section)
         }
     }
 
@@ -141,10 +142,11 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
         &self,
         name: &str,
         ty: Type<'gcc>,
+        global_kind: GlobalKind,
         is_tls: bool,
         link_section: Option<Symbol>,
     ) -> LValue<'gcc> {
-        self.get_or_insert_global(name, ty, is_tls, link_section)
+        self.get_or_insert_global(name, ty, global_kind, is_tls, link_section)
     }
 
     pub fn get_declared_value(&self, name: &str) -> Option<RValue<'gcc>> {

src/mono_item.rs

@@ -21,7 +21,7 @@ impl<'gcc, 'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
     fn predefine_static(
         &mut self,
         def_id: DefId,
-        _linkage: Linkage,
+        linkage: Linkage,
         visibility: Visibility,
         global_name: &str,
     ) {
@@ -47,10 +47,29 @@ impl<'gcc, 'tcx> PreDefineCodegenMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
         };
 
         let is_tls = attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL);
-        let global = self.define_global(global_name, gcc_type, is_tls, attrs.link_section);
+        let global_kind = base::global_linkage_to_gcc(linkage);
+        let global =
+            self.define_global(global_name, gcc_type, global_kind, is_tls, attrs.link_section);
         #[cfg(feature = "master")]
-        global.add_attribute(VarAttribute::Visibility(base::visibility_to_gcc(visibility)));
-        // FIXME(antoyo): set linkage.
+        {
+            // GCC warns that it ignores `visibility` on an internal global, and cg_gcc turns
+            // libgccjit warnings into errors.
+            if !matches!(global_kind, GlobalKind::Internal) {
+                // If we're compiling the compiler-builtins crate, e.g., the equivalent of
+                // compiler-rt, then we want to implicitly compile everything with hidden
+                // visibility as we're going to link this object all over the place but
+                // don't want the symbols to get exported.
+                let visibility = if self.tcx.is_compiler_builtins(LOCAL_CRATE) {
+                    gccjit::Visibility::Hidden
+                } else {
+                    base::visibility_to_gcc(visibility)
+                };
+                global.add_attribute(VarAttribute::Visibility(visibility));
+            }
+            if base::linkage_needs_weak_attribute(linkage) {
+                global.add_attribute(VarAttribute::Weak);
+            }
+        }
 
         #[cfg(feature = "master")]
         self.add_static_aliases(gcc_type, global_name, attrs, &attrs.foreign_item_symbol_aliases);
@@ -172,7 +191,7 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
         attributes::from_fn_attrs(self, fn_decl, instance, Some(fn_abi));
 
         #[cfg(feature = "master")]
-        if linkage == Linkage::WeakAny {
+        if base::linkage_needs_weak_attribute(linkage) {
             fn_decl.add_attribute(FnAttribute::Weak);
         }
 

tests/c/import_linkage.c

@@ -0,0 +1,17 @@
+/* The symbols that `tests/run/import_linkage.rs` imports with an explicit `#[linkage]`.
+ *
+ * Such an import is a pointer whose value is the address of the symbol, so what the Rust side
+ * reads back is `&value_*`, not the pointer stored in it. The distinct values make a mix-up
+ * visible. */
+
+#include <stdint.h>
+
+int32_t external_value = 1;
+int32_t available_externally_value = 2;
+int32_t linkonce_value = 3;
+int32_t linkonce_odr_value = 4;
+int32_t weak_value = 5;
+int32_t weak_odr_value = 6;
+int32_t common_value = 7;
+int32_t extern_weak_value = 8;
+int32_t internal_value = 9;

tests/c/static_linkage.c

@@ -0,0 +1,33 @@
+/* Strong definitions of the statics that `tests/run/static_linkage.rs` also defines, but weakly.
+ * The linker has to keep these and drop the Rust ones; a backend that emits the Rust definitions
+ * as ordinary global symbols fails the link with a duplicate definition instead.
+ *
+ * `internal_static` is the opposite case: the Rust side keeps its own, and the two definitions
+ * coexist because the Rust one is local. */
+
+#include <stdint.h>
+
+int32_t weak_static = 1;
+int32_t weak_odr_static = 2;
+int32_t linkonce_static = 3;
+int32_t linkonce_odr_static = 4;
+int32_t common_static = 5;
+int32_t internal_static = 200;
+
+/* Called from Rust, so that the reads also happen in a translation unit GCC compiled. */
+int32_t c_read_all(void)
+{
+    if (weak_static != 1)
+        return 11;
+    if (weak_odr_static != 2)
+        return 12;
+    if (linkonce_static != 3)
+        return 13;
+    if (linkonce_odr_static != 4)
+        return 14;
+    if (common_static != 5)
+        return 15;
+    if (internal_static != 200)
+        return 16;
+    return 0;
+}

tests/c/weak_function_linkage.c

@@ -0,0 +1,49 @@
+/* Strong definitions of the functions that `tests/run/weak_function_linkage.rs` also defines, but
+ * weakly. The linker has to keep these and drop the Rust ones.
+ *
+ * A backend that emits the Rust definitions as ordinary global symbols does not merely pick the
+ * wrong one: the link fails outright with a duplicate definition. */
+
+#include <stdint.h>
+
+int32_t weak_function(void)
+{
+    return 1;
+}
+
+int32_t weak_odr_function(void)
+{
+    return 2;
+}
+
+int32_t linkonce_function(void)
+{
+    return 3;
+}
+
+int32_t linkonce_odr_function(void)
+{
+    return 4;
+}
+
+int32_t common_function(void)
+{
+    return 5;
+}
+
+/* Called from Rust, so that the calls also go through a caller that GCC compiled: a cg_gcc caller
+ * could inline the weak body it can see instead of calling the symbol. */
+int32_t c_call_all(void)
+{
+    if (weak_function() != 1)
+        return 11;
+    if (weak_odr_function() != 2)
+        return 12;
+    if (linkonce_function() != 3)
+        return 13;
+    if (linkonce_odr_function() != 4)
+        return 14;
+    if (common_function() != 5)
+        return 15;
+    return 0;
+}