[llvm-objcopy] -O binary: skip empty sections

[MaskRay]

Summary

After SHF_ALLOC sections are ordered by LMA:

• If initial sections are empty, GNU objcopy skips their contents while we emit leading zeros. (binary-paddr.test %t4)
• If trailing sections are empty, GNU objcopy skips their contents while we emit trailing zeros. (binary-paddr.test %t5)

This patch matches GNU objcopy's behavior. Linkers don't keep p_memsz
PT_LOAD segments. Such empty sections would not have a containing
PT_LOAD and Section::ParentSegment might be null if linkers fail to
optimize the file offsets (lld D79254).

In particular, without D79254, the arm Linux kernel's multi_v5_defconfig
depends on this behavior: in vmlinux, an empty .text_itcm is mapped at
a very high address (0xfffe0000) but the kernel does not expect `objcopy
-O binary` to create a very large arch/arm/boot/Image
(0xfffe0000-0xc0000000 ~= 1GiB). See
https://bugs.llvm.org/show_bug.cgi?id=45632

[nathanchance]

Thanks for the fix! I can confirm that a multi_v5_defconfig kernel looks normal size wise and boots in QEMU. I will do a set of kernel builds overnight to make sure that nothing else regresses.

[jhenderson]

LGTM, with a couple of nits.

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

138

Nit: I'd find it clearer if there was a blank line after the RUN lines before the SKIP-EMPTY lines.

Aside: I'm slightly surprised this works, given that -EMPTY is also a FileCheck suffix!

177

Nit: blank line before this line.

[MaskRay]

Address comments

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

138

Switched to SKIPEMPTY.

The -EMPTY -DAG` arguments may suggest that we probably should prefer FOO_BAR to FOO-BAR...

[MaskRay]

Simplify tests

[vitalybuka]

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/41020/steps/check-llvm%20ubsan/logs/stdio

--
/b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/Object.cpp:147:51: runtime error: addition of unsigned offset to 0x7f7a7b973000 overflowed to 0x7f7a7b972000
    #0 0x48e24c in llvm::objcopy::elf::SectionWriter::visit(llvm::objcopy::elf::Section const&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/Object.cpp:147:51
    #1 0x495b37 in llvm::objcopy::elf::BinaryWriter::write() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/Object.cpp:2214:9
    #2 0x459738 in llvm::objcopy::elf::writeOutput(llvm::objcopy::CopyConfig const&, llvm::objcopy::elf::Object&, llvm::objcopy::Buffer&, llvm::objcopy::elf::ElfType) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:757:18
    #3 0x459d09 in llvm::objcopy::elf::executeObjcopyOnBinary(llvm::objcopy::CopyConfig const&, llvm::object::ELFObjectFileBase&, llvm::objcopy::Buffer&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:819:17
    #4 0x43f1cb in executeObjcopyOnBinary(llvm::objcopy::CopyConfig&, llvm::object::Binary&, llvm::objcopy::Buffer&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/llvm-objcopy.cpp:173:12
    #5 0x43cd75 in executeObjcopy /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/llvm-objcopy.cpp:301:21
    #6 0x43cd75 in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/llvm-objcopy.cpp:374:19
    #7 0x7f7a7b26109a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #8 0x40ab79 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llvm-objcopy+0x40ab79)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/Object.cpp:147:51 in 

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failing Tests (1):
  LLVM :: tools/llvm-objcopy/ELF/binary-paddr.test

[MaskRay]

In D79229#2016967, @vitalybuka wrote:

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/41020/steps/check-llvm%20ubsan/logs/stdio

--
/b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/Object.cpp:147:51: runtime error: addition of unsigned offset to 0x7f7a7b973000 overflowed to 0x7f7a7b972000
    #0 0x48e24c in llvm::objcopy::elf::SectionWriter::visit(llvm::objcopy::elf::Section const&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/Object.cpp:147:51
    #1 0x495b37 in llvm::objcopy::elf::BinaryWriter::write() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/Object.cpp:2214:9
    #2 0x459738 in llvm::objcopy::elf::writeOutput(llvm::objcopy::CopyConfig const&, llvm::objcopy::elf::Object&, llvm::objcopy::Buffer&, llvm::objcopy::elf::ElfType) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:757:18
    #3 0x459d09 in llvm::objcopy::elf::executeObjcopyOnBinary(llvm::objcopy::CopyConfig const&, llvm::object::ELFObjectFileBase&, llvm::objcopy::Buffer&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:819:17
    #4 0x43f1cb in executeObjcopyOnBinary(llvm::objcopy::CopyConfig&, llvm::object::Binary&, llvm::objcopy::Buffer&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/llvm-objcopy.cpp:173:12
    #5 0x43cd75 in executeObjcopy /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/llvm-objcopy.cpp:301:21
    #6 0x43cd75 in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/llvm-objcopy.cpp:374:19
    #7 0x7f7a7b26109a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #8 0x40ab79 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llvm-objcopy+0x40ab79)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llvm-objcopy/ELF/Object.cpp:147:51 in 

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
********************
Failing Tests (1):
  LLVM :: tools/llvm-objcopy/ELF/binary-paddr.test

Thanks! Fixed by 762fb1c40eea6878c2d6a1f0f1fc7915c8747981

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

 
 ## The computed LMA of a section not in a PT_LOAD equals its sh_addr.
 
 # RUN: yaml2obj --docnum=2 %s -o %t2
 # RUN: llvm-objcopy -O binary %t2 %t2.out
 # RUN: od -A x -t x2 %t2.out | FileCheck %s --check-prefix=CHECK2 --ignore-case
 # RUN: wc -c %t2.out | FileCheck %s --check-prefix=SIZE2
 
-## The computed LMA of .data is 0x4000. The minimum LMA of all sections is 0x1000.
+## The computed LMA of .data is 0x4000. The minimum LMA of all non-empty sections is 0x1000.
 ## The content of .data will be written at 0x4000-0x1000 = 0x3000.
 # CHECK2:      000000 c3c3 c3c3 0000 0000 0000 0000 0000 0000
 # CHECK2-NEXT: 000010 0000 0000 0000 0000 0000 0000 0000 0000
 # CHECK2-NEXT: *
 # CHECK2-NEXT: 003000 3232
 # SIZE2:       12290
 
 --- !ELF

 
 ## Check that we use sh_offset instead of sh_addr to decide where to write section contents.
 
 # RUN: yaml2obj --docnum=3 %s -o %t3
 # RUN: llvm-objcopy -O binary %t3 %t3.out
 # RUN: od -A x -t x2 %t3.out | FileCheck %s --check-prefix=CHECK3 --ignore-case
 # RUN: wc -c %t3.out | FileCheck %s --check-prefix=SIZE3
 
-## The minimum LMA of all sections is 0x1000.
+## The minimum LMA of all non-empty sections is 0x1000.
 ## The content of .data will be written at 0x3000-0x1000 = 0x2000.
 # CHECK3:      000000 c3c3 c3c3 0000 0000 0000 0000 0000 0000
 # CHECK3-NEXT: 000010 0000 0000 0000 0000 0000 0000 0000 0000
 # CHECK3-NEXT: *
 # CHECK3-NEXT: 002000 3232
 # SIZE3:       8194
 
 --- !ELF
- Name:            .data
     AddressAlign:    0x1000
     Content:         "3232"
 ProgramHeaders:
   - Type: PT_LOAD
     Flags: [ PF_R, PF_W ]
     VAddr: 0x3000
     Sections:
       - Section: .data
+
+## The first section (.text) is empty. Test that we skip its LMA until the first
+## non-empty section, otherwise we would leave a large number of leading zeroes.
+# RUN: yaml2obj --docnum=4 %s -o %t4
+# RUN: llvm-objcopy -O binary %t4 %t4.out
+# RUN: od -A x -t x2 %t4.out | FileCheck %s --check-prefix=SKIPEMPTY
+
+# SKIPEMPTY:      000000 3232
+# SKIPEMPTY-NEXT: 000002
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:         .text
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:      0x1000
+    AddressAlign: 0x1000
+  - Name:         gap
+    Type:         Fill
+    Size:         0x1000
+  - Name:         .data
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_WRITE ]
+    Content:      "3232"
+
+## The last section (.data) is empty. Test that we stop dumping after the last
+## non-empty section, otherwise we would leave a large number of trailing zeroes.
+# RUN: yaml2obj --docnum=5 %s -o %t5
+# RUN: llvm-objcopy -O binary %t5 %t5.out
+# RUN: od -A x -t x2 %t5.out | FileCheck %s --check-prefix=SKIPEMPTY
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:         .text
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:      0x1000
+    AddressAlign: 0x1000
+    Content:      "3232"
+  - Name:         gap
+    Type:         Fill
+    Size:         0xffd
+  - Name:         .data
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_WRITE ]

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

 Error BinaryWriter::write() {
   for (const SectionBase &Sec : Obj.allocSections())
     Sec.accept(*SecWriter);
   return Buf.commit();
 }
 
 Error BinaryWriter::finalize() {
   // Compute the section LMA based on its sh_offset and the containing segment's
-  // p_offset and p_paddr. Also compute the minimum LMA of all sections as
-  // MinAddr. In the output, the contents between address 0 and MinAddr will be
-  // skipped.
+  // 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)
       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 section, to match GNU objcopy's behaviour.
+  // its last non-empty section, to match GNU objcopy's behaviour.
   TotalSize = 0;
   for (SectionBase &Sec : Obj.allocSections()) {
     Sec.Offset = Sec.Addr - MinAddr;
-    if (Sec.Type != SHT_NOBITS)
+    if (Sec.Type != SHT_NOBITS && Sec.Size > 0)
       TotalSize = std::max(TotalSize, Sec.Offset + Sec.Size);
   }
 
   if (Error E = Buf.allocate(TotalSize))
     return E;
   SecWriter = std::make_unique<BinarySectionWriter>(Buf);
   return Error::success();
 }