Merge pull request #13 from dpaoliello/thin

Add tests for thin archives, fix bugs found in testing

Author: bjorn3

src/archive_writer.rs

@@ -490,7 +490,8 @@ fn compute_member_data<'a, S: Write + Seek>(
     for m in new_members {
         let mut header = Vec::new();
 
-        let data: &[u8] = if thin { &[][..] } else { (*m.buf).as_ref() };
+        let buf = m.buf.as_ref().as_ref();
+        let data = if thin { &[][..] } else { buf };
 
         // ld64 expects the members to be 8-byte aligned for 64-bit content and at
         // least 4-byte aligned for 32-bit content.  Opt for the larger encoding
@@ -513,7 +514,7 @@ fn compute_member_data<'a, S: Write + Seek>(
             m.mtime
         };
 
-        let size = u64::try_from(data.len()).unwrap() + member_padding;
+        let size = u64::try_from(buf.len()).unwrap() + member_padding;
         if size > MAX_MEMBER_SIZE {
             return Err(io::Error::new(
                 io::ErrorKind::Other,
@@ -552,7 +553,7 @@ fn compute_member_data<'a, S: Write + Seek>(
             )?;
         }
 
-        let symbols = write_symbols(data, m.get_symbols, sym_names, &mut has_object)?;
+        let symbols = write_symbols(buf, m.get_symbols, sym_names, &mut has_object)?;
 
         pos += u64::try_from(header.len() + data.len() + padding.len()).unwrap();
         ret.push(MemberData {

tests/common.rs

@@ -25,18 +25,29 @@ pub fn create_tmp_dir(test_name: &str) -> PathBuf {
     tmpdir
 }
 
-fn run_llvm_ar(object_paths: &[PathBuf], archive_path: &Path, archive_kind: ArchiveKind) {
+fn run_llvm_ar(
+    object_paths: &[PathBuf],
+    archive_path: &Path,
+    archive_kind: ArchiveKind,
+    thin: bool,
+) {
     let ar_path = cargo_binutils::Tool::Ar.path().unwrap();
 
+    let mut command = Command::new(ar_path);
+
     let format_arg = match archive_kind {
         ArchiveKind::Gnu => "gnu",
         ArchiveKind::Darwin => "darwin",
         ArchiveKind::AixBig => "bigarchive",
         _ => panic!("unsupported archive kind: {:?}", archive_kind),
     };
+    command.arg(format!("--format={}", format_arg));
 
-    let output = Command::new(ar_path)
-        .arg(format!("--format={}", format_arg))
+    if thin {
+        command.arg("--thin");
+    }
+
+    let output = command
         .arg("rcs")
         .arg(archive_path)
         .args(object_paths)
@@ -55,6 +66,7 @@ pub fn create_archive_with_llvm_ar<'a>(
     tmpdir: &Path,
     archive_kind: ArchiveKind,
     input_objects: impl IntoIterator<Item = (&'static str, &'a [u8])>,
+    thin: bool,
 ) -> Vec<u8> {
     let archive_file_path = tmpdir.join("output_llvm_ar.a");
 
@@ -70,7 +82,7 @@ pub fn create_archive_with_llvm_ar<'a>(
         })
         .collect::<Vec<_>>();
 
-    run_llvm_ar(&input_file_paths, &archive_file_path, archive_kind);
+    run_llvm_ar(&input_file_paths, &archive_file_path, archive_kind, thin);
     fs::read(archive_file_path).unwrap()
 }
 
@@ -80,24 +92,37 @@ pub fn create_archive_with_ar_archive_writer<'a>(
     tmpdir: &Path,
     archive_kind: ArchiveKind,
     input_objects: impl IntoIterator<Item = (&'static str, &'a [u8])>,
+    thin: bool,
 ) -> Vec<u8> {
     let members = input_objects
         .into_iter()
-        .map(|(name, bytes)| NewArchiveMember {
-            buf: Box::new(bytes) as Box<dyn AsRef<[u8]>>,
-            get_symbols: ar_archive_writer::get_native_object_symbols,
-            member_name: name
-                .rsplit_once('/')
-                .map_or(name, |(_, filename)| filename)
-                .to_string(),
-            mtime: 0,
-            uid: 0,
-            gid: 0,
-            perms: 0o644,
+        .map(|(name, bytes)| {
+            let member_name = if thin {
+                // Thin archives use the full path to the object file.
+                tmpdir
+                    .join(name)
+                    .to_string_lossy()
+                    .replace(std::path::MAIN_SEPARATOR, "/")
+            } else {
+                // Non-thin archives use the file name only.
+                name.rsplit_once('/')
+                    .map_or(name, |(_, filename)| filename)
+                    .to_string()
+            };
+
+            NewArchiveMember {
+                buf: Box::new(bytes) as Box<dyn AsRef<[u8]>>,
+                get_symbols: ar_archive_writer::get_native_object_symbols,
+                member_name,
+                mtime: 0,
+                uid: 0,
+                gid: 0,
+                perms: 0o644,
+            }
         })
         .collect::<Vec<_>>();
     let mut output_bytes = Cursor::new(Vec::new());
-    ar_archive_writer::write_archive_to_stream(&mut output_bytes, &members, archive_kind, false)
+    ar_archive_writer::write_archive_to_stream(&mut output_bytes, &members, archive_kind, thin)
         .unwrap();
 
     let output_archive_bytes = output_bytes.into_inner();
@@ -112,58 +137,88 @@ pub fn generate_archive_and_compare<F>(test_name: &str, generate_objects: F)
 where
     F: Fn(Architecture, Endianness, BinaryFormat) -> Vec<(&'static str, Vec<u8>)>,
 {
-    for (architecture, endianness, binary_format, archive_kind) in [
-        // Elf + GNU
+    for (architecture, endianness, binary_format, archive_kind, thin) in [
+        // Elf + GNU + non-thin
+        (
+            Architecture::X86_64,
+            Endianness::Little,
+            BinaryFormat::Elf,
+            ArchiveKind::Gnu,
+            false,
+        ),
+        (
+            Architecture::I386,
+            Endianness::Little,
+            BinaryFormat::Elf,
+            ArchiveKind::Gnu,
+            false,
+        ),
+        (
+            Architecture::Aarch64,
+            Endianness::Little,
+            BinaryFormat::Elf,
+            ArchiveKind::Gnu,
+            false,
+        ),
+        // Elf + GNU + thin
         (
             Architecture::X86_64,
             Endianness::Little,
             BinaryFormat::Elf,
             ArchiveKind::Gnu,
+            true,
         ),
         (
             Architecture::I386,
             Endianness::Little,
             BinaryFormat::Elf,
             ArchiveKind::Gnu,
+            true,
         ),
         (
             Architecture::Aarch64,
             Endianness::Little,
             BinaryFormat::Elf,
             ArchiveKind::Gnu,
+            true,
         ),
         // AIX Big
         (
             Architecture::PowerPc64,
             Endianness::Big,
             BinaryFormat::Elf,
             ArchiveKind::AixBig,
+            false,
         ),
         // PE + GNU
         (
             Architecture::X86_64,
             Endianness::Little,
             BinaryFormat::Coff,
             ArchiveKind::Gnu,
+            false,
         ),
         (
             Architecture::I386,
             Endianness::Little,
             BinaryFormat::Coff,
             ArchiveKind::Gnu,
+            false,
         ),
         // MachO
         (
             Architecture::X86_64,
             Endianness::Little,
             BinaryFormat::MachO,
             ArchiveKind::Darwin,
+            false,
         ),
         (
             Architecture::Aarch64,
             Endianness::Little,
             BinaryFormat::MachO,
             ArchiveKind::Darwin,
+            false,
         ),
     ] {
         let tmpdir = create_tmp_dir(test_name);
@@ -174,18 +229,20 @@ where
             input_objects
                 .iter()
                 .map(|(name, bytes)| (*name, bytes.as_slice())),
+            thin,
         );
         let ar_archive_writer_archive = create_archive_with_ar_archive_writer(
             &tmpdir,
             archive_kind,
             input_objects
                 .iter()
                 .map(|(name, bytes)| (*name, bytes.as_slice())),
+            thin,
         );
 
         assert_eq!(
             llvm_ar_archive, ar_archive_writer_archive,
-            "Archives differ for architecture: {architecture:?}, binary_format: {binary_format:?}, archive_kind: {archive_kind:?}",
+            "Archives differ for architecture: {architecture:?}, binary_format: {binary_format:?}, archive_kind: {archive_kind:?}, thin: {thin}",
         );
     }
 }

tests/round_trip.rs

@@ -25,6 +25,7 @@ fn round_trip_and_diff(
         &tmpdir,
         archive_kind,
         [("input.o", input_bytes.as_slice())],
+        false,
     );
 
     // Read the archive and member using object and diff with original data.
@@ -50,6 +51,7 @@ fn round_trip_and_diff(
         &tmpdir,
         archive_kind,
         [("input.o", input_bytes.as_slice())],
+        false,
     );
     assert_eq!(
         output_archive_bytes, output_llvm_ar_bytes,