From 8f4f515898e2664ef8b4c1988b444cfd041f3fa5 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 30 Mar 2026 09:35:17 -0500 Subject: [PATCH] [lld][Hexagon] Fix TLS GD PLT to only create PLT entry for __tls_get_addr (#180297) Previously, R_HEX_GD_PLT_* relocations would create PLT entries for TLS symbols like 'foo' in addition to __tls_get_addr. This fix skips NEEDS_PLT on TLS symbols with R_HEX_GD_PLT_*, creates __tls_get_addr symbol earlier with NEEDS_PLT, changes hexagonTLSSymbolUpdate to only rebind relocations. Also a test for the edge case where a GD_PLT relocation directly references __tls_get_addr which previously caused a crash due to duplicate PLT entry creation. --------- Co-authored-by: Fangrui Song --- lld/ELF/Arch/Hexagon.cpp | 47 +++++++++++++++++++- lld/ELF/Relocations.cpp | 42 +---------------- lld/ELF/Relocations.h | 3 -- lld/ELF/Target.h | 6 +++ lld/ELF/Writer.cpp | 15 ------- lld/test/ELF/hexagon-thunk-range-gdplt.s | 36 ++++++--------- lld/test/ELF/hexagon-tls-gd-nonpreemptible.s | 1 - lld/test/ELF/hexagon-tls-gd-plt-direct.s | 31 +++++++++++++ lld/test/ELF/hexagon-tls-gd-xform.s | 17 ++++--- 9 files changed, 106 insertions(+), 92 deletions(-) create mode 100644 lld/test/ELF/hexagon-tls-gd-plt-direct.s diff --git a/lld/ELF/Arch/Hexagon.cpp b/lld/ELF/Arch/Hexagon.cpp index 435ee8a4d4f8..45b5188845b1 100644 --- a/lld/ELF/Arch/Hexagon.cpp +++ b/lld/ELF/Arch/Hexagon.cpp @@ -9,6 +9,7 @@ #include "InputFiles.h" #include "OutputSections.h" #include "RelocScan.h" +#include "SymbolTable.h" #include "Symbols.h" #include "SyntheticSections.h" #include "Target.h" @@ -43,6 +44,7 @@ public: void scanSection(InputSectionBase &sec) override { elf::scanSection1(*this, sec); } + void finalizeRelocScan() override; bool needsThunk(RelExpr expr, RelType type, const InputFile *file, uint64_t branchAddr, const Symbol &s, int64_t a) const override; @@ -182,7 +184,11 @@ void Hexagon::scanSectionImpl(InputSectionBase &sec, Relocs rels) { case R_HEX_GD_PLT_B22_PCREL: case R_HEX_GD_PLT_B22_PCREL_X: case R_HEX_GD_PLT_B32_PCREL_X: - sym.setFlags(NEEDS_PLT); + // GD PLT: call foo@GDPLT becomes call __tls_get_addr. + // Record R_PLT_PC on the TLS symbol; finalizeRelocScan (called + // single-threaded after scanning) will create __tls_get_addr and + // rebind these relocations. We cannot access the symbol table here + // because scanSectionImpl runs in parallel. sec.addReloc({R_PLT_PC, type, offset, addend, &sym}); continue; @@ -664,4 +670,43 @@ void elf::mergeHexagonAttributesSections(Ctx &ctx) { mergeAttributesSection(ctx, sections)); } +static bool isGDPLT(RelType type) { + switch (type) { + case R_HEX_GD_PLT_B22_PCREL: + case R_HEX_GD_PLT_B22_PCREL_X: + case R_HEX_GD_PLT_B32_PCREL_X: + return true; + default: + return false; + } +} + +void Hexagon::finalizeRelocScan() { + Symbol *tga = nullptr; + + // Scan for R_HEX_GD_PLT_* relocations (recorded as R_PLT_PC by + // scanSectionImpl) and rebind them to __tls_get_addr. + for (ELFFileBase *f : ctx.objectFiles) { + for (InputSectionBase *s : f->getSections()) { + auto *isec = dyn_cast_or_null(s); + if (!isec || !isec->isLive()) + continue; + for (Relocation &rel : isec->relocs()) { + if (rel.expr != R_PLT_PC || !isGDPLT(rel.type)) + continue; + if (!tga) { + tga = ctx.symtab->addSymbol(Undefined{ctx.internalFile, + "__tls_get_addr", STB_GLOBAL, + STV_DEFAULT, STT_FUNC}); + tga->isUsedInRegularObj = true; + tga->used = true; + tga->isPreemptible = true; + tga->setFlags(NEEDS_PLT); + } + rel.sym = tga; + } + } + } +} + void elf::setHexagonTargetInfo(Ctx &ctx) { ctx.target.reset(new Hexagon(ctx)); } diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index ee6b14f2798e..0000f1b4d905 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -1406,6 +1406,8 @@ void elf::postScanRelocations(Ctx &ctx) { } }; + ctx.target->finalizeRelocScan(); + GotSection *got = ctx.in.got.get(); if (ctx.needsTlsLd.load(std::memory_order_relaxed) && got->addTlsIndex()) { if (ctx.arg.shared) @@ -1991,46 +1993,6 @@ bool ThunkCreator::createThunks(uint32_t pass, return addressesChanged; } -// The following aid in the conversion of call x@GDPLT to call __tls_get_addr -// hexagonNeedsTLSSymbol scans for relocations would require a call to -// __tls_get_addr. -// hexagonTLSSymbolUpdate rebinds the relocation to __tls_get_addr. -bool elf::hexagonNeedsTLSSymbol(ArrayRef outputSections) { - bool needTlsSymbol = false; - forEachInputSectionDescription( - outputSections, [&](OutputSection *os, InputSectionDescription *isd) { - for (InputSection *isec : isd->sections) - for (Relocation &rel : isec->relocs()) - if (rel.sym->type == llvm::ELF::STT_TLS && rel.expr == R_PLT_PC) { - needTlsSymbol = true; - return; - } - }); - return needTlsSymbol; -} - -void elf::hexagonTLSSymbolUpdate(Ctx &ctx) { - Symbol *sym = ctx.symtab->find("__tls_get_addr"); - if (!sym) - return; - bool needEntry = true; - forEachInputSectionDescription( - ctx.outputSections, [&](OutputSection *os, InputSectionDescription *isd) { - for (InputSection *isec : isd->sections) - for (Relocation &rel : isec->relocs()) - if (rel.sym->type == llvm::ELF::STT_TLS && rel.expr == R_PLT_PC) { - if (needEntry) { - if (sym->auxIdx == 0) - sym->allocateAux(ctx); - addPltEntry(ctx, *ctx.in.plt, *ctx.in.gotPlt, *ctx.in.relaPlt, - ctx.target->pltRel, *sym); - needEntry = false; - } - rel.sym = sym; - } - }); -} - static bool matchesRefTo(const NoCrossRefCommand &cmd, StringRef osec) { if (cmd.toFirst) return cmd.outputSections[0] == osec; diff --git a/lld/ELF/Relocations.h b/lld/ELF/Relocations.h index 8de6b7dc2a6f..abc62bf01a42 100644 --- a/lld/ELF/Relocations.h +++ b/lld/ELF/Relocations.h @@ -160,9 +160,6 @@ bool maybeReportUndefined(Ctx &, Undefined &sym, InputSectionBase &sec, void postScanRelocations(Ctx &ctx); void addGotEntry(Ctx &ctx, Symbol &sym); -void hexagonTLSSymbolUpdate(Ctx &ctx); -bool hexagonNeedsTLSSymbol(ArrayRef outputSections); - bool isAbsolute(const Symbol &sym); class ThunkSection; diff --git a/lld/ELF/Target.h b/lld/ELF/Target.h index 8fa8cd13f1b6..1904892fdd2c 100644 --- a/lld/ELF/Target.h +++ b/lld/ELF/Target.h @@ -99,6 +99,12 @@ public: template void scanSectionImpl(InputSectionBase &, Relocs); + // Called after parallel relocation scanning is complete but before + // postScanRelocations processes symbol flags. Targets may override this to + // perform single-threaded fixups that cannot run during parallel scanning + // (e.g. symbol table modifications). + virtual void finalizeRelocScan() {} + virtual void relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const = 0; void relocateNoSym(uint8_t *loc, RelType type, uint64_t val) const { diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp index b8ec53e78f6e..6b0382090f78 100644 --- a/lld/ELF/Writer.cpp +++ b/lld/ELF/Writer.cpp @@ -1538,10 +1538,6 @@ template void Writer::finalizeAddressDependentContent() { }; finalizeOrderDependentContent(); - // Converts call x@GDPLT to call __tls_get_addr - if (ctx.arg.emachine == EM_HEXAGON) - hexagonTLSSymbolUpdate(ctx); - if (ctx.arg.randomizeSectionPadding) randomizeSectionPadding(ctx); @@ -2042,17 +2038,6 @@ template void Writer::finalizeSections() { sec->addrExpr = [=] { return i->second; }; } - // With the ctx.outputSections available check for GDPLT relocations - // and add __tls_get_addr symbol if needed. - if (ctx.arg.emachine == EM_HEXAGON && - hexagonNeedsTLSSymbol(ctx.outputSections)) { - Symbol *sym = - ctx.symtab->addSymbol(Undefined{ctx.internalFile, "__tls_get_addr", - STB_GLOBAL, STV_DEFAULT, STT_NOTYPE}); - sym->isPreemptible = true; - ctx.partitions[0].dynSymTab->addSymbol(sym); - } - // This is a bit of a hack. A value of 0 means undef, so we set it // to 1 to make __ehdr_start defined. The section number is not // particularly relevant. diff --git a/lld/test/ELF/hexagon-thunk-range-gdplt.s b/lld/test/ELF/hexagon-thunk-range-gdplt.s index 77fd0e575456..f646e81026ce 100644 --- a/lld/test/ELF/hexagon-thunk-range-gdplt.s +++ b/lld/test/ELF/hexagon-thunk-range-gdplt.s @@ -48,27 +48,29 @@ tls_var_distant: # CHECK: Disassembly of section .text: # CHECK: <_start>: -# CHECK-NEXT: 102d4: { immext(#0x420100) -# CHECK-NEXT: r2 = add(pc,##0x420130) } +# CHECK-NEXT: 102b0: { immext(#0x420100) +# CHECK-NEXT: r2 = add(pc,##0x420104) } # CHECK-NEXT: { immext(#0xfffeffc0) # CHECK-NEXT: r0 = add(r2,##-0x10018) } -# CHECK-NEXT: { call 0x410360 <__tls_get_addr@plt> } +# CHECK-NEXT: { call 0x410310 <__tls_get_addr@plt> } # CHECK-NEXT: { immext(#0xfffeffc0) # CHECK-NEXT: r0 = add(r2,##-0x10010) } -# CHECK-NEXT: { call 0x410360 <__tls_get_addr@plt> } +# CHECK-NEXT: { call 0x410310 <__tls_get_addr@plt> } # CHECK-NEXT: { jumpr r31 } # CHECK: : -# CHECK-NEXT: 4102f8: { immext(#0xfffeffc0) +# CHECK-NEXT: 4102d4: { immext(#0xfffeffc0) # CHECK-NEXT: r0 = add(r2,##-0x10008) } -# CHECK-NEXT: { call 0x410360 <__tls_get_addr@plt> } +# CHECK-NEXT: { call 0x410310 <__tls_get_addr@plt> } # CHECK-NEXT: { jumpr r31 } -## Verify PLT entries are created for TLS +## Verify PLT entry is created for __tls_get_addr +## TLS symbols (tls_var_close, tls_var_far, tls_var_distant) should NOT have +## PLT entries - only __tls_get_addr needs one. # CHECK: Disassembly of section .plt: # CHECK: <.plt>: -# CHECK-NEXT: 410310: { immext(#0x200c0) -# CHECK-NEXT: r28 = add(pc,##0x200f4) } +# CHECK-NEXT: 4102f0: { immext(#0x200c0) +# CHECK-NEXT: r28 = add(pc,##0x200c4) } # CHECK-NEXT: { r14 -= add(r28,#0x10) # CHECK-NEXT: r15 = memw(r28+#0x8) # CHECK-NEXT: r28 = memw(r28+#0x4) } @@ -76,20 +78,8 @@ tls_var_distant: # CHECK-NEXT: jumpr r28 } # CHECK-NEXT: { trap0(#0xdb) } -# CHECK: : -# CHECK-NEXT: 410340: { immext(#0x200c0) -# CHECK-NEXT: r14 = add(pc,##0x200d8) } -# CHECK-NEXT: { r28 = memw(r14+#0x0) } -# CHECK-NEXT: { jumpr r28 } - -# CHECK: : -# CHECK-NEXT: 410350: { immext(#0x200c0) -# CHECK-NEXT: r14 = add(pc,##0x200cc) } -# CHECK-NEXT: { r28 = memw(r14+#0x0) } -# CHECK-NEXT: { jumpr r28 } - # CHECK: <__tls_get_addr@plt>: -# CHECK-NEXT: 410360: { immext(#0x200c0) -# CHECK-NEXT: r14 = add(pc,##0x200c0) } +# CHECK-NEXT: 410310: { immext(#0x20080) +# CHECK-NEXT: r14 = add(pc,##0x200b4) } # CHECK-NEXT: { r28 = memw(r14+#0x0) } # CHECK-NEXT: { jumpr r28 } diff --git a/lld/test/ELF/hexagon-tls-gd-nonpreemptible.s b/lld/test/ELF/hexagon-tls-gd-nonpreemptible.s index ff5e6dbaac71..ca84865e1dba 100644 --- a/lld/test/ELF/hexagon-tls-gd-nonpreemptible.s +++ b/lld/test/ELF/hexagon-tls-gd-nonpreemptible.s @@ -14,7 +14,6 @@ .type _start, @function # RELOC: Section ({{.*}}) .rela.plt { -# RELOC-NEXT: R_HEX_JMP_SLOT - 0x0 # RELOC-NEXT: R_HEX_JMP_SLOT __tls_get_addr 0x0 # RELOC-NEXT: } diff --git a/lld/test/ELF/hexagon-tls-gd-plt-direct.s b/lld/test/ELF/hexagon-tls-gd-plt-direct.s new file mode 100644 index 000000000000..f52cb28130de --- /dev/null +++ b/lld/test/ELF/hexagon-tls-gd-plt-direct.s @@ -0,0 +1,31 @@ +# REQUIRES: hexagon +# RUN: llvm-mc -filetype=obj -triple=hexagon-unknown-elf %s -o %t.o +# RUN: ld.lld -shared %t.o --gc-sections -o %t.so +# RUN: llvm-readobj -r %t.so | FileCheck %s + +## This test verifies that a GD_PLT relocation on a TLS variable does not +## create a spurious R_HEX_JMP_SLOT for that variable — only +## __tls_get_addr should get a PLT entry. + +# CHECK: Section ({{.*}}) .rela.dyn { +# CHECK-NEXT: R_HEX_DTPMOD_32 foo 0x0 +# CHECK-NEXT: R_HEX_DTPREL_32 foo 0x0 +# CHECK-NEXT: } +# CHECK: Section ({{.*}}) .rela.plt { +# CHECK-NEXT: R_HEX_JMP_SLOT __tls_get_addr 0x0 +# CHECK-NEXT: } + +.globl _start +.type _start, @function +_start: + ## Use GD_GOT to set up TLS GOT entry for foo + r2 = add(pc, ##_GLOBAL_OFFSET_TABLE_@PCREL) + r0 = add(r2, ##foo@GDGOT) + call foo@GDPLT + jumpr r31 + +.section .tdata,"awT",@progbits +.globl foo +.type foo, @object +foo: + .word 0x11111111 diff --git a/lld/test/ELF/hexagon-tls-gd-xform.s b/lld/test/ELF/hexagon-tls-gd-xform.s index ade54e8a16fa..a87ec4bd61cd 100644 --- a/lld/test/ELF/hexagon-tls-gd-xform.s +++ b/lld/test/ELF/hexagon-tls-gd-xform.s @@ -18,25 +18,24 @@ _start: .ifdef GDPLT call x@gdplt -# CHECK_GDPLT: 101ec: { call 0x10220 <__tls_get_addr@plt> } +# CHECK_GDPLT: 101e0: { call 0x10210 <__tls_get_addr@plt> } .else call x # CHECK: 101b8: { call 0x101e0 } .endif -# CHECK_GDPLT: 10220: { immext(#0x20040) -# CHECK_GDPLT-NEXT: 10224: r14 = add(pc,##0x2007c) } -# CHECK_GDPLT-NEXT: 10228: { r28 = memw(r14+#0x0) } -# CHECK_GDPLT-NEXT: 1022c: { jumpr r28 } +# CHECK_GDPLT: 10210: { immext(#0x20040) +# CHECK_GDPLT-NEXT: 10214: r14 = add(pc,##0x20078) } +# CHECK_GDPLT-NEXT: 10218: { r28 = memw(r14+#0x0) } +# CHECK_GDPLT-NEXT: 1021c: { jumpr r28 } -## Looking at the above check, 0x10220+0x2007c must equal the entry for -## __tls_get_addr, 0x3029C +## Looking at the above check, 0x10210+0x20078 must equal the entry for +## __tls_get_addr, 0x30288 # RELA_GDPLT: Relocations [ # RELA_GDPLT-NEXT: Section (5) .rela.plt { -# RELA_GDPLT-NEXT: 0x30298 R_HEX_JMP_SLOT x 0x0 -# RELA_GDPLT-NEXT: 0x3029C R_HEX_JMP_SLOT __tls_get_addr 0x0 +# RELA_GDPLT-NEXT: 0x30288 R_HEX_JMP_SLOT __tls_get_addr 0x0 # RELA_GDPLT-NEXT: } # RELA_GDPLT-NEXT:]