From 1c33d14bcd6f8b4670fa62536eee5f3191677298 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Tue, 18 Sep 2018 00:24:48 +0000 Subject: [PATCH] [ELF] Set Out::TlsPhdr earlier for encoding packed reloc tables Summary: For --pack-dyn-relocs=android, finalizeSections calls LinkerScript::assignAddresses and AndroidPackedRelocationSection::updateAllocSize in a loop, where assignAddresses lays out the ELF image, then updateAllocSize determines the size of the Android packed relocation table by encoding it. Encoding the table requires knowing the values of relocation addends. To get the addend of a TLS relocation, updateAllocSize can call getSymVA on a TLS symbol before setPhdrs has initialized Out::TlsPhdr, producing an error: has an STT_TLS symbol but doesn't have an SHF_TLS section Fix the problem by initializing Out::TlsPhdr immediately after the program headers are created. The segment's p_vaddr field isn't initialized until setPhdrs, so use FirstSec->Addr, which is what setPhdrs would use. FirstSec will typically refer to the .tdata or .tbss output section, whose (tentative) address was computed by assignAddresses. Android currently avoids this problem because it uses emutls and doesn't support ELF TLS. This problem doesn't apply to --pack-dyn-relocs=relr because SHR_RELR only handles relative relocations without explicit addends or info. Fixes https://bugs.llvm.org/show_bug.cgi?id=37841. Reviewers: ruiu, pcc, chh, javed.absar, espindola Subscribers: emaste, arichardson, llvm-commits, srhines Differential Revision: https://reviews.llvm.org/D51671 llvm-svn: 342432 --- lld/ELF/Symbols.cpp | 9 ++++-- lld/ELF/Writer.cpp | 18 +++++++----- lld/test/ELF/pack-dyn-relocs-tls-aarch64.s | 34 ++++++++++++++++++++++ lld/test/ELF/pack-dyn-relocs-tls-x86-64.s | 23 +++++++++++++++ 4 files changed, 75 insertions(+), 9 deletions(-) create mode 100644 lld/test/ELF/pack-dyn-relocs-tls-aarch64.s create mode 100644 lld/test/ELF/pack-dyn-relocs-tls-x86-64.s diff --git a/lld/ELF/Symbols.cpp b/lld/ELF/Symbols.cpp index d007e445371b..c37d20c17253 100644 --- a/lld/ELF/Symbols.cpp +++ b/lld/ELF/Symbols.cpp @@ -91,10 +91,15 @@ static uint64_t getSymVA(const Symbol &Sym, int64_t &Addend) { uint64_t VA = IS->getVA(Offset); if (D.isTls() && !Config->Relocatable) { - if (!Out::TlsPhdr) + // Use the address of the TLS segment's first section rather than the + // segment's address, because segment addresses aren't initialized until + // after sections are finalized. (e.g. Measuring the size of .rela.dyn + // for Android relocation packing requires knowing TLS symbol addresses + // during section finalization.) + if (!Out::TlsPhdr || !Out::TlsPhdr->FirstSec) fatal(toString(D.File) + " has an STT_TLS symbol but doesn't have an SHF_TLS section"); - return VA - Out::TlsPhdr->p_vaddr; + return VA - Out::TlsPhdr->FirstSec->Addr; } return VA; } diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp index 93d3bc9a1afc..0d1606b13732 100644 --- a/lld/ELF/Writer.cpp +++ b/lld/ELF/Writer.cpp @@ -1656,6 +1656,12 @@ template void Writer::finalizeSections() { Phdrs = Script->hasPhdrsCommands() ? Script->createPhdrs() : createPhdrs(); addPtArmExid(Phdrs); Out::ProgramHeaders->Size = sizeof(Elf_Phdr) * Phdrs.size(); + + // Find the TLS segment. This happens before the section layout loop so that + // Android relocation packing can look up TLS symbol addresses. + for (PhdrEntry *P : Phdrs) + if (P->p_type == PT_TLS) + Out::TlsPhdr = P; } // Some symbols are defined in term of program headers. Now that we @@ -2093,13 +2099,11 @@ template void Writer::setPhdrs() { P->p_memsz = alignTo(P->p_memsz, Target->PageSize); } - // The TLS pointer goes after PT_TLS. At least glibc will align it, - // so round up the size to make sure the offsets are correct. - if (P->p_type == PT_TLS) { - Out::TlsPhdr = P; - if (P->p_memsz) - P->p_memsz = alignTo(P->p_memsz, P->p_align); - } + // The TLS pointer goes after PT_TLS for variant 2 targets. At least glibc + // will align it, so round up the size to make sure the offsets are + // correct. + if (P->p_type == PT_TLS && P->p_memsz) + P->p_memsz = alignTo(P->p_memsz, P->p_align); } } diff --git a/lld/test/ELF/pack-dyn-relocs-tls-aarch64.s b/lld/test/ELF/pack-dyn-relocs-tls-aarch64.s new file mode 100644 index 000000000000..978e2e7b2426 --- /dev/null +++ b/lld/test/ELF/pack-dyn-relocs-tls-aarch64.s @@ -0,0 +1,34 @@ +// REQUIRES: aarch64 + +// RUN: llvm-mc -filetype=obj -triple=aarch64-unknown-linux %s -o %t.o +// RUN: ld.lld -shared --pack-dyn-relocs=android %t.o -o %t.so +// RUN: llvm-readobj -relocations %t.so | FileCheck %s + +// Bug 37841: Symbol::getVA must work on TLS symbols during the layout loop in +// finalizeSections. + + .global foo +foo: + adrp x0, :tlsdesc:tlsvar1 + ldr x1, [x0, :tlsdesc_lo12:tlsvar1] + add x0, x0, :tlsdesc_lo12:tlsvar1 + .tlsdesccall tlsvar1 + +// Also test an atypical IE access from a shared object to a local TLS symbol. + + .global bar +bar: + adrp x0, :gottprel:tlsvar2 + ldr x0, [x0, #:gottprel_lo12:tlsvar2] + + .section .tdata,"awT",@progbits + .space 0x1234 +tlsvar1: + .word 42 +tlsvar2: + .word 17 + +// CHECK: Section ({{.+}}) .rela.dyn { +// CHECK-NEXT: R_AARCH64_TLSDESC - 0x1234 +// CHECK-NEXT: R_AARCH64_TLS_TPREL64 - 0x1238 +// CHECK-NEXT: } diff --git a/lld/test/ELF/pack-dyn-relocs-tls-x86-64.s b/lld/test/ELF/pack-dyn-relocs-tls-x86-64.s new file mode 100644 index 000000000000..491efa6cc1d4 --- /dev/null +++ b/lld/test/ELF/pack-dyn-relocs-tls-x86-64.s @@ -0,0 +1,23 @@ +// REQUIRES: x86 + +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o +// RUN: ld.lld -shared --pack-dyn-relocs=android %t.o -o %t.so +// RUN: llvm-readobj -relocations %t.so | FileCheck %s + +// Bug 37841: Symbol::getVA must work on TLS symbols during the layout loop in +// finalizeSections. This test uses an atypical IE access in a shared object to +// access a local TLS symbol, because a more typical access would avoid the +// bug. + + .globl foo +foo: + movq tlsvar@GOTTPOFF(%rip), %rcx + + .section .tdata,"awT",@progbits + .space 0x1234 +tlsvar: + .word 42 + +// CHECK: Section ({{.+}}) .rela.dyn { +// CHECK-NEXT: R_X86_64_TPOFF64 - 0x1234 +// CHECK-NEXT: }