[lldb] [debugserver] Use "full" x86_64 GPR state when available.

[mrpippy]

macOS 10.15 added a "full" x86_64 GPR thread state flavor, equivalent to the normal one but with DS, ES, SS, and GSbase added. This flavor can only be used with processes that install a custom LDT (functionality that was also added in 10.15 and is used by apps like Wine to execute 32-bit code).

Along with allowing DS, ES, SS, and GSbase to be viewed/modified, using the full flavor is necessary when debugging a thread executing 32-bit code.
If thread_set_state() is used with the regular thread state flavor, the kernel resets CS to the 64-bit code segment (see set_thread_state64(), which makes debugging impossible.

There's no way to detect whether the full flavor is available, try to use it and fall back to the regular one if it's not available.

A downside is that this patch exposes the DS, ES, SS, and GSbase registers for all x86_64 processes, even though they are not populated unless the full thread state is available.
I'm not sure if there's a way to tell LLDB that a register is unavailable. The classic GDB g command allows returning x to denote unavailable registers, but it seems like the debug server uses newer commands like jThreadsInfo and I'm not sure if those have the same support.

Fixes #57591
(also filed as Apple FB11464104)

@jasonmolenda

[github-actions]

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 @ followed by their GitHub username.

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.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Brendan Shanks (mrpippy)

Changes

macOS 10.15 added a "full" x86_64 GPR thread state flavor, equivalent to the normal one but with DS, ES, SS, and GSbase added. This flavor can only be used with processes that install a custom LDT (functionality that was also added in 10.15 and is used by apps like Wine to execute 32-bit code).

Along with allowing DS, ES, SS, and GSbase to be viewed/modified, using the full flavor is necessary when debugging a thread executing 32-bit code.
If thread_set_state() is used with the regular thread state flavor, the kernel resets CS to the 64-bit code segment (see set_thread_state64(), which makes debugging impossible.

There's no way to detect whether the full flavor is available, try to use it and fall back to the regular one if it's not available.

A downside is that this patch exposes the DS, ES, SS, and GSbase registers for all x86_64 processes, even though they are not populated unless the full thread state is available.
I'm not sure if there's a way to tell LLDB that a register is unavailable. The classic GDB g command allows returning x to denote unavailable registers, but it seems like the debug server uses newer commands like jThreadsInfo and I'm not sure if those have the same support.

Fixes #57591
(also filed as Apple FB11464104)

@jasonmolenda

---

Full diff: https://github.com/llvm/llvm-project/pull/108663.diff

3 Files Affected:

• (modified) lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp (+43-9)
• (modified) lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.h (+3-1)
• (modified) lldb/tools/debugserver/source/MacOSX/x86_64/MachRegisterStatesX86_64.h (+5)

diff --git a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
index 5a62e2a8d12e2c..286fd72267b349 100644
--- a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -182,22 +182,39 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
     m_state.context.gpr.__gs = ('g' << 8) + 's';
     m_state.SetError(e_regSetGPR, Read, 0);
 #else
-    mach_msg_type_number_t count = e_regSetWordSizeGPR;
+    mach_msg_type_number_t count = e_regSetWordSizeGPRFull;
+    int flavor = __x86_64_THREAD_FULL_STATE;
     m_state.SetError(
         e_regSetGPR, Read,
-        ::thread_get_state(m_thread->MachPortNumber(), __x86_64_THREAD_STATE,
+        ::thread_get_state(m_thread->MachPortNumber(), flavor,
                            (thread_state_t)&m_state.context.gpr, &count));
+
+    if (!m_state.GetError(e_regSetGPR, Read)) {
+      m_state.hasFullGPRState = true;
+    } else {
+      m_state.hasFullGPRState = false;
+      count = e_regSetWordSizeGPR;
+      flavor = __x86_64_THREAD_STATE;
+      m_state.SetError(
+          e_regSetGPR, Read,
+          ::thread_get_state(m_thread->MachPortNumber(), flavor,
+                             (thread_state_t)&m_state.context.gpr, &count));
+    }
     DNBLogThreadedIf(
         LOG_THREAD,
-        "::thread_get_state (0x%4.4x, %u, &gpr, %u) => 0x%8.8x"
+        "::thread_get_state (0x%4.4x, %u (%s), &gpr, %u) => 0x%8.8x"
         "\n\trax = %16.16llx rbx = %16.16llx rcx = %16.16llx rdx = %16.16llx"
         "\n\trdi = %16.16llx rsi = %16.16llx rbp = %16.16llx rsp = %16.16llx"
         "\n\t r8 = %16.16llx  r9 = %16.16llx r10 = %16.16llx r11 = %16.16llx"
         "\n\tr12 = %16.16llx r13 = %16.16llx r14 = %16.16llx r15 = %16.16llx"
         "\n\trip = %16.16llx"
-        "\n\tflg = %16.16llx  cs = %16.16llx  fs = %16.16llx  gs = %16.16llx",
-        m_thread->MachPortNumber(), x86_THREAD_STATE64,
-        x86_THREAD_STATE64_COUNT, m_state.GetError(e_regSetGPR, Read),
+        "\n\tflg = %16.16llx  cs = %16.16llx  fs = %16.16llx  gs = %16.16llx"
+        "\n\t ds = %16.16llx  es = %16.16llx  ss = %16.16llx gsB = %16.16llx",
+        m_thread->MachPortNumber(), flavor,
+        m_state.hasFullGPRState ? "full" : "non-full",
+        m_state.hasFullGPRState ? __x86_64_THREAD_FULL_STATE
+                                : __x86_64_THREAD_STATE,
+        m_state.GetError(e_regSetGPR, Read),
         m_state.context.gpr.__rax, m_state.context.gpr.__rbx,
         m_state.context.gpr.__rcx, m_state.context.gpr.__rdx,
         m_state.context.gpr.__rdi, m_state.context.gpr.__rsi,
@@ -208,7 +225,9 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
         m_state.context.gpr.__r14, m_state.context.gpr.__r15,
         m_state.context.gpr.__rip, m_state.context.gpr.__rflags,
         m_state.context.gpr.__cs, m_state.context.gpr.__fs,
-        m_state.context.gpr.__gs);
+        m_state.context.gpr.__gs, m_state.context.gpr.__ds,
+        m_state.context.gpr.__es, m_state.context.gpr.__ss,
+        m_state.context.gpr.__gsbase );
 
 //      DNBLogThreadedIf (LOG_THREAD, "thread_get_state(0x%4.4x, %u, &gpr, %u)
 //      => 0x%8.8x"
@@ -461,9 +480,11 @@ kern_return_t DNBArchImplX86_64::SetGPRState() {
 
   m_state.SetError(e_regSetGPR, Write,
                    ::thread_set_state(m_thread->MachPortNumber(),
-                                      __x86_64_THREAD_STATE,
+                                      m_state.hasFullGPRState ? __x86_64_THREAD_FULL_STATE
+                                                              : __x86_64_THREAD_STATE,
                                       (thread_state_t)&m_state.context.gpr,
-                                      e_regSetWordSizeGPR));
+                                      m_state.hasFullGPRState ? e_regSetWordSizeGPRFull
+                                                              : e_regSetWordSizeGPR));
   DNBLogThreadedIf(
       LOG_THREAD,
       "::thread_set_state (0x%4.4x, %u, &gpr, %u) => 0x%8.8x"
@@ -1157,6 +1178,10 @@ enum {
   gpr_cs,
   gpr_fs,
   gpr_gs,
+  gpr_ds,
+  gpr_es,
+  gpr_ss,
+  gpr_gsbase,
   gpr_eax,
   gpr_ebx,
   gpr_ecx,
@@ -1543,6 +1568,7 @@ enum debugserver_regnums {
   debugserver_k5 = 123,
   debugserver_k6 = 124,
   debugserver_k7 = 125,
+  debugserver_gsbase = 126,
 };
 
 #define GPR_OFFSET(reg) (offsetof(DNBArchImplX86_64::GPR, __##reg))
@@ -1690,6 +1716,10 @@ const DNBRegisterInfo DNBArchImplX86_64::g_gpr_registers[] = {
     DEFINE_GPR_ALT2(cs, NULL),
     DEFINE_GPR_ALT2(fs, NULL),
     DEFINE_GPR_ALT2(gs, NULL),
+    DEFINE_GPR_ALT2(ds, NULL),
+    DEFINE_GPR_ALT2(es, NULL),
+    DEFINE_GPR_ALT2(ss, NULL),
+    DEFINE_GPR_ALT2(gsbase, NULL),
     DEFINE_GPR_PSEUDO_32(eax, rax),
     DEFINE_GPR_PSEUDO_32(ebx, rbx),
     DEFINE_GPR_PSEUDO_32(ecx, rcx),
@@ -2313,6 +2343,8 @@ bool DNBArchImplX86_64::GetRegisterValue(uint32_t set, uint32_t reg,
     value->info = *regInfo;
     switch (set) {
     case e_regSetGPR:
+      if (reg > gpr_gs && !m_state.hasFullGPRState)
+        return false;
       if (reg < k_num_gpr_registers) {
         value->value.uint64 = ((uint64_t *)(&m_state.context.gpr))[reg];
         return true;
@@ -2524,6 +2556,8 @@ bool DNBArchImplX86_64::SetRegisterValue(uint32_t set, uint32_t reg,
   if (regInfo) {
     switch (set) {
     case e_regSetGPR:
+      if (reg > gpr_gs && !m_state.hasFullGPRState)
+        return false;
       if (reg < k_num_gpr_registers) {
         ((uint64_t *)(&m_state.context.gpr))[reg] = value->value.uint64;
         success = true;
diff --git a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.h b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.h
index 96da02a4c9ff9f..7fffd60b2064e0 100644
--- a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.h
+++ b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.h
@@ -103,7 +103,8 @@ class DNBArchImplX86_64 : public DNBArchProtocol {
   };
 
   enum RegisterSetWordSize {
-    e_regSetWordSizeGPR = sizeof(GPR) / sizeof(int),
+    e_regSetWordSizeGPR = (sizeof(GPR) - 32) / sizeof(int),
+    e_regSetWordSizeGPRFull = sizeof(GPR) / sizeof(int),
     e_regSetWordSizeFPU = sizeof(FPU) / sizeof(int),
     e_regSetWordSizeEXC = sizeof(EXC) / sizeof(int),
     e_regSetWordSizeAVX = sizeof(AVX) / sizeof(int),
@@ -130,6 +131,7 @@ class DNBArchImplX86_64 : public DNBArchProtocol {
     kern_return_t fpu_errs[2]; // Read/Write errors
     kern_return_t exc_errs[2]; // Read/Write errors
     kern_return_t dbg_errs[2]; // Read/Write errors
+    bool hasFullGPRState;
 
     State() {
       uint32_t i;
diff --git a/lldb/tools/debugserver/source/MacOSX/x86_64/MachRegisterStatesX86_64.h b/lldb/tools/debugserver/source/MacOSX/x86_64/MachRegisterStatesX86_64.h
index b566accd397285..743c665b691067 100644
--- a/lldb/tools/debugserver/source/MacOSX/x86_64/MachRegisterStatesX86_64.h
+++ b/lldb/tools/debugserver/source/MacOSX/x86_64/MachRegisterStatesX86_64.h
@@ -22,6 +22,7 @@
 #define __x86_64_DEBUG_STATE 11
 #define __x86_64_AVX_STATE 17
 #define __x86_64_AVX512F_STATE 20
+#define __x86_64_THREAD_FULL_STATE 23
 
 typedef struct {
   uint64_t __rax;
@@ -45,6 +46,10 @@ typedef struct {
   uint64_t __cs;
   uint64_t __fs;
   uint64_t __gs;
+  uint64_t __ds;
+  uint64_t __es;
+  uint64_t __ss;
+  uint64_t __gsbase;
 } __x86_64_thread_state_t;
 
 typedef struct {

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff b74e7792194d9a8a9ef32c7dc1ffcd205b299336 a9b19cbd20083e2b1d6c8de342206cb0ec6183b7 --extensions h,cpp -- lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.h lldb/tools/debugserver/source/MacOSX/x86_64/MachRegisterStatesX86_64.h

View the diff from clang-format here.

diff --git a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
index 3b3f1f02a2..106514f7cf 100644
--- a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -184,10 +184,10 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
 #else
     mach_msg_type_number_t count = e_regSetWordSizeGPRFull;
     int flavor = __x86_64_THREAD_FULL_STATE;
-    m_state.SetError(
-        e_regSetGPR, Read,
-        ::thread_get_state(m_thread->MachPortNumber(), flavor,
-                           (thread_state_t)&m_state.context.gpr, &count));
+    m_state.SetError(e_regSetGPR, Read,
+                     ::thread_get_state(m_thread->MachPortNumber(), flavor,
+                                        (thread_state_t)&m_state.context.gpr,
+                                        &count));
 
     if (!m_state.GetError(e_regSetGPR, Read)) {
       m_state.hasFullGPRState = true;
@@ -195,10 +195,10 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
       m_state.hasFullGPRState = false;
       count = e_regSetWordSizeGPR;
       flavor = __x86_64_THREAD_STATE;
-      m_state.SetError(
-          e_regSetGPR, Read,
-          ::thread_get_state(m_thread->MachPortNumber(), flavor,
-                             (thread_state_t)&m_state.context.gpr, &count));
+      m_state.SetError(e_regSetGPR, Read,
+                       ::thread_get_state(m_thread->MachPortNumber(), flavor,
+                                          (thread_state_t)&m_state.context.gpr,
+                                          &count));
     }
     DNBLogThreadedIf(
         LOG_THREAD,
@@ -212,22 +212,20 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
         "\n\t ds = %16.16llx  es = %16.16llx  ss = %16.16llx gsB = %16.16llx",
         m_thread->MachPortNumber(), flavor,
         m_state.hasFullGPRState ? "full" : "non-full",
-        m_state.hasFullGPRState ? e_regSetWordSizeGPRFull
-                                : e_regSetWordSizeGPR,
-        m_state.GetError(e_regSetGPR, Read),
-        m_state.context.gpr.__rax, m_state.context.gpr.__rbx,
-        m_state.context.gpr.__rcx, m_state.context.gpr.__rdx,
-        m_state.context.gpr.__rdi, m_state.context.gpr.__rsi,
-        m_state.context.gpr.__rbp, m_state.context.gpr.__rsp,
-        m_state.context.gpr.__r8, m_state.context.gpr.__r9,
-        m_state.context.gpr.__r10, m_state.context.gpr.__r11,
-        m_state.context.gpr.__r12, m_state.context.gpr.__r13,
-        m_state.context.gpr.__r14, m_state.context.gpr.__r15,
-        m_state.context.gpr.__rip, m_state.context.gpr.__rflags,
-        m_state.context.gpr.__cs, m_state.context.gpr.__fs,
-        m_state.context.gpr.__gs, m_state.context.gpr.__ds,
-        m_state.context.gpr.__es, m_state.context.gpr.__ss,
-        m_state.context.gpr.__gsbase );
+        m_state.hasFullGPRState ? e_regSetWordSizeGPRFull : e_regSetWordSizeGPR,
+        m_state.GetError(e_regSetGPR, Read), m_state.context.gpr.__rax,
+        m_state.context.gpr.__rbx, m_state.context.gpr.__rcx,
+        m_state.context.gpr.__rdx, m_state.context.gpr.__rdi,
+        m_state.context.gpr.__rsi, m_state.context.gpr.__rbp,
+        m_state.context.gpr.__rsp, m_state.context.gpr.__r8,
+        m_state.context.gpr.__r9, m_state.context.gpr.__r10,
+        m_state.context.gpr.__r11, m_state.context.gpr.__r12,
+        m_state.context.gpr.__r13, m_state.context.gpr.__r14,
+        m_state.context.gpr.__r15, m_state.context.gpr.__rip,
+        m_state.context.gpr.__rflags, m_state.context.gpr.__cs,
+        m_state.context.gpr.__fs, m_state.context.gpr.__gs,
+        m_state.context.gpr.__ds, m_state.context.gpr.__es,
+        m_state.context.gpr.__ss, m_state.context.gpr.__gsbase);
 
 //      DNBLogThreadedIf (LOG_THREAD, "thread_get_state(0x%4.4x, %u, &gpr, %u)
 //      => 0x%8.8x"

[jasonmolenda]

Nice improvement. Let me apply it and experiment a tiny bit next week, but a quick read looks good.

[jasonmolenda]

(I hadn't noticed the kernel added this new flavor for intel machines, I've had people ask for access to these before.)

[jasonmolenda]

Thanks so much for putting together the patch for this. Yeah enabling these registers only when the thread flavor is available is not that simple, and I don't think the consequence of including them with zero values in all cases is bad one. Most general users will not be aware of what these registers indicate and ignore the 0's, and those people who are working on code where they are important, will see values for them, it seems fine to me. I played around on an intel mac and the only side effect was that you'll get an error message if you try to modify one of the new registers when the thread state is not supported -- not a big surprise. (DNBArchImplX86_64::SetRegisterValue will return an error if the full_state flavor was unavailable earlier in the debug session)

Do you have the permissions to merge this PR yourself? I can do it if not.

[mrpippy]

Great, no I don't think I have permissions to merge.

I pushed a new version that fixed a typo in the GetGPRState log call, and in SetGPRState adds the new fields to the log call.

[github-actions]

@mrpippy 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!