[PATCH] D78005: [yaml2obj] - Reimplement how tool calculates memory sizes of segments.

Wed Jun 3 03:48:21 PDT 2020

grimar added inline comments.

================
Comment at: llvm/test/tools/yaml2obj/ELF/program-header-nobits.yaml:27
+## This segment includes sections 1 to 6.
+# NOOFFSET-NEXT: LOAD 0x000000 0x0000000200000000 0x0000000200000000 0x000220 0x000510
+# OFFSET-NEXT:   LOAD 0x000158 0x0000000200000158 0x0000000200000158 0x0000c8 0x0003b8
----------------
jhenderson wrote:
> I might be failing at counting, but shouldn't MemSiz here be 0x4f0? (i.e. 0x2000003f0 + 0x100 - 0x200000000 == Final section address + final section size - base address).
Yes. Now, after D80629 was landed, this and other calculations are fixed.

================
Comment at: llvm/test/tools/yaml2obj/ELF/program-header-nobits.yaml:35
+# OFFSET-NEXT:   LOAD 0x000158 0x0000000200000158 0x0000000200000158 0x0000a8 0x0000c8
+## This segment includes the first two sections.
+# NOOFFSET-NEXT: LOAD 0x000000 0x0000000200000000 0x0000000200000000 0x00015a 0x00015a
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > I was slightly surprised to not see a case for sections 1 to 4 or 1 to 3, but I assume that's because the 1 to 6 and 1 to 5 are essentially equivalent?
> > Honestly I do not remember why I did not include these initially. but I guess I tried to avoid having similar cases. Though now I think this test would benefit in readability from having all possibile cases. I am inclined to add them.
> Sounds reasonable to me.
I've did it. Unfortunately offsets changed again because of adding 2 more program headers :(

But before adding those, I've checked that they were fixed after landing D80629 (i.e. they matched values you expected to see).
And the new values also look right to me now.

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

https://reviews.llvm.org/D78005