Open
Conversation
Member
|
awesome, thanks! I don't think I have to look into this in the next week, but let's see. |
phip1611
reviewed
Mar 21, 2026
Member
phip1611
left a comment
phip1611
left a comment
There was a problem hiding this comment.
Generally, this is a good direction. I'm doing a comprehensive review once you resolve the CI failures.
phip1611
requested changes
Mar 21, 2026
Member
|
I'm not 100% sure if this is the right design. I wonder if we should forward all functionality to an existing elf library; somehow like this diff --git a/Cargo.lock b/Cargo.lock
index e8a9055..208f7d3 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -26,6 +26,12 @@ version = "1.15.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "48c757948c5ede0e46177b7add2e67155f70e33c07fea8284df6576da70b3719"
+[[package]]
+name = "elf"
+version = "0.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "55dd888a213fc57e957abf2aa305ee3e8a28dbe05687a251f33b637cd46b0070"
+
[[package]]
name = "elf_rs"
version = "0.3.1"
@@ -84,6 +90,7 @@ name = "multiboot2"
version = "0.24.1"
dependencies = [
"bitflags",
+ "elf",
"log",
"multiboot2-common",
"ptr_meta",
diff --git a/Cargo.toml b/Cargo.toml
index cfe3747..41f8ebd 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -23,6 +23,7 @@ package.license = "MIT/Apache-2.0"
[workspace.dependencies]
# Dependencies of multiboot2 et al.
bitflags = { version = "2.11", default-features = false }
+elf = { version = "~0.8", default-features = false }
log = { version = "~0.4", default-features = false }
ptr_meta = { version = "~0.3", default-features = false }
thiserror = { version = "2.0", default-features = false }
diff --git a/multiboot2/Cargo.toml b/multiboot2/Cargo.toml
index e21aa17..46cf61e 100644
--- a/multiboot2/Cargo.toml
+++ b/multiboot2/Cargo.toml
@@ -41,6 +41,7 @@ builder = ["alloc", "multiboot2-common/builder"]
[dependencies]
bitflags = { workspace = true }
+elf = { workspace = true }
log = { workspace = true }
multiboot2-common = { workspace = true }
ptr_meta = { workspace = true }
diff --git a/multiboot2/src/boot_information.rs b/multiboot2/src/boot_information.rs
index 8771fff..198b831 100644
--- a/multiboot2/src/boot_information.rs
+++ b/multiboot2/src/boot_information.rs
@@ -5,13 +5,15 @@ use crate::tag::TagHeader;
use crate::{
ApmTag, BasicMemoryInfoTag, BootLoaderNameTag, BootdevTag, CommandLineTag,
EFIBootServicesNotExitedTag, EFIImageHandle32Tag, EFIImageHandle64Tag, EFIMemoryMapTag,
- EFISdt32Tag, EFISdt64Tag, ElfSectionIter, ElfSectionsTag, EndTag, FramebufferTag,
+ EFISdt32Tag, EFISdt64Tag, ElfSectionsTag, EndTag, FramebufferTag,
ImageLoadPhysAddrTag, MemoryMapTag, ModuleIter, NetworkTag, RsdpV1Tag, RsdpV2Tag, SmbiosTag,
TagIter, TagType, VBEInfoTag, module,
};
use core::fmt;
use core::mem;
use core::ptr::NonNull;
+use elf::endian::AnyEndian;
+use elf::section::SectionHeaderTable;
use multiboot2_common::{DynSizedStructure, Header, MaybeDynSized, MemoryError, Tag};
use thiserror::Error;
@@ -254,7 +256,7 @@ impl<'a> BootInformation<'a> {
/// ```
#[must_use]
#[deprecated = "Use elf_sections_tag() instead and corresponding getters"]
- pub fn elf_sections(&self) -> Option<ElfSectionIter<'_>> {
+ pub fn elf_sections(&self) -> Option<SectionHeaderTable<AnyEndian>> {
let tag = self.get_tag::<ElfSectionsTag>();
tag.map(|t| {
assert!((t.entry_size() * t.shndx()) <= t.header().size);
diff --git a/multiboot2/src/elf_sections.rs b/multiboot2/src/elf_sections.rs
index 78096e3..a7cae55 100644
--- a/multiboot2/src/elf_sections.rs
+++ b/multiboot2/src/elf_sections.rs
@@ -5,13 +5,18 @@ use core::fmt::{Debug, Formatter};
use core::marker::PhantomData;
use core::mem;
use core::str::Utf8Error;
+use elf::endian::{AnyEndian, EndianParse, LittleEndian};
+use elf::file::Class::{ELF32, ELF64};
+use elf::section::{Elf32_Shdr, SectionHeaderTable};
use multiboot2_common::{MaybeDynSized, Tag};
#[cfg(feature = "builder")]
use {alloc::boxed::Box, multiboot2_common::new_boxed};
-/// This tag contains the section header table from an ELF binary.
-// The sections iterator is provided via the [`ElfSectionsTag::sections`]
-// method.
+pub enum SectionsTable<'a> {
+ Elf32(elf::section::SectionHeaderTable<'a, LittleEndian>)
+}
+
+/// This tag contains the section header table from the loaded ELF binary.
#[derive(ptr_meta::Pointee, PartialEq, Eq)]
#[repr(C, align(8))]
pub struct ElfSectionsTag {
@@ -39,17 +44,13 @@ impl ElfSectionsTag {
/// Get an iterator over the ELF sections.
#[must_use]
- pub const fn sections(&self) -> ElfSectionIter<'_> {
- let string_section_offset = (self.shndx * self.entry_size) as isize;
- let string_section_ptr =
- unsafe { self.sections.as_ptr().offset(string_section_offset) as *const _ };
- ElfSectionIter {
- current_section: self.sections.as_ptr(),
- remaining_sections: self.number_of_sections,
- entry_size: self.entry_size,
- string_section: string_section_ptr,
- _phantom_data: PhantomData,
- }
+ pub fn sections<E: EndianParse>(&self) -> SectionHeaderTable<E> {
+ let class = match self.entry_size {
+ 32 => ELF32,
+ 64 => ELF64,
+ _ => panic!("Unsupported entry size {}", self.entry_size),
+ };
+ SectionHeaderTable::new(E::default(), class, &self.sections)
}
/// Returns the amount of sections.
@@ -74,7 +75,7 @@ impl ElfSectionsTag {
impl MaybeDynSized for ElfSectionsTag {
type Header = TagHeader;
- const BASE_SIZE: usize = mem::size_of::<TagHeader>() + 3 * mem::size_of::<u32>();
+ const BASE_SIZE: usize = size_of::<TagHeader>() + 3 * size_of::<u32>();
fn dst_len(header: &TagHeader) -> usize {
assert!(header.size as usize >= Self::BASE_SIZE);
@@ -96,384 +97,8 @@ impl Debug for ElfSectionsTag {
.field("number_of_sections", &self.number_of_sections)
.field("entry_size", &self.entry_size)
.field("shndx", &self.shndx)
- .field("sections", &self.sections())
+ .field("sections", &self.sections::<AnyEndian>())
.finish()
}
}
-/// An iterator over [`ElfSection`]s.
-#[derive(Clone)]
-pub struct ElfSectionIter<'a> {
- current_section: *const u8,
- remaining_sections: u32,
- entry_size: u32,
- string_section: *const u8,
- _phantom_data: PhantomData<&'a ()>,
-}
-
-impl<'a> Iterator for ElfSectionIter<'a> {
- type Item = ElfSection<'a>;
-
- fn next(&mut self) -> Option<ElfSection<'a>> {
- while self.remaining_sections != 0 {
- let section = ElfSection {
- inner: self.current_section,
- string_section: self.string_section,
- entry_size: self.entry_size,
- _phantom: PhantomData,
- };
-
- self.current_section = unsafe { self.current_section.offset(self.entry_size as isize) };
- self.remaining_sections -= 1;
-
- if section.section_type() != ElfSectionType::Unused {
- return Some(section);
- }
- }
- None
- }
-
- fn size_hint(&self) -> (usize, Option<usize>) {
- (
- self.remaining_sections as usize,
- Some(self.remaining_sections as usize),
- )
- }
-}
-
-impl ExactSizeIterator for ElfSectionIter<'_> {
- fn len(&self) -> usize {
- self.remaining_sections as usize
- }
-}
-
-impl Debug for ElfSectionIter<'_> {
- fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
- /// Limit how many Elf-Sections should be debug-formatted.
- /// Can be thousands of sections for a Rust binary => this is useless output.
- /// If the user really wants this, they should debug-format the field directly.
- const ELF_SECTIONS_LIMIT: usize = 7;
-
- let mut debug = f.debug_list();
-
- self.clone().take(ELF_SECTIONS_LIMIT).for_each(|ref e| {
- debug.entry(e);
- });
-
- if self.clone().len() > ELF_SECTIONS_LIMIT {
- debug.entry(&"...");
- }
-
- debug.finish()
- }
-}
-
-/// A single generic ELF Section.
-// TODO Shouldn't this be called ElfSectionPtrs, ElfSectionWrapper or so?
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct ElfSection<'a> {
- inner: *const u8,
- string_section: *const u8,
- entry_size: u32,
- _phantom: PhantomData<&'a ()>,
-}
-
-impl Debug for ElfSection<'_> {
- fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
- let inner = self.get();
- f.debug_struct("ElfSection")
- .field("inner", &inner)
- .field("string_section_ptr", &self.string_section)
- .finish()
- }
-}
-
-#[derive(Clone, Copy, Debug)]
-#[repr(C, packed)]
-struct ElfSectionInner32 {
- name_index: u32,
- typ: u32,
- flags: u32,
- addr: u32,
- offset: u32,
- size: u32,
- link: u32,
- info: u32,
- addralign: u32,
- entry_size: u32,
-}
-
-#[derive(Clone, Copy, Debug)]
-#[repr(C, packed)]
-struct ElfSectionInner64 {
- name_index: u32,
- typ: u32,
- flags: u64,
- addr: u64,
- offset: u64,
- size: u64,
- link: u32,
- info: u32,
- addralign: u64,
- entry_size: u64,
-}
-
-impl ElfSection<'_> {
- /// Get the section type as an `ElfSectionType` enum variant.
- #[must_use]
- pub fn section_type(&self) -> ElfSectionType {
- match self.get().typ() {
- 0 => ElfSectionType::Unused,
- 1 => ElfSectionType::ProgramSection,
- 2 => ElfSectionType::LinkerSymbolTable,
- 3 => ElfSectionType::StringTable,
- 4 => ElfSectionType::RelaRelocation,
- 5 => ElfSectionType::SymbolHashTable,
- 6 => ElfSectionType::DynamicLinkingTable,
- 7 => ElfSectionType::Note,
- 8 => ElfSectionType::Uninitialized,
- 9 => ElfSectionType::RelRelocation,
- 10 => ElfSectionType::Reserved,
- 11 => ElfSectionType::DynamicLoaderSymbolTable,
- 0x6000_0000..=0x6FFF_FFFF => ElfSectionType::EnvironmentSpecific,
- 0x7000_0000..=0x7FFF_FFFF => ElfSectionType::ProcessorSpecific,
- e => {
- log::warn!("Unknown section type {e:x}. Treating as ElfSectionType::Unused");
- ElfSectionType::Unused
- }
- }
- }
-
- /// Get the "raw" section type as a `u32`
- #[must_use]
- pub fn section_type_raw(&self) -> u32 {
- self.get().typ()
- }
-
- /// Read the name of the section.
- pub fn name(&self) -> Result<&str, Utf8Error> {
- use core::{slice, str};
-
- let name_ptr = unsafe { self.string_table().offset(self.get().name_index() as isize) };
-
- // strlen without null byte
- let strlen = {
- let mut len = 0;
- while unsafe { *name_ptr.offset(len) } != 0 {
- len += 1;
- }
- len as usize
- };
-
- str::from_utf8(unsafe { slice::from_raw_parts(name_ptr, strlen) })
- }
-
- /// Get the physical start address of the section.
- #[must_use]
- pub fn start_address(&self) -> u64 {
- self.get().addr()
- }
-
- /// Get the physical end address of the section.
- ///
- /// This is the same as doing `section.start_address() + section.size()`
- #[must_use]
- pub fn end_address(&self) -> u64 {
- self.get().addr() + self.get().size()
- }
-
- /// Get the section's size in bytes.
- #[must_use]
- pub fn size(&self) -> u64 {
- self.get().size()
- }
-
- /// Get the section's address alignment constraints.
- ///
- /// That is, the value of `start_address` must be congruent to 0,
- /// modulo the value of `addrlign`. Currently, only 0 and positive
- /// integral powers of two are allowed. Values 0 and 1 mean the section has no
- /// alignment constraints.
- #[must_use]
- pub fn addralign(&self) -> u64 {
- self.get().addralign()
- }
-
- /// Get the section's flags.
- #[must_use]
- pub fn flags(&self) -> ElfSectionFlags {
- ElfSectionFlags::from_bits_truncate(self.get().flags())
- }
-
- /// Check if the `ALLOCATED` flag is set in the section flags.
- #[must_use]
- pub fn is_allocated(&self) -> bool {
- self.flags().contains(ElfSectionFlags::ALLOCATED)
- }
-
- fn get(&self) -> &dyn ElfSectionInner {
- match self.entry_size {
- 40 => unsafe { &*(self.inner as *const ElfSectionInner32) },
- 64 => unsafe { &*(self.inner as *const ElfSectionInner64) },
- s => panic!("Unexpected entry size: {s}"),
- }
- }
-
- unsafe fn string_table(&self) -> *const u8 {
- match self.entry_size {
- 40 => {
- let ptr = self.string_section.cast::<ElfSectionInner32>();
- let reference = unsafe { ptr.as_ref().unwrap() };
- reference.addr() as *const u8
- }
- 64 => {
- let ptr = self.string_section.cast::<ElfSectionInner64>();
- let reference = unsafe { ptr.as_ref().unwrap() };
- reference.addr() as *const u8
- }
- s => panic!("Unexpected entry size: {s}"),
- }
- }
-}
-
-trait ElfSectionInner: Debug {
- fn name_index(&self) -> u32;
-
- fn typ(&self) -> u32;
-
- fn flags(&self) -> u64;
-
- fn addr(&self) -> u64;
-
- fn size(&self) -> u64;
-
- fn addralign(&self) -> u64;
-}
-
-impl ElfSectionInner for ElfSectionInner32 {
- fn name_index(&self) -> u32 {
- self.name_index
- }
-
- fn typ(&self) -> u32 {
- self.typ
- }
-
- fn flags(&self) -> u64 {
- self.flags.into()
- }
-
- fn addr(&self) -> u64 {
- self.addr.into()
- }
-
- fn size(&self) -> u64 {
- self.size.into()
- }
-
- fn addralign(&self) -> u64 {
- self.addralign.into()
- }
-}
-
-impl ElfSectionInner for ElfSectionInner64 {
- fn name_index(&self) -> u32 {
- self.name_index
- }
-
- fn typ(&self) -> u32 {
- self.typ
- }
-
- fn flags(&self) -> u64 {
- self.flags
- }
-
- fn addr(&self) -> u64 {
- self.addr
- }
-
- fn size(&self) -> u64 {
- self.size
- }
-
- fn addralign(&self) -> u64 {
- self.addralign
- }
-}
-
-/// An enum abstraction over raw ELF section types.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-#[repr(u32)]
-pub enum ElfSectionType {
- /// This value marks the section header as inactive; it does not have an
- /// associated section. Other members of the section header have undefined
- /// values.
- Unused = 0,
-
- /// The section holds information defined by the program, whose format and
- /// meaning are determined solely by the program.
- ProgramSection = 1,
-
- /// This section holds a linker symbol table.
- LinkerSymbolTable = 2,
-
- /// The section holds a string table.
- StringTable = 3,
-
- /// The section holds relocation entries with explicit addends, such as type
- /// Elf32_Rela for the 32-bit class of object files. An object file may have
- /// multiple relocation sections.
- RelaRelocation = 4,
-
- /// The section holds a symbol hash table.
- SymbolHashTable = 5,
-
- /// The section holds dynamic linking tables.
- DynamicLinkingTable = 6,
-
- /// This section holds information that marks the file in some way.
- Note = 7,
-
- /// A section of this type occupies no space in the file but otherwise resembles
- /// `ProgramSection`. Although this section contains no bytes, the
- /// sh_offset member contains the conceptual file offset.
- Uninitialized = 8,
-
- /// The section holds relocation entries without explicit addends, such as type
- /// Elf32_Rel for the 32-bit class of object files. An object file may have
- /// multiple relocation sections.
- RelRelocation = 9,
-
- /// This section type is reserved but has unspecified semantics.
- Reserved = 10,
-
- /// This section holds a dynamic loader symbol table.
- DynamicLoaderSymbolTable = 11,
-
- /// Values in this inclusive range (`[0x6000_0000, 0x6FFF_FFFF)`) are
- /// reserved for environment-specific semantics.
- EnvironmentSpecific = 0x6000_0000,
-
- /// Values in this inclusive range (`[0x7000_0000, 0x7FFF_FFFF)`) are
- /// reserved for processor-specific semantics.
- ProcessorSpecific = 0x7000_0000,
-}
-
-bitflags! {
- /// ELF Section bitflags.
- #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord)]
- #[repr(transparent)]
- pub struct ElfSectionFlags: u64 {
- /// The section contains data that should be writable during program execution.
- const WRITABLE = 0x1;
-
- /// The section occupies memory during the process execution.
- const ALLOCATED = 0x2;
-
- /// The section contains executable machine instructions.
- const EXECUTABLE = 0x4;
- // plus environment-specific use at 0x0F000000
- // plus processor-specific use at 0xF0000000
- }
-}
#[test]
Unfortunately, I don't have the time currently to protype different designs and I'm unsure what's the best way to move this forward. Happy to hear ideas from you! |
Author
|
That actually just looks like an easy change from my iterator, it should clean up my code a bit too. I think a lot of the higher level parsing code you've removed should stay however. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I needed to get the full ELF header I figured I'd implement #247 in the process.
::elf::abicore::ffi::CStrto handle it. Originally I wanted to useCStrto store the section name inElfSectionhowever it doesn't implementCopyElfSection::string_tablenever needed to beunsafe, it is no longerunsafeElfSectionInnertrait is no longer needed because it now useself::section::SectionHeaderfor both ELF32 and ELF64. I've left it in solely for familiarity of others. I can remove it if you want.ElfSectionnow copies the section header instead of maintaining a pointer to it. I can revert this behavior if you want.