[LLDB][Minidump] Extend the minidump x86_64 registers to include fs_base and gs_base

[Jlalond]

A follow up to #106473 Minidump wasn't collecting fs or gs_base. This patch extends the x86_64 register context and gated reading it behind an lldb specific flag. Additionally these registers are explicitly checked in the tests.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)

Changes

A follow up to #106473 Minidump wasn't collecting fs or gs_base. This patch extends the x86_64 register context and gated reading it behind an lldb specific flag. Additionally these registers are explicitly checked in the tests.

---

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

4 Files Affected:

• (modified) lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp (+4-1)
• (modified) lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp (+8)
• (modified) lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h (+6-1)
• (modified) lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py (+12)

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 13355afb58dbd1..5c9ba223ad143e 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -473,7 +473,8 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
       lldb_private::minidump::MinidumpContext_x86_64_Flags::x86_64_Flag |
       lldb_private::minidump::MinidumpContext_x86_64_Flags::Control |
       lldb_private::minidump::MinidumpContext_x86_64_Flags::Segments |
-      lldb_private::minidump::MinidumpContext_x86_64_Flags::Integer);
+      lldb_private::minidump::MinidumpContext_x86_64_Flags::Integer |
+      lldb_private::minidump::MinidumpContext_x86_64_Flags::LLDBSpecific);
   thread_context.rax = read_register_u64(reg_ctx, "rax");
   thread_context.rbx = read_register_u64(reg_ctx, "rbx");
   thread_context.rcx = read_register_u64(reg_ctx, "rcx");
@@ -499,6 +500,8 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
   thread_context.gs = read_register_u64(reg_ctx, "gs");
   thread_context.ss = read_register_u64(reg_ctx, "ss");
   thread_context.ds = read_register_u64(reg_ctx, "ds");
+  thread_context.fs_base = read_register_u64(reg_ctx, "fs_base");
+  thread_context.gs_base = read_register_u64(reg_ctx, "gs_base");
   return thread_context;
 }
 
diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
index 917140cab29767..e879c493156593 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
@@ -67,6 +67,7 @@ lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContext_x86_64(
   auto ControlFlag = MinidumpContext_x86_64_Flags::Control;
   auto IntegerFlag = MinidumpContext_x86_64_Flags::Integer;
   auto SegmentsFlag = MinidumpContext_x86_64_Flags::Segments;
+  auto LLDBSpecificFlag = MinidumpContext_x86_64_Flags::LLDBSpecific;
 
   if ((context_flags & x86_64_Flag) != x86_64_Flag)
     return nullptr;
@@ -104,6 +105,13 @@ lldb::DataBufferSP lldb_private::minidump::ConvertMinidumpContext_x86_64(
     writeRegister(&context->r15, result_base, reg_info[lldb_r15_x86_64]);
   }
 
+  if ((context_flags & LLDBSpecificFlag) == LLDBSpecificFlag) {
+    writeRegister(&context->fs_base, result_base,
+                  reg_info[x86_64_with_base::lldb_fs_base]);
+    writeRegister(&context->gs_base, result_base,
+                  reg_info[x86_64_with_base::lldb_gs_base]);
+  }
+
   // TODO parse the floating point registers
 
   return result_context_buf;
diff --git a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
index d920ea9d823f4f..f214e04a315a8e 100644
--- a/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
+++ b/lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
@@ -153,6 +153,10 @@ struct MinidumpContext_x86_64 {
   llvm::support::ulittle64_t last_branch_from_rip;
   llvm::support::ulittle64_t last_exception_to_rip;
   llvm::support::ulittle64_t last_exception_from_rip;
+
+  // These registers are LLDB specific.
+  llvm::support::ulittle64_t fs_base;
+  llvm::support::ulittle64_t gs_base;
 };
 
 // For context_flags. These values indicate the type of
@@ -168,9 +172,10 @@ enum class MinidumpContext_x86_64_Flags : uint32_t {
   FloatingPoint = x86_64_Flag | 0x00000008,
   DebugRegisters = x86_64_Flag | 0x00000010,
   XState = x86_64_Flag | 0x00000040,
+  LLDBSpecific = x86_64_Flag | 0x80000000,
 
   Full = Control | Integer | FloatingPoint,
-  All = Full | Segments | DebugRegisters,
+  All = Full | Segments | DebugRegisters | LLDBSpecific,
 
   LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue = */ All)
 };
diff --git a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index ea59aef004aff5..ed15793b527fc9 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -67,6 +67,18 @@ def verify_core_file(
             self.assertIn(thread_id, stacks_to_registers_map)
             register_val_list = stacks_to_registers_map[thread_id]
             frame_register_list = frame.GetRegisters()
+            # explicitly verify we collected fs and gs base for x86_64
+            explicit_registers = ["fs_base", "gs_base"]
+            for reg in explicit_registers:
+                register = frame_register_list.GetFirstValueByName(reg)
+                self.assertNotEqual(None, register)
+                self.assertEqual(
+                    register.GetValueAsUnsigned(),
+                    stacks_to_registers_map[thread_id]
+                    .GetFirstValueByName("fs_base")
+                    .GetValueAsUnsigned(),
+                )
+
             for x in register_val_list:
                 self.assertEqual(
                     x.GetValueAsUnsigned(),

[jeffreytan81]

I do not think fs_base/gs_base is lldb specific. They are virtual registers but they are returned as part of ptrace call on Linux. For example, gdb and other debugger tools are showing them. I think we should include them as part of MinidumpContext_x86_64_Flags::x86_64_Flag flag.

[Jlalond]

I agree, the concern is extending the registers and other consumers/producers being unable to consume the new registers. @clayborg and I talked about this and the concern was if MSFT added new fields to the register context. However looking at the minidump docs Thread Context is actually just an RVA. So we should be able to get away with this.

@labath does Google or Brakepad include fs/gs_base and if they do can you point me to some docs so we follow suite?

[clayborg]

@jeffreytan81 we made this LLDB specific because this doesn't match the registers that are saved by Breakpad. I just checked the breakpad sources (https://github.com/google/breakpad.git) and they don't support fs_base or gs_base.

[clayborg]

@jeffreytan81 we made this LLDB specific because this doesn't match the registers that are saved by Breakpad. I just checked the breakpad sources (https://github.com/google/breakpad.git) and they don't support fs_base or gs_base.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.