[LLD][AArch64] Handle R_AARCH64_TLS_DTPREL64 in non-alloc sections - #183962
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-lld-elf Author: Shivam Gupta (xgupta) ChangesWith PR#155776, clang started to emit R_AARCH64_TLS_DTPREL64 in .debug_info section to help in debugging TLS variables. This patch helps recognise that relocation in LLD linker. Full diff: https://github.com/llvm/llvm-project/pull/183962.diff 2 Files Affected:
diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index f85a3f48f2183..c329e653b9853 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -156,6 +156,8 @@ RelExpr AArch64::getRelExpr(RelType type, const Symbol &s,
case R_AARCH64_PREL32:
case R_AARCH64_PREL64:
return R_PC;
+ case R_AARCH64_TLS_DTPREL64:
+ return R_DTPREL;
case R_AARCH64_NONE:
return R_NONE;
default:
@@ -649,6 +651,10 @@ void AArch64::relocate(uint8_t *loc, const Relocation &rel,
checkInt(ctx, loc, val, 32, rel);
write32(ctx, loc, val);
break;
+ case R_AARCH64_TLS_DTPREL64:
+ checkInt(ctx, loc, val, 64, rel);
+ write64(ctx, loc, val);
+ break;
case R_AARCH64_ADD_ABS_LO12_NC:
case R_AARCH64_AUTH_GOT_ADD_LO12_NC:
write32Imm12(loc, val);
diff --git a/lld/test/ELF/aarch64-tls-dtprel.s b/lld/test/ELF/aarch64-tls-dtprel.s
new file mode 100644
index 0000000000000..49c1ac1456354
--- /dev/null
+++ b/lld/test/ELF/aarch64-tls-dtprel.s
@@ -0,0 +1,41 @@
+# REQUIRES: aarch64
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu %s -o %t.o
+# RUN: llvm-readobj -r %t.o | FileCheck %s
+# RUN: ld.lld %t.o -o %t
+
+# CHECK: .rela.debug_info {
+# CHECK-NEXT: 0x6 R_AARCH64_ABS32 .debug_abbrev 0x0
+# CHECK-NEXT: 0xD R_AARCH64_TLS_DTPREL64 var 0x0
+# CHECK-NEXT: }
+
+.section .tdata,"awT",@progbits
+.globl var
+var:
+ .word 0
+
+.section .debug_abbrev,"",@progbits
+.byte 1 // Abbreviation Code
+.byte 17 // DW_TAG_compile_unit
+.byte 1 // DW_CHILDREN_yes
+.byte 0 // EOM(1)
+.byte 0 // EOM(2)
+
+.byte 2 // Abbreviation Code
+.byte 52 // DW_TAG_variable
+.byte 0 // DW_CHILDREN_no
+.byte 2; // DW_AT_location
+.byte 24 // DW_FORM_exprloc
+.byte 0 // EOM(1)
+.byte 0 // EOM(2)
+
+.section .debug_info,"",@progbits
+.Lcu_begin0:
+ .word .Lcu_end - .Lcu_body // Length of Unit
+.Lcu_body:
+ .hword 4 // DWARF version number
+ .word .debug_abbrev // Offset Into Abbrev. Section
+ .byte 8 // Address Size (in bytes)
+ .byte 1 // Abbrev [1] DW_TAG_compile_unit
+ .byte 2 // Abbrev [2] DW_TAG_variable
+ .xword %dtprel(var)
+.Lcu_end:
|
|
@llvm/pr-subscribers-lld Author: Shivam Gupta (xgupta) ChangesWith PR#155776, clang started to emit R_AARCH64_TLS_DTPREL64 in .debug_info section to help in debugging TLS variables. This patch helps recognise that relocation in LLD linker. Full diff: https://github.com/llvm/llvm-project/pull/183962.diff 2 Files Affected:
diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index f85a3f48f2183..c329e653b9853 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -156,6 +156,8 @@ RelExpr AArch64::getRelExpr(RelType type, const Symbol &s,
case R_AARCH64_PREL32:
case R_AARCH64_PREL64:
return R_PC;
+ case R_AARCH64_TLS_DTPREL64:
+ return R_DTPREL;
case R_AARCH64_NONE:
return R_NONE;
default:
@@ -649,6 +651,10 @@ void AArch64::relocate(uint8_t *loc, const Relocation &rel,
checkInt(ctx, loc, val, 32, rel);
write32(ctx, loc, val);
break;
+ case R_AARCH64_TLS_DTPREL64:
+ checkInt(ctx, loc, val, 64, rel);
+ write64(ctx, loc, val);
+ break;
case R_AARCH64_ADD_ABS_LO12_NC:
case R_AARCH64_AUTH_GOT_ADD_LO12_NC:
write32Imm12(loc, val);
diff --git a/lld/test/ELF/aarch64-tls-dtprel.s b/lld/test/ELF/aarch64-tls-dtprel.s
new file mode 100644
index 0000000000000..49c1ac1456354
--- /dev/null
+++ b/lld/test/ELF/aarch64-tls-dtprel.s
@@ -0,0 +1,41 @@
+# REQUIRES: aarch64
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu %s -o %t.o
+# RUN: llvm-readobj -r %t.o | FileCheck %s
+# RUN: ld.lld %t.o -o %t
+
+# CHECK: .rela.debug_info {
+# CHECK-NEXT: 0x6 R_AARCH64_ABS32 .debug_abbrev 0x0
+# CHECK-NEXT: 0xD R_AARCH64_TLS_DTPREL64 var 0x0
+# CHECK-NEXT: }
+
+.section .tdata,"awT",@progbits
+.globl var
+var:
+ .word 0
+
+.section .debug_abbrev,"",@progbits
+.byte 1 // Abbreviation Code
+.byte 17 // DW_TAG_compile_unit
+.byte 1 // DW_CHILDREN_yes
+.byte 0 // EOM(1)
+.byte 0 // EOM(2)
+
+.byte 2 // Abbreviation Code
+.byte 52 // DW_TAG_variable
+.byte 0 // DW_CHILDREN_no
+.byte 2; // DW_AT_location
+.byte 24 // DW_FORM_exprloc
+.byte 0 // EOM(1)
+.byte 0 // EOM(2)
+
+.section .debug_info,"",@progbits
+.Lcu_begin0:
+ .word .Lcu_end - .Lcu_body // Length of Unit
+.Lcu_body:
+ .hword 4 // DWARF version number
+ .word .debug_abbrev // Offset Into Abbrev. Section
+ .byte 8 // Address Size (in bytes)
+ .byte 1 // Abbrev [1] DW_TAG_compile_unit
+ .byte 2 // Abbrev [2] DW_TAG_variable
+ .xword %dtprel(var)
+.Lcu_end:
|
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
MaskRay
left a comment
MaskRay
left a comment
There was a problem hiding this comment.
Needs to wait for the assembler %dtprel() to land first
|
I'm curious about this being pushed forward despite #146572 (comment) |
@jrtc27 |
|
Okay, we will first try for lldb, why it is necessary to have this relocation. Without this relocation in clang for test case llvm-dwarfdump shows So it is missing the DW_AT_location attribute. New change in clang emitted it. Which that help, LLDB debug the variable. Without the location even after LLDB fix in #183993, LLDB could not use symbol table. |
|
Also at the end of https://sourceware.org/bugzilla/show_bug.cgi?id=27886, Tom de Vries prove that only symbol table approach not work for static TLS variables, it need DW_AT_location. |
|
If it helps the original ABI issue ARM-software/abi-aa#176 took inspiration from x86_64 which emits R_X86_64_DTPOFF64 against a DWARF location Our GNU community recieved a request for something similar (assembler expression) https://sourceware.org/bugzilla/show_bug.cgi?id=28351 which claims as a pre-requisite for GDB support. I tried to see if I could examine the location of a TLS variable on an AArch64 in qemu-user mode without DW_AT_location (works on x86), but it seems like qemu user mode doesn't implement enough of the remote protocol. If I get time this evening I'll try on hardware. |
|
Thanks @smithp35 for testing this. Let me know if you have been succeeded in examine the location of TLS variable without DW_AT_location. |
Apologies for the delay. Happen to be working from home today so I've been able to check on a Raspberry Pi. I can reproduce the behaviour in https://sourceware.org/bugzilla/show_bug.cgi?id=28351 and on my own examples. The There may be an advantage in clang not using the Dwarf expression for preemptible global TLS variables. I note that the definition of the |
No worries, thank you very much for taking time to check on your system.
So I have updated the corresponding clang patch to conditionally emit DW_OP_form_tls_address(with DW_AT_location). I am able to debug it with gdb but it seems something is missing on lldb side so it is not able to debug without DW_AT_location. But I hope that should not block clang and lld patches which are doing things according to ABI. |
Is this gdb with no further patches? |
Yes with the test case given at end of https://sourceware.org/bugzilla/show_bug.cgi?id=27886. gdb and g++ and now with gdb and clang++(updated with DW_AT_location) I have which was before printing in bug report as With lldb and clang++(update) I still have same With preemptible global variable v(without DW_AT_location) - $ cat lib.cpp With lldb and clang++ with gdb and clang++ |
|
Thanks. |
Hi @smithp35 can you please share the example when it gets wrong, someone object to adding the check that forbid emitting DW_AT_location on clang patch. |
|
I'm assuming that "going wrong" is what happens when the symbol is pre-empted. You will need to run this on x86_64 or some other target that uses the
Note that the value of x is 20, but the debugger prints 0xa (10) when we ask for the value of the variable. By the definition of I found that gdb for AArch64 without It may that various debuggers can't handle the mix of Thinking about this from first principles pre-emptible symbol isn't ideal either, if we were to be precise it would be "symbol is in the dynamic symbol table", and that is very difficult for the compiler to know. In the example above the symbol x in tlsmain.c would get exported into the dynamic symbol table because tlslib.so also defines it (and it may be pre-empted by it). If I add another global I think this points to either always using |
|
Thanks @smithp35 for clearing the things. I feel could keep preemptible check(also Aarch64) and let debugger handle that case with dynamic symbol table. GDB is doing it so LLDB can also do it in future. Or maybe as in #146572 (comment) it is being point out that it is not a very common user case to override TLS variables so emit in all cases and let debuggers handle it in whichever way it can/want. WDTY? Which approach to choose. (And sorry it created a noise when this discussion should be on LLVM side patch.) |
|
Perhaps a stupid question: why does preempting normal globals not hit the same issue with debug info? |
I think in this case it comes from how the
I'm no expert in that area so there's probably a better answer to be found. |
Given that there are cases when the compiler can't know at compile time if the linker will put a symbol in the dynamic symbol table (exporting symbols from executables only when shared-libraries reference them), I think it would be best to keep it simple and always use the |
This is out of order. The lld patch must be merged before Clang begins emitting relocations that would trigger an error. Similarly, ensure GNU ld supports R_AARCH64_TLS_DTPREL64 before updating Clang. Clang and lld versions are usually synchronized, but we need to support very old GNU ld. Once GNU ld support is upstreamed we could gate this emission under -fbinutils-version. However, I’m still concerned about the Clang change; can the debugger use |
It can, but this is sub-optimal. Some thread-local variables do not have entries in the symbol table, like ones inside functions. Others may have entries with the same symbol names, for example, when similar variables are declared in anonymous namespaces in different files. Only the compiler, together with the linker, can provide accurate location information. |
Do you have an example that a thread-locaal variable doesn't have a symbol table entry? TLS relocations cannot be adjusted to be against the section symbols, so all STT_TLS symbols are retained. int foo() {
thread_local int x = 0;
return ++x;
}
[[gnu::noinline]] static int loc() {
thread_local int x = 0;
return ++x;
}
int use_loc() { return loc(); }While an executable can have multiple local symbols of the same name, local symbols are grouped starting with a STT_FILE symbol, which could be used to resolve ambiguity.
Internal linkage TLS variables still lower to local symbols. There is no lost information. Perhaps inaccurate location information happens with struct member access ( |
From looking up what GDB does, there seems to be a difference in how variables that are described as Not entirely sure why this is, possibly because only global symbols are guaranteed to have unique names in the ELF files symbol table, whereas the debugger wouldn't necessarily have enough information to find the object file. |
You are right, they have symtab entries. However, as compilers often do not generate the
Knowing the linkage name can be beneficial for global variables that can be interposed. But for the basic scenario, when the debugger simply needs to find the location of the variable, it is overcomplicated. The symtab entry contains the offset of the variable in the module's TLS data, and exactly the same value is stored in the |
Your question is absolutely legit. > cat app.c
int var = 11;
int get_dso_var();
int main() {
int var_from_dso = get_dso_var();
}
> cat lib.c
int var = 22;
int get_dso_var() { return var; }
> clang -g -O0 -fpic -c app.c -o app.o
> clang -g -O0 -fpic -c lib.c -o lib.o
> clang -shared lib.o -o lib.so
> clang app.o ./lib.so -o app.out
> lldb -v
lldb version 21.1.8
> lldb app.out
(lldb) br set -n get_dso_var
(lldb) r
-> 2 int get_dso_var() { return var; }
(lldb) p var
(int) 22
(lldb) n
-> 4 int var_from_dso = get_dso_var();
(lldb) p var
(int) 11
(lldb) q
> gdb -v
GNU gdb (Debian 17.1-3) 17.1
> gdb app.out
(gdb) br get_dso_var
(gdb) r
Breakpoint 1, get_dso_var () at ../lib.c:2
2 int get_dso_var() { return var; }
(gdb) p var
$1 = 11
(gdb) n
main () at ../app.c:5
5 }
(gdb) p var
$2 = 11
(gdb) q |
Thanks for the explanation! |
Updated the PR description and also sent a GNU patch(with gas, ld and gold) -https://sourceware.org/pipermail/binutils/2026-March/148550.html. |
With PR#155776, clang started to emit R_AARCH64_TLS_DTPREL64 in .debug_info section to help in debugging TLS variables. This patch help recognise that relocation in LLD linker.
|
Hi @MaskRay, I rebased the branch since Assembler side of patch is committed. Had a question, does this feature require GNU support first? Or it can be submitted since the user have to pass clang I observed from archive GNU/binutils patches takes longer time to get reviewed and commit... |
|
Gentle Ping! |
|
Gentle Ping! Can we merge this now? So other two PRs(llc and llvm-dwarfdump) can be merge. llvm-mc support already merged. |
|
@xgupta Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/21454 Here is the relevant piece of the build log for the reference |
|
Thanks a lot while working on this. While this isn't fixing a regression, I think a backport to 22.1 is warranted, the aarch64 port is useless without it for a class of programs that use thread-local storage heavily. |
|
/cherry-pick 14ce208 |
|
Failed to cherry-pick: 14ce208 https://github.com/llvm/llvm-project/actions/runs/23795326630 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
…lvm#183962) Clang plan to emit R_AARCH64_TLS_DTPREL64 in .debug_info (see PR This prevent the debugger from correctly locating TLS variables when using the DWARF DW_OP_GNU_push_tls_address or DW_AT_location with DTPREL offsets. This patch adds support for R_AARCH64_TLS_DTPREL64, adds its mapping to R_DTPREL.
This patch allows R_AARCH64_TLS_DTPREL64 relocations in non-allocated sections, which is required for DWARF debug information when using Thread Local Storage. This matches the behavior in LLD. Also a new syntax to parse dtprel operator use to describe tls location in debug information. Please see the reference 3 below. References: - llvm/llvm-project#146572 [AArch64] Support TLS variables in debug info - llvm/llvm-project#183962 [LLD][AArch64] Handle R_AARCH64_TLS_DTPREL64 in non-alloc sections - ARM-software/abi-aa#330 [AAELF64] Allow R_AARCH64_TLS_DTPREL to be used statically. bfd/ * elfnn-aarch64.c (elfNN_aarch64_final_link_relocate): Handle BFD_RELOC_AARCH64_TLS_DTPREL. gas/ * config/tc-aarch64.c (s_aarch64_cons): Parse %dtprel(var) syntax. * testsuite/gas/aarch64/tls-debug.s: New test. * testsuite/gas/aarch64/tls-debug.d: Run the test. ld/ * testsuite/ld-aarch64/tls-debug.s: New test. * testsuite/ld-aarch64/tls-debug.d: Run the test. Bug: https://sourceware.org/PR28351 Signed-off-by: Shivam Gupta <shivam98.tkg@gmail.com>
…lvm#183962) Clang plan to emit R_AARCH64_TLS_DTPREL64 in .debug_info (see PR llvm#146572). LLD currently fails to recognize this relocation. This prevent the debugger from correctly locating TLS variables when using the DWARF DW_OP_GNU_push_tls_address or DW_AT_location with DTPREL offsets. This patch adds support for R_AARCH64_TLS_DTPREL64, adds its mapping to R_DTPREL.
8 participants
Clang plan to emit R_AARCH64_TLS_DTPREL64 in .debug_info (see PR #146572). LLD currently fails to recognize this relocation.
This prevent the debugger from correctly locating TLS variables when using the DWARF DW_OP_GNU_push_tls_address or DW_AT_location with DTPREL offsets.
This patch adds support for R_AARCH64_TLS_DTPREL64, adds its mapping to R_DTPREL.