[llvm-branch-commits] [lld] [llvm] release/18.x: [RISCV][lld] Set the type of TLSDESC relocation's referenced local symbol to STT_NOTYPE (PR #91678)

Paul Kirth via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Mon May 13 11:01:31 PDT 2024


https://github.com/ilovepi updated https://github.com/llvm/llvm-project/pull/91678

>From 9c6b3b2733a99543b19e3cd38752ebd99188bd6d Mon Sep 17 00:00:00 2001
From: Paul Kirth <paulkirth at google.com>
Date: Fri, 22 Mar 2024 12:27:41 -0700
Subject: [PATCH] [RISCV][lld] Set the type of TLSDESC relocation's referenced
 local symbol 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: https://github.com/llvm/llvm-project/pull/85817

(cherry picked from commit dfe4ca9b7f4a422500d78280dc5eefd1979939e6)
---
 lld/ELF/Relocations.cpp                       |  5 +++-
 lld/test/ELF/riscv-tlsdesc-relax.s            |  8 ++++++
 lld/test/ELF/riscv-tlsdesc.s                  | 27 +++++++++++--------
 .../Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp |  2 --
 4 files changed, 28 insertions(+), 14 deletions(-)

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 c583e15cf30ce..935ecbddfbfff 100644
--- a/lld/test/ELF/riscv-tlsdesc.s
+++ b/lld/test/ELF/riscv-tlsdesc.s
@@ -29,11 +29,13 @@
 # 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
 
-# RUN: llvm-mc -triple=riscv64 -filetype=obj d.s -o d.64.o
-# RUN: not ld.lld -shared -soname=d.64.so -o d.64.so d.64.o 2>&1 | FileCheck %s --check-prefix=BADTLSLABEL
+## 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: not ld.lld -shared -soname=d.32.so -o d.32.so d.32.o 2>&1 | FileCheck %s --check-prefix=BADTLSLABEL
+# 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
@@ -74,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)
@@ -89,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
@@ -97,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
@@ -116,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
@@ -136,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
@@ -144,21 +150,20 @@
 # 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
 # IE32-NEXT:         lw      a0, 0x80(a0)
 # IE32-NEXT:         add     a0, a0, tp
 
-## FIXME This should not pass, but the code MC layer needs a fix to prevent this.
-# BADTLSLABEL: error: d.{{.*}}.o has an STT_TLS symbol but doesn't have an SHF_TLS section
-
 #--- a.s
 .macro load dst, src
 .ifdef ELF32
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;
   }
 



More information about the llvm-branch-commits mailing list