Rewrite: Add elf library for elf_sections.rs#292
Conversation
…tionHeaderTable.into_iter()` & type alias
…nto SectionHeaderTable
…with an extension trait for elf::section::SectionHeader
…tionsTag was set to an invalid entry size causing the test to fail
… `ElfSectionFlags`
…tionHeaderTable.into_iter()` & type alias
…nto SectionHeaderTable
…with an extension trait for elf::section::SectionHeader
…tionsTag was set to an invalid entry size causing the test to fail
… `ElfSectionFlags`
an-owl
changed the title
|
Any progress on this? |
|
Hey! thanks for pinging me! Will look into this soon - I'll put it on my list |
phip1611
left a comment
phip1611
left a comment
There was a problem hiding this comment.
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> { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
The idea is that ElfSectionTag is effectively
struct ElfSectionTag {
multiboot_data: ...,
header_table: SectionHeaderTable,
}
So it would be nice to implement AsRef so it can be passed to functions taking impl AsRef<SectionHeaderTable>. But this causes a use-after-free because this SectionHeaderTable::new() does not return a reference.
| entry_size: u32, | ||
| } | ||
| /// Extension trait for [SectionHeader] containing getters for rust-native types | ||
| pub trait ElfSectionExt { |
There was a problem hiding this comment.
There is only one impl for this trait. You do this to extend the type SectionHeader which comes from the elf library, yes?
There was a problem hiding this comment.
That's the idea, otherwise you need to cast the section type & flags through raw pointers. It also adds a method to fetch the section name, elf can only fetch the string table from elf::elfBytes or elf::ElfStream which obviously we don't have.
| }; | ||
|
|
||
| let strtab_hdr = shdr_table.get(strtab_index).ok()?; | ||
| // todo: Should this check that `strtab_hdr.sh_type == elf::abi::SHT_STRTAB`? |
There was a problem hiding this comment.
I have absolutely no strong feelings one way or another.
elf does this, But I'm not sure if we should bother checking. I feel it's the bootloaders responsibility to check this.
There was a problem hiding this comment.
fine with me, we can leave this comment there for now
…h_exposed_provenance`
# Conflicts: # multiboot2/CHANGELOG.md # multiboot2/src/elf_sections.rs
|
You have a weird git history. Please clean up your git history without any |
2 participants
This is a complete rewrite from #290, using
elf::sections::SectionHeaderTableto 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::CStrinstead 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(_).