read: use SectionIndex/SymbolIndex in low level API

Note that many APIs returned unvalidated indices. Validation is performed when they are used to access a table entry. For cases where a 0 index is normal, an Option may be returned.
gimli-rs · May 9, 2024 · f270452 · f270452
1 parent a739147
commit f270452
Show file tree

Hide file tree

Showing 23 changed files with 351 additions and 210 deletions.
diff --git a/crates/examples/src/bin/pecopy.rs b/crates/examples/src/bin/pecopy.rs
@@ -98,11 +98,11 @@ fn copy_file<Pe: ImageNtHeaders>(in_data: &[u8]) -> Result<Vec<u8>, Box<dyn Erro
     // Determine which sections to copy.
     // We ignore any existing ".reloc" section since we recreate it ourselves.
     let mut in_sections_index = Vec::new();
-    for (index, in_section) in in_sections.iter().enumerate() {
+    for (index, in_section) in in_sections.enumerate() {
         if reloc_dir == Some(in_section.pe_address_range()) {
             continue;
         }
-        in_sections_index.push(index + 1);
+        in_sections_index.push(index);
     }
 
     let mut out_sections_len = in_sections_index.len();

diff --git a/crates/examples/src/objdump.rs b/crates/examples/src/objdump.rs
@@ -183,21 +183,21 @@ fn dump_parsed_object<W: Write, E: Write>(w: &mut W, e: &mut E, file: &object::F
     }
 
     for section in file.sections() {
-        writeln!(w, "{}: {:x?}", section.index().0, section)?;
+        writeln!(w, "{}: {:x?}", section.index(), section)?;
     }
 
     for comdat in file.comdats() {
         write!(w, "{:?} Sections:", comdat)?;
         for section in comdat.sections() {
-            write!(w, " {}", section.0)?;
+            write!(w, " {}", section)?;
         }
         writeln!(w)?;
     }
 
     writeln!(w)?;
     writeln!(w, "Symbols")?;
     for symbol in file.symbols() {
-        writeln!(w, "{}: {:x?}", symbol.index().0, symbol)?;
+        writeln!(w, "{}: {:x?}", symbol.index(), symbol)?;
     }
 
     for section in file.sections() {
@@ -216,7 +216,7 @@ fn dump_parsed_object<W: Write, E: Write>(w: &mut W, e: &mut E, file: &object::F
     writeln!(w)?;
     writeln!(w, "Dynamic symbols")?;
     for symbol in file.dynamic_symbols() {
-        writeln!(w, "{}: {:x?}", symbol.index().0, symbol)?;
+        writeln!(w, "{}: {:x?}", symbol.index(), symbol)?;
     }
 
     if let Some(relocations) = file.dynamic_relocations() {

diff --git a/crates/examples/src/readobj/elf.rs b/crates/examples/src/readobj/elf.rs
@@ -1,7 +1,7 @@
 use super::*;
 use object::elf::*;
 use object::read::elf::*;
-use object::read::{SectionIndex, StringTable};
+use object::read::{SectionIndex, StringTable, SymbolIndex};
 
 pub(super) fn print_elf32(p: &mut Printer<'_>, data: &[u8]) {
     if let Some(elf) = FileHeader32::<Endianness>::parse(data).print_err(p) {
@@ -239,7 +239,7 @@ fn print_section_headers<Elf: FileHeader>(
     elf: &Elf,
     sections: &SectionTable<Elf>,
 ) {
-    for (index, section) in sections.iter().enumerate() {
+    for (index, section) in sections.enumerate() {
         let sh_type = section.sh_type(endian);
         if !p.options.sections
             && !(p.options.symbols && sh_type == SHT_SYMTAB)
@@ -255,7 +255,6 @@ fn print_section_headers<Elf: FileHeader>(
         {
             continue;
         }
-        let index = SectionIndex(index);
         p.group("SectionHeader", |p| {
             p.field("Index", index.0);
             p.field_string(
@@ -394,10 +393,10 @@ fn print_section_symbols<Elf: FileHeader>(
             EM_PARISC => FLAGS_SHN_PARISC,
             _ => &[],
         };
-        for (index, symbol) in symbols.iter().enumerate() {
+        for (index, symbol) in symbols.enumerate() {
             p.group("Symbol", |p| {
-                p.field("Index", index);
-                if index == 0 {
+                p.field("Index", index.0);
+                if index == SymbolIndex(0) {
                     p.field_hex("Name", symbol.st_name(endian));
                 } else {
                     p.field_string(
@@ -520,7 +519,7 @@ fn print_rel_symbol<Elf: FileHeader>(
     }
     let name = symbols.and_then(|symbols| {
         symbols
-            .symbol(sym as usize)
+            .symbol(SymbolIndex(sym as usize))
             .and_then(|symbol| symbol.name(endian, symbols.strings()))
             .print_err(p)
     });
@@ -760,7 +759,7 @@ fn print_hash<Elf: FileHeader>(
     if let Ok(Some((hash_table, link))) = section.hash(endian, data) {
         if let Ok(symbols) = _sections.symbol_table_by_index(endian, data, link) {
             if let Ok(versions) = _sections.versions(endian, data) {
-                for (index, symbol) in symbols.symbols().iter().enumerate() {
+                for (index, symbol) in symbols.symbols().enumerate() {
                     let name = symbols.symbol_name(endian, symbol).unwrap();
                     if name.is_empty() {
                         continue;
@@ -802,7 +801,6 @@ fn print_gnu_hash<Elf: FileHeader>(
             if let Ok(versions) = _sections.versions(endian, data) {
                 for (index, symbol) in symbols
                     .symbols()
-                    .iter()
                     .enumerate()
                     .skip(hash_table.symbol_base() as usize)
                 {

diff --git a/crates/examples/src/readobj/pe.rs b/crates/examples/src/readobj/pe.rs
@@ -3,6 +3,7 @@ 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};
 
@@ -643,7 +644,7 @@ fn print_relocations<'data, Coff: CoffHeader>(
                 let index = relocation.symbol_table_index.get(LE);
                 let name = symbols.and_then(|symbols| {
                     symbols
-                        .symbol(index as usize)
+                        .symbol(SymbolIndex(index as usize))
                         .and_then(|symbol| symbol.name(symbols.strings()))
                         .print_err(p)
                 });
@@ -714,7 +715,7 @@ fn print_symbols<'data, Coff: CoffHeader>(
             } else {
                 let section_name = sections.and_then(|sections| {
                     sections
-                        .section(section as usize)
+                        .section(SectionIndex(section as usize))
                         .and_then(|section| section.name(symbols.strings()))
                         .print_err(p)
                 });

diff --git a/crates/examples/src/readobj/xcoff.rs b/crates/examples/src/readobj/xcoff.rs
@@ -1,6 +1,6 @@
 use super::*;
 use object::read::xcoff::*;
-use object::read::SectionIndex;
+use object::read::{SectionIndex, SymbolIndex};
 use object::xcoff::*;
 
 pub(super) fn print_xcoff32(p: &mut Printer<'_>, data: &[u8]) {
@@ -129,7 +129,7 @@ fn print_sections<'data, Xcoff: FileHeader>(
                         let index = relocation.r_symndx();
                         let name = symbols.and_then(|symbols| {
                             symbols
-                                .symbol(index as usize)
+                                .symbol(SymbolIndex(index as usize))
                                 .and_then(|symbol| symbol.name(symbols.strings()))
                                 .print_err(p)
                         });
@@ -188,7 +188,7 @@ fn print_symbols<'data, Xcoff: FileHeader>(
             p.field("NumberOfAuxSymbols", numaux);
             if symbol.has_aux_file() {
                 for i in 1..=numaux {
-                    if let Some(aux_file) = symbols.aux_file(index.0, i).print_err(p) {
+                    if let Some(aux_file) = symbols.aux_file(index, i).print_err(p) {
                         p.group("FileAux", |p| {
                             p.field("Index", index.0 + i);
                             let name = aux_file.fname(symbols.strings());
@@ -206,7 +206,7 @@ fn print_symbols<'data, Xcoff: FileHeader>(
                 }
             }
             if symbol.has_aux_csect() {
-                if let Some(aux_csect) = symbols.aux_csect(index.0, numaux).print_err(p) {
+                if let Some(aux_csect) = symbols.aux_csect(index, numaux).print_err(p) {
                     p.group("CsectAux", |p| {
                         p.field("Index", index.0 + numaux);
                         p.field_hex("SectionLength", aux_csect.x_scnlen());

diff --git a/src/build/elf.rs b/src/build/elf.rs
@@ -116,7 +116,7 @@ impl<'data> Builder<'data> {
         let header = Elf::parse(data)?;
         let endian = header.endian()?;
         let is_mips64el = header.is_mips64el(endian);
-        let shstrndx = header.shstrndx(endian, data)? as usize;
+        let section_strings_index = header.section_strings_index(endian, data)?;
         let segments = header.program_headers(endian, data)?;
         let sections = header.sections(endian, data)?;
         let symbols = sections.symbols(endian, data, elf::SHT_SYMTAB)?;
@@ -179,8 +179,8 @@ impl<'data> Builder<'data> {
             builder.load_align = 1;
         }
 
-        for (index, section) in sections.iter().enumerate().skip(1) {
-            let id = SectionId(index - 1);
+        for (index, section) in sections.enumerate().skip(1) {
+            let id = SectionId(index.0 - 1);
             let relocations = if let Some((rels, link)) = section.rel(endian, data)? {
                 Self::read_relocations(
                     index,
@@ -222,7 +222,7 @@ impl<'data> Builder<'data> {
                 | elf::SHT_PREINIT_ARRAY => SectionData::Data(section.data(endian, data)?.into()),
                 elf::SHT_REL | elf::SHT_RELA => relocations,
                 elf::SHT_SYMTAB => {
-                    if index == symbols.section().0 {
+                    if index == symbols.section() {
                         SectionData::Symbol
                     } else {
                         return Err(Error(format!(
@@ -232,7 +232,7 @@ impl<'data> Builder<'data> {
                     }
                 }
                 elf::SHT_SYMTAB_SHNDX => {
-                    if index == symbols.shndx_section().0 {
+                    if index == symbols.shndx_section() {
                         SectionData::SymbolSectionIndex
                     } else {
                         return Err(Error(format!(
@@ -242,7 +242,7 @@ impl<'data> Builder<'data> {
                     }
                 }
                 elf::SHT_DYNSYM => {
-                    if index == dynamic_symbols.section().0 {
+                    if index == dynamic_symbols.section() {
                         SectionData::DynamicSymbol
                     } else {
                         return Err(Error(format!(
@@ -252,11 +252,11 @@ impl<'data> Builder<'data> {
                     }
                 }
                 elf::SHT_STRTAB => {
-                    if index == symbols.string_section().0 {
+                    if index == symbols.string_section() {
                         SectionData::String
-                    } else if index == dynamic_symbols.string_section().0 {
+                    } else if index == dynamic_symbols.string_section() {
                         SectionData::DynamicString
-                    } else if index == shstrndx {
+                    } else if index == section_strings_index {
                         SectionData::SectionString
                     } else {
                         return Err(Error(format!(
@@ -372,7 +372,7 @@ impl<'data> Builder<'data> {
 
     #[allow(clippy::too_many_arguments)]
     fn read_relocations<Elf, Rel, R>(
-        index: usize,
+        index: read::SectionIndex,
         endian: Elf::Endian,
         is_mips64el: bool,
         section: &'data Elf::SectionHeader,
@@ -427,7 +427,7 @@ impl<'data> Builder<'data> {
     }
 
     fn read_relocations_impl<Elf, Rel, const DYNAMIC: bool>(
-        index: usize,
+        index: read::SectionIndex,
         endian: Elf::Endian,
         is_mips64el: bool,
         rels: &'data [Rel],
@@ -440,17 +440,16 @@ impl<'data> Builder<'data> {
         let mut relocations = Vec::new();
         for rel in rels {
             let rel = (*rel).into();
-            let r_sym = rel.r_sym(endian, is_mips64el);
-            let symbol = if r_sym == 0 {
-                None
-            } else {
-                if r_sym as usize >= symbols_len {
+            let symbol = if let Some(symbol) = rel.symbol(endian, is_mips64el) {
+                if symbol.0 >= symbols_len {
                     return Err(Error(format!(
                         "Invalid symbol index {} in relocation section at index {}",
-                        r_sym, index,
+                        symbol, index,
                     )));
                 }
-                Some(SymbolId(r_sym as usize - 1))
+                Some(SymbolId(symbol.0 - 1))
+            } else {
+                None
             };
             relocations.push(Relocation {
                 r_offset: rel.r_offset(endian).into(),
@@ -523,8 +522,8 @@ impl<'data> Builder<'data> {
         Elf: FileHeader<Endian = Endianness>,
         R: ReadRef<'data>,
     {
-        for (index, symbol) in symbols.iter().enumerate().skip(1) {
-            let id = SymbolId(index - 1);
+        for (index, symbol) in symbols.enumerate().skip(1) {
+            let id = SymbolId(index.0 - 1);
             let section =
                 if let Some(section_index) = symbols.symbol_section(endian, symbol, index)? {
                     let section_id = section_index.0.wrapping_sub(1);
@@ -553,7 +552,7 @@ impl<'data> Builder<'data> {
     }
 
     fn read_attributes<Elf>(
-        index: usize,
+        index: read::SectionIndex,
         attributes: read::elf::AttributesSection<'data, Elf>,
         sections_len: usize,
         symbols_len: usize,

diff --git a/src/read/coff/comdat.rs b/src/read/coff/comdat.rs
@@ -20,8 +20,17 @@ pub struct CoffComdatIterator<
     R: ReadRef<'data> = &'data [u8],
     Coff: CoffHeader = pe::ImageFileHeader,
 > {
-    pub(super) file: &'file CoffFile<'data, R, Coff>,
-    pub(super) index: usize,
+    file: &'file CoffFile<'data, R, Coff>,
+    index: SymbolIndex,
+}
+
+impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> CoffComdatIterator<'data, 'file, R, Coff> {
+    pub(crate) fn new(file: &'file CoffFile<'data, R, Coff>) -> Self {
+        CoffComdatIterator {
+            file,
+            index: SymbolIndex(0),
+        }
+    }
 }
 
 impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> Iterator
@@ -33,7 +42,7 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> Iterator
         loop {
             let index = self.index;
             let symbol = self.file.common.symbols.symbol(index).ok()?;
-            self.index += 1 + symbol.number_of_aux_symbols() as usize;
+            self.index.0 += 1 + symbol.number_of_aux_symbols() as usize;
             if let Some(comdat) = CoffComdat::parse(self.file, symbol, index) {
                 return Some(comdat);
             }
@@ -67,7 +76,7 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> CoffComdat<'data, 'file,
     fn parse(
         file: &'file CoffFile<'data, R, Coff>,
         section_symbol: &'data Coff::ImageSymbol,
-        index: usize,
+        index: SymbolIndex,
     ) -> Option<CoffComdat<'data, 'file, R, Coff>> {
         // Must be a section symbol.
         if !section_symbol.has_aux_section() {
@@ -86,7 +95,7 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> CoffComdat<'data, 'file,
         let mut symbol = section_symbol;
         let section_number = section_symbol.section_number();
         loop {
-            symbol_index += 1 + symbol.number_of_aux_symbols() as usize;
+            symbol_index.0 += 1 + symbol.number_of_aux_symbols() as usize;
             symbol = file.common.symbols.symbol(symbol_index).ok()?;
             if section_number == symbol.section_number() {
                 break;
@@ -95,7 +104,7 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> CoffComdat<'data, 'file,
 
         Some(CoffComdat {
             file,
-            symbol_index: SymbolIndex(symbol_index),
+            symbol_index,
             symbol,
             selection,
         })
@@ -149,7 +158,7 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> ObjectComdat<'data>
         CoffComdatSectionIterator {
             file: self.file,
             section_number: self.symbol.section_number(),
-            index: 0,
+            index: SymbolIndex(0),
         }
     }
 }
@@ -168,7 +177,7 @@ pub struct CoffComdatSectionIterator<
 > {
     file: &'file CoffFile<'data, R, Coff>,
     section_number: i32,
-    index: usize,
+    index: SymbolIndex,
 }
 
 impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> Iterator
@@ -182,7 +191,7 @@ impl<'data, 'file, R: ReadRef<'data>, Coff: CoffHeader> Iterator
         loop {
             let index = self.index;
             let symbol = self.file.common.symbols.symbol(index).ok()?;
-            self.index += 1 + symbol.number_of_aux_symbols() as usize;
+            self.index.0 += 1 + symbol.number_of_aux_symbols() as usize;
 
             // Must be a section symbol.
             if !symbol.has_aux_section() {