llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) <details> <summary>Changes</summary> 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 &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 + self.assertTrue(error.Fail(), error.GetCString()) + self.assertTrue(stacks_to_sps_map.__contains__(thread_id), "stacks_to_sps_map does not contain thread_idx: %d" % thread_idx) + # Ensure the SP is correct + self.assertEqual(stacks_to_sps_map[thread_id], sp) + + self.dbg.DeleteTarget(target) @skipUnlessArch("x86_64") @skipUnlessPlatform(["linux"]) def test_save_linux_mini_dump(self): """Test that we can save a Linux mini dump.""" + self.build() exe = self.getBuildArtifact("a.out") core_stack = self.getBuildArtifact("core.stack.dmp") @@ -69,30 +88,32 @@ def test_save_linux_mini_dump(self): expected_modules = target.modules expected_number_of_threads = process.GetNumThreads() expected_threads = [] + stacks_to_sp_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() # save core and, kill process and verify corefile existence base_command = "process save-core --plugin-name=minidump " self.runCmd(base_command + " --style=stack '%s'" % (core_stack)) self.assertTrue(os.path.isfile(core_stack)) self.verify_core_file( - core_stack, expected_pid, expected_modules, expected_threads + core_stack, expected_pid, expected_modules, expected_threads, stacks_to_sp_map ) self.runCmd(base_command + " --style=modified-memory '%s'" % (core_dirty)) self.assertTrue(os.path.isfile(core_dirty)) self.verify_core_file( - core_dirty, expected_pid, expected_modules, expected_threads + core_dirty, expected_pid, expected_modules, expected_threads, stacks_to_sp_map ) self.runCmd(base_command + " --style=full '%s'" % (core_full)) self.assertTrue(os.path.isfile(core_full)) self.verify_core_file( - core_full, expected_pid, expected_modules, expected_threads + core_full, expected_pid, expected_modules, expected_threads, stacks_to_sp_map ) # validate saving via SBProcess @@ -100,14 +121,14 @@ def test_save_linux_mini_dump(self): self.assertTrue(error.Success()) self.assertTrue(os.path.isfile(core_sb_stack)) self.verify_core_file( - core_sb_stack, expected_pid, expected_modules, expected_threads + core_sb_stack, expected_pid, expected_modules, expected_threads, stacks_to_sp_map ) error = process.SaveCore(core_sb_dirty, "minidump", lldb.eSaveCoreDirtyOnly) self.assertTrue(error.Success()) self.assertTrue(os.path.isfile(core_sb_dirty)) self.verify_core_file( - core_sb_dirty, expected_pid, expected_modules, expected_threads + core_sb_dirty, expected_pid, expected_modules, expected_threads, stacks_to_sp_map ) # Minidump can now save full core files, but they will be huge and @@ -116,7 +137,7 @@ def test_save_linux_mini_dump(self): self.assertTrue(error.Success()) self.assertTrue(os.path.isfile(core_sb_full)) self.verify_core_file( - core_sb_full, expected_pid, expected_modules, expected_threads + core_sb_full, expected_pid, expected_modules, expected_threads, stacks_to_sp_map ) self.assertSuccess(process.Kill()) `````````` </details> 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
