[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)

2024-05-13 Thread Tom Stellard via llvm-branch-commits 

tstellar wrote:

@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.


https://github.com/llvm/llvm-project/pull/91678
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[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)

2024-05-13 Thread Tom Stellard via llvm-branch-commits 

https://github.com/tstellar closed 
https://github.com/llvm/llvm-project/pull/91678
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[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)

2024-05-13 Thread Tom Stellard via llvm-branch-commits 

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

>From 6cfa40e450cfe7980a0c4aa0e17a8367b89f8d39 Mon Sep 17 00:00:00 2001
From: Paul Kirth 
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  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(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 {{.*}} 
 ## &.got[c]-. = 0x20c0+8 - 0x1020 = 0x10a8
+# GD64-LABEL: <.Ltlsdesc_hi1>:
 # GD64-NEXT:   1020: auipc   a4, 0x1
 # GD64-NEXT: ld  a5, 0xa8(a4)
 # GD64-NEXT: addia0, a4, 0xa8
 # GD64-NEXT: jalrt0, 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: addia0, a6, 0x96
@@ -64,6 +66,7 @@
 # LE64-NEXT: jal {{.*}} 
 # LE64-NEXT: R_RISCV_JAL foo
 # LE64-NEXT: R_RISCV_RELAX *ABS*
+# LE64-LABEL: <.Ltlsdesc_hi1>:
 # LE64-NEXT: addia0, 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: addizero, zero, 0x0
 # LE64-NEXT: R_RISCV_TLSDESC_HI20 b
 # LE64-NEXT: addizero, zero, 0x0
@@ -93,9 +97,11 @@
 # LE64A-NEXT: addia0, a0, -0x479
 # LE64A-NEXT: c.add   a0, tp
 # LE64A-NEXT: jal {{.*}} 
+# LE64A-LABEL: <.Ltlsdesc_hi1>:
 # LE64A-NEXT: lui a0, 0x2
 # LE64A-NEXT: addia0, a0, -0x479
 # LE64A-NEXT: c.add   a0, tp
+# LE64A-LABEL: <.Ltlsdesc_hi2>:
 # LE64A-NEXT: addizero, zero, 0x0
 # LE64A-NEXT: addizero, zero, 0x0
 # LE64A-NEXT: lui a0, 0x2
@@ -115,10 +121,12 @@
 # IE64-NEXT: c.add   a0, tp
 # IE64-NEXT: jal {{.*}} 
 ## &.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: addizero, zero, 0x0
 # IE64-NEXT: addizero, 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 --c

[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)

2024-05-13 Thread Paul Kirth via llvm-branch-commits 

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 
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  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(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 {{.*}} 
 ## &.got[c]-. = 0x20c0+8 - 0x1020 = 0x10a8
+# GD64-LABEL: <.Ltlsdesc_hi1>:
 # GD64-NEXT:   1020: auipc   a4, 0x1
 # GD64-NEXT: ld  a5, 0xa8(a4)
 # GD64-NEXT: addia0, a4, 0xa8
 # GD64-NEXT: jalrt0, 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: addia0, a6, 0x96
@@ -64,6 +66,7 @@
 # LE64-NEXT: jal {{.*}} 
 # LE64-NEXT: R_RISCV_JAL foo
 # LE64-NEXT: R_RISCV_RELAX *ABS*
+# LE64-LABEL: <.Ltlsdesc_hi1>:
 # LE64-NEXT: addia0, 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: addizero, zero, 0x0
 # LE64-NEXT: R_RISCV_TLSDESC_HI20 b
 # LE64-NEXT: addizero, zero, 0x0
@@ -93,9 +97,11 @@
 # LE64A-NEXT: addia0, a0, -0x479
 # LE64A-NEXT: c.add   a0, tp
 # LE64A-NEXT: jal {{.*}} 
+# LE64A-LABEL: <.Ltlsdesc_hi1>:
 # LE64A-NEXT: lui a0, 0x2
 # LE64A-NEXT: addia0, a0, -0x479
 # LE64A-NEXT: c.add   a0, tp
+# LE64A-LABEL: <.Ltlsdesc_hi2>:
 # LE64A-NEXT: addizero, zero, 0x0
 # LE64A-NEXT: addizero, zero, 0x0
 # LE64A-NEXT: lui a0, 0x2
@@ -115,10 +121,12 @@
 # IE64-NEXT: c.add   a0, tp
 # IE64-NEXT: jal {{.*}} 
 ## &.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: addizero, zero, 0x0
 # IE64-NEXT: addizero, 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 --ch

[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)

2024-05-10 Thread Tom Stellard via llvm-branch-commits 

tstellar wrote:

This branch needs to be rebased.

https://github.com/llvm/llvm-project/pull/91678
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[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)

2024-05-10 Thread Fangrui Song via llvm-branch-commits 

https://github.com/MaskRay approved this pull request.


https://github.com/llvm/llvm-project/pull/91678
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[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)

2024-05-09 Thread Fangrui Song via llvm-branch-commits 

MaskRay wrote:

LGTM

https://github.com/llvm/llvm-project/pull/91678
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[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)

2024-05-09 Thread via llvm-branch-commits 

llvmbot wrote:




@llvm/pr-subscribers-lld-elf

Author: None (llvmbot)


Changes

Backport f6f474c4ef9694a4ca8f08d59fd112c250fb9c73 
dfe4ca9b7f4a422500d78280dc5eefd1979939e6

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
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  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(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 {{.*}} 
 ## &.got[c]-. = 0x20c0+8 - 0x1020 = 0x10a8
+# GD64-LABEL: <.Ltlsdesc_hi1>:
 # GD64-NEXT:   1020: auipc   a4, 0x1
 # GD64-NEXT: ld  a5, 0xa8(a4)
 # GD64-NEXT: addia0, a4, 0xa8
 # GD64-NEXT: jalrt0, 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: addia0, a6, 0x96
@@ -64,6 +66,7 @@
 # LE64-NEXT: jal {{.*}} 
 # LE64-NEXT: R_RISCV_JAL foo
 # LE64-NEXT: R_RISCV_RELAX *ABS*
+# LE64-LABEL: <.Ltlsdesc_hi1>:
 # LE64-NEXT: addia0, 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: addizero, zero, 0x0
 # LE64-NEXT: R_RISCV_TLSDESC_HI20 b
 # LE64-NEXT: addizero, zero, 0x0
@@ -93,9 +97,11 @@
 # LE64A-NEXT: addia0, a0, -0x479
 # LE64A-NEXT: c.add   a0, tp
 # LE64A-NEXT: jal {{.*}} 
+# LE64A-LABEL: <.Ltlsdesc_hi1>:
 # LE64A-NEXT: lui a0, 0x2
 # LE64A-NEXT: addia0, a0, -0x479
 # LE64A-NEXT: c.add   a0, tp
+# LE64A-LABEL: <.Ltlsdesc_hi2>:
 # LE64A-NEXT: addizero, zero, 0x0
 # LE64A-NEXT: addizero, zero, 0x0
 # LE64A-NEXT: lui a0, 0x2
@@ -115,10 +121,12 @@
 # IE64-NEXT: c.add   a0, tp
 # IE64-NEXT: jal {{.*}} 
 ## &.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: addizero, zero, 0x0
 # IE64-NEXT: addizero, 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: addia0, a2, 0x114

[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)

2024-05-09 Thread via llvm-branch-commits 

llvmbot wrote:




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

Author: None (llvmbot)


Changes

Backport f6f474c4ef9694a4ca8f08d59fd112c250fb9c73 
dfe4ca9b7f4a422500d78280dc5eefd1979939e6

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
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  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(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 {{.*}} 
 ## &.got[c]-. = 0x20c0+8 - 0x1020 = 0x10a8
+# GD64-LABEL: <.Ltlsdesc_hi1>:
 # GD64-NEXT:   1020: auipc   a4, 0x1
 # GD64-NEXT: ld  a5, 0xa8(a4)
 # GD64-NEXT: addia0, a4, 0xa8
 # GD64-NEXT: jalrt0, 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: addia0, a6, 0x96
@@ -64,6 +66,7 @@
 # LE64-NEXT: jal {{.*}} 
 # LE64-NEXT: R_RISCV_JAL foo
 # LE64-NEXT: R_RISCV_RELAX *ABS*
+# LE64-LABEL: <.Ltlsdesc_hi1>:
 # LE64-NEXT: addia0, 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: addizero, zero, 0x0
 # LE64-NEXT: R_RISCV_TLSDESC_HI20 b
 # LE64-NEXT: addizero, zero, 0x0
@@ -93,9 +97,11 @@
 # LE64A-NEXT: addia0, a0, -0x479
 # LE64A-NEXT: c.add   a0, tp
 # LE64A-NEXT: jal {{.*}} 
+# LE64A-LABEL: <.Ltlsdesc_hi1>:
 # LE64A-NEXT: lui a0, 0x2
 # LE64A-NEXT: addia0, a0, -0x479
 # LE64A-NEXT: c.add   a0, tp
+# LE64A-LABEL: <.Ltlsdesc_hi2>:
 # LE64A-NEXT: addizero, zero, 0x0
 # LE64A-NEXT: addizero, zero, 0x0
 # LE64A-NEXT: lui a0, 0x2
@@ -115,10 +121,12 @@
 # IE64-NEXT: c.add   a0, tp
 # IE64-NEXT: jal {{.*}} 
 ## &.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: addizero, zero, 0x0
 # IE64-NEXT: addizero, 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: addia0, a2

[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)

2024-05-09 Thread via llvm-branch-commits 

llvmbot wrote:

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

https://github.com/llvm/llvm-project/pull/91678
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[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)

2024-05-09 Thread via llvm-branch-commits 

https://github.com/llvmbot milestoned 
https://github.com/llvm/llvm-project/pull/91678
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

[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)

2024-05-09 Thread via llvm-branch-commits 

https://github.com/llvmbot created 
https://github.com/llvm/llvm-project/pull/91678

Backport f6f474c4ef9694a4ca8f08d59fd112c250fb9c73 
dfe4ca9b7f4a422500d78280dc5eefd1979939e6

Requested by: @ilovepi

>From fe4854d3b2e49c816aaf37a7f3421bde55215ba1 Mon Sep 17 00:00:00 2001
From: Paul Kirth 
Date: Wed, 20 Mar 2024 13:39:39 -0700
Subject: [PATCH 1/2] [llvm][lld] Pre-commit tests for RISCV TLSDESC symbols

Currently, we mistakenly mark the local labels used in RISC-V TLSDESC as
TLS symbols, when they should not be. This patch adds tests with the
current incorrect behavior, and subsequent patches will address the
issue.

Reviewers: MaskRay, topperc

Reviewed By: MaskRay

Pull Request: https://github.com/llvm/llvm-project/pull/85816

(cherry picked from commit f6f474c4ef9694a4ca8f08d59fd112c250fb9c73)
---
 lld/test/ELF/riscv-tlsdesc.s  | 25 +++
 llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll | 24 ++
 2 files changed, 49 insertions(+)
 create mode 100644 llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll

diff --git a/lld/test/ELF/riscv-tlsdesc.s b/lld/test/ELF/riscv-tlsdesc.s
index 1738f86256caa..c583e15cf30ce 100644
--- a/lld/test/ELF/riscv-tlsdesc.s
+++ b/lld/test/ELF/riscv-tlsdesc.s
@@ -29,6 +29,12 @@
 # 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
+
+# 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
+
 # GD64-RELA:  .rela.dyn {
 # GD64-RELA-NEXT:   0x2408 R_RISCV_TLSDESC - 0x7FF
 # GD64-RELA-NEXT:   0x23E8 R_RISCV_TLSDESC a 0x0
@@ -150,6 +156,9 @@
 # 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
@@ -192,3 +201,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:
+  auipca0, %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/test/CodeGen/RISCV/tlsdesc-symbol.ll 
b/llvm/test/CodeGen/RISCV/tlsdesc-symbol.ll
new file mode 100644
index 0..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
+}

>From f7b87341e88e26fa90acd36dac0f139ca46961bb Mon Sep 17 00:00:00 2001
From: Paul Kirth 
Date: Fri, 22 Mar 2024 12:27:41 -0700
Subject: [PATCH 2/2] [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/ll