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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay updated this revision to Diff 304204.
MaskRay added a comment.

Adjust comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90897/new/

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
@@ -2304,12 +2304,19 @@
   return Off;
 }
 
-// Rewrite p_offset and p_filesz of non-empty non-PT_PHDR segments after
-// sh_offset values have been updated.
+// Rewrite p_offset and p_filesz of non-PT_PHDR segments after sh_offset values
+// have been updated.
 static uint64_t layoutSegmentsForOnlyKeepDebug(std::vector<Segment *> &Segments,
                                                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 offset field. This
+    // works for empty PT_TLS. We don't handle empty segments without a parent
+    // for now.
+    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
@@ -216,3 +216,65 @@
     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] .text       NOBITS   0000000000000200 000200 000001 00  AX  0   0  0
+# CHECK4-NEXT: [ 2] .tdata      NOBITS   0000000000001240 000240 000000 00 WAT  0   0 64
+# CHECK4-NEXT: [ 3] .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 0x000000 0x000001 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:         .text
+    Type:         SHT_PROGBITS
+    Flags:        [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:      0x200
+    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
+    FirstSec: .text
+    LastSec:  .text
+  ## Add .got so that the PT_LOAD does not have zero p_memsz. We don't add
+  ## sections to zero-sized segments so zero-sized segments may have strange
+  ## offsets. In practice, the Linux kernel errors when mmapping a p_memsz
+  ## PT_LOAD,so for practical so this assumption can generally be made.
+  - Type:     PT_LOAD
+    Flags:    [ PF_R, PF_W ]
+    VAddr:    0x1240
+    Align:    0x1000
+    FirstSec: .tdata
+    LastSec:  .got
+  - Type:     PT_TLS
+    Flags:    [ PF_R ]
+    VAddr:    0x1240
+    FirstSec: .tdata
+    LastSec:  .tdata


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D90897.304204.patch
Type: text/x-patch
Size: 3976 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201110/c7ab5a3f/attachment.bin>