It says here that if there are no program headers than e_phoff should be 0, but currently it is always set after the header. GNU's readelf (but not llvm-readelf) complains about this: readelf: Warning: possibly corrupt ELF header - it has a non-zero program header offset, but no program headers.
Details
Diff Detail
Event Timeline
| llvm/test/tools/yaml2obj/no-segments-phoff.test | ||
|---|---|---|
| 1 ↗ | (On Diff #218277) | The test may be renamed to elf-header-ephoff.yaml |
LGTM
| llvm/lib/ObjectYAML/ELFEmitter.cpp | ||
|---|---|---|
| 231 | Perhaps we might want to set this to 0 too when there are no headers. | |
| llvm/lib/ObjectYAML/ELFEmitter.cpp | ||
|---|---|---|
| 231 | +1 to this (it's not required by the gABI, but it seems to be common practice). We might as well do it at the same time. | |
| llvm/test/tools/yaml2obj/elf-header-ephoff.yaml | ||
| 1 ↗ | (On Diff #218285) | Nit: missing trailing full stop. If you go with changing the size field too, I'd test that and e_phnum in the same test, and rename this to "elf-header-no-phdrs.yaml" or even "elf-no-phdrs.yaml". |
LGTM, with one suggestion.
| llvm/test/tools/yaml2obj/elf-no-phdrs.yaml | ||
|---|---|---|
| 9 | Minor thing: it's probably worth adding a {{$}} to the end of this line (and optionally the previous line for symmetry). Without it, ProgramHeaderEntrySize: 0 could match ProgramHeaderEntrySize: 0x1234 which would obviously be bad. | |
Thanks. I was still seeing warnings from object files yesterday and I know your patch can fix that :)
Perhaps we might want to set this to 0 too when there are no headers.