[Lldb-commits] [lldb] ee58200 - [LLDB] Properly return errors from "memory region --all"

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-09-23T12:32:12Z
New Revision: ee582001bf19be8611257df7c5fc5a5e7e7da586

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

LOG: [LLDB] Properly return errors from "memory region --all"

When I wrote the initial version I forgot that a region being
unmapped is not an error. There are real errors that we don't
want to hide, such as the remote not supporting the
qMemoryRegionInfo packet (gdbserver does not).

Reviewed By: labath

Differential Revision: https://reviews.llvm.org/D134029

Added: 
    

Modified: 
    lldb/source/Commands/CommandObjectMemory.cpp
    lldb/test/API/functionalities/memory-region/TestMemoryRegion.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index 22483e7c597c7..33c40a9ed8902 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -1830,9 +1830,6 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
           addr = region_info.GetRange().GetRangeEnd();
         }
       }
-
-      // 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);

diff  --git a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
index 66e75e0905aed..5e4c80bbb5767 100644
--- a/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ b/lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -8,6 +8,8 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
+from lldbsuite.test.gdbclientutils import MockGDBServerResponder
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
 
 
 class MemoryCommandRegion(TestBase):
@@ -126,3 +128,36 @@ def test_no_overlapping_regions(self):
 
                 previous_base = region_base
                 previous_end = region_end
+
+class MemoryCommandRegionAll(GDBRemoteTestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_all_error(self):
+        # The --all option should keep looping until the end of the memory range.
+        # If there is an error it should be reported as if you were just asking
+        # for one region. In this case the error is the remote not supporting
+        # qMemoryRegionInfo.
+        # (a region being unmapped is not an error, we just get a result
+        # describing an unmapped range)
+        class MyResponder(MockGDBServerResponder):
+            def qMemoryRegionInfo(self, addr):
+                # Empty string means unsupported.
+                return ""
+
+        self.server.responder = MyResponder()
+        target = self.dbg.CreateTarget('')
+        if self.TraceOn():
+            self.runCmd("log enable gdb-remote packets")
+            self.addTearDownHook(
+                  lambda: self.runCmd("log disable gdb-remote packets"))
+
+        process = self.connect(target)
+        lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
+                                      [lldb.eStateStopped])
+
+        interp = self.dbg.GetCommandInterpreter()
+        result = lldb.SBCommandReturnObject()
+        interp.HandleCommand("memory region --all ", result)
+        self.assertFalse(result.Succeeded())
+        self.assertEqual(result.GetError(),
+                    "error: qMemoryRegionInfo is not supported\n")
\ No newline at end of file