Skip to content

Conversation

@ZequanWu
Copy link
Contributor

@ZequanWu ZequanWu commented Apr 4, 2024

SBProcess::GetMemoryRegionInfo uses qMemoryRegionInfo packet to get memory region info, but this is not supported in gdb-server and causing downstream lldb test failures. This change ignores the the error from SBProcess::GetMemoryRegionInfo .

Reported by @tedwoodward @jerinphilip.

@ZequanWu ZequanWu requested a review from clayborg April 4, 2024 16:11
@ZequanWu ZequanWu requested a review from JDevlieghere as a code owner April 4, 2024 16:11
@llvmbot llvmbot added the lldb label Apr 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 4, 2024

@llvm/pr-subscribers-lldb

Author: Zequan Wu (ZequanWu)

Changes

SBProcess::GetMemoryRegionInfo uses qMemoryRegionInfo packet to get memory region info, but this is not supported in gdb-server and causing downstream lldb test failures. This change ignores the the error from SBProcess::GetMemoryRegionInfo .

Reported by @tedwoodward @jerinphilip.


Full diff: https://github.com/llvm/llvm-project/pull/87649.diff

1 Files Affected:

  • (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+15-19)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 55f8c920e60016..4b1b328c1bd561 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -2769,32 +2769,28 @@ void request_dataBreakpointInfo(const llvm::json::Object &request) {
                                           : "evaluation failed");
     } else {
       uint64_t load_addr = value.GetValueAsUnsigned();
-      addr = llvm::utohexstr(load_addr);
-      lldb::SBMemoryRegionInfo region;
-      lldb::SBError err =
-          g_dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region);
-      if (err.Success()) {
-        if (!(region.IsReadable() || region.IsWritable())) {
-          body.try_emplace("dataId", nullptr);
-          body.try_emplace("description",
-                           "memory region for address " + addr +
-                               " has no read or write permissions");
-        } else {
-          lldb::SBData data = value.GetPointeeData();
-          if (data.IsValid())
-            size = llvm::utostr(data.GetByteSize());
-          else {
+      lldb::SBData data = value.GetPointeeData();
+      if (data.IsValid()) {
+        size = llvm::utostr(data.GetByteSize());
+        addr = llvm::utohexstr(load_addr);
+        lldb::SBMemoryRegionInfo region;
+        lldb::SBError err =
+            g_dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region);
+        // Only lldb-server supports "qMemoryRegionInfo". So, don't fail this
+        // request if SBProcess::GetMemoryRegionInfo returns error.
+        if (err.Success()) {
+          if (!(region.IsReadable() || region.IsWritable())) {
             body.try_emplace("dataId", nullptr);
             body.try_emplace("description",
-                             "unable to get byte size for expression: " +
-                                 name.str());
+                             "memory region for address " + addr +
+                                 " has no read or write permissions");
           }
         }
       } else {
         body.try_emplace("dataId", nullptr);
         body.try_emplace("description",
-                         "unable to get memory region info for address " +
-                             addr);
+                         "unable to get byte size for expression: " +
+                             name.str());
       }
     }
   } else {

@ZequanWu
Copy link
Contributor Author

Ping.

@ZequanWu ZequanWu requested a review from walter-erquinigo May 2, 2024 18:00
Copy link
Member

@walter-erquinigo walter-erquinigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense to me

@ZequanWu ZequanWu merged commit 7b040d0 into llvm:main May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants