Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Oct 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2370,9 +2370,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,
}
}
}

Expand Down
15 changes: 10 additions & 5 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -304,14 +305,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;
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/reachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() ||
Copy link
Contributor

@petrochenkov petrochenkov Sep 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look correct, #[linkage] can be used to decrease visibilities as well, in this case we don't need to mark the item and everything it uses as reachable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that #[linkage] is an unstable attribute and has always been sort of wonky with how everything else in the compiler has evolved over time. This is just preserving the original behavior currently.

TBH the #[linkage] attribute is basically buggy enough to not be worth using any more, it needs a larger overhaul than changing the behavior at this location to be usable.

(for example there's no way to control visibility, and it's also not well defined whether the attribute makes it reachable).

Would you be ok leaving these sorts of questions to a later PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.

codegen_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) {
self.worklist.push(item.id);
}
Expand Down
61 changes: 1 addition & 60 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 { .. } => {}
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
8 changes: 4 additions & 4 deletions src/libstd/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
29 changes: 29 additions & 0 deletions src/test/codegen/export-no-mangle.rs
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add test cases for these patterns?

fn internal() {
    #[no_mangle]
    static EXTERNAL: fn() = internal;
}

static FOO: u32 = {
    #[no_mangle]
    static BAR: &u32 = &FOO;
    0
};

const PRIVATE: () = {
    #[no_mangle]
     fn exported() {}
};

I can see myself writing attributes / macros that expand into those patterns.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly!

// 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() {}
}
63 changes: 63 additions & 0 deletions src/test/codegen/external-no-mangle-fns.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
88 changes: 88 additions & 0 deletions src/test/codegen/external-no-mangle-statics.rs
Original file line number Diff line number Diff line change
@@ -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{{.*$}}
6 changes: 2 additions & 4 deletions src/test/codegen/issue-44056-macos-tls-align.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down
Loading