[PATCH] D78628: [obj2yaml] - Program headers: simplify the computation of p_filesz.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:778-780
+      uint64_t FileSize = Fragments.back().Offset - PHeader.p_offset;
+      if (Fragments.back().Type != llvm::ELF::SHT_NOBITS)
+        FileSize += Fragments.back().Size;
----------------
This is a little subtle, and probably deserves a comment to explain why we pay attention to offset but not size of a single trailing SHT_NOBITS sections.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:784-790
+    // Find the maximum offset of the end of a section in order to set p_memsz.
+    uint64_t MemOffset = PHeader.p_offset;
+    for (const Fragment &F : Fragments)
+      MemOffset = std::max(MemOffset, F.Offset + F.Size);
+    // Set the memory size if not set explicitly.
     PHeader.p_memsz = YamlPhdr.MemSize ? uint64_t(*YamlPhdr.MemSize)
                                        : MemOffset - PHeader.p_offset;
----------------
Probably tangential to this change, but if I'm reading this right, this will unconditionally cause all non-empty segments to have a memory size, even if there are no allocatable sections in them. This doesn't seem right to me, especially coming from an environment where segments with non-alloc sections (and therefore no address) are quite normal.


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

https://reviews.llvm.org/D78628