read: use SectionIndex/SymbolIndex in low level API (#684)

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 10, 2024 · 017624a · 017624a
1 parent a739147
commit 017624a
Show file tree

Hide file tree

Showing 23 changed files with 358 additions and 219 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(
@@ -466,7 +465,7 @@ fn print_section_rel<Elf: FileHeader>(
             p.group("Relocation", |p| {
                 p.field_hex("Offset", relocation.r_offset(endian).into());
                 p.field_enum("Type", relocation.r_type(endian), proc);
-                let sym = relocation.r_sym(endian);
+                let sym = relocation.symbol(endian);
                 print_rel_symbol(p, endian, symbols, sym);
             });
         }
@@ -497,7 +496,7 @@ fn print_section_rela<Elf: FileHeader>(
                     relocation.r_type(endian, elf.is_mips64el(endian)),
                     proc,
                 );
-                let sym = relocation.r_sym(endian, elf.is_mips64el(endian));
+                let sym = relocation.symbol(endian, elf.is_mips64el(endian));
                 print_rel_symbol(p, endian, symbols, sym);
                 let addend = relocation.r_addend(endian).into() as u64;
                 if addend != 0 {
@@ -512,19 +511,19 @@ fn print_rel_symbol<Elf: FileHeader>(
     p: &mut Printer<'_>,
     endian: Elf::Endian,
     symbols: Option<SymbolTable<'_, Elf>>,
-    sym: u32,
+    index: Option<SymbolIndex>,
 ) {
-    if sym == 0 {
-        p.field_hex("Symbol", sym);
+    let Some(index) = index else {
+        p.field_hex("Symbol", 0);
         return;
-    }
+    };
     let name = symbols.and_then(|symbols| {
         symbols
-            .symbol(sym as usize)
+            .symbol(index)
             .and_then(|symbol| symbol.name(endian, symbols.strings()))
             .print_err(p)
     });
-    p.field_string_option("Symbol", sym, name);
+    p.field_string_option("Symbol", index.0, name);
 }
 
 fn rel_flag_type<Elf: FileHeader>(endian: Elf::Endian, elf: &Elf) -> &'static [Flag<u32>] {
@@ -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,