[lld][Hexagon] convert call x@GDPLT to call __tls_get_addr

[sidneym]

Summary

In the General Dynamic model, TLS variables are referenced by :

r0 = add(r24, #x@GDGOT)    // R_HEX_GD_GOT...
call x@GDPLT                         // R_HEX_GD_PLT_B22_PCREL

The linker must rebind the call x@GDPLT to call __tls_get_addr in order to dereference the tls variable.
The method Hexagon uses is similar to 32-bit SPARC.

The thunking code was very useful since it modeled what I needed to do. I would have liked to have been able to add the symbol in HexagonTLSSymbolUpdate if it was found to be needed. When I tried that the dynamic symbol table entry was not correct.

[MaskRay]

The linker must rebind the call x@GDPLT to call __tls_get_addr in order to dereference the tls variable.

This is weird. Is it fixable in the psABI?

lld/ELF/Relocations.cpp

1978

//

1985

lld uses variableName

lld/ELF/Relocations.h

117

lld uses functionName

[sidneym]

Update functionNames.

The original ABI, which is over 10 years old, may have used SPARC and 32-bit x86 as a reference:
0x00 leal x@tlsgd(,%ebx,1), %eax
0x07 call x@tlsgdplt

or:
0x00 sethi %hi(@dtlndx(x)), %o0
0x04 add %o0, %lo(@dtlndx(x)), %o0
0x08 add %l7, %o0, %o0
0x0c call x@TLSPLT

SPARC adds the __tls_get_addr symbol in the assembler when a VK_Sparc_TLS_GD_CALL is found. The same could be done for Hexagon (VK_Hexagon_GD_PLT), avoiding the need to create the symbol, but that would mean objects not built with the change would fail to link if using lld and that relocation was used. The symbol update would still be needed.

With the above in mind, I should probably check for the existence of __tls_get_addr prior to calling hexagonNeedsTLSSymbol.

[sidneym]

Update variable names.
make sure __tls_get_addr doesn't already exist before creating it

[bcain]

LGTM 👍

[MaskRay]

lld/ELF/Relocations.cpp

1988

excess parentheses can be dropped.

lld/ELF/Writer.cpp

1824

partitions[0]

lld/test/ELF/hexagon-tls-gd-xform.s

34

Use ## for comments.

37

# RELA_GDPLT

[MaskRay]

r0 = add(r24, #x@GDGOT)    // R_HEX_GD_GOT...
call x@GDPLT                         // R_HEX_GD_PLT_B22_PCREL

Align //

[sidneym]

remove parens, ## comments, change partition index.

[bcain]

@MaskRay and @sidneym -- is this change ready to go or are there outstanding review items?

[MaskRay]

The method Hexagon uses is similar to 32-bit SPARC.

Can you explain how 32-bit SPARC do this? What about 64-bit SPARC?

lld/test/ELF/hexagon-tls-gd-xform.s

3

It can be on the same line.

[sidneym]

In D74443#1891747, @MaskRay wrote:

The method Hexagon uses is similar to 32-bit SPARC.

Can you explain how 32-bit SPARC do this? What about 64-bit SPARC?

One method SPARC would have used can be found in older versions of binutils.
+ if (!sparc_tls_symbol)
+ sparc_tls_symbol = gen_rtx_SYMBOL_REF (Pmode, "__tls_get_addr");
See: https://gcc.gnu.org/ml/gcc-patches/2003-09/msg00456.html

The Introduction section of the Hexagon ABI doc refers to some of the resources used, http://docs.oracle.com/cd/E19253-01/817-1984/ -- "The call instruction uses the special syntax, x@TLSPLT. This call references the TLS variable and generates the R_SPARC_TLS_GD_CALL relocation. This relocation instructs the link-editor to bind the call to the __tls_get_addr() function, and associates the call instruction with the GD code sequence." Hexagon does the same.

The 64Bit ABI didn't do this, it just makes the call.

[sidneym]

Merged 2 lines in testcase.

[sidneym]

Fix clang-format issues

[sidneym]

fix formatting concerns from clang-tidy

[bcain]

Ping @MaskRay and @sidneym -- anything left barring commit?

[MaskRay]

Many variable names are still not fixed.

lld/ELF/Relocations.cpp

1982

outputSections

1983

needTlsSymbol

1985

Not done. os isd

1996

outputSections

lld/test/ELF/hexagon-tls-gd-xform.s

39

Without a colon, these checks are not effective.

[grimar]

lld/ELF/Relocations.cpp

1986

I wonder if you could do the following?

bool hexagonNeedsTLSSymbol(...) {
  for (InputSectionBase *s : inputSections)
    if (s->isLive())
      for (Relocation &Rel : Isec->relocations)
        if (Rel.sym->type == llvm::ELF::STT_TLS && Rel.expr == R_PLT_PC)
          return true;
  return false;
}

2003

I think it is better to move this find call out of the loops.

2005

How can this error happen?

[sidneym]

lld/ELF/Relocations.cpp

2003

Yes, that would be better. I will do that.

[sidneym]

In D74443#1908679, @MaskRay wrote:

Many variable names are still not fixed.

These had been changed per your original comment but changed back because a pre-checkin formatting pass flagged them as wrong. I made those name changes between diff set 6 and 7 (current).

[MaskRay]

In D74443#1909797, @sidneym wrote:

In D74443#1908679, @MaskRay wrote:

Many variable names are still not fixed.

These had been changed per your original comment but changed back because a pre-checkin formatting pass flagged them as wrong. I made those name changes between diff set 6 and 7 (current).

https://github.com/google/llvm-premerge-checks/issues/142

You can just ignore the clang-tidy VariableName warnings.

[sidneym]

• Add the missing colon's that kept the part of the testcase from running.
• Move the find of __tls_get_addr outside of the loop and return if it is not found.
• Make __tls_get_addr preemptible. If isPreemptible is not set that symbol's symbol index is 0, since part of the testcase wasn't running, the missing colon's, I missed this.
• Change the case of the variables.

[sidneym]

Fix variable name noted by: https://results.llvm-merge-guard.org/amd64_debian_testing_clang8-4524/clang-tidy.txt

[bcain]

In D74443#1920486, @sidneym wrote:

Fix variable name noted by: https://results.llvm-merge-guard.org/amd64_debian_testing_clang8-4524/clang-tidy.txt

Ok, thanks. Now I think this is ready to go? LGTM at least.

lld/ELF/Arch/Hexagon.cpp

RelExpr Hexagon::getRelExpr(RelType type, const Symbol &s,
   case R_HEX_32_PCREL:
     return R_PC;
   case R_HEX_B9_PCREL_X:
   case R_HEX_B15_PCREL_X:
   case R_HEX_B22_PCREL:
   case R_HEX_PLT_B22_PCREL:
   case R_HEX_B22_PCREL_X:
   case R_HEX_B32_PCREL_X:
+  case R_HEX_GD_PLT_B22_PCREL:
     return R_PLT_PC;
   case R_HEX_IE_32_6_X:
   case R_HEX_IE_16_X:
   case R_HEX_IE_HI16:
   case R_HEX_IE_LO16:
     return R_GOT;
   case R_HEX_GD_GOT_11_X:
   case R_HEX_GD_GOT_16_X:
case R_HEX_B13_PCREL:
     break;
   case R_HEX_B15_PCREL:
     checkInt(loc, val, 17, rel);
     or32le(loc, applyMask(0x00df20fe, val >> 2));
     break;
   case R_HEX_B15_PCREL_X:
     or32le(loc, applyMask(0x00df20fe, val & 0x3f));
     break;
+  case R_HEX_GD_PLT_B22_PCREL:
   case R_HEX_B22_PCREL:
   case R_HEX_PLT_B22_PCREL:
     checkInt(loc, val, 22, rel);
     or32le(loc, applyMask(0x1ff3ffe, val >> 2));
     break;
   case R_HEX_B22_PCREL_X:
     or32le(loc, applyMask(0x1ff3ffe, val & 0x3f));
     break;

lld/ELF/Relocations.h

 
 // This function writes undefined symbol diagnostics to an internal buffer.
 // Call reportUndefinedSymbols() after calling scanRelocations() to emit
 // the diagnostics.
 template <class ELFT> void scanRelocations(InputSectionBase &);
 
 template <class ELFT> void reportUndefinedSymbols();
 
+void hexagonTLSSymbolUpdate(ArrayRef<OutputSection *> outputSections);
+bool hexagonNeedsTLSSymbol(ArrayRef<OutputSection *> outputSections);
+
 class ThunkSection;
 class Thunk;
 struct InputSectionDescription;
 
 class ThunkCreator {
 public:
   // Return true if Thunks have been added to OutputSections
   bool createThunks(ArrayRef<OutputSection *> outputSections);

lld/ELF/Relocations.cpp

for (auto &p : thunkedSections)
     addressesChanged |= p.second->assignOffsets();
 
   // Merge all created synthetic ThunkSections back into OutputSection
   mergeThunks(outputSections);
   ++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 hexagonNeedsTLSSymbol(ArrayRef<OutputSection *> outputSections) {
+  bool needTLSSymbol = false;
+  forEachInputSectionDescription(
+      outputSections, [&](OutputSection *os, InputSectionDescription *isd) {
+        for (InputSection *isec : isd->sections)
+          for (Relocation &rel : isec->relocations)
+            if ((rel.sym->type == llvm::ELF::STT_TLS) &&
+                (rel.expr == R_PLT_PC)) {
+              needTLSSymbol = true;
+              return;
+            }
+      });
+  return needTLSSymbol;
+}
+
+void hexagonTLSSymbolUpdate(ArrayRef<OutputSection *> outputSections) {
+  bool needEntry = true;
+  forEachInputSectionDescription(
+      outputSections, [&](OutputSection *os, InputSectionDescription *isd) {
+        for (InputSection *isec : isd->sections)
+          for (Relocation &rel : isec->relocations)
+            if ((rel.sym->type == llvm::ELF::STT_TLS) &&
+                (rel.expr == R_PLT_PC)) {
+              Symbol *sym = symtab->find("__tls_get_addr");
+              if (!sym)
+                error("unable to find __tls_get_addr");
+              if (needEntry) {
+                addPltEntry(in.plt, in.gotPlt, in.relaPlt, target->pltRel,
+                            *sym);
+                needEntry = false;
+              }
+              rel.sym = sym;
+            }
+      });
+}
+
 template void scanRelocations<ELF32LE>(InputSectionBase &);
 template void scanRelocations<ELF32BE>(InputSectionBase &);
 template void scanRelocations<ELF64LE>(InputSectionBase &);
 template void scanRelocations<ELF64BE>(InputSectionBase &);
 template void reportUndefinedSymbols<ELF32LE>();
 template void reportUndefinedSymbols<ELF32BE>();
 template void reportUndefinedSymbols<ELF64LE>();
 template void reportUndefinedSymbols<ELF64BE>();
 
 } // namespace elf
 } // namespace lld

lld/ELF/Writer.cpp

 // addresses we must converge to a fixed point. We do that here. See the comment
 // in Writer<ELFT>::finalizeSections().
 template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
   ThunkCreator tc;
   AArch64Err843419Patcher a64p;
   ARMErr657417Patcher a32p;
   script->assignAddresses();
 
+  // Converts call x@GDPLT to call __tls_get_addr
+  if (config->emachine == EM_HEXAGON)
+    hexagonTLSSymbolUpdate(outputSections);
+
   int assignPasses = 0;
   for (;;) {
     bool changed = target->needsThunks && tc.createThunks(outputSections);
 
     // With Thunk Size much smaller than branch range we expect to
     // converge quickly; if we get to 10 something has gone wrong.
     if (changed && tc.pass >= 10) {
       error("thunk creation not converged");
template <class ELFT> void Writer<ELFT>::finalizeSections() {
 
   // Prefer command line supplied address over other constraints.
   for (OutputSection *sec : outputSections) {
     auto i = config->sectionStartMap.find(sec->name);
     if (i != config->sectionStartMap.end())
       sec->addrExpr = [=] { return i->second; };
   }
 
+  // With the outputSections available check for GDPLT relocations
+  // and add __tls_get_addr symbol if needed.
+  if (config->emachine == EM_HEXAGON && !symtab->find("__tls_get_addr") &&
+      hexagonNeedsTLSSymbol(outputSections)) {
+    Symbol *sym = symtab->addSymbol(Undefined{
+        nullptr, "__tls_get_addr", STB_GLOBAL, STV_DEFAULT, STT_NOTYPE});
+    partitions[sym->partition - 1].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.
   Out::elfHeader->sectionIndex = 1;
 
   for (size_t i = 0, e = outputSections.size(); i != e; ++i) {
     OutputSection *sec = outputSections[i];
     sec->sectionIndex = i + 1;

lld/test/ELF/hexagon-tls-gd-xform.s

+# REQUIRES: hexagon
+# RUN: llvm-mc -filetype=obj -defsym GDPLT=1 -triple=hexagon-unknown-elf \
+# RUN:   %s -o %t.o
+# RUN: llvm-mc -filetype=obj -triple=hexagon-unknown-elf %s -o %t1.o
+# RUN: ld.lld -shared %t.o -o %t.so
+# RUN: ld.lld -shared %t1.o -o %t1.so
+# RUN: llvm-objdump -d --no-show-raw-insn --print-imm-hex %t.so | \
+# RUN:   FileCheck --check-prefix=CHECK_GDPLT %s
+# RUN: llvm-objdump -d --no-show-raw-insn --print-imm-hex %t1.so | FileCheck %s
+# RUN: llvm-readobj -r %t.so | FileCheck -check-prefix=RELA_GDPLT  %s
+
+# Make sure __tls_get_addr is not present unless there is a GDPLT relocation.
+# RUN: llvm-readobj -r %t1.so | FileCheck -check-prefix=RELA \
+# RUN:   --implicit-check-not="__tls_get_addr" %s
+
+.globl _start
+.type _start, @function
+
+_start:
+.ifdef GDPLT
+                        call x@gdplt
+#CHECK_GDPLT:  101ec: { call 0x10220 }
+.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 }
+
+
+# Looking at the above check, 0x10220+0x2007c must equal the entry for
+#  __tls_get_addr, 0x3029C
+
+#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  }
+#RELA_GDPLT-NEXT]
+
+#RELA: Relocations [
+#RELA-NEXT  Section (5) .rela.plt {
+#RELA-NEXT    0x30298 R_HEX_JMP_SLOT x 0x0
+#RELA-NEXT  }
+#RELA-NEXT]