[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:778-780
+      uint64_t FileSize = Fragments.back().Offset - PHeader.p_offset;
+      if (Fragments.back().Type != llvm::ELF::SHT_NOBITS)
+        FileSize += Fragments.back().Size;
----------------
grimar wrote:
> jhenderson wrote:
> > This is a little subtle, and probably deserves a comment to explain why we pay attention to offset but not size of a single trailing SHT_NOBITS sections.
> I was about to place the following comment:
> 
> ```
> // SHT_NOBITS sections occupy no physical space in a file, we should not
> // take their sizes into account when calculating the file size of a
> // segment.
> ```
> 
> But then remembered one thing and started to doubt about the usage of its offset.
> It was mentioned in D69192 that the sh_offset of a SHT_NOBITS can be
> "larger than the file size with some usage of objcopy".
> 
> I was unable to find a proper test case committed for this and I do not think I know a way to achieve it.
> (`elf-disassemble-bss.test` (https://github.com/llvm/llvm-project/blob/master/llvm/test/tools/llvm-objdump/X86/elf-disassemble-bss.test) has a non-trailing .bss section that has a too large sh_offset though).
> 
> 
> ELF spec says about sh_offset:
> 
> "This member's value gives the byte offset from the beginning of the file to
> the first byte in the section. One section type, SHT_NOBITS described
> below, occupies no space in the file, and its sh_offset member locates
> the conceptual placement in the file."
> 
> Does "sh_offset member locates the conceptual placement in the file" imply that the
> `sh_offset` have to be less than the file size? I am not sure that "in the file." == "can be outside of the file", it is wierd.
> 
> So can we use `sh_offset` like I do here?
FTR, the approach of this patch it is the same as what LLD does:

```
template <class ELFT> void Writer<ELFT>::setPhdrs(Partition &part) {
  for (PhdrEntry *p : part.phdrs) {
    OutputSection *first = p->firstSec;
    OutputSection *last = p->lastSec;

    if (first) {
      p->p_filesz = last->offset - first->offset;
      if (last->type != SHT_NOBITS)
        p->p_filesz += last->size;
...
```


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

https://reviews.llvm.org/D78628