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

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
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;
----------------
jhenderson wrote:
> 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.
My intention is to land this patch (for `p_filesz`) and then I'll be able update and rebase the D78005 to make it focus only on the `p_memsz` calculation. I'll try to address this comment there.


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

https://reviews.llvm.org/D78628