[Lldb-commits] [lldb] [LLDB][Minidump] Fix ProcessMinidump::GetMemoryRegions to include 64b regions when /proc/pid maps are missing. (PR #101086)

Jacob Lalonde via lldb-commits lldb-commits at lists.llvm.org
Thu Aug 15 14:37:23 PDT 2024


https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/101086

>From 2860a75fdc243f55d1e675068f9d120fb43cb21d Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Fri, 26 Jul 2024 14:14:42 -0700
Subject: [PATCH] Remove 64b specific method and create Cache from both memory
 32 and memory 64.

Add Mem64List to Obj2Yaml and Yaml2Minidump

Fix yaml2obj minidump emission where I was passing a stack variable by reference. Change CreateRegionsCache to not double evaluate Expected<T> as it was causing unchecked to flip back to true instead slotting into an optional.
Uncomment python tests"

Run C++ and Python formatters.

Revert unchanged files.

Modify test yaml to show support for multiple mem64 entries

Remove llvm related components because they were changed in a different patch
---
 .../Minidump/MinidumpFileBuilder.cpp          |  13 ++--
 .../Process/minidump/MinidumpParser.cpp       |  58 +++++++++---------
 .../minidump-new/TestMiniDumpNew.py           |  19 ++++++
 .../minidump-new/linux-x86_64_mem64.dmp       | Bin 0 -> 860 bytes
 .../minidump-new/linux-x86_64_mem64.yaml      |  40 ++++++++++++
 5 files changed, 96 insertions(+), 34 deletions(-)
 create mode 100644 lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.dmp
 create mode 100644 lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml

diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index de212c6b20da7e..b941748bd7ccbb 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -1020,15 +1020,17 @@ MinidumpFileBuilder::AddMemoryList_32(Process::CoreFileMemoryRanges &ranges) {
   // With a size of the number of ranges as a 32 bit num
   // And then the size of all the ranges
   error = AddDirectory(StreamType::MemoryList,
-                       sizeof(llvm::support::ulittle32_t) +
+                       sizeof(llvm::minidump::MemoryListHeader) +
                            descriptors.size() *
                                sizeof(llvm::minidump::MemoryDescriptor));
   if (error.Fail())
     return error;
 
+  llvm::minidump::MemoryListHeader list_header;
   llvm::support::ulittle32_t memory_ranges_num =
       static_cast<llvm::support::ulittle32_t>(descriptors.size());
-  m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle32_t));
+  list_header.NumberOfMemoryRanges = memory_ranges_num;
+  m_data.AppendData(&list_header, sizeof(llvm::minidump::MemoryListHeader));
   // For 32b we can get away with writing off the descriptors after the data.
   // This means no cleanup loop needed.
   m_data.AppendData(descriptors.data(),
@@ -1050,9 +1052,10 @@ MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
   if (error.Fail())
     return error;
 
+  llvm::minidump::Memory64ListHeader list_header;
   llvm::support::ulittle64_t memory_ranges_num =
       static_cast<llvm::support::ulittle64_t>(ranges.size());
-  m_data.AppendData(&memory_ranges_num, sizeof(llvm::support::ulittle64_t));
+  list_header.NumberOfMemoryRanges = memory_ranges_num;
   // Capture the starting offset for all the descriptors so we can clean them up
   // if needed.
   offset_t starting_offset =
@@ -1064,8 +1067,8 @@ MinidumpFileBuilder::AddMemoryList_64(Process::CoreFileMemoryRanges &ranges) {
       (ranges.size() * sizeof(llvm::minidump::MemoryDescriptor_64));
   llvm::support::ulittle64_t memory_ranges_base_rva =
       static_cast<llvm::support::ulittle64_t>(base_rva);
-  m_data.AppendData(&memory_ranges_base_rva,
-                    sizeof(llvm::support::ulittle64_t));
+  list_header.BaseRVA = memory_ranges_base_rva;
+  m_data.AppendData(&list_header, sizeof(llvm::minidump::Memory64ListHeader));
 
   bool cleanup_required = false;
   std::vector<MemoryDescriptor_64> descriptors;
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index be9fae938e2276..7e19acd3d08347 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -553,43 +553,45 @@ static bool
 CreateRegionsCacheFromMemoryList(MinidumpParser &parser,
                                  std::vector<MemoryRegionInfo> &regions) {
   Log *log = GetLog(LLDBLog::Modules);
+  // Cache the expected memory32 into an optional
+  // because double checking the expected triggers the unchecked warning.
+  std::optional<llvm::ArrayRef<MemoryDescriptor>> memory32_list;
   auto ExpectedMemory = parser.GetMinidumpFile().getMemoryList();
   if (!ExpectedMemory) {
     LLDB_LOG_ERROR(log, ExpectedMemory.takeError(),
                    "Failed to read memory list: {0}");
-    return false;
-  }
-  regions.reserve(ExpectedMemory->size());
-  for (const MemoryDescriptor &memory_desc : *ExpectedMemory) {
-    if (memory_desc.Memory.DataSize == 0)
-      continue;
-    MemoryRegionInfo region;
-    region.GetRange().SetRangeBase(memory_desc.StartOfMemoryRange);
-    region.GetRange().SetByteSize(memory_desc.Memory.DataSize);
-    region.SetReadable(MemoryRegionInfo::eYes);
-    region.SetMapped(MemoryRegionInfo::eYes);
-    regions.push_back(region);
+  } else {
+    memory32_list = *ExpectedMemory;
   }
-  regions.shrink_to_fit();
-  return !regions.empty();
-}
 
-static bool
-CreateRegionsCacheFromMemory64List(MinidumpParser &parser,
-                                   std::vector<MemoryRegionInfo> &regions) {
+  size_t num_regions = memory32_list ? memory32_list->size() : 0;
+
   llvm::ArrayRef<uint8_t> data =
       parser.GetStream(StreamType::Memory64List);
-  if (data.empty())
-    return false;
+
   llvm::ArrayRef<MinidumpMemoryDescriptor64> memory64_list;
-  uint64_t base_rva;
-  std::tie(memory64_list, base_rva) =
-      MinidumpMemoryDescriptor64::ParseMemory64List(data);
+  if (!data.empty()) {
+    uint64_t base_rva;
+    std::tie(memory64_list, base_rva) =
+        MinidumpMemoryDescriptor64::ParseMemory64List(data);
 
-  if (memory64_list.empty())
-    return false;
+    num_regions += memory64_list.size();
+  }
+
+  regions.reserve(num_regions);
+  if (memory32_list) {
+    for (const MemoryDescriptor &memory_desc : *memory32_list) {
+      if (memory_desc.Memory.DataSize == 0)
+        continue;
+      MemoryRegionInfo region;
+      region.GetRange().SetRangeBase(memory_desc.StartOfMemoryRange);
+      region.GetRange().SetByteSize(memory_desc.Memory.DataSize);
+      region.SetReadable(MemoryRegionInfo::eYes);
+      region.SetMapped(MemoryRegionInfo::eYes);
+      regions.push_back(region);
+    }
+  }
 
-  regions.reserve(memory64_list.size());
   for (const auto &memory_desc : memory64_list) {
     if (memory_desc.data_size == 0)
       continue;
@@ -620,9 +622,7 @@ std::pair<MemoryRegionInfos, bool> MinidumpParser::BuildMemoryRegions() {
     return return_sorted(true);
   if (CreateRegionsCacheFromMemoryInfoList(*this, result))
     return return_sorted(true);
-  if (CreateRegionsCacheFromMemoryList(*this, result))
-    return return_sorted(false);
-  CreateRegionsCacheFromMemory64List(*this, result);
+  CreateRegionsCacheFromMemoryList(*this, result);
   return return_sorted(false);
 }
 
diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
index 91fd2439492b54..2de3e36b507341 100644
--- a/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
@@ -491,3 +491,22 @@ def test_minidump_sysroot(self):
         spec_dir_norm = os.path.normcase(module.GetFileSpec().GetDirectory())
         exe_dir_norm = os.path.normcase(exe_dir)
         self.assertEqual(spec_dir_norm, exe_dir_norm)
+
+    def test_minidump_memory64list(self):
+        """Test that lldb can read from the memory64list in a minidump."""
+        self.process_from_yaml("linux-x86_64_mem64.yaml")
+
+        region_count = 3
+        region_info_list = self.process.GetMemoryRegions()
+        self.assertEqual(region_info_list.GetSize(), region_count)
+
+        region = lldb.SBMemoryRegionInfo()
+        self.assertTrue(region_info_list.GetMemoryRegionAtIndex(0, region))
+        self.assertEqual(region.GetRegionBase(), 0x7FFF12A84030)
+        self.assertTrue(region.GetRegionEnd(), 0x7FFF12A84030 + 0x2FD0)
+        self.assertTrue(region_info_list.GetMemoryRegionAtIndex(1, region))
+        self.assertEqual(region.GetRegionBase(), 0x00007fff12a87000)
+        self.assertTrue(region.GetRegionEnd(), 0x00007fff12a87000 + 0x00000018)
+        self.assertTrue(region_info_list.GetMemoryRegionAtIndex(2, region))
+        self.assertEqual(region.GetRegionBase(), 0x00007fff12a87018)
+        self.assertTrue(region.GetRegionEnd(), 0x00007fff12a87018 + 0x00000400)
diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.dmp b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.dmp
new file mode 100644
index 0000000000000000000000000000000000000000..a1069ef45ad3543bcc2862fa8699b239c2fefa8e
GIT binary patch
literal 860
zcmeZu at eP=~oPmLffq_8*h|vKvP{0I;Er6I4h&$LA7;J!oj6nA8X&OKhKga<}UjP~o
zqDckp3<6*+|AByk0YpLospb$;zrkUJ(EoZus>qiQxUf8W7Y)rP-({mZDG7lJ|1VOk
XVGIm6(&az?t7l*|fTp7h`VdnAg=Zxs

literal 0
HcmV?d00001

diff --git a/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml
new file mode 100644
index 00000000000000..4a290355a863a1
--- /dev/null
+++ b/lldb/test/API/functionalities/postmortem/minidump-new/linux-x86_64_mem64.yaml
@@ -0,0 +1,40 @@
+--- !minidump
+Streams:
+  - Type:            SystemInfo
+    Processor Arch:  AMD64
+    Processor Level: 6
+    Processor Revision: 15876
+    Number of Processors: 40
+    Platform ID:     Linux
+    CSD Version:     'Linux 3.13.0-91-generic'
+    CPU:
+      Vendor ID:       GenuineIntel
+      Version Info:    0x00000000
+      Feature Info:    0x00000000
+  - Type:            LinuxProcStatus
+    Text:             |
+      Name:	linux-x86_64
+      State:	t (tracing stop)
+      Tgid:	29917
+      Ngid:	0
+      Pid:	29917
+      PPid:	29370
+      TracerPid:	29918
+      Uid:	1001	1001	1001	1001
+      Gid:	1001	1001	1001	1001
+  - Type:            ThreadList
+    Threads:
+      - Thread Id:       0x2896BB
+        Context:         0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000700100000000000FFFFFFFF0000FFFFFFFFFFFFFFFFFFFF0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B040A812FF7F00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000050D0A75BBA7F00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+        Stack:
+          Start of Memory Range: 0x0
+          Content:         ''
+  - Type:            Memory64List
+    Memory Ranges:
+      - Start of memory range: 0x7FFF12A84030
+        Data Size:       0x2FD0
+      - Start of memory range: 0x00007fff12a87000
+        Data Size:       0x00000018
+      - Start of memory range: 0x00007fff12a87018
+        Data Size:       0x00000400
+...



More information about the lldb-commits mailing list