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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
