write/coff: Section::flags should override flags due Section::kind (

#412) This matches ELF and Mach-O behaviour.
gimli-rs · Dec 20, 2021 · 56299a3 · 56299a3
1 parent f9c8b3f
commit 56299a3
Show file tree

Hide file tree

Showing 4 changed files with 139 additions and 46 deletions.
diff --git a/src/write/coff.rs b/src/write/coff.rs
@@ -295,58 +295,60 @@ impl<'a> Object<'a> {
 
         // Write section headers.
         for (index, section) in self.sections.iter().enumerate() {
-            let mut characteristics = match section.flags {
-                SectionFlags::Coff {
-                    characteristics, ..
-                } => characteristics,
-                _ => 0,
+            let mut characteristics = if let SectionFlags::Coff {
+                characteristics, ..
+            } = section.flags
+            {
+                characteristics
+            } else {
+                match section.kind {
+                    SectionKind::Text => {
+                        coff::IMAGE_SCN_CNT_CODE
+                            | coff::IMAGE_SCN_MEM_EXECUTE
+                            | coff::IMAGE_SCN_MEM_READ
+                    }
+                    SectionKind::Data => {
+                        coff::IMAGE_SCN_CNT_INITIALIZED_DATA
+                            | coff::IMAGE_SCN_MEM_READ
+                            | coff::IMAGE_SCN_MEM_WRITE
+                    }
+                    SectionKind::UninitializedData => {
+                        coff::IMAGE_SCN_CNT_UNINITIALIZED_DATA
+                            | coff::IMAGE_SCN_MEM_READ
+                            | coff::IMAGE_SCN_MEM_WRITE
+                    }
+                    SectionKind::ReadOnlyData | SectionKind::ReadOnlyString => {
+                        coff::IMAGE_SCN_CNT_INITIALIZED_DATA | coff::IMAGE_SCN_MEM_READ
+                    }
+                    SectionKind::Debug | SectionKind::Other | SectionKind::OtherString => {
+                        coff::IMAGE_SCN_CNT_INITIALIZED_DATA
+                            | coff::IMAGE_SCN_MEM_READ
+                            | coff::IMAGE_SCN_MEM_DISCARDABLE
+                    }
+                    SectionKind::Linker => coff::IMAGE_SCN_LNK_INFO | coff::IMAGE_SCN_LNK_REMOVE,
+                    SectionKind::Common
+                    | SectionKind::Tls
+                    | SectionKind::UninitializedTls
+                    | SectionKind::TlsVariables
+                    | SectionKind::Note
+                    | SectionKind::Unknown
+                    | SectionKind::Metadata
+                    | SectionKind::Elf(_) => {
+                        return Err(Error(format!(
+                            "unimplemented section `{}` kind {:?}",
+                            section.name().unwrap_or(""),
+                            section.kind
+                        )));
+                    }
+                }
             };
             if section_offsets[index].selection != 0 {
                 characteristics |= coff::IMAGE_SCN_LNK_COMDAT;
             };
             if section.relocations.len() > 0xffff {
                 characteristics |= coff::IMAGE_SCN_LNK_NRELOC_OVFL;
             }
-            characteristics |= match section.kind {
-                SectionKind::Text => {
-                    coff::IMAGE_SCN_CNT_CODE
-                        | coff::IMAGE_SCN_MEM_EXECUTE
-                        | coff::IMAGE_SCN_MEM_READ
-                }
-                SectionKind::Data => {
-                    coff::IMAGE_SCN_CNT_INITIALIZED_DATA
-                        | coff::IMAGE_SCN_MEM_READ
-                        | coff::IMAGE_SCN_MEM_WRITE
-                }
-                SectionKind::UninitializedData => {
-                    coff::IMAGE_SCN_CNT_UNINITIALIZED_DATA
-                        | coff::IMAGE_SCN_MEM_READ
-                        | coff::IMAGE_SCN_MEM_WRITE
-                }
-                SectionKind::ReadOnlyData | SectionKind::ReadOnlyString => {
-                    coff::IMAGE_SCN_CNT_INITIALIZED_DATA | coff::IMAGE_SCN_MEM_READ
-                }
-                SectionKind::Debug | SectionKind::Other | SectionKind::OtherString => {
-                    coff::IMAGE_SCN_CNT_INITIALIZED_DATA
-                        | coff::IMAGE_SCN_MEM_READ
-                        | coff::IMAGE_SCN_MEM_DISCARDABLE
-                }
-                SectionKind::Linker => coff::IMAGE_SCN_LNK_INFO | coff::IMAGE_SCN_LNK_REMOVE,
-                SectionKind::Common
-                | SectionKind::Tls
-                | SectionKind::UninitializedTls
-                | SectionKind::TlsVariables
-                | SectionKind::Note
-                | SectionKind::Unknown
-                | SectionKind::Metadata
-                | SectionKind::Elf(_) => {
-                    return Err(Error(format!(
-                        "unimplemented section `{}` kind {:?}",
-                        section.name().unwrap_or(""),
-                        section.kind
-                    )));
-                }
-            } | match section.align {
+            characteristics |= match section.align {
                 1 => coff::IMAGE_SCN_ALIGN_1BYTES,
                 2 => coff::IMAGE_SCN_ALIGN_2BYTES,
                 4 => coff::IMAGE_SCN_ALIGN_4BYTES,

diff --git a/tests/round_trip/comdat.rs b/tests/round_trip/comdat.rs
@@ -128,7 +128,7 @@ fn coff_x86_64_comdat() {
 }
 
 #[test]
-fn elf_x86_64_common() {
+fn elf_x86_64_comdat() {
     let mut object =
         write::Object::new(BinaryFormat::Elf, Architecture::X86_64, Endianness::Little);
 

diff --git a/tests/round_trip/mod.rs b/tests/round_trip/mod.rs
@@ -13,6 +13,7 @@ mod comdat;
 mod common;
 mod elf;
 mod macho;
+mod section_flags;
 mod tls;
 
 #[test]

diff --git a/tests/round_trip/section_flags.rs b/tests/round_trip/section_flags.rs
@@ -0,0 +1,90 @@
+#![cfg(all(feature = "read", feature = "write"))]
+
+use object::read::{Object, ObjectSection};
+use object::{read, write};
+use object::{Architecture, BinaryFormat, Endianness, SectionFlags, SectionKind};
+
+#[test]
+fn coff_x86_64_section_flags() {
+    let mut object =
+        write::Object::new(BinaryFormat::Coff, Architecture::X86_64, Endianness::Little);
+
+    let section = object.add_section(Vec::new(), b".text".to_vec(), SectionKind::Text);
+    object.section_mut(section).flags = SectionFlags::Coff {
+        characteristics: object::pe::IMAGE_SCN_MEM_WRITE,
+    };
+
+    let bytes = object.write().unwrap();
+
+    let object = read::File::parse(&*bytes).unwrap();
+    assert_eq!(object.format(), BinaryFormat::Coff);
+    assert_eq!(object.architecture(), Architecture::X86_64);
+
+    let mut sections = object.sections();
+    let section = sections.next().unwrap();
+    assert_eq!(section.name(), Ok(".text"));
+    assert_eq!(
+        section.flags(),
+        SectionFlags::Coff {
+            characteristics: object::pe::IMAGE_SCN_MEM_WRITE | object::pe::IMAGE_SCN_ALIGN_1BYTES,
+        }
+    );
+}
+
+#[test]
+fn elf_x86_64_section_flags() {
+    let mut object =
+        write::Object::new(BinaryFormat::Elf, Architecture::X86_64, Endianness::Little);
+
+    let section = object.add_section(Vec::new(), b".text".to_vec(), SectionKind::Text);
+    object.section_mut(section).flags = SectionFlags::Elf {
+        sh_flags: object::elf::SHF_WRITE.into(),
+    };
+
+    let bytes = object.write().unwrap();
+
+    let object = read::File::parse(&*bytes).unwrap();
+    assert_eq!(object.format(), BinaryFormat::Elf);
+    assert_eq!(object.architecture(), Architecture::X86_64);
+
+    let mut sections = object.sections();
+    sections.next().unwrap();
+    let section = sections.next().unwrap();
+    assert_eq!(section.name(), Ok(".text"));
+    assert_eq!(
+        section.flags(),
+        SectionFlags::Elf {
+            sh_flags: object::elf::SHF_WRITE.into(),
+        }
+    );
+}
+
+#[test]
+fn macho_x86_64_section_flags() {
+    let mut object = write::Object::new(
+        BinaryFormat::MachO,
+        Architecture::X86_64,
+        Endianness::Little,
+    );
+
+    let section = object.add_section(Vec::new(), b".text".to_vec(), SectionKind::Text);
+    object.section_mut(section).flags = SectionFlags::MachO {
+        flags: object::macho::S_ATTR_SELF_MODIFYING_CODE,
+    };
+
+    let bytes = object.write().unwrap();
+
+    let object = read::File::parse(&*bytes).unwrap();
+    assert_eq!(object.format(), BinaryFormat::MachO);
+    assert_eq!(object.architecture(), Architecture::X86_64);
+
+    let mut sections = object.sections();
+    let section = sections.next().unwrap();
+    assert_eq!(section.name(), Ok(".text"));
+    assert_eq!(
+        section.flags(),
+        SectionFlags::MachO {
+            flags: object::macho::S_ATTR_SELF_MODIFYING_CODE,
+        }
+    );
+}