auto merge of #10777 : alexcrichton/rust/issue-10743, r=luqmana

bors · bors · commit e2d192c9f4ba · 2013-12-03T08:21:26.000-08:00
Commit messages have the fun details, the focus of this is closing #10743 though
diff --git a/mk/tests.mk b/mk/tests.mk
@@ -920,11 +920,11 @@ $$(call TEST_OK_FILE,$(1),$(2),$(3),rmake): \
 
 $(3)/test/run-make/%-$(1)-T-$(2)-H-$(3).ok: \
 		$(S)src/test/run-make/%/Makefile \
-		$$(HBIN$(1)_H_$(3))/rustc$$(X_$(3))
+		$$(HSREQ$(1)_H_$(3))
 	@rm -rf $(3)/test/run-make/$$*
 	@mkdir -p $(3)/test/run-make/$$*
 	@echo maketest: $$*
-	$$(Q)python $(S)src/etc/maketest.py $$(dir $$<) \
+	$$(Q)$$(CFG_PYTHON) $(S)src/etc/maketest.py $$(dir $$<) \
 	    $$(HBIN$(1)_H_$(3))/rustc$$(X_$(3)) \
 	    $(3)/test/run-make/$$* \
 	    "$$(CC_$(3)) $$(CFG_GCCISH_CFLAGS_$(3))"
diff --git a/src/librustc/back/link.rs b/src/librustc/back/link.rs
@@ -1127,8 +1127,9 @@ fn link_args(sess: Session,
         }
     }
 
-    add_upstream_rust_crates(&mut args, sess, dylib);
     add_local_native_libraries(&mut args, sess);
+    add_upstream_rust_crates(&mut args, sess, dylib);
+    add_upstream_native_libraries(&mut args, sess);
 
     // # Telling the linker what we're doing
 
@@ -1168,6 +1169,42 @@ fn link_args(sess: Session,
     return args;
 }
 
+// # Native library linking
+//
+// User-supplied library search paths (-L on the cammand line) These are
+// the same paths used to find Rust crates, so some of them may have been
+// added already by the previous crate linking code. This only allows them
+// to be found at compile time so it is still entirely up to outside
+// forces to make sure that library can be found at runtime.
+//
+// Also note that the native libraries linked here are only the ones located
+// in the current crate. Upstream crates with native library dependencies
+// may have their native library pulled in above.
+fn add_local_native_libraries(args: &mut ~[~str], sess: Session) {
+    for path in sess.opts.addl_lib_search_paths.iter() {
+        // FIXME (#9639): This needs to handle non-utf8 paths
+        args.push("-L" + path.as_str().unwrap().to_owned());
+    }
+
+    let rustpath = filesearch::rust_path();
+    for path in rustpath.iter() {
+        // FIXME (#9639): This needs to handle non-utf8 paths
+        args.push("-L" + path.as_str().unwrap().to_owned());
+    }
+
+    for &(ref l, kind) in cstore::get_used_libraries(sess.cstore).iter() {
+        match kind {
+            cstore::NativeUnknown | cstore::NativeStatic => {
+                args.push("-l" + *l);
+            }
+            cstore::NativeFramework => {
+                args.push(~"-framework");
+                args.push(l.to_owned());
+            }
+        }
+    }
+}
+
 // # Rust Crate linking
 //
 // Rust crates are not considered at all when creating an rlib output. All
@@ -1197,30 +1234,11 @@ fn add_upstream_rust_crates(args: &mut ~[~str], sess: Session,
         // all dynamic libaries require dynamic dependencies (see above), so
         // it's satisfactory to include either all static libraries or all
         // dynamic libraries.
-        let crates = cstore::get_used_crates(cstore,
-                                             cstore::RequireStatic);
+        let crates = cstore::get_used_crates(cstore, cstore::RequireStatic);
         if crates.iter().all(|&(_, ref p)| p.is_some()) {
-            for &(cnum, ref path) in crates.iter() {
-                let cratepath = path.clone().unwrap();
-
-                // If we're linking to the static version of the crate, then
-                // we're mostly good to go. The caveat here is that we need to
-                // pull in the static crate's native dependencies.
-                args.push(cratepath.as_str().unwrap().to_owned());
-
-                let libs = csearch::get_native_libraries(sess.cstore, cnum);
-                for &(kind, ref lib) in libs.iter() {
-                    match kind {
-                        cstore::NativeUnknown => args.push("-l" + *lib),
-                        cstore::NativeFramework => {
-                            args.push(~"-framework");
-                            args.push(lib.to_owned());
-                        }
-                        cstore::NativeStatic => {
-                            sess.bug("statics shouldn't be propagated");
-                        }
-                    }
-                }
+            for (_, path) in crates.move_iter() {
+                let path = path.unwrap();
+                args.push(path.as_str().unwrap().to_owned());
             }
             return;
         }
@@ -1253,38 +1271,39 @@ fn add_upstream_rust_crates(args: &mut ~[~str], sess: Session,
     }
 }
 
-// # Native library linking
+// Link in all of our upstream crates' native dependencies. Remember that
+// all of these upstream native depenencies are all non-static
+// dependencies. We've got two cases then:
 //
-// User-supplied library search paths (-L on the cammand line) These are
-// the same paths used to find Rust crates, so some of them may have been
-// added already by the previous crate linking code. This only allows them
-// to be found at compile time so it is still entirely up to outside
-// forces to make sure that library can be found at runtime.
+// 1. The upstream crate is an rlib. In this case we *must* link in the
+//    native dependency because the rlib is just an archive.
 //
-// Also note that the native libraries linked here are only the ones located
-// in the current crate. Upstream crates with native library dependencies
-// may have their native library pulled in above.
-fn add_local_native_libraries(args: &mut ~[~str], sess: Session) {
-    for path in sess.opts.addl_lib_search_paths.iter() {
-        // FIXME (#9639): This needs to handle non-utf8 paths
-        args.push("-L" + path.as_str().unwrap().to_owned());
-    }
-
-    let rustpath = filesearch::rust_path();
-    for path in rustpath.iter() {
-        // FIXME (#9639): This needs to handle non-utf8 paths
-        args.push("-L" + path.as_str().unwrap().to_owned());
-    }
-
-    for &(ref l, kind) in cstore::get_used_libraries(sess.cstore).iter() {
-        match kind {
-            cstore::NativeUnknown | cstore::NativeStatic => {
-                args.push("-l" + *l);
-            }
-            cstore::NativeFramework => {
-                args.push(~"-framework");
-                args.push(l.to_owned());
+// 2. The upstream crate is a dylib. In order to use the dylib, we have to
+//    have the dependency present on the system somewhere. Thus, we don't
+//    gain a whole lot from not linking in the dynamic dependency to this
+//    crate as well.
+//
+// The use case for this is a little subtle. In theory the native
+// dependencies of a crate a purely an implementation detail of the crate
+// itself, but the problem arises with generic and inlined functions. If a
+// generic function calls a native function, then the generic function must
+// be instantiated in the target crate, meaning that the native symbol must
+// also be resolved in the target crate.
+fn add_upstream_native_libraries(args: &mut ~[~str], sess: Session) {
+    let cstore = sess.cstore;
+    cstore::iter_crate_data(cstore, |cnum, _| {
+        let libs = csearch::get_native_libraries(cstore, cnum);
+        for &(kind, ref lib) in libs.iter() {
+            match kind {
+                cstore::NativeUnknown => args.push("-l" + *lib),
+                cstore::NativeFramework => {
+                    args.push(~"-framework");
+                    args.push(lib.to_owned());
+                }
+                cstore::NativeStatic => {
+                    sess.bug("statics shouldn't be propagated");
+                }
             }
         }
-    }
+    });
 }
diff --git a/src/librustc/middle/reachable.rs b/src/librustc/middle/reachable.rs
@@ -128,25 +128,44 @@ impl Visitor<()> for MarkSymbolVisitor {
                 };
 
                 let def_id = def_id_of_def(def);
-                if ReachableContext::
-                    def_id_represents_local_inlined_item(self.tcx, def_id) {
-                        self.worklist.push(def_id.node)
+                if is_local(def_id) {
+                    if ReachableContext::
+                        def_id_represents_local_inlined_item(self.tcx, def_id) {
+                            self.worklist.push(def_id.node)
+                    } else {
+                        match def {
+                            // If this path leads to a static, then we may have
+                            // to do some work to figure out whether the static
+                            // is indeed reachable (address_insignificant
+                            // statics are *never* reachable).
+                            ast::DefStatic(..) => {
+                                self.worklist.push(def_id.node);
+                            }
+
+                            // If this wasn't a static, then this destination is
+                            // surely reachable.
+                            _ => {
+                                self.reachable_symbols.insert(def_id.node);
+                            }
+                        }
                     }
-                self.reachable_symbols.insert(def_id.node);
+                }
             }
             ast::ExprMethodCall(..) => {
                 match self.method_map.find(&expr.id) {
                     Some(&typeck::method_map_entry {
                         origin: typeck::method_static(def_id),
                         ..
                     }) => {
-                        if ReachableContext::
-                            def_id_represents_local_inlined_item(
-                                self.tcx,
-                                def_id) {
-                                self.worklist.push(def_id.node)
-                            }
-                        self.reachable_symbols.insert(def_id.node);
+                        if is_local(def_id) {
+                            if ReachableContext::
+                                def_id_represents_local_inlined_item(
+                                    self.tcx,
+                                    def_id) {
+                                    self.worklist.push(def_id.node)
+                                }
+                            self.reachable_symbols.insert(def_id.node);
+                        }
                     }
                     Some(_) => {}
                     None => {
@@ -310,10 +329,19 @@ impl ReachableContext {
                         }
                     }
 
+                    // Statics with insignificant addresses are not reachable
+                    // because they're inlined specially into all other crates.
+                    ast::item_static(..) => {
+                        if attr::contains_name(item.attrs,
+                                               "address_insignificant") {
+                            self.reachable_symbols.remove(&search_item);
+                        }
+                    }
+
                     // These are normal, nothing reachable about these
                     // inherently and their children are already in the
                     // worklist, as determined by the privacy pass
-                    ast::item_static(..) | ast::item_ty(..) |
+                    ast::item_ty(..) |
                     ast::item_mod(..) | ast::item_foreign_mod(..) |
                     ast::item_impl(..) | ast::item_trait(..) |
                     ast::item_struct(..) | ast::item_enum(..) => {}
diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs
@@ -2521,9 +2521,12 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
                                 // requested
                                 if attr::contains_name(i.attrs,
                                                        "address_insignificant"){
+                                    if ccx.reachable.contains(&id) {
+                                        ccx.sess.span_bug(i.span,
+                                            "insignificant static is \
+                                             reachable");
+                                    }
                                     lib::llvm::SetUnnamedAddr(g, true);
-                                    lib::llvm::SetLinkage(g,
-                                        lib::llvm::InternalLinkage);
 
                                     // This is a curious case where we must make
                                     // all of these statics inlineable. If a
diff --git a/src/test/auxiliary/anon-extern-mod-cross-crate-1.rs b/src/test/auxiliary/anon-extern-mod-cross-crate-1.rs
@@ -14,7 +14,7 @@
 
 use std::libc;
 
+#[link(name = "rustrt")]
 extern {
-    #[link(name = "rustrt")]
     pub fn rust_get_test_int() -> libc::intptr_t;
 }
diff --git a/src/test/run-pass/anon-extern-mod-cross-crate-2.rs b/src/test/run-pass/anon-extern-mod-cross-crate-2.rs
@@ -15,9 +15,6 @@ extern mod anonexternmod;
 
 use anonexternmod::rust_get_test_int;
 
-#[link(name = "rustrt")] // we have explicitly chosen to require this
-extern {}
-
 pub fn main() {
     unsafe {
         rust_get_test_int();
diff --git a/src/test/run-pass/invoke-external-foreign.rs b/src/test/run-pass/invoke-external-foreign.rs
@@ -18,9 +18,6 @@
 
 extern mod foreign_lib;
 
-#[link(name = "rustrt")] // we have explicitly chosen to require this
-extern {}
-
 pub fn main() {
     unsafe {
         let _foo = foreign_lib::rustrt::rust_get_test_int();

Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,7 @@`
`14`	`14`
`15`	`15`	`use std::libc;`
`16`	`16`
	`17`	`+#[link(name = "rustrt")]`
`17`	`18`	`extern {`
`18`		`- #[link(name = "rustrt")]`
`19`	`19`	`pub fn rust_get_test_int() -> libc::intptr_t;`
`20`	`20`	`}`