[llvm] 20de182 - [llvm-objcopy] --only-keep-debug: place zero-size segment according to its parent segment
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 11 09:21:17 PST 2020
Author: Fangrui Song
Date: 2020-11-11T09:21:10-08:00
New Revision: 20de1822466e87c137b2369d8601b0f4c575981b
URL: https://github.com/llvm/llvm-project/commit/20de1822466e87c137b2369d8601b0f4c575981b
DIFF: https://github.com/llvm/llvm-project/commit/20de1822466e87c137b2369d8601b0f4c575981b.diff
LOG: [llvm-objcopy] --only-keep-debug: place zero-size segment according to its parent segment
Alternative to D74755. 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. Note, p_memsz PT_LOAD is rejected by both Linux and FreeBSD.
Reviewed By: jhenderson
Differential Revision: https://reviews.llvm.org/D90897
Added:
Modified:
llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
llvm/tools/llvm-objcopy/ELF/Object.cpp
Removed:
################################################################################
diff --git a/llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test b/llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
index fe58792256f6..c8abb056478f 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
@@ -216,3 +216,65 @@ Sections:
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
diff --git a/llvm/tools/llvm-objcopy/ELF/Object.cpp b/llvm/tools/llvm-objcopy/ELF/Object.cpp
index b0315d6fdc46..76db901e4abf 100644
--- a/llvm/tools/llvm-objcopy/ELF/Object.cpp
+++ b/llvm/tools/llvm-objcopy/ELF/Object.cpp
@@ -2304,12 +2304,19 @@ static uint64_t layoutSectionsForOnlyKeepDebug(Object &Obj, uint64_t Off) {
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 contains no 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;
More information about the llvm-commits
mailing list