From 9ab0475d940342a670ad323c151db565e6318e08 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 14 Apr 2015 16:28:50 -0700 Subject: [PATCH] rustc: Handle duplicate names merging archives When linking an archive statically to an rlib, the compiler will extract all contents of the archive and add them all to the rlib being generated. The current method of extraction is to run `ar x`, dumping all files into a temporary directory. Object archives, however, are allowed to have multiple entries with the same file name, so there is no method for them to extract their contents into a directory in a lossless fashion. This commit adds iterator support to the `ArchiveRO` structure which hooks into LLVM's support for reading object archives. This iterator is then used to inspect each object in turn and extract it to a unique location for later assembly. --- src/librustc/metadata/loader.rs | 15 ++-- src/librustc_back/archive.rs | 75 +++++++++++------- src/librustc_back/lib.rs | 1 + src/librustc_llvm/archive_ro.rs | 73 +++++++++++++---- src/librustc_llvm/lib.rs | 16 +++- src/librustc_trans/back/lto.rs | 19 ++--- src/rustllvm/RustWrapper.cpp | 79 +++++++++++++------ .../run-make/archive-duplicate-names/Makefile | 11 +++ .../run-make/archive-duplicate-names/bar.c | 11 +++ .../run-make/archive-duplicate-names/bar.rs | 15 ++++ .../run-make/archive-duplicate-names/foo.c | 11 +++ .../run-make/archive-duplicate-names/foo.rs | 24 ++++++ 12 files changed, 265 insertions(+), 85 deletions(-) create mode 100644 src/test/run-make/archive-duplicate-names/Makefile create mode 100644 src/test/run-make/archive-duplicate-names/bar.c create mode 100644 src/test/run-make/archive-duplicate-names/bar.rs create mode 100644 src/test/run-make/archive-duplicate-names/foo.c create mode 100644 src/test/run-make/archive-duplicate-names/foo.rs diff --git a/src/librustc/metadata/loader.rs b/src/librustc/metadata/loader.rs index 398e4cd33b9..bbb2452ca29 100644 --- a/src/librustc/metadata/loader.rs +++ b/src/librustc/metadata/loader.rs @@ -692,11 +692,16 @@ pub fn note_crate_name(diag: &SpanHandler, name: &str) { impl ArchiveMetadata { fn new(ar: ArchiveRO) -> Option { - let data = match ar.read(METADATA_FILENAME) { - Some(data) => data as *const [u8], - None => { - debug!("didn't find '{}' in the archive", METADATA_FILENAME); - return None; + let data = { + let section = ar.iter().find(|sect| { + sect.name() == Some(METADATA_FILENAME) + }); + match section { + Some(s) => s.data() as *const [u8], + None => { + debug!("didn't find '{}' in the archive", METADATA_FILENAME); + return None; + } } }; diff --git a/src/librustc_back/archive.rs b/src/librustc_back/archive.rs index 9f5751c421e..37d784692fd 100644 --- a/src/librustc_back/archive.rs +++ b/src/librustc_back/archive.rs @@ -11,13 +11,14 @@ //! A helper class for dealing with static archives use std::env; -use std::fs; +use std::fs::{self, File}; use std::io::prelude::*; use std::io; use std::path::{Path, PathBuf}; use std::process::{Command, Output, Stdio}; use std::str; use syntax::diagnostic::Handler as ErrorHandler; +use rustc_llvm::archive_ro::ArchiveRO; use tempdir::TempDir; @@ -282,17 +283,14 @@ impl<'a> ArchiveBuilder<'a> { mut skip: F) -> io::Result<()> where F: FnMut(&str) -> bool, { - let loc = TempDir::new("rsar").unwrap(); - - // First, extract the contents of the archive to a temporary directory. - // We don't unpack directly into `self.work_dir` due to the possibility - // of filename collisions. - let archive = env::current_dir().unwrap().join(archive); - run_ar(self.archive.handler, &self.archive.maybe_ar_prog, - "x", Some(loc.path()), &[&archive]); + let archive = match ArchiveRO::open(archive) { + Some(ar) => ar, + None => return Err(io::Error::new(io::ErrorKind::Other, + "failed to open archive")), + }; // Next, we must rename all of the inputs to "guaranteed unique names". - // We move each file into `self.work_dir` under its new unique name. + // We write each file into `self.work_dir` under its new unique name. // The reason for this renaming is that archives are keyed off the name // of the files, so if two files have the same name they will override // one another in the archive (bad). @@ -300,27 +298,46 @@ impl<'a> ArchiveBuilder<'a> { // We skip any files explicitly desired for skipping, and we also skip // all SYMDEF files as these are just magical placeholders which get // re-created when we make a new archive anyway. - let files = try!(fs::read_dir(loc.path())); - for file in files { - let file = try!(file).path(); - let filename = file.file_name().unwrap().to_str().unwrap(); - if skip(filename) { continue } - if filename.contains(".SYMDEF") { continue } - - let filename = format!("r-{}-{}", name, filename); - // LLDB (as mentioned in back::link) crashes on filenames of exactly - // 16 bytes in length. If we're including an object file with - // exactly 16-bytes of characters, give it some prefix so that it's - // not 16 bytes. - let filename = if filename.len() == 16 { - format!("lldb-fix-{}", filename) - } else { - filename + for file in archive.iter() { + let filename = match file.name() { + Some(s) => s, + None => continue, }; - let new_filename = self.work_dir.path().join(&filename[..]); - try!(fs::rename(&file, &new_filename)); - self.members.push(PathBuf::from(filename)); + if filename.contains(".SYMDEF") { continue } + if skip(filename) { continue } + + // An archive can contain files of the same name multiple times, so + // we need to be sure to not have them overwrite one another when we + // extract them. Consequently we need to find a truly unique file + // name for us! + let mut new_filename = String::new(); + for n in 0.. { + let n = if n == 0 {String::new()} else {format!("-{}", n)}; + new_filename = format!("r{}-{}-{}", n, name, filename); + + // LLDB (as mentioned in back::link) crashes on filenames of + // exactly + // 16 bytes in length. If we're including an object file with + // exactly 16-bytes of characters, give it some prefix so + // that it's not 16 bytes. + new_filename = if new_filename.len() == 16 { + format!("lldb-fix-{}", new_filename) + } else { + new_filename + }; + + let present = self.members.iter().filter_map(|p| { + p.file_name().and_then(|f| f.to_str()) + }).any(|s| s == new_filename); + if !present { + break + } + } + let dst = self.work_dir.path().join(&new_filename); + try!(try!(File::create(&dst)).write_all(file.data())); + self.members.push(PathBuf::from(new_filename)); } + Ok(()) } } diff --git a/src/librustc_back/lib.rs b/src/librustc_back/lib.rs index 3c54d6631f8..22dea4757ed 100644 --- a/src/librustc_back/lib.rs +++ b/src/librustc_back/lib.rs @@ -46,6 +46,7 @@ extern crate syntax; extern crate libc; extern crate serialize; +extern crate rustc_llvm; #[macro_use] extern crate log; pub mod abi; diff --git a/src/librustc_llvm/archive_ro.rs b/src/librustc_llvm/archive_ro.rs index 647f4bc6a40..c8f3e204c4e 100644 --- a/src/librustc_llvm/archive_ro.rs +++ b/src/librustc_llvm/archive_ro.rs @@ -10,15 +10,23 @@ //! A wrapper around LLVM's archive (.a) code -use libc; use ArchiveRef; use std::ffi::CString; -use std::slice; use std::path::Path; +use std::slice; +use std::str; -pub struct ArchiveRO { - ptr: ArchiveRef, +pub struct ArchiveRO { ptr: ArchiveRef } + +pub struct Iter<'a> { + archive: &'a ArchiveRO, + ptr: ::ArchiveIteratorRef, +} + +pub struct Child<'a> { + name: Option<&'a str>, + data: &'a [u8], } impl ArchiveRO { @@ -52,18 +60,9 @@ impl ArchiveRO { } } - /// Reads a file in the archive - pub fn read<'a>(&'a self, file: &str) -> Option<&'a [u8]> { + pub fn iter(&self) -> Iter { unsafe { - let mut size = 0 as libc::size_t; - let file = CString::new(file).unwrap(); - let ptr = ::LLVMRustArchiveReadSection(self.ptr, file.as_ptr(), - &mut size); - if ptr.is_null() { - None - } else { - Some(slice::from_raw_parts(ptr as *const u8, size as usize)) - } + Iter { ptr: ::LLVMRustArchiveIteratorNew(self.ptr), archive: self } } } } @@ -75,3 +74,47 @@ impl Drop for ArchiveRO { } } } + +impl<'a> Iterator for Iter<'a> { + type Item = Child<'a>; + + fn next(&mut self) -> Option> { + unsafe { + let ptr = ::LLVMRustArchiveIteratorCurrent(self.ptr); + if ptr.is_null() { + return None + } + let mut name_len = 0; + let name_ptr = ::LLVMRustArchiveChildName(ptr, &mut name_len); + let mut data_len = 0; + let data_ptr = ::LLVMRustArchiveChildData(ptr, &mut data_len); + let child = Child { + name: if name_ptr.is_null() { + None + } else { + let name = slice::from_raw_parts(name_ptr as *const u8, + name_len as usize); + str::from_utf8(name).ok().map(|s| s.trim()) + }, + data: slice::from_raw_parts(data_ptr as *const u8, + data_len as usize), + }; + ::LLVMRustArchiveIteratorNext(self.ptr); + Some(child) + } + } +} + +#[unsafe_destructor] +impl<'a> Drop for Iter<'a> { + fn drop(&mut self) { + unsafe { + ::LLVMRustArchiveIteratorFree(self.ptr); + } + } +} + +impl<'a> Child<'a> { + pub fn name(&self) -> Option<&'a str> { self.name } + pub fn data(&self) -> &'a [u8] { self.data } +} diff --git a/src/librustc_llvm/lib.rs b/src/librustc_llvm/lib.rs index 7030ee56979..104fc6700cc 100644 --- a/src/librustc_llvm/lib.rs +++ b/src/librustc_llvm/lib.rs @@ -30,6 +30,7 @@ #![feature(libc)] #![feature(link_args)] #![feature(staged_api)] +#![feature(unsafe_destructor)] extern crate libc; #[macro_use] #[no_link] extern crate rustc_bitflags; @@ -488,9 +489,12 @@ pub type PassRef = *mut Pass_opaque; #[allow(missing_copy_implementations)] pub enum TargetMachine_opaque {} pub type TargetMachineRef = *mut TargetMachine_opaque; -#[allow(missing_copy_implementations)] pub enum Archive_opaque {} pub type ArchiveRef = *mut Archive_opaque; +pub enum ArchiveIterator_opaque {} +pub type ArchiveIteratorRef = *mut ArchiveIterator_opaque; +pub enum ArchiveChild_opaque {} +pub type ArchiveChildRef = *mut ArchiveChild_opaque; #[allow(missing_copy_implementations)] pub enum Twine_opaque {} pub type TwineRef = *mut Twine_opaque; @@ -2051,8 +2055,14 @@ extern { pub fn LLVMRustMarkAllFunctionsNounwind(M: ModuleRef); pub fn LLVMRustOpenArchive(path: *const c_char) -> ArchiveRef; - pub fn LLVMRustArchiveReadSection(AR: ArchiveRef, name: *const c_char, - out_len: *mut size_t) -> *const c_char; + pub fn LLVMRustArchiveIteratorNew(AR: ArchiveRef) -> ArchiveIteratorRef; + pub fn LLVMRustArchiveIteratorNext(AIR: ArchiveIteratorRef); + pub fn LLVMRustArchiveIteratorCurrent(AIR: ArchiveIteratorRef) -> ArchiveChildRef; + pub fn LLVMRustArchiveChildName(ACR: ArchiveChildRef, + size: *mut size_t) -> *const c_char; + pub fn LLVMRustArchiveChildData(ACR: ArchiveChildRef, + size: *mut size_t) -> *const c_char; + pub fn LLVMRustArchiveIteratorFree(AIR: ArchiveIteratorRef); pub fn LLVMRustDestroyArchive(AR: ArchiveRef); pub fn LLVMRustSetDLLExportStorageClass(V: ValueRef); diff --git a/src/librustc_trans/back/lto.rs b/src/librustc_trans/back/lto.rs index 4e099a4ca87..e06a7b882e6 100644 --- a/src/librustc_trans/back/lto.rs +++ b/src/librustc_trans/back/lto.rs @@ -63,13 +63,13 @@ pub fn run(sess: &session::Session, llmod: ModuleRef, let file = &file[3..file.len() - 5]; // chop off lib/.rlib debug!("reading {}", file); for i in 0.. { - let bc_encoded = time(sess.time_passes(), - &format!("check for {}.{}.bytecode.deflate", name, i), - (), - |_| { - archive.read(&format!("{}.{}.bytecode.deflate", - file, i)) - }); + let filename = format!("{}.{}.bytecode.deflate", file, i); + let msg = format!("check for {}", filename); + let bc_encoded = time(sess.time_passes(), &msg, (), |_| { + archive.iter().find(|section| { + section.name() == Some(&filename[..]) + }) + }); let bc_encoded = match bc_encoded { Some(data) => data, None => { @@ -79,9 +79,10 @@ pub fn run(sess: &session::Session, llmod: ModuleRef, path.display())); } // No more bitcode files to read. - break; - }, + break + } }; + let bc_encoded = bc_encoded.data(); let bc_decoded = if is_versioned_bytecode_format(bc_encoded) { time(sess.time_passes(), &format!("decode {}.{}.bc", file, i), (), |_| { diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 31f75ae03b0..492993ec9a9 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -770,37 +770,68 @@ LLVMRustOpenArchive(char *path) { return ret; } -extern "C" const char* #if LLVM_VERSION_MINOR >= 6 -LLVMRustArchiveReadSection(OwningBinary *ob, char *name, size_t *size) { - - Archive *ar = ob->getBinary(); +typedef OwningBinary RustArchive; +#define GET_ARCHIVE(a) ((a)->getBinary()) #else -LLVMRustArchiveReadSection(Archive *ar, char *name, size_t *size) { +typedef Archive RustArchive; +#define GET_ARCHIVE(a) (a) #endif - Archive::child_iterator child = ar->child_begin(), - end = ar->child_end(); - for (; child != end; ++child) { - ErrorOr name_or_err = child->getName(); - if (name_or_err.getError()) continue; - StringRef sect_name = name_or_err.get(); - if (sect_name.trim(" ") == name) { - StringRef buf = child->getBuffer(); - *size = buf.size(); - return buf.data(); - } - } - return NULL; +extern "C" void +LLVMRustDestroyArchive(RustArchive *ar) { + delete ar; +} + +struct RustArchiveIterator { + Archive::child_iterator cur; + Archive::child_iterator end; +}; + +extern "C" RustArchiveIterator* +LLVMRustArchiveIteratorNew(RustArchive *ra) { + Archive *ar = GET_ARCHIVE(ra); + RustArchiveIterator *rai = new RustArchiveIterator(); + rai->cur = ar->child_begin(); + rai->end = ar->child_end(); + return rai; +} + +extern "C" const Archive::Child* +LLVMRustArchiveIteratorCurrent(RustArchiveIterator *rai) { + if (rai->cur == rai->end) + return NULL; + const Archive::Child &ret = *rai->cur; + return &ret; } extern "C" void -#if LLVM_VERSION_MINOR >= 6 -LLVMRustDestroyArchive(OwningBinary *ar) { -#else -LLVMRustDestroyArchive(Archive *ar) { -#endif - delete ar; +LLVMRustArchiveIteratorNext(RustArchiveIterator *rai) { + if (rai->cur == rai->end) + return; + ++rai->cur; +} + +extern "C" void +LLVMRustArchiveIteratorFree(RustArchiveIterator *rai) { + delete rai; +} + +extern "C" const char* +LLVMRustArchiveChildName(const Archive::Child *child, size_t *size) { + ErrorOr name_or_err = child->getName(); + if (name_or_err.getError()) + return NULL; + StringRef name = name_or_err.get(); + *size = name.size(); + return name.data(); +} + +extern "C" const char* +LLVMRustArchiveChildData(Archive::Child *child, size_t *size) { + StringRef buf = child->getBuffer(); + *size = buf.size(); + return buf.data(); } extern "C" void diff --git a/src/test/run-make/archive-duplicate-names/Makefile b/src/test/run-make/archive-duplicate-names/Makefile new file mode 100644 index 00000000000..72c2d389e2a --- /dev/null +++ b/src/test/run-make/archive-duplicate-names/Makefile @@ -0,0 +1,11 @@ +-include ../tools.mk + +all: + mkdir $(TMPDIR)/a + mkdir $(TMPDIR)/b + $(CC) -c -o $(TMPDIR)/a/foo.o foo.c + $(CC) -c -o $(TMPDIR)/b/foo.o bar.c + ar crus $(TMPDIR)/libfoo.a $(TMPDIR)/a/foo.o $(TMPDIR)/b/foo.o + $(RUSTC) foo.rs + $(RUSTC) bar.rs + $(call RUN,bar) diff --git a/src/test/run-make/archive-duplicate-names/bar.c b/src/test/run-make/archive-duplicate-names/bar.c new file mode 100644 index 00000000000..a25fa10f4d3 --- /dev/null +++ b/src/test/run-make/archive-duplicate-names/bar.c @@ -0,0 +1,11 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +void bar() {} diff --git a/src/test/run-make/archive-duplicate-names/bar.rs b/src/test/run-make/archive-duplicate-names/bar.rs new file mode 100644 index 00000000000..1200a6de8e2 --- /dev/null +++ b/src/test/run-make/archive-duplicate-names/bar.rs @@ -0,0 +1,15 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +extern crate foo; + +fn main() { + foo::baz(); +} diff --git a/src/test/run-make/archive-duplicate-names/foo.c b/src/test/run-make/archive-duplicate-names/foo.c new file mode 100644 index 00000000000..61d5d154078 --- /dev/null +++ b/src/test/run-make/archive-duplicate-names/foo.c @@ -0,0 +1,11 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +void foo() {} diff --git a/src/test/run-make/archive-duplicate-names/foo.rs b/src/test/run-make/archive-duplicate-names/foo.rs new file mode 100644 index 00000000000..24b4734f2cd --- /dev/null +++ b/src/test/run-make/archive-duplicate-names/foo.rs @@ -0,0 +1,24 @@ +// Copyright 2015 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![crate_type = "rlib"] + +#[link(name = "foo", kind = "static")] +extern { + fn foo(); + fn bar(); +} + +pub fn baz() { + unsafe { + foo(); + bar(); + } +}