diff --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt --- a/lldb/docs/lldb-gdb-remote.txt +++ b/lldb/docs/lldb-gdb-remote.txt @@ -1206,6 +1206,9 @@ // 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 --- a/lldb/include/lldb/Target/MemoryRegionInfo.h +++ b/lldb/include/lldb/Target/MemoryRegionInfo.h @@ -28,10 +28,10 @@ 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 @@ 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 @@ 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 @@ 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 --- a/lldb/include/lldb/lldb-enumerations.h +++ b/lldb/include/lldb/lldb-enumerations.h @@ -1137,6 +1137,7 @@ eSaveCoreUnspecified = 0, eSaveCoreFull = 1, eSaveCoreDirtyOnly = 2, + eSaveCoreStackOnly = 3, }; } // namespace lldb diff --git a/lldb/source/Commands/CommandObjectProcess.cpp b/lldb/source/Commands/CommandObjectProcess.cpp --- a/lldb/source/Commands/CommandObjectProcess.cpp +++ b/lldb/source/Commands/CommandObjectProcess.cpp @@ -1166,7 +1166,9 @@ 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 @@ 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 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -6325,13 +6325,17 @@ // 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()); @@ -6346,10 +6350,12 @@ 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; @@ -6491,12 +6497,9 @@ 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(); @@ -6551,13 +6554,23 @@ 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; @@ -6600,6 +6613,12 @@ 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); @@ -6750,7 +6769,8 @@ 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 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -1572,6 +1572,18 @@ } } } + } 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); + } + } + 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 --- 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 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 --- /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 --- /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 --- /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 --- a/lldb/tools/debugserver/source/DNBDefs.h +++ b/lldb/tools/debugserver/source/DNBDefs.h @@ -318,11 +318,13 @@ struct DNBRegionInfo { public: - DNBRegionInfo() : addr(0), size(0), permissions(0), dirty_pages() {} + DNBRegionInfo() + : addr(0), size(0), permissions(0), dirty_pages(), is_stack(false) {} nub_addr_t addr; nub_addr_t size; uint32_t permissions; std::vector dirty_pages; + bool is_stack; }; enum DNBProfileDataScanType { diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp b/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp --- a/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp +++ b/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp @@ -125,6 +125,8 @@ region_info->permissions = vmRegion.GetDNBPermissions(); region_info->dirty_pages = get_dirty_pages(task, vmRegion.StartAddress(), vmRegion.GetByteSize()); + if (vmRegion.GetIsStackMemory()) + region_info->is_stack = true; } else { region_info->addr = address; region_info->size = 0; diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h --- a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h +++ b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h @@ -40,6 +40,7 @@ vm_prot_t prot); bool RestoreProtections(); bool GetRegionForAddress(nub_addr_t addr); + bool GetIsStackMemory() const; uint32_t GetDNBPermissions() const; diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp --- a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp +++ b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp @@ -182,3 +182,11 @@ dnb_permissions |= eMemoryPermissionsExecutable; return dnb_permissions; } + +bool MachVMRegion::GetIsStackMemory() const { + // VM_MEMORY_STACK and VM_PROT_NONE is the STACK GUARD region. + if (m_data.user_tag == VM_MEMORY_STACK && m_data.protection != VM_PROT_NONE) + return true; + else + return false; +} diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -4310,6 +4310,8 @@ } } ostrm << ";"; + if (region_info.is_stack) + ostrm << "type:stack;"; } return SendPacket(ostrm.str()); }