[llvm] 13e1cf8 - Reland "[lldb] Add --all option to "memory region""

David Spickett via llvm-commits llvm-commits at lists.llvm.org
Thu May 19 05:16:50 PDT 2022


Author: David Spickett
Date: 2022-05-19T13:16:36+01:00
New Revision: 13e1cf806567bc4987817e14a774198c3e3f2709

URL: https://github.com/llvm/llvm-project/commit/13e1cf806567bc4987817e14a774198c3e3f2709
DIFF: https://github.com/llvm/llvm-project/commit/13e1cf806567bc4987817e14a774198c3e3f2709.diff

LOG: Reland "[lldb] Add --all option to "memory region""

This reverts commit 3e928c4b9dfb01efd2cb968795e605760828e873.

This fixes an issue seen on Windows where we did not properly
get the section names of regions if they overlapped. Windows
has regions like:
[0x00007fff928db000-0x00007fff949a0000) ---
[0x00007fff949a0000-0x00007fff949a1000) r-- PECOFF header
[0x00007fff949a0000-0x00007fff94a3d000) r-x .hexpthk
[0x00007fff949a0000-0x00007fff94a85000) r-- .rdata
[0x00007fff949a0000-0x00007fff94a88000) rw- .data
[0x00007fff949a0000-0x00007fff94a94000) r-- .pdata
[0x00007fff94a94000-0x00007fff95250000) ---

I assumed that you could just resolve the address and get the section
name using the start of the region but here you'd always get
"PECOFF header" because they all have the same start point.

The usual command repeating loop used the end address of the previous
region when requesting the next, or getting the section name.
So I've matched this in the --all scenario.

In the example above, somehow asking for the region at
0x00007fff949a1000 would get you a region that starts at
0x00007fff949a0000 but has a different end point. Using the load
address you get (what I assume is) the correct section name.

Added: 
    

Modified: 
    lldb/source/Commands/CommandObjectMemory.cpp
    lldb/source/Commands/Options.td
    lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
    lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
    llvm/docs/ReleaseNotes.rst

Removed: 
    


################################################################################
diff  --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index f13f749da2d21..7005db3088069 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -1643,19 +1643,113 @@ class CommandObjectMemoryHistory : public CommandObjectParsed {
 // CommandObjectMemoryRegion
 #pragma mark CommandObjectMemoryRegion
 
+#define LLDB_OPTIONS_memory_region
+#include "CommandOptions.inc"
+
 class CommandObjectMemoryRegion : public CommandObjectParsed {
 public:
+  class OptionGroupMemoryRegion : public OptionGroup {
+  public:
+    OptionGroupMemoryRegion() : OptionGroup(), m_all(false, false) {}
+
+    ~OptionGroupMemoryRegion() override = default;
+
+    llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
+      return llvm::makeArrayRef(g_memory_region_options);
+    }
+
+    Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_value,
+                          ExecutionContext *execution_context) override {
+      Status status;
+      const int short_option = g_memory_region_options[option_idx].short_option;
+
+      switch (short_option) {
+      case 'a':
+        m_all.SetCurrentValue(true);
+        m_all.SetOptionWasSet();
+        break;
+      default:
+        llvm_unreachable("Unimplemented option");
+      }
+
+      return status;
+    }
+
+    void OptionParsingStarting(ExecutionContext *execution_context) override {
+      m_all.Clear();
+    }
+
+    OptionValueBoolean m_all;
+  };
+
   CommandObjectMemoryRegion(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "memory region",
                             "Get information on the memory region containing "
                             "an address in the current target process.",
-                            "memory region ADDR",
+                            "memory region <address-expression> (or --all)",
                             eCommandRequiresProcess | eCommandTryTargetAPILock |
-                                eCommandProcessMustBeLaunched) {}
+                                eCommandProcessMustBeLaunched) {
+    // Address in option set 1.
+    m_arguments.push_back(CommandArgumentEntry{CommandArgumentData(
+        eArgTypeAddressOrExpression, eArgRepeatPlain, LLDB_OPT_SET_1)});
+    // "--all" will go in option set 2.
+    m_option_group.Append(&m_memory_region_options);
+    m_option_group.Finalize();
+  }
 
   ~CommandObjectMemoryRegion() override = default;
 
+  Options *GetOptions() override { return &m_option_group; }
+
 protected:
+  void DumpRegion(CommandReturnObject &result, Target &target,
+                  const MemoryRegionInfo &range_info, lldb::addr_t load_addr) {
+    lldb_private::Address addr;
+    ConstString section_name;
+    if (target.ResolveLoadAddress(load_addr, addr)) {
+      SectionSP section_sp(addr.GetSection());
+      if (section_sp) {
+        // Got the top most section, not the deepest section
+        while (section_sp->GetParent())
+          section_sp = section_sp->GetParent();
+        section_name = section_sp->GetName();
+      }
+    }
+
+    ConstString name = range_info.GetName();
+    result.AppendMessageWithFormatv(
+        "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}",
+        range_info.GetRange().GetRangeBase(),
+        range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
+        range_info.GetWritable(), range_info.GetExecutable(), name ? " " : "",
+        name, section_name ? " " : "", section_name);
+    MemoryRegionInfo::OptionalBool memory_tagged = range_info.GetMemoryTagged();
+    if (memory_tagged == MemoryRegionInfo::OptionalBool::eYes)
+      result.AppendMessage("memory tagging: enabled");
+
+    const llvm::Optional<std::vector<addr_t>> &dirty_page_list =
+        range_info.GetDirtyPageList();
+    if (dirty_page_list.hasValue()) {
+      const size_t page_count = dirty_page_list.getValue().size();
+      result.AppendMessageWithFormat(
+          "Modified memory (dirty) page list provided, %zu entries.\n",
+          page_count);
+      if (page_count > 0) {
+        bool print_comma = false;
+        result.AppendMessageWithFormat("Dirty pages: ");
+        for (size_t i = 0; i < page_count; i++) {
+          if (print_comma)
+            result.AppendMessageWithFormat(", ");
+          else
+            print_comma = true;
+          result.AppendMessageWithFormat("0x%" PRIx64,
+                                         dirty_page_list.getValue()[i]);
+        }
+        result.AppendMessageWithFormat(".\n");
+      }
+    }
+  }
+
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     ProcessSP process_sp = m_exe_ctx.GetProcessSP();
     if (!process_sp) {
@@ -1672,6 +1766,13 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
     const lldb::ABISP &abi = process_sp->GetABI();
 
     if (argc == 1) {
+      if (m_memory_region_options.m_all) {
+        result.AppendError(
+            "The \"--all\" option cannot be used when an address "
+            "argument is given");
+        return false;
+      }
+
       auto load_addr_str = command[0].ref();
       // Non-address bits in this will be handled later by GetMemoryRegion
       load_addr = OptionArgParser::ToAddress(&m_exe_ctx, load_addr_str,
@@ -1685,67 +1786,58 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
                // When we're repeating the command, the previous end address is
                // used for load_addr. If that was 0xF...F then we must have
                // reached the end of memory.
-               (argc == 0 && load_addr == LLDB_INVALID_ADDRESS) ||
+               (argc == 0 && !m_memory_region_options.m_all &&
+                load_addr == LLDB_INVALID_ADDRESS) ||
                // If the target has non-address bits (tags, limited virtual
                // address size, etc.), the end of mappable memory will be lower
                // than that. So if we find any non-address bit set, we must be
                // at the end of the mappable range.
                (abi && (abi->FixAnyAddress(load_addr) != load_addr))) {
-      result.AppendErrorWithFormat("'%s' takes one argument:\nUsage: %s\n",
-                                   m_cmd_name.c_str(), m_cmd_syntax.c_str());
+      result.AppendErrorWithFormat(
+          "'%s' takes one argument or \"--all\" option:\nUsage: %s\n",
+          m_cmd_name.c_str(), m_cmd_syntax.c_str());
       return false;
     }
 
-    lldb_private::MemoryRegionInfo range_info;
-    error = process_sp->GetMemoryRegionInfo(load_addr, range_info);
-    if (error.Success()) {
-      lldb_private::Address addr;
-      ConstString name = range_info.GetName();
-      ConstString section_name;
-      if (process_sp->GetTarget().ResolveLoadAddress(load_addr, addr)) {
-        SectionSP section_sp(addr.GetSection());
-        if (section_sp) {
-          // Got the top most section, not the deepest section
-          while (section_sp->GetParent())
-            section_sp = section_sp->GetParent();
-          section_name = section_sp->GetName();
+    // Is is important that we track the address used to request the region as
+    // this will give the correct section name in the case that regions overlap.
+    // On Windows we get mutliple regions that start at the same place but are
+    // 
diff erent sizes and refer to 
diff erent sections.
+    std::vector<std::pair<lldb_private::MemoryRegionInfo, lldb::addr_t>>
+        region_list;
+    if (m_memory_region_options.m_all) {
+      // We don't use GetMemoryRegions here because it doesn't include unmapped
+      // areas like repeating the command would. So instead, emulate doing that.
+      lldb::addr_t addr = 0;
+      while (error.Success() && addr != LLDB_INVALID_ADDRESS &&
+             // When there are non-address bits the last range will not extend
+             // to LLDB_INVALID_ADDRESS but to the max virtual address.
+             // This prevents us looping forever if that is the case.
+             (abi && (abi->FixAnyAddress(addr) == addr))) {
+        lldb_private::MemoryRegionInfo region_info;
+        error = process_sp->GetMemoryRegionInfo(addr, region_info);
+
+        if (error.Success()) {
+          region_list.push_back({region_info, addr});
+          addr = region_info.GetRange().GetRangeEnd();
         }
       }
 
-      result.AppendMessageWithFormatv(
-          "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}",
-          range_info.GetRange().GetRangeBase(),
-          range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
-          range_info.GetWritable(), range_info.GetExecutable(), name ? " " : "",
-          name, section_name ? " " : "", section_name);
-      MemoryRegionInfo::OptionalBool memory_tagged =
-          range_info.GetMemoryTagged();
-      if (memory_tagged == MemoryRegionInfo::OptionalBool::eYes)
-        result.AppendMessage("memory tagging: enabled");
-
-      const llvm::Optional<std::vector<addr_t>> &dirty_page_list =
-          range_info.GetDirtyPageList();
-      if (dirty_page_list.hasValue()) {
-        const size_t page_count = dirty_page_list.getValue().size();
-        result.AppendMessageWithFormat(
-            "Modified memory (dirty) page list provided, %zu entries.\n",
-            page_count);
-        if (page_count > 0) {
-          bool print_comma = false;
-          result.AppendMessageWithFormat("Dirty pages: ");
-          for (size_t i = 0; i < page_count; i++) {
-            if (print_comma)
-              result.AppendMessageWithFormat(", ");
-            else
-              print_comma = true;
-            result.AppendMessageWithFormat("0x%" PRIx64,
-                                           dirty_page_list.getValue()[i]);
-          }
-          result.AppendMessageWithFormat(".\n");
-        }
+      // Even if we read nothing, don't error for --all
+      error.Clear();
+    } else {
+      lldb_private::MemoryRegionInfo region_info;
+      error = process_sp->GetMemoryRegionInfo(load_addr, region_info);
+      if (error.Success())
+        region_list.push_back({region_info, load_addr});
+    }
+
+    if (error.Success()) {
+      for (std::pair<MemoryRegionInfo, addr_t> &range : region_list) {
+        DumpRegion(result, process_sp->GetTarget(), range.first, range.second);
+        m_prev_end_addr = range.first.GetRange().GetRangeEnd();
       }
 
-      m_prev_end_addr = range_info.GetRange().GetRangeEnd();
       result.SetStatus(eReturnStatusSuccessFinishResult);
       return true;
     }
@@ -1762,6 +1854,9 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
   }
 
   lldb::addr_t m_prev_end_addr = LLDB_INVALID_ADDRESS;
+
+  OptionGroupOptions m_option_group;
+  OptionGroupMemoryRegion m_memory_region_options;
 };
 
 // CommandObjectMemory

diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 6b17b766a0e2e..c326f8a320748 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -513,6 +513,12 @@ let Command = "memory write" in {
     Desc<"Start writing bytes from an offset within the input file.">;
 }
 
+let Command = "memory region" in {
+  def memory_region_all : Option<"all", "a">, Group<2>, Required,
+    Desc<"Show all memory regions. This is equivalent to starting from address "
+         "0 and repeating the command. Unmapped areas are included.">;
+}
+
 let Command = "memory tag write" in {
   def memory_write_end_addr : Option<"end-addr", "e">, Group<1>,
   Arg<"AddressOrExpression">, Desc<

diff  --git a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
index 61e64d44e7945..cad54c8f892f2 100644
--- a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -23,6 +23,12 @@ def setUp(self):
             'main.cpp',
             '// Run here before printing memory regions')
 
+    def test_help(self):
+        """ Test that help shows you must have one of address or --all, not both."""
+        self.expect("help memory region",
+            substrs=["memory region <address-expression>",
+                     "memory region -a"])
+
     def test(self):
         self.build()
 
@@ -39,7 +45,15 @@ def test(self):
         # Test that the first 'memory region' command prints the usage.
         interp.HandleCommand("memory region", result)
         self.assertFalse(result.Succeeded())
-        self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+        self.assertEqual(result.GetError(),
+                    "error: 'memory region' takes one argument or \"--all\" option:\n"
+                    "Usage: memory region <address-expression> (or --all)\n")
+
+        # We allow --all or an address argument, not both
+        interp.HandleCommand("memory region --all 0", result)
+        self.assertFalse(result.Succeeded())
+        self.assertRegexpMatches(result.GetError(),
+                "The \"--all\" option cannot be used when an address argument is given")
 
         # Test that when the address fails to parse, we show an error and do not continue
         interp.HandleCommand("memory region not_an_address", result)
@@ -47,18 +61,28 @@ def test(self):
         self.assertEqual(result.GetError(),
                 "error: invalid address argument \"not_an_address\": address expression \"not_an_address\" evaluation failed\n")
 
+        # Accumulate the results to compare with the --all output
+        all_regions = ""
+
         # Now let's print the memory region starting at 0 which should always work.
         interp.HandleCommand("memory region 0x0", result)
         self.assertTrue(result.Succeeded())
         self.assertRegexpMatches(result.GetOutput(), "\\[0x0+-")
+        all_regions += result.GetOutput()
 
         # Keep printing memory regions until we printed all of them.
         while True:
             interp.HandleCommand("memory region", result)
             if not result.Succeeded():
                 break
+            all_regions += result.GetOutput()
 
         # Now that we reached the end, 'memory region' should again print the usage.
         interp.HandleCommand("memory region", result)
         self.assertFalse(result.Succeeded())
-        self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+        self.assertRegexpMatches(result.GetError(), "Usage: memory region <address\-expression> \(or \-\-all\)")
+
+        # --all should match what repeating the command gives you
+        interp.HandleCommand("memory region --all", result)
+        self.assertTrue(result.Succeeded())
+        self.assertEqual(result.GetOutput(), all_regions)

diff  --git a/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py b/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
index 0b5f341574e00..cbd772ecebbc6 100644
--- a/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
+++ b/lldb/test/API/linux/aarch64/tagged_memory_region/TestAArch64LinuxTaggedMemoryRegion.py
@@ -55,7 +55,7 @@ def test_mte_regions(self):
             if result.Succeeded():
                 repeats += 1
             else:
-                self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
+                self.assertRegexpMatches(result.GetError(), "Usage: memory region")
                 break
 
         # This time repeat until we get the last region. At that

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 8529bb76e13e3..101cec511792a 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -172,6 +172,11 @@ Changes to the LLVM tools
 Changes to LLDB
 ---------------------------------
 
+* The "memory region" command now has a "--all" option to list all
+  memory regions (including unmapped ranges). This is the equivalent
+  of using address 0 then repeating the command until all regions
+  have been listed.
+
 Changes to Sanitizers
 ---------------------
 


        


More information about the llvm-commits mailing list