[PATCH] D90897: [llvm-objcopy] --only-keep-debug: place zero-size segment according to its parent segment

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: grimar, jhenderson, kongyi, rupprecht.
Herald added subscribers: llvm-commits, abrachet, emaste.
Herald added a reviewer: espindola.
Herald added a reviewer: alexshap.
Herald added a project: LLVM.
MaskRay requested review of this revision.

Alternative to D62055 <https://reviews.llvm.org/D62055>. sectionWithinSegment() treats an empty section as having
a size of 1. Due to the rule, an empty .tdata will not be attributed to an
empty PT_TLS. (The empty p_align=64 PT_TLS is for Android Bionic's TCB
compatibility (ELF-TLS). See https://reviews.llvm.org/D62055#1507426)

Currently --only-keep-debug will not layout a segment with no section
(layoutSegmentsForOnlyKeepDebug()), thus p_offset of PT_TLS can go past the end
of the file. The strange p_offset can trigger validation errors for subsequent
tools, e.g. llvm-objcopy errors when reading back the separate debug file
(readProgramHeaders()).

This patch places such an empty segment according to its parent segment.  This
special cases works for the empty PT_TLS used in Android. For a non-empty
segment, it should have at least one non-empty section and will be handled by
the normal code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90897

Files:
  llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
  llvm/tools/llvm-objcopy/ELF/Object.cpp


Index: llvm/tools/llvm-objcopy/ELF/Object.cpp
===================================================================
--- llvm/tools/llvm-objcopy/ELF/Object.cpp
+++ llvm/tools/llvm-objcopy/ELF/Object.cpp
@@ -2310,6 +2310,12 @@
                                                uint64_t HdrEnd) {
   uint64_t MaxOffset = 0;
   for (Segment *Seg : Segments) {
+    // An empty segment has no containing section (see sectionWithinSegment).
+    // If it has a parent segment, copy the parent segment's size field.  This
+    // works for empty PT_TLS.
+    if (Seg->ParentSegment != nullptr && Seg->MemSize == 0)
+      Seg->Offset = Seg->ParentSegment->Offset;
+
     const SectionBase *FirstSec = Seg->firstSection();
     if (Seg->Type == PT_PHDR || !FirstSec)
       continue;
Index: llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
===================================================================
--- llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
+++ llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
@@ -222,3 +222,70 @@
     Flags: [ SHF_ALLOC ]
 DynamicSymbols: []
 Symbols: []
+
+## PT_TLS and .tdata are empty. Test that we set its p_offset to the parent
+## segment's p_offset. If we don't rewrite the p_offset of PT_TLS and the deleted
+## bytes are large, p_offset can be larger than the file size, and trigger
+## validation errors with subsequent tools.
+# RUN: yaml2obj --docnum=4 %s -o %t4
+# RUN: llvm-objcopy --only-keep-debug %t4 %t4.dbg
+# RUN: llvm-readelf -S -l %t4.dbg | FileCheck --check-prefix=CHECK4 %s
+
+# CHECK4:      [Nr] Name        Type     Address          Off    Size   ES Flg Lk Inf Al
+# CHECK4:      [ 1] .note       NOTE     0000000000000200 000200 000001 00   A  0   0 512
+# CHECK4-NEXT: [ 2] .text       NOBITS   0000000000000201 000201 000001 00  AX  0   0  0
+# CHECK4-NEXT: [ 3] .tdata      NOBITS   0000000000001240 000240 000000 00 WAT  0   0 64
+# CHECK4-NEXT: [ 4] .got        NOBITS   0000000000001240 000240 000008 00  WA  0   0 0
+
+# CHECK4:      Type Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
+# CHECK4-NEXT: LOAD 0x000200 0x0000000000000200 0x0000000000000200 0x000001 0x000002 R E 0x1000
+# CHECK4-NEXT: LOAD 0x000240 0x0000000000001240 0x0000000000001240 0x000000 0x000008 RW  0x1000
+# CHECK4-NEXT: TLS  0x000240 0x0000000000001240 0x0000000000001240 0x000000 0x000000 R   0x40
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:         .note
+    Type:         SHT_NOTE
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x200
+    AddressAlign: 0x200
+    Size:         1
+  - Name:         .text
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:      0x201
+    Size:         1
+  - Name:         .tdata
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_WRITE, SHF_TLS ]
+    Address:      0x1240   # Ensure Address=0x1000+Offset
+    AddressAlign: 0x40
+  - Name:         .got
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_WRITE ]
+    Size:         8
+ProgramHeaders:
+  - Type:     PT_LOAD
+    Flags:    [ PF_R, PF_X ]
+    VAddr:    0x200
+    Align:    0x1000
+    Sections:
+      - Section: .note
+      - Section: .text
+  - Type:     PT_LOAD
+    Flags:    [ PF_R, PF_W ]
+    VAddr:    0x1240
+    Align:    0x1000
+    Sections:
+      - Section: .tdata
+      - Section: .got
+  - Type:     PT_TLS
+    Flags:    [ PF_R ]
+    VAddr:    0x1240
+    Sections:
+      - Section: .tdata


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90897.303317.patch
Type: text/x-patch
Size: 3571 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201106/82163b65/attachment.bin>