From 4f8042e22ebaf962ebbffc6056373b387dc65515 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 10 Aug 2024 17:42:56 +0000 Subject: [PATCH 1/5] Support reading thin archives in ArArchiveBuilder --- compiler/rustc_codegen_ssa/src/back/archive.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index ce55d99f506d5..a35aff096a453 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -307,10 +307,17 @@ impl<'a> ArchiveBuilder for ArArchiveBuilder<'a> { let file_name = String::from_utf8(entry.name().to_vec()) .map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?; if !skip(&file_name) { - self.entries.push(( - file_name.into_bytes(), - ArchiveEntry::FromArchive { archive_index, file_range: entry.file_range() }, - )); + if entry.is_thin() { + self.entries.push(( + file_name.clone().into_bytes(), + ArchiveEntry::File(PathBuf::from(file_name)), + )); + } else { + self.entries.push(( + file_name.into_bytes(), + ArchiveEntry::FromArchive { archive_index, file_range: entry.file_range() }, + )); + } } } From a57f73d32060ac27bfdb6635b98025f362598574 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 10 Aug 2024 17:43:22 +0000 Subject: [PATCH 2/5] Add test for thin archive reading support --- .../run-make-support/src/external_deps/llvm.rs | 6 ++++++ tests/run-make/staticlib-thin-archive/bin.rs | 10 ++++++++++ tests/run-make/staticlib-thin-archive/rmake.rs | 13 +++++++++++++ .../run-make/staticlib-thin-archive/rust_archive.rs | 4 ++++ tests/run-make/staticlib-thin-archive/simple_obj.rs | 4 ++++ 5 files changed, 37 insertions(+) create mode 100644 tests/run-make/staticlib-thin-archive/bin.rs create mode 100644 tests/run-make/staticlib-thin-archive/rmake.rs create mode 100644 tests/run-make/staticlib-thin-archive/rust_archive.rs create mode 100644 tests/run-make/staticlib-thin-archive/simple_obj.rs diff --git a/src/tools/run-make-support/src/external_deps/llvm.rs b/src/tools/run-make-support/src/external_deps/llvm.rs index dc651fdd8205a..4003287d7629d 100644 --- a/src/tools/run-make-support/src/external_deps/llvm.rs +++ b/src/tools/run-make-support/src/external_deps/llvm.rs @@ -271,6 +271,12 @@ impl LlvmAr { self } + /// Like `obj_to_ar` except creating a thin archive. + pub fn obj_to_thin_ar(&mut self) -> &mut Self { + self.cmd.arg("rcus").arg("--thin"); + self + } + /// Extract archive members back to files. pub fn extract(&mut self) -> &mut Self { self.cmd.arg("x"); diff --git a/tests/run-make/staticlib-thin-archive/bin.rs b/tests/run-make/staticlib-thin-archive/bin.rs new file mode 100644 index 0000000000000..b37107d9c9435 --- /dev/null +++ b/tests/run-make/staticlib-thin-archive/bin.rs @@ -0,0 +1,10 @@ +#[link(name = "rust_archive", kind = "static")] +extern "C" { + fn simple_fn(); +} + +fn main() { + unsafe { + simple_fn(); + } +} diff --git a/tests/run-make/staticlib-thin-archive/rmake.rs b/tests/run-make/staticlib-thin-archive/rmake.rs new file mode 100644 index 0000000000000..137dffcc33fc6 --- /dev/null +++ b/tests/run-make/staticlib-thin-archive/rmake.rs @@ -0,0 +1,13 @@ +// Regression test for https://github.com/rust-lang/rust/issues/107407 + +use run_make_support::{llvm_ar, rustc, static_lib_name}; + +fn main() { + rustc().input("simple_obj.rs").emit("obj").run(); + llvm_ar().obj_to_thin_ar().output_input(static_lib_name("thin_archive"), "simple_obj.o").run(); + rustc().input("rust_archive.rs").run(); + // Disable lld as it ignores the symbol table in the archive file. + rustc() + .input("bin.rs") /*.arg("-Zlinker-features=-lld")*/ + .run(); +} diff --git a/tests/run-make/staticlib-thin-archive/rust_archive.rs b/tests/run-make/staticlib-thin-archive/rust_archive.rs new file mode 100644 index 0000000000000..3d11f02df2016 --- /dev/null +++ b/tests/run-make/staticlib-thin-archive/rust_archive.rs @@ -0,0 +1,4 @@ +#![crate_type = "staticlib"] + +#[link(name = "thin_archive", kind = "static")] +extern "C" {} diff --git a/tests/run-make/staticlib-thin-archive/simple_obj.rs b/tests/run-make/staticlib-thin-archive/simple_obj.rs new file mode 100644 index 0000000000000..a120c9b3e676b --- /dev/null +++ b/tests/run-make/staticlib-thin-archive/simple_obj.rs @@ -0,0 +1,4 @@ +#![crate_type = "staticlib"] + +#[no_mangle] +extern "C" fn simple_fn() {} From c1f5350df5559709506510c1334f9664b0a171e1 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 10 Aug 2024 17:45:39 +0000 Subject: [PATCH 3/5] Use ArArchiveBuilder with the LLVM backend too All regressions that were blocking usage of ArArchiveBuilder should now be fixed. --- compiler/rustc_codegen_llvm/src/back/archive.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs index a2ab19ac800b7..ce05336f07191 100644 --- a/compiler/rustc_codegen_llvm/src/back/archive.rs +++ b/compiler/rustc_codegen_llvm/src/back/archive.rs @@ -106,9 +106,7 @@ pub struct LlvmArchiveBuilderBuilder; impl ArchiveBuilderBuilder for LlvmArchiveBuilderBuilder { fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box { - // FIXME use ArArchiveBuilder on most targets again once reading thin archives is - // implemented - if true { + if false { Box::new(LlvmArchiveBuilder { sess, additions: Vec::new() }) } else { Box::new(ArArchiveBuilder::new(sess, &LLVM_OBJECT_READER)) From d63a067bfd9d0674e637fbfc83e0cbd526fb92b5 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 10 Aug 2024 18:49:36 +0000 Subject: [PATCH 4/5] Add fixme for removing LlvmArchiveBuilder in the future --- compiler/rustc_codegen_llvm/src/back/archive.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs index ce05336f07191..0a8728c385ca1 100644 --- a/compiler/rustc_codegen_llvm/src/back/archive.rs +++ b/compiler/rustc_codegen_llvm/src/back/archive.rs @@ -106,6 +106,10 @@ pub struct LlvmArchiveBuilderBuilder; impl ArchiveBuilderBuilder for LlvmArchiveBuilderBuilder { fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box { + // Keeping LlvmArchiveBuilder around in case of a regression caused by using + // ArArchiveBuilder. + // FIXME remove a couple of months after #128936 gets merged in case no + // regression is found. if false { Box::new(LlvmArchiveBuilder { sess, additions: Vec::new() }) } else { From db68a19b619ffc4b4ee9d1118d064d184d0bcd37 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 11 Aug 2024 10:29:32 +0000 Subject: [PATCH 5/5] Fix review comments and other improvements --- .../rustc_codegen_llvm/src/back/archive.rs | 4 +-- .../rustc_codegen_ssa/src/back/archive.rs | 6 ++-- tests/run-make/staticlib-thin-archive/bin.rs | 7 +---- .../run-make/staticlib-thin-archive/rmake.rs | 31 ++++++++++++++----- .../staticlib-thin-archive/rust_archive.rs | 4 --- .../staticlib-thin-archive/rust_lib.rs | 6 ++++ 6 files changed, 34 insertions(+), 24 deletions(-) delete mode 100644 tests/run-make/staticlib-thin-archive/rust_archive.rs create mode 100644 tests/run-make/staticlib-thin-archive/rust_lib.rs diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs index 0a8728c385ca1..60e6346254877 100644 --- a/compiler/rustc_codegen_llvm/src/back/archive.rs +++ b/compiler/rustc_codegen_llvm/src/back/archive.rs @@ -108,8 +108,8 @@ impl ArchiveBuilderBuilder for LlvmArchiveBuilderBuilder { fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box { // Keeping LlvmArchiveBuilder around in case of a regression caused by using // ArArchiveBuilder. - // FIXME remove a couple of months after #128936 gets merged in case no - // regression is found. + // FIXME(#128955) remove a couple of months after #128936 gets merged in case + // no regression is found. if false { Box::new(LlvmArchiveBuilder { sess, additions: Vec::new() }) } else { diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index a35aff096a453..8eb44d1201640 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -308,10 +308,8 @@ impl<'a> ArchiveBuilder for ArArchiveBuilder<'a> { .map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?; if !skip(&file_name) { if entry.is_thin() { - self.entries.push(( - file_name.clone().into_bytes(), - ArchiveEntry::File(PathBuf::from(file_name)), - )); + let member_path = archive_path.parent().unwrap().join(Path::new(&file_name)); + self.entries.push((file_name.into_bytes(), ArchiveEntry::File(member_path))); } else { self.entries.push(( file_name.into_bytes(), diff --git a/tests/run-make/staticlib-thin-archive/bin.rs b/tests/run-make/staticlib-thin-archive/bin.rs index b37107d9c9435..97a2751f20bcc 100644 --- a/tests/run-make/staticlib-thin-archive/bin.rs +++ b/tests/run-make/staticlib-thin-archive/bin.rs @@ -1,10 +1,5 @@ -#[link(name = "rust_archive", kind = "static")] -extern "C" { - fn simple_fn(); -} - fn main() { unsafe { - simple_fn(); + rust_lib::simple_fn(); } } diff --git a/tests/run-make/staticlib-thin-archive/rmake.rs b/tests/run-make/staticlib-thin-archive/rmake.rs index 137dffcc33fc6..f7e3e43d53581 100644 --- a/tests/run-make/staticlib-thin-archive/rmake.rs +++ b/tests/run-make/staticlib-thin-archive/rmake.rs @@ -1,13 +1,28 @@ -// Regression test for https://github.com/rust-lang/rust/issues/107407 +// Regression test for https://github.com/rust-lang/rust/issues/107407 which +// checks that rustc can read thin archive. Before the object crate added thin +// archive support rustc would add emit object files to the staticlib and after +// the object crate added thin archive support it would previously crash the +// compiler due to a missing special case for thin archive members. +use std::path::Path; -use run_make_support::{llvm_ar, rustc, static_lib_name}; +use run_make_support::{llvm_ar, rust_lib_name, rustc, static_lib_name}; fn main() { - rustc().input("simple_obj.rs").emit("obj").run(); - llvm_ar().obj_to_thin_ar().output_input(static_lib_name("thin_archive"), "simple_obj.o").run(); - rustc().input("rust_archive.rs").run(); - // Disable lld as it ignores the symbol table in the archive file. - rustc() - .input("bin.rs") /*.arg("-Zlinker-features=-lld")*/ + std::fs::create_dir("archive").unwrap(); + + // Build a thin archive + rustc().input("simple_obj.rs").emit("obj").output("archive/simple_obj.o").run(); + llvm_ar() + .obj_to_thin_ar() + .output_input( + Path::new("archive").join(static_lib_name("thin_archive")), + "archive/simple_obj.o", + ) .run(); + + // Build an rlib which includes the members of this thin archive + rustc().input("rust_lib.rs").library_search_path("archive").run(); + + // Build a binary which requires a symbol from the thin archive + rustc().input("bin.rs").extern_("rust_lib", rust_lib_name("rust_lib")).run(); } diff --git a/tests/run-make/staticlib-thin-archive/rust_archive.rs b/tests/run-make/staticlib-thin-archive/rust_archive.rs deleted file mode 100644 index 3d11f02df2016..0000000000000 --- a/tests/run-make/staticlib-thin-archive/rust_archive.rs +++ /dev/null @@ -1,4 +0,0 @@ -#![crate_type = "staticlib"] - -#[link(name = "thin_archive", kind = "static")] -extern "C" {} diff --git a/tests/run-make/staticlib-thin-archive/rust_lib.rs b/tests/run-make/staticlib-thin-archive/rust_lib.rs new file mode 100644 index 0000000000000..c76b0f2543376 --- /dev/null +++ b/tests/run-make/staticlib-thin-archive/rust_lib.rs @@ -0,0 +1,6 @@ +#![crate_type = "rlib"] + +#[link(name = "thin_archive", kind = "static")] +extern "C" { + pub fn simple_fn(); +}