[lldb-dap] Use protocol types for modules request and events. #146966
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) ChangesCan now limit the number of modules fetched. 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:
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]
|
|
Not to sure if I should split it into different PR since they are all module related. |
da-viper
force-pushed
the
use-protocol-modules
branch
from
8fcb5ae to
4a9dd25
Compare
JDevlieghere
reviewed
Jul 3, 2025
ashgti
reviewed
Jul 7, 2025
da-viper
force-pushed
the
use-protocol-modules
branch
from
4a9dd25 to
cc0846c
Compare
ashgti
approved these changes
Jul 8, 2025
JDevlieghere
approved these changes
Jul 8, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/197/builds/6790 Here is the relevant piece of the build log for the reference |
|
Fixed in 4647398 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/10026 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Can now limit the number of modules fetched.
Show the non-standard
debugInfoSizefield in the vs-code extension.Update tests to fix silently failing test and handle when a module is removed.