Skip to content

Rewrite: Add elf library for elf_sections.rs#292

Open
an-owl wants to merge 12 commits into
rust-osdev:mainfrom
an-owl:elf
Open

Rewrite: Add elf library for elf_sections.rs#292
an-owl wants to merge 12 commits into
rust-osdev:mainfrom
an-owl:elf

Conversation

@an-owl

@an-owl an-owl commented Apr 21, 2026

Copy link
Copy Markdown

This is a complete rewrite from #290, using elf::sections::SectionHeaderTable to construct the iterator. Unlike #290 however, I decided not to care about version compatibility. This removes or replaces a number of existing functions.

One change in particular is how to fetch the section name. The existing method in my opinion is flawed, if the ELF sections are relocated then it's not possible to get the name. I've changed this so the string table must be fetched separately, which can then be passed to a helper. This helper returns a &core::ffi::CStr instead of a &str, the ELF specification doesn't define the character encoding so we should not force it to be Unicode.

An important behavioral change that isn't obvious is that "unused" (SHT_NULL) sections are no longer skipped. This threw me for a loop during testing. If they're there it's probably there for a reason, so I think its a bad idea to skip it, especially if you can just use .filter(_).

@phip1611 phip1611 self-requested a review May 4, 2026 05:46
@an-owl an-owl changed the title Add elf library for elf_sections.rs Rewrite: Add elf library for elf_sections.rs May 14, 2026
@an-owl

an-owl commented Jun 3, 2026

Copy link
Copy Markdown
Author

Any progress on this?

@phip1611

phip1611 commented Jun 3, 2026

Copy link
Copy Markdown
Member

Hey! thanks for pinging me! Will look into this soon - I'll put it on my list

@phip1611 phip1611 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely - thanks! This is a great contribution! Only a few nits. Also, please update the changelog.

}
}

impl<'a> From<&'a ElfSectionsTag> for SectionHeaderTable<'a, NativeEndian> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure about the value-add of this. It feels like a misuse of From - could you please add a motivation to the commit?

addralign: u32,
entry_size: u32,
}
/// Extension trait for [SectionHeader] containing getters for rust-native types

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/// Extension trait for [`SectionHeader`] containing getters for rust-native types

entry_size: u32,
}
/// Extension trait for [SectionHeader] containing getters for rust-native types
pub trait ElfSectionExt {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is only one impl for this trait. You do this to extend the type SectionHeader which comes from the elf library, yes?

};

let strtab_hdr = shdr_table.get(strtab_index).ok()?;
// todo: Should this check that `strtab_hdr.sh_type == elf::abi::SHT_STRTAB`?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about this todo?

// Casting through `usize` will not truncate data on 32bit systems because the multiboot2 loads all sections below u32::MAX
Some(unsafe {
core::slice::from_raw_parts(
strtab_hdr.sh_addr as usize as *const u8,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be:

core::ptr::with_exposed_provenance(strtab_hdr.sh_addr as usize)

which is the more future-proof API

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants