[obj2yaml] - Dump section offsets in some cases.

[grimar]

Summary

Currently we never dump the sh_offset key.
Though it sometimes an important information.

To reduce the noise this patch implements the following logic:

1. The "Offset" key for the first section is always emitted.
2. If we can derive the offset for a next section naturally, then the "Offset" key is omitted.

By "naturally" I mean that section[X] offset is expected to be:

offsetOf(section[X]) == alignTo(section[X - 1].sh_offset + section[X - 1].sh_size, section[X].sh_addralign)

So, when it has the expected value, we omit it from the output.

[grimar]

• Cosmetic fix.

[jhenderson]

I don't think you want to set the offset unconditionally for the first section either. Much of the time the first section's position will be automatically derivable, namely immediately after the ELF header and potential program headers (after alignment has been taken into account). If we explicitly print the offset, people are going to start including it unnecessarily.

[grimar]

• Addressed review comment (don't dump the offset of the first section).

[grimar]

• Fixed test case.

[jhenderson]

Mostly looks fine. Do NOBITS sections come into play here at all? In particular, their size wouldn't normally impact the offset of subsequent sections.

llvm/test/tools/obj2yaml/ELF/offset.yaml

87

100

[grimar]

• Addressed review comments.

[grimar]

In D91152#2388303, @jhenderson wrote:

Mostly looks fine. Do NOBITS sections come into play here at all? In particular, their size wouldn't normally impact the offset of subsequent sections.

I've updated the patch and added the test case for SHT_NOBITS sections. Thanks!

[jhenderson]

Doesn't the behaviour of a SHT_NOBITS section depend on whether it is a non-last member of a segment? IIRC, SHT_NOBITS sections in a segment which are last do not need file space, but to ensure correct addresses for sections appearing after them in a segment, they are allocated filespace by a linker if non-NOBITS appear after them in the same segment. (The behaviour you're doing here looks fine for when such sections are not inside segments, e.g. for ET_REL objects).

[grimar]

In D91152#2388491, @jhenderson wrote:

Doesn't the behaviour of a SHT_NOBITS section depend on whether it is a non-last member of a segment? IIRC, SHT_NOBITS sections in a segment which are last do not need file space, but to ensure correct addresses for sections appearing after them in a segment, they are allocated filespace by a linker if non-NOBITS appear after them in the same segment. (The behaviour you're doing here looks fine for when such sections are not inside segments, e.g. for ET_REL objects).

True. But how much necessary to support this case now? The problem is that we at first dump sections and then dump program headers.
To implement the precise behavior, I think we need to delay assigning offsets to be able to call shouldAllocateFileSpace, which does the job:

static bool shouldAllocateFileSpace(ArrayRef<ELFYAML::ProgramHeader> Phdrs,
                                    const ELFYAML::NoBitsSection &S) {
  for (const ELFYAML::ProgramHeader &PH : Phdrs) {
    auto It = llvm::find_if(
        PH.Chunks, [&](ELFYAML::Chunk *C) { return C->Name == S.Name; });
    if (std::any_of(It, PH.Chunks.end(), [](ELFYAML::Chunk *C) {
          return (isa<ELFYAML::Fill>(C) ||
                  cast<ELFYAML::Section>(C)->Type != ELF::SHT_NOBITS);
        }))
      return true;
  }
  return false;
}

I wonder - do we want adding this complexity? I thought that nobits sections are usually placed after progbits anyways.

[MaskRay]

In D91152#2388533, @grimar wrote:

In D91152#2388491, @jhenderson wrote:

Doesn't the behaviour of a SHT_NOBITS section depend on whether it is a non-last member of a segment? IIRC, SHT_NOBITS sections in a segment which are last do not need file space, but to ensure correct addresses for sections appearing after them in a segment, they are allocated filespace by a linker if non-NOBITS appear after them in the same segment. (The behaviour you're doing here looks fine for when such sections are not inside segments, e.g. for ET_REL objects).

True. But how much necessary to support this case now? The problem is that we at first dump sections and then dump program headers.
To implement the precise behavior, I think we need to delay assigning offsets to be able to call shouldAllocateFileSpace, which does the job:

In dumpProgramHeaders we call isInSegment to know the parent segment of a section. If we move the logic earlier before setOffsets, we can refine the computation of the expected sh_offset.
With a containing PT_LOAD, sh_offset should equal its address modulo p_align.

static bool shouldAllocateFileSpace(ArrayRef<ELFYAML::ProgramHeader> Phdrs,
                                    const ELFYAML::NoBitsSection &S) {
  for (const ELFYAML::ProgramHeader &PH : Phdrs) {
    auto It = llvm::find_if(
        PH.Chunks, [&](ELFYAML::Chunk *C) { return C->Name == S.Name; });
    if (std::any_of(It, PH.Chunks.end(), [](ELFYAML::Chunk *C) {
          return (isa<ELFYAML::Fill>(C) ||
                  cast<ELFYAML::Section>(C)->Type != ELF::SHT_NOBITS);
        }))
      return true;
  }
  return false;
}

I wonder - do we want adding this complexity? I thought that nobits sections are usually placed after progbits anyways.

[jhenderson]

I mostly want to make sure we don't result in invalid output caused by missing the corner case. If all that happens is that Offset is produced unnecessarily, it's probably fine either way, but what I don't want is Offset to be omitted due to an incorrect calculation, when it shouldn't be, as this will lead to an invalid output, which might mask a problem a user is trying to locate. As things stand, I think we can run into that situation.

llvm/tools/obj2yaml/elf2yaml.cpp

518–521

By the way, I think it feels more natural to do the special case first (i.e. Sec.Type == ELF::SHT_NOBITS), before the default case, as it is easier to add further cases in the future, but it's not a big deal either way.

[grimar]

In D91152#2390116, @MaskRay wrote:

In D91152#2388533, @grimar wrote:

In D91152#2388491, @jhenderson wrote:

Doesn't the behaviour of a SHT_NOBITS section depend on whether it is a non-last member of a segment? IIRC, SHT_NOBITS sections in a segment which are last do not need file space, but to ensure correct addresses for sections appearing after them in a segment, they are allocated filespace by a linker if non-NOBITS appear after them in the same segment. (The behaviour you're doing here looks fine for when such sections are not inside segments, e.g. for ET_REL objects).

True. But how much necessary to support this case now? The problem is that we at first dump sections and then dump program headers.
To implement the precise behavior, I think we need to delay assigning offsets to be able to call shouldAllocateFileSpace, which does the job:

In dumpProgramHeaders we call isInSegment to know the parent segment of a section. If we move the logic earlier before setOffsets, we can refine the computation of the expected sh_offset.
With a containing PT_LOAD, sh_offset should equal its address modulo p_align.

This should work, but it introduces the depencency on the VA. Perhaps I'd try to avoid adding more dependencies. I don't have a strong opinion though,
but I have a feeling that just delaying of the offsets assigning might look a bit simpler (or at least not that bad as it sounds). Going to try to see how it will look like.

[grimar]

• Updated implementation to delay dumping of offsets to fix the logic for SHT_NOBITS sections.

[jhenderson]

llvm/test/Object/obj2yaml.test

365

Just trying to understand why this and the .data Offsets are needed explicitly here, when they weren't prior to this patch?

[grimar]

llvm/test/Object/obj2yaml.test

365

1. Before this patch we never dumped the "Offset" field.
2. This is the case when we unable to properly derive the offset of the .rel.text and .data because they

are not sorted by the file offset in the section header table:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS        00000000 000034 00004c 00  AX  0   0  4
  [ 2] .rel.text         REL             00000000 000434 000030 08     10   1  4
  [ 3] .data             PROGBITS        00000000 000080 000000 00  WA  0   0  4
  [ 4] .bss              NOBITS          00000000 000080 000004 00  WA  0   0  4
  [ 5] .mdebug.abi32     PROGBITS        00000000 000080 000000 00      0   0  1

[jhenderson]

LGTM, thanks. Might want to wait for @MaskRay too.

[grimar]

Ping @MaskRay?

llvm/include/llvm/ObjectYAML/ELFYAML.h

for (const std::unique_ptr<Chunk> &Sec : Chunks)
       if (auto S = dyn_cast<ELFYAML::Section>(Sec.get()))
         Ret.push_back(S);
     return Ret;
   }
 
   unsigned getMachine() const;
 };
 
+bool shouldAllocateFileSpace(ArrayRef<ProgramHeader> Phdrs,
+                             const NoBitsSection &S);
+
 } // end namespace ELFYAML
 } // end namespace llvm
 
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::StackSizeEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::BBAddrMapEntry::BBEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::DynamicEntry)
 LLVM_YAML_IS_SEQUENCE_VECTOR(llvm::ELFYAML::LinkerOption)

llvm/lib/ObjectYAML/ELFEmitter.cpp

if (YamlPhdr.Align) {
       // alignment.
       PHeader.p_align = 1;
       for (const Fragment &F : Fragments)
         PHeader.p_align = std::max((uint64_t)PHeader.p_align, F.AddrAlign);
     }
   }
 }
 
-static bool shouldAllocateFileSpace(ArrayRef<ELFYAML::ProgramHeader> Phdrs,
-                                    const ELFYAML::NoBitsSection &S) {
+bool llvm::ELFYAML::shouldAllocateFileSpace(
+    ArrayRef<ELFYAML::ProgramHeader> Phdrs, const ELFYAML::NoBitsSection &S) {
   for (const ELFYAML::ProgramHeader &PH : Phdrs) {
     auto It = llvm::find_if(
         PH.Chunks, [&](ELFYAML::Chunk *C) { return C->Name == S.Name; });
     if (std::any_of(It, PH.Chunks.end(), [](ELFYAML::Chunk *C) {
           return (isa<ELFYAML::Fill>(C) ||
                   cast<ELFYAML::Section>(C)->Type != ELF::SHT_NOBITS);
         }))
       return true;

llvm/test/Object/obj2yaml.test

 # ELF-MIPSEL-NEXT:     Type:            SHT_PROGBITS
 # ELF-MIPSEL-NEXT:     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
 # ELF-MIPSEL-NEXT:     AddressAlign:    0x4
 # ELF-MIPSEL-NEXT:     Content:         0000023C00004224E8FFBD271400BFAF1000B0AF218059000000018E000024240000198E09F8200321E000020000198E09F8200321E00002000002241000B08F1400BF8F0800E0031800BD27
 # ELF-MIPSEL-NEXT:   - Name:            .rel.text
 # ELF-MIPSEL-NEXT:     Type:            SHT_REL
 # ELF-MIPSEL-NEXT:     Link:            .symtab
 # ELF-MIPSEL-NEXT:     AddressAlign:    0x4
+# ELF-MIPSEL-NEXT:     Offset:          0x434
 # ELF-MIPSEL-NEXT:     Info:            .text
 # ELF-MIPSEL-NEXT:     Relocations:
 # ELF-MIPSEL-NEXT:       - Symbol:          _gp_disp
 # ELF-MIPSEL-NEXT:         Type:            R_MIPS_HI16
 # ELF-MIPSEL-NEXT:       - Offset:          0x4
 # ELF-MIPSEL-NEXT:         Symbol:          _gp_disp
 # ELF-MIPSEL-NEXT:         Type:            R_MIPS_LO16
 # ELF-MIPSEL-NEXT:       - Offset:          0x18
 # ELF-MIPSEL-NEXT:         Symbol:          '$.str'
 # ELF-MIPSEL-NEXT:         Type:            R_MIPS_GOT16
 # ELF-MIPSEL-NEXT:       - Offset:          0x1C
 # ELF-MIPSEL-NEXT:         Symbol:          '$.str'
 # ELF-MIPSEL-NEXT:         Type:            R_MIPS_LO16
 # ELF-MIPSEL-NEXT:       - Offset:          0x20
 # ELF-MIPSEL-NEXT:         Symbol:          puts
 # ELF-MIPSEL-NEXT:         Type:            R_MIPS_CALL16
 # ELF-MIPSEL-NEXT:       - Offset:          0x2C
 # ELF-MIPSEL-NEXT:         Symbol:          SomeOtherFunction
 # ELF-MIPSEL-NEXT:         Type:            R_MIPS_CALL16
 # ELF-MIPSEL-NEXT:   - Name:            .data
 # ELF-MIPSEL-NEXT:     Type:            SHT_PROGBITS
 # ELF-MIPSEL-NEXT:     Flags:           [ SHF_WRITE, SHF_ALLOC ]
 # ELF-MIPSEL-NEXT:     AddressAlign:    0x4
+# ELF-MIPSEL-NEXT:     Offset:          0x80
 # ELF-MIPSEL-NEXT:   - Name:            .bss
 # ELF-MIPSEL-NEXT:     Type:            SHT_NOBITS
 # ELF-MIPSEL-NEXT:     Flags:           [ SHF_WRITE, SHF_ALLOC ]
 # ELF-MIPSEL-NEXT:     AddressAlign:    0x4
 # ELF-MIPSEL-NEXT:     Size:            0x4
 # ELF-MIPSEL-NEXT:   - Name:            .mdebug.abi32
 # ELF-MIPSEL-NEXT:     Type:            SHT_PROGBITS
 # ELF-MIPSEL-NEXT:     AddressAlign:    0x1

 # ELF-MIPS64EL-NEXT:     Type:            SHT_PROGBITS
 # ELF-MIPS64EL-NEXT:     Flags:           [ SHF_WRITE, SHF_ALLOC ]
 # ELF-MIPS64EL-NEXT:     AddressAlign:    0x10
 # ELF-MIPS64EL-NEXT:     Content:         '00000000000000000000000000000000'
 # ELF-MIPS64EL-NEXT:   - Name:            .rela.data
 # ELF-MIPS64EL-NEXT:     Type:            SHT_RELA
 # ELF-MIPS64EL-NEXT:     Link:            .symtab
 # ELF-MIPS64EL-NEXT:     AddressAlign:    0x8
+# ELF-MIPS64EL-NEXT:     Offset:          0x410
 # ELF-MIPS64EL-NEXT:     Info:            .data
 # ELF-MIPS64EL-NEXT:     Relocations:
 # ELF-MIPS64EL-NEXT:       - Symbol:          zed
 # ELF-MIPS64EL-NEXT:         Type:            R_MIPS_64
 # ELF-MIPS64EL-NEXT:   - Name:            .bss
 # ELF-MIPS64EL-NEXT:     Type:            SHT_NOBITS
 # ELF-MIPS64EL-NEXT:     Flags:           [ SHF_WRITE, SHF_ALLOC ]
 # ELF-MIPS64EL-NEXT:     AddressAlign:    0x10
+# ELF-MIPS64EL-NEXT:     Offset:          0x50
 # ELF-MIPS64EL-NEXT:   - Name:            .MIPS.options
 # ELF-MIPS64EL-NEXT:     Type:            SHT_MIPS_OPTIONS
 # ELF-MIPS64EL-NEXT:     Flags:           [ SHF_ALLOC, SHF_MIPS_NOSTRIP ]
 # ELF-MIPS64EL-NEXT:     AddressAlign:    0x8
 # ELF-MIPS64EL-NEXT:     EntSize:         0x1
 # ELF-MIPS64EL-NEXT:     Content:         '01280000000000000000000000000000000000000000000000000000000000000000000000000000'
 # ELF-MIPS64EL-NEXT:   - Name:            .pdr
 # ELF-MIPS64EL-NEXT:     Type:            SHT_PROGBITS

llvm/test/tools/obj2yaml/ELF/offset.yaml

+## Check how the "Offset" field is dumped by obj2yaml.
+## For each section we calulate the expected offset.
+## When it does not match the actual offset, we emit the "Offset" key.
+
+# RUN: yaml2obj %s -o %t1.o
+# RUN: obj2yaml %t1.o | FileCheck %s --check-prefix=BASIC
+
+# BASIC:      --- !ELF
+# BASIC-NEXT: FileHeader:
+# BASIC-NEXT:   Class: ELFCLASS64
+# BASIC-NEXT:   Data:  ELFDATA2LSB
+# BASIC-NEXT:   Type:  ET_REL
+# BASIC-NEXT: Sections:
+# BASIC-NEXT:   - Name:         .foo1
+# BASIC-NEXT:     Type:         SHT_PROGBITS
+# BASIC-NEXT:     Content:      '00'
+# BASIC-NEXT:   - Name:         .foo2
+# BASIC-NEXT:     Type:         SHT_PROGBITS
+# BASIC-NEXT:     Content:      '00'
+# BASIC-NEXT:   - Name:         .foo3
+# BASIC-NEXT:     Type:         SHT_PROGBITS
+# BASIC-NEXT:     Content:      '00'
+# BASIC-NEXT:   - Name:         .bar1
+# BASIC-NEXT:     Type:         SHT_PROGBITS
+# BASIC-NEXT:     Offset:       0x100
+# BASIC-NEXT:     Content:      '00'
+# BASIC-NEXT:   - Name:         .bar2
+# BASIC-NEXT:     Type:         SHT_PROGBITS
+# BASIC-NEXT:     AddressAlign: 0x10
+# BASIC-NEXT:     Content:      '00'
+# BASIC-NEXT:   - Name:         .bar3
+# BASIC-NEXT:     Type:         SHT_PROGBITS
+# BASIC-NEXT:     AddressAlign: 0x10
+# BASIC-NEXT:     Offset:       0x200
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_REL
+Sections:
+## The offset of .foo1 by default is 0x40, because it is placed right
+## after the ELF header. In this case we don't dump the "Offset" key,
+## because the file offset is naturally expected.
+  - Name:         .foo1
+    Type:         SHT_PROGBITS
+    Size:         1
+    Offset:       [[FIRSTOFF=<none>]]
+    AddressAlign: [[FIRSTADDRALIGN=0]]
+## Offset of .foo2 == offset of .foo1 + size of .foo1.
+## We don't dump the "Offset" key in this case.
+## sh_offset of .foo2 is 0x41.
+  - Name: .foo2
+    Type: SHT_PROGBITS
+    Size: 1
+## Offset of .foo3 == offset of .foo2 + size of .foo2,
+## We don't dump the "Offset" key in this case.
+## sh_offset of .foo3 is 0x42.
+  - Name: .foo3
+    Type: SHT_PROGBITS
+    Size: 1
+## Offset of .bar1 != offset of .foo3 + size of .foo3.
+## We dump the "Offset" key in this case.
+## sh_offset of .bar1 is 0x100.
+  - Name:   .bar1
+    Type:   SHT_PROGBITS
+    Offset: 0x100
+    Size: 1
+## [Offset of .bar1 + size of .bar1] aligned by 0x10 is equal to the offset
+## of .bar2. We don't dump the "Offset" key in this case.
+## sh_offset of .bar2 is 0x110.
+  - Name:         .bar2
+    Type:         SHT_PROGBITS
+    AddressAlign: 0x10
+    Offset:       0x110
+    Size:         1
+## [Offset of .bar2 + size of .bar2] aligned by 0x10 is not equal to the offset
+## of .bar3. We dump the "Offset" key in this case.
+## sh_offset of .bar3 is 0x200.
+  - Name:         .bar3
+    Type:         SHT_PROGBITS
+    AddressAlign: 0x10
+    Offset:       0x200
+
+## Show we dump the "Offset" key for the first section when
+## it has an unexpected file offset.
+
+# RUN: yaml2obj %s -DFIRSTOFF=0x40 -o %t2a.o
+# RUN: obj2yaml %t2a.o | FileCheck %s --check-prefix=BASIC
+# RUN: yaml2obj %s -DFIRSTOFF=0x41 -o %t2b.o
+# RUN: obj2yaml %t2b.o | FileCheck %s --check-prefix=FIRSTSEC
+
+# FIRSTSEC:      Sections:
+# FIRSTSEC-NEXT:   - Name:    .foo1
+# FIRSTSEC-NEXT:     Type:    SHT_PROGBITS
+# FIRSTSEC-NEXT:     Offset:  0x41
+# FIRSTSEC-NEXT:     Content: '00'
+
+## Test that we take the alignment of the first section into account
+## when calculating the expected offset for it. In this case we don't
+## dump the "Offset", because it is expected.
+
+# RUN: yaml2obj %s -DFIRSTOFF=0x80 -DFIRSTADDRALIGN=0x80 -o %t3.o
+# RUN: obj2yaml %t3.o | FileCheck %s --check-prefix=FIRSTSECALIGN
+
+# FIRSTSECALIGN:      - Name:         .foo1
+# FIRSTSECALIGN-NEXT:   Type:         SHT_PROGBITS
+# FIRSTSECALIGN-NEXT:   AddressAlign: 0x80
+# FIRSTSECALIGN-NEXT:   Content:      '00'
+# FIRSTSECALIGN-NEXT: - Name:
+
+## Test that we take the program headers offset and size into account when calculating
+## the expected file offset of the first section.
+
+# RUN: yaml2obj %s --docnum=2 -o %t4a.o
+# RUN: obj2yaml %t4a.o | FileCheck %s --check-prefix=FIRSTSECPHDRS
+## The expected file offset of the first section is:
+## 0x40 (start of program headers) + 0x38 (size of program headers) * 2(number of program headers) = 0xB0
+# RUN: yaml2obj %s --docnum=2 -DFIRSTOFF=0xB0 -o %t4b.o
+# RUN: obj2yaml %t4b.o | FileCheck %s --check-prefix=FIRSTSECPHDRS
+# RUN: yaml2obj %s --docnum=2 -DFIRSTOFF=0xB1 -o %t4c.o
+# RUN: obj2yaml %t4c.o | FileCheck %s --check-prefixes=FIRSTSECPHDRS,FIRSTSECPHDRSOFFSET
+
+#       FIRSTSECPHDRS:      Sections:
+#       FIRSTSECPHDRS-NEXT:   - Name:   .foo
+#       FIRSTSECPHDRS-NEXT:     Type:   SHT_PROGBITS
+# FIRSTSECPHDRSOFFSET-NEXT:     Offset: 0xB1
+#       FIRSTSECPHDRS-NEXT:   ...
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_REL
+Sections:
+  - Name:   .foo
+    Type:   SHT_PROGBITS
+    Offset: [[FIRSTOFF=<none>]]
+ProgramHeaders:
+  - Type: PT_LOAD
+  - Type: PT_LOAD
+
+## Test that when there are no program headers in the file, we don't take SHT_NOBITS
+## section sizes into account, but respect their alignment when calculating the expected
+## section offsets.
+
+# RUN: yaml2obj %s --docnum=3 -o %t5.o
+# RUN: obj2yaml %t5.o | FileCheck %s --check-prefix=NOBITS
+
+# NOBITS:      Sections:
+# NOBITS-NEXT:   - Name:         .progbits1
+# NOBITS-NEXT:     Type:         SHT_PROGBITS
+# NOBITS-NEXT:     Content:      '00'
+# NOBITS-NEXT:   - Name:         .nobits1
+# NOBITS-NEXT:     Type:         SHT_NOBITS
+# NOBITS-NEXT:     Size:         0x10
+# NOBITS-NEXT:   - Name:         .progbits2
+# NOBITS-NEXT:     Type:         SHT_PROGBITS
+# NOBITS-NEXT:     Content:      '0000'
+# NOBITS-NEXT:   - Name:         .nobits2
+# NOBITS-NEXT:     Type:         SHT_NOBITS
+# NOBITS-NEXT:     AddressAlign: 0x100
+# NOBITS-NEXT:     Size:         0x100
+# NOBITS-NEXT:   - Name:         .progbits3
+# NOBITS-NEXT:     Type:         SHT_PROGBITS
+# NOBITS-NEXT:     Content:      '000000'
+# NOBITS-NEXT: ...
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_REL
+Sections:
+## sh_offset == 0x40.
+  - Name: .progbits1
+    Type: SHT_PROGBITS
+    Size: 0x1
+## sh_offset == 0x41.
+  - Name: .nobits1
+    Type: SHT_NOBITS
+    Size: 0x10
+## sh_offset == 0x41.
+  - Name: .progbits2
+    Type: SHT_PROGBITS
+    Size: 0x2
+## sh_offset == 0x100.
+  - Name:         .nobits2
+    Type:         SHT_NOBITS
+    Size:         0x100
+    AddressAlign: 0x100
+## sh_offset == 0x100.
+  - Name: .progbits3
+    Type: SHT_PROGBITS
+    Size: 0x3
+
+## Check that we might take sizes of SHT_NOBITS sections into account when calculating
+## the expected offsets when there are program headers in the file. The rule is the following:
+## we assume that the file space is allocated for the SHT_NOBITS section when there are
+## other non-nobits sections in the same segment that follows it.
+
+# RUN: yaml2obj %s --docnum=4 -o %t6.o
+# RUN: obj2yaml %t6.o | FileCheck %s --check-prefix=NOBITS-PHDRS
+
+# NOBITS-PHDRS:      Sections:
+# NOBITS-PHDRS-NEXT:   - Name:    .nobits1
+# NOBITS-PHDRS-NEXT:     Type:    SHT_NOBITS
+# NOBITS-PHDRS-NEXT:     Size:    0x1
+# NOBITS-PHDRS-NEXT:   - Name:    .progbits
+# NOBITS-PHDRS-NEXT:     Type:    SHT_PROGBITS
+# NOBITS-PHDRS-NEXT:     Content: '0000'
+# NOBITS-PHDRS-NEXT:   - Name:    .nobits3
+# NOBITS-PHDRS-NEXT:     Type:    SHT_NOBITS
+# NOBITS-PHDRS-NEXT:     Size:    0x100
+# NOBITS-PHDRS-NEXT:   - Name:    .nobits4
+# NOBITS-PHDRS-NEXT:     Type:    SHT_NOBITS
+# NOBITS-PHDRS-NEXT:     Size:    0x200
+# NOBITS-PHDRS-NEXT:   - Name:    .nobits5
+# NOBITS-PHDRS-NEXT:     Type:    SHT_NOBITS
+# NOBITS-PHDRS-NEXT:     Offset:  0x100
+# NOBITS-PHDRS-NEXT:     Size:    0x300
+# NOBITS-PHDRS-NEXT: ...
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_REL
+Sections:
+## sh_offset == 0xe8.
+  - Name: .nobits1
+    Type: SHT_NOBITS
+    Size: 0x1
+## sh_offset == 0xe9.
+  - Name: .progbits
+    Type: SHT_PROGBITS
+    Size: 0x2
+## sh_offset == 0xeb.
+  - Name: .nobits3
+    Type: SHT_NOBITS
+    Size: 0x100
+## sh_offset == 0xeb.
+  - Name: .nobits4
+    Type: SHT_NOBITS
+    Size: 0x200
+## sh_offset == 0x100.
+  - Name:   .nobits5
+    Type:   SHT_NOBITS
+    Size:   0x300
+    Offset: 0x100
+ProgramHeaders:
+  - Type:     PT_LOAD
+    FirstSec: .nobits1
+    LastSec:  .progbits
+  - Type:     PT_LOAD
+    FirstSec: .nobits3
+    LastSec:  .nobits4
+  - Type:     PT_LOAD
+    FirstSec: .nobits5
+    LastSec:  .nobits5

llvm/tools/obj2yaml/elf2yaml.cpp

bool ELFDumper<ELFT>::shouldPrintSection(const ELFYAML::Section &S,
   // assume they are placed at the end.
   if (S.Type == ELF::SHT_STRTAB || S.Type == ELF::SHT_SYMTAB ||
       S.Type == ELF::SHT_DYNSYM)
     return S.Flags.getValueOr(ELFYAML::ELF_SHF(0)) & ELF::SHF_ALLOC;
 
   return true;
 }
 
+template <class ELFT>
+static void dumpSectionOffsets(const typename ELFT::Ehdr &Header,
+                               ArrayRef<ELFYAML::ProgramHeader> Phdrs,
+                               std::vector<std::unique_ptr<ELFYAML::Chunk>> &V,
+                               ArrayRef<typename ELFT::Shdr> S) {
+  uint64_t ExpectedOffset;
+  if (Header.e_phoff > 0)
+    ExpectedOffset = Header.e_phoff + Header.e_phentsize * Header.e_phnum;
+  else
+    ExpectedOffset = sizeof(typename ELFT::Ehdr);
+
+  for (const std::unique_ptr<ELFYAML::Chunk> &C :
+       makeArrayRef(V).drop_front()) {
+    ELFYAML::Section &Sec = *cast<ELFYAML::Section>(C.get());
+    const typename ELFT::Shdr &SecHdr = S[Sec.OriginalSecNdx];
+
+    ExpectedOffset =
+        alignTo(ExpectedOffset, SecHdr.sh_addralign ? SecHdr.sh_addralign : 1);
+
+    // We only set the "Offset" field when it can't be naturally derived
+    // from the offset and size of the previous section. This reduces
+    // the noise in the YAML output.
+    if (SecHdr.sh_offset != ExpectedOffset)
+      Sec.Offset = (yaml::Hex64)SecHdr.sh_offset;
+
+    if (Sec.Type == ELF::SHT_NOBITS &&
+        !ELFYAML::shouldAllocateFileSpace(Phdrs,
+                                          *cast<ELFYAML::NoBitsSection>(&Sec)))
+      ExpectedOffset = SecHdr.sh_offset;
+    else
+      ExpectedOffset = SecHdr.sh_offset + SecHdr.sh_size;
+  }
+}
+
 template <class ELFT> Expected<ELFYAML::Object *> ELFDumper<ELFT>::dump() {
   auto Y = std::make_unique<ELFYAML::Object>();
 
   // Dump header. We do not dump EPh* and ESh* fields. When not explicitly set,
   // the values are set by yaml2obj automatically and there is no need to dump
   // them here.
   Y->Header.Class = ELFYAML::ELF_ELFCLASS(Obj.getHeader().getFileClass());
   Y->Header.Data = ELFYAML::ELF_ELFDATA(Obj.getHeader().getDataEncoding());
template <class ELFT> Expected<ELFYAML::Object *> ELFDumper<ELFT>::dump() {
 
   // Dump program headers.
   Expected<std::vector<ELFYAML::ProgramHeader>> PhdrsOrErr =
       dumpProgramHeaders(Chunks);
   if (!PhdrsOrErr)
     return PhdrsOrErr.takeError();
   Y->ProgramHeaders = std::move(*PhdrsOrErr);
 
+  dumpSectionOffsets<ELFT>(Obj.getHeader(), Y->ProgramHeaders, Chunks,
+                           Sections);
+
   // Dump DWARF sections.
   Y->DWARF = dumpDWARFSections(Chunks);
 
   llvm::erase_if(Chunks, [this, &Y](const std::unique_ptr<ELFYAML::Chunk> &C) {
     const ELFYAML::Section &S = cast<ELFYAML::Section>(*C.get());
     return !shouldPrintSection(S, Sections[S.OriginalSecNdx], Y->DWARF);
   });
 
for (const typename ELFT::Phdr &Phdr : *PhdrsOrErr) {
     // It is not possible to have a non-Section chunk, because
     // obj2yaml does not create Fill chunks.
     for (const std::unique_ptr<ELFYAML::Chunk> &C : Chunks) {
       ELFYAML::Section &S = cast<ELFYAML::Section>(*C.get());
       if (isInSegment<ELFT>(S, Sections[S.OriginalSecNdx], Phdr)) {
         if (!PH.FirstSec)
           PH.FirstSec = S.Name;
         PH.LastSec = S.Name;
+        PH.Chunks.push_back(C.get());
       }
     }
 
     Ret.push_back(PH);
   }
 
   return Ret;
 }
auto GetDumper = [this](unsigned Type)
     if (Obj.getHeader().e_machine == ELF::EM_MIPS &&
         Type == ELF::SHT_MIPS_ABIFLAGS)
       return [this](const Elf_Shdr *S) { return dumpMipsABIFlags(S); };
 
     switch (Type) {
     case ELF::SHT_DYNAMIC:
       return [this](const Elf_Shdr *S) { return dumpDynamicSection(S); };
     case ELF::SHT_SYMTAB_SHNDX:
       return [this](const Elf_Shdr *S) { return dumpSymtabShndxSection(S); };
     case ELF::SHT_REL:
     case ELF::SHT_RELA:
       return [this](const Elf_Shdr *S) { return dumpRelocSection(S); };
     case ELF::SHT_RELR:
       return [this](const Elf_Shdr *S) { return dumpRelrSection(S); };
     case ELF::SHT_GROUP:
       return [this](const Elf_Shdr *S) { return dumpGroupSection(S); };
     case ELF::SHT_NOBITS:
       return [this](const Elf_Shdr *S) { return dumpNoBitsSection(S); };
     case ELF::SHT_NOTE:
       return [this](const Elf_Shdr *S) { return dumpNoteSection(S); };