[PATCH] D80629: [yaml2obj] - Allocate the file space for SHT_NOBITS sections in some cases.

[George Rimar via Phabricator via llvm-commits]

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


================
Comment at: llvm/test/tools/obj2yaml/ELF/program-headers.yaml:499-500
+    Flags:  [ SHF_WRITE, SHF_ALLOC ]
 ## Use a size that is larger than the file size.
-    Size:  0x00000000FFFFFFFF
+    ShSize: 0x00000000FFFFFFFF
   - Name:  .data.2
----------------
jhenderson wrote:
> Looking at this test, I'm not convinced this is the right fix for the test. My impression is that this test is supposed to be about how segments are created and their file and memory sizes calculated. By using `ShSize` instead of `Size`, we are effectively saying (in theory) "ignore the size of this section for segment calculations" when I think what is meant is to ignore it for the calculation of the file size.
> 
> I think we need to properly change this test to use a real `Size` value here, just much smaller than what was chosen previously. Alternatively, we probably don't need this set of tests (nobits + segments) here at all, since we have the new tests you added that highlighted the bug of not allocating file space for nobits sections.
> My impression is that this test is supposed to be about how segments are created and their file and memory sizes calculated. 

This is obj2yaml test. It is about how segments are dumped. This change does not change the original behavior, which is described as 
"Use a size that is larger than the file size.", i.e. it checks how we dump such situation.

> I think we need to properly change this test to use a real Size value here, just much smaller than what was chosen previously.

It just will break the original intention ("Use a size that is larger than the file size.").


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

https://reviews.llvm.org/D80629