Skip to content

release/18.x: [RISCV][lld] Set the type of TLSDESC relocation's referenced local symbol to STT_NOTYPE #91678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 14, 2024
Merged

release/18.x: [RISCV][lld] Set the type of TLSDESC relocation's referenced local symbol to STT_NOTYPE #91678

merged 1 commit into from
May 14, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented May 9, 2024

Backport f6f474c dfe4ca9

Requested by: @ilovepi

@llvmbot llvmbot added this to the LLVM 18.X Release milestone May 9, 2024
@llvmbot
Copy link
Member Author

llvmbot commented May 9, 2024

@MaskRay @ilovepi What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested review from ilovepi and MaskRay May 9, 2024 23:10
@llvmbot llvmbot mentioned this pull request May 9, 2024
Merged
@llvmbot
Copy link
Member Author

llvmbot commented May 9, 2024 •
edited
Loading

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: None (llvmbot)

Changes

Backport f6f474c dfe4ca9

Requested by: @ilovepi

Full diff: https://github.com/llvm/llvm-project/pull/91678.diff

5 Files Affected:

  • (modified) lld/ELF/Relocations.cpp (+4-1)
  • (modified) lld/test/ELF/riscv-tlsdesc-relax.s (+8)
  • (modified) lld/test/ELF/riscv-tlsdesc.s (+35-5)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp (-2)
  • (added) llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll (+24)
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 619fbaf5dc545..92a1b9baaca3d 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -1480,7 +1480,10 @@ template <class ELFT, class RelTy> void RelocationScanner::scanOne(RelTy *&i) {
 
   // Process TLS relocations, including TLS optimizations. Note that
   // R_TPREL and R_TPREL_NEG relocations are resolved in processAux.
-  if (sym.isTls()) {
+  //
+  // Some RISCV TLSDESC relocations reference a local NOTYPE symbol,
+  // but we need to process them in handleTlsRelocation.
+  if (sym.isTls() || oneof<R_TLSDESC_PC, R_TLSDESC_CALL>(expr)) {
     if (unsigned processed =
             handleTlsRelocation(type, sym, *sec, offset, addend, expr)) {
       i += processed - 1;
diff --git a/lld/test/ELF/riscv-tlsdesc-relax.s b/lld/test/ELF/riscv-tlsdesc-relax.s
index fb24317e6535c..5718d4175be11 100644
--- a/lld/test/ELF/riscv-tlsdesc-relax.s
+++ b/lld/test/ELF/riscv-tlsdesc-relax.s
@@ -33,12 +33,14 @@
 # GD64-NEXT:         c.add   a0, tp
 # GD64-NEXT:         jal     {{.*}} <foo>
 ## &.got[c]-. = 0x20c0+8 - 0x1020 = 0x10a8
+# GD64-LABEL: <.Ltlsdesc_hi1>:
 # GD64-NEXT:   1020: auipc   a4, 0x1
 # GD64-NEXT:         ld      a5, 0xa8(a4)
 # GD64-NEXT:         addi    a0, a4, 0xa8
 # GD64-NEXT:         jalr    t0, 0x0(a5)
 # GD64-NEXT:         c.add   a0, tp
 ## &.got[c]-. = 0x20c0+8 - 0x1032 = 0x1096
+# GD64-LABEL: <.Ltlsdesc_hi2>:
 # GD64-NEXT:   1032: auipc   a6, 0x1
 # GD64-NEXT:         ld      a7, 0x96(a6)
 # GD64-NEXT:         addi    a0, a6, 0x96
@@ -64,6 +66,7 @@
 # LE64-NEXT:         jal     {{.*}} <foo>
 # LE64-NEXT:                 R_RISCV_JAL foo
 # LE64-NEXT:                 R_RISCV_RELAX *ABS*
+# LE64-LABEL: <.Ltlsdesc_hi1>:
 # LE64-NEXT:         addi    a0, zero, 0x7ff
 # LE64-NEXT:                 R_RISCV_TLSDESC_HI20 b
 # LE64-NEXT:                 R_RISCV_RELAX *ABS*
@@ -71,6 +74,7 @@
 # LE64-NEXT:                 R_RISCV_TLSDESC_ADD_LO12 .Ltlsdesc_hi1
 # LE64-NEXT:                 R_RISCV_TLSDESC_CALL .Ltlsdesc_hi1
 # LE64-NEXT:         c.add   a0, tp
+# LE64-LABEL: <.Ltlsdesc_hi2>:
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:                 R_RISCV_TLSDESC_HI20 b
 # LE64-NEXT:         addi    zero, zero, 0x0
@@ -93,9 +97,11 @@
 # LE64A-NEXT:         addi    a0, a0, -0x479
 # LE64A-NEXT:         c.add   a0, tp
 # LE64A-NEXT:         jal     {{.*}} <foo>
+# LE64A-LABEL: <.Ltlsdesc_hi1>:
 # LE64A-NEXT:         lui     a0, 0x2
 # LE64A-NEXT:         addi    a0, a0, -0x479
 # LE64A-NEXT:         c.add   a0, tp
+# LE64A-LABEL: <.Ltlsdesc_hi2>:
 # LE64A-NEXT:         addi    zero, zero, 0x0
 # LE64A-NEXT:         addi    zero, zero, 0x0
 # LE64A-NEXT:         lui     a0, 0x2
@@ -115,10 +121,12 @@
 # IE64-NEXT:         c.add   a0, tp
 # IE64-NEXT:         jal     {{.*}} <foo>
 ## &.got[c]-. = 0x120e0+8 - 0x11018 = 0x10d0
+# IE64-LABEL: <.Ltlsdesc_hi1>:
 # IE64-NEXT:  11018: auipc   a0, 0x1
 # IE64-NEXT:         ld      a0, 0xd0(a0)
 # IE64-NEXT:         c.add   a0, tp
 ## &.got[c]-. = 0x120e0+8 - 0x1102a = 0x10be
+# IE64-LABEL: <.Ltlsdesc_hi2>:
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:  1102a: auipc   a0, 0x1
diff --git a/lld/test/ELF/riscv-tlsdesc.s b/lld/test/ELF/riscv-tlsdesc.s
index 1738f86256caa..935ecbddfbfff 100644
--- a/lld/test/ELF/riscv-tlsdesc.s
+++ b/lld/test/ELF/riscv-tlsdesc.s
@@ -29,6 +29,14 @@
 # RUN: ld.lld -e 0 -z now a.32.o c.32.so -o a.32.ie
 # RUN: llvm-objdump --no-show-raw-insn -M no-aliases -h -d a.32.ie | FileCheck %s --check-prefix=IE32
 
+## Prior to https://github.com/llvm/llvm-project/pull/85817 the local TLSDESC
+## labels would be marked STT_TLS, resulting in an error "has an STT_TLS symbol but doesn't have an SHF_TLS section"
+
+# RUN: llvm-mc -triple=riscv64 -filetype=obj d.s -o d.64.o
+# RUN: ld.lld -shared -soname=d.64.so -o d.64.so d.64.o --fatal-warnings
+# RUN: llvm-mc -triple=riscv32 -filetype=obj d.s -o d.32.o --defsym ELF32=1
+# RUN: ld.lld -shared -soname=d.32.so -o d.32.so d.32.o --fatal-warnings
+
 # GD64-RELA:      .rela.dyn {
 # GD64-RELA-NEXT:   0x2408 R_RISCV_TLSDESC - 0x7FF
 # GD64-RELA-NEXT:   0x23E8 R_RISCV_TLSDESC a 0x0
@@ -68,14 +76,14 @@
 # GD64-NEXT:         add     a0, a0, tp
 
 ## &.got[b]-. = 0x23e0+40 - 0x12f4 = 0x1114
-# GD64-NEXT:   12f4: auipc   a2, 0x1
+# GD64:        12f4: auipc   a2, 0x1
 # GD64-NEXT:         ld      a3, 0x114(a2)
 # GD64-NEXT:         addi    a0, a2, 0x114
 # GD64-NEXT:         jalr    t0, 0x0(a3)
 # GD64-NEXT:         add     a0, a0, tp
 
 ## &.got[c]-. = 0x23e0+24 - 0x1308 = 0x10f0
-# GD64-NEXT:   1308: auipc   a4, 0x1
+# GD64:        1308: auipc   a4, 0x1
 # GD64-NEXT:         ld      a5, 0xf0(a4)
 # GD64-NEXT:         addi    a0, a4, 0xf0
 # GD64-NEXT:         jalr    t0, 0x0(a5)
@@ -83,7 +91,7 @@
 
 # NOREL: no relocations
 
-# LE64-LABEL: <.text>:
+# LE64-LABEL: <.Ltlsdesc_hi0>:
 ## st_value(a) = 8
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    zero, zero, 0x0
@@ -91,12 +99,14 @@
 # LE64-NEXT:         addi    a0, zero, 0x8
 # LE64-NEXT:         add     a0, a0, tp
 ## st_value(b) = 2047
+# LE64-LABEL: <.Ltlsdesc_hi1>:
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    a0, zero, 0x7ff
 # LE64-NEXT:         add     a0, a0, tp
 ## st_value(c) = 2048
+# LE64-LABEL: <.Ltlsdesc_hi2>:
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         lui     a0, 0x1
@@ -110,18 +120,20 @@
 # IE64:       .got     00000010 00000000000123a8
 
 ## a and b are optimized to use LE. c is optimized to IE.
-# IE64-LABEL: <.text>:
+# IE64-LABEL: <.Ltlsdesc_hi0>:
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    a0, zero, 0x8
 # IE64-NEXT:         add     a0, a0, tp
+# IE64-LABEL: <.Ltlsdesc_hi1>:
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    a0, zero, 0x7ff
 # IE64-NEXT:         add     a0, a0, tp
 ## &.got[c]-. = 0x123a8+8 - 0x112b8 = 0x10f8
+# IE64-LABEL: <.Ltlsdesc_hi2>:
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:  112b8: auipc   a0, 0x1
@@ -130,7 +142,7 @@
 
 # IE32:       .got     00000008 00012248
 
-# IE32-LABEL: <.text>:
+# IE32-LABEL: <.Ltlsdesc_hi0>:
 ## st_value(a) = 8
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    zero, zero, 0x0
@@ -138,12 +150,14 @@
 # IE32-NEXT:         addi    a0, zero, 0x8
 # IE32-NEXT:         add     a0, a0, tp
 ## st_value(b) = 2047
+# IE32-LABEL: <.Ltlsdesc_hi1>:
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    a0, zero, 0x7ff
 # IE32-NEXT:         add     a0, a0, tp
 ## &.got[c]-. = 0x12248+4 - 0x111cc = 0x1080
+# IE32-LABEL: <.Ltlsdesc_hi2>:
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:  111cc: auipc   a0, 0x1
@@ -192,3 +206,19 @@ b:
 .tbss
 .globl c
 c: .zero 4
+
+#--- d.s
+.macro load dst, src
+.ifdef ELF32
+lw \dst, \src
+.else
+ld \dst, \src
+.endif
+.endm
+
+.Ltlsdesc_hi0:
+  auipc	a0, %tlsdesc_hi(foo)
+  load	a1, %tlsdesc_load_lo(.Ltlsdesc_hi0)(a0)
+  addi	a0, a0, %tlsdesc_add_lo(.Ltlsdesc_hi0)
+  jalr	t0, 0(a1), %tlsdesc_call(.Ltlsdesc_hi0)
+  add	a1, a0, tp
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
index 254a9a4bc0ef0..b8e0f3a867f40 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
@@ -207,8 +207,6 @@ void RISCVMCExpr::fixELFSymbolsInTLSFixups(MCAssembler &Asm) const {
   case VK_RISCV_TLS_GOT_HI:
   case VK_RISCV_TLS_GD_HI:
   case VK_RISCV_TLSDESC_HI:
-  case VK_RISCV_TLSDESC_ADD_LO:
-  case VK_RISCV_TLSDESC_LOAD_LO:
     break;
   }
 
diff --git a/llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll b/llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll
new file mode 100644
index 0000000000000..23ba2ffb1ad76
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll
@@ -0,0 +1,24 @@
+;; The test in this file do not appear in tls-models.ll because
+;; they are not auto-generated.
+; RUN: llc -mtriple=riscv64 -relocation-model=pic -enable-tlsdesc < %s \
+; RUN:     | llvm-mc -triple=riscv64 -filetype=obj -o - \
+; RUN:     | llvm-readelf --symbols - \
+; RUN:     | FileCheck %s
+
+; RUN: llc -mtriple=riscv32 -relocation-model=pic -enable-tlsdesc < %s \
+; RUN:     | llvm-mc -triple=riscv32 -filetype=obj -o - \
+; RUN:     | llvm-readelf --symbols - \
+; RUN:     | FileCheck %s
+
+; Check that TLS symbols are lowered correctly based on the specified
+; model. Make sure they're external to avoid them all being optimised to Local
+; Exec for the executable.
+
+@unspecified = external thread_local global i32
+
+define ptr @f1() nounwind {
+entry:
+  ret ptr @unspecified
+  ; CHECK: Symbol table '.symtab' contains 7 entries:
+  ; CHECK: TLS {{.*}} unspecified
+}

@llvmbot
Copy link
Member Author

llvmbot commented May 9, 2024

@llvm/pr-subscribers-backend-risc-v

Author: None (llvmbot)

Changes

Backport f6f474c dfe4ca9

Requested by: @ilovepi

Full diff: https://github.com/llvm/llvm-project/pull/91678.diff

5 Files Affected:

  • (modified) lld/ELF/Relocations.cpp (+4-1)
  • (modified) lld/test/ELF/riscv-tlsdesc-relax.s (+8)
  • (modified) lld/test/ELF/riscv-tlsdesc.s (+35-5)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp (-2)
  • (added) llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll (+24)
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 619fbaf5dc545..92a1b9baaca3d 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -1480,7 +1480,10 @@ template <class ELFT, class RelTy> void RelocationScanner::scanOne(RelTy *&i) {
 
   // Process TLS relocations, including TLS optimizations. Note that
   // R_TPREL and R_TPREL_NEG relocations are resolved in processAux.
-  if (sym.isTls()) {
+  //
+  // Some RISCV TLSDESC relocations reference a local NOTYPE symbol,
+  // but we need to process them in handleTlsRelocation.
+  if (sym.isTls() || oneof<R_TLSDESC_PC, R_TLSDESC_CALL>(expr)) {
     if (unsigned processed =
             handleTlsRelocation(type, sym, *sec, offset, addend, expr)) {
       i += processed - 1;
diff --git a/lld/test/ELF/riscv-tlsdesc-relax.s b/lld/test/ELF/riscv-tlsdesc-relax.s
index fb24317e6535c..5718d4175be11 100644
--- a/lld/test/ELF/riscv-tlsdesc-relax.s
+++ b/lld/test/ELF/riscv-tlsdesc-relax.s
@@ -33,12 +33,14 @@
 # GD64-NEXT:         c.add   a0, tp
 # GD64-NEXT:         jal     {{.*}} <foo>
 ## &.got[c]-. = 0x20c0+8 - 0x1020 = 0x10a8
+# GD64-LABEL: <.Ltlsdesc_hi1>:
 # GD64-NEXT:   1020: auipc   a4, 0x1
 # GD64-NEXT:         ld      a5, 0xa8(a4)
 # GD64-NEXT:         addi    a0, a4, 0xa8
 # GD64-NEXT:         jalr    t0, 0x0(a5)
 # GD64-NEXT:         c.add   a0, tp
 ## &.got[c]-. = 0x20c0+8 - 0x1032 = 0x1096
+# GD64-LABEL: <.Ltlsdesc_hi2>:
 # GD64-NEXT:   1032: auipc   a6, 0x1
 # GD64-NEXT:         ld      a7, 0x96(a6)
 # GD64-NEXT:         addi    a0, a6, 0x96
@@ -64,6 +66,7 @@
 # LE64-NEXT:         jal     {{.*}} <foo>
 # LE64-NEXT:                 R_RISCV_JAL foo
 # LE64-NEXT:                 R_RISCV_RELAX *ABS*
+# LE64-LABEL: <.Ltlsdesc_hi1>:
 # LE64-NEXT:         addi    a0, zero, 0x7ff
 # LE64-NEXT:                 R_RISCV_TLSDESC_HI20 b
 # LE64-NEXT:                 R_RISCV_RELAX *ABS*
@@ -71,6 +74,7 @@
 # LE64-NEXT:                 R_RISCV_TLSDESC_ADD_LO12 .Ltlsdesc_hi1
 # LE64-NEXT:                 R_RISCV_TLSDESC_CALL .Ltlsdesc_hi1
 # LE64-NEXT:         c.add   a0, tp
+# LE64-LABEL: <.Ltlsdesc_hi2>:
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:                 R_RISCV_TLSDESC_HI20 b
 # LE64-NEXT:         addi    zero, zero, 0x0
@@ -93,9 +97,11 @@
 # LE64A-NEXT:         addi    a0, a0, -0x479
 # LE64A-NEXT:         c.add   a0, tp
 # LE64A-NEXT:         jal     {{.*}} <foo>
+# LE64A-LABEL: <.Ltlsdesc_hi1>:
 # LE64A-NEXT:         lui     a0, 0x2
 # LE64A-NEXT:         addi    a0, a0, -0x479
 # LE64A-NEXT:         c.add   a0, tp
+# LE64A-LABEL: <.Ltlsdesc_hi2>:
 # LE64A-NEXT:         addi    zero, zero, 0x0
 # LE64A-NEXT:         addi    zero, zero, 0x0
 # LE64A-NEXT:         lui     a0, 0x2
@@ -115,10 +121,12 @@
 # IE64-NEXT:         c.add   a0, tp
 # IE64-NEXT:         jal     {{.*}} <foo>
 ## &.got[c]-. = 0x120e0+8 - 0x11018 = 0x10d0
+# IE64-LABEL: <.Ltlsdesc_hi1>:
 # IE64-NEXT:  11018: auipc   a0, 0x1
 # IE64-NEXT:         ld      a0, 0xd0(a0)
 # IE64-NEXT:         c.add   a0, tp
 ## &.got[c]-. = 0x120e0+8 - 0x1102a = 0x10be
+# IE64-LABEL: <.Ltlsdesc_hi2>:
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:  1102a: auipc   a0, 0x1
diff --git a/lld/test/ELF/riscv-tlsdesc.s b/lld/test/ELF/riscv-tlsdesc.s
index 1738f86256caa..935ecbddfbfff 100644
--- a/lld/test/ELF/riscv-tlsdesc.s
+++ b/lld/test/ELF/riscv-tlsdesc.s
@@ -29,6 +29,14 @@
 # RUN: ld.lld -e 0 -z now a.32.o c.32.so -o a.32.ie
 # RUN: llvm-objdump --no-show-raw-insn -M no-aliases -h -d a.32.ie | FileCheck %s --check-prefix=IE32
 
+## Prior to https://github.com/llvm/llvm-project/pull/85817 the local TLSDESC
+## labels would be marked STT_TLS, resulting in an error "has an STT_TLS symbol but doesn't have an SHF_TLS section"
+
+# RUN: llvm-mc -triple=riscv64 -filetype=obj d.s -o d.64.o
+# RUN: ld.lld -shared -soname=d.64.so -o d.64.so d.64.o --fatal-warnings
+# RUN: llvm-mc -triple=riscv32 -filetype=obj d.s -o d.32.o --defsym ELF32=1
+# RUN: ld.lld -shared -soname=d.32.so -o d.32.so d.32.o --fatal-warnings
+
 # GD64-RELA:      .rela.dyn {
 # GD64-RELA-NEXT:   0x2408 R_RISCV_TLSDESC - 0x7FF
 # GD64-RELA-NEXT:   0x23E8 R_RISCV_TLSDESC a 0x0
@@ -68,14 +76,14 @@
 # GD64-NEXT:         add     a0, a0, tp
 
 ## &.got[b]-. = 0x23e0+40 - 0x12f4 = 0x1114
-# GD64-NEXT:   12f4: auipc   a2, 0x1
+# GD64:        12f4: auipc   a2, 0x1
 # GD64-NEXT:         ld      a3, 0x114(a2)
 # GD64-NEXT:         addi    a0, a2, 0x114
 # GD64-NEXT:         jalr    t0, 0x0(a3)
 # GD64-NEXT:         add     a0, a0, tp
 
 ## &.got[c]-. = 0x23e0+24 - 0x1308 = 0x10f0
-# GD64-NEXT:   1308: auipc   a4, 0x1
+# GD64:        1308: auipc   a4, 0x1
 # GD64-NEXT:         ld      a5, 0xf0(a4)
 # GD64-NEXT:         addi    a0, a4, 0xf0
 # GD64-NEXT:         jalr    t0, 0x0(a5)
@@ -83,7 +91,7 @@
 
 # NOREL: no relocations
 
-# LE64-LABEL: <.text>:
+# LE64-LABEL: <.Ltlsdesc_hi0>:
 ## st_value(a) = 8
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    zero, zero, 0x0
@@ -91,12 +99,14 @@
 # LE64-NEXT:         addi    a0, zero, 0x8
 # LE64-NEXT:         add     a0, a0, tp
 ## st_value(b) = 2047
+# LE64-LABEL: <.Ltlsdesc_hi1>:
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    a0, zero, 0x7ff
 # LE64-NEXT:         add     a0, a0, tp
 ## st_value(c) = 2048
+# LE64-LABEL: <.Ltlsdesc_hi2>:
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         addi    zero, zero, 0x0
 # LE64-NEXT:         lui     a0, 0x1
@@ -110,18 +120,20 @@
 # IE64:       .got     00000010 00000000000123a8
 
 ## a and b are optimized to use LE. c is optimized to IE.
-# IE64-LABEL: <.text>:
+# IE64-LABEL: <.Ltlsdesc_hi0>:
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    a0, zero, 0x8
 # IE64-NEXT:         add     a0, a0, tp
+# IE64-LABEL: <.Ltlsdesc_hi1>:
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    a0, zero, 0x7ff
 # IE64-NEXT:         add     a0, a0, tp
 ## &.got[c]-. = 0x123a8+8 - 0x112b8 = 0x10f8
+# IE64-LABEL: <.Ltlsdesc_hi2>:
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:         addi    zero, zero, 0x0
 # IE64-NEXT:  112b8: auipc   a0, 0x1
@@ -130,7 +142,7 @@
 
 # IE32:       .got     00000008 00012248
 
-# IE32-LABEL: <.text>:
+# IE32-LABEL: <.Ltlsdesc_hi0>:
 ## st_value(a) = 8
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    zero, zero, 0x0
@@ -138,12 +150,14 @@
 # IE32-NEXT:         addi    a0, zero, 0x8
 # IE32-NEXT:         add     a0, a0, tp
 ## st_value(b) = 2047
+# IE32-LABEL: <.Ltlsdesc_hi1>:
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    a0, zero, 0x7ff
 # IE32-NEXT:         add     a0, a0, tp
 ## &.got[c]-. = 0x12248+4 - 0x111cc = 0x1080
+# IE32-LABEL: <.Ltlsdesc_hi2>:
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:         addi    zero, zero, 0x0
 # IE32-NEXT:  111cc: auipc   a0, 0x1
@@ -192,3 +206,19 @@ b:
 .tbss
 .globl c
 c: .zero 4
+
+#--- d.s
+.macro load dst, src
+.ifdef ELF32
+lw \dst, \src
+.else
+ld \dst, \src
+.endif
+.endm
+
+.Ltlsdesc_hi0:
+  auipc	a0, %tlsdesc_hi(foo)
+  load	a1, %tlsdesc_load_lo(.Ltlsdesc_hi0)(a0)
+  addi	a0, a0, %tlsdesc_add_lo(.Ltlsdesc_hi0)
+  jalr	t0, 0(a1), %tlsdesc_call(.Ltlsdesc_hi0)
+  add	a1, a0, tp
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
index 254a9a4bc0ef0..b8e0f3a867f40 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
@@ -207,8 +207,6 @@ void RISCVMCExpr::fixELFSymbolsInTLSFixups(MCAssembler &Asm) const {
   case VK_RISCV_TLS_GOT_HI:
   case VK_RISCV_TLS_GD_HI:
   case VK_RISCV_TLSDESC_HI:
-  case VK_RISCV_TLSDESC_ADD_LO:
-  case VK_RISCV_TLSDESC_LOAD_LO:
     break;
   }
 
diff --git a/llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll b/llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll
new file mode 100644
index 0000000000000..23ba2ffb1ad76
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll
@@ -0,0 +1,24 @@
+;; The test in this file do not appear in tls-models.ll because
+;; they are not auto-generated.
+; RUN: llc -mtriple=riscv64 -relocation-model=pic -enable-tlsdesc < %s \
+; RUN:     | llvm-mc -triple=riscv64 -filetype=obj -o - \
+; RUN:     | llvm-readelf --symbols - \
+; RUN:     | FileCheck %s
+
+; RUN: llc -mtriple=riscv32 -relocation-model=pic -enable-tlsdesc < %s \
+; RUN:     | llvm-mc -triple=riscv32 -filetype=obj -o - \
+; RUN:     | llvm-readelf --symbols - \
+; RUN:     | FileCheck %s
+
+; Check that TLS symbols are lowered correctly based on the specified
+; model. Make sure they're external to avoid them all being optimised to Local
+; Exec for the executable.
+
+@unspecified = external thread_local global i32
+
+define ptr @f1() nounwind {
+entry:
+  ret ptr @unspecified
+  ; CHECK: Symbol table '.symtab' contains 7 entries:
+  ; CHECK: TLS {{.*}} unspecified
+}

@MaskRay
Copy link
Member

MaskRay commented May 10, 2024

LGTM

@tstellar
Copy link
Collaborator

This branch needs to be rebased.

@ilovepi @tstellar
[RISCV][lld] Set the type of TLSDESC relocation's referenced local sy…
6cfa40e 
…mbol to STT_NOTYPE

When adding fixups for RISCV_TLSDESC_ADD_LO and RISCV_TLSDESC_LOAD_LO,
the local label added for RISCV TLSDESC relocations have STT_TLS set,
which is incorrect. Instead, these labels should have `STT_NOTYPE`.

This patch stops adding such fixups and avoid setting the STT_TLS on
these symbols. Failing to do so can cause LLD to emit an error `has an
STT_TLS symbol but doesn't have an SHF_TLS section`. We additionally,
adjust how LLD services these relocations to avoid errors with
incompatible relocation and symbol types.

Reviewers: topperc, MaskRay

Reviewed By: MaskRay

Pull Request: llvm#85817

(cherry picked from commit dfe4ca9)
@tstellar tstellar merged commit 6cfa40e into llvm:release/18.x May 14, 2024
10 of 11 checks passed
@tstellar
Copy link
Collaborator

@ilovepi (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaskRay MaskRay MaskRay approved these changes

@ilovepi ilovepi Awaiting requested review from ilovepi

Assignees
No one assigned
Labels
backend:RISC-V lld:ELF lld
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 18.X Release
Development

Successfully merging this pull request may close these issues.
4 participants
@llvmbot @MaskRay @tstellar @ilovepi