[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/test/tools/yaml2obj/ELF/program-header-size-offset.yaml:33
 # CHECK:    Offset: 0x2004
-# CHECK:    FileSize: 4
+# CHECK:    FileSize: 5
 # CHECK:    MemSize: 6
----------------
MaskRay wrote:
> grimar wrote:
> > MaskRay wrote:
> > > Why is this changed?
> > This patch fixes a bug I beliece: it's 
> > 
> > ```
> >   # Program header with 2 SHT_NOBITS sections.
> >   - Type:     0x6abcdef0
> >     Offset:   0x2004
> >     Sections:
> >       - Section: .data
> >       - Section: .nobits1
> >       - Section: .nobits2
> > ```
> > 
> > The layout is:
> > 
> > ```
> > Section Headers:
> >   [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
> >   [ 0]                   NULL            0000000000000000 000000 000000 00     0   0  0
> >   [ 1] .text             PROGBITS        0000000000000000 001000 000004 00     0   0 4096
> >   [ 2] .rodata           PROGBITS        0000000000000000 002000 000004 00     0   0 4096
> >   [ 3] .data             PROGBITS        0000000000000000 002004 000004 00     0   0  0
> >   [ 4] .nobits1          NOBITS          0000000000000000 002008 000001 00     0   0  0
> >   [ 5] .nobits2          NOBITS          0000000000000000 002009 000001 00     0   0  0
> >   [ 6] .strtab           STRTAB          0000000000000000 002008 000001 00     0   0  1
> >   [ 7] .shstrtab         STRTAB          0000000000000000 002009 000039 00     0   0  1
> > ```
> > 
> > `0x2009 - 0x2004 == 0x5`, not `0x4`
> I think it is hard to say that this is a bug. Conceptually sh_offset of a SHT_NOBITS section can be ignored. Usually, the ELF writer should set the sh_offset field of `.nobits2` to 0x2008 because there is no need to leave a one-byte gap.
> 
> I don't think this trivia matter much though, handling it either way is ok. If doing it one way helps simplify our overall logic, let's choose that way.
Is there a risk that leaving the FileSize of the segment too high might result in it going outside the range of the file? In other words, does yaml2obj pay any attention to the segment sizes when it lays things out? In other words, in this example, if .nobits1 was say size 0xFFFF0000, would it cause the program header to be referencing data outside the file?

Knowing that llvm-objcopy reads segment data based on the segment file size property, I think we need to be careful about what the FileSize is for segments. It cannot go beyond the file's end.


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

https://reviews.llvm.org/D78628