feat: generate a .symtab_shndx section when shnum > 65k#1783
Conversation
davidlattimore
left a comment
There was a problem hiding this comment.
Thanks for working on this. I haven't properly reviewed this yet, since I see there's still some work to do getting tests to pass. If you need any guidance on the tests feel free to ask.
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
791da77 to
ada56b9
Compare
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
| total_sizes: &mut OutputSectionPartMap<u64>, | ||
| num_sections: u32, | ||
| ) { | ||
| if num_sections < u32::from(object::elf::SHN_LORESERVE) { |
There was a problem hiding this comment.
We want to avoid references to object::elf from platform-generic code. Probably the whole shndx mechanism is elf-specific, so maybe this function should be moved to elf.rs, most likely by adding a new method to the Platform trait.
|
|
||
| let num_sections = keep_sections.values_iter().filter(|p| **p).count(); | ||
| let mut num_sections = keep_sections.values_iter().filter(|p| **p).count(); | ||
| if num_sections >= object::elf::SHN_LORESERVE as usize { |
There was a problem hiding this comment.
Can this bit of elf-specific code be moved to elf.rs by adding a new method to the Platform trait?
Signed-off-by: vishruth-thimmaiah <vishruththimmaiah@gmail.com>
| format!("Expected .symtab_shndx section when writing symbol {} with shndx set to SHN_XINDEX.", String::from_utf8_lossy(name)) | ||
| })?; | ||
| object::elf::SHN_XINDEX | ||
| }; |
There was a problem hiding this comment.
I'm not sure that this logic is correct (and I'm not sure how to go about testing the u16 section overflow to verify). The problem is that we're using a u32 to encode two different kinds of values: a section index (which can logically be any value in the u32 range), and reserved index values such as SHN_ABS which have values in the 0xfff0-0xffff range, and these value ranges overlap. So if we happened to have over 0xfff0 sections, we would wrongly encode one of them as SHN_ABS instead of SHN_XINDEX.
There was a problem hiding this comment.
Thanks for pointing that out. You're right, both that this isn't right as it currently stands and that testing it is likely to be hard. From a testing perspective, we probably need some way to define tests that compile generated code, which we don't currently support. That way, we can generate an input file with a sufficiently large number of sections. I guess the other problem is that the sections aren't especially used at runtime, so there's not much we can do at runtime to check that they're right. Probably we need to rely on test assertions to verify that it's correct.
There was a problem hiding this comment.
We can probably use macros in c to generate >65k sections. To verify the symbols' section indexes, we can probably use the ExpectSym? I can take a look at this and submit a PR.
There was a problem hiding this comment.
That sounds like it would work. Thanks!
This PR adds support for generating a
.symtab_shndxwhen the number of sections crosses SHN_LORESERVE. Additionaly we also set the header'sshnumto 0 andshstrndxto SHN_XINDEX, using the first section'ssh_sizeandsh_linkfields respectively instead.