read: add more helpers for SymbolIndex and SectionIndex (#689)

philipc · web-flow · commit 81767fde32a7 · 2024-05-25T14:12:54.000+10:00
diff --git a/crates/examples/src/readobj/pe.rs b/crates/examples/src/readobj/pe.rs
@@ -3,7 +3,6 @@ use object::pe::*;
 use object::read::coff::ImageSymbol as _;
 use object::read::coff::*;
 use object::read::pe::*;
-use object::read::{SectionIndex, SymbolIndex};
 use object::LittleEndian as LE;
 use object::{Bytes, U32Bytes, U64Bytes};
 
@@ -641,14 +640,14 @@ fn print_relocations<'data, Coff: CoffHeader>(
         for relocation in relocations {
             p.group("ImageRelocation", |p| {
                 p.field_hex("VirtualAddress", relocation.virtual_address.get(LE));
-                let index = relocation.symbol_table_index.get(LE);
+                let index = relocation.symbol();
                 let name = symbols.and_then(|symbols| {
                     symbols
-                        .symbol(SymbolIndex(index as usize))
+                        .symbol(index)
                         .and_then(|symbol| symbol.name(symbols.strings()))
                         .print_err(p)
                 });
-                p.field_string_option("Symbol", index, name);
+                p.field_string_option("Symbol", index.0, name);
                 let proc = match machine {
                     IMAGE_FILE_MACHINE_I386 => FLAGS_IMAGE_REL_I386,
                     IMAGE_FILE_MACHINE_MIPS16
@@ -709,17 +708,16 @@ fn print_symbols<'data, Coff: CoffHeader>(
                 p.field("Name", format!("{:X?}", symbol.raw_name()));
             }
             p.field_hex("Value", symbol.value());
-            let section = symbol.section_number();
-            if section <= 0 {
-                p.field_enum_display("Section", section, FLAGS_IMAGE_SYM);
-            } else {
+            if let Some(section_index) = symbol.section() {
                 let section_name = sections.and_then(|sections| {
                     sections
-                        .section(SectionIndex(section as usize))
+                        .section(section_index)
                         .and_then(|section| section.name(symbols.strings()))
                         .print_err(p)
                 });
-                p.field_string_option("Section", section, section_name);
+                p.field_string_option("Section", section_index.0, section_name);
+            } else {
+                p.field_enum_display("Section", symbol.section_number(), FLAGS_IMAGE_SYM);
             }
             p.field_hex("Type", symbol.typ());
             p.field_enum("BaseType", symbol.base_type(), FLAGS_IMAGE_SYM_TYPE);
diff --git a/crates/examples/src/readobj/xcoff.rs b/crates/examples/src/readobj/xcoff.rs
@@ -1,6 +1,5 @@
 use super::*;
 use object::read::xcoff::*;
-use object::read::{SectionIndex, SymbolIndex};
 use object::xcoff::*;
 
 pub(super) fn print_xcoff32(p: &mut Printer<'_>, data: &[u8]) {
@@ -126,14 +125,14 @@ fn print_sections<'data, Xcoff: FileHeader>(
                 for relocation in relocations {
                     p.group("Relocation", |p| {
                         p.field_hex("VirtualAddress", relocation.r_vaddr().into());
-                        let index = relocation.r_symndx();
+                        let index = relocation.symbol();
                         let name = symbols.and_then(|symbols| {
                             symbols
-                                .symbol(SymbolIndex(index as usize))
+                                .symbol(index)
                                 .and_then(|symbol| symbol.name(symbols.strings()))
                                 .print_err(p)
                         });
-                        p.field_string_option("Symbol", index, name);
+                        p.field_string_option("Symbol", index.0, name);
                         p.field_hex("Size", relocation.r_rsize());
                         p.field_enum("Type", relocation.r_rtype(), FLAGS_R);
                     });
@@ -161,17 +160,16 @@ fn print_symbols<'data, Xcoff: FileHeader>(
                 p.field_inline_string("Name", name);
             }
             p.field_hex("Value", symbol.n_value().into());
-            let section = symbol.n_scnum();
-            if section <= 0 {
-                p.field_enum_display("Section", section, FLAGS_N);
-            } else {
+            if let Some(section_index) = symbol.section() {
                 let section_name = sections.and_then(|sections| {
                     sections
-                        .section(SectionIndex(section as usize))
+                        .section(section_index)
                         .map(|section| section.name())
                         .print_err(p)
                 });
-                p.field_string_option("Section", section, section_name);
+                p.field_string_option("Section", section_index.0, section_name);
+            } else {
+                p.field_enum_display("Section", symbol.n_scnum(), FLAGS_N);
             }
             if symbol.n_sclass() == C_FILE {
                 p.field_hex("SourceLanguage", symbol.n_type() >> 8);
diff --git a/src/read/coff/relocation.rs b/src/read/coff/relocation.rs
@@ -80,9 +80,7 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> Iterator
                 },
                 _ => (RelocationKind::Unknown, 0, 0),
             };
-            let target = RelocationTarget::Symbol(SymbolIndex(
-                relocation.symbol_table_index.get(LE) as usize,
-            ));
+            let target = RelocationTarget::Symbol(relocation.symbol());
             (
                 u64::from(relocation.virtual_address.get(LE)),
                 Relocation {
@@ -106,3 +104,10 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> fmt::Debug
         f.debug_struct("CoffRelocationIterator").finish()
     }
 }
+
+impl pe::ImageRelocation {
+    /// Get the index of the symbol referenced by this relocation.
+    pub fn symbol(&self) -> SymbolIndex {
+        SymbolIndex(self.symbol_table_index.get(LE) as usize)
+    }
+}
diff --git a/src/read/coff/symbol.rs b/src/read/coff/symbol.rs
@@ -357,21 +357,9 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> ObjectSymbol<'data>
     }
 
     fn address(&self) -> u64 {
-        // Only return an address for storage classes that we know use an address.
-        match self.symbol.storage_class() {
-            pe::IMAGE_SYM_CLASS_STATIC
-            | pe::IMAGE_SYM_CLASS_WEAK_EXTERNAL
-            | pe::IMAGE_SYM_CLASS_LABEL => {}
-            pe::IMAGE_SYM_CLASS_EXTERNAL => {
-                if self.symbol.section_number() == pe::IMAGE_SYM_UNDEFINED {
-                    // Undefined or common data, neither of which have an address.
-                    return 0;
-                }
-            }
-            _ => return 0,
-        }
         self.symbol
             .address(self.file.image_base, &self.file.sections)
+            .unwrap_or(None)
             .unwrap_or(0)
     }
 
@@ -565,13 +553,34 @@ pub trait ImageSymbol: Debug + Pod {
 
     /// Return the symbol address.
     ///
-    /// This takes into account the image base and the section address.
-    fn address(&self, image_base: u64, sections: &SectionTable<'_>) -> Result<u64> {
-        let section_number = SectionIndex(self.section_number() as usize);
-        let section = sections.section(section_number)?;
+    /// This takes into account the image base and the section address,
+    /// and only returns an address for symbols that have an address.
+    fn address(&self, image_base: u64, sections: &SectionTable<'_>) -> Result<Option<u64>> {
+        // Only return an address for storage classes that we know use an address.
+        match self.storage_class() {
+            pe::IMAGE_SYM_CLASS_STATIC
+            | pe::IMAGE_SYM_CLASS_WEAK_EXTERNAL
+            | pe::IMAGE_SYM_CLASS_LABEL
+            | pe::IMAGE_SYM_CLASS_EXTERNAL => {}
+            _ => return Ok(None),
+        }
+        let Some(section_index) = self.section() else {
+            return Ok(None);
+        };
+        let section = sections.section(section_index)?;
         let virtual_address = u64::from(section.virtual_address.get(LE));
         let value = u64::from(self.value());
-        Ok(image_base + virtual_address + value)
+        Ok(Some(image_base + virtual_address + value))
+    }
+
+    /// Return the section index for the symbol.
+    fn section(&self) -> Option<SectionIndex> {
+        let section_number = self.section_number();
+        if section_number > 0 {
+            Some(SectionIndex(section_number as usize))
+        } else {
+            None
+        }
     }
 
     /// Return true if the symbol is a definition of a function or data object.
diff --git a/src/read/xcoff/relocation.rs b/src/read/xcoff/relocation.rs
@@ -56,7 +56,7 @@ where
                 _ => (RelocationKind::Unknown, 0),
             };
             let size = (r_rsize & 0x3F) + 1;
-            let target = RelocationTarget::Symbol(SymbolIndex(relocation.r_symndx() as usize));
+            let target = RelocationTarget::Symbol(relocation.symbol());
             (
                 relocation.r_vaddr().into(),
                 Relocation {
@@ -91,6 +91,10 @@ pub trait Rel: Debug + Pod {
     fn r_symndx(&self) -> u32;
     fn r_rsize(&self) -> u8;
     fn r_rtype(&self) -> u8;
+
+    fn symbol(&self) -> SymbolIndex {
+        SymbolIndex(self.r_symndx() as usize)
+    }
 }
 
 impl Rel for xcoff::Rel32 {
diff --git a/src/read/xcoff/symbol.rs b/src/read/xcoff/symbol.rs
@@ -552,6 +552,16 @@ pub trait Symbol: Debug + Pod {
         strings: StringTable<'data, R>,
     ) -> Result<&'data [u8]>;
 
+    /// Return the section index for the symbol.
+    fn section(&self) -> Option<SectionIndex> {
+        let index = self.n_scnum();
+        if index > 0 {
+            Some(SectionIndex(index as usize))
+        } else {
+            None
+        }
+    }
+
     /// Return true if the symbol is a null placeholder.
     #[inline]
     fn is_null(&self) -> bool {
