Fix linkage

.gitignore

@@ -20,4 +20,5 @@ llvm
 build_system/target
 config.toml
 build
-rustlantis
+rustlantis
+stuff/

Cargo.lock

@@ -56,18 +56,18 @@ dependencies = [
 
 [[package]]
 name = "gccjit"
-version = "6.0.0"
+version = "6.1.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5bb358d2563af5e32af92620915e6b05839ae60645343473735619441f45eb04"
+checksum = "6d85b5754389edaad832ba320709a25086b3081a8c6c0fab2322965e5fb512b3"
 dependencies = [
  "gccjit_sys",
 ]
 
 [[package]]
 name = "gccjit_sys"
-version = "3.1.0"
+version = "3.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2389fb01673e9cc63684d996a58079edccc5de89008274f3be59f1b16ac1f017"
+checksum = "e081669728b490723537f9def7eb674b7c9acd8de0b92ad4f4abf5f5cc75ea4b"
 dependencies = [
  "libc",
 ]

Cargo.toml

@@ -20,7 +20,7 @@ default = ["master"]
 [dependencies]
 object = { version = "0.37.0", default-features = false, features = ["std", "read"] }
 tempfile = "3.20"
-gccjit = { version = "6.0.0", features = ["dlopen"] }
+gccjit = { version = "6.1.0", features = ["dlopen"] }
 #gccjit = { git = "https://github.com/rust-lang/gccjit.rs", branch = "error-dlopen", features = ["dlopen"] }
 
 # Local copy.

libgccjit.version

@@ -1 +1 @@
-201ca90ac810d1c6509c252cc9c87d3ace0661d7
+badf78d09d16e66f4ca07971c51aa6a227558d4f

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,16 @@ pub fn from_fn_attrs<'gcc, 'tcx>(
         } else {
             codegen_fn_attrs.inline
         };
+        // GCC drops `weak` from a function that is also `inline`, leaving the symbol strong, and
+        // the linkage is what has to survive. `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

@@ -1,6 +1,8 @@
 use std::sync::Arc;
 use std::time::Instant;
 
+#[cfg(feature = "master")]
+use gccjit::VarAttribute;
 use gccjit::{CType, FunctionType, GlobalKind};
 use rustc_codegen_ssa::ModuleCodegen;
 use rustc_codegen_ssa::base::maybe_create_entry_wrapper;
@@ -39,32 +41,72 @@ 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
+/// `global_linkage_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 no comdat, so `weak` stands in for the linkonce flavours.
+        Linkage::LinkOnceAny
+        | Linkage::LinkOnceODR
+        | Linkage::WeakAny
+        | Linkage::WeakODR
+        | Linkage::ExternalWeak
+        | Linkage::Common => GlobalKind::Exported,
+    }
+}
+
+/// The attribute a global *definition* needs on top of its [`GlobalKind`] to get this linkage.
+#[cfg(feature = "master")]
+pub fn global_linkage_attribute<'gcc>(linkage: Linkage) -> Option<VarAttribute<'gcc>> {
+    match linkage {
+        Linkage::Common => Some(VarAttribute::Common),
+        _ if linkage_needs_weak_attribute(linkage) => Some(VarAttribute::Weak),
+        _ => None,
     }
 }
 
+/// 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.
+///
+/// `common` is in here for functions only: GCC honours that attribute on a variable, but drops it
+/// on a function, so a common function falls back to weak. Globals go through
+/// `global_linkage_attribute` instead.
+#[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

@@ -13,15 +13,15 @@ use rustc_hir::def_id::LOCAL_CRATE;
 use rustc_log::tracing::trace;
 use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
 use rustc_middle::mir::interpret::{
-    self, ConstAllocation, CtfeProvenance, ErrorHandled, Scalar as InterpScalar, read_target_uint,
+    self, Allocation, ConstAllocation, CtfeProvenance, ErrorHandled, Scalar as InterpScalar,
+    read_target_uint,
 };
 use rustc_middle::mono::MonoItem;
 use rustc_middle::ty::layout::LayoutOf;
 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;
@@ -113,7 +113,12 @@ impl<'gcc, 'tcx> StaticCodegenMethods for CodegenCx<'gcc, 'tcx> {
         // NOTE: Alignment from attributes has already been applied to the allocation.
         set_global_alignment(self, global, alloc.align);
 
-        global.global_set_initializer_rvalue(value);
+        // A common symbol is storage the linker allocates and zero-fills, so giving the definition
+        // an initializer — even an all-zero one — takes it back out of `.comm`. A non-zero one is
+        // kept: the symbol is then an ordinary definition, which is what GCC does with it too.
+        if attrs.linkage != Some(Linkage::Common) || !is_zero_initializer(alloc) {
+            global.global_set_initializer_rvalue(value);
+        }
 
         // As an optimization, all shared statics which do not have interior
         // mutability are placed into read-only memory.
@@ -453,6 +458,17 @@ pub(crate) fn const_alloc_to_gcc_uncached<'gcc>(
     cx.const_struct(&llvals, true)
 }
 
+/// Whether this allocation is all zeroes, and so needs no initializer to be spelled out.
+fn is_zero_initializer(alloc: &Allocation) -> bool {
+    alloc.provenance().ptrs().is_empty()
+        // This `inspect` is okay: it is within the bounds of the allocation, there is no provenance
+        // to misread, and it does not affect interpreter execution.
+        && alloc
+            .inspect_with_uninit_and_ptr_outside_interpreter(0..alloc.size().bytes_usize())
+            .iter()
+            .all(|&byte| byte == 0)
+}
+
 fn codegen_static_initializer<'gcc, 'tcx>(
     cx: &CodegenCx<'gcc, 'tcx>,
     def_id: DefId,
@@ -469,10 +485,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 +502,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.
+        {
+            // Visibility is meaningless on an internal global: GCC ignores the attribute and
+            // warns about it.
+            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 let Some(attribute) = base::global_linkage_attribute(linkage) {
+                global.add_attribute(attribute);
+            }
+        }
 
         #[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,37 @@
+/* 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;
+
+/* `available_externally` promises the real definition lives elsewhere: a backend may read this one
+ * or emit an equivalent copy of the Rust initializer, so the two have to hold the same value. */
+int32_t available_externally_static = 7;
+
+/* 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;
+}