Merge pull request #1074 from bjorn3/fix_weak_linkage

Fix weak linkage

example/mini_core_hello_world.rs

@@ -241,19 +241,22 @@ fn main() {
 
     assert_eq!(((|()| 42u8) as fn(()) -> u8)(()), 42);
 
-    extern {
+    #[cfg(not(jit))]
-        #[linkage = "extern_weak"]
-        static ABC: *const u8;
-    }
-
     {
         extern {
             #[linkage = "extern_weak"]
             static ABC: *const u8;
         }
-    }
 
-    unsafe { assert_eq!(ABC as usize, 0); }
+        {
+            extern {
+                #[linkage = "extern_weak"]
+                static ABC: *const u8;
+            }
+        }
+
+        unsafe { assert_eq!(ABC as usize, 0); }
+    }
 
     &mut (|| Some(0 as *const ())) as &mut dyn FnMut() -> Option<*const ()>;
 

patches/0025-Remove-usage-of-extern_weak-in-thread_local_dtor.patch

@@ -1,45 +0,0 @@
-From 7496bb94a2fb2bc34629c9781c3bc3a470becaee Mon Sep 17 00:00:00 2001
-From: bjorn3 <bjorn3@users.noreply.github.com>
-Date: Fri, 10 Apr 2020 13:47:42 +0200
-Subject: [PATCH] Remove usage of extern_weak in thread_local_dtor
-
----
- library/std/src/sys/unix/thread_local_dtor.rs | 20 --------------------
- 1 files changed, 0 insertions(+), 20 deletions(-)
-
-diff --git a/library/std/src/sys/unix/thread_local_dtor.rs b/library/std/src/sys/unix/thread_local_dtor.rs
-index 8730b4d..8d6e3d5 100644
---- a/library/std/src/sys/unix/thread_local_dtor.rs
-+++ b/library/std/src/sys/unix/thread_local_dtor.rs
-@@ -15,28 +15,8 @@
-     target_os = "emscripten"
- ))]
- pub unsafe fn register_dtor(t: *mut u8, dtor: unsafe extern "C" fn(*mut u8)) {
--    use crate::mem;
-     use crate::sys_common::thread_local_dtor::register_dtor_fallback;
- 
--    extern "C" {
--        #[linkage = "extern_weak"]
--        static __dso_handle: *mut u8;
--        #[linkage = "extern_weak"]
--        static __cxa_thread_atexit_impl: *const libc::c_void;
--    }
--    if !__cxa_thread_atexit_impl.is_null() {
--        type F = unsafe extern "C" fn(
--            dtor: unsafe extern "C" fn(*mut u8),
--            arg: *mut u8,
--            dso_handle: *mut u8,
--        ) -> libc::c_int;
--        mem::transmute::<*const libc::c_void, F>(__cxa_thread_atexit_impl)(
--            dtor,
--            t,
--            &__dso_handle as *const _ as *mut _,
--        );
--        return;
--    }
-     register_dtor_fallback(t, dtor);
- }
- 
--- 
-2.20.1
-

src/constant.rs

@@ -43,8 +43,7 @@ pub(crate) fn codegen_tls_ref<'tcx>(
     def_id: DefId,
     layout: TyAndLayout<'tcx>,
 ) -> CValue<'tcx> {
-    let linkage = crate::linkage::get_static_ref_linkage(fx.tcx, def_id);
+    let data_id = data_id_for_static(fx.tcx, fx.module, def_id, false);
-    let data_id = data_id_for_static(fx.tcx, fx.module, def_id, linkage);
     let local_data_id = fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
     #[cfg(debug_assertions)]
     fx.add_comment(local_data_id, format!("tls {:?}", def_id));
@@ -57,8 +56,7 @@ fn codegen_static_ref<'tcx>(
     def_id: DefId,
     layout: TyAndLayout<'tcx>,
 ) -> CPlace<'tcx> {
-    let linkage = crate::linkage::get_static_ref_linkage(fx.tcx, def_id);
+    let data_id = data_id_for_static(fx.tcx, fx.module, def_id, false);
-    let data_id = data_id_for_static(fx.tcx, fx.module, def_id, linkage);
     let local_data_id = fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
     #[cfg(debug_assertions)]
     fx.add_comment(local_data_id, format!("{:?}", def_id));
@@ -159,8 +157,7 @@ pub(crate) fn trans_const_value<'tcx>(
                         }
                         Some(GlobalAlloc::Static(def_id)) => {
                             assert!(fx.tcx.is_static(def_id));
-                            let linkage = crate::linkage::get_static_ref_linkage(fx.tcx, def_id);
+                            let data_id = data_id_for_static(fx.tcx, fx.module, def_id, false);
-                            let data_id = data_id_for_static(fx.tcx, fx.module, def_id, linkage);
                             let local_data_id = fx.module.declare_data_in_func(data_id, &mut fx.bcx.func);
                             #[cfg(debug_assertions)]
                             fx.add_comment(local_data_id, format!("{:?}", def_id));
@@ -226,8 +223,20 @@ fn data_id_for_static(
     tcx: TyCtxt<'_>,
     module: &mut Module<impl Backend>,
     def_id: DefId,
-    linkage: Linkage,
+    definition: bool,
 ) -> DataId {
+    let rlinkage = tcx.codegen_fn_attrs(def_id).linkage;
+    let linkage = if definition {
+        crate::linkage::get_static_linkage(tcx, def_id)
+    } else {
+        if rlinkage == Some(rustc_middle::mir::mono::Linkage::ExternalWeak)
+        || rlinkage == Some(rustc_middle::mir::mono::Linkage::WeakAny) {
+            Linkage::Preemptible
+        } else {
+            Linkage::Import
+        }
+    };
+
     let instance = Instance::mono(tcx, def_id).polymorphize(tcx);
     let symbol_name = tcx.symbol_name(instance).name;
     let ty = instance.ty(tcx, ParamEnv::reveal_all());
@@ -255,26 +264,39 @@ fn data_id_for_static(
         )
         .unwrap();
 
-    if linkage == Linkage::Preemptible {
+    if rlinkage.is_some() {
-        if let ty::RawPtr(_) = ty.kind {
+        // Comment copied from https://github.com/rust-lang/rust/blob/45060c2a66dfd667f88bd8b94261b28a58d85bd5/src/librustc_codegen_llvm/consts.rs#L141
-        } else {
+        // Declare an internal global `extern_with_linkage_foo` which
-            tcx.sess.span_fatal(
+        // is initialized with the address of `foo`.  If `foo` is
-                tcx.def_span(def_id),
+        // discarded during linking (for example, if `foo` has weak
-                "must have type `*const T` or `*mut T` due to `#[linkage]` attribute",
+        // linkage and there are no definitions), then
-            )
+        // `extern_with_linkage_foo` will instead be initialized to
-        }
+        // zero.
 
+        let ref_name = format!("_rust_extern_with_linkage_{}", symbol_name);
+        let ref_data_id = module
+            .declare_data(
+                &ref_name,
+                Linkage::Local,
+                true,
+                false,
+                Some(align.try_into().unwrap()),
+            )
+            .unwrap();
         let mut data_ctx = DataContext::new();
-        data_ctx.define_zeroinit(pointer_ty(tcx).bytes() as usize);
+        let data = module.declare_data_in_data(data_id, &mut data_ctx);
-        match module.define_data(data_id, &data_ctx) {
+        data_ctx.define(std::iter::repeat(0).take(pointer_ty(tcx).bytes() as usize).collect());
-            // Everytime a weak static is referenced, there will be a zero pointer definition,
+        data_ctx.write_data_addr(0, data, 0);
+        match module.define_data(ref_data_id, &data_ctx) {
+            // Every time the static is referenced there will be another definition of this global,
             // so duplicate definitions are expected and allowed.
             Err(ModuleError::DuplicateDefinition(_)) => {}
             res => res.unwrap(),
         }
+        ref_data_id
+    } else {
+        data_id
     }
-
-    data_id
 }
 
 fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut Module<impl Backend>, cx: &mut ConstantCx) {
@@ -301,16 +323,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut Module<impl Backend>, cx: &mu
                     _ => bug!("static const eval returned {:#?}", const_),
                 };
 
-                let data_id = data_id_for_static(
+                let data_id = data_id_for_static(tcx, module, def_id, true);
-                    tcx,
-                    module,
-                    def_id,
-                    if tcx.is_reachable_non_generic(def_id) {
-                        Linkage::Export
-                    } else {
-                        Linkage::Export // FIXME Set hidden visibility
-                    },
-                );
                 (data_id, alloc, section_name)
             }
         };
@@ -360,12 +373,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut Module<impl Backend>, cx: &mu
                     // Don't push a `TodoItem::Static` here, as it will cause statics used by
                     // multiple crates to be duplicated between them. It isn't necessary anyway,
                     // as it will get pushed by `codegen_static` when necessary.
-                    data_id_for_static(
+                    data_id_for_static(tcx, module, def_id, false)
-                        tcx,
-                        module,
-                        def_id,
-                        crate::linkage::get_static_ref_linkage(tcx, def_id),
-                    )
                 }
             };
 

src/linkage.rs

@@ -11,7 +11,7 @@ pub(crate) fn get_clif_linkage(mono_item: MonoItem<'_>, linkage: RLinkage, visib
     }
 }
 
-pub(crate) fn get_static_ref_linkage(tcx: TyCtxt<'_>, def_id: DefId) -> Linkage {
+pub(crate) fn get_static_linkage(tcx: TyCtxt<'_>, def_id: DefId) -> Linkage {
     let fn_attrs = tcx.codegen_fn_attrs(def_id);
 
     if let Some(linkage) = fn_attrs.linkage {
@@ -22,6 +22,10 @@ pub(crate) fn get_static_ref_linkage(tcx: TyCtxt<'_>, def_id: DefId) -> Linkage
             _ => panic!("{:?}", linkage),
         }
     } else {
-        Linkage::Import
+        if tcx.is_reachable_non_generic(def_id) {
+            Linkage::Export
+        } else {
+            Linkage::Export // FIXME use Linkage::Hidden
+        }
     }
 }