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/9] 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/9] 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/9] 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/9] 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/9] 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(); +} From 221701421066fc91eee4c7bde129b607088cf328 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 14 Aug 2024 16:44:05 +0000 Subject: [PATCH 6/9] Apply some suggestions to the test rmake file --- tests/run-make/staticlib-thin-archive/rmake.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/run-make/staticlib-thin-archive/rmake.rs b/tests/run-make/staticlib-thin-archive/rmake.rs index f7e3e43d53581..955c50da2011c 100644 --- a/tests/run-make/staticlib-thin-archive/rmake.rs +++ b/tests/run-make/staticlib-thin-archive/rmake.rs @@ -3,21 +3,16 @@ // 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, rust_lib_name, rustc, static_lib_name}; +use run_make_support::{llvm_ar, path, rfs, rust_lib_name, rustc, static_lib_name}; fn main() { - std::fs::create_dir("archive").unwrap(); + rfs::create_dir("archive"); // 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", - ) + .output_input(path("archive").join(static_lib_name("thin_archive")), "archive/simple_obj.o") .run(); // Build an rlib which includes the members of this thin archive From 9de0d147f479f94c7cb49f1573e90fc529cda476 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 14 Aug 2024 16:50:48 +0000 Subject: [PATCH 7/9] Unconditionally use the LLVM symbol reader This may fix a linker error on MSVC --- .../rustc_codegen_llvm/src/back/archive.rs | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs index 60e6346254877..2120fc1815cd8 100644 --- a/compiler/rustc_codegen_llvm/src/back/archive.rs +++ b/compiler/rustc_codegen_llvm/src/back/archive.rs @@ -200,25 +200,11 @@ static LLVM_OBJECT_READER: ObjectReader = ObjectReader { get_xcoff_member_alignment: DEFAULT_OBJECT_READER.get_xcoff_member_alignment, }; -fn should_use_llvm_reader(buf: &[u8]) -> bool { - let is_bitcode = unsafe { llvm::LLVMRustIsBitcode(buf.as_ptr(), buf.len()) }; - - // COFF bigobj file, msvc LTO file or import library. See - // https://github.com/llvm/llvm-project/blob/453f27bc9/llvm/lib/BinaryFormat/Magic.cpp#L38-L51 - let is_unsupported_windows_obj_file = buf.get(0..4) == Some(b"\0\0\xFF\xFF"); - - is_bitcode || is_unsupported_windows_obj_file -} - #[deny(unsafe_op_in_unsafe_fn)] fn get_llvm_object_symbols( buf: &[u8], f: &mut dyn FnMut(&[u8]) -> io::Result<()>, ) -> io::Result { - if !should_use_llvm_reader(buf) { - return (DEFAULT_OBJECT_READER.get_symbols)(buf, f); - } - let mut state = Box::new(f); let err = unsafe { @@ -255,18 +241,10 @@ fn get_llvm_object_symbols( } fn llvm_is_64_bit_object_file(buf: &[u8]) -> bool { - if !should_use_llvm_reader(buf) { - return (DEFAULT_OBJECT_READER.is_64_bit_object_file)(buf); - } - unsafe { llvm::LLVMRustIs64BitSymbolicFile(buf.as_ptr(), buf.len()) } } fn llvm_is_ec_object_file(buf: &[u8]) -> bool { - if !should_use_llvm_reader(buf) { - return (DEFAULT_OBJECT_READER.is_ec_object_file)(buf); - } - unsafe { llvm::LLVMRustIsECObject(buf.as_ptr(), buf.len()) } } From 7c972d75dca7d1ef0e5a7d356cdc4d61afcd739f Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 14 Aug 2024 19:21:44 +0000 Subject: [PATCH 8/9] Use toString instead of raw_svector_ostream for error messages --- compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp index ccf1a5429e2b5..d119a995c61b5 100644 --- a/compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp @@ -77,11 +77,7 @@ LLVMRustGetSymbols(char *BufPtr, size_t BufLen, void *State, Expected> ObjOrErr = getSymbolicFile(Buf->getMemBufferRef(), Context); if (!ObjOrErr) { - Error E = ObjOrErr.takeError(); - SmallString<0> ErrorBuf; - auto Error = raw_svector_ostream(ErrorBuf); - Error << E << '\0'; - return ErrorCallback(Error.str().data()); + return ErrorCallback(toString(ObjOrErr.takeError()).c_str()); } std::unique_ptr Obj = std::move(*ObjOrErr); @@ -89,10 +85,7 @@ LLVMRustGetSymbols(char *BufPtr, size_t BufLen, void *State, if (!isArchiveSymbol(S)) continue; if (Error E = S.printName(SymName)) { - SmallString<0> ErrorBuf; - auto Error = raw_svector_ostream(ErrorBuf); - Error << E << '\0'; - return ErrorCallback(Error.str().data()); + return ErrorCallback(toString(std::move(E)).c_str()); } SymName << '\0'; if (void *E = Callback(State, SymNameBuf.str().data())) { From 901c9daa05ec817c3a0f7aad87a928cfcfe00ce2 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 14 Aug 2024 19:37:14 +0000 Subject: [PATCH 9/9] Fix null pointer dereference when a file is not an object file --- compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp index d119a995c61b5..d625935d92594 100644 --- a/compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/SymbolWrapper.cpp @@ -80,6 +80,9 @@ LLVMRustGetSymbols(char *BufPtr, size_t BufLen, void *State, return ErrorCallback(toString(ObjOrErr.takeError()).c_str()); } std::unique_ptr Obj = std::move(*ObjOrErr); + if (Obj == nullptr) { + return 0; + } for (const object::BasicSymbolRef &S : Obj->symbols()) { if (!isArchiveSymbol(S))