[yaml2obj] Make e_phoff and e_phentsize 0 if there are no program headers

[abrachet]

Summary

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.

[MaskRay]

llvm/test/tools/yaml2obj/no-segments-phoff.test

1 ↗

(On Diff #218277)

The test may be renamed to elf-header-ephoff.yaml

[abrachet]

Changed test name

[grimar]

LGTM

llvm/lib/ObjectYAML/ELFEmitter.cpp

231 ↗

(On Diff #218285)

Perhaps we might want to set this to 0 too when there are no headers.

[jhenderson]

llvm/lib/ObjectYAML/ELFEmitter.cpp

231 ↗

(On Diff #218285)

+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".

[abrachet]

Made e_phentsize 0 when there are no program headers too.

[MaskRay]

llvm/test/tools/yaml2obj/elf-no-phdrs.yaml

1 ↗

(On Diff #218401)

e_phoff and e_phnum are set to 0

1 ↗

(On Diff #218401)

Sorry, I meant e_phoff and e_phentsize

[abrachet]

Changed test comment.

[jhenderson]

LGTM, with one suggestion.

llvm/test/tools/yaml2obj/elf-no-phdrs.yaml

8 ↗

(On Diff #218404)

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.

[MaskRay]

Ping:)

[abrachet]

Addressed review comments.

[MaskRay]

In D67054#1660201, @abrachet wrote:

Addressed review comments.

Thanks. I was still seeing warnings from object files yesterday and I know your patch can fix that :)

llvm/trunk/lib/ObjectYAML/ELFEmitter.cpp

void ELFState<ELFT>::writeELFHeader(ContiguousBlobAccumulator &CBA, raw_ostream &OS) {
   Header.e_ident[EI_DATA] = Doc.Header.Data;
   Header.e_ident[EI_VERSION] = EV_CURRENT;
   Header.e_ident[EI_OSABI] = Doc.Header.OSABI;
   Header.e_ident[EI_ABIVERSION] = Doc.Header.ABIVersion;
   Header.e_type = Doc.Header.Type;
   Header.e_machine = Doc.Header.Machine;
   Header.e_version = EV_CURRENT;
   Header.e_entry = Doc.Header.Entry;
-  Header.e_phoff = sizeof(Header);
+  Header.e_phoff = Doc.ProgramHeaders.size() ? sizeof(Header) : 0;
   Header.e_flags = Doc.Header.Flags;
   Header.e_ehsize = sizeof(Elf_Ehdr);
-  Header.e_phentsize = sizeof(Elf_Phdr);
+  Header.e_phentsize = Doc.ProgramHeaders.size() ? sizeof(Elf_Phdr) : 0;
   Header.e_phnum = Doc.ProgramHeaders.size();
 
   Header.e_shentsize =
       Doc.Header.SHEntSize ? (uint16_t)*Doc.Header.SHEntSize : sizeof(Elf_Shdr);
   // Immediately following the ELF header and program headers.
   // Align the start of the section header and write the ELF header.
   uint64_t ShOffset;
   CBA.getOSAndAlignedOffset(ShOffset, sizeof(typename ELFT::uint));

llvm/trunk/test/tools/yaml2obj/elf-no-phdrs.yaml

+## Check that e_phoff and e_phentsize are set to 0 when there are no
+## program headers.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --file-headers %t | FileCheck %s
+
+# CHECK: ProgramHeaderOffset: 0x0{{$}}
+# CHECK: ProgramHeaderEntrySize: 0{{$}}
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64