[llvm-objcopy] -O binary: consider SHT_NOBITS sections to be empty

[pattop]

Summary

This is consistent with BFD objcopy.

Previously llvm objcopy would allocate space for SHT_NOBITS sections
often resulting in enormous binary files.

New test case (binary-paddr.test %t6).

[MaskRay]

I did not add Sec.Type != SHT_NOBITS && in D79229 because I did not find a use case for this rule... If yours is a real usage, I think it is fine.

llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test

214

I'll use a smaller address. If something goes off, this would not create a 260MB output.

[pattop]

Yes, this came from a real use case.

Thanks!

[pattop]

I forgot to mention that I do not have commit access. If this is acceptable someone else will need to commit on my behalf. Thanks again.

[pattop]

llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test

214

Do you want me to update the patch with this changed, or are you happy to adjust it when committing?

[MaskRay]

llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test

214

Please update it.

I am happy with the patch, just wanted to give others some time reviewing it as well.

[pattop]

Use a smaller address in test case. This means that if something goes wrong we don't try to make a huge file.

[jhenderson]

I haven't verified the correctness of this versus GNU objcopy, but assuming the two behave identically after this patch, this is fine by me. I'd like someone else to commit though, as I have too much else on my plate currently.

llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test

214

This could be as small as 0x4000 maybe?

[pattop]

Use smaller address in test case.

[MaskRay]

llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test

190

I'll add --ignore-case and commit.

llvm/test/tools/llvm-objcopy/ELF/binary-paddr.test

- Name:         .text
     AddressAlign: 0x1000
     Content:      "3232"
   - Name:         gap
     Type:         Fill
     Size:         0xffd
   - Name:         .data
     Type:         SHT_PROGBITS
     Flags:        [ SHF_ALLOC, SHF_WRITE ]
+
+## NOBITS sections should not appear in output.
+# RUN: yaml2obj --docnum=6 %s -o %t6
+# RUN: llvm-objcopy -O binary %t6 %t6.out
+# RUN: od -A x -t x2 %t6.out | FileCheck %s --check-prefix=SKIPNOBITS --ignore-case
+
+# SKIPNOBITS:      000000 c3c3 c3c3
+# SKIPNOBITS-NEXT: 000004
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:         .bss
+    Type:         SHT_NOBITS
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x1000
+    AddressAlign: 0x1000
+    Size:         0x123
+  - Name:         gap
+    Type:         Fill
+    Size:         0xffd
+  - Name:         .text
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:      0x4000
+    AddressAlign: 0x1000
+    Content:      "c3c3c3c3"

llvm/tools/llvm-objcopy/ELF/Object.cpp

Error BinaryWriter::finalize() {
   // p_offset and p_paddr. Also compute the minimum LMA of all non-empty
   // sections as MinAddr. In the output, the contents between address 0 and
   // MinAddr will be skipped.
   uint64_t MinAddr = UINT64_MAX;
   for (SectionBase &Sec : Obj.allocSections()) {
     if (Sec.ParentSegment != nullptr)
       Sec.Addr =
           Sec.Offset - Sec.ParentSegment->Offset + Sec.ParentSegment->PAddr;
-    if (Sec.Size > 0)
+    if (Sec.Type != SHT_NOBITS && Sec.Size > 0)
       MinAddr = std::min(MinAddr, Sec.Addr);
   }
 
   // Now that every section has been laid out we just need to compute the total
   // file size. This might not be the same as the offset returned by
   // layoutSections, because we want to truncate the last segment to the end of
   // its last non-empty section, to match GNU objcopy's behaviour.
   TotalSize = 0;