[Lldb-commits] [PATCH] D87694: [lldb] Don't send invalid region addresses to lldb server

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a reviewer: JDevlieghere.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
DavidSpickett requested review of this revision.

Previously when <addr> in "memory region <addr>" didn't
parse correctly, we'd print an error then also ask lldb-server
for a region containing LLDB_INVALID_ADDRESS.

(lldb) memory region not_an_address
error: invalid address argument "not_an_address"...
error: Server returned invalid range

Only send the command to lldb-server if the address
parsed correctly.

(lldb) memory region not_an_address
error: invalid address argument "not_an_address"...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D87694

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


Index: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
===================================================================
--- lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
+++ lldb/test/API/functionalities/memory-region/TestMemoryRegion.py
@@ -41,6 +41,16 @@
         self.assertFalse(result.Succeeded())
         self.assertRegexpMatches(result.GetError(), "Usage: memory region ADDR")
 
+        # Test that when the address fails to parse, we do not carry on
+        # and ask lldb-server for an invalid address
+        interp.HandleCommand("memory region not_an_address", result)
+        self.assertFalse(result.Succeeded())
+        self.assertRegexpMatches(result.GetError(),
+                "error: invalid address argument \"not_an_address\"")
+        # This would be found if we carried on despite the error
+        self.assertNotRegexpMatches(result.GetError(),
+                "error: Server returned invalid range")
+
         # Now let's print the memory region starting at 0 which should always work.
         interp.HandleCommand("memory region 0x0", result)
         self.assertTrue(result.Succeeded())
Index: lldb/source/Commands/CommandObjectMemory.cpp
===================================================================
--- lldb/source/Commands/CommandObjectMemory.cpp
+++ lldb/source/Commands/CommandObjectMemory.cpp
@@ -1710,32 +1710,34 @@
           }
         }
 
-        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();
+        if (load_addr != LLDB_INVALID_ADDRESS) {
+          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();
+              }
             }
+            result.AppendMessageWithFormatv(
+                "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}\n",
+                range_info.GetRange().GetRangeBase(),
+                range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
+                range_info.GetWritable(), range_info.GetExecutable(),
+                name ? " " : "", name, section_name ? " " : "", section_name);
+            m_prev_end_addr = range_info.GetRange().GetRangeEnd();
+            result.SetStatus(eReturnStatusSuccessFinishResult);
+          } else {
+            result.SetStatus(eReturnStatusFailed);
+            result.AppendErrorWithFormat("%s\n", error.AsCString());
           }
-          result.AppendMessageWithFormatv(
-              "[{0:x16}-{1:x16}) {2:r}{3:w}{4:x}{5}{6}{7}{8}\n",
-              range_info.GetRange().GetRangeBase(),
-              range_info.GetRange().GetRangeEnd(), range_info.GetReadable(),
-              range_info.GetWritable(), range_info.GetExecutable(),
-              name ? " " : "", name, section_name ? " " : "", section_name);
-          m_prev_end_addr = range_info.GetRange().GetRangeEnd();
-          result.SetStatus(eReturnStatusSuccessFinishResult);
-        } else {
-          result.SetStatus(eReturnStatusFailed);
-          result.AppendErrorWithFormat("%s\n", error.AsCString());
         }
       }
     } else {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87694.291921.patch
Type: text/x-patch
Size: 4254 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200915/c323290d/attachment.bin>