write/object: adjust addend using relocation flags

Previously we adjusted the addend based on the relocation kind. For cases where the flags are derived from the relocation kind, this should not change the behaviour. For cases where the user supplies the flags, this ensures the adjustment is consistent with the flags that are actually used (since previously the kind and flags did not need to agree). This is preparation for removing the relocation kind field when the user supplies the flags.
gimli-rs · Nov 10, 2023 · f37e1ea · f37e1ea
1 parent 2fc3c32
commit f37e1ea
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 73 deletions.
diff --git a/src/write/coff.rs b/src/write/coff.rs
@@ -168,31 +168,50 @@ impl<'a> Object<'a> {
         Ok(RelocationFlags::Coff { typ })
     }
 
-    pub(crate) fn coff_adjust_addend(&self, relocation: &mut Relocation) -> bool {
-        let addend = match self.architecture {
-            Architecture::I386 | Architecture::Arm | Architecture::Aarch64 => match relocation.kind
-            {
-                RelocationKind::Relative => {
-                    // IMAGE_REL_I386_REL32, IMAGE_REL_ARM_REL32, IMAGE_REL_ARM64_REL32
-                    relocation.addend + 4
+    pub(crate) fn coff_adjust_addend(
+        &self,
+        relocation: &mut crate::write::RawRelocation,
+    ) -> Result<bool> {
+        let typ = if let RelocationFlags::Coff { typ } = relocation.flags {
+            typ
+        } else {
+            return Err(Error(format!("invalid relocation flags {:?}", relocation)));
+        };
+        let offset = match self.architecture {
+            Architecture::Arm => {
+                if typ == coff::IMAGE_REL_ARM_REL32 {
+                    4
+                } else {
+                    0
                 }
-                _ => relocation.addend,
-            },
-            Architecture::X86_64 => match relocation.kind {
-                RelocationKind::Relative => {
-                    // IMAGE_REL_AMD64_REL32 through to IMAGE_REL_AMD64_REL32_5
-                    if relocation.addend <= -4 && relocation.addend >= -9 {
-                        0
-                    } else {
-                        relocation.addend + 4
-                    }
+            }
+            Architecture::Aarch64 => {
+                if typ == coff::IMAGE_REL_ARM64_REL32 {
+                    4
+                } else {
+                    0
+                }
+            }
+            Architecture::I386 => {
+                if typ == coff::IMAGE_REL_I386_REL32 {
+                    4
+                } else {
+                    0
                 }
-                _ => relocation.addend,
+            }
+            Architecture::X86_64 => match typ {
+                coff::IMAGE_REL_AMD64_REL32 => 4,
+                coff::IMAGE_REL_AMD64_REL32_1 => 5,
+                coff::IMAGE_REL_AMD64_REL32_2 => 6,
+                coff::IMAGE_REL_AMD64_REL32_3 => 7,
+                coff::IMAGE_REL_AMD64_REL32_4 => 8,
+                coff::IMAGE_REL_AMD64_REL32_5 => 9,
+                _ => 0,
             },
-            _ => unimplemented!(),
+            _ => return Err(Error(format!("unimplemented relocation {:?}", relocation))),
         };
-        relocation.addend = addend;
-        true
+        relocation.addend += offset;
+        Ok(true)
     }
 
     pub(crate) fn coff_relocation_size(&self, reloc: &crate::write::RawRelocation) -> Result<u8> {

diff --git a/src/write/elf/object.rs b/src/write/elf/object.rs
@@ -416,7 +416,10 @@ impl<'a> Object<'a> {
         Ok(RelocationFlags::Elf { r_type })
     }
 
-    pub(crate) fn elf_adjust_addend(&mut self, _relocation: &mut Relocation) -> Result<bool> {
+    pub(crate) fn elf_adjust_addend(
+        &mut self,
+        _relocation: &mut crate::write::RawRelocation,
+    ) -> Result<bool> {
         // Determine whether the addend is stored in the relocation or the data.
         let implicit = !self.elf_has_relocation_addend()?;
         Ok(implicit)

diff --git a/src/write/macho.rs b/src/write/macho.rs
@@ -331,58 +331,49 @@ impl<'a> Object<'a> {
         })
     }
 
-    pub(crate) fn macho_adjust_addend(&mut self, relocation: &mut Relocation) -> bool {
-        let relative = match relocation.kind {
-            RelocationKind::Relative
-            | RelocationKind::GotRelative
-            | RelocationKind::PltRelative
-            | RelocationKind::MachO { relative: true, .. } => true,
-            _ => false,
+    pub(crate) fn macho_adjust_addend(
+        &mut self,
+        relocation: &mut crate::write::RawRelocation,
+    ) -> Result<bool> {
+        let (r_type, r_pcrel) = if let RelocationFlags::MachO {
+            r_type, r_pcrel, ..
+        } = relocation.flags
+        {
+            (r_type, r_pcrel)
+        } else {
+            return Err(Error(format!("invalid relocation flags {:?}", relocation)));
         };
-        if relative {
+        if r_pcrel {
             // For PC relative relocations on some architectures, the
             // addend does not include the offset required due to the
             // PC being different from the place of the relocation.
             // This differs from other file formats, so adjust the
             // addend here to account for this.
             let pcrel_offset = match self.architecture {
                 Architecture::I386 => 4,
-                Architecture::X86_64 => match relocation.kind {
-                    RelocationKind::MachO {
-                        value: macho::X86_64_RELOC_SIGNED_1,
-                        ..
-                    } => 5,
-                    RelocationKind::MachO {
-                        value: macho::X86_64_RELOC_SIGNED_2,
-                        ..
-                    } => 6,
-                    RelocationKind::MachO {
-                        value: macho::X86_64_RELOC_SIGNED_4,
-                        ..
-                    } => 8,
+                Architecture::X86_64 => match r_type {
+                    macho::X86_64_RELOC_SIGNED_1 => 5,
+                    macho::X86_64_RELOC_SIGNED_2 => 6,
+                    macho::X86_64_RELOC_SIGNED_4 => 8,
                     _ => 4,
                 },
                 // TODO: maybe missing support for some architectures and relocations
                 _ => 0,
             };
             relocation.addend += pcrel_offset;
         }
-        // Check for relocations that use an explicit addend.
-        if self.architecture == Architecture::Aarch64 {
-            if relocation.encoding == RelocationEncoding::AArch64Call {
-                return false;
-            }
-            if let RelocationKind::MachO { value, .. } = relocation.kind {
-                match value {
-                    macho::ARM64_RELOC_BRANCH26
-                    | macho::ARM64_RELOC_PAGE21
-                    | macho::ARM64_RELOC_PAGEOFF12 => return false,
-                    _ => {}
-                }
+        // Determine if addend is implicit.
+        let implicit = if self.architecture == Architecture::Aarch64 {
+            match r_type {
+                macho::ARM64_RELOC_BRANCH26
+                | macho::ARM64_RELOC_PAGE21
+                | macho::ARM64_RELOC_PAGEOFF12 => false,
+                _ => true,
             }
-        }
-        // Signify implicit addend.
-        true
+        } else {
+            true
+        };
+        Ok(implicit)
     }
 
     pub(crate) fn macho_relocation_size(&self, reloc: &crate::write::RawRelocation) -> Result<u8> {

diff --git a/src/write/mod.rs b/src/write/mod.rs
@@ -541,23 +541,23 @@ impl<'a> Object<'a> {
             BinaryFormat::Xcoff => self.xcoff_relocation_flags(&relocation)?,
             _ => unimplemented!(),
         };
+        let mut relocation = RawRelocation {
+            offset: relocation.offset,
+            symbol: relocation.symbol,
+            addend: relocation.addend,
+            flags,
+        };
         let implicit = match self.format {
             #[cfg(feature = "coff")]
-            BinaryFormat::Coff => self.coff_adjust_addend(&mut relocation),
+            BinaryFormat::Coff => self.coff_adjust_addend(&mut relocation)?,
             #[cfg(feature = "elf")]
             BinaryFormat::Elf => self.elf_adjust_addend(&mut relocation)?,
             #[cfg(feature = "macho")]
-            BinaryFormat::MachO => self.macho_adjust_addend(&mut relocation),
+            BinaryFormat::MachO => self.macho_adjust_addend(&mut relocation)?,
             #[cfg(feature = "xcoff")]
-            BinaryFormat::Xcoff => self.xcoff_adjust_addend(&mut relocation),
+            BinaryFormat::Xcoff => self.xcoff_adjust_addend(&mut relocation)?,
             _ => unimplemented!(),
         };
-        let mut relocation = RawRelocation {
-            offset: relocation.offset,
-            symbol: relocation.symbol,
-            addend: relocation.addend,
-            flags,
-        };
         if implicit && relocation.addend != 0 {
             self.write_relocation_addend(section, &relocation)?;
             relocation.addend = 0;

diff --git a/src/write/xcoff.rs b/src/write/xcoff.rs
@@ -82,13 +82,19 @@ impl<'a> Object<'a> {
         Ok(RelocationFlags::Xcoff { r_rtype, r_rsize })
     }
 
-    pub(crate) fn xcoff_adjust_addend(&mut self, relocation: &mut Relocation) -> bool {
-        let addend = match relocation.kind {
-            RelocationKind::Relative => relocation.addend + 4,
-            _ => relocation.addend,
+    pub(crate) fn xcoff_adjust_addend(
+        &mut self,
+        relocation: &mut crate::write::RawRelocation,
+    ) -> Result<bool> {
+        let r_rtype = if let RelocationFlags::Xcoff { r_rtype, .. } = relocation.flags {
+            r_rtype
+        } else {
+            return Err(Error(format!("invalid relocation flags {:?}", relocation)));
         };
-        relocation.addend = addend;
-        true
+        if r_rtype == xcoff::R_REL {
+            relocation.addend += 4;
+        }
+        Ok(true)
     }
 
     pub(crate) fn xcoff_relocation_size(&self, reloc: &crate::write::RawRelocation) -> Result<u8> {