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

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/ELF/program-header-nobits.yaml:33-34
+## This segment includes sections 1 to 4 + the .filler chunk.
+# NOOFFSET-NEXT: LOAD 0x000000 0x0000000200000000 0x0000000200000000 0x000200 0x000220
+# OFFSET-NEXT:   LOAD 0x000158 0x0000000200000158 0x0000000200000158 0x0000a8 0x0000c8
+## This segment includes the first two sections.
----------------
grimar wrote:
> jhenderson wrote:
> > FileSize here looks like it should be 0x220, matching the memory size (which looks to be correct). It looks like the .nobits2 section is not being allocated file space, even though there's something non-nobits appearing after it. Please correct my understanding if I've misunderstood something. Same problem with the OFFSET case (off by 0x20 in file size).
> > It looks like the .nobits2 section is not being allocated file space, even though there's something non-nobits appearing after it. 
> 
> Yes, yaml2obj does not allocate a file space in such case. In fact the following piece is the all we do for SHT_NOBITS:
> 
> ```
>     } else if (auto S = dyn_cast<ELFYAML::NoBitsSection>(Sec)) {
>       // SHT_NOBITS sections do not have any content to write.
>       SHeader.sh_entsize = 0;
>       SHeader.sh_size = S->Size;
>     }
> ```
> 
> I.e. it never allocates a file space for SHT_NOBITS. Seems to be the root of the problem.
> 
> So, should we teach yaml2obj to allocate a file space for a SHT_NOBITS section if there is any no-nobits chunk after it?
> How does this behavior sound for you?
I think that would make sense. As far as I know, when a SHT_NOBITS section appears in the middle of a segment (i.e. with other non-nobits sections afterwards), there's no other way of representing it correctly without allocating file space - if you include it in the address space, the later progbits sections will have an incorrect address to content mapping at runtime.

What do linkers do with a linker script where a nobits section is followed by a progbits section in the same segment?


================
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
----------------
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.


================
Comment at: llvm/test/tools/yaml2obj/ELF/program-header-nobits.yaml:137
+    Size:   0x1D0
+    Offset: 0x220
+  - Name:  .nobits4
----------------
grimar wrote:
> jhenderson wrote:
> > Am I right in thinking that this is unnecessary now? The fill chunk should cause this section to be placed after it, and if my counting is correct, that'll result in it appearing at 0x220 naturally.
> In addition to my comment above, and to clarify: so 0x220 naturally could happen here if yaml2obj allocated a file space for `.nobits2`.
> Then I would not need to use "Offset". 
Right. I misread something and thought yaml2obj was already allocating space.


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

https://reviews.llvm.org/D78005