[yaml2obj] - Add a syntax to override e_phoff, e_phentsize and e_phnum fields.

[grimar]

Summary

This adds EPhOff, EPhEntSize and EPhNum keys.
Will be useful for creating broken objects for testing llvm-readelf.

[grimar]

• Fix the test, broken by mistake.

[jhenderson]

llvm/lib/ObjectYAML/ELFYAML.cpp

873–875

The section header table related fields use "SH" not "Sh". It probably makes sense for us to standardise on one form or the other. In other words, I think we should probably use "PH*" here.

llvm/test/tools/yaml2obj/ELF/header-sh-fields.yaml

4–15

This case isn't really checking the default values any more. It's checking that the default variable values as specified in the YAML are followed, but the purpose of the check seems to me to be to show what happens when the fields aren't specified.

4–15

Didn't realise this is part of a stack - the comment applies to the previous commit.

[grimar]

llvm/lib/ObjectYAML/ELFYAML.cpp

873–875

Fields in Sections class also use Sh*:

// This can be used to override the offset stored in the sh_name field.
// It does not affect the name stored in the string table.
Optional<llvm::yaml::Hex64> ShName;

// This can be used to override the sh_offset field. It does not place the
// section data at the offset specified.
Optional<llvm::yaml::Hex64> ShOffset;

// This can be used to override the sh_size field. It does not affect the
// content written.
Optional<llvm::yaml::Hex64> ShSize;

// This can be used to override the sh_flags field.
Optional<llvm::yaml::Hex64> ShFlags;

So we have 2 options it seems:

1. Use "SH*" + "PH*" everywhere (i.e make the renaming you suggest + rename fields in Sections).
2. Rename "PH*" -> "Ph*" in a follow-up here (I assumed this action when wrote this patch).

What should we do?

[jhenderson]

Relatedly, how about we actually name these fields more precisely like their spec names, e.g. EShoff, EPhoff, EShentsize (or something equivalent). I don't mind which approach we go with naming, but I'd recommend we be consistent.

[grimar]

• Address review comments.

[grimar]

llvm/lib/ObjectYAML/ELFYAML.cpp

873–875

In D83482#2147128, @jhenderson wrote:

Relatedly, how about we actually name these fields more precisely like their spec names, e.g. EShoff, EPhoff, EShentsize (or something equivalent).

But we do not add E prefix to other fields. E.g: we call e_type as Type.
I renamed them to PH* (e.g. PhEntSize -> PHEntSize), because phentsize is Program Header Entry Size, so it makes sense I think.

[grimar]

llvm/lib/ObjectYAML/ELFYAML.cpp

873–875

But we do not add E prefix to other fields. E.g: we call e_type as Type.

Thinking about it again, probably we should use the E prefix for these fields, because them are special.
Currently we name sh_name -> ShName, sh_offset -> ShOffset etc for section fields.
So, probably, the consistent way would indeed be to name e_phentsize as EPHEntSize/`EPhEntSize. I.e. to use the E prefix.
I'll add it.

[grimar]

• Use E prefix for names.

[jhenderson]

LGTM, thanks.

llvm/lib/ObjectYAML/ELFYAML.cpp

873–875

Right, exactly. The convention we seem to have adopted is <Prefix><Thing> for the direct writing to fields that would typically be auto-generated/inferred from other fields etc. which should have no impact on anything except the final value of the field in the output, and just <Thing> for the other fields.

llvm/include/llvm/ObjectYAML/ELFYAML.h

struct FileHeader {
   ELF_ELFDATA Data;
   ELF_ELFOSABI OSABI;
   llvm::yaml::Hex8 ABIVersion;
   ELF_ET Type;
   ELF_EM Machine;
   ELF_EF Flags;
   llvm::yaml::Hex64 Entry;
 
+  Optional<llvm::yaml::Hex64> PhOff;
+  Optional<llvm::yaml::Hex16> PhEntSize;
+  Optional<llvm::yaml::Hex16> PhNum;
+
   Optional<llvm::yaml::Hex16> SHEntSize;
   Optional<llvm::yaml::Hex64> SHOff;
   Optional<llvm::yaml::Hex16> SHNum;
   Optional<llvm::yaml::Hex16> SHStrNdx;
 };
 
 struct SectionHeader {
   StringRef Name;

llvm/lib/ObjectYAML/ELFEmitter.cpp

void ELFState<ELFT>::writeELFHeader(raw_ostream &OS, uint64_t SHOff) {
   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 = Doc.ProgramHeaders.size() ? sizeof(Header) : 0;
   Header.e_flags = Doc.Header.Flags;
   Header.e_ehsize = sizeof(Elf_Ehdr);
-  Header.e_phentsize = Doc.ProgramHeaders.size() ? sizeof(Elf_Phdr) : 0;
+
+  if (Doc.Header.PhOff)
+    Header.e_phoff = *Doc.Header.PhOff;
+  else if (!Doc.ProgramHeaders.empty())
+    Header.e_phoff = sizeof(Header);
+  else
+    Header.e_phoff = 0;
+
+  if (Doc.Header.PhEntSize)
+    Header.e_phentsize = *Doc.Header.PhEntSize;
+  else if (!Doc.ProgramHeaders.empty())
+    Header.e_phentsize = sizeof(Elf_Phdr);
+  else
+    Header.e_phentsize = 0;
+
+  if (Doc.Header.PhNum)
+    Header.e_phnum = *Doc.Header.PhNum;
+  else if (!Doc.ProgramHeaders.empty())
     Header.e_phnum = Doc.ProgramHeaders.size();
+  else
+    Header.e_phnum = 0;
 
   Header.e_shentsize =
       Doc.Header.SHEntSize ? (uint16_t)*Doc.Header.SHEntSize : sizeof(Elf_Shdr);
 
   const bool NoShdrs = Doc.SectionHeaders && Doc.SectionHeaders->NoHeaders;
 
   if (Doc.Header.SHOff)
     Header.e_shoff = *Doc.Header.SHOff;

llvm/lib/ObjectYAML/ELFYAML.cpp

void MappingTraits<ELFYAML::FileHeader>::mapping(IO &IO,
   IO.mapRequired("Data", FileHdr.Data);
   IO.mapOptional("OSABI", FileHdr.OSABI, ELFYAML::ELF_ELFOSABI(0));
   IO.mapOptional("ABIVersion", FileHdr.ABIVersion, Hex8(0));
   IO.mapRequired("Type", FileHdr.Type);
   IO.mapRequired("Machine", FileHdr.Machine);
   IO.mapOptional("Flags", FileHdr.Flags, ELFYAML::ELF_EF(0));
   IO.mapOptional("Entry", FileHdr.Entry, Hex64(0));
 
+  // obj2yaml does not dump these fields.
+  assert(!IO.outputting() ||
+         (!FileHdr.PhOff.hasValue() && !FileHdr.PhEntSize.hasValue() &&
+          !FileHdr.PhNum.hasValue()));
+  IO.mapOptional("PhOff", FileHdr.PhOff);
+  IO.mapOptional("PhEntSize", FileHdr.PhEntSize);
+  IO.mapOptional("PhNum", FileHdr.PhNum);
+
   IO.mapOptional("SHEntSize", FileHdr.SHEntSize);
   IO.mapOptional("SHOff", FileHdr.SHOff);
   IO.mapOptional("SHNum", FileHdr.SHNum);
   IO.mapOptional("SHStrNdx", FileHdr.SHStrNdx);
 }
 
 void MappingTraits<ELFYAML::ProgramHeader>::mapping(
     IO &IO, ELFYAML::ProgramHeader &Phdr) {

llvm/test/tools/yaml2obj/ELF/header-sh-fields.yaml

 ## In this test case we check that we can override the default values for
-## e_shentsize, e_shoff, e_shnum and e_shstrndx fields in the YAML.
+## ELF header fields in the YAML.
 
 ## First we check the default values.
 
-# RUN: yaml2obj --docnum=1 %s -o %t1
-# RUN: llvm-readelf --file-headers %t1 | FileCheck %s --check-prefix=DEFAULT
+# RUN: yaml2obj %s -o %t-default
+# RUN: llvm-readelf --file-headers %t-default | FileCheck %s --check-prefix=DEFAULT
 
-# DEFAULT:   Start of section headers:          88 (bytes into file)
-# DEFAULT:   Size of section headers:           64 (bytes)
-# DEFAULT:   Number of section headers:         3
-# DEFAULT:   Section header string table index: 2
+# DEFAULT: Start of program headers:          64 (bytes into file)
+# DEFAULT: Start of section headers:          88 (bytes into file)
+# DEFAULT: Size of program headers:           56 (bytes)
+# DEFAULT: Number of program headers:         2
+# DEFAULT: Size of section headers:           64 (bytes)
+# DEFAULT: Number of section headers:         3
+# DEFAULT: Section header string table index: 2
 
 --- !ELF
 FileHeader:
-  Class:   ELFCLASS64
-  Data:    ELFDATA2LSB
-  Type:    ET_REL
-  Machine: EM_X86_64
+  Class:     ELFCLASS64
+  Data:      ELFDATA2LSB
+  Type:      ET_REL
+  Machine:   EM_X86_64
+  SHEntSize: [[SHENTSIZE=64]]
+  SHOff:     [[SHOFF=88]]
+  SHNum:     [[SHNUM=3]]
+  SHStrNdx:  [[SHSTRNDX=2]]
+  PhOff:     [[PHOFF=64]]
+  PhEntSize: [[PHENTSIZE=56]]
+  PhNum:     [[PHNUM=2]]
+ProgramHeaders:
+  - Type:     PT_LOAD
+    Sections: []
+  - Type:     PT_LOAD
+    Sections: []
+
+## Override different fields to check the output produced.
+
+## Override the e_shoff field.
+# RUN: yaml2obj %s -DSHOFF=3 -o %t2
+# RUN: llvm-readelf --file-headers %t2 | FileCheck %s --check-prefix=SHOFF
+
+# SHOFF: Start of section headers: 3 (bytes into file)
+
+## Override the e_shnum field.
+# RUN: yaml2obj %s -DSHNUM=2 -o %t3
+# RUN: llvm-readelf --file-headers %t3 | FileCheck %s --check-prefix=SHNUM
+
+# SHNUM: Number of section headers: 2{{$}}
+
+## Override the e_shstrndx field.
+# RUN: yaml2obj %s -DSHSTRNDX=4 -o %t4
+# RUN: llvm-readelf --file-headers %t4 | FileCheck %s --check-prefix=SHSTRNDX
 
-## Override 3 fields: e_shoff, e_shnum and e_shstrndx. Check the output.
+# SHSTRNDX: Section header string table index: 4{{$}}
 
-# RUN: yaml2obj --docnum=2 %s -o %t2
-# RUN: llvm-readelf --file-headers %t2 | FileCheck %s --check-prefix=CUSTOM
-
-# CUSTOM: Start of section headers:          2 (bytes into file)
-# CUSTOM: Size of section headers:           64 (bytes)
-# CUSTOM: Number of section headers:         3
-# CUSTOM: Section header string table index: 4
-
---- !ELF
-FileHeader:
-  Class:     ELFCLASS64
-  Data:      ELFDATA2LSB
-  Type:      ET_REL
-  Machine:   EM_X86_64
-  SHEntSize: 64
-  SHOff:     2
-  SHNum:     3
-  SHStrNdx:  4
-
-## Finally, we use the same YAML as above, but set e_shentsize to 1.
+## Override the e_shentsize field.
 ## Check the result using raw output from 'od' because llvm-readelf
 ## is unable to dump such headers.
 
-# RUN: yaml2obj --docnum=3 %s -o %t3
-# RUN: od -A n -t x1 -v -j 0x3a -N 1 %t3 | FileCheck %s --check-prefix=NEWSIZE
-# RUN: od -A n -t x1 -v -j 0x3a -N 1 %t2 | FileCheck %s --check-prefix=OLDSIZE
+# RUN: yaml2obj %s -DSHENTSIZE=1 -o %t5
+# RUN: od -A n -t x1 -v -j 0x3a -N 1 %t5 | FileCheck %s --check-prefix=NEWSIZE
+# RUN: od -A n -t x1 -v -j 0x3a -N 1 %t-default | FileCheck %s --check-prefix=OLDSIZE
 # NEWSIZE: 01
 # OLDSIZE: 40
 
---- !ELF
-FileHeader:
-  Class:     ELFCLASS64
-  Data:      ELFDATA2LSB
-  Type:      ET_REL
-  Machine:   EM_X86_64
-  SHEntSize: 1
-  SHOff:     2
-  SHNum:     3
-  SHStrNdx:  4
+## Override the e_phoff field.
+# RUN: yaml2obj %s -DPHOFF=3 -o %t6
+# RUN: llvm-readelf --file-headers %t6 | FileCheck %s --check-prefix=PHOFF
+
+# PHOFF: Start of program headers: 3 (bytes into file){{$}}
+
+## Override the e_phnum field.
+# RUN: yaml2obj %s -DPHNUM=1 -o %t7
+# RUN: llvm-readelf --file-headers %t7 | FileCheck %s --check-prefix=PHNUM
+
+# PHNUM: Number of program headers: 1{{$}}
+
+## Override the e_phentsize field.
+# RUN: yaml2obj %s -DPHENTSIZE=1 -o %t8
+# RUN: not llvm-readelf --file-headers %t8 2>&1 | FileCheck %s --check-prefix=PHENTSIZE
+
+# PHENTSIZE: invalid e_phentsize: 1{{$}}

llvm/tools/obj2yaml/elf2yaml.cpp

if (S.Type == ELF::SHT_STRTAB || S.Type == ELF::SHT_SYMTAB ||
     return S.Flags.getValueOr(ELFYAML::ELF_SHF(0)) & ELF::SHF_ALLOC;
 
   return true;
 }
 
 template <class ELFT> Expected<ELFYAML::Object *> ELFDumper<ELFT>::dump() {
   auto Y = std::make_unique<ELFYAML::Object>();
 
-  // Dump header. We do not dump SHEntSize, SHOff, SHNum and SHStrNdx fields.
-  // When not explicitly set, the values are set by yaml2obj automatically
-  // and there is no need to dump them here.
+  // Dump header. We do not dump Ph* and SH* fields. When not explicitly set,
+  // the values are set by yaml2obj automatically and there is no need to dump
+  // them here.
   Y->Header.Class = ELFYAML::ELF_ELFCLASS(Obj.getHeader()->getFileClass());
   Y->Header.Data = ELFYAML::ELF_ELFDATA(Obj.getHeader()->getDataEncoding());
   Y->Header.OSABI = Obj.getHeader()->e_ident[ELF::EI_OSABI];
   Y->Header.ABIVersion = Obj.getHeader()->e_ident[ELF::EI_ABIVERSION];
   Y->Header.Type = Obj.getHeader()->e_type;
   Y->Header.Machine = Obj.getHeader()->e_machine;
   Y->Header.Flags = Obj.getHeader()->e_flags;
   Y->Header.Entry = Obj.getHeader()->e_entry;