[Lldb-commits] [lldb] [lldb-dap] Followup fixs for data breakpoints (PR #81680)

[Zequan Wu via lldb-commits]

https://github.com/ZequanWu updated https://github.com/llvm/llvm-project/pull/81680

>From f9c509c519c9937d2e2de5098c4241e936187527 Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Tue, 13 Feb 2024 17:41:26 -0500
Subject: [PATCH 1/2] [lldb-dap] Followup fixs for data breakpoints

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 6bf2ec28432cd3..a3c6109370e685 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -2741,8 +2741,20 @@ void request_dataBreakpointInfo(const llvm::json::Object &request) {
   std::string addr, size;
 
   if (variable.IsValid()) {
-    addr = llvm::utohexstr(variable.GetLoadAddress());
-    size = llvm::utostr(variable.GetByteSize());
+    lldb::addr_t load_addr = variable.GetLoadAddress();
+    size_t byte_size = variable.GetByteSize();
+    if (load_addr == LLDB_INVALID_ADDRESS) {
+      body.try_emplace("dataId", nullptr);
+      body.try_emplace("description",
+                       "does not exist in memory, its location is " +
+                           std::string(variable.GetLocation()));
+    } else if (byte_size == 0) {
+      body.try_emplace("dataId", nullptr);
+      body.try_emplace("description", "variable size is 0");
+    } else {
+      addr = llvm::utohexstr(load_addr);
+      size = llvm::utostr(byte_size);
+    }
   } else if (variablesReference == 0 && frame.IsValid()) {
     // Name might be an expression. In this case we assume that name is composed
     // of the number of bytes to watch and expression, separated by '@':
@@ -2757,13 +2769,18 @@ void request_dataBreakpointInfo(const llvm::json::Object &request) {
       body.try_emplace("description", error_cstr && error_cstr[0]
                                           ? std::string(error_cstr)
                                           : "evaluation failed");
-    } else
-      addr = llvm::utohexstr(value.GetValueAsUnsigned());
+    } else {
+      uint64_t value_as_unsigned = value.GetValueAsUnsigned();
+      if (value_as_unsigned == 0) {
+        body.try_emplace("dataId", nullptr);
+        body.try_emplace("description",
+                         "unable to evaluate expression to an address.");
+      }
+      addr = llvm::utohexstr(value_as_unsigned);
+    }
   } else {
-    auto state = g_dap.target.GetProcess().GetState();
     body.try_emplace("dataId", nullptr);
-    body.try_emplace("description",
-                     "variable not found: " + llvm::utostr(state));
+    body.try_emplace("description", "variable not found");
   }
 
   if (!body.getObject("dataId")) {

>From f52343ab697153a51790cc7daf7b6d8eb22557a2 Mon Sep 17 00:00:00 2001
From: Zequan Wu <zequanwu at google.com>
Date: Wed, 14 Feb 2024 11:20:35 -0500
Subject: [PATCH 2/2] address comments

---
 lldb/tools/lldb-dap/lldb-dap.cpp | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index a3c6109370e685..cfd495e1a57030 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -8,6 +8,7 @@
 
 #include "DAP.h"
 #include "Watchpoint.h"
+#include "lldb/API/SBMemoryRegionInfo.h"
 
 #include <cassert>
 #include <climits>
@@ -2770,13 +2771,24 @@ void request_dataBreakpointInfo(const llvm::json::Object &request) {
                                           ? std::string(error_cstr)
                                           : "evaluation failed");
     } else {
-      uint64_t value_as_unsigned = value.GetValueAsUnsigned();
-      if (value_as_unsigned == 0) {
+      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 {
         body.try_emplace("dataId", nullptr);
         body.try_emplace("description",
-                         "unable to evaluate expression to an address.");
+                         "unable to get memory region info for address " +
+                             addr);
       }
-      addr = llvm::utohexstr(value_as_unsigned);
     }
   } else {
     body.try_emplace("dataId", nullptr);