-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LLDB][AArch64] Make TPIDR a generic tp register #154444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) ChangesUnlike x86, ARM doesn't support a generic thread pointer for TLS data, so things like Don't work, and you need to specify tpidr. This works, especially because that's the name GDB uses. But for ease of use, and at the request of @aperez I've made it so we can reference it via I personally don't have an aarch machine, and all the arm examples in Full diff: https://github.com/llvm/llvm-project/pull/154444.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
index fbf128553fd5e..d29af3c568dd7 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -79,7 +79,16 @@ static lldb_private::RegisterInfo g_register_infos_mte[] = {
DEFINE_EXTENSION_REG(mte_ctrl)};
static lldb_private::RegisterInfo g_register_infos_tls[] = {
- DEFINE_EXTENSION_REG(tpidr),
+ {"tpidr",
+ nullptr,
+ 8,
+ 0,
+ lldb::eEncodingUint,
+ lldb::eFormatHex,
+ {LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_REGNUM_GENERIC_TP},
+ nullptr,
+ nullptr,
+ nullptr},
// Only present when SME is present
DEFINE_EXTENSION_REG(tpidr2)};
diff --git a/lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py b/lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
index ec8eb1c05dfb8..09c2cca18f9c0 100644
--- a/lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ b/lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -93,6 +93,8 @@ def check_tls_reg(self, registers):
for register in registers:
self.expect("p {}_was_set".format(register), substrs=["true"])
+ self.expect("reg read tp", substrs=[hex(set_values["tpidr"])])
+
@skipUnlessArch("aarch64")
@skipUnlessPlatform(["linux"])
def test_tls_no_sme(self):
@@ -100,6 +102,7 @@ def test_tls_no_sme(self):
self.skipTest("SME must not be present.")
self.check_tls_reg(["tpidr"])
+
@skipUnlessArch("aarch64")
@skipUnlessPlatform(["linux"])
|
789dd0d to
77689bd
Compare
77689bd to
84f48ee
Compare
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
Outdated
Show resolved
Hide resolved
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
35a9f53 to
1292a0d
Compare
|
Please title this AArch64 or Arm64 instead of ARM. I know that's our fault on the branding side but in this case, TLS is very different between 32-bit Arm sometimes aka "ARM" and AArch64 aka the 64-bit mode of Armv8/Armv9. Not that it has to matter to you but to me seeing ARM makes me think 32-bit. |
DavidSpickett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't make TLS variables work (#71666), but I assume you're aware of that. Someone was working on this problem recently but I don't know how far they got.
Making the generic name available for reading is a good idea regardless.
lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
Outdated
Show resolved
Hide resolved
|
@DavidSpickett Please take a second look when you get a chance |
DavidSpickett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Remove the @ mention in the description before merging, so that they don't get spammed when this gets merged into forks.
|
Thanks @DavidSpickett! |
Unlike x86, ARM doesn't support a generic thread pointer for TLS data, so things like
Don't work, and you need to specify tpidr. This works, especially because that's the name GDB uses. But for ease of use, and at the request of aperez I've made it so we can reference it via
tp.I personally don't have an aarch machine, and all the arm examples in
Shell/Register/Coreare freebsd and don't contain tpidr, so I was unable to add a shell test for this. I added a test to the AARCH register tests, but without an Aarch machine I'm hoping these work.