-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[lldb-dap] Add module symbol table viewer to VS Code extension #140626 #153836
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
Conversation
@llvm/pr-subscribers-lldb Author: Ely Ronnen (eronnen) Changes
Screencast.From.2025-08-15.19-12-33.webmPatch is 49.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/153836.diff 22 Files Affected:
diff --git a/lldb/include/lldb/API/SBSymbol.h b/lldb/include/lldb/API/SBSymbol.h
index 94521881f82f9..6777494bad1d2 100644
--- a/lldb/include/lldb/API/SBSymbol.h
+++ b/lldb/include/lldb/API/SBSymbol.h
@@ -85,6 +85,8 @@ class LLDB_API SBSymbol {
SymbolType GetType();
+ uint32_t GetID();
+
bool operator==(const lldb::SBSymbol &rhs) const;
bool operator!=(const lldb::SBSymbol &rhs) const;
@@ -99,6 +101,9 @@ class LLDB_API SBSymbol {
// other than the actual symbol table itself in the object file.
bool IsSynthetic();
+ /// Returns true if the symbol is a debug symbol.
+ bool IsDebug();
+
protected:
lldb_private::Symbol *get();
diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h
index 22b6c63ed5b97..90fada3aea4ec 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -324,6 +324,8 @@ class LLDB_API SBTarget {
lldb::SBModule FindModule(const lldb::SBFileSpec &file_spec);
+ lldb::SBModule FindModule(const lldb::SBModuleSpec &module_spec);
+
/// Find compile units related to *this target and passed source
/// file.
///
diff --git a/lldb/source/API/SBSymbol.cpp b/lldb/source/API/SBSymbol.cpp
index 79477dd3a70fc..7bcc7f99079c4 100644
--- a/lldb/source/API/SBSymbol.cpp
+++ b/lldb/source/API/SBSymbol.cpp
@@ -193,6 +193,14 @@ SymbolType SBSymbol::GetType() {
return eSymbolTypeInvalid;
}
+uint32_t SBSymbol::GetID() {
+ LLDB_INSTRUMENT_VA(this);
+
+ if (m_opaque_ptr)
+ return m_opaque_ptr->GetID();
+ return 0;
+}
+
bool SBSymbol::IsExternal() {
LLDB_INSTRUMENT_VA(this);
@@ -208,3 +216,11 @@ bool SBSymbol::IsSynthetic() {
return m_opaque_ptr->IsSynthetic();
return false;
}
+
+bool SBSymbol::IsDebug() {
+ LLDB_INSTRUMENT_VA(this);
+
+ if (m_opaque_ptr)
+ return m_opaque_ptr->IsDebug();
+ return false;
+}
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 6aa41c52f3731..c761b96135a86 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1570,6 +1570,18 @@ SBModule SBTarget::FindModule(const SBFileSpec &sb_file_spec) {
return sb_module;
}
+SBModule SBTarget::FindModule(const SBModuleSpec &sb_module_spec) {
+ LLDB_INSTRUMENT_VA(this, sb_module_spec);
+
+ SBModule sb_module;
+ if (TargetSP target_sp = GetSP(); target_sp && sb_module_spec.IsValid()) {
+ // The module list is thread safe, no need to lock
+ sb_module.SetSP(
+ target_sp->GetImages().FindFirstModule(*sb_module_spec.m_opaque_up));
+ }
+ return sb_module;
+}
+
SBSymbolContextList SBTarget::FindCompileUnits(const SBFileSpec &sb_file_spec) {
LLDB_INSTRUMENT_VA(this, sb_file_spec);
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 5e0ad53b82f89..c3ba53754f8ad 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -36,6 +36,7 @@ add_lldb_library(lldbDAP
Handler/CompletionsHandler.cpp
Handler/ConfigurationDoneRequestHandler.cpp
Handler/ContinueRequestHandler.cpp
+ Handler/DAPGetModuleSymbolsRequestHandler.cpp
Handler/DataBreakpointInfoRequestHandler.cpp
Handler/DisassembleRequestHandler.cpp
Handler/DisconnectRequestHandler.cpp
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index ce910b1f60b85..08d50c9b93a7e 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -1566,6 +1566,7 @@ void DAP::RegisterRequests() {
// Custom requests
RegisterRequest<CompileUnitsRequestHandler>();
RegisterRequest<ModulesRequestHandler>();
+ RegisterRequest<DAPGetModuleSymbolsRequestHandler>();
// Testing requests
RegisterRequest<TestGetTargetBreakpointsRequestHandler>();
diff --git a/lldb/tools/lldb-dap/Handler/DAPGetModuleSymbolsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DAPGetModuleSymbolsRequestHandler.cpp
new file mode 100644
index 0000000000000..99199fb2ac9ca
--- /dev/null
+++ b/lldb/tools/lldb-dap/Handler/DAPGetModuleSymbolsRequestHandler.cpp
@@ -0,0 +1,160 @@
+//===-- DAPGetModuleSymbolsRequestHandler.cpp -----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DAP.h"
+#include "DAPError.h"
+#include "Protocol/DAPTypes.h"
+#include "RequestHandler.h"
+#include "lldb/API/SBAddress.h"
+#include "lldb/API/SBFileSpec.h"
+#include "lldb/API/SBModule.h"
+#include "lldb/API/SBModuleSpec.h"
+#include "lldb/Utility/UUID.h"
+#include "lldb/lldb-enumerations.h"
+#include "llvm/Support/Error.h"
+#include <cstddef>
+
+using namespace lldb_dap::protocol;
+namespace lldb_dap {
+
+static std::string SymbolTypeToString(lldb::SymbolType symbol_type) {
+ switch (symbol_type) {
+ case lldb::eSymbolTypeInvalid:
+ return "Invalid";
+ case lldb::eSymbolTypeAbsolute:
+ return "Absolute";
+ case lldb::eSymbolTypeCode:
+ return "Code";
+ case lldb::eSymbolTypeResolver:
+ return "Resolver";
+ case lldb::eSymbolTypeData:
+ return "Data";
+ case lldb::eSymbolTypeTrampoline:
+ return "Trampoline";
+ case lldb::eSymbolTypeRuntime:
+ return "Runtime";
+ case lldb::eSymbolTypeException:
+ return "Exception";
+ case lldb::eSymbolTypeSourceFile:
+ return "SourceFile";
+ case lldb::eSymbolTypeHeaderFile:
+ return "HeaderFile";
+ case lldb::eSymbolTypeObjectFile:
+ return "ObjectFile";
+ case lldb::eSymbolTypeCommonBlock:
+ return "CommonBlock";
+ case lldb::eSymbolTypeBlock:
+ return "Block";
+ case lldb::eSymbolTypeLocal:
+ return "Local";
+ case lldb::eSymbolTypeParam:
+ return "Param";
+ case lldb::eSymbolTypeVariable:
+ return "Variable";
+ case lldb::eSymbolTypeVariableType:
+ return "VariableType";
+ case lldb::eSymbolTypeLineEntry:
+ return "LineEntry";
+ case lldb::eSymbolTypeLineHeader:
+ return "LineHeader";
+ case lldb::eSymbolTypeScopeBegin:
+ return "ScopeBegin";
+ case lldb::eSymbolTypeScopeEnd:
+ return "ScopeEnd";
+ case lldb::eSymbolTypeAdditional:
+ return "Additional";
+ case lldb::eSymbolTypeCompiler:
+ return "Compiler";
+ case lldb::eSymbolTypeInstrumentation:
+ return "Instrumentation";
+ case lldb::eSymbolTypeUndefined:
+ return "Undefined";
+ case lldb::eSymbolTypeObjCClass:
+ return "ObjCClass";
+ case lldb::eSymbolTypeObjCMetaClass:
+ return "ObjCMetaClass";
+ case lldb::eSymbolTypeObjCIVar:
+ return "ObjCIVar";
+ case lldb::eSymbolTypeReExported:
+ return "ReExported";
+ }
+
+ llvm_unreachable("unhandled symbol type.");
+}
+
+/// 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<DAPGetModuleSymbolsResponseBody>
+DAPGetModuleSymbolsRequestHandler::Run(
+ const DAPGetModuleSymbolsArguments &args) const {
+ DAPGetModuleSymbolsResponseBody response;
+
+ lldb::SBModuleSpec module_spec;
+ if (args.moduleId) {
+ llvm::SmallVector<uint8_t, 20> uuid_bytes;
+ if (!lldb_private::UUID::DecodeUUIDBytesFromString(*args.moduleId,
+ uuid_bytes)
+ .empty())
+ return llvm::make_error<DAPError>("Invalid module ID");
+
+ module_spec.SetUUIDBytes(uuid_bytes.data(), uuid_bytes.size());
+ }
+
+ if (args.moduleName) {
+ lldb::SBFileSpec file_spec;
+ file_spec.SetFilename(args.moduleName->c_str());
+ module_spec.SetFileSpec(file_spec);
+ }
+
+ // Empty request, return empty response.
+ // We use it in the client to check if the lldb-dap server supports this
+ // request.
+ if (!module_spec.IsValid())
+ return response;
+
+ std::vector<DAPSymbol> &symbols = response.symbols;
+ lldb::SBModule module = dap.target.FindModule(module_spec);
+ if (!module.IsValid())
+ return llvm::make_error<DAPError>("Module not found");
+
+ size_t num_symbols = module.GetNumSymbols();
+ for (size_t i = 0; i < num_symbols; ++i) {
+ lldb::SBSymbol symbol = module.GetSymbolAtIndex(i);
+ if (!symbol.IsValid())
+ continue;
+
+ DAPSymbol dap_symbol;
+ dap_symbol.userId = symbol.GetID();
+ dap_symbol.type = SymbolTypeToString(symbol.GetType());
+ dap_symbol.isDebug = symbol.IsDebug();
+ dap_symbol.isSynthetic = symbol.IsSynthetic();
+ dap_symbol.isExternal = symbol.IsExternal();
+
+ lldb::SBAddress start_address = symbol.GetStartAddress();
+ if (start_address.IsValid()) {
+ lldb::addr_t file_address = start_address.GetFileAddress();
+ if (file_address != LLDB_INVALID_ADDRESS)
+ dap_symbol.fileAddress = file_address;
+
+ lldb::addr_t load_address = start_address.GetLoadAddress(dap.target);
+ if (load_address != LLDB_INVALID_ADDRESS)
+ dap_symbol.loadAddress = load_address;
+ }
+
+ dap_symbol.size = symbol.GetSize();
+ dap_symbol.name = symbol.GetName();
+ symbols.push_back(std::move(dap_symbol));
+ }
+
+ return response;
+}
+
+} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 16f8062f97d7b..e605a8bc1b9f1 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -594,6 +594,17 @@ class CancelRequestHandler : public RequestHandler<protocol::CancelArguments,
llvm::Error Run(const protocol::CancelArguments &args) const override;
};
+class DAPGetModuleSymbolsRequestHandler
+ : public RequestHandler<
+ protocol::DAPGetModuleSymbolsArguments,
+ llvm::Expected<protocol::DAPGetModuleSymbolsResponseBody>> {
+public:
+ using RequestHandler::RequestHandler;
+ static llvm::StringLiteral GetCommand() { return "dapGetModuleSymbols"; }
+ llvm::Expected<protocol::DAPGetModuleSymbolsResponseBody>
+ Run(const protocol::DAPGetModuleSymbolsArguments &args) const override;
+};
+
/// A request used in testing to get the details on all breakpoints that are
/// currently set in the target. This helps us to test "setBreakpoints" and
/// "setFunctionBreakpoints" requests to verify we have the correct set of
diff --git a/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp b/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp
index ecb4baef56e80..212bbd2fe7be6 100644
--- a/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp
+++ b/lldb/tools/lldb-dap/Protocol/DAPTypes.cpp
@@ -33,4 +33,31 @@ llvm::json::Value toJSON(const SourceLLDBData &SLD) {
return result;
}
+bool fromJSON(const llvm::json::Value &Params, DAPSymbol &DS,
+ llvm::json::Path P) {
+ json::ObjectMapper O(Params, P);
+ return O && O.map("userId", DS.userId) && O.map("isDebug", DS.isDebug) &&
+ O.map("isSynthetic", DS.isSynthetic) &&
+ O.map("isExternal", DS.isExternal) && O.map("type", DS.type) &&
+ O.map("fileAddress", DS.fileAddress) &&
+ O.mapOptional("loadAddress", DS.loadAddress) &&
+ O.map("size", DS.size) && O.map("name", DS.name);
+}
+
+llvm::json::Value toJSON(const DAPSymbol &DS) {
+ json::Object result{
+ {"userId", DS.userId},
+ {"isDebug", DS.isDebug},
+ {"isSynthetic", DS.isSynthetic},
+ {"isExternal", DS.isExternal},
+ {"type", DS.type},
+ {"fileAddress", DS.fileAddress},
+ {"loadAddress", DS.loadAddress},
+ {"size", DS.size},
+ {"name", DS.name},
+ };
+
+ return result;
+}
+
} // namespace lldb_dap::protocol
\ No newline at end of file
diff --git a/lldb/tools/lldb-dap/Protocol/DAPTypes.h b/lldb/tools/lldb-dap/Protocol/DAPTypes.h
index 716d8b491b258..83f9947b1ab57 100644
--- a/lldb/tools/lldb-dap/Protocol/DAPTypes.h
+++ b/lldb/tools/lldb-dap/Protocol/DAPTypes.h
@@ -48,6 +48,38 @@ struct SourceLLDBData {
bool fromJSON(const llvm::json::Value &, SourceLLDBData &, llvm::json::Path);
llvm::json::Value toJSON(const SourceLLDBData &);
+struct DAPSymbol {
+ /// The symbol uid.
+ uint32_t userId;
+
+ /// True if this symbol is debug information in a symbol.
+ bool isDebug;
+
+ /// True if this symbol is not actually in the symbol table, but synthesized
+ /// from other info in the object file.
+ bool isSynthetic;
+
+ /// True if this symbol is globally visible.
+ bool isExternal;
+
+ /// The symbol type.
+ std::string type;
+
+ /// The symbol file address.
+ lldb::addr_t fileAddress;
+
+ /// The symbol load address.
+ std::optional<lldb::addr_t> loadAddress;
+
+ /// The symbol size.
+ lldb::addr_t size;
+
+ /// The symbol name.
+ std::string name;
+};
+bool fromJSON(const llvm::json::Value &, DAPSymbol &, llvm::json::Path);
+llvm::json::Value toJSON(const DAPSymbol &);
+
} // namespace lldb_dap::protocol
#endif
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 29855ca50e9e0..e0d49a7d03ce5 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -598,4 +598,17 @@ json::Value toJSON(const WriteMemoryResponseBody &WMR) {
return result;
}
+bool fromJSON(const llvm::json::Value &Params,
+ DAPGetModuleSymbolsArguments &Args, llvm::json::Path P) {
+ json::ObjectMapper O(Params, P);
+ return O && O.mapOptional("moduleId", Args.moduleId) &&
+ O.mapOptional("moduleName", Args.moduleName);
+}
+
+llvm::json::Value toJSON(const DAPGetModuleSymbolsResponseBody &DGMSR) {
+ json::Object result;
+ result.insert({"symbols", DGMSR.symbols});
+ return result;
+}
+
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index c45ee10e77d1c..ebf58424f6aa4 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -981,6 +981,23 @@ struct WriteMemoryResponseBody {
};
llvm::json::Value toJSON(const WriteMemoryResponseBody &);
+struct DAPGetModuleSymbolsArguments {
+ /// The module UUID for which to retrieve symbols.
+ std::optional<std::string> moduleId;
+
+ /// The module path.
+ std::optional<std::string> moduleName;
+};
+bool fromJSON(const llvm::json::Value &, DAPGetModuleSymbolsArguments &,
+ llvm::json::Path);
+
+/// Response to `getModuleSymbols` request.
+struct DAPGetModuleSymbolsResponseBody {
+ /// The symbols for the specified module.
+ std::vector<DAPSymbol> symbols;
+};
+llvm::json::Value toJSON(const DAPGetModuleSymbolsResponseBody &);
+
} // namespace lldb_dap::protocol
#endif
diff --git a/lldb/tools/lldb-dap/package-lock.json b/lldb/tools/lldb-dap/package-lock.json
index 1969b196accc6..26db1ce6df2fd 100644
--- a/lldb/tools/lldb-dap/package-lock.json
+++ b/lldb/tools/lldb-dap/package-lock.json
@@ -1,20 +1,24 @@
{
"name": "lldb-dap",
- "version": "0.2.15",
+ "version": "0.2.16",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "lldb-dap",
- "version": "0.2.15",
+ "version": "0.2.16",
"license": "Apache 2.0 License with LLVM exceptions",
"devDependencies": {
"@types/node": "^18.19.41",
+ "@types/tabulator-tables": "^6.2.10",
"@types/vscode": "1.75.0",
+ "@types/vscode-webview": "^1.57.5",
"@vscode/debugprotocol": "^1.68.0",
"@vscode/vsce": "^3.2.2",
+ "esbuild": "^0.25.9",
"prettier": "^3.4.2",
"prettier-plugin-curly": "^0.3.1",
+ "tabulator-tables": "^6.3.1",
"typescript": "^5.7.3"
},
"engines": {
@@ -318,6 +322,448 @@
"node": ">=6.9.0"
}
},
+ "node_modules/@esbuild/aix-ppc64": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.org/@esbuild/aix-ppc64/-/aix-ppc64-0.25.9.tgz",
+ "integrity": "sha512-OaGtL73Jck6pBKjNIe24BnFE6agGl+6KxDtTfHhy1HmhthfKouEcOhqpSL64K4/0WCtbKFLOdzD/44cJ4k9opA==",
+ "cpu": [
+ "ppc64"
+ ],
+ "dev": true,
+ "license": "MIT",
+ "optional": true,
+ "os": [
+ "aix"
+ ],
+ "engines": {
+ "node": ">=18"
+ }
+ },
+ "node_modules/@esbuild/android-arm": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.org/@esbuild/android-arm/-/android-arm-0.25.9.tgz",
+ "integrity": "sha512-5WNI1DaMtxQ7t7B6xa572XMXpHAaI/9Hnhk8lcxF4zVN4xstUgTlvuGDorBguKEnZO70qwEcLpfifMLoxiPqHQ==",
+ "cpu": [
+ "arm"
+ ],
+ "dev": true,
+ "license": "MIT",
+ "optional": true,
+ "os": [
+ "android"
+ ],
+ "engines": {
+ "node": ">=18"
+ }
+ },
+ "node_modules/@esbuild/android-arm64": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.org/@esbuild/android-arm64/-/android-arm64-0.25.9.tgz",
+ "integrity": "sha512-IDrddSmpSv51ftWslJMvl3Q2ZT98fUSL2/rlUXuVqRXHCs5EUF1/f+jbjF5+NG9UffUDMCiTyh8iec7u8RlTLg==",
+ "cpu": [
+ "arm64"
+ ],
+ "dev": true,
+ "license": "MIT",
+ "optional": true,
+ "os": [
+ "android"
+ ],
+ "engines": {
+ "node": ">=18"
+ }
+ },
+ "node_modules/@esbuild/android-x64": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.org/@esbuild/android-x64/-/android-x64-0.25.9.tgz",
+ "integrity": "sha512-I853iMZ1hWZdNllhVZKm34f4wErd4lMyeV7BLzEExGEIZYsOzqDWDf+y082izYUE8gtJnYHdeDpN/6tUdwvfiw==",
+ "cpu": [
+ "x64"
+ ],
+ "dev": true,
+ "license": "MIT",
+ "optional": true,
+ "os": [
+ "android"
+ ],
+ "engines": {
+ "node": ">=18"
+ }
+ },
+ "node_modules/@esbuild/darwin-arm64": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.org/@esbuild/darwin-arm64/-/darwin-arm64-0.25.9.tgz",
+ "integrity": "sha512-XIpIDMAjOELi/9PB30vEbVMs3GV1v2zkkPnuyRRURbhqjyzIINwj+nbQATh4H9GxUgH1kFsEyQMxwiLFKUS6Rg==",
+ "cpu": [
+ "arm64"
+ ],
+ "dev": true,
+ "license": "MIT",
+ "optional": true,
+ "os": [
+ "darwin"
+ ],
+ "engines": {
+ "node": ">=18"
+ }
+ },
+ "node_modules/@esbuild/darwin-x64": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.org/@esbuild/darwin-x64/-/darwin-x64-0.25.9.tgz",
+ "integrity": "sha512-jhHfBzjYTA1IQu8VyrjCX4ApJDnH+ez+IYVEoJHeqJm9VhG9Dh2BYaJritkYK3vMaXrf7Ogr/0MQ8/MeIefsPQ==",
+ "cpu": [
+ "x64"
+ ],
+ "dev": true,
+ "license": "MIT",
+ "optional": true,
+ "os": [
+ "darwin"
+ ],
+ "engines": {
+ "node": ">=18"
+ }
+ },
+ "node_modules/@esbuild/freebsd-arm64": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.org/@esbuild/freebsd-arm64/-/freebsd-arm64-0.25.9.tgz",
+ "integrity": "sha512-z93DmbnY6fX9+KdD4Ue/H6sYs+bhFQJNCPZsi4XWJoYblUqT06MQUdBCpcSfuiN72AbqeBFu5LVQTjfXDE2A6Q==",
+ "cpu": [
+ "arm64"
+ ],
+ "dev": true,
+ "license": "MIT",
+ "optional": true,
+ "os": [
+ "freebsd"
+ ],
+ "engines": {
+ "node": ">=18"
+ }
+ },
+ "node_modules/@esbuild/freebsd-x64": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.org/@esbuild/freebsd-x64/-/freebsd-x64-0.25.9.tgz",
+ "integrity": "sha512-mrKX6H/vOyo5v71YfXWJxLVxgy1kyt1MQaD8wZJgJfG4gq4DpQGpgTB74e5yBeQdyMTbgxp0YtNj7NuHN0PoZg==",
+ "cpu": [
+ "x64"
+ ],
+ "dev": true,
+ "license": "MIT",
+ "optional": true,
+ "os": [
+ "freebsd"
+ ],
+ "engines": {
+ "node": ">=18"
+ }
+ },
+ "node_modules/@esbuild/linux-arm": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.org/@esbuild/linux-arm/-/linux-arm-0.25.9.tgz",
+ "integrity": "sha512-HBU2Xv78SMgaydBmdor38lg8YDnFKSARg1Q6AT0/y2ezUAKiZvc211RDFHlEZRFNRVhcMamiToo7bDx3VEOYQw==",
+ "cpu": [
+ "arm"
+ ],
+ "dev": true,
+ "license": "MIT",
+ "optional": true,
+ "os": [
+ "linux"
+ ],
+ "engines": {
+ "node": ">=18"
+ }
+ },
+ "node_modules/@esbuild/linux-arm64": {
+ "version": "0.25.9",
+ "resolved": "https://registry.npmjs.o...
[truncated]
|
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.
Do we need to add these files to the lldb-dap/.vscodeignore file? Otherwise they might not get picked up when we package the extension.
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.
it's copied to ./out/webview
, so it's preserved by the !out/**
rule
✅ With the latest revision this PR passed the C/C++ code formatter. |
✅ With the latest revision this PR passed the Python code formatter. |
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.
This looks almost good to go, but there are just a few remaining minor issues left.
lldb/include/lldb/API/SBSymbol.h
Outdated
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.
please add a docstring
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.
💯
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.
use camel case
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.
💯
lldb/test/API/tools/lldb-dap/moduleSymbols/TestDAP_moduleSymbols.py
Outdated
Show resolved
Hide resolved
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.
With this, we'd potentially send 2 capabilities events back to back.
Should we merge this with the SendTargetBasedCapabilities on line 47?
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.
Changed it to send these capabilities in the same event
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.
I think we could put the command behind some kind of namespace, to prevent name collisions in future dap-capabilities. something along lldb_moduleSymbols
or __moduleSymbols
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.
Good suggestion. I've been trying to use the prefix __lldb_
for most extensions. I think we can keep using it.
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.
+1
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.
💯
This lgtm, please get a final approval from @ashgti |
Just to double check, can you run Otherwise, LGTM with the last few comments. |
I haven't looked at the code yet, but based on the video, I'm assuming this happens "on demand", right? A user has to interact with the module view to request the symbols? |
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.
This could return an llvm::StringLiteral
. Would it be worthwhile to expose Symbol::GetTypeAsString
through the SB API instead of duplicating this?
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.
💯
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.
Similar question, we already have a StringSwitch like this for the JSON encoding in SymbolFileJSON. Would it be worthwhile to make that a method an expose it through the SB API?
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.
💯
Yes, the user has to click on "Show Module Symbols" from the context menu of the module tree or through the command palette, and only then the symbols are retrieved from the lldb-dap server |
…ead of lldb version
1904676
to
300819c
Compare
@JDevlieghere Added public functions to |
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 modulo comments.
lldb/source/API/SBTarget.cpp
Outdated
|
||
SBModule sb_module; | ||
if (TargetSP target_sp = GetSP(); target_sp && sb_module_spec.IsValid()) { | ||
// The module list is thread safe, no need to lock |
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.
// The module list is thread safe, no need to lock | |
// The module list is thread safe, no need to lock. |
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.
💯
} | ||
|
||
const char *SBSymbol::GetTypeAsString(lldb::SymbolType symbol_type) { | ||
return Symbol::GetTypeAsString(symbol_type); |
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.
return Symbol::GetTypeAsString(symbol_type); | |
LLDB_INSTRUMENT_VA(type); | |
return Symbol::GetTypeAsString(symbol_type); |
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.
💯
} | ||
|
||
lldb::SymbolType SBSymbol::GetTypeFromString(const char *str) { | ||
return Symbol::GetTypeFromString(str); |
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.
return Symbol::GetTypeFromString(str); | |
LLDB_INSTRUMENT_VA(type); | |
return Symbol::GetTypeFromString(str); |
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.
💯
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/22959 Here is the relevant piece of the build log for the reference
|
Hi The commit [8b64cd8] (“[lldb-dap] Add module symbol table viewer to VS Code extension”) is breaking the LLDB testsuite on the AArch64 buildbots. https://lab.llvm.org/buildbot/#/builders/59/builds/22959 The issue seems to be that a __lldb_moduleSymbols request to the lldb-dap is resulting in the adapter crash. I am reverting the commit to get the bots green again. Please take another look at the implementation, and feel free to re-land once the crash is resolved. Thanks! |
…#140626 (#153836)" This reverts commit 8b64cd8. This breaks lldb-aarch64-* bots causing a crash in lldb-dap while running test TestDAP_moduleSymbols.py https://lab.llvm.org/buildbot/#/builders/59/builds/22959 https://lab.llvm.org/buildbot/#/builders/141/builds/10975
Seems the crash is caused by invalid symbols with a |
llvm#140626 (llvm#153836)" This reverts commit 2b8e806.
…40626 (llvm#153836) - VS Code extension: - Add module symbol table viewer using [Tabulator](https://tabulator.info/) for sorting and formatting rows. - Add context menu action to the modules tree. - lldb-dap - Add `DAPGetModuleSymbolsRequest` to get symbols from a module. Fixes llvm#140626 [Screencast From 2025-08-15 19-12-33.webm](https://github.com/user-attachments/assets/75e2f229-ac82-487c-812e-3ea33a575b70) (cherry picked from commit 8b64cd8)
…llvm#140626 (llvm#153836)" This reverts commit 8b64cd8. This breaks lldb-aarch64-* bots causing a crash in lldb-dap while running test TestDAP_moduleSymbols.py https://lab.llvm.org/buildbot/#/builders/59/builds/22959 https://lab.llvm.org/buildbot/#/builders/141/builds/10975 (cherry picked from commit 2b8e806)
…40626 (llvm#153836) - VS Code extension: - Add module symbol table viewer using [Tabulator](https://tabulator.info/) for sorting and formatting rows. - Add context menu action to the modules tree. - lldb-dap - Add `DAPGetModuleSymbolsRequest` to get symbols from a module. Fixes llvm#140626 [Screencast From 2025-08-15 19-12-33.webm](https://github.com/user-attachments/assets/75e2f229-ac82-487c-812e-3ea33a575b70) (cherry picked from commit 8b64cd8)
…llvm#140626 (llvm#153836)" This reverts commit 8b64cd8. This breaks lldb-aarch64-* bots causing a crash in lldb-dap while running test TestDAP_moduleSymbols.py https://lab.llvm.org/buildbot/#/builders/59/builds/22959 https://lab.llvm.org/buildbot/#/builders/141/builds/10975 (cherry picked from commit 2b8e806)
DAPGetModuleSymbolsRequest
to get symbols from a module.Fixes #140626
Screencast.From.2025-08-15.19-12-33.webm