[Lldb-commits] [lldb] [LLDB][Minidumps] Read x64 registers as 64b and handle truncation in the file builder (PR #106473)

Jacob Lalonde via lldb-commits lldb-commits at lists.llvm.org
Wed Aug 28 16:57:05 PDT 2024


https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/106473

>From 88a1a5d2b8698a5474bff0756012369401f7f433 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 28 Aug 2024 16:50:57 -0700
Subject: [PATCH 1/2] 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))

>From 7cb76d150643d1b70b15a00d431a5b324dac826d Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Wed, 28 Aug 2024 16:56:56 -0700
Subject: [PATCH 2/2] Run python formatter

---
 .../TestProcessSaveCoreMinidump.py            | 30 ++++++++++++-------
 1 file changed, 19 insertions(+), 11 deletions(-)

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 81b0e44146a44e..f4ad25421c6a83 100644
--- a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -68,7 +68,12 @@ def verify_core_file(
             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.assertEqual(
+                    x.GetValueAsUnsigned(),
+                    frame_register_list.GetFirstValueByName(
+                        x.GetName()
+                    ).GetValueAsUnsigned(),
+                )
 
         self.dbg.DeleteTarget(target)
 
@@ -107,7 +112,9 @@ def test_save_linux_mini_dump(self):
                 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()
+                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 "
@@ -119,7 +126,7 @@ def test_save_linux_mini_dump(self):
                 expected_modules,
                 expected_threads,
                 stacks_to_sp_map,
-                stakcs_to_registers_map
+                stakcs_to_registers_map,
             )
 
             self.runCmd(base_command + " --style=modified-memory '%s'" % (core_dirty))
@@ -130,7 +137,7 @@ def test_save_linux_mini_dump(self):
                 expected_modules,
                 expected_threads,
                 stacks_to_sp_map,
-                stakcs_to_registers_map
+                stakcs_to_registers_map,
             )
 
             self.runCmd(base_command + " --style=full '%s'" % (core_full))
@@ -141,7 +148,7 @@ def test_save_linux_mini_dump(self):
                 expected_modules,
                 expected_threads,
                 stacks_to_sp_map,
-                stakcs_to_registers_map
+                stakcs_to_registers_map,
             )
 
             options = lldb.SBSaveCoreOptions()
@@ -159,7 +166,7 @@ def test_save_linux_mini_dump(self):
                 expected_modules,
                 expected_threads,
                 stacks_to_sp_map,
-                stakcs_to_registers_map
+                stakcs_to_registers_map,
             )
 
             options = lldb.SBSaveCoreOptions()
@@ -176,7 +183,7 @@ def test_save_linux_mini_dump(self):
                 expected_modules,
                 expected_threads,
                 stacks_to_sp_map,
-                stakcs_to_registers_map
+                stakcs_to_registers_map,
             )
 
             # Minidump can now save full core files, but they will be huge and
@@ -195,7 +202,7 @@ def test_save_linux_mini_dump(self):
                 expected_modules,
                 expected_threads,
                 stacks_to_sp_map,
-                stakcs_to_registers_map
+                stakcs_to_registers_map,
             )
 
             self.assertSuccess(process.Kill())
@@ -298,8 +305,9 @@ def test_save_linux_mini_dump_default_options(self):
                 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()
-
+                stacks_to_registers_map[thread_id] = thread.GetFrameAtIndex(
+                    0
+                ).GetRegisters()
 
             # This is almost identical to the single thread test case because
             # minidump defaults to stacks only, so we want to see if the
@@ -317,7 +325,7 @@ def test_save_linux_mini_dump_default_options(self):
                 expected_modules,
                 expected_threads,
                 stacks_to_sp_map,
-                stacks_to_registers_map
+                stacks_to_registers_map,
             )
 
         finally:



More information about the lldb-commits mailing list