Skip to content

Commit

Permalink
[RISCV][lld] Set the type of TLSDESC relocation's referenced local sy…
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
ilovepi authored and tstellar committed May 14, 2024
1 parent 1184a9c commit 6cfa40e
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 14 deletions.
5 changes: 4 additions & 1 deletion lld/ELF/Relocations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions lld/test/ELF/riscv-tlsdesc-relax.s
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -64,13 +66,15 @@
# 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*
# LE64-NEXT: R_RISCV_TLSDESC_LOAD_LO12 .Ltlsdesc_hi1
# 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
Expand All @@ -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
Expand All @@ -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
Expand Down
27 changes: 16 additions & 11 deletions lld/test/ELF/riscv-tlsdesc.s
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -74,35 +76,37 @@
# 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)
# GD64-NEXT: add a0, a0, tp

# 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
# LE64-NEXT: addi zero, zero, 0x0
# 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
Expand All @@ -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
Expand All @@ -136,29 +142,28 @@

# 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
# IE32-NEXT: addi zero, zero, 0x0
# 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
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 6cfa40e

Please sign in to comment.