Auto merge of #54451 - alexcrichton:no-mangle-extern-linkage, r=micha…

…elwoerister

rustc: Allow `#[no_mangle]` anywhere in a crate

This commit updates the compiler to allow the `#[no_mangle]` (and
`#[export_name]` attributes) to be located anywhere within a crate.
These attributes are unconditionally processed, causing the compiler to
always generate an exported symbol with the appropriate name.

After some discussion on #54135 it was found that not a great reason
this hasn't been allowed already, and it seems to match the behavior
that many expect! Previously the compiler would only export a
`#[no_mangle]` symbol if it were *publicly reachable*, meaning that it
itself is `pub` and it's otherwise publicly reachable from the root of
the crate. This new definition is that `#[no_mangle]` *is always
reachable*, no matter where it is in a crate or whether it has `pub` or
not.

This should make it much easier to declare an exported symbol with a
known and unique name, even when it's an internal implementation detail
of the crate itself. Note that these symbols will persist beyond LTO as
well, always making their way to the linker.

Along the way this commit removes the `private_no_mangle_functions` lint
(also for statics) as there's no longer any need to lint these
situations. Furthermore a good number of tests were updated now that
symbol visibility has been changed.

Closes #54135

src/librustc/hir/mod.rs

@@ -2467,9 +2467,22 @@ impl CodegenFnAttrs {
         }
     }
 
-    /// True if `#[no_mangle]` or `#[export_name(...)]` is present.
+    /// True if it looks like this symbol needs to be exported, for example:
+    ///
+    /// * `#[no_mangle]` is present
+    /// * `#[export_name(...)]` is present
+    /// * `#[linkage]` is present
     pub fn contains_extern_indicator(&self) -> bool {
-        self.flags.contains(CodegenFnAttrFlags::NO_MANGLE) || self.export_name.is_some()
+        self.flags.contains(CodegenFnAttrFlags::NO_MANGLE) ||
+            self.export_name.is_some() ||
+            match self.linkage {
+                // these are private, make sure we don't try to consider
+                // them external
+                None |
+                Some(Linkage::Internal) |
+                Some(Linkage::Private) => false,
+                Some(_) => true,
+            }
     }
 }
 

src/librustc/middle/dead.rs

@@ -18,6 +18,7 @@ use hir::intravisit::{self, Visitor, NestedVisitorMap};
 use hir::itemlikevisit::ItemLikeVisitor;
 
 use hir::def::Def;
+use hir::CodegenFnAttrFlags;
 use hir::def_id::{DefId, LOCAL_CRATE};
 use lint;
 use middle::privacy;
@@ -302,14 +303,18 @@ fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_, '_, '_>,
         return true;
     }
 
-    // #[used] also keeps the item alive forcefully,
-    // e.g. for placing it in a specific section.
-    if attr::contains_name(attrs, "used") {
+    // Don't lint about global allocators
+    if attr::contains_name(attrs, "global_allocator") {
         return true;
     }
 
-    // Don't lint about global allocators
-    if attr::contains_name(attrs, "global_allocator") {
+    let def_id = tcx.hir.local_def_id(id);
+    let cg_attrs = tcx.codegen_fn_attrs(def_id);
+
+    // #[used], #[no_mangle], #[export_name], etc also keeps the item alive
+    // forcefully, e.g. for placing it in a specific section.
+    if cg_attrs.contains_extern_indicator() ||
+        cg_attrs.flags.contains(CodegenFnAttrFlags::USED) {
         return true;
     }
 

src/librustc/middle/reachable.rs

@@ -352,7 +352,7 @@ impl<'a, 'tcx: 'a> ItemLikeVisitor<'tcx> for CollectPrivateImplItemsVisitor<'a,
         // which are currently akin to allocator symbols.
         let def_id = self.tcx.hir.local_def_id(item.id);
         let codegen_attrs = self.tcx.codegen_fn_attrs(def_id);
-        if codegen_attrs.linkage.is_some() ||
+        if codegen_attrs.contains_extern_indicator() ||
             codegen_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) {
             self.worklist.push(item.id);
         }

src/librustc_lint/builtin.rs

@@ -1163,18 +1163,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PluginAsLibrary {
     }
 }
 
-declare_lint! {
-    PRIVATE_NO_MANGLE_FNS,
-    Warn,
-    "functions marked #[no_mangle] should be exported"
-}
-
-declare_lint! {
-    PRIVATE_NO_MANGLE_STATICS,
-    Warn,
-    "statics marked #[no_mangle] should be exported"
-}
-
 declare_lint! {
     NO_MANGLE_CONST_ITEMS,
     Deny,
@@ -1192,52 +1180,16 @@ pub struct InvalidNoMangleItems;
 
 impl LintPass for InvalidNoMangleItems {
     fn get_lints(&self) -> LintArray {
-        lint_array!(PRIVATE_NO_MANGLE_FNS,
-                    PRIVATE_NO_MANGLE_STATICS,
-                    NO_MANGLE_CONST_ITEMS,
+        lint_array!(NO_MANGLE_CONST_ITEMS,
                     NO_MANGLE_GENERIC_ITEMS)
     }
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
     fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
-        let suggest_export = |vis: &hir::Visibility, err: &mut DiagnosticBuilder| {
-            let suggestion = match vis.node {
-                hir::VisibilityKind::Inherited => {
-                    // inherited visibility is empty span at item start; need an extra space
-                    Some("pub ".to_owned())
-                },
-                hir::VisibilityKind::Restricted { .. } |
-                hir::VisibilityKind::Crate(_) => {
-                    Some("pub".to_owned())
-                },
-                hir::VisibilityKind::Public => {
-                    err.help("try exporting the item with a `pub use` statement");
-                    None
-                }
-            };
-            if let Some(replacement) = suggestion {
-                err.span_suggestion_with_applicability(
-                    vis.span,
-                    "try making it public",
-                    replacement,
-                    Applicability::MachineApplicable
-                );
-            }
-        };
-
         match it.node {
             hir::ItemKind::Fn(.., ref generics, _) => {
                 if let Some(no_mangle_attr) = attr::find_by_name(&it.attrs, "no_mangle") {
-                    if attr::contains_name(&it.attrs, "linkage") {
-                        return;
-                    }
-                    if !cx.access_levels.is_reachable(it.id) {
-                        let msg = "function is marked #[no_mangle], but not exported";
-                        let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_FNS, it.span, msg);
-                        suggest_export(&it.vis, &mut err);
-                        err.emit();
-                    }
                     for param in &generics.params {
                         match param.kind {
                             GenericParamKind::Lifetime { .. } => {}
@@ -1261,15 +1213,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidNoMangleItems {
                     }
                 }
             }
-            hir::ItemKind::Static(..) => {
-                if attr::contains_name(&it.attrs, "no_mangle") &&
-                    !cx.access_levels.is_reachable(it.id) {
-                        let msg = "static is marked #[no_mangle], but not exported";
-                        let mut err = cx.struct_span_lint(PRIVATE_NO_MANGLE_STATICS, it.span, msg);
-                        suggest_export(&it.vis, &mut err);
-                        err.emit();
-                    }
-            }
             hir::ItemKind::Const(..) => {
                 if attr::contains_name(&it.attrs, "no_mangle") {
                     // Const items do not refer to a particular location in memory, and therefore
@@ -1785,8 +1728,6 @@ impl LintPass for SoftLints {
             UNUSED_DOC_COMMENTS,
             UNCONDITIONAL_RECURSION,
             PLUGIN_AS_LIBRARY,
-            PRIVATE_NO_MANGLE_FNS,
-            PRIVATE_NO_MANGLE_STATICS,
             NO_MANGLE_CONST_ITEMS,
             NO_MANGLE_GENERIC_ITEMS,
             MUTABLE_TRANSMUTES,

src/librustc_lint/lib.rs

@@ -387,4 +387,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
         "converted into hard error, see https://github.com/rust-lang/rust/issues/48950");
     store.register_removed("resolve_trait_on_defaulted_unit",
         "converted into hard error, see https://github.com/rust-lang/rust/issues/48950");
+    store.register_removed("private_no_mangle_fns",
+        "no longer an warning, #[no_mangle] functions always exported");
+    store.register_removed("private_no_mangle_statics",
+        "no longer an warning, #[no_mangle] statics always exported");
 }

src/libstd/panicking.rs

@@ -515,11 +515,11 @@ pub fn update_count_then_panic(msg: Box<dyn Any + Send>) -> ! {
     rust_panic(&mut RewrapBox(msg))
 }
 
-/// A private no-mangle function on which to slap yer breakpoints.
+/// An unmangled function (through `rustc_std_internal_symbol`) on which to slap
+/// yer breakpoints.
 #[inline(never)]
-#[no_mangle]
-#[allow(private_no_mangle_fns)] // yes we get it, but we like breakpoints
-pub fn rust_panic(mut msg: &mut dyn BoxMeUp) -> ! {
+#[cfg_attr(not(test), rustc_std_internal_symbol)]
+fn rust_panic(mut msg: &mut dyn BoxMeUp) -> ! {
     let code = unsafe {
         let obj = &mut msg as *mut &mut dyn BoxMeUp;
         __rust_start_panic(obj as usize)

src/test/codegen/export-no-mangle.rs

@@ -0,0 +1,29 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![crate_type = "lib"]
+
+mod private {
+    // CHECK: @FOO =
+    #[no_mangle]
+    pub static FOO: u32 = 3;
+
+    // CHECK: @BAR =
+    #[export_name = "BAR"]
+    static BAR: u32 = 3;
+
+    // CHECK: void @foo()
+    #[no_mangle]
+    pub extern fn foo() {}
+
+    // CHECK: void @bar()
+    #[export_name = "bar"]
+    extern fn bar() {}
+}

src/test/codegen/external-no-mangle-fns.rs

@@ -0,0 +1,63 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -O
+// `#[no_mangle]`d functions always have external linkage, i.e. no `internal` in their `define`s
+
+#![crate_type = "lib"]
+#![no_std]
+
+// CHECK: define void @a()
+#[no_mangle]
+fn a() {}
+
+// CHECK: define void @b()
+#[no_mangle]
+pub fn b() {}
+
+mod private {
+    // CHECK: define void @c()
+    #[no_mangle]
+    fn c() {}
+
+    // CHECK: define void @d()
+    #[no_mangle]
+    pub fn d() {}
+}
+
+const HIDDEN: () = {
+    // CHECK: define void @e()
+    #[no_mangle]
+    fn e() {}
+
+    // CHECK: define void @f()
+    #[no_mangle]
+    pub fn f() {}
+};
+
+// The surrounding item should not accidentally become external
+// CHECK: define internal {{.*}} void @_ZN22external_no_mangle_fns1x
+#[inline(never)]
+fn x() {
+    // CHECK: define void @g()
+    #[no_mangle]
+    fn g() {
+        x();
+    }
+
+    // CHECK: define void @h()
+    #[no_mangle]
+    pub fn h() {}
+
+    // side effect to keep `x` around
+    unsafe {
+        core::ptr::read_volatile(&42);
+    }
+}

src/test/codegen/external-no-mangle-statics.rs

@@ -0,0 +1,88 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -O
+// `#[no_mangle]`d static variables always have external linkage, i.e. no `internal` in their
+// definitions
+
+#![crate_type = "lib"]
+#![no_std]
+
+// CHECK: @A = local_unnamed_addr constant
+#[no_mangle]
+static A: u8 = 0;
+
+// CHECK: @B = local_unnamed_addr global
+#[no_mangle]
+static mut B: u8 = 0;
+
+// CHECK: @C = local_unnamed_addr constant
+#[no_mangle]
+pub static C: u8 = 0;
+
+// CHECK: @D = local_unnamed_addr global
+#[no_mangle]
+pub static mut D: u8 = 0;
+
+mod private {
+    // CHECK: @E = local_unnamed_addr constant
+    #[no_mangle]
+    static E: u8 = 0;
+
+    // CHECK: @F = local_unnamed_addr global
+    #[no_mangle]
+    static mut F: u8 = 0;
+
+    // CHECK: @G = local_unnamed_addr constant
+    #[no_mangle]
+    pub static G: u8 = 0;
+
+    // CHECK: @H = local_unnamed_addr global
+    #[no_mangle]
+    pub static mut H: u8 = 0;
+}
+
+const HIDDEN: () = {
+    // CHECK: @I = local_unnamed_addr constant
+    #[no_mangle]
+    static I: u8 = 0;
+
+    // CHECK: @J = local_unnamed_addr global
+    #[no_mangle]
+    static mut J: u8 = 0;
+
+    // CHECK: @K = local_unnamed_addr constant
+    #[no_mangle]
+    pub static K: u8 = 0;
+
+    // CHECK: @L = local_unnamed_addr global
+    #[no_mangle]
+    pub static mut L: u8 = 0;
+};
+
+// The surrounding item should not accidentally become external
+fn x() {
+    // CHECK: @M = local_unnamed_addr constant
+    #[no_mangle]
+    static M: fn() = x;
+
+    // CHECK: @N = local_unnamed_addr global
+    #[no_mangle]
+    static mut N: u8 = 0;
+
+    // CHECK: @O = local_unnamed_addr constant
+    #[no_mangle]
+    pub static O: u8 = 0;
+
+    // CHECK: @P = local_unnamed_addr global
+    #[no_mangle]
+    pub static mut P: u8 = 0;
+}
+// CHECK: define internal void @_ZN26external_no_mangle_statics1x{{.*$}}

src/test/codegen/issue-44056-macos-tls-align.rs

@@ -17,15 +17,13 @@
 #![crate_type = "rlib"]
 #![feature(thread_local)]
 
-// CHECK: @STATIC_VAR_1 = internal thread_local unnamed_addr global <{ [32 x i8] }> zeroinitializer, section "__DATA,__thread_bss", align 4
+// CHECK: @STATIC_VAR_1 = thread_local local_unnamed_addr global <{ [32 x i8] }> zeroinitializer, section "__DATA,__thread_bss", align 4
 #[no_mangle]
-#[allow(private_no_mangle_statics)]
 #[thread_local]
 static mut STATIC_VAR_1: [u32; 8] = [0; 8];
 
-// CHECK: @STATIC_VAR_2 = internal thread_local unnamed_addr global <{ [32 x i8] }> <{{[^>]*}}>, section "__DATA,__thread_data", align 4
+// CHECK: @STATIC_VAR_2 = thread_local local_unnamed_addr global <{ [32 x i8] }> <{{[^>]*}}>, section "__DATA,__thread_data", align 4
 #[no_mangle]
-#[allow(private_no_mangle_statics)]
 #[thread_local]
 static mut STATIC_VAR_2: [u32; 8] = [4; 8];