https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/106473
This patch addresses a bug where `cs`/`gs` 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. >From 88a1a5d2b8698a5474bff0756012369401f7f433 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 28 Aug 2024 16:50:57 -0700 Subject: [PATCH] Have minidump read all registers as 64 and handle truncation itself, add verification in the test for registers --- .../Minidump/MinidumpFileBuilder.cpp | 14 +++++----- .../TestProcessSaveCoreMinidump.py | 26 ++++++++++++++++++- 2 files changed, 33 insertions(+), 7 deletions(-) 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)) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits