read: do not return null sections or symbols in unified API (#679)

This affects ELF and XCOFF, including some of the lower level APIs too.
gimli-rs · May 6, 2024 · 284975d · 284975d
1 parent 2d2fc7e
commit 284975d
Show file tree

Hide file tree

Showing 22 changed files with 99 additions and 117 deletions.
diff --git a/crates/examples/src/objcopy.rs b/crates/examples/src/objcopy.rs
@@ -63,9 +63,6 @@ pub fn copy(in_data: &[u8]) -> Vec<u8> {
 
     let mut out_symbols = HashMap::new();
     for in_symbol in in_object.symbols() {
-        if in_symbol.kind() == SymbolKind::Null {
-            continue;
-        }
         let (section, value) = match in_symbol.section() {
             SymbolSection::None => (write::SymbolSection::None, in_symbol.address()),
             SymbolSection::Undefined => (write::SymbolSection::Undefined, in_symbol.address()),

diff --git a/crates/examples/testfiles/elf/base-aarch64.o.objdump b/crates/examples/testfiles/elf/base-aarch64.o.objdump
@@ -4,7 +4,6 @@ Architecture: Aarch64
 Flags: Elf { os_abi: 0, abi_version: 0, e_flags: 0 }
 Relative Address Base: 0
 Entry Address: 0
-0: Section { name: "", address: 0, size: 0, align: 0, kind: Metadata, flags: Elf { sh_flags: 0 } }
 1: Section { name: ".text", address: 0, size: 20, align: 4, kind: Text, flags: Elf { sh_flags: 6 } }
 2: Section { name: ".rela.text", address: 0, size: 48, align: 8, kind: Metadata, flags: Elf { sh_flags: 40 } }
 3: Section { name: ".data", address: 0, size: 0, align: 1, kind: Data, flags: Elf { sh_flags: 3 } }
@@ -17,7 +16,6 @@ Entry Address: 0
 10: Section { name: ".shstrtab", address: 0, size: 52, align: 1, kind: Metadata, flags: Elf { sh_flags: 0 } }
 
 Symbols
-0: Symbol { name: "", address: 0, size: 0, kind: Null, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: 0, st_other: 0 } }
 1: Symbol { name: "base.c", address: 0, size: 0, kind: File, section: None, scope: Compilation, weak: false, flags: Elf { st_info: 4, st_other: 0 } }
 2: Symbol { name: "", address: 0, size: 0, kind: Section, section: Section(SectionIndex(1)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
 3: Symbol { name: "", address: 0, size: 0, kind: Section, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }

diff --git a/crates/examples/testfiles/elf/base-aarch64.objdump b/crates/examples/testfiles/elf/base-aarch64.objdump
@@ -7,7 +7,6 @@ Entry Address: 620
 Build ID: [f7, 5e, 1f, d9, 4, 9a, 3f, cb, 21, dc, 15, f2, 7b, 5, 32, d8, 61, cf, 16, 2]
 Segment { address: 0, size: 7fc }
 Segment { address: 10d80, size: 298 }
-0: Section { name: "", address: 0, size: 0, align: 0, kind: Metadata, flags: Elf { sh_flags: 0 } }
 1: Section { name: ".interp", address: 200, size: 1b, align: 1, kind: ReadOnlyData, flags: Elf { sh_flags: 2 } }
 2: Section { name: ".note.ABI-tag", address: 21c, size: 20, align: 4, kind: Note, flags: Elf { sh_flags: 2 } }
 3: Section { name: ".note.gnu.build-id", address: 23c, size: 24, align: 4, kind: Note, flags: Elf { sh_flags: 2 } }
@@ -36,7 +35,6 @@ Segment { address: 10d80, size: 298 }
 26: Section { name: ".shstrtab", address: 0, size: ec, align: 1, kind: Metadata, flags: Elf { sh_flags: 0 } }
 
 Symbols
-0: Symbol { name: "", address: 0, size: 0, kind: Null, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: 0, st_other: 0 } }
 1: Symbol { name: "", address: 200, size: 0, kind: Section, section: Section(SectionIndex(1)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
 2: Symbol { name: "", address: 21c, size: 0, kind: Section, section: Section(SectionIndex(2)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
 3: Symbol { name: "", address: 23c, size: 0, kind: Section, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
@@ -126,7 +124,6 @@ Symbols
 87: Symbol { name: "_init", address: 598, size: 0, kind: Text, section: Section(SectionIndex(b)), scope: Dynamic, weak: false, flags: Elf { st_info: 12, st_other: 0 } }
 
 Dynamic symbols
-0: Symbol { name: "", address: 0, size: 0, kind: Null, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: 0, st_other: 0 } }
 1: Symbol { name: "", address: 598, size: 0, kind: Section, section: Section(SectionIndex(b)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
 2: Symbol { name: "", address: 11000, size: 0, kind: Section, section: Section(SectionIndex(15)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
 3: Symbol { name: "_ITM_deregisterTMCloneTable", address: 0, size: 0, kind: Unknown, section: Undefined, scope: Unknown, weak: true, flags: Elf { st_info: 20, st_other: 0 } }

diff --git a/crates/examples/testfiles/elf/base.o.objdump b/crates/examples/testfiles/elf/base.o.objdump
@@ -4,7 +4,6 @@ Architecture: X86_64
 Flags: Elf { os_abi: 0, abi_version: 0, e_flags: 0 }
 Relative Address Base: 0
 Entry Address: 0
-0: Section { name: "", address: 0, size: 0, align: 0, kind: Metadata, flags: Elf { sh_flags: 0 } }
 1: Section { name: ".text", address: 0, size: 1c, align: 1, kind: Text, flags: Elf { sh_flags: 6 } }
 2: Section { name: ".rela.text", address: 0, size: 30, align: 8, kind: Metadata, flags: Elf { sh_flags: 40 } }
 3: Section { name: ".data", address: 0, size: 0, align: 1, kind: Data, flags: Elf { sh_flags: 3 } }
@@ -19,7 +18,6 @@ Entry Address: 0
 12: Section { name: ".shstrtab", address: 0, size: 61, align: 1, kind: Metadata, flags: Elf { sh_flags: 0 } }
 
 Symbols
-0: Symbol { name: "", address: 0, size: 0, kind: Null, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: 0, st_other: 0 } }
 1: Symbol { name: "base.c", address: 0, size: 0, kind: File, section: None, scope: Compilation, weak: false, flags: Elf { st_info: 4, st_other: 0 } }
 2: Symbol { name: "", address: 0, size: 0, kind: Section, section: Section(SectionIndex(1)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
 3: Symbol { name: "", address: 0, size: 0, kind: Section, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }

diff --git a/crates/examples/testfiles/elf/base.objdump b/crates/examples/testfiles/elf/base.objdump
@@ -7,7 +7,6 @@ Entry Address: 570
 Build ID: [d4, 46, a0, 61, bb, 9a, c2, 7a, b4, 3b, 11, 71, 8f, de, df, 5b, 7f, 3a, f6, f4]
 Segment { address: 0, size: 878 }
 Segment { address: 200da8, size: 270 }
-0: Section { name: "", address: 0, size: 0, align: 0, kind: Metadata, flags: Elf { sh_flags: 0 } }
 1: Section { name: ".interp", address: 238, size: 1c, align: 1, kind: ReadOnlyData, flags: Elf { sh_flags: 2 } }
 2: Section { name: ".note.ABI-tag", address: 254, size: 20, align: 4, kind: Note, flags: Elf { sh_flags: 2 } }
 3: Section { name: ".note.gnu.build-id", address: 274, size: 24, align: 4, kind: Note, flags: Elf { sh_flags: 2 } }
@@ -39,7 +38,6 @@ Segment { address: 200da8, size: 270 }
 29: Section { name: ".shstrtab", address: 0, size: fe, align: 1, kind: Metadata, flags: Elf { sh_flags: 0 } }
 
 Symbols
-0: Symbol { name: "", address: 0, size: 0, kind: Null, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: 0, st_other: 0 } }
 1: Symbol { name: "", address: 238, size: 0, kind: Section, section: Section(SectionIndex(1)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
 2: Symbol { name: "", address: 254, size: 0, kind: Section, section: Section(SectionIndex(2)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
 3: Symbol { name: "", address: 274, size: 0, kind: Section, section: Section(SectionIndex(3)), scope: Compilation, weak: false, flags: Elf { st_info: 3, st_other: 0 } }
@@ -105,7 +103,6 @@ Symbols
 63: Symbol { name: "_init", address: 520, size: 0, kind: Text, section: Section(SectionIndex(c)), scope: Dynamic, weak: false, flags: Elf { st_info: 12, st_other: 0 } }
 
 Dynamic symbols
-0: Symbol { name: "", address: 0, size: 0, kind: Null, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: 0, st_other: 0 } }
 1: Symbol { name: "_ITM_deregisterTMCloneTable", address: 0, size: 0, kind: Unknown, section: Undefined, scope: Unknown, weak: true, flags: Elf { st_info: 20, st_other: 0 } }
 2: Symbol { name: "printf", address: 0, size: 0, kind: Text, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: 12, st_other: 0 } }
 3: Symbol { name: "__libc_start_main", address: 0, size: 0, kind: Text, section: Undefined, scope: Unknown, weak: false, flags: Elf { st_info: 12, st_other: 0 } }

diff --git a/src/common.rs b/src/common.rs
@@ -270,8 +270,6 @@ pub enum ComdatKind {
 pub enum SymbolKind {
     /// The symbol kind is unknown.
     Unknown,
-    /// The symbol is a null placeholder.
-    Null,
     /// The symbol is for executable code.
     Text,
     /// The symbol is for a data object.

diff --git a/src/read/elf/comdat.rs b/src/read/elf/comdat.rs
@@ -21,8 +21,22 @@ where
     Elf: FileHeader,
     R: ReadRef<'data>,
 {
-    pub(super) file: &'file ElfFile<'data, Elf, R>,
-    pub(super) iter: iter::Enumerate<slice::Iter<'data, Elf::SectionHeader>>,
+    file: &'file ElfFile<'data, Elf, R>,
+    iter: iter::Enumerate<slice::Iter<'data, Elf::SectionHeader>>,
+}
+
+impl<'data, 'file, Elf, R> ElfComdatIterator<'data, 'file, Elf, R>
+where
+    Elf: FileHeader,
+    R: ReadRef<'data>,
+{
+    pub(super) fn new(
+        file: &'file ElfFile<'data, Elf, R>,
+    ) -> ElfComdatIterator<'data, 'file, Elf, R> {
+        let mut iter = file.sections.iter().enumerate();
+        iter.next(); // Skip null section.
+        ElfComdatIterator { file, iter }
+    }
 }
 
 impl<'data, 'file, Elf, R> Iterator for ElfComdatIterator<'data, 'file, Elf, R>

diff --git a/src/read/elf/file.rs b/src/read/elf/file.rs
@@ -278,17 +278,11 @@ where
     }
 
     fn sections(&self) -> ElfSectionIterator<'data, '_, Elf, R> {
-        ElfSectionIterator {
-            file: self,
-            iter: self.sections.iter().enumerate(),
-        }
+        ElfSectionIterator::new(self)
     }
 
     fn comdats(&self) -> ElfComdatIterator<'data, '_, Elf, R> {
-        ElfComdatIterator {
-            file: self,
-            iter: self.sections.iter().enumerate(),
-        }
+        ElfComdatIterator::new(self)
     }
 
     fn symbol_by_index(&self, index: SymbolIndex) -> read::Result<ElfSymbol<'data, '_, Elf, R>> {
@@ -302,11 +296,7 @@ where
     }
 
     fn symbols(&self) -> ElfSymbolIterator<'data, '_, Elf, R> {
-        ElfSymbolIterator {
-            endian: self.endian,
-            symbols: &self.symbols,
-            index: 0,
-        }
+        ElfSymbolIterator::new(self.endian, &self.symbols)
     }
 
     fn symbol_table(&self) -> Option<ElfSymbolTable<'data, '_, Elf, R>> {
@@ -320,11 +310,7 @@ where
     }
 
     fn dynamic_symbols(&self) -> ElfSymbolIterator<'data, '_, Elf, R> {
-        ElfSymbolIterator {
-            endian: self.endian,
-            symbols: &self.dynamic_symbols,
-            index: 0,
-        }
+        ElfSymbolIterator::new(self.endian, &self.dynamic_symbols)
     }
 
     fn dynamic_symbol_table(&self) -> Option<ElfSymbolTable<'data, '_, Elf, R>> {

diff --git a/src/read/elf/section.rs b/src/read/elf/section.rs
@@ -54,8 +54,13 @@ impl<'data, Elf: FileHeader, R: ReadRef<'data>> SectionTable<'data, Elf, R> {
         self.sections.len()
     }
 
-    /// Return the section header at the given index.
+    /// Get the section header at the given index.
+    ///
+    /// Returns an error for the null section at index 0.
     pub fn section(&self, index: SectionIndex) -> read::Result<&'data Elf::SectionHeader> {
+        if index.0 == 0 {
+            return Err(read::Error("Invalid ELF symbol index"));
+        }
         self.sections
             .get(index.0)
             .read_error("Invalid ELF section index")
@@ -338,8 +343,20 @@ where
     Elf: FileHeader,
     R: ReadRef<'data>,
 {
-    pub(super) file: &'file ElfFile<'data, Elf, R>,
-    pub(super) iter: iter::Enumerate<slice::Iter<'data, Elf::SectionHeader>>,
+    file: &'file ElfFile<'data, Elf, R>,
+    iter: iter::Enumerate<slice::Iter<'data, Elf::SectionHeader>>,
+}
+
+impl<'data, 'file, Elf, R> ElfSectionIterator<'data, 'file, Elf, R>
+where
+    Elf: FileHeader,
+    R: ReadRef<'data>,
+{
+    pub(super) fn new(file: &'file ElfFile<'data, Elf, R>) -> Self {
+        let mut iter = file.sections.iter().enumerate();
+        iter.next(); // Skip null section.
+        ElfSectionIterator { file, iter }
+    }
 }
 
 impl<'data, 'file, Elf, R> Iterator for ElfSectionIterator<'data, 'file, Elf, R>

diff --git a/src/read/elf/symbol.rs b/src/read/elf/symbol.rs
@@ -138,8 +138,13 @@ impl<'data, Elf: FileHeader, R: ReadRef<'data>> SymbolTable<'data, Elf, R> {
         self.symbols.len()
     }
 
-    /// Return the symbol at the given index.
+    /// Get the symbol at the given index.
+    ///
+    /// Returns an error for null entry at index 0.
     pub fn symbol(&self, index: usize) -> read::Result<&'data Elf::Sym> {
+        if index == 0 {
+            return Err(read::Error("Invalid ELF symbol index"));
+        }
         self.symbols
             .get(index)
             .read_error("Invalid ELF symbol index")
@@ -229,11 +234,7 @@ impl<'data, 'file, Elf: FileHeader, R: ReadRef<'data>> ObjectSymbolTable<'data>
     type SymbolIterator = ElfSymbolIterator<'data, 'file, Elf, R>;
 
     fn symbols(&self) -> Self::SymbolIterator {
-        ElfSymbolIterator {
-            endian: self.endian,
-            symbols: self.symbols,
-            index: 0,
-        }
+        ElfSymbolIterator::new(self.endian, self.symbols)
     }
 
     fn symbol_by_index(&self, index: SymbolIndex) -> read::Result<Self::Symbol> {
@@ -260,9 +261,23 @@ where
     Elf: FileHeader,
     R: ReadRef<'data>,
 {
-    pub(super) endian: Elf::Endian,
-    pub(super) symbols: &'file SymbolTable<'data, Elf, R>,
-    pub(super) index: usize,
+    endian: Elf::Endian,
+    symbols: &'file SymbolTable<'data, Elf, R>,
+    index: usize,
+}
+
+impl<'data, 'file, Elf, R> ElfSymbolIterator<'data, 'file, Elf, R>
+where
+    Elf: FileHeader,
+    R: ReadRef<'data>,
+{
+    pub(super) fn new(endian: Elf::Endian, symbols: &'file SymbolTable<'data, Elf, R>) -> Self {
+        ElfSymbolIterator {
+            endian,
+            symbols,
+            index: 1,
+        }
+    }
 }
 
 impl<'data, 'file, Elf: FileHeader, R: ReadRef<'data>> fmt::Debug
@@ -368,7 +383,6 @@ impl<'data, 'file, Elf: FileHeader, R: ReadRef<'data>> ObjectSymbol<'data>
 
     fn kind(&self) -> SymbolKind {
         match self.symbol.st_type() {
-            elf::STT_NOTYPE if self.index.0 == 0 => SymbolKind::Null,
             elf::STT_NOTYPE => SymbolKind::Unknown,
             elf::STT_OBJECT | elf::STT_COMMON => SymbolKind::Data,
             elf::STT_FUNC | elf::STT_GNU_IFUNC => SymbolKind::Text,

diff --git a/src/read/xcoff/symbol.rs b/src/read/xcoff/symbol.rs
@@ -87,6 +87,8 @@ where
     }
 
     /// Iterate over the symbols.
+    ///
+    /// This does not return null symbols.
     #[inline]
     pub fn iter<'table>(&'table self) -> SymbolIterator<'data, 'table, Xcoff, R> {
         SymbolIterator {
@@ -114,11 +116,25 @@ where
         Bytes(bytes).read().read_error("Invalid XCOFF symbol data")
     }
 
-    /// Return the symbol at the given index.
-    pub fn symbol(&self, index: usize) -> Result<&'data Xcoff::Symbol> {
+    /// Get the symbol at the given index.
+    ///
+    /// This does not check if the symbol is null, but does check if the index is in bounds.
+    fn symbol_unchecked(&self, index: usize) -> Result<&'data Xcoff::Symbol> {
         self.get::<Xcoff::Symbol>(index, 0)
     }
 
+    /// Get the symbol at the given index.
+    ///
+    /// Returns an error for null symbols and out of bounds indices.
+    /// Note that this is unable to check whether the index is an auxiliary symbol.
+    pub fn symbol(&self, index: usize) -> Result<&'data Xcoff::Symbol> {
+        let symbol = self.symbol_unchecked(index)?;
+        if symbol.is_null() {
+            return Err(Error("Invalid XCOFF symbol index"));
+        }
+        Ok(symbol)
+    }
+
     /// Return a file auxiliary symbol.
     pub fn aux_file(&self, index: usize, offset: usize) -> Result<&'data Xcoff::FileAux> {
         debug_assert!(self.symbol(index)?.has_aux_file());
@@ -177,10 +193,14 @@ impl<'data, 'table, Xcoff: FileHeader, R: ReadRef<'data>> Iterator
     type Item = (SymbolIndex, &'data Xcoff::Symbol);
 
     fn next(&mut self) -> Option<Self::Item> {
-        let index = self.index;
-        let symbol = self.symbols.symbol(index).ok()?;
-        self.index += 1 + symbol.n_numaux() as usize;
-        Some((SymbolIndex(index), symbol))
+        loop {
+            let index = self.index;
+            let symbol = self.symbols.symbol_unchecked(index).ok()?;
+            self.index += 1 + symbol.n_numaux() as usize;
+            if !symbol.is_null() {
+                return Some((SymbolIndex(index), symbol));
+            }
+        }
     }
 }
 
@@ -405,7 +425,6 @@ impl<'data, 'file, Xcoff: FileHeader, R: ReadRef<'data>> ObjectSymbol<'data>
             }
         }
         match self.symbol.n_sclass() {
-            xcoff::C_NULL => SymbolKind::Null,
             xcoff::C_FILE => SymbolKind::File,
             _ => SymbolKind::Unknown,
         }
@@ -532,6 +551,12 @@ pub trait Symbol: Debug + Pod {
         strings: StringTable<'data, R>,
     ) -> Result<&'data [u8]>;
 
+    /// Return true if the symbol is a null placeholder.
+    #[inline]
+    fn is_null(&self) -> bool {
+        self.n_sclass() == xcoff::C_NULL
+    }
+
     /// Return true if the symbol is undefined.
     #[inline]
     fn is_undefined(&self) -> bool {

diff --git a/src/write/coff/object.rs b/src/write/coff/object.rs
@@ -615,7 +615,7 @@ impl<'a> Object<'a> {
                         }
                     }
                 }
-                SymbolKind::Unknown | SymbolKind::Null => {
+                SymbolKind::Unknown => {
                     return Err(Error(format!(
                         "unimplemented symbol `{}` kind {:?}",
                         symbol.name().unwrap_or(""),

diff --git a/src/write/elf/object.rs b/src/write/elf/object.rs
@@ -679,7 +679,6 @@ impl<'a> Object<'a> {
                 st_info
             } else {
                 let st_type = match symbol.kind {
-                    SymbolKind::Null => elf::STT_NOTYPE,
                     SymbolKind::Text => {
                         if symbol.is_undefined() {
                             elf::STT_NOTYPE

diff --git a/src/write/macho.rs b/src/write/macho.rs
@@ -460,7 +460,7 @@ impl<'a> Object<'a> {
             match symbol.kind {
                 SymbolKind::Text | SymbolKind::Data | SymbolKind::Tls | SymbolKind::Unknown => {}
                 SymbolKind::File | SymbolKind::Section => continue,
-                SymbolKind::Null | SymbolKind::Label => {
+                SymbolKind::Label => {
                     return Err(Error(format!(
                         "unimplemented symbol `{}` kind {:?}",
                         symbol.name().unwrap_or(""),

diff --git a/src/write/xcoff.rs b/src/write/xcoff.rs
@@ -194,7 +194,6 @@ impl<'a> Object<'a> {
                 n_sclass
             } else {
                 match symbol.kind {
-                    SymbolKind::Null => xcoff::C_NULL,
                     SymbolKind::File => xcoff::C_FILE,
                     SymbolKind::Text | SymbolKind::Data | SymbolKind::Tls => {
                         if symbol.is_local() {