Skip to content

Don't mark reachable extern fns as internal #10539

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

Merged
merged 1 commit into from
Nov 18, 2013
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
183 changes: 106 additions & 77 deletions src/librustc/middle/reachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,104 +260,133 @@ impl ReachableContext {
continue
}
scanned.insert(search_item);
self.reachable_symbols.insert(search_item);

// Find the AST block corresponding to the item and visit it,
// marking all path expressions that resolve to something
// interesting.
match self.tcx.items.find(&search_item) {
Some(&ast_map::node_item(item, _)) => {
Some(item) => self.propagate_node(item, search_item,
&mut visitor),
None if search_item == ast::CRATE_NODE_ID => {}
None => {
self.tcx.sess.bug(format!("found unmapped ID in worklist: \
{}",
search_item))
}
}
}
}

fn propagate_node(&self, node: &ast_map::ast_node,
search_item: ast::NodeId,
visitor: &mut MarkSymbolVisitor) {
if !*self.tcx.sess.building_library {
// If we are building an executable, then there's no need to flag
// anything as external except for `extern fn` types. These
// functions may still participate in some form of native interface,
// but all other rust-only interfaces can be private (they will not
// participate in linkage after this product is produced)
match *node {
ast_map::node_item(item, _) => {
match item.node {
ast::item_fn(_, _, _, _, ref search_block) => {
if item_might_be_inlined(item) {
visit::walk_block(&mut visitor, search_block, ())
}
ast::item_fn(_, ast::extern_fn, _, _, _) => {
self.reachable_symbols.insert(search_item);
}
_ => {}
}
}
_ => {}
}
} else {
// If we are building a library, then reachable symbols will
// continue to participate in linkage after this product is
// produced. In this case, we traverse the ast node, recursing on
// all reachable nodes from this one.
self.reachable_symbols.insert(search_item);
}

// Implementations of exported structs/enums need to get
// added to the worklist (as all their methods should be
// accessible)
ast::item_struct(*) | ast::item_enum(*) => {
let def = local_def(item.id);
let impls = match self.tcx.inherent_impls.find(&def) {
Some(&impls) => impls,
None => continue
};
for imp in impls.iter() {
if is_local(imp.did) {
self.worklist.push(imp.did.node);
}
}
match *node {
ast_map::node_item(item, _) => {
match item.node {
ast::item_fn(_, _, _, _, ref search_block) => {
if item_might_be_inlined(item) {
visit::walk_block(visitor, search_block, ())
}
}

// Propagate through this impl
ast::item_impl(_, _, _, ref methods) => {
for method in methods.iter() {
self.worklist.push(method.id);
// Implementations of exported structs/enums need to get
// added to the worklist (as all their methods should be
// accessible)
ast::item_struct(*) | ast::item_enum(*) => {
let def = local_def(item.id);
let impls = match self.tcx.inherent_impls.find(&def) {
Some(&impls) => impls,
None => return
};
for imp in impls.iter() {
if is_local(imp.did) {
self.worklist.push(imp.did.node);
}
}
}

// Propagate through this impl
ast::item_impl(_, _, _, ref methods) => {
for method in methods.iter() {
self.worklist.push(method.id);
}
}

// Default methods of exported traits need to all be
// accessible.
ast::item_trait(_, _, ref methods) => {
for method in methods.iter() {
match *method {
ast::required(*) => {}
ast::provided(ref method) => {
self.worklist.push(method.id);
}
// Default methods of exported traits need to all be
// accessible.
ast::item_trait(_, _, ref methods) => {
for method in methods.iter() {
match *method {
ast::required(*) => {}
ast::provided(ref method) => {
self.worklist.push(method.id);
}
}
}
}

// These are normal, nothing reachable about these
// inherently and their children are already in the
// worklist
ast::item_static(*) | ast::item_ty(*) |
ast::item_mod(*) | ast::item_foreign_mod(*) => {}
// These are normal, nothing reachable about these
// inherently and their children are already in the
// worklist
ast::item_static(*) | ast::item_ty(*) |
ast::item_mod(*) | ast::item_foreign_mod(*) => {}

_ => {
self.tcx.sess.span_bug(item.span,
"found non-function item \
in worklist?!")
}
_ => {
self.tcx.sess.span_bug(item.span,
"found non-function item \
in worklist?!")
}
}
Some(&ast_map::node_trait_method(trait_method, _, _)) => {
match *trait_method {
ast::required(*) => {
// Keep going, nothing to get exported
}
ast::provided(ref method) => {
visit::walk_block(&mut visitor, &method.body, ())
}
}
ast_map::node_trait_method(trait_method, _, _) => {
match *trait_method {
ast::required(*) => {
// Keep going, nothing to get exported
}
}
Some(&ast_map::node_method(method, did, _)) => {
if method_might_be_inlined(self.tcx, method, did) {
visit::walk_block(&mut visitor, &method.body, ())
ast::provided(ref method) => {
visit::walk_block(visitor, &method.body, ())
}
}
// Nothing to recurse on for these
Some(&ast_map::node_foreign_item(*)) |
Some(&ast_map::node_variant(*)) |
Some(&ast_map::node_struct_ctor(*)) => {}
Some(_) => {
let ident_interner = token::get_ident_interner();
let desc = ast_map::node_id_to_str(self.tcx.items,
search_item,
ident_interner);
self.tcx.sess.bug(format!("found unexpected thingy in \
worklist: {}",
desc))
}
None if search_item == ast::CRATE_NODE_ID => {}
None => {
self.tcx.sess.bug(format!("found unmapped ID in worklist: \
{}",
search_item))
}
ast_map::node_method(method, did, _) => {
if method_might_be_inlined(self.tcx, method, did) {
visit::walk_block(visitor, &method.body, ())
}
}
// Nothing to recurse on for these
ast_map::node_foreign_item(*) |
ast_map::node_variant(*) |
ast_map::node_struct_ctor(*) => {}
_ => {
let ident_interner = token::get_ident_interner();
let desc = ast_map::node_id_to_str(self.tcx.items,
search_item,
ident_interner);
self.tcx.sess.bug(format!("found unexpected thingy in \
worklist: {}",
desc))
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2294,7 +2294,7 @@ fn finish_register_fn(ccx: @mut CrateContext, sp: Span, sym: ~str, node_id: ast:
llfn: ValueRef) {
ccx.item_symbols.insert(node_id, sym);

if !*ccx.sess.building_library {
if !ccx.reachable.contains(&node_id) {
lib::llvm::SetLinkage(llfn, lib::llvm::InternalLinkage);
}

Expand Down Expand Up @@ -2504,7 +2504,7 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
llvm::LLVMAddGlobal(ccx.llmod, llty, buf)
};

if !*ccx.sess.building_library {
if !ccx.reachable.contains(&id) {
lib::llvm::SetLinkage(g, lib::llvm::InternalLinkage);
}

Expand Down
39 changes: 39 additions & 0 deletions src/test/run-pass/extern-fn-reachable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright 2013 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.

// xfail-fast
// xfail-linux apparently dlsym doesn't work on program symbols?
// xfail-android apparently dlsym doesn't work on program symbols?
// xfail-freebsd apparently dlsym doesn't work on program symbols?

use std::unstable::dynamic_lib::DynamicLibrary;

#[no_mangle] pub extern "C" fn fun1() {}
#[no_mangle] extern "C" fn fun2() {}

mod foo {
#[no_mangle] pub extern "C" fn fun3() {}
}
pub mod bar {
#[no_mangle] pub extern "C" fn fun4() {}
}

#[no_mangle] pub fn fun5() {}

fn main() {
unsafe {
let a = DynamicLibrary::open(None).unwrap();
assert!(a.symbol::<int>("fun1").is_ok());
Copy link
Member

Choose a reason for hiding this comment

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

You could call these external1, ..., internal1, ..., rather than relying on this assertion chain to describe the intended behaviour.

Copy link
Member

Choose a reason for hiding this comment

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

(Relying on just this assertion chain.)

assert!(a.symbol::<int>("fun2").is_err());
assert!(a.symbol::<int>("fun3").is_err());
assert!(a.symbol::<int>("fun4").is_ok());
assert!(a.symbol::<int>("fun5").is_err());
}
}