Merge pull request #23 from ChrisDenton/rename

Make the null import descriptor name unique to the import library

Author: bjorn3

src/archive_writer.rs

@@ -542,7 +542,7 @@ fn write_ec_symbols<W: Write + Seek>(w: &mut W, sym_map: &SymMap) -> io::Result<
 
 fn is_import_descriptor(name: &[u8]) -> bool {
     name.starts_with(coff_import_file::IMPORT_DESCRIPTOR_PREFIX)
-        || name == coff_import_file::NULL_IMPORT_DESCRIPTOR_SYMBOL_NAME
+        || name.starts_with(coff_import_file::NULL_IMPORT_DESCRIPTOR_SYMBOL_NAME)
         || (name.starts_with(coff_import_file::NULL_THUNK_DATA_PREFIX)
             && name.ends_with(coff_import_file::NULL_THUNK_DATA_SUFFIX))
 }

src/coff_import_file.rs

@@ -10,13 +10,12 @@ use std::path::PathBuf;
 use std::str::from_utf8;
 
 use object::pe::{
-    ImageAuxSymbolSection, ImageFileHeader, ImageImportDescriptor, ImageRelocation,
-    ImageSectionHeader, ImageSymbol, ImportObjectHeader, IMAGE_COMDAT_SELECT_ANY,
-    IMAGE_FILE_32BIT_MACHINE, IMAGE_REL_AMD64_ADDR32NB, IMAGE_REL_ARM64_ADDR32NB,
-    IMAGE_REL_ARM_ADDR32NB, IMAGE_REL_I386_DIR32NB, IMAGE_SCN_ALIGN_2BYTES, IMAGE_SCN_ALIGN_4BYTES,
-    IMAGE_SCN_ALIGN_8BYTES, IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_COMDAT,
-    IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE,
-    IMAGE_SYM_CLASS_EXTERNAL, IMAGE_SYM_CLASS_NULL, IMAGE_SYM_CLASS_SECTION,
+    ImageFileHeader, ImageImportDescriptor, ImageRelocation, ImageSectionHeader, ImageSymbol,
+    ImportObjectHeader, IMAGE_FILE_32BIT_MACHINE, IMAGE_REL_AMD64_ADDR32NB,
+    IMAGE_REL_ARM64_ADDR32NB, IMAGE_REL_ARM_ADDR32NB, IMAGE_REL_I386_DIR32NB,
+    IMAGE_SCN_ALIGN_2BYTES, IMAGE_SCN_ALIGN_4BYTES, IMAGE_SCN_ALIGN_8BYTES,
+    IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE, IMAGE_SCN_MEM_READ,
+    IMAGE_SCN_MEM_WRITE, IMAGE_SYM_CLASS_EXTERNAL, IMAGE_SYM_CLASS_NULL, IMAGE_SYM_CLASS_SECTION,
     IMAGE_SYM_CLASS_STATIC, IMAGE_SYM_CLASS_WEAK_EXTERNAL, IMAGE_WEAK_EXTERN_SEARCH_ALIAS,
 };
 use object::pod::bytes_of;
@@ -27,6 +26,7 @@ use crate::{write_archive_to_stream, ArchiveKind, NewArchiveMember, DEFAULT_OBJE
 
 pub(crate) const IMPORT_DESCRIPTOR_PREFIX: &[u8] = b"__IMPORT_DESCRIPTOR_";
 pub(crate) const NULL_IMPORT_DESCRIPTOR_SYMBOL_NAME: &[u8] = b"__NULL_IMPORT_DESCRIPTOR";
+pub(crate) const NULL_IMPORT_DESCRIPTOR_SYMBOL_NAME_PREFIX: &[u8] = b"__NULL_IMPORT_DESCRIPTOR_";
 pub(crate) const NULL_THUNK_DATA_PREFIX: &[u8] = b"\x7f";
 pub(crate) const NULL_THUNK_DATA_SUFFIX: &[u8] = b"_NULL_THUNK_DATA";
 
@@ -209,10 +209,11 @@ struct ObjectFactory<'a> {
     import_name: &'a str,
     import_descriptor_symbol_name: Vec<u8>,
     null_thunk_symbol_name: Vec<u8>,
+    null_import_descriptor_symbol_name: Vec<u8>,
 }
 
 impl<'a> ObjectFactory<'a> {
-    fn new(s: &'a str, m: MachineTypes) -> Result<Self> {
+    fn new(s: &'a str, m: MachineTypes, whole_archive_compat: bool) -> Result<Self> {
         let import_as_path = PathBuf::from(s);
         let library = import_as_path
             .file_stem()
@@ -239,6 +240,15 @@ impl<'a> ObjectFactory<'a> {
                 .chain(NULL_THUNK_DATA_SUFFIX)
                 .copied()
                 .collect(),
+            null_import_descriptor_symbol_name: if whole_archive_compat {
+                NULL_IMPORT_DESCRIPTOR_SYMBOL_NAME_PREFIX
+                    .iter()
+                    .chain(library)
+                    .copied()
+                    .collect()
+            } else {
+                NULL_IMPORT_DESCRIPTOR_SYMBOL_NAME.into()
+            },
         })
     }
 
@@ -439,7 +449,7 @@ impl<'a> ObjectFactory<'a> {
             size_of::<u32>()
                 + self.import_descriptor_symbol_name.len()
                 + 1
-                + NULL_IMPORT_DESCRIPTOR_SYMBOL_NAME.len()
+                + self.null_import_descriptor_symbol_name.len()
                 + 1,
         );
         buffer.write_all(bytes_of(&symbol_table))?;
@@ -449,7 +459,7 @@ impl<'a> ObjectFactory<'a> {
             &mut buffer,
             &[
                 &self.import_descriptor_symbol_name,
-                NULL_IMPORT_DESCRIPTOR_SYMBOL_NAME,
+                &self.null_import_descriptor_symbol_name,
                 &self.null_thunk_symbol_name,
             ],
         )?;
@@ -464,14 +474,11 @@ impl<'a> ObjectFactory<'a> {
     /// Creates a NULL import descriptor.  This is a small object file whcih
     /// contains a NULL import descriptor.  It is used to terminate the imports
     /// from a specific DLL.
-    ///
-    /// If `comdat` is set to true, the NULL import descriptor's section (`.idata$3`) will be
-    /// turned into a COMDAT section.
-    fn create_null_import_descriptor(&self, comdat: bool) -> Result<NewArchiveMember<'_>> {
+    fn create_null_import_descriptor(&self) -> Result<NewArchiveMember<'_>> {
         let mut buffer = Vec::new();
 
         const NUMBER_OF_SECTIONS: usize = 1;
-        let number_of_symbols = if comdat { 3 } else { 1 };
+        const NUMBER_OF_SYMBOLS: usize = 1;
 
         // COFF Header
         let header = ImageFileHeader {
@@ -481,7 +488,7 @@ impl<'a> ObjectFactory<'a> {
             pointer_to_symbol_table: u32!((size_of::<ImageFileHeader>() + (NUMBER_OF_SECTIONS * size_of::<ImageSectionHeader>()) +
                 // .idata$3
                 size_of::<ImageImportDescriptor>()).try_into().unwrap()),
-            number_of_symbols: u32!(number_of_symbols.try_into().unwrap()),
+            number_of_symbols: u32!(NUMBER_OF_SYMBOLS.try_into().unwrap()),
             size_of_optional_header: u16!(0),
             characteristics: u16!(if self.is_64_bit() {
                 0
@@ -510,7 +517,6 @@ impl<'a> ObjectFactory<'a> {
                     | IMAGE_SCN_CNT_INITIALIZED_DATA
                     | IMAGE_SCN_MEM_READ
                     | IMAGE_SCN_MEM_WRITE
-                    | if comdat { IMAGE_SCN_LNK_COMDAT } else { 0 }
             ),
         }];
         buffer.write_all(bytes_of(&section_table))?;
@@ -526,41 +532,19 @@ impl<'a> ObjectFactory<'a> {
         buffer.write_all(bytes_of(&import_descriptor))?;
 
         // Symbol Table
-        if comdat {
-            let symbol = ImageSymbol {
-                name: *b".idata$3",
-                value: u32!(0),
-                section_number: u16!(1),
-                typ: u16!(0),
-                storage_class: IMAGE_SYM_CLASS_STATIC,
-                number_of_aux_symbols: 1,
-            };
-            let aux = ImageAuxSymbolSection {
-                length: u32!(size_of::<ImageImportDescriptor>().try_into().unwrap()),
-                number_of_relocations: u16!(0),
-                number_of_linenumbers: u16!(0),
-                check_sum: u32!(0),
-                selection: IMAGE_COMDAT_SELECT_ANY,
-                number: u16!(0),
-                reserved: 0,
-                high_number: u16!(0),
-            };
-            buffer.write_all(bytes_of(&symbol))?;
-            buffer.write_all(bytes_of(&aux))?;
-        }
-        let mut null_descriptor_symbol = ImageSymbol {
+        let mut symbol_table: [_; NUMBER_OF_SYMBOLS] = [ImageSymbol {
             name: [0; 8],
             value: u32!(0),
             section_number: u16!(1),
             typ: u16!(0),
             storage_class: IMAGE_SYM_CLASS_EXTERNAL,
             number_of_aux_symbols: 0,
-        };
-        set_name_to_string_table_entry(&mut null_descriptor_symbol, size_of::<u32>());
-        buffer.write_all(bytes_of(&null_descriptor_symbol))?;
+        }];
+        set_name_to_string_table_entry(&mut symbol_table[0], size_of::<u32>());
+        buffer.write_all(bytes_of(&symbol_table))?;
 
         // String Table
-        write_string_table(&mut buffer, &[NULL_IMPORT_DESCRIPTOR_SYMBOL_NAME])?;
+        write_string_table(&mut buffer, &[&self.null_import_descriptor_symbol_name])?;
 
         Ok(NewArchiveMember::new(
             buffer.into_boxed_slice(),
@@ -853,20 +837,20 @@ pub fn write_import_library<W: Write + Seek>(
     exports: &[COFFShortExport],
     machine: MachineTypes,
     mingw: bool,
-    comdat: bool,
+    whole_archive_compat: bool,
 ) -> Result<()> {
     let native_machine = if machine == MachineTypes::ARM64EC {
         MachineTypes::ARM64
     } else {
         machine
     };
 
-    let of = ObjectFactory::new(import_name, native_machine)?;
+    let of = ObjectFactory::new(import_name, native_machine, whole_archive_compat)?;
     let mut members = Vec::new();
 
     members.push(of.create_import_descriptor()?);
 
-    members.push(of.create_null_import_descriptor(comdat)?);
+    members.push(of.create_null_import_descriptor()?);
 
     members.push(of.create_null_thunk()?);
 

tests/import_library.rs

@@ -5,10 +5,10 @@ use std::process::Command;
 
 use ar_archive_writer::{ArchiveKind, COFFShortExport, MachineTypes};
 use common::{create_archive_with_ar_archive_writer, create_archive_with_llvm_ar};
-use object::coff::CoffHeader;
+use object::coff::CoffFile;
 use object::pe::ImageFileHeader;
 use object::read::archive::ArchiveFile;
-use object::{bytes_of, Architecture, Object, SubArchitecture, U32Bytes};
+use object::{bytes_of, Architecture, Object, ObjectSection, ObjectSymbol, SubArchitecture};
 use pretty_assertions::assert_eq;
 
 mod common;
@@ -305,43 +305,45 @@ fn compare_comdat(archive_writer_bytes: &[u8], llvm_bytes: &[u8]) {
         let archive_member = archive_member.unwrap();
         let llvm_member = llvm_member.unwrap();
 
+        // skip the EC symbols table.
+        if archive_member.name() == b"/<ECSYMBOLS>/" {
+            continue;
+        }
+
         if archive_member.size() != llvm_member.size() {
             // Ensure that the member header is the same except for the file size.
             let mut llvm_file_header = *llvm_member.header().unwrap();
-            llvm_file_header.size = *b"163       ";
+            llvm_file_header.size = archive_member.header().unwrap().size;
             assert_eq!(
                 bytes_of(archive_member.header().unwrap()),
                 bytes_of(&llvm_file_header)
             );
 
-            // Ensure that the LLVM generated object contains .idata$3
-            object::File::parse(llvm_member.data(llvm_bytes).unwrap())
-                .unwrap()
-                .section_by_name_bytes(b".idata$3")
-                .unwrap();
-
-            // Ensure the COFF file headers are the same except for the symbol count.
-            let llvm_data = llvm_member.data(llvm_bytes).unwrap();
+            // Make sure they are both COFF files with the same sections and symbols,
+            // except for the different naming for the null import descriptor.
             let archive_data = archive_member.data(archive_writer_bytes).unwrap();
-            let mut offset = 0;
-            let mut header = *ImageFileHeader::parse(llvm_data, &mut offset).unwrap();
-            header.number_of_symbols = U32Bytes::from_bytes(3_u32.to_le_bytes());
-            assert_eq!(
-                &archive_data[..size_of::<ImageFileHeader>()],
-                bytes_of(&header)
-            );
+            let llvm_data = llvm_member.data(llvm_bytes).unwrap();
+            let archive_file = CoffFile::<_, ImageFileHeader>::parse(archive_data).unwrap();
+            let llvm_file = CoffFile::<_, ImageFileHeader>::parse(llvm_data).unwrap();
 
-            // The rest of the object file will always be the same as `expected`
-            // for all import files no matter the platform.
-            let expected = [
-                46, 105, 100, 97, 116, 97, 36, 51, 0, 0, 0, 0, 0, 0, 0, 0, 20, 0, 0, 0, 60, 0, 0,
-                0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 64, 16, 48, 192, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-                0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 46, 105, 100, 97, 116, 97, 36, 51, 0, 0, 0, 0, 1,
-                0, 0, 0, 3, 1, 20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0,
-                4, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 2, 0, 29, 0, 0, 0, 95, 95, 78, 85, 76, 76, 95,
-                73, 77, 80, 79, 82, 84, 95, 68, 69, 83, 67, 82, 73, 80, 84, 79, 82, 0,
-            ];
-            assert_eq!(&archive_data[size_of::<ImageFileHeader>()..], &expected);
+            for (archive_section, llvm_section) in archive_file.sections().zip(llvm_file.sections())
+            {
+                assert_eq!(archive_section.data(), llvm_section.data());
+            }
+            for (archive_symbol, llvm_symbol) in archive_file.symbols().zip(llvm_file.symbols()) {
+                if llvm_symbol.name().unwrap() == "__NULL_IMPORT_DESCRIPTOR" {
+                    assert!(archive_symbol
+                        .name()
+                        .unwrap()
+                        .starts_with("__NULL_IMPORT_DESCRIPTOR_"));
+                } else {
+                    assert_eq!(archive_symbol.name(), llvm_symbol.name());
+                }
+                let archive_coff_symbol = archive_symbol.coff_symbol();
+                let mut llvm_coff_symbol = *llvm_symbol.coff_symbol();
+                llvm_coff_symbol.name = archive_coff_symbol.name;
+                assert_eq!(bytes_of(archive_coff_symbol), bytes_of(&llvm_coff_symbol));
+            }
         } else {
             assert_eq!(
                 bytes_of(archive_member.header().unwrap()),