-
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][Minidumps] Read x64 registers as 64b and handle truncation in the file builder #106473
Conversation
…add verification in the test for registers
@llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) ChangesThis patch addresses a bug where In addition to that fix, I added comparing the registers from the live process to the loaded core for the generic minidump test. Prior only being ARM register tests. This explains why this was not detected before. Full diff: https://github.com/llvm/llvm-project/pull/106473.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 689a3fb0e84852..13355afb58dbd1 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -491,12 +491,14 @@ GetThreadContext_x86_64(RegisterContext *reg_ctx) {
thread_context.r14 = read_register_u64(reg_ctx, "r14");
thread_context.r15 = read_register_u64(reg_ctx, "r15");
thread_context.rip = read_register_u64(reg_ctx, "rip");
- thread_context.eflags = read_register_u32(reg_ctx, "rflags");
- thread_context.cs = read_register_u16(reg_ctx, "cs");
- thread_context.fs = read_register_u16(reg_ctx, "fs");
- thread_context.gs = read_register_u16(reg_ctx, "gs");
- thread_context.ss = read_register_u16(reg_ctx, "ss");
- thread_context.ds = read_register_u16(reg_ctx, "ds");
+ // To make our code agnostic to whatever type the register value identifies
+ // itself as, we read as a u64 and truncate to u32/u16 ourselves.
+ thread_context.eflags = read_register_u64(reg_ctx, "rflags");
+ thread_context.cs = read_register_u64(reg_ctx, "cs");
+ thread_context.fs = read_register_u64(reg_ctx, "fs");
+ 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");
return thread_context;
}
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 5abaa05a90f63e..81b0e44146a44e 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -17,6 +17,7 @@ def verify_core_file(
expected_modules,
expected_threads,
stacks_to_sps_map,
+ stacks_to_registers_map,
):
# To verify, we'll launch with the mini dump
target = self.dbg.CreateTarget(None)
@@ -62,6 +63,12 @@ def verify_core_file(
# Try to read just past the red zone and fail
process.ReadMemory(sp - red_zone - 1, 1, error)
self.assertTrue(error.Fail(), "No failure when reading past the red zone")
+ # Verify the registers are the same
+ self.assertTrue(thread_id, stacks_to_registers_map)
+ register_val_list = stacks_to_registers_map[thread_id]
+ frame_register_list = frame.GetRegisters()
+ for x in register_val_list:
+ self.assertEqual(x.GetValueAsUnsigned(), frame_register_list.GetFirstValueByName(x.GetName()).GetValueAsUnsigned())
self.dbg.DeleteTarget(target)
@@ -93,12 +100,14 @@ def test_save_linux_mini_dump(self):
expected_number_of_threads = process.GetNumThreads()
expected_threads = []
stacks_to_sp_map = {}
+ stakcs_to_registers_map = {}
for thread_idx in range(process.GetNumThreads()):
thread = process.GetThreadAtIndex(thread_idx)
thread_id = thread.GetThreadID()
expected_threads.append(thread_id)
stacks_to_sp_map[thread_id] = thread.GetFrameAtIndex(0).GetSP()
+ stakcs_to_registers_map[thread_id] = thread.GetFrameAtIndex(0).GetRegisters()
# save core and, kill process and verify corefile existence
base_command = "process save-core --plugin-name=minidump "
@@ -110,6 +119,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
+ stakcs_to_registers_map
)
self.runCmd(base_command + " --style=modified-memory '%s'" % (core_dirty))
@@ -120,6 +130,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
+ stakcs_to_registers_map
)
self.runCmd(base_command + " --style=full '%s'" % (core_full))
@@ -130,6 +141,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
+ stakcs_to_registers_map
)
options = lldb.SBSaveCoreOptions()
@@ -147,6 +159,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
+ stakcs_to_registers_map
)
options = lldb.SBSaveCoreOptions()
@@ -163,6 +176,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
+ stakcs_to_registers_map
)
# Minidump can now save full core files, but they will be huge and
@@ -181,6 +195,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
+ stakcs_to_registers_map
)
self.assertSuccess(process.Kill())
@@ -276,12 +291,14 @@ def test_save_linux_mini_dump_default_options(self):
expected_threads = []
stacks_to_sp_map = {}
expected_pid = process.GetProcessInfo().GetProcessID()
+ stacks_to_registers_map = {}
for thread_idx in range(process.GetNumThreads()):
thread = process.GetThreadAtIndex(thread_idx)
thread_id = thread.GetThreadID()
expected_threads.append(thread_id)
stacks_to_sp_map[thread_id] = thread.GetFrameAtIndex(0).GetSP()
+ stacks_to_registers_map[thread_id] = thread.GetFrameAtIndex(0).GetRegisters()
# This is almost identical to the single thread test case because
@@ -294,7 +311,14 @@ def test_save_linux_mini_dump_default_options(self):
error = process.SaveCore(options)
self.assertTrue(error.Success())
- self.verify_core_file(default_value_file, expected_pid, expected_modules, expected_threads, stacks_to_sp_map)
+ self.verify_core_file(
+ default_value_file,
+ expected_pid,
+ expected_modules,
+ expected_threads,
+ stacks_to_sp_map,
+ stacks_to_registers_map
+ )
finally:
self.assertTrue(self.dbg.DeleteTarget(target))
|
✅ With the latest revision this PR passed the Python code formatter. |
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
Outdated
Show resolved
Hide resolved
…the file builder (llvm#106473) This patch addresses a bug where `cs`/`fs` and other segmentation flags were being identified as having a type of `32b` and `64b` for `rflags`. In that case the register value was returning the fail value `0xF...` and this was corrupting some minidumps. Here we just read it as a 64b value and truncate it. In addition to that fix, I added comparing the registers from the live process to the loaded core for the generic minidump test. Prior only being ARM register tests. This explains why this was not detected before.
…ase and gs_base (llvm#106767) A follow up to llvm#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.
…d0cecbbff Local branch amd-gfx 0e8d0ce Merged main:438ad9f2bf25575c474313de4ad85a5da6f69e4c into amd-gfx:b82566cedfb0 Remote branch main 82ebd33 [LLDB][Minidumps] Read x64 registers as 64b and handle truncation in the file builder (llvm#106473)
This patch addresses a bug where
cs
/fs
and other segmentation flags were being identified as having a type of32b
and64b
forrflags
. In that case the register value was returning the fail value0xF...
and this was corrupting some minidumps. Here we just read it as a 64b value and truncate it.In addition to that fix, I added comparing the registers from the live process to the loaded core for the generic minidump test. Prior only being ARM register tests. This explains why this was not detected before.