-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
[lldb][AArch64] Do not crash if NT_ARM_TLS is missing #106478
[lldb][AArch64] Do not crash if NT_ARM_TLS is missing #106478
Conversation
@llvm/pr-subscribers-lldb Author: Igor Kudrin (igorkudrin) ChangesD156118 states that this note is always present, but it is better to check it explicitly, as otherwise Full diff: https://github.com/llvm/llvm-project/pull/106478.diff 3 Files Affected:
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
index 054b7d9b2ec575..8ebdd214458c45 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -254,9 +254,10 @@ RegisterInfoPOSIX_arm64::RegisterInfoPOSIX_arm64(
if (m_opt_regsets.AllSet(eRegsetMaskMTE))
AddRegSetMTE();
- // The TLS set always contains tpidr but only has tpidr2 when SME is
- // present.
- AddRegSetTLS(m_opt_regsets.AllSet(eRegsetMaskSSVE));
+ if (m_opt_regsets.AllSet(eRegsetMaskTLS))
+ // The TLS set always contains tpidr but only has tpidr2 when SME is
+ // present.
+ AddRegSetTLS(m_opt_regsets.AllSet(eRegsetMaskSSVE));
if (m_opt_regsets.AnySet(eRegsetMaskSSVE))
AddRegSetSME(m_opt_regsets.AnySet(eRegsetMaskZT));
diff --git a/lldb/test/Shell/Process/elf-core/aarch64-no-NT_ARM_TLS.yaml b/lldb/test/Shell/Process/elf-core/aarch64-no-NT_ARM_TLS.yaml
new file mode 100644
index 00000000000000..bd8ce225c9e071
--- /dev/null
+++ b/lldb/test/Shell/Process/elf-core/aarch64-no-NT_ARM_TLS.yaml
@@ -0,0 +1,34 @@
+# REQUIRES: aarch64
+
+## Check that lldb does not crash if a core file does not contain an NT_ARM_TLS
+## note while there are notes for other dynamic register sets.
+
+# RUN: yaml2obj %s -o %t
+# RUN: %lldb -c %t -o "re r -a" | FileCheck %s
+
+# CHECK: Pointer Authentication Registers:
+# CHECK-NEXT: data_mask =
+# CHECK-NEXT: code_mask =
+# CHECK-NOT: Thread Local Storage Registers:
+
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_CORE
+ Machine: EM_AARCH64
+ProgramHeaders:
+ - Type: PT_NOTE
+ FirstSec: .note
+ LastSec: .note
+Sections:
+ - Name: .note
+ Type: SHT_NOTE
+ Notes:
+ - Name: CORE
+ Desc: 0b00000000000000000000000b00000000000000000000000000000000000000389300001b930000389300001b930000000000000000000000000000000000000000000000000000e02e0000000000000000000000000000119100000000000000000000000000005ee100000000000000000000000000002f00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000400162e2ffff00005801400000000000200162e2ffff0000240140000000000000000000000000000100000000000000
+ Type: NT_PRSTATUS
+ - Name: LINUX
+ Desc: 0000000000007f000000000000007f00
+ Type: NT_ARM_PAC_MASK
+...
diff --git a/lldb/test/Shell/Process/elf-core/lit.local.cfg b/lldb/test/Shell/Process/elf-core/lit.local.cfg
new file mode 100644
index 00000000000000..8169b9f95e118c
--- /dev/null
+++ b/lldb/test/Shell/Process/elf-core/lit.local.cfg
@@ -0,0 +1 @@
+config.suffixes = ['.yaml']
|
[D156118](https://reviews.llvm.org/D156118) states that this note is always present, but it is better to check it explicitly, as otherwise `lldb` may crash when trying to read registers.
e588c3c
to
6854730
Compare
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
Outdated
Show resolved
Hide resolved
Interested in how you ended up with a file without it, perhaps there was some era of kernels that produces that result. But given we do a check whether to set the regset bit, we should use the result of that later anyway, so this looks ok to me. |
And "don't crash on invalid input" is a good enough reason to fix this regardless of how the core file was generated, even if the core file is corrupted. |
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, thanks!
D156118 states that this note is always present, but it is better to check it explicitly, as otherwise
lldb
may crash when trying to read registers.