[lldb-dap] Add invalidated event (#157530)

This patch fixes the problem, when after a `setVariable` request
pointers and references to the variable are not updated. VSCode doesn't
send a `variables` request after a `setVariable` request, so we should
trigger it explicitly via`invalidated` event .Also, updated
`writeMemory` request in similar way.

Author: DrSergei

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -215,6 +215,7 @@ def __init__(
         self.terminated: bool = False
         self.events: List[Event] = []
         self.progress_events: List[Event] = []
+        self.invalidated_event: Optional[Event] = None
         self.reverse_requests: List[Request] = []
         self.module_events: List[Dict] = []
         self.sequence: int = 1
@@ -440,6 +441,8 @@ def _handle_event(self, packet: Event) -> None:
         elif event == "capabilities" and body:
             # Update the capabilities with new ones from the event.
             self.capabilities.update(body["capabilities"])
+        elif event == "invalidated":
+            self.invalidated_event = packet
 
     def _handle_reverse_request(self, request: Request) -> None:
         if request in self.reverse_requests:
@@ -1014,6 +1017,7 @@ def request_initialize(self, sourceInitFile=False):
                 "supportsVariableType": True,
                 "supportsStartDebuggingRequest": True,
                 "supportsProgressReporting": True,
+                "supportsInvalidatedEvent": True,
                 "$__lldb_sourceInitFile": sourceInitFile,
             },
         }

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py

@@ -241,6 +241,13 @@ def verify_commands(self, flavor: str, output: str, commands: list[str]):
                 f"Command '{flavor}' - '{cmd}' not found in output: {output}",
             )
 
+    def verify_invalidated_event(self, expected_areas):
+        event = self.dap_server.invalidated_event
+        self.dap_server.invalidated_event = None
+        self.assertIsNotNone(event)
+        areas = event["body"].get("areas", [])
+        self.assertEqual(set(expected_areas), set(areas))
+
     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
@@ -352,13 +359,20 @@ def get_local_as_int(self, name, threadId=None):
         else:
             return int(value)
 
+    def set_variable(self, varRef, name, value, id=None):
+        """Set a variable."""
+        response = self.dap_server.request_setVariable(varRef, name, str(value), id=id)
+        if response["success"]:
+            self.verify_invalidated_event(["variables"])
+        return response
+
     def set_local(self, name, value, id=None):
         """Set a top level local variable only."""
-        return self.dap_server.request_setVariable(1, name, str(value), id=id)
+        return self.set_variable(1, name, str(value), id=id)
 
     def set_global(self, name, value, id=None):
         """Set a top level global variable only."""
-        return self.dap_server.request_setVariable(2, name, str(value), id=id)
+        return self.set_variable(2, name, str(value), id=id)
 
     def stepIn(
         self,
@@ -577,4 +591,6 @@ def writeMemory(self, memoryReference, data=None, offset=0, allowPartial=False):
         response = self.dap_server.request_writeMemory(
             memoryReference, encodedData, offset=offset, allowPartial=allowPartial
         )
+        if response["success"]:
+            self.verify_invalidated_event(["all"])
         return response

lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py

@@ -72,9 +72,7 @@ def test_memory_refs_set_variable(self):
         ptr_value = self.get_local_as_int("rawptr")
         self.assertIn(
             "memoryReference",
-            self.dap_server.request_setVariable(1, "rawptr", ptr_value + 2)[
-                "body"
-            ].keys(),
+            self.set_local("rawptr", ptr_value + 2)["body"].keys(),
         )
 
     @skipIfWindows

lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py

@@ -298,7 +298,7 @@ def do_test_scopes_variables_setVariable_evaluate(
         # Set a variable value whose name is synthetic, like a variable index
         # and verify the value by reading it
         variable_value = 100
-        response = self.dap_server.request_setVariable(varRef, "[0]", variable_value)
+        response = self.set_variable(varRef, "[0]", variable_value)
         # Verify dap sent the correct response
         verify_response = {
             "type": "int",
@@ -315,7 +315,7 @@ def do_test_scopes_variables_setVariable_evaluate(
         # Set a variable value whose name is a real child value, like "pt.x"
         # and verify the value by reading it
         varRef = varref_dict["pt"]
-        self.dap_server.request_setVariable(varRef, "x", 111)
+        self.set_variable(varRef, "x", 111)
         response = self.dap_server.request_variables(varRef, start=0, count=1)
         value = response["body"]["variables"][0]["value"]
         self.assertEqual(
@@ -341,27 +341,15 @@ def do_test_scopes_variables_setVariable_evaluate(
         self.verify_variables(verify_locals, self.dap_server.get_local_variables())
 
         # Now we verify that we correctly change the name of a variable with and without differentiator suffix
-        self.assertFalse(self.dap_server.request_setVariable(1, "x2", 9)["success"])
-        self.assertFalse(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:0", 9)["success"]
-        )
+        self.assertFalse(self.set_local("x2", 9)["success"])
+        self.assertFalse(self.set_local("x @ main.cpp:0", 9)["success"])
 
-        self.assertTrue(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:19", 19)["success"]
-        )
-        self.assertTrue(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:21", 21)["success"]
-        )
-        self.assertTrue(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:23", 23)["success"]
-        )
+        self.assertTrue(self.set_local("x @ main.cpp:19", 19)["success"])
+        self.assertTrue(self.set_local("x @ main.cpp:21", 21)["success"])
+        self.assertTrue(self.set_local("x @ main.cpp:23", 23)["success"])
 
         # The following should have no effect
-        self.assertFalse(
-            self.dap_server.request_setVariable(1, "x @ main.cpp:23", "invalid")[
-                "success"
-            ]
-        )
+        self.assertFalse(self.set_local("x @ main.cpp:23", "invalid")["success"])
 
         verify_locals["x @ main.cpp:19"]["equals"]["value"] = "19"
         verify_locals["x @ main.cpp:21"]["equals"]["value"] = "21"
@@ -370,7 +358,7 @@ def do_test_scopes_variables_setVariable_evaluate(
         self.verify_variables(verify_locals, self.dap_server.get_local_variables())
 
         # The plain x variable shold refer to the innermost x
-        self.assertTrue(self.dap_server.request_setVariable(1, "x", 22)["success"])
+        self.assertTrue(self.set_local("x", 22)["success"])
         verify_locals["x @ main.cpp:23"]["equals"]["value"] = "22"
 
         self.verify_variables(verify_locals, self.dap_server.get_local_variables())
@@ -708,9 +696,7 @@ def test_return_variables(self):
                 self.verify_variables(verify_locals, local_variables, varref_dict)
                 break
 
-        self.assertFalse(
-            self.dap_server.request_setVariable(1, "(Return Value)", 20)["success"]
-        )
+        self.assertFalse(self.set_local("(Return Value)", 20)["success"])
 
     @skipIfWindows
     def test_indexedVariables(self):

lldb/tools/lldb-dap/EventHelper.cpp

@@ -12,9 +12,11 @@
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
 #include "Protocol/ProtocolEvents.h"
+#include "Protocol/ProtocolRequests.h"
 #include "Protocol/ProtocolTypes.h"
 #include "lldb/API/SBFileSpec.h"
 #include "llvm/Support/Error.h"
+#include <utility>
 
 #if defined(_WIN32)
 #define NOMINMAX
@@ -273,4 +275,13 @@ void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process) {
   dap.SendJSON(llvm::json::Value(std::move(event)));
 }
 
+void SendInvalidatedEvent(
+    DAP &dap, llvm::ArrayRef<protocol::InvalidatedEventBody::Area> areas) {
+  if (!dap.clientFeatures.contains(protocol::eClientFeatureInvalidatedEvent))
+    return;
+  protocol::InvalidatedEventBody body;
+  body.areas = areas;
+  dap.Send(protocol::Event{"invalidated", std::move(body)});
+}
+
 } // namespace lldb_dap

lldb/tools/lldb-dap/EventHelper.h

@@ -10,6 +10,8 @@
 #define LLDB_TOOLS_LLDB_DAP_EVENTHELPER_H
 
 #include "DAPForward.h"
+#include "Protocol/ProtocolEvents.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 
 namespace lldb_dap {
@@ -32,6 +34,9 @@ void SendContinuedEvent(DAP &dap);
 
 void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);
 
+void SendInvalidatedEvent(
+    DAP &dap, llvm::ArrayRef<protocol::InvalidatedEventBody::Area> areas);
+
 } // namespace lldb_dap
 
 #endif

lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp

@@ -9,6 +9,7 @@
 #include "DAP.h"
 #include "EventHelper.h"
 #include "JSONUtils.h"
+#include "Protocol/ProtocolEvents.h"
 #include "RequestHandler.h"
 
 using namespace lldb_dap::protocol;
@@ -77,6 +78,10 @@ SetVariableRequestHandler::Run(const SetVariableArguments &args) const {
   if (ValuePointsToCode(variable))
     body.valueLocationReference = new_var_ref;
 
+  // Also send invalidated event to signal client that some variables
+  // (e.g. references) can be changed.
+  SendInvalidatedEvent(dap, {InvalidatedEventBody::eAreaVariables});
+
   return body;
 }
 

lldb/tools/lldb-dap/Handler/WriteMemoryRequestHandler.cpp

@@ -7,21 +7,24 @@
 //===----------------------------------------------------------------------===//
 
 #include "DAP.h"
+#include "EventHelper.h"
 #include "JSONUtils.h"
+#include "Protocol/ProtocolEvents.h"
 #include "RequestHandler.h"
 #include "lldb/API/SBMemoryRegionInfo.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Base64.h"
 
+using namespace lldb_dap::protocol;
+
 namespace lldb_dap {
 
 // Writes bytes to memory at the provided location.
 //
 // Clients should only call this request if the corresponding capability
 //  supportsWriteMemoryRequest is true.
-llvm::Expected<protocol::WriteMemoryResponseBody>
-WriteMemoryRequestHandler::Run(
-    const protocol::WriteMemoryArguments &args) const {
+llvm::Expected<WriteMemoryResponseBody>
+WriteMemoryRequestHandler::Run(const WriteMemoryArguments &args) const {
   const lldb::addr_t address = args.memoryReference + args.offset;
 
   lldb::SBProcess process = dap.target.GetProcess();
@@ -91,8 +94,13 @@ WriteMemoryRequestHandler::Run(
   if (bytes_written == 0) {
     return llvm::make_error<DAPError>(write_error.GetCString());
   }
-  protocol::WriteMemoryResponseBody response;
+  WriteMemoryResponseBody response;
   response.bytesWritten = bytes_written;
+
+  // Also send invalidated event to signal client that some things
+  // (e.g. variables) can be changed.
+  SendInvalidatedEvent(dap, {InvalidatedEventBody::eAreaAll});
+
   return response;
 }
 

lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp

@@ -33,4 +33,27 @@ json::Value toJSON(const ModuleEventBody &MEB) {
   return json::Object{{"reason", MEB.reason}, {"module", MEB.module}};
 }
 
+llvm::json::Value toJSON(const InvalidatedEventBody::Area &IEBA) {
+  switch (IEBA) {
+  case InvalidatedEventBody::eAreaAll:
+    return "all";
+  case InvalidatedEventBody::eAreaStacks:
+    return "stacks";
+  case InvalidatedEventBody::eAreaThreads:
+    return "threads";
+  case InvalidatedEventBody::eAreaVariables:
+    return "variables";
+  }
+  llvm_unreachable("unhandled invalidated event area!.");
+}
+
+llvm::json::Value toJSON(const InvalidatedEventBody &IEB) {
+  json::Object Result{{"areas", IEB.areas}};
+  if (IEB.threadId)
+    Result.insert({"threadID", IEB.threadId});
+  if (IEB.frameId)
+    Result.insert({"frameId", IEB.frameId});
+  return Result;
+}
+
 } // namespace lldb_dap::protocol

lldb/tools/lldb-dap/Protocol/ProtocolEvents.h

@@ -21,7 +21,11 @@
 #define LLDB_TOOLS_LLDB_DAP_PROTOCOL_PROTOCOL_EVENTS_H
 
 #include "Protocol/ProtocolTypes.h"
+#include "lldb/lldb-types.h"
 #include "llvm/Support/JSON.h"
+#include <cstdint>
+#include <optional>
+#include <vector>
 
 namespace lldb_dap::protocol {
 
@@ -56,6 +60,34 @@ struct ModuleEventBody {
 llvm::json::Value toJSON(const ModuleEventBody::Reason &);
 llvm::json::Value toJSON(const ModuleEventBody &);
 
+/// This event signals that some state in the debug adapter has changed and
+/// requires that the client needs to re-render the data snapshot previously
+/// requested.
+///
+/// Debug adapters do not have to emit this event for runtime changes like
+/// stopped or thread events because in that case the client refetches the new
+/// state anyway. But the event can be used for example to refresh the UI after
+/// rendering formatting has changed in the debug adapter.
+///
+/// This event should only be sent if the corresponding capability
+/// supportsInvalidatedEvent is true.
+struct InvalidatedEventBody {
+  enum Area : unsigned { eAreaAll, eAreaStacks, eAreaThreads, eAreaVariables };
+
+  /// Set of logical areas that got invalidated.
+  std::vector<Area> areas;
+
+  /// If specified, the client only needs to refetch data related to this
+  /// thread.
+  std::optional<lldb::tid_t> threadId;
+
+  /// If specified, the client only needs to refetch data related to this stack
+  /// frame (and the `threadId` is ignored).
+  std::optional<uint64_t> frameId;
+};
+llvm::json::Value toJSON(const InvalidatedEventBody::Area &);
+llvm::json::Value toJSON(const InvalidatedEventBody &);
+
 } // end namespace lldb_dap::protocol
 
 #endif