[llvm-objcopy] -O binary: align sh_offset for section changed from SHT_NOBITS

[MaskRay]

Summary

For a SHT_NOBITS section like .bss, its sh_offset is typically not
aligned by sh_addralign. If it is converted to SHT_PROGBITS by
--set-section-flags .bss=alloc,contents, we should conceptually align
it when computing the output size for -O binary. Otherwise the output
size may be smaller than GNU objcopy produced output.

• binary-no-paddr.test has a case with non-sensical p_paddr=1 which has a changed behavior. Update it.

Close https://github.com/llvm/llvm-project/issues/55246

[MaskRay]

use -NEXT:

[MaskRay]

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

7–8

WIll change to:

For the p_paddr=0 case: .text and .data are rewritten to the same place. The size is 4.

[jhenderson]

LGTM, I think (took me a while to wrap my head around it, but the behaviour matches GNU, so that's a win in my book).

llvm/lib/ObjCopy/ELF/ELFObject.cpp

 
 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 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's type is changed from SHT_NOBITS due to --set-section-flags,
+    // Offset may not be aligned. Align it to max(Align, 1).
     if (Sec.ParentSegment != nullptr)
-      Sec.Addr =
-          Sec.Offset - Sec.ParentSegment->Offset + Sec.ParentSegment->PAddr;
+      Sec.Addr = alignTo(Sec.Offset - Sec.ParentSegment->Offset +
+                             Sec.ParentSegment->PAddr,
+                         std::max(Sec.Align, uint64_t(1)));
     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.

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

-# RUN: yaml2obj -D PADDR=1 %s -o %t1
+# RUN: yaml2obj -D PADDR1=0x1000 -D PADDR2=0x1004 %s -o %t1
 # RUN: llvm-objcopy -O binary %t1 %t1.out
 # RUN: od -t x2 -v %t1.out | FileCheck %s --ignore-case
-# RUN: wc -c < %t1.out | FileCheck %s --check-prefix=SIZE
 
 ## When all p_paddr fields are 0, GNU objcopy resets LMA to VMA
 ## and gives a different output.
 ## https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=6ffd79000b45e77b3625143932ffbf781b6aecab
-## We don't implement this special rule. The p_paddr=0 output is the same as
-## the p_paddr=1 case.
-# RUN: yaml2obj -D PADDR=0 %s -o %t0
+## We don't implement this special rule. For the p_paddr=0 case: .text and
+## .data are rewritten to the same place. The size is 4. # the p_paddr=1 case.
+# RUN: yaml2obj %s -o %t0
 # RUN: llvm-objcopy -O binary %t0 %t0.out
-# RUN: cmp %t1.out %t0.out
+# RUN: od -t x2 -v %t0.out | FileCheck %s --check-prefix=CHECK0 --ignore-case
 
 !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:    0x0000000000001000
     Content:         "c3c3c3c3"
   - Name:            .data
     Type:            SHT_PROGBITS
     Flags:           [ SHF_ALLOC ]
     Address:         0x1004
     AddressAlign:    0x0000000000000004
     Content:         "3232"
 ProgramHeaders:
   - Type:     PT_LOAD
     Flags:    [ PF_X, PF_R ]
     VAddr:    0x1000
-    PAddr:    [[PADDR]]
+    PAddr:    [[PADDR1=0]]
     FirstSec: .text
     LastSec:  .text
   - Type:     PT_LOAD
     Flags:    [ PF_R, PF_W ]
     VAddr:    0x1004
-    PAddr:    [[PADDR]]
+    PAddr:    [[PADDR2=0]]
     FirstSec: .data
     LastSec:  .data
 
-# CHECK: 0000000 3232 c3c3
-# SIZE:  4
+# CHECK:       0000000 c3c3 c3c3 3232
+# CHECK-NEXT:  0000006
+
+# CHECK0:      0000000 3232 c3c3
+# CHECK0-NEXT: 0000004

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

- Name:         gap
     Type:         Fill
     Size:         0xffd
   - Name:         .text
     Type:         SHT_PROGBITS
     Flags:        [ SHF_ALLOC, SHF_EXECINSTR ]
     Address:      0x4000
     AddressAlign: 0x1000
     Content:      "c3c3c3c3"
+
+## If .bss is converted to non-SHT_NOBITS, align its new offset. This may affect
+## the output size.
+# RUN: yaml2obj --docnum=7 %s -o %t7
+# RUN: llvm-objcopy -O binary --set-section-flags .bss=alloc,contents %t7 %t7.out
+# RUN: od -A x -t x2 %t7.out | FileCheck %s --check-prefix=FILLNOBITS --ignore-case
+
+# FILLNOBITS:      000000 c3c3 0000 0000 0000 0000 0000 0000 0000
+# FILLNOBITS-NEXT: 000010 0000 00
+# FILLNOBITS-NEXT: 000013
+
+--- !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:      "c3c3"
+  - Name:         .bss
+    Type:         SHT_NOBITS
+    Flags:        [ SHF_ALLOC ]
+    ## sh_offset is not aligned.
+    ShOffset:     0x1002
+    Size:         0x3
+    AddressAlign: 0x10
+ProgramHeaders:
+  - Type:     PT_LOAD
+    Flags:    [ PF_R, PF_W, PF_X ]
+    Offset:   0x1000
+    VAddr:    0x1000
+    PAddr:    0x1000
+    FileSize: 0x2
+    MemSize:  0x13
+    Align:    0x1000