From 8c31efeed6007bade1e4ddda2ceccab311661cbc Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Wed, 11 Aug 2021 13:37:31 -0700 Subject: [PATCH] Add the ability to process save-core stack-memory-only corefiles Add a field to the qMemoryRegionInfo packet where the remote stub can describe the type of memory -- heap, stack. Keep track of memory regions that are stack memory in lldb. Add a new "--style stack" to process save-core to request that only stack memory be included in the corefile. Differential Revision: https://reviews.llvm.org/D107625 --- lldb/docs/lldb-gdb-remote.txt | 3 + lldb/include/lldb/Target/MemoryRegionInfo.h | 12 +++- lldb/include/lldb/lldb-enumerations.h | 1 + lldb/source/Commands/CommandObjectProcess.cpp | 13 ++-- .../Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp | 52 +++++++++++----- .../gdb-remote/GDBRemoteCommunicationClient.cpp | 13 ++++ .../macosx/skinny-corefile/TestSkinnyCorefile.py | 2 +- lldb/test/API/macosx/stack-corefile/Makefile | 3 + .../API/macosx/stack-corefile/TestStackCorefile.py | 69 ++++++++++++++++++++++ lldb/test/API/macosx/stack-corefile/main.c | 15 +++++ lldb/tools/debugserver/source/DNBDefs.h | 5 +- .../debugserver/source/MacOSX/MachVMMemory.cpp | 1 + .../debugserver/source/MacOSX/MachVMRegion.cpp | 40 +++++++++++++ .../tools/debugserver/source/MacOSX/MachVMRegion.h | 1 + lldb/tools/debugserver/source/RNBRemote.cpp | 8 +++ 15 files changed, 212 insertions(+), 26 deletions(-) create mode 100644 lldb/test/API/macosx/stack-corefile/Makefile create mode 100644 lldb/test/API/macosx/stack-corefile/TestStackCorefile.py create mode 100644 lldb/test/API/macosx/stack-corefile/main.c diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt index 3eb3dc5..1758802 100644 --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -1206,6 +1206,9 @@ tuples to return are: // is "mt" for AArch64 memory tagging. lldb will // ignore any other flags in this field. + type:[][,]; // memory types that apply to this region, e.g. + // "stack" for stack memory. + error:; // where is // a hex encoded string value that // contains an error string diff --git a/lldb/include/lldb/Target/MemoryRegionInfo.h b/lldb/include/lldb/Target/MemoryRegionInfo.h index c43f27e..fc5fcff 100644 --- a/lldb/include/lldb/Target/MemoryRegionInfo.h +++ b/lldb/include/lldb/Target/MemoryRegionInfo.h @@ -28,10 +28,10 @@ public: MemoryRegionInfo(RangeType range, OptionalBool read, OptionalBool write, OptionalBool execute, OptionalBool mapped, ConstString name, OptionalBool flash, lldb::offset_t blocksize, - OptionalBool memory_tagged) + OptionalBool memory_tagged, OptionalBool stack_memory) : m_range(range), m_read(read), m_write(write), m_execute(execute), m_mapped(mapped), m_name(name), m_flash(flash), m_blocksize(blocksize), - m_memory_tagged(memory_tagged) {} + m_memory_tagged(memory_tagged), m_is_stack_memory(stack_memory) {} RangeType &GetRange() { return m_range; } @@ -98,7 +98,8 @@ public: m_mapped == rhs.m_mapped && m_name == rhs.m_name && m_flash == rhs.m_flash && m_blocksize == rhs.m_blocksize && m_memory_tagged == rhs.m_memory_tagged && - m_pagesize == rhs.m_pagesize; + m_pagesize == rhs.m_pagesize && + m_is_stack_memory == rhs.m_is_stack_memory; } bool operator!=(const MemoryRegionInfo &rhs) const { return !(*this == rhs); } @@ -116,6 +117,10 @@ public: return m_dirty_pages; } + OptionalBool IsStackMemory() const { return m_is_stack_memory; } + + void SetIsStackMemory(OptionalBool val) { m_is_stack_memory = val; } + void SetPageSize(int pagesize) { m_pagesize = pagesize; } void SetDirtyPageList(std::vector pagelist) { @@ -134,6 +139,7 @@ protected: OptionalBool m_flash = eDontKnow; lldb::offset_t m_blocksize = 0; OptionalBool m_memory_tagged = eDontKnow; + OptionalBool m_is_stack_memory = eDontKnow; int m_pagesize = 0; llvm::Optional> m_dirty_pages; }; diff --git a/lldb/include/lldb/lldb-enumerations.h b/lldb/include/lldb/lldb-enumerations.h index 81f6be3..f5cabc0 100644 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1137,6 +1137,7 @@ enum SaveCoreStyle { eSaveCoreUnspecified = 0, eSaveCoreFull = 1, eSaveCoreDirtyOnly = 2, + eSaveCoreStackOnly = 3, }; } // namespace lldb diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp index 1a8ed02..c65b661 100644 --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1166,7 +1166,9 @@ protected: static constexpr OptionEnumValueElement g_corefile_save_style[] = { {eSaveCoreFull, "full", "Create a core file with all memory saved"}, {eSaveCoreDirtyOnly, "modified-memory", - "Create a corefile with only modified memory saved"}}; + "Create a corefile with only modified memory saved"}, + {eSaveCoreStackOnly, "stack", + "Create a corefile with only stack memory saved"}}; static constexpr OptionEnumValues SaveCoreStyles() { return OptionEnumValues(g_corefile_save_style); @@ -1237,11 +1239,12 @@ protected: Status error = PluginManager::SaveCore(process_sp, output_file, corefile_style); if (error.Success()) { - if (corefile_style == SaveCoreStyle::eSaveCoreDirtyOnly) { + if (corefile_style == SaveCoreStyle::eSaveCoreDirtyOnly || + corefile_style == SaveCoreStyle::eSaveCoreStackOnly) { result.AppendMessageWithFormat( - "\nModified-memory only corefile " - "created. This corefile may not show \n" - "library/framework/app binaries " + "\nModified-memory or stack-memory only corefile " + "created. This corefile may \n" + "not show library/framework/app binaries " "on a different system, or when \n" "those binaries have " "been updated/modified. Copies are not included\n" diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index ce53249..decccc4 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6342,13 +6342,17 @@ struct segment_vmaddr { // are some multiple passes over the image list while calculating // everything. -static offset_t -CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp, - offset_t initial_file_offset, - StreamString &all_image_infos_payload) { +static offset_t CreateAllImageInfosPayload( + const lldb::ProcessSP &process_sp, offset_t initial_file_offset, + StreamString &all_image_infos_payload, SaveCoreStyle core_style) { Target &target = process_sp->GetTarget(); - const ModuleList &modules = target.GetImages(); - size_t modules_count = modules.GetSize(); + ModuleList modules = target.GetImages(); + + // stack-only corefiles have no reason to include binaries that + // are not executing; we're trying to make the smallest corefile + // we can, so leave the rest out. + if (core_style == SaveCoreStyle::eSaveCoreStackOnly) + modules.Clear(); std::set executing_uuids; ThreadList &thread_list(process_sp->GetThreadList()); @@ -6363,10 +6367,12 @@ CreateAllImageInfosPayload(const lldb::ProcessSP &process_sp, UUID uuid = module_sp->GetUUID(); if (uuid.IsValid()) { executing_uuids.insert(uuid.GetAsString()); + modules.AppendIfNeeded(module_sp); } } } } + size_t modules_count = modules.GetSize(); struct all_image_infos_header infos; infos.version = 1; @@ -6508,12 +6514,9 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, if (!process_sp) return false; - // For Mach-O, we can only create full corefiles or dirty-page-only - // corefiles. The default is dirty-page-only. - if (core_style != SaveCoreStyle::eSaveCoreFull) { + // Default on macOS is to create a dirty-memory-only corefile. + if (core_style == SaveCoreStyle::eSaveCoreUnspecified) { core_style = SaveCoreStyle::eSaveCoreDirtyOnly; - } else { - core_style = SaveCoreStyle::eSaveCoreFull; } Target &target = process_sp->GetTarget(); @@ -6568,13 +6571,23 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, if (size == 0) break; - if (prot != 0) { + bool include_this_region = true; + bool dirty_pages_only = false; + if (core_style == SaveCoreStyle::eSaveCoreStackOnly) { + dirty_pages_only = true; + if (range_info.IsStackMemory() != MemoryRegionInfo::eYes) { + include_this_region = false; + } + } + if (core_style == SaveCoreStyle::eSaveCoreDirtyOnly) { + dirty_pages_only = true; + } + + if (prot != 0 && include_this_region) { addr_t pagesize = range_info.GetPageSize(); const llvm::Optional> &dirty_page_list = range_info.GetDirtyPageList(); - if (core_style == SaveCoreStyle::eSaveCoreDirtyOnly && - dirty_page_list.hasValue()) { - core_style = SaveCoreStyle::eSaveCoreDirtyOnly; + if (dirty_pages_only && dirty_page_list.hasValue()) { for (addr_t dirtypage : dirty_page_list.getValue()) { page_object obj; obj.addr = dirtypage; @@ -6617,6 +6630,12 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, last_obj = obj; } + // If we only ended up with one contiguous memory segment + if (combined_page_objects.size() == 0 && + last_obj.addr != LLDB_INVALID_ADDRESS) { + combined_page_objects.push_back(last_obj); + } + for (page_object obj : combined_page_objects) { uint32_t cmd_type = LC_SEGMENT_64; uint32_t segment_size = sizeof(llvm::MachO::segment_command_64); @@ -6767,7 +6786,8 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp, all_image_infos_lcnote_up->name = "all image infos"; all_image_infos_lcnote_up->payload_file_offset = file_offset; file_offset = CreateAllImageInfosPayload( - process_sp, file_offset, all_image_infos_lcnote_up->payload); + process_sp, file_offset, all_image_infos_lcnote_up->payload, + core_style); lc_notes.push_back(std::move(all_image_infos_lcnote_up)); // Add LC_NOTE load commands diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index ee8c7c6..e898a08 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -1572,6 +1572,19 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo( } } } + } else if (name.equals("type")) { + std::string comma_sep_str = value.str(); + size_t comma_pos; + while ((comma_pos = comma_sep_str.find(',')) != std::string::npos) { + comma_sep_str[comma_pos] = '\0'; + if (comma_sep_str == "stack") { + region_info.SetIsStackMemory(MemoryRegionInfo::eYes); + } + } + // handle final (or only) type of "stack" + if (comma_sep_str == "stack") { + region_info.SetIsStackMemory(MemoryRegionInfo::eYes); + } } else if (name.equals("error")) { StringExtractorGDBRemote error_extractor(value); std::string error_string; diff --git a/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py b/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py index c18ef44..d313bb9 100644 --- a/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py +++ b/lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py @@ -12,7 +12,7 @@ from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil -class TestFirmwareCorefiles(TestBase): +class TestSkinnyCorefile(TestBase): mydir = TestBase.compute_mydir(__file__) diff --git a/lldb/test/API/macosx/stack-corefile/Makefile b/lldb/test/API/macosx/stack-corefile/Makefile new file mode 100644 index 0000000..58c3a46 --- /dev/null +++ b/lldb/test/API/macosx/stack-corefile/Makefile @@ -0,0 +1,3 @@ +C_SOURCES = main.c + +include Makefile.rules diff --git a/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py b/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py new file mode 100644 index 0000000..a9104e7 --- /dev/null +++ b/lldb/test/API/macosx/stack-corefile/TestStackCorefile.py @@ -0,0 +1,69 @@ +"""Test that lldb can create a stack-only corefile, and load the main binary.""" + +import os +import re +import subprocess + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +class TestStackCorefile(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + @no_debug_info_test + @skipUnlessDarwin + def test(self): + + corefile = self.getBuildArtifact("process.core") + self.build() + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "// break here", lldb.SBFileSpec("main.c")) + + frame = thread.GetFrameAtIndex(0) + stack_int = frame.GetValueForVariablePath("stack_int") + heap_int = frame.GetValueForVariablePath("*heap_int") + stack_str = frame.GetValueForVariablePath("stack_str") + heap_str = frame.GetValueForVariablePath("heap_str") + self.assertEqual(stack_int.GetValueAsUnsigned(), 5) + self.assertEqual(heap_int.GetValueAsUnsigned(), 10) + self.assertEqual(stack_str.summary, '"stack string"') + self.assertEqual(heap_str.summary, '"heap string"') + + self.runCmd("process save-core -s stack " + corefile) + process.Kill() + self.dbg.DeleteTarget(target) + + # Now load the corefile + target = self.dbg.CreateTarget('') + process = target.LoadCore(corefile) + thread = process.GetSelectedThread() + self.assertTrue(process.IsValid()) + self.assertTrue(process.GetSelectedThread().IsValid()) + if self.TraceOn(): + self.runCmd("image list") + self.runCmd("bt") + self.runCmd("fr v") + num_modules = target.GetNumModules() + # We should only have the a.out binary and possibly + # the libdyld.dylib. Extra libraries loaded means + # extra LC_NOTE and unnecessarily large corefile. + self.assertTrue(num_modules == 1 or num_modules == 2) + + # The heap variables should be unavailable now. + frame = thread.GetFrameAtIndex(0) + stack_int = frame.GetValueForVariablePath("stack_int") + heap_int = frame.GetValueForVariablePath("*heap_int") + stack_str = frame.GetValueForVariablePath("stack_str") + heap_str = frame.GetValueForVariablePath("heap_str") + + ## The heap SBValues both come back as IsValid()==true, + ## which I'm not so sure is a great/correct thing -- + ## it hides the memory read error that actually happened, + ## and we don't have a real value. + self.assertEqual(stack_int.GetValueAsUnsigned(), 5) + self.assertEqual(heap_int.GetValueAsUnsigned(), 0) + self.assertEqual(stack_str.summary, '"stack string"') + self.assertEqual(heap_str.summary, '""') diff --git a/lldb/test/API/macosx/stack-corefile/main.c b/lldb/test/API/macosx/stack-corefile/main.c new file mode 100644 index 0000000..a52fb30 --- /dev/null +++ b/lldb/test/API/macosx/stack-corefile/main.c @@ -0,0 +1,15 @@ +#include +#include +#include +int main() { + int stack_int = 5; + int *heap_int = (int*) malloc(sizeof (int)); + *heap_int = 10; + + char stack_str[80]; + strcpy (stack_str, "stack string"); + char *heap_str = (char*) malloc(80); + strcpy (heap_str, "heap string"); + + return stack_int; // break here; +} diff --git a/lldb/tools/debugserver/source/DNBDefs.h b/lldb/tools/debugserver/source/DNBDefs.h index 739d453..6579642 100644 --- a/lldb/tools/debugserver/source/DNBDefs.h +++ b/lldb/tools/debugserver/source/DNBDefs.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -318,11 +319,13 @@ struct DNBExecutableImageInfo { struct DNBRegionInfo { public: - DNBRegionInfo() : addr(0), size(0), permissions(0), dirty_pages() {} + DNBRegionInfo() + : addr(0), size(0), permissions(0), dirty_pages(), vm_types() {} nub_addr_t addr; nub_addr_t size; uint32_t permissions; std::vector dirty_pages; + std::vector vm_types; }; enum DNBProfileDataScanType { diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp b/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp index 1422064..15f1b5d 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp +++ b/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp @@ -125,6 +125,7 @@ nub_bool_t MachVMMemory::GetMemoryRegionInfo(task_t task, nub_addr_t address, region_info->permissions = vmRegion.GetDNBPermissions(); region_info->dirty_pages = get_dirty_pages(task, vmRegion.StartAddress(), vmRegion.GetByteSize()); + region_info->vm_types = vmRegion.GetMemoryTypes(); } else { region_info->addr = address; region_info->size = 0; diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp index ed2964c..54785e5 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp +++ b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp @@ -182,3 +182,43 @@ uint32_t MachVMRegion::GetDNBPermissions() const { dnb_permissions |= eMemoryPermissionsExecutable; return dnb_permissions; } + +std::vector MachVMRegion::GetMemoryTypes() const { + std::vector types; + if (m_data.user_tag == VM_MEMORY_STACK) { + if (m_data.protection == VM_PROT_NONE) { + types.push_back("stack-guard"); + } else { + types.push_back("stack"); + } + } + if (m_data.user_tag == VM_MEMORY_MALLOC) { + if (m_data.protection == VM_PROT_NONE) + types.push_back("malloc-guard"); + else if (m_data.share_mode == SM_EMPTY) + types.push_back("malloc-reserved"); + else + types.push_back("malloc-metadata"); + } + if (m_data.user_tag == VM_MEMORY_MALLOC_NANO || + m_data.user_tag == VM_MEMORY_MALLOC_TINY || + m_data.user_tag == VM_MEMORY_MALLOC_SMALL || + m_data.user_tag == VM_MEMORY_MALLOC_LARGE || + m_data.user_tag == VM_MEMORY_MALLOC_LARGE_REUSED || + m_data.user_tag == VM_MEMORY_MALLOC_LARGE_REUSABLE || + m_data.user_tag == VM_MEMORY_MALLOC_HUGE || + m_data.user_tag == VM_MEMORY_REALLOC || + m_data.user_tag == VM_MEMORY_SBRK) { + types.push_back("heap"); + if (m_data.user_tag == VM_MEMORY_MALLOC_TINY) { + types.push_back("malloc-tiny"); + } + if (m_data.user_tag == VM_MEMORY_MALLOC_LARGE) { + types.push_back("malloc-large"); + } + if (m_data.user_tag == VM_MEMORY_MALLOC_SMALL) { + types.push_back("malloc-small"); + } + } + return types; +} diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h index 210b1c9..cb77058 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h +++ b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h @@ -40,6 +40,7 @@ public: vm_prot_t prot); bool RestoreProtections(); bool GetRegionForAddress(nub_addr_t addr); + std::vector GetMemoryTypes() const; uint32_t GetDNBPermissions() const; diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index b28b6f3..b70c7ae 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -4310,6 +4310,14 @@ rnb_err_t RNBRemote::HandlePacket_MemoryRegionInfo(const char *p) { } } ostrm << ";"; + if (!region_info.vm_types.empty()) { + for (size_t i = 0; i < region_info.vm_types.size(); i++) { + if (i) + ostrm << ","; + ostrm << region_info.vm_types[i]; + } + ostrm << ";"; + } } return SendPacket(ostrm.str()); } -- 2.7.4