Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 5, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ceb20d6
Extend the minidump x86_64 registers to include fs_base and gs_base
Jlalond Aug 30, 2024
7b4cf90
Rebase and format
Jlalond Aug 30, 2024
3da1e51
Comment rewording
Jlalond Sep 4, 2024
dc342c0
Run GCF
Jlalond Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

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");
Expand All @@ -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;
}

Expand Down
8 changes: 8 additions & 0 deletions lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion lldb/source/Plugins/Process/minidump/RegisterContextMinidump_x86_64.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ 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;

// LLDB can save core files and save extra information that isn't available
// from Google breakpad, or similar, minidump files.
llvm::support::ulittle64_t fs_base;
llvm::support::ulittle64_t gs_base;
};

// For context_flags. These values indicate the type of
Expand All @@ -168,9 +173,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)
};
Expand Down
12 changes: 12 additions & 0 deletions lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
Loading