[PATCH] D95591: [yaml2obj/obj2yaml] - Implement program header table as a special Chunk.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I might well have missed it, but I don't recall seeing any dedicated obj2yaml tests?



================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:757
+            dyn_cast<ELFYAML::ProgramHeaderTable>(D.get())) {
+      PHT->Offset = alignToOffset(CBA, /*Align=*/1, PHT->Offset);
+      uint64_t Size = sizeof(Elf_Phdr) * PHT->Segments.size();
----------------
My reading of the ELF gABI is that this will need to be aligned on a 4-byte/8-byte boundary dependent on ELF32/ELF64. We should do that for an implicit offset (but allow it if the offset is explicit):

> All data structures that the object file format defines follow the ``natural'' size and alignment guidelines for the relevant class. If necessary, data structures contain explicit padding to ensure 8-byte alignment for 8-byte objects, 4-byte alignment for 4-byte objects, to force structure sizes to a multiple of 4 or 8, and so forth. Data also have suitable alignment from the beginning of the file. Thus, for example, a structure containing an Elf32_Addr member will be aligned on a 4-byte boundary within the file. 

Speaking of which, this code looks quite different to the section header table code, which has two separate calls to `alignToOffset` dependent on whether the Offset has been specified, if I read it correctly. Why is that?


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:759
+      uint64_t Size = sizeof(Elf_Phdr) * PHT->Segments.size();
+      // The full program header information might be not available here, so
+      // fill the space with zeroes as a placeholder.
----------------
(sorry - missed this same issue in the section header table case)


================
Comment at: llvm/test/tools/yaml2obj/ELF/program-header.yaml:176
+
+## Check that the size of the program header might affect on virtual
+## addresses and offsets of following sections.
----------------



================
Comment at: llvm/test/tools/yaml2obj/ELF/program-header.yaml:203-204
+
+## Check that we can place the program header table at an
+## arbitrary offset before other sections.
+
----------------
What about an offset that isn't already aligned to 4/8?


================
Comment at: llvm/test/tools/yaml2obj/ELF/program-header.yaml:218-219
+
+## Check we can place the program header table somewhere in
+## the middle of the sections list.
+
----------------
Should we have one for after all other chunks so that the total file size is based on the end of the last chunk? I don't know if that's really needed or not.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95591/new/

https://reviews.llvm.org/D95591