Skip to content

Commit

Permalink
[LLDB][Minidumps] Read x64 registers as 64b and handle truncation in …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Jlalond authored and 5c4lar committed Aug 29, 2024
1 parent c44f66f commit c873678
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 8 deletions.
14 changes: 8 additions & 6 deletions lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -62,6 +63,17 @@ 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.assertIn(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)

Expand Down Expand Up @@ -93,12 +105,16 @@ 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 "
Expand All @@ -110,6 +126,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))
Expand All @@ -120,6 +137,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))
Expand All @@ -130,6 +148,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
stakcs_to_registers_map,
)

options = lldb.SBSaveCoreOptions()
Expand All @@ -147,6 +166,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
stakcs_to_registers_map,
)

options = lldb.SBSaveCoreOptions()
Expand All @@ -163,6 +183,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
Expand All @@ -181,6 +202,7 @@ def test_save_linux_mini_dump(self):
expected_modules,
expected_threads,
stacks_to_sp_map,
stakcs_to_registers_map,
)

self.assertSuccess(process.Kill())
Expand Down Expand Up @@ -276,13 +298,16 @@ 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
# minidump defaults to stacks only, so we want to see if the
Expand All @@ -294,7 +319,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))
Expand Down

0 comments on commit c873678

Please sign in to comment.