[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
@@ -6410,12 +6410,20 @@ GetCoreFileSaveRangesStackOnly(Process , if (!reg_ctx_sp) continue; const addr_t sp = reg_ctx_sp->GetSP(); +const size_t red_zone = process.GetABI()->GetRedZoneSize(); Jlalond wrote: Correct, but on systems where it would be greater than 0, we ensure we capture it for the memory dump as well. https://github.com/llvm/llvm-project/pull/92002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
@@ -6410,12 +6410,20 @@ GetCoreFileSaveRangesStackOnly(Process , if (!reg_ctx_sp) continue; const addr_t sp = reg_ctx_sp->GetSP(); +const size_t red_zone = process.GetABI()->GetRedZoneSize(); bulbazord wrote: Asking for my understanding: This will only really do anything if the size of the red zone is greater than 0 right? Otherwise, this change is a no-op. Is that right? https://github.com/llvm/llvm-project/pull/92002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jacob Lalonde (Jlalond)
Changes
Currently in Core dumps, the entire pthread is copied, including the unused
space beyond the stack pointer. This causes large amounts of core dump
inflation when the number of threads is high, but the stack usage is low. Such
as when an application is using a thread pool.
This change will optimize for these situations in addition to generally
improving the core dump performance for all of lldb.
---
Full diff: https://github.com/llvm/llvm-project/pull/92002.diff
2 Files Affected:
- (modified) lldb/source/Target/Process.cpp (+11-3)
- (modified)
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
(+29-8)
``diff
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 25afade9a8275..a11e45909202f 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3857,8 +3857,8 @@ thread_result_t Process::RunPrivateStateThread(bool
is_secondary_thread) {
// case we should tell it to stop doing that. Normally, we don't NEED
// to do that because we will next close the communication to the stub
// and that will get it to shut down. But there are remote debugging
-// cases where relying on that side-effect causes the shutdown to be
-// flakey, so we should send a positive signal to interrupt the wait.
+// cases where relying on that side-effect causes the shutdown to be
+// flakey, so we should send a positive signal to interrupt the wait.
Status error = HaltPrivate();
BroadcastEvent(eBroadcastBitInterrupt, nullptr);
} else if (StateIsRunningState(m_last_broadcast_state)) {
@@ -6410,12 +6410,20 @@ GetCoreFileSaveRangesStackOnly(Process ,
if (!reg_ctx_sp)
continue;
const addr_t sp = reg_ctx_sp->GetSP();
+const size_t red_zone = process.GetABI()->GetRedZoneSize();
lldb_private::MemoryRegionInfo sp_region;
if (process.GetMemoryRegionInfo(sp, sp_region).Success()) {
// Only add this region if not already added above. If our stack pointer
// is pointing off in the weeds, we will want this range.
- if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0)
+ if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0) {
+// Take only the start of the stack to the stack pointer and include
the redzone.
+// Because stacks grow 'down' to include the red_zone we have to
subtract it from the sp.
+const size_t stack_head = (sp - red_zone);
+const size_t stack_size = sp_region.GetRange().GetRangeEnd() -
(stack_head);
+sp_region.GetRange().SetRangeBase(stack_head);
+sp_region.GetRange().SetByteSize(stack_size);
AddRegion(sp_region, try_dirty_pages, ranges);
+ }
}
}
}
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 9fe5e89142987..56f75ec7e9708 100644
---
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -9,10 +9,9 @@
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
-
class ProcessSaveCoreMinidumpTestCase(TestBase):
def verify_core_file(
-self, core_path, expected_pid, expected_modules, expected_threads
+self, core_path, expected_pid, expected_modules, expected_threads,
stacks_to_sps_map
):
# To verify, we'll launch with the mini dump
target = self.dbg.CreateTarget(None)
@@ -36,17 +35,37 @@ def verify_core_file(
self.assertEqual(module_file_name, expected_file_name)
self.assertEqual(module.GetUUIDString(), expected.GetUUIDString())
+red_zone = process.GetTarget().GetStackRedZoneSize()
for thread_idx in range(process.GetNumThreads()):
thread = process.GetThreadAtIndex(thread_idx)
self.assertTrue(thread.IsValid())
thread_id = thread.GetThreadID()
self.assertIn(thread_id, expected_threads)
+frame = thread.GetFrameAtIndex(0)
+sp_region = lldb.SBMemoryRegionInfo()
+sp = frame.GetSP()
+err = process.GetMemoryRegionInfo(sp, sp_region)
+self.assertTrue(err.Success(), err.GetCString())
+error = lldb.SBError()
+# Try to read at the end of the stack red zone and succeed
+process.ReadMemory(sp_region.GetRegionEnd() - 1, 1, error)
+self.assertTrue(error.Success(), error.GetCString())
+# Try to read just past the red zone and fail
+process.ReadMemory(sp_region.GetRegionEnd() + 1, 1, error)
+# Try to read from the base of the stack
+
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
Jlalond wrote: @clayborg Could you take a look at this? https://github.com/llvm/llvm-project/pull/92002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/92002 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB/Coredump] Only take the Pthread from start start to the stackpointer + red_zone (PR #92002)
https://github.com/Jlalond created
https://github.com/llvm/llvm-project/pull/92002
Currently in Core dumps, the entire pthread is copied, including the unused
space beyond the stack pointer. This causes large amounts of core dump
inflation when the number of threads is high, but the stack usage is low. Such
as when an application is using a thread pool.
This change will optimize for these situations in addition to generally
improving the core dump performance for all of lldb.
>From 2d192f640b332c2f1381cf96b75be60ad18de3ac Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Fri, 10 May 2024 09:35:11 -0700
Subject: [PATCH 1/3] change core dump stacks to only include up to the stack
pointer (+ red zone) Add python tests to verify we can read up to sp +
redzone - 1.
---
lldb/source/Target/Process.cpp | 14 +++---
.../TestProcessSaveCoreMinidump.py | 12
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 25afade9a8275..a11e45909202f 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3857,8 +3857,8 @@ thread_result_t Process::RunPrivateStateThread(bool
is_secondary_thread) {
// case we should tell it to stop doing that. Normally, we don't NEED
// to do that because we will next close the communication to the stub
// and that will get it to shut down. But there are remote debugging
-// cases where relying on that side-effect causes the shutdown to be
-// flakey, so we should send a positive signal to interrupt the wait.
+// cases where relying on that side-effect causes the shutdown to be
+// flakey, so we should send a positive signal to interrupt the wait.
Status error = HaltPrivate();
BroadcastEvent(eBroadcastBitInterrupt, nullptr);
} else if (StateIsRunningState(m_last_broadcast_state)) {
@@ -6410,12 +6410,20 @@ GetCoreFileSaveRangesStackOnly(Process ,
if (!reg_ctx_sp)
continue;
const addr_t sp = reg_ctx_sp->GetSP();
+const size_t red_zone = process.GetABI()->GetRedZoneSize();
lldb_private::MemoryRegionInfo sp_region;
if (process.GetMemoryRegionInfo(sp, sp_region).Success()) {
// Only add this region if not already added above. If our stack pointer
// is pointing off in the weeds, we will want this range.
- if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0)
+ if (stack_bases.count(sp_region.GetRange().GetRangeBase()) == 0) {
+// Take only the start of the stack to the stack pointer and include
the redzone.
+// Because stacks grow 'down' to include the red_zone we have to
subtract it from the sp.
+const size_t stack_head = (sp - red_zone);
+const size_t stack_size = sp_region.GetRange().GetRangeEnd() -
(stack_head);
+sp_region.GetRange().SetRangeBase(stack_head);
+sp_region.GetRange().SetByteSize(stack_size);
AddRegion(sp_region, try_dirty_pages, ranges);
+ }
}
}
}
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 9fe5e89142987..123bbd472be05 100644
---
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -14,6 +14,7 @@ class ProcessSaveCoreMinidumpTestCase(TestBase):
def verify_core_file(
self, core_path, expected_pid, expected_modules, expected_threads
):
+breakpoint()
# To verify, we'll launch with the mini dump
target = self.dbg.CreateTarget(None)
process = target.LoadCore(core_path)
@@ -36,11 +37,22 @@ def verify_core_file(
self.assertEqual(module_file_name, expected_file_name)
self.assertEqual(module.GetUUIDString(), expected.GetUUIDString())
+red_zone = process.GetTarget().GetStackRedZoneSize()
for thread_idx in range(process.GetNumThreads()):
thread = process.GetThreadAtIndex(thread_idx)
self.assertTrue(thread.IsValid())
thread_id = thread.GetThreadID()
self.assertIn(thread_id, expected_threads)
+oldest_frame = thread.GetFrameAtIndex(thread.GetNumFrames() - 1)
+stack_start =
oldest_frame.GetSymbol().GetStartAddress().GetFileAddress()
+frame = thread.GetFrameAtIndex(0)
+sp = frame.GetSP()
+stack_size = stack_start - (sp - red_zone)
+byte_array = process.ReadMemory(sp - red_zone + 1, stack_size,
error)
+self.assertTrue(error.Success(), "Failed to read stack")
+self.assertEqual(stack_size - 1, len(byte_array), "Incorrect stack
size read"),
+
+
