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

Fix #64153 #65435

Merged
merged 3 commits into from
Oct 29, 2019
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
6 changes: 4 additions & 2 deletions src/librustc_codegen_llvm/back/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use std::str;

use crate::llvm::archive_ro::{ArchiveRO, Child};
use crate::llvm::{self, ArchiveKind};
use rustc_codegen_ssa::{METADATA_FILENAME, RLIB_BYTECODE_EXTENSION};
use rustc_codegen_ssa::{
METADATA_FILENAME, RLIB_BYTECODE_EXTENSION, looks_like_rust_object_file
};
use rustc_codegen_ssa::back::archive::{ArchiveBuilder, find_library};
use rustc::session::Session;
use syntax::symbol::Symbol;
Expand Down Expand Up @@ -141,7 +143,7 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> {
}

// Don't include Rust objects if LTO is enabled
if lto && fname.starts_with(&obj_start) && fname.ends_with(".o") {
if lto && looks_like_rust_object_file(fname) {
return true
}

Expand Down
21 changes: 4 additions & 17 deletions src/librustc_codegen_ssa/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use rustc::session::{Session, filesearch};
use rustc::session::config::{
self, RUST_CGU_EXT, DebugInfo, OutputFilenames, OutputType, PrintRequest, Sanitizer
self, DebugInfo, OutputFilenames, OutputType, PrintRequest, Sanitizer
};
use rustc::session::search_paths::PathKind;
use rustc::middle::dependency_format::Linkage;
Expand All @@ -15,7 +15,8 @@ use rustc_fs_util::fix_windows_verbatim_for_gcc;
use rustc_target::spec::{PanicStrategy, RelroLevel, LinkerFlavor};
use syntax::symbol::Symbol;

use crate::{METADATA_FILENAME, RLIB_BYTECODE_EXTENSION, CrateInfo, CodegenResults};
use crate::{METADATA_FILENAME, RLIB_BYTECODE_EXTENSION, CrateInfo,
looks_like_rust_object_file, CodegenResults};
use super::archive::ArchiveBuilder;
use super::command::Command;
use super::linker::Linker;
Expand Down Expand Up @@ -1549,23 +1550,9 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>(
let canonical = f.replace("-", "_");
let canonical_name = name.replace("-", "_");

// Look for `.rcgu.o` at the end of the filename to conclude
// that this is a Rust-related object file.
fn looks_like_rust(s: &str) -> bool {
let path = Path::new(s);
let ext = path.extension().and_then(|s| s.to_str());
if ext != Some(OutputType::Object.extension()) {
return false
}
let ext2 = path.file_stem()
.and_then(|s| Path::new(s).extension())
.and_then(|s| s.to_str());
ext2 == Some(RUST_CGU_EXT)
}

let is_rust_object =
canonical.starts_with(&canonical_name) &&
looks_like_rust(&f);
looks_like_rust_object_file(&f);

// If we've been requested to skip all native object files
// (those not generated by the rust compiler) then we can skip
Expand Down
24 changes: 22 additions & 2 deletions src/librustc_codegen_ssa/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
#[macro_use] extern crate rustc;
#[macro_use] extern crate syntax;

use std::path::PathBuf;
use std::path::{Path, PathBuf};
use rustc::dep_graph::WorkProduct;
use rustc::session::config::{OutputFilenames, OutputType};
use rustc::session::config::{OutputFilenames, OutputType, RUST_CGU_EXT};
use rustc::middle::lang_items::LangItem;
use rustc::hir::def_id::CrateNum;
use rustc::ty::query::Providers;
Expand Down Expand Up @@ -62,6 +62,7 @@ pub struct ModuleCodegen<M> {
pub const METADATA_FILENAME: &str = "rust.metadata.bin";
pub const RLIB_BYTECODE_EXTENSION: &str = "bc.z";


impl<M> ModuleCodegen<M> {
pub fn into_compiled_module(self,
emit_obj: bool,
Expand Down Expand Up @@ -166,3 +167,22 @@ pub fn provide_extern(providers: &mut Providers<'_>) {
crate::back::symbol_export::provide_extern(providers);
crate::base::provide_both(providers);
}

/// Checks if the given filename ends with the `.rcgu.o` extension that `rustc`
/// uses for the object files it generates.
pub fn looks_like_rust_object_file(filename: &str) -> bool {
let path = Path::new(filename);
let ext = path.extension().and_then(|s| s.to_str());
if ext != Some(OutputType::Object.extension()) {
// The file name does not end with ".o", so it can't be an object file.
return false
}

// Strip the ".o" at the end
let ext2 = path.file_stem()
.and_then(|s| Path::new(s).extension())
.and_then(|s| s.to_str());

// Check if the "inner" extension
ext2 == Some(RUST_CGU_EXT)
}
26 changes: 26 additions & 0 deletions src/test/run-make-fulldeps/issue-64153/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
-include ../tools.mk

# `llvm-objdump`'s output looks different on windows than on other platforms.
# It should be enough to check on Unix platforms, so:
# ignore-windows

# Staticlibs don't include Rust object files from upstream crates if the same
# code was already pulled into the lib via LTO. However, the bug described in
# https://github.com/rust-lang/rust/issues/64153 lead to this exclusion not
# working properly if the upstream crate was compiled with an explicit filename
# (via `-o`).
#
# This test makes sure that functions defined in the upstream crates do not
# appear twice in the final staticlib when listing all the symbols from it.

all:
$(RUSTC) --crate-type rlib upstream.rs -o $(TMPDIR)/libupstream.rlib -Ccodegen-units=1
$(RUSTC) --crate-type staticlib downstream.rs -Clto -Ccodegen-units=1 -o $(TMPDIR)/libdownstream.a
# Dump all the symbols from the staticlib into `syms`
"$(LLVM_BIN_DIR)"/llvm-objdump -t $(TMPDIR)/libdownstream.a > $(TMPDIR)/syms
# Count the global instances of `issue64153_test_function`. There'll be 2
# if the `upstream` object file got erronously included twice.
# The line we are testing for with the regex looks something like:
# 0000000000000000 g F .text.issue64153_test_function 00000023 issue64153_test_function
grep -c -e "[[:space:]]g[[:space:]]*F[[:space:]].*issue64153_test_function" $(TMPDIR)/syms > $(TMPDIR)/count
[ "$$(cat $(TMPDIR)/count)" -eq "1" ]
6 changes: 6 additions & 0 deletions src/test/run-make-fulldeps/issue-64153/downstream.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extern crate upstream;

#[no_mangle]
pub extern "C" fn foo() {
print!("1 + 1 = {}", upstream::issue64153_test_function(1));
}
6 changes: 6 additions & 0 deletions src/test/run-make-fulldeps/issue-64153/upstream.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Make this function extern "C", public, and no-mangle, so that it gets
// exported from the downstream staticlib.
#[no_mangle]
pub extern "C" fn issue64153_test_function(x: u32) -> u32 {
x + 1
}