-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb-dap] Add memory event #158437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb-dap] Add memory event #158437
Conversation
@llvm/pr-subscribers-lldb Author: Druzhkov Sergei (DrSergei) ChangesThis patch adds support for the Full diff: https://github.com/llvm/llvm-project/pull/158437.diff 8 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 9fe8ca22e820b..3a9e076eff9f0 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -216,6 +216,7 @@ def __init__(
self.events: List[Event] = []
self.progress_events: List[Event] = []
self.invalidated_event: Optional[Event] = None
+ self.memory_event: Optional[Event] = None
self.reverse_requests: List[Request] = []
self.module_events: List[Dict] = []
self.sequence: int = 1
@@ -443,6 +444,8 @@ def _handle_event(self, packet: Event) -> None:
self.capabilities.update(body["capabilities"])
elif event == "invalidated":
self.invalidated_event = packet
+ elif event == "memory":
+ self.memory_event = packet
def _handle_reverse_request(self, request: Request) -> None:
if request in self.reverse_requests:
@@ -1018,6 +1021,7 @@ def request_initialize(self, sourceInitFile=False):
"supportsStartDebuggingRequest": True,
"supportsProgressReporting": True,
"supportsInvalidatedEvent": True,
+ "supportsMemoryEvent": True,
"$__lldb_sourceInitFile": sourceInitFile,
},
}
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index a0a009ae6cc9a..882eec9971a73 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -248,6 +248,14 @@ def verify_invalidated_event(self, expected_areas):
areas = event["body"].get("areas", [])
self.assertEqual(set(expected_areas), set(areas))
+ def verify_memory_event(self, memoryReference):
+ if memoryReference is None:
+ self.assertIsNone(self.dap_server.memory_event)
+ event = self.dap_server.memory_event
+ self.dap_server.memory_event = None
+ self.assertIsNotNone(event)
+ self.assertEqual(memoryReference, event["body"].get("memoryReference"))
+
def get_dict_value(self, d: dict, key_path: list[str]) -> Any:
"""Verify each key in the key_path array is in contained in each
dictionary within "d". Assert if any key isn't in the
@@ -364,6 +372,7 @@ def set_variable(self, varRef, name, value, id=None):
response = self.dap_server.request_setVariable(varRef, name, str(value), id=id)
if response["success"]:
self.verify_invalidated_event(["variables"])
+ self.verify_memory_event(response["body"].get("memoryReference"))
return response
def set_local(self, name, value, id=None):
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 6eb468e76b16c..9c2c1f846d11e 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -284,4 +284,17 @@ void SendInvalidatedEvent(
dap.Send(protocol::Event{"invalidated", std::move(body)});
}
+void SendMemoryEvent(DAP &dap, lldb::SBValue variable) {
+ if (!dap.clientFeatures.contains(protocol::eClientFeatureMemoryEvent))
+ return;
+ const lldb::addr_t addr = variable.GetLoadAddress();
+ if (addr == LLDB_INVALID_ADDRESS)
+ return;
+ protocol::MemoryEventBody body;
+ body.memoryReference = addr;
+ body.offset = 0;
+ body.count = variable.GetByteSize();
+ dap.Send(protocol::Event{"memory", std::move(body)});
+}
+
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/EventHelper.h b/lldb/tools/lldb-dap/EventHelper.h
index 0c57afbaf1f33..48eb5af6bd0b9 100644
--- a/lldb/tools/lldb-dap/EventHelper.h
+++ b/lldb/tools/lldb-dap/EventHelper.h
@@ -37,6 +37,8 @@ void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);
void SendInvalidatedEvent(
DAP &dap, llvm::ArrayRef<protocol::InvalidatedEventBody::Area> areas);
+void SendMemoryEvent(DAP &dap, lldb::SBValue variable);
+
} // namespace lldb_dap
#endif
diff --git a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
index 2a50dea0b4ada..b9ae28d6772ac 100644
--- a/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp
@@ -82,6 +82,9 @@ SetVariableRequestHandler::Run(const SetVariableArguments &args) const {
// (e.g. references) can be changed.
SendInvalidatedEvent(dap, {InvalidatedEventBody::eAreaVariables});
+ // Also send memory event to signal client that variable memory was changed.
+ SendMemoryEvent(dap, variable);
+
return body;
}
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
index 9598c69878d66..98bc18f880aa8 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "Protocol/ProtocolEvents.h"
+#include "JSONUtils.h"
#include "llvm/Support/JSON.h"
using namespace llvm;
@@ -56,4 +57,11 @@ llvm::json::Value toJSON(const InvalidatedEventBody &IEB) {
return Result;
}
+llvm::json::Value toJSON(const MemoryEventBody &MEB) {
+ return json::Object{
+ {"memoryReference", EncodeMemoryReference(MEB.memoryReference)},
+ {"offset", MEB.offset},
+ {"count", MEB.count}};
+}
+
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h
index 138b622e01210..045b2ac7d8ea8 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolEvents.h
@@ -88,6 +88,34 @@ struct InvalidatedEventBody {
llvm::json::Value toJSON(const InvalidatedEventBody::Area &);
llvm::json::Value toJSON(const InvalidatedEventBody &);
+/// This event indicates that some memory range has been updated. It should only
+/// be sent if the corresponding capability supportsMemoryEvent is true.
+///
+/// Clients typically react to the event by re-issuing a readMemory request if
+/// they show the memory identified by the memoryReference and if the updated
+/// memory range overlaps the displayed range. Clients should not make
+/// assumptions how individual memory references relate to each other, so they
+/// should not assume that they are part of a single continuous address range
+/// and might overlap.
+///
+/// Debug adapters can use this event to indicate that the contents of a memory
+/// range has changed due to some other request like setVariable or
+/// setExpression. Debug adapters are not expected to emit this event for each
+/// and every memory change of a running program, because that information is
+/// typically not available from debuggers and it would flood clients with too
+/// many events.
+struct MemoryEventBody {
+ /// Memory reference of a memory range that has been updated.
+ lldb::addr_t memoryReference;
+
+ /// Starting offset in bytes where memory has been updated. Can be negative.
+ int64_t offset;
+
+ /// Number of bytes updated.
+ uint64_t count;
+};
+llvm::json::Value toJSON(const MemoryEventBody &);
+
} // end namespace lldb_dap::protocol
#endif
diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp
index a964592495347..be1e6a1470252 100644
--- a/lldb/unittests/DAP/ProtocolTypesTest.cpp
+++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp
@@ -1088,3 +1088,16 @@ TEST(ProtocolTypesTest, InvalidatedEventBody) {
})";
EXPECT_EQ(json, pp(body));
}
+
+TEST(ProtocolTypesTest, MemoryEventBody) {
+ MemoryEventBody body;
+ body.memoryReference = 12345;
+ body.offset = 0;
+ body.count = 4;
+ StringRef json = R"({
+ "count": 4,
+ "memoryReference": "0x3039",
+ "offset": 0
+})";
+ EXPECT_EQ(json, pp(body));
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
/// Memory reference of a memory range that has been updated. | ||
lldb::addr_t memoryReference; | ||
|
||
/// Starting offset in bytes where memory has been updated. Can be negative. | ||
int64_t offset; | ||
|
||
/// Number of bytes updated. | ||
uint64_t count; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a default of 0 or a some define/const value for these?
In case we don't need to set a value.
|
I think it is not the best solution. These fields are required, so we should initialize them with meaningful values. |
Declaring a variable, should just work in case we don't set a value. If we wanted, we could assert in But doing |
LGTM, I can merge it. |
This patch adds support for the
memory
event from the DAP. After this event is emitted, VS Code refetches the corresponding memory range and re-renders the memory view. I think this patch and PR can improve experience for users who usesetVariable
request.