jasonmolenda updated this revision to Diff 364731.
jasonmolenda added a comment.
Handle a bit of state tracking in ObjectFileMachO a little bit more readably.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107625/new/
https://reviews.llvm.org/D107625
Files:
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Target/MemoryRegionInfo.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
lldb/test/API/macosx/stack-corefile/Makefile
lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
lldb/test/API/macosx/stack-corefile/main.c
lldb/tools/debugserver/source/DNBDefs.h
lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
lldb/tools/debugserver/source/RNBRemote.cpp
Index: lldb/tools/debugserver/source/RNBRemote.cpp
===================================================================
--- lldb/tools/debugserver/source/RNBRemote.cpp
+++ lldb/tools/debugserver/source/RNBRemote.cpp
@@ -4310,6 +4310,8 @@
}
}
ostrm << ";";
+ if (region_info.is_stack)
+ ostrm << "type:stack;";
}
return SendPacket(ostrm.str());
}
Index: lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
===================================================================
--- lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
+++ 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;
Index: lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
===================================================================
--- lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
+++ 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;
+}
Index: lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
===================================================================
--- lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
+++ 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;
Index: lldb/tools/debugserver/source/DNBDefs.h
===================================================================
--- lldb/tools/debugserver/source/DNBDefs.h
+++ 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<nub_addr_t> dirty_pages;
+ bool is_stack;
};
enum DNBProfileDataScanType {
Index: lldb/test/API/macosx/stack-corefile/main.c
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/stack-corefile/main.c
@@ -0,0 +1,15 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+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;
+}
Index: lldb/test/API/macosx/stack-corefile/TestStackCorefile.py
===================================================================
--- /dev/null
+++ 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, '""')
Index: lldb/test/API/macosx/stack-corefile/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/stack-corefile/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES = main.c
+
+include Makefile.rules
Index: lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
===================================================================
--- lldb/test/API/macosx/skinny-corefile/TestSkinnyCorefile.py
+++ 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__)
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ 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;
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ 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<std::string> 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<std::vector<addr_t>> &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
Index: lldb/source/Commands/CommandObjectProcess.cpp
===================================================================
--- lldb/source/Commands/CommandObjectProcess.cpp
+++ 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"
Index: lldb/include/lldb/lldb-enumerations.h
===================================================================
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -1137,6 +1137,7 @@
eSaveCoreUnspecified = 0,
eSaveCoreFull = 1,
eSaveCoreDirtyOnly = 2,
+ eSaveCoreStackOnly = 3,
};
} // namespace lldb
Index: lldb/include/lldb/Target/MemoryRegionInfo.h
===================================================================
--- lldb/include/lldb/Target/MemoryRegionInfo.h
+++ 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<lldb::addr_t> 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<std::vector<lldb::addr_t>> m_dirty_pages;
};
Index: lldb/docs/lldb-gdb-remote.txt
===================================================================
--- lldb/docs/lldb-gdb-remote.txt
+++ 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:[<type>][,<type>]; // memory types that apply to this region, e.g.
+ // "stack" for stack memory.
+
error:<ascii-byte-error-string>; // where <ascii-byte-error-string> is
// a hex encoded string value that
// contains an error string
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
