[Lldb-commits] [lldb] [lldb-dap] Use protocol types for modules request and events. (PR #146966)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 3 14:53:29 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Ebuka Ezike (da-viper)
<details>
<summary>Changes</summary>
Can now limit the number of modules fetched.
Show the non-standard `debugInfoSize` field in the vs-code extension.
Update tests to fix silently failing test and handle when a module is removed.
---
Patch is 42.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/146966.diff
25 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+10-4)
- (modified) lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py (+13-6)
- (modified) lldb/test/API/tools/lldb-dap/module-event/main.cpp (+1-1)
- (modified) lldb/test/API/tools/lldb-dap/module/TestDAP_module.py (+26-6)
- (modified) lldb/tools/lldb-dap/DAP.cpp (+1-39)
- (modified) lldb/tools/lldb-dap/EventHelper.cpp (+46)
- (modified) lldb/tools/lldb-dap/EventHelper.h (+2)
- (modified) lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp (+30-51)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+6-3)
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (-95)
- (modified) lldb/tools/lldb-dap/JSONUtils.h (-18)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp (+16)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolEvents.h (+15)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+14)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+21)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+25)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+42)
- (modified) lldb/tools/lldb-dap/ProtocolUtils.cpp (+103)
- (modified) lldb/tools/lldb-dap/ProtocolUtils.h (+20)
- (modified) lldb/tools/lldb-dap/src-ts/debug-session-tracker.ts (+6-2)
- (modified) lldb/tools/lldb-dap/src-ts/ui/modules-data-provider.ts (+9-6)
- (modified) lldb/unittests/DAP/CMakeLists.txt (+1)
- (modified) lldb/unittests/DAP/JSONUtilsTest.cpp (-11)
- (modified) lldb/unittests/DAP/ProtocolTypesTest.cpp (+91)
- (added) lldb/unittests/DAP/ProtocolUtilsTest.cpp (+24)
``````````diff
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 0fe36cd4bc71f..d227a66a703c1 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
@@ -199,8 +199,8 @@ def _read_packet_thread(self):
finally:
dump_dap_log(self.log_file)
- def get_modules(self):
- module_list = self.request_modules()["body"]["modules"]
+ def get_modules(self, startModule: int = 0, moduleCount: int = 0):
+ module_list = self.request_modules(startModule, moduleCount)["body"]["modules"]
modules = {}
for module in module_list:
modules[module["name"]] = module
@@ -1143,8 +1143,14 @@ def request_completions(self, text, frameId=None):
}
return self.send_recv(command_dict)
- def request_modules(self):
- return self.send_recv({"command": "modules", "type": "request"})
+ def request_modules(self, startModule: int, moduleCount: int):
+ return self.send_recv(
+ {
+ "command": "modules",
+ "type": "request",
+ "arguments": {"startModule": startModule, "moduleCount": moduleCount},
+ }
+ )
def request_stackTrace(
self, threadId=None, startFrame=None, levels=None, format=None, dump=False
diff --git a/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py b/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py
index 1ef2f2a8235a4..64ed4154b035d 100644
--- a/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py
+++ b/lldb/test/API/tools/lldb-dap/module-event/TestDAP_module_event.py
@@ -32,7 +32,7 @@ def test_module_event(self):
# Make sure we got a module event for libother.
event = self.dap_server.wait_for_event("module", 5)
- self.assertTrue(event, "didn't get a module event")
+ self.assertIsNotNone(event, "didn't get a module event")
module_name = event["body"]["module"]["name"]
module_id = event["body"]["module"]["id"]
self.assertEqual(event["body"]["reason"], "new")
@@ -43,13 +43,20 @@ def test_module_event(self):
# Make sure we got a module event for libother.
event = self.dap_server.wait_for_event("module", 5)
- self.assertTrue(event, "didn't get a module event")
+ self.assertIsNotNone(event, "didn't get a module event")
reason = event["body"]["reason"]
- self.assertEqual(event["body"]["reason"], "removed")
+ self.assertEqual(reason, "removed")
self.assertEqual(event["body"]["module"]["id"], module_id)
- # The removed module event should omit everything but the module id.
- # Check that there's no module name in the event.
- self.assertNotIn("name", event["body"]["module"])
+ # The removed module event should omit everything but the module id and name
+ # as they are required fields.
+ module_data = event["body"]["module"]
+ required_keys = ["id", "name"]
+ self.assertListEqual(list(module_data.keys()), required_keys)
+ self.assertEqual(module_data["name"], "", "expects empty name.")
+
+ # Make sure we do not send another event
+ event = self.dap_server.wait_for_event("module", 3)
+ self.assertIsNone(event, "expects no events.")
self.continue_to_exit()
diff --git a/lldb/test/API/tools/lldb-dap/module-event/main.cpp b/lldb/test/API/tools/lldb-dap/module-event/main.cpp
index 711471b9fadd0..283b2f8214b4a 100644
--- a/lldb/test/API/tools/lldb-dap/module-event/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/module-event/main.cpp
@@ -18,5 +18,5 @@ int main(int argc, char const *argv[]) {
dlclose(handle);
printf("after dlclose\n"); // breakpoint 3
- return 0; // breakpoint 1
+ return 0;
}
diff --git a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py
index 4fc221668a8ee..d8d9cf24c5e41 100644
--- a/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py
+++ b/lldb/test/API/tools/lldb-dap/module/TestDAP_module.py
@@ -45,16 +45,20 @@ def run_test(self, symbol_basename, expect_debug_info_size):
context="repl",
)
- def checkSymbolsLoadedWithSize():
+ def check_symbols_loaded_with_size():
active_modules = self.dap_server.get_modules()
program_module = active_modules[program_basename]
self.assertIn("symbolFilePath", program_module)
self.assertIn(symbols_path, program_module["symbolFilePath"])
- symbol_regex = re.compile(r"[0-9]+(\.[0-9]*)?[KMG]?B")
- return symbol_regex.match(program_module["symbolStatus"])
+ size_regex = re.compile(r"[0-9]+(\.[0-9]*)?[KMG]?B")
+ return size_regex.match(program_module["debugInfoSize"])
if expect_debug_info_size:
- self.waitUntil(checkSymbolsLoadedWithSize)
+ self.assertTrue(
+ self.waitUntil(check_symbols_loaded_with_size),
+ "expect has debug info size",
+ )
+
active_modules = self.dap_server.get_modules()
program_module = active_modules[program_basename]
self.assertEqual(program_basename, program_module["name"])
@@ -84,6 +88,20 @@ def checkSymbolsLoadedWithSize():
self.assertNotEqual(len(module_changed_names), 0)
self.assertIn(program_module["name"], module_changed_names)
+ # fetch modules from offset
+ if len(active_modules.keys()) > 2:
+ resp = self.dap_server.request_modules(startModule=1, moduleCount=1)
+ self.assertTrue(resp["success"], f"expects a successful response {resp!r}")
+ resp_total_modules = resp["body"]["totalModules"]
+ self.assertEqual(resp_total_modules, len(active_modules))
+ resp_modules = resp["body"]["modules"]
+ self.assertEqual(len(resp_modules), 1, "expects only one module")
+
+ module_name = resp_modules[0]["name"]
+ self.assertIn(module_name, active_modules.keys())
+
+ self.continue_to_exit()
+
@skipIfWindows
def test_modules(self):
"""
@@ -119,8 +137,10 @@ def test_compile_units(self):
lines = [breakpoint1_line]
breakpoint_ids = self.set_source_breakpoints(source, lines)
self.continue_to_breakpoints(breakpoint_ids)
- moduleId = self.dap_server.get_modules()["a.out"]["id"]
- response = self.dap_server.request_compileUnits(moduleId)
+ module_id = self.dap_server.get_modules()["a.out"]["id"]
+ response = self.dap_server.request_compileUnits(module_id)
self.assertTrue(response["body"])
cu_paths = [cu["compileUnitPath"] for cu in response["body"]["compileUnits"]]
self.assertIn(main_source_path, cu_paths, "Real path to main.cpp matches")
+
+ self.continue_to_exit()
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index cd97458bd4aa8..1d97a6d26df28 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -1336,45 +1336,7 @@ void DAP::EventThread() {
SendStdOutStdErr(*this, process);
}
} else if (lldb::SBTarget::EventIsTargetEvent(event)) {
- if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
- event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
- event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded ||
- event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged) {
- const uint32_t num_modules =
- lldb::SBTarget::GetNumModulesFromEvent(event);
- std::lock_guard<std::mutex> guard(modules_mutex);
- for (uint32_t i = 0; i < num_modules; ++i) {
- lldb::SBModule module =
- lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
- if (!module.IsValid())
- continue;
- llvm::StringRef module_id = module.GetUUIDString();
- if (module_id.empty())
- continue;
-
- llvm::StringRef reason;
- bool id_only = false;
- if (modules.contains(module_id)) {
- if (event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded) {
- modules.erase(module_id);
- reason = "removed";
- id_only = true;
- } else {
- reason = "changed";
- }
- } else {
- modules.insert(module_id);
- reason = "new";
- }
-
- llvm::json::Object body;
- body.try_emplace("reason", reason);
- body.try_emplace("module", CreateModule(target, module, id_only));
- llvm::json::Object module_event = CreateEventObject("module");
- module_event.try_emplace("body", std::move(body));
- SendJSON(llvm::json::Value(std::move(module_event)));
- }
- }
+ HandleTargetEvent(*this, event);
} else if (lldb::SBBreakpoint::EventIsBreakpointEvent(event)) {
if (event_mask & lldb::SBTarget::eBroadcastBitBreakpointChanged) {
auto event_type =
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp b/lldb/tools/lldb-dap/EventHelper.cpp
index 364cc7ab4ef8c..5f7739da1580d 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -13,6 +13,8 @@
#include "LLDBUtils.h"
#include "Protocol/ProtocolEvents.h"
#include "Protocol/ProtocolTypes.h"
+#include "ProtocolUtils.h"
+#include "lldb/API/SBEvent.h"
#include "lldb/API/SBFileSpec.h"
#include "llvm/Support/Error.h"
@@ -269,4 +271,48 @@ void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process) {
dap.SendJSON(llvm::json::Value(std::move(event)));
}
+void HandleTargetEvent(DAP &dap, const lldb::SBEvent &event) {
+ const lldb::SBTarget &target = dap.target;
+ const uint32_t event_mask = event.GetType();
+ if (!(event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) &&
+ !(event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded) &&
+ !(event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded) &&
+ !(event_mask & lldb::SBTarget::eBroadcastBitSymbolsChanged))
+ return;
+
+ const uint32_t num_modules = lldb::SBTarget::GetNumModulesFromEvent(event);
+ std::lock_guard<std::mutex> guard(dap.modules_mutex);
+
+ for (uint32_t i = 0; i < num_modules; ++i) {
+ lldb::SBModule module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
+
+ const bool remove_module =
+ event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded;
+ auto p_module = lldb_dap::CreateModule(target, module, remove_module);
+ if (!p_module)
+ continue;
+
+ const llvm::StringRef module_id = p_module->id;
+ if (dap.modules.contains(module_id)) {
+ if (event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded) {
+ dap.modules.erase(module_id);
+ dap.Send(protocol::Event{
+ "module", protocol::ModuleEventBody{
+ std::move(p_module).value(),
+ protocol::ModuleEventBody::eReasonRemoved}});
+ } else {
+ dap.Send(protocol::Event{
+ "module", protocol::ModuleEventBody{
+ std::move(p_module).value(),
+ protocol::ModuleEventBody::eReasonChanged}});
+ }
+ } else if (!remove_module) {
+ dap.modules.insert(module_id);
+ dap.Send(protocol::Event{
+ "module",
+ protocol::ModuleEventBody{std::move(p_module).value(),
+ protocol::ModuleEventBody::eReasonNew}});
+ }
+ }
+}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/EventHelper.h b/lldb/tools/lldb-dap/EventHelper.h
index 72ad5308a2b0c..ba82c89a31119 100644
--- a/lldb/tools/lldb-dap/EventHelper.h
+++ b/lldb/tools/lldb-dap/EventHelper.h
@@ -32,6 +32,8 @@ void SendContinuedEvent(DAP &dap);
void SendProcessExitedEvent(DAP &dap, lldb::SBProcess &process);
+void HandleTargetEvent(DAP &dap, const lldb::SBEvent &event);
+
} // namespace lldb_dap
#endif
diff --git a/lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp
index d37f302b06958..e8440f591d2b2 100644
--- a/lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp
@@ -7,64 +7,43 @@
//===----------------------------------------------------------------------===//
#include "DAP.h"
-#include "EventHelper.h"
-#include "JSONUtils.h"
+#include "ProtocolUtils.h"
#include "RequestHandler.h"
+using namespace lldb_dap::protocol;
namespace lldb_dap {
-// "modulesRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "Modules request; value of command field is
-// 'modules'.",
-// "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "modules" ]
-// },
-// },
-// "required": [ "command" ]
-// }]
-// },
-// "modulesResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to 'modules' request.",
-// "properties": {
-// "body": {
-// "description": "Response to 'modules' request. Array of
-// module objects."
-// }
-// }
-// }]
-// }
-void ModulesRequestHandler::operator()(
- const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
-
- llvm::json::Array modules;
-
- {
- std::lock_guard<std::mutex> guard(dap.modules_mutex);
- for (size_t i = 0; i < dap.target.GetNumModules(); i++) {
- lldb::SBModule module = dap.target.GetModuleAtIndex(i);
- if (!module.IsValid())
- continue;
-
- llvm::StringRef module_id = module.GetUUIDString();
- if (!module_id.empty())
- dap.modules.insert(module_id);
-
- modules.emplace_back(CreateModule(dap.target, module));
+/// Modules can be retrieved from the debug adapter with this request which can
+/// either return all modules or a range of modules to support paging.
+///
+/// Clients should only call this request if the corresponding capability
+/// `supportsModulesRequest` is true.
+llvm::Expected<ModulesResponseBody>
+ModulesRequestHandler::Run(const std::optional<ModulesArguments> &args) const {
+ ModulesResponseBody response;
+ const auto [start_module, module_count] = args.value_or({});
+
+ std::vector<Module> &modules = response.modules;
+ std::lock_guard<std::mutex> guard(dap.modules_mutex);
+ const uint32_t total_modules = dap.target.GetNumModules();
+ const uint32_t max_num_module =
+ module_count == 0 ? total_modules
+ : std::min(start_module + module_count, total_modules);
+ modules.reserve(max_num_module);
+
+ response.totalModules = total_modules;
+
+ for (uint32_t i = start_module; i < max_num_module; i++) {
+ lldb::SBModule module = dap.target.GetModuleAtIndex(i);
+
+ std::optional<Module> result = CreateModule(dap.target, module);
+ if (result && !result->id.empty()) {
+ dap.modules.insert(result->id);
+ modules.emplace_back(std::move(result).value());
}
}
- llvm::json::Object body;
- body.try_emplace("modules", std::move(modules));
- response.try_emplace("body", std::move(body));
- dap.SendJSON(llvm::json::Value(std::move(response)));
+ return response;
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index e35b9830ab60f..3b910abe81992 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -466,14 +466,17 @@ class CompileUnitsRequestHandler : public LegacyRequestHandler {
void operator()(const llvm::json::Object &request) const override;
};
-class ModulesRequestHandler : public LegacyRequestHandler {
+class ModulesRequestHandler final
+ : public RequestHandler<std::optional<protocol::ModulesArguments>,
+ llvm::Expected<protocol::ModulesResponseBody>> {
public:
- using LegacyRequestHandler::LegacyRequestHandler;
+ using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "modules"; }
FeatureSet GetSupportedFeatures() const override {
return {protocol::eAdapterFeatureModulesRequest};
}
- void operator()(const llvm::json::Object &request) const override;
+ llvm::Expected<protocol::ModulesResponseBody>
+ Run(const std::optional<protocol::ModulesArguments> &args) const override;
};
class PauseRequestHandler : public LegacyRequestHandler {
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 08e65ab835a57..f52661cf9d6c2 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -341,101 +341,6 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
return llvm::json::Value(std::move(object));
}
-static uint64_t GetDebugInfoSizeInSection(lldb::SBSection section) {
- uint64_t debug_info_size = 0;
- llvm::StringRef section_name(section.GetName());
- if (section_name.starts_with(".debug") ||
- section_name.starts_with("__debug") ||
- section_name.starts_with(".apple") || section_name.starts_with("__apple"))
- debug_info_size += section.GetFileByteSize();
- size_t num_sub_sections = section.GetNumSubSections();
- for (size_t i = 0; i < num_sub_sections; i++) {
- debug_info_size +=
- GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
- }
- return debug_info_size;
-}
-
-static uint64_t GetDebugInfoSize(lldb::SBModule module) {
- uint64_t debug_info_size = 0;
- size_t num_sections = module.GetNumSections();
- for (size_t i = 0; i < num_sections; i++) {
- debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
- }
- return debug_info_size;
-}
-
-static std::string ConvertDebugInfoSizeToString(uint64_t debug_info) {
- std::ostringstream oss;
- oss << std::fixed << std::setprecision(1);
- if (debug_info < 1024) {
- oss << debug_info << "B";
- } else if (debug_info < 1024 * 1024) {
- double kb = double(debug_info) / 1024.0;
- oss << kb << "KB";
- } else if (debug_info < 1024 * 1024 * 1024) {
- double mb = double(debug_info) / (1024.0 * 1024.0);
- oss << mb << "MB";
- } else {
- double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
- oss << gb << "GB";
- }
- return oss.str();
-}
-
-llvm::json::Value CreateModule(lldb::SBTarget &target, lldb::SBModule &module,
- bool id_only) {
- llvm::json::Object object;
- if (!target.IsValid() || !module.IsValid())
- return llvm::json::Value(std::move(object));
-
- const char *uuid = module.GetUUIDString();
- object.try_emplace("id", uuid ? std::string(uuid) : std::string(""));
-
- if (id_only)
- return llvm::json::Value(std::move(object));
-
- object.try_emplace("name", std::string(module.GetFileSpec().GetFilename()));
- char module_path_arr[PATH_MAX];
- module.GetFileSpec().GetPath(module_path_arr, sizeof(module_path_arr));
- std::string module_path(module_path_arr);
- object.try_emplace("path", module_path);
- if (module.GetNumCompileUnits() > 0) {
- std::string symbol_str = "Symbols loaded.";
- std::string debug_info_size;
- uint64_t debug_info = GetDebugInfoSize(module);
- if (debug_info > 0) {
- debug_info_size = ConvertDebugInfoSizeToString(debug_info);
- }
- object.try_emplace("symbolStatus", symbol_str);
- object.try_emplace("debugInfoSize", debug_info_size);
- char symbol_path_arr[PATH_MAX];
- module.GetSymbolFileSpec().GetPath(symbol_path_arr,
- sizeof(symbol_path_arr));
- std::string symbol_path(symbol_path_arr);
- object.try_emplace("symbolFilePath", symbol_path);
- } else {
- object.try_emplace("symbolStatus", "Symbols not found.");
- }
- std::string load_address =
- llvm::formatv("{0:x}",
- module.GetObjectFileHeaderAddress().GetLoadAddress(target))
- .str();
- object.try_emplace("addressRange", load_address);
- std::string version_str;
- uint32_t v...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/146966
More information about the lldb-commits
mailing list