Skip to content

Commit

Permalink
[ELF,RISCV] Fix TLSDESC=>IE when there is no TLS section
Browse files Browse the repository at this point in the history
See the comment in handleTlsRelocation. For TLSDESC=>IE (the TLS symbol
is defined in another DSO), R_RISCV_TLSDESC_{LOAD_LO12,ADD_LO12_I,CALL}
referencing a non-preemptible label uses the `R_RELAX_TLS_GD_TO_LE` code
path.

If there is no TLS section, `getTlsTpOffset` will be called with null
`Out::tlsPhdr`, leading to a null pointer dereference. Since the return
value is used by `RISCV::relocateAlloc` and ignored there, just return
0.

LoongArch TLSDESC doesn't use STT_NOTYPE labels. The `if (..) return 0;`
is a no-op for LoongArch.

This patch is a follow-up to llvm#79239 and fixes some comments.

Pull Request: llvm#98569
  • Loading branch information
MaskRay authored and aaryanshukla committed Jul 14, 2024
1 parent 3795d2f commit e5acbe4
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
4 changes: 2 additions & 2 deletions lld/ELF/Arch/RISCV.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -631,8 +631,8 @@ void RISCV::relocateAlloc(InputSectionBase &sec, uint8_t *buf) const {
continue;
case R_RELAX_TLS_GD_TO_LE:
// See the comment in handleTlsRelocation. For TLSDESC=>IE,
// R_RISCV_TLSDESC_{LOAD_LO12,ADD_LO12,CALL} also reach here. If isToIe is
// true, this is actually TLSDESC=>IE optimization.
// R_RISCV_TLSDESC_{LOAD_LO12,ADD_LO12,CALL} also reach here. If isToLe is
// false, this is actually TLSDESC=>IE optimization.
if (rel.type == R_RISCV_TLSDESC_HI20) {
tlsdescVal = val;
isToLe = true;
Expand Down
5 changes: 5 additions & 0 deletions lld/ELF/InputSection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,11 @@ static int64_t getTlsTpOffset(const Symbol &s) {
return s.getVA(0) + (tls->p_vaddr & (tls->p_align - 1)) - 0x7000;
case EM_LOONGARCH:
case EM_RISCV:
// See the comment in handleTlsRelocation. For TLSDESC=>IE,
// R_RISCV_TLSDESC_{LOAD_LO12,ADD_LO12_I,CALL} also reach here. While
// `tls` may be null, the return value is ignored.
if (s.type != STT_TLS)
return 0;
return s.getVA(0) + (tls->p_vaddr & (tls->p_align - 1));

// Variant 2.
Expand Down
4 changes: 2 additions & 2 deletions lld/ELF/Relocations.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1399,8 +1399,8 @@ static unsigned handleTlsRelocation(RelType type, Symbol &sym,
// depending on the symbol being locally defined or not.
//
// R_RISCV_TLSDESC_{LOAD_LO12,ADD_LO12_I,CALL} reference a non-preemptible
// label, so the LE optimization will be categorized as
// R_RELAX_TLS_GD_TO_LE. We fix the categorization in RISCV::relocateAlloc.
// label, so TLSDESC=>IE will be categorized as R_RELAX_TLS_GD_TO_LE. We fix
// the categorization in RISCV::relocateAlloc.
if (sym.isPreemptible) {
sym.setFlags(NEEDS_TLSGD_TO_IE);
c.addReloc({target->adjustTlsExpr(type, R_RELAX_TLS_GD_TO_IE), type,
Expand Down
25 changes: 25 additions & 0 deletions lld/test/ELF/riscv-tlsdesc.s
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@
# 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

## The output has a TLS reference but no TLS section.
# RUN: llvm-mc -filetype=obj -triple=riscv64 a1.s -o a1.64.o
# RUN: ld.lld -pie a1.64.o c.64.so -o a1.64
# RUN: llvm-objdump --no-show-raw-insn -M no-aliases -Rd a1.64 | FileCheck %s --check-prefix=IE64A

# GD64-RELA: .rela.dyn {
# GD64-RELA-NEXT: 0x2408 R_RISCV_TLSDESC - 0x7FF
# GD64-RELA-NEXT: 0x23E8 R_RISCV_TLSDESC a 0x0
Expand Down Expand Up @@ -164,6 +169,17 @@
# IE32-NEXT: lw a0, 0x80(a0)
# IE32-NEXT: add a0, a0, tp

# IE64A: OFFSET TYPE VALUE
# IE64A-NEXT: 0000000000002340 R_RISCV_TLS_TPREL64 c
# IE64A-EMPTY:
## &.got[c]-. = 0x2340 - 0x1258 = 0x10e8
# IE64A-LABEL: <.Ltlsdesc_hi2>:
# IE64A-NEXT: addi zero, zero, 0x0
# IE64A-NEXT: addi zero, zero, 0x0
# IE64A-NEXT: 1258: auipc a0, 0x1
# IE64A-NEXT: ld a0, 0xe8(a0)
# IE64A-NEXT: add a0, a0, tp

#--- a.s
.macro load dst, src
.ifdef ELF32
Expand Down Expand Up @@ -202,6 +218,15 @@ a:
b:
.zero 1

#--- a1.s
## a.s without TLS definitions.
.Ltlsdesc_hi2:
auipc a4, %tlsdesc_hi(c)
ld a5, %tlsdesc_load_lo(.Ltlsdesc_hi2)(a4)
addi a0, a4, %tlsdesc_add_lo(.Ltlsdesc_hi2)
jalr t0, 0(a5), %tlsdesc_call(.Ltlsdesc_hi2)
add a0, a0, tp

#--- c.s
.tbss
.globl c
Expand Down

0 comments on commit e5acbe4

Please sign in to comment.