[ELF] Fix TLS GD against non-preemptible dynamic symbols in DSOs (#207881)
Consider a DSO that uses TLS GD against a dynamic non-preemptible symbol (e.g. protected, or linked with -Bsymbolic). The expected meaning of this is to always reference the DSO's own TLS segment for that symbol, regardless of any attempts to preempt it at load time, and LLD will indeed emit a constant offset rather than a symbolic DTPOFF relocation for the second GOT word. However, for the first GOT word, because we currently only treat the isLocalInExecutable case as special (where the module index is 1), we end up creating a symbolic DTPMOD relocation, rather than a non-symbolic (i.e. against the null symbol) one to get this DSO's index, and so any load-time preemption will cause us to use a different module's TLS block. As a result, not only do we have symbol preemption when we shouldn't, but also the two halves of the TLS GD GOT entry end up inconsistent, using the offset of the symbol in this module's TLS block for a different module's. Fix this by breaking down the two !isLocalInExecutable cases, emitting the expected non-symbolic DTPMOD relocation for any non-preemptible symbol references in a DSO. Whilst here, combine the DTPMOD and DTPOFF code rather than have two different if statements, so it's clearer what the three cases are and they don't get out of sync. Note that although, for the existing code, local symbols take the exact same code paths in postScanRelocations, they do not end up with symbolic DTPMOD relocations in the output, despite creating them. DynamicReloc::getSymIndex has the quirk of silently squashing the symbol index to 0 for symbolic relocations against non-dynamic symbols, which applies to local symbols but not other non-preemptible symbols, i.e. the case relevant in the first paragraph. This quirky behaviour of DynamicReloc::getSymIndex is probably something that should be retired; I can't see a good reason for it, and any case that it squashes could be hiding similar bugs like this. I take the view that a request to create a symbolic relocation against a symbol that won't be in the dynamic symbol table is a strong sign that the code is confused and doing something wrong, even if it happens to work out.
This commit is contained in:
1 parent
4a717571c2
commit
4e5a44a332
4 files changed
+48
-37
No files matched your search
+11
-11
@@ -1340,7 +1340,6 @@ void elf::postScanRelocations(Ctx &ctx) {
|
||||
|
||||
if (!sym.isTls())
|
||||
return;
|
||||
bool isLocalInExecutable = !sym.isPreemptible && !ctx.arg.shared;
|
||||
GotSection *got = ctx.in.got.get();
|
||||
|
||||
if (flags & NEEDS_TLSDESC) {
|
||||
@@ -1363,21 +1362,22 @@ void elf::postScanRelocations(Ctx &ctx) {
|
||||
if (flags & NEEDS_TLSGD) {
|
||||
got->addDynTlsEntry(sym);
|
||||
uint64_t off = got->getGlobalDynOffset(sym);
|
||||
if (isLocalInExecutable)
|
||||
// Write one to the GOT slot.
|
||||
got->addConstant({R_ADDEND, ctx.target->symbolicRel, off, 1, &sym});
|
||||
else
|
||||
uint64_t offsetOff = off + ctx.arg.wordsize;
|
||||
if (sym.isPreemptible) {
|
||||
ctx.in.relaDyn->addSymbolReloc(ctx.target->tlsModuleIndexRel, *got, off,
|
||||
sym);
|
||||
|
||||
// If the symbol is preemptible we need the dynamic linker to write
|
||||
// the offset too.
|
||||
uint64_t offsetOff = off + ctx.arg.wordsize;
|
||||
if (sym.isPreemptible)
|
||||
// If the symbol is preemptible we need the dynamic linker to write
|
||||
// the offset too.
|
||||
ctx.in.relaDyn->addSymbolReloc(ctx.target->tlsOffsetRel, *got,
|
||||
offsetOff, sym);
|
||||
else
|
||||
} else {
|
||||
if (ctx.arg.shared)
|
||||
ctx.in.relaDyn->addReloc({ctx.target->tlsModuleIndexRel, got, off});
|
||||
else
|
||||
// Write one to the GOT slot.
|
||||
got->addConstant({R_ADDEND, ctx.target->symbolicRel, off, 1, &sym});
|
||||
got->addConstant({R_ABS, ctx.target->tlsOffsetRel, offsetOff, 0, &sym});
|
||||
}
|
||||
}
|
||||
if (flags & NEEDS_GOT_DTPREL) {
|
||||
got->addEntry(sym);
|
||||
|
||||
@@ -897,33 +897,35 @@ void MipsGotSection::build() {
|
||||
}
|
||||
for (std::pair<Symbol *, size_t> &p : got.dynTlsSymbols) {
|
||||
Symbol *s = p.first;
|
||||
uint64_t offset = p.second * ctx.arg.wordsize;
|
||||
uint64_t off = p.second * ctx.arg.wordsize;
|
||||
if (s == nullptr) {
|
||||
if (ctx.arg.shared)
|
||||
ctx.in.relaDyn->addReloc(
|
||||
{ctx.target->tlsModuleIndexRel, this, offset});
|
||||
ctx.in.relaDyn->addReloc({ctx.target->tlsModuleIndexRel, this, off});
|
||||
else
|
||||
addConstant(
|
||||
{R_ADDEND, ctx.target->symbolicRel, offset, 1, ctx.dummySym});
|
||||
{R_ADDEND, ctx.target->symbolicRel, off, 1, ctx.dummySym});
|
||||
} else {
|
||||
// When building a shared library we still need a dynamic relocation
|
||||
// for the module index. Therefore only checking for
|
||||
// S->isPreemptible is not sufficient (this happens e.g. for
|
||||
// thread-locals that have been marked as local through a linker script)
|
||||
if (!s->isPreemptible && !ctx.arg.shared)
|
||||
// Write one to the GOT slot.
|
||||
addConstant({R_ADDEND, ctx.target->symbolicRel, offset, 1, s});
|
||||
else
|
||||
ctx.in.relaDyn->addSymbolReloc(ctx.target->tlsModuleIndexRel, *this,
|
||||
offset, *s);
|
||||
offset += ctx.arg.wordsize;
|
||||
// However, we can skip writing the TLS offset reloc for non-preemptible
|
||||
// symbols since it is known even in shared libraries
|
||||
if (s->isPreemptible)
|
||||
uint64_t offsetOff = off + ctx.arg.wordsize;
|
||||
if (s->isPreemptible) {
|
||||
ctx.in.relaDyn->addSymbolReloc(ctx.target->tlsModuleIndexRel, *this,
|
||||
off, *s);
|
||||
ctx.in.relaDyn->addSymbolReloc(ctx.target->tlsOffsetRel, *this,
|
||||
offset, *s);
|
||||
else
|
||||
addConstant({R_ABS, ctx.target->tlsOffsetRel, offset, 0, s});
|
||||
offsetOff, *s);
|
||||
} else {
|
||||
if (ctx.arg.shared)
|
||||
ctx.in.relaDyn->addReloc(
|
||||
{ctx.target->tlsModuleIndexRel, this, off});
|
||||
else
|
||||
// Write one to the GOT slot.
|
||||
addConstant({R_ADDEND, ctx.target->symbolicRel, off, 1, s});
|
||||
addConstant({R_ABS, ctx.target->tlsOffsetRel, offsetOff, 0, s});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
# we do the right thing now:
|
||||
|
||||
# RUN: llvm-mc -filetype=obj -triple=mips64-unknown-freebsd %s -o %t.o
|
||||
# RUN: echo "{ global: foo; local: *; };" > %t.script
|
||||
# RUN: echo "{ global: foo; y; local: *; };" > %t.script
|
||||
# RUN: echo "SECTIONS { \
|
||||
# RUN: . = 0x10000; .text : { *(.text) } \
|
||||
# RUN: . = 0x20000; .got : { *(.got) } \
|
||||
@@ -18,13 +18,15 @@
|
||||
# GOT: Contents of section .got:
|
||||
# GOT-NEXT: 20000 00000000 00000000 80000000 00000000
|
||||
# GOT-NEXT: 20010 00000000 00000000 00000000 00000000
|
||||
# GOT-NEXT: 20020 ffffffff ffff8000
|
||||
# GOT-NEXT: 20020 ffffffff ffff8000 00000000 00000000
|
||||
# GOT-NEXT: 20030 ffffffff ffff8004
|
||||
|
||||
# RELOCS: Section ({{.+}}) .rel.dyn {
|
||||
# RELOCS-NEXT: 0x20018 R_MIPS_TLS_DTPMOD64/R_MIPS_NONE/R_MIPS_NONE
|
||||
# RELOCS-NEXT: 0x20018 R_MIPS_TLS_DTPMOD64/R_MIPS_NONE/R_MIPS_NONE -
|
||||
# RELOCS-NEXT: 0x20028 R_MIPS_TLS_DTPMOD64/R_MIPS_NONE/R_MIPS_NONE -
|
||||
# RELOCS-NEXT: }
|
||||
|
||||
# Test case generated using:
|
||||
# Test case initially generated using:
|
||||
# clang -mcpu=mips4 -target mips64-unknown-freebsd12.0 \
|
||||
# -fpic -O -G0 -EB -mabi=n64 -msoft-float -std=gnu99 -S %s -o %t.s
|
||||
# from the following source:
|
||||
@@ -44,12 +46,19 @@ foo:
|
||||
ld $25, %call16(__tls_get_addr)($gp)
|
||||
jalr $25
|
||||
daddiu $4, $gp, %tlsgd(x)
|
||||
daddiu $4, $gp, %tlsgd(y)
|
||||
.end foo
|
||||
|
||||
.type x,@object
|
||||
.section .tbss,"awT",@nobits
|
||||
.globl x
|
||||
.p2align 2
|
||||
.type x,@object
|
||||
.globl x
|
||||
x:
|
||||
.4byte 0
|
||||
.size x, 4
|
||||
.type y,@object
|
||||
.globl y
|
||||
.protected y
|
||||
y:
|
||||
.4byte 0
|
||||
.size y, 4
|
||||
@@ -22,7 +22,7 @@
|
||||
foo:
|
||||
.zero 4
|
||||
|
||||
.hidden bar
|
||||
.protected bar
|
||||
.globl bar
|
||||
bar:
|
||||
.zero 4
|
||||
@@ -34,8 +34,8 @@ bar:
|
||||
// CHECK-NEXT: SHF_ALLOC (0x2)
|
||||
// CHECK-NEXT: SHF_WRITE (0x1)
|
||||
// CHECK-NEXT: ]
|
||||
// CHECK-NEXT: Address: 0x23F0
|
||||
// CHECK-NEXT: Offset: 0x3F0
|
||||
// CHECK-NEXT: Address: 0x2410
|
||||
// CHECK-NEXT: Offset: 0x410
|
||||
// CHECK-NEXT: Size: 32
|
||||
// CHECK-NEXT: Link: 0
|
||||
// CHECK-NEXT: Info: 0
|
||||
@@ -47,6 +47,6 @@ bar:
|
||||
// CHECK-NEXT: )
|
||||
|
||||
// CHECK: Section ({{.*}}) .rela.dyn {
|
||||
// CHECK-NEXT: 0x23F0 R_X86_64_DTPMOD64 - 0x0
|
||||
// CHECK-NEXT: 0x2400 R_X86_64_DTPMOD64 - 0x0
|
||||
// CHECK-NEXT: 0x2410 R_X86_64_DTPMOD64 - 0x0
|
||||
// CHECK-NEXT: 0x2420 R_X86_64_DTPMOD64 - 0x0
|
||||
// CHECK-NEXT: }
|
||||
Reference in new issue
Block a user