https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/222512
Add StackFrame.compileUnitId, an id that names the frame's compile unit within the module already reported as StackFrame.moduleId. Clients resolve it through the compileUnits request, which now reports the same id on each CompileUnit and takes an optional compileUnitIds filter to ask for individual compile units instead of the module's full list. CompileUnitsRequestHandler declared its arguments optional but dereferenced them unconditionally, so a compileUnits request carrying no arguments crashed the adapter. Drop the optional and let the request framework reject the request instead. >From acabd6af4f65cac902254281145be7f4a2437ef9 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <[email protected]> Date: Wed, 9 Sep 2026 21:49:26 -0700 Subject: [PATCH] [lldb-dap] Report the compile unit of a stack frame Add StackFrame.compileUnitId, an id that names the frame's compile unit within the module already reported as StackFrame.moduleId. Clients resolve it through the compileUnits request, which now reports the same id on each CompileUnit and takes an optional compileUnitIds filter to ask for individual compile units instead of the module's full list. CompileUnitsRequestHandler declared its arguments optional but dereferenced them unconditionally, so a compileUnits request carrying no arguments crashed the adapter. Drop the optional and let the request framework reject the request instead. --- .../lldbsuite/test/tools/lldb_dap/types.py | 13 ++-- .../lldb-dap/coreFile/TestDAP_coreFile.py | 3 + .../API/tools/lldb-dap/stackTrace/Makefile | 2 +- .../lldb-dap/stackTrace/TestDAP_stackTrace.py | 70 ++++++++++++++++++- .../API/tools/lldb-dap/stackTrace/other.c | 1 + .../lldb-dap/utils/TestDAPUtils_Types.py | 1 + .../Handler/CompileUnitsRequestHandler.cpp | 42 ++++++----- lldb/tools/lldb-dap/Handler/RequestHandler.h | 5 +- .../Handler/StackTraceRequestHandler.cpp | 3 + .../lldb-dap/Protocol/ProtocolRequests.cpp | 3 +- .../lldb-dap/Protocol/ProtocolRequests.h | 3 + .../tools/lldb-dap/Protocol/ProtocolTypes.cpp | 4 +- lldb/tools/lldb-dap/Protocol/ProtocolTypes.h | 6 ++ lldb/tools/lldb-dap/ProtocolUtils.cpp | 16 +++++ lldb/tools/lldb-dap/ProtocolUtils.h | 4 ++ lldb/unittests/DAP/ProtocolRequestsTest.cpp | 10 ++- lldb/unittests/DAP/ProtocolTypesTest.cpp | 18 +++++ 17 files changed, 172 insertions(+), 32 deletions(-) create mode 100644 lldb/test/API/tools/lldb-dap/stackTrace/other.c diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb_dap/types.py b/lldb/packages/Python/lldbsuite/test/tools/lldb_dap/types.py index 94980cb081f73..24b33ae938161 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb_dap/types.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb_dap/types.py @@ -779,6 +779,12 @@ class Thread: name: str +@dataclass(frozen=True) +class CompileUnit: + id: int + compileUnitPath: str + + @dataclass(frozen=True) class StackFrame: id: int @@ -791,6 +797,7 @@ class StackFrame: canRestart: Optional[bool] = None instructionPointerReference: Optional[str] = None moduleId: Optional[Union[int, str]] = None + compileUnitId: Optional[int] = None presentationHint: Optional[StackFramePresentationHint] = None @@ -1349,11 +1356,6 @@ class SetInstructionBreakpointsArgs: response_class_ = AnyBreakpointsResponse -@dataclass(frozen=True) -class CompileUnit: - compileUnitPath: str - - @dataclass(frozen=True) class CompileUnitsResponse(Response): @dataclass(frozen=True) @@ -1367,6 +1369,7 @@ class Body: @args_protocol class CompileUnitsArgs: moduleId: str + compileUnitIds: Optional[List[int]] = None command_ = "compileUnits" response_class_ = CompileUnitsResponse diff --git a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py index 460efd1757735..4529c92ea1a82 100644 --- a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py +++ b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py @@ -28,6 +28,7 @@ presentationHint="deemphasize", ), instructionPointerReference="0x40011C", + compileUnitId=0, ), StackFrame( column=0, @@ -41,6 +42,7 @@ presentationHint="deemphasize", ), instructionPointerReference="0x400142", + compileUnitId=0, ), StackFrame( column=0, @@ -54,6 +56,7 @@ presentationHint="deemphasize", ), instructionPointerReference="0x40015F", + compileUnitId=0, ), ] diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/Makefile b/lldb/test/API/tools/lldb-dap/stackTrace/Makefile index 10495940055b6..118f0aa59ef6f 100644 --- a/lldb/test/API/tools/lldb-dap/stackTrace/Makefile +++ b/lldb/test/API/tools/lldb-dap/stackTrace/Makefile @@ -1,3 +1,3 @@ -C_SOURCES := main.c +C_SOURCES := main.c other.c include Makefile.rules diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py index 7909f92ec7414..f5ce101a8cbb3 100644 --- a/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py +++ b/lldb/test/API/tools/lldb-dap/stackTrace/TestDAP_stackTrace.py @@ -8,7 +8,13 @@ from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import line_number from lldbsuite.test.tools.lldb_dap import DAPTestCaseBase -from lldbsuite.test.tools.lldb_dap.types import LaunchArgs, StackFrame, StackFrameFormat +from lldbsuite.test.tools.lldb_dap.types import ( + CompileUnit, + CompileUnitsArgs, + LaunchArgs, + StackFrame, + StackFrameFormat, +) class _RecurseSource(NamedTuple): @@ -294,3 +300,65 @@ def test_stack_frame_module_id(self) -> None: expected_id, f"expected moduleId '{expected_id}' for {source_name}, got: {module_id}", ) + + @skipIfWindows + def test_stack_frame_compile_unit_id(self) -> None: + """Test that a stack frame's compileUnitId resolves to its source file.""" + program = self.getBuildArtifact("a.out") + session = self.build_and_create_session() + source = self.getSourcePath("main.c") + lines = [line_number(source, "recurse end")] + + with session.configure(LaunchArgs(program=program)) as ctx: + breakpoint_ids = session.resolve_source_breakpoints(source, lines) + + stop_event = session.verify_stopped_on_breakpoint( + breakpoint_ids, after=ctx.process_event + ) + thread_id = self.expect_not_none(stop_event.body.threadId) + stack_frames = session.stack_trace(thread_id).body.stackFrames + + ids = { + frame.compileUnitId + for frame in stack_frames + if frame.compileUnitId is not None + } + self.assertEqual(len(ids), 1, f"expected a single compile unit, got: {ids}") + compile_unit_id = ids.pop() + self.assertEqual(stack_frames[0].compileUnitId, compile_unit_id) + + expected = CompileUnit(id=compile_unit_id, compileUnitPath=source) + module_id = self.expect_not_none(stack_frames[0].moduleId) + + response = session.send_request(CompileUnitsArgs(moduleId=module_id)).result() + all_units = response.body.compileUnits + self.assertIn(expected, all_units) + self.assertGreater(len(all_units), 1, "test needs a module with several CUs") + + response = session.send_request( + CompileUnitsArgs(moduleId=module_id, compileUnitIds=[compile_unit_id]) + ).result() + self.assertEqual(response.body.compileUnits, [expected]) + + @skipIfWindows + def test_unknown_compile_unit_id(self) -> None: + """Test that an id no compile unit has resolves to nothing.""" + program = self.getBuildArtifact("a.out") + session = self.build_and_create_session() + source = self.getSourcePath("main.c") + lines = [line_number(source, "recurse end")] + + with session.configure(LaunchArgs(program=program)) as ctx: + breakpoint_ids = session.resolve_source_breakpoints(source, lines) + + stop_event = session.verify_stopped_on_breakpoint( + breakpoint_ids, after=ctx.process_event + ) + thread_id = self.expect_not_none(stop_event.body.threadId) + top_frame = session.stack_trace(thread_id).body.stackFrames[0] + module_id = self.expect_not_none(top_frame.moduleId) + + response = session.send_request( + CompileUnitsArgs(moduleId=module_id, compileUnitIds=[9999]) + ).result() + self.assertEqual(response.body.compileUnits, []) diff --git a/lldb/test/API/tools/lldb-dap/stackTrace/other.c b/lldb/test/API/tools/lldb-dap/stackTrace/other.c new file mode 100644 index 0000000000000..365ec2cde1cc6 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/stackTrace/other.c @@ -0,0 +1 @@ +int other(int val) { return val + 1; } diff --git a/lldb/test/API/tools/lldb-dap/utils/TestDAPUtils_Types.py b/lldb/test/API/tools/lldb-dap/utils/TestDAPUtils_Types.py index 08d080cb4e49d..3211eb1e55de6 100644 --- a/lldb/test/API/tools/lldb-dap/utils/TestDAPUtils_Types.py +++ b/lldb/test/API/tools/lldb-dap/utils/TestDAPUtils_Types.py @@ -81,6 +81,7 @@ def test_encode_and_decode(self): "line": 23, "moduleId": "2833EAD0-0FDC-66C8-88B1-8C6E1D82736C-AE103AE6", "name": "main", + "compileUnitId": 4, "source": { "name": "convert.cpp", "path": "/path/to/where/convert.cpp", diff --git a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp index d24072c8cc05d..df842bee45806 100644 --- a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp @@ -9,35 +9,39 @@ #include "DAP.h" #include "EventHelper.h" #include "Protocol/ProtocolRequests.h" +#include "ProtocolUtils.h" #include "RequestHandler.h" -#include "lldb/Host/PosixApi.h" // IWYU pragma: keep using namespace lldb_dap; using namespace lldb_dap::protocol; -static CompileUnit CreateCompileUnit(lldb::SBCompileUnit &unit) { - char unit_path_arr[PATH_MAX]; - unit.GetFileSpec().GetPath(unit_path_arr, sizeof(unit_path_arr)); - std::string unit_path(unit_path_arr); - return {std::move(unit_path)}; -} - -/// The `compileUnits` request returns an array of path of compile units for -/// given module specified by `moduleId`. -llvm::Expected<CompileUnitsResponseBody> CompileUnitsRequestHandler::Run( - const std::optional<CompileUnitsArguments> &args) const { +/// The `compileUnits` request returns the compile units of the module named by +/// `moduleId`, narrowed to `compileUnitIds` when specified. +llvm::Expected<CompileUnitsResponseBody> +CompileUnitsRequestHandler::Run(const CompileUnitsArguments &args) const { std::vector<CompileUnit> units; + int num_modules = dap.target.GetNumModules(); for (int i = 0; i < num_modules; i++) { - auto curr_module = dap.target.GetModuleAtIndex(i); - if (args->moduleId == curr_module.GetUUIDString()) { - int num_units = curr_module.GetNumCompileUnits(); - for (int j = 0; j < num_units; j++) { - auto curr_unit = curr_module.GetCompileUnitAtIndex(j); - units.emplace_back(CreateCompileUnit(curr_unit)); + lldb::SBModule curr_module = dap.target.GetModuleAtIndex(i); + if (args.moduleId != curr_module.GetUUIDString()) + continue; + + if (args.compileUnitIds.empty()) { + const uint32_t num_units = curr_module.GetNumCompileUnits(); + for (uint32_t j = 0; j < num_units; j++) { + if (std::optional<CompileUnit> unit = + CreateCompileUnit(curr_module.GetCompileUnitAtIndex(j))) + units.emplace_back(std::move(*unit)); + } + } else { + for (const uint32_t id : args.compileUnitIds) { + if (std::optional<CompileUnit> unit = + CreateCompileUnit(curr_module.GetCompileUnitAtIndex(id))) + units.emplace_back(std::move(*unit)); } - break; } + break; } return CompileUnitsResponseBody{std::move(units)}; } diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index 0cab2b1a9b2e6..e99b287b28f58 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -473,14 +473,13 @@ class SetInstructionBreakpointsRequestHandler class CompileUnitsRequestHandler : public RequestHandler< - std::optional<protocol::CompileUnitsArguments>, + protocol::CompileUnitsArguments, llvm::Expected<protocol::CompileUnitsResponseBody>> { public: using RequestHandler::RequestHandler; static llvm::StringLiteral GetCommand() { return "compileUnits"; } llvm::Expected<protocol::CompileUnitsResponseBody> - Run(const std::optional<protocol::CompileUnitsArguments> &args) - const override; + Run(const protocol::CompileUnitsArguments &args) const override; }; class ModulesRequestHandler final diff --git a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp index 6e6f9b0e5b285..5bf95f42cee41 100644 --- a/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp @@ -81,6 +81,9 @@ static StackFrame CreateStackFrame(DAP &dap, lldb::SBFrame &frame, if (llvm::StringRef uuid = module.GetUUIDString(); !uuid.empty()) stack_frame.moduleId = uuid.str(); } + if (const uint32_t cu_id = frame.GetCompileUnit().GetIDInModule(); + cu_id != LLDB_INVALID_INDEX32) + stack_frame.compileUnitId = cu_id; return stack_frame; } diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index 078f607dec2b8..c952c17512952 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -776,7 +776,8 @@ llvm::json::Value toJSON(const LocationsResponseBody &Body) { bool fromJSON(const llvm::json::Value &Params, CompileUnitsArguments &Args, llvm::json::Path Path) { json::ObjectMapper O(Params, Path); - return O && O.map("moduleId", Args.moduleId); + return O && O.map("moduleId", Args.moduleId) && + O.mapOptional("compileUnitIds", Args.compileUnitIds); } llvm::json::Value toJSON(const CompileUnitsResponseBody &Body) { diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 7d7a4647c2328..d8cc9d4cad223 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -1241,6 +1241,9 @@ llvm::json::Value toJSON(const LocationsResponseBody &); struct CompileUnitsArguments { /// The ID of the module. String moduleId; + + /// IDs of compile units to return, or empty for all. + std::vector<uint32_t> compileUnitIds; }; bool fromJSON(const llvm::json::Value &, CompileUnitsArguments &, llvm::json::Path); diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp index b70aeb661f3e1..929e0765c197f 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -1171,7 +1171,7 @@ json::Value toJSON(const ExceptionDetails &ED) { } llvm::json::Value toJSON(const CompileUnit &CU) { - json::Object result{{"compileUnitPath", CU.compileUnitPath}}; + json::Object result{{"id", CU.id}, {"compileUnitPath", CU.compileUnitPath}}; return result; } @@ -1224,6 +1224,8 @@ llvm::json::Value toJSON(const StackFrame &SF) { EncodeMemoryReference(SF.instructionPointerReference)}); if (SF.moduleId) result.insert({"moduleId", *SF.moduleId}); + if (SF.compileUnitId) + result.insert({"compileUnitId", *SF.compileUnitId}); if (SF.presentationHint != StackFrame::ePresentationHintNone) result.insert({"presentationHint", SF.presentationHint}); diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h index 39eaeb064ccd2..15a158c7688e8 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -1044,6 +1044,9 @@ struct ExceptionDetails { llvm::json::Value toJSON(const ExceptionDetails &); struct CompileUnit { + /// Identifier within the module. Not unique across modules. + uint32_t id = 0; + /// Path of compile unit. String compileUnitPath; }; @@ -1126,6 +1129,9 @@ struct StackFrame { /// The module associated with this frame, if any. std::optional<String> moduleId; + /// The compile unit associated with this frame, if any. + std::optional<uint32_t> compileUnitId; + /// A hint for how to present this frame in the UI. A value of `label` can be /// used to indicate that the frame is an artificial frame that is used as a /// visual label or separator. A value of `subtle` can be used to change the diff --git a/lldb/tools/lldb-dap/ProtocolUtils.cpp b/lldb/tools/lldb-dap/ProtocolUtils.cpp index 3bd5115f02972..1d1349455bdf0 100644 --- a/lldb/tools/lldb-dap/ProtocolUtils.cpp +++ b/lldb/tools/lldb-dap/ProtocolUtils.cpp @@ -95,6 +95,22 @@ std::string ConvertDebugInfoSizeToString(uint64_t debug_size) { return oss.str(); } +std::optional<protocol::CompileUnit> +CreateCompileUnit(const lldb::SBCompileUnit &unit) { + const lldb::SBFileSpec file_spec = unit.GetFileSpec(); + if (!file_spec.IsValid()) + return std::nullopt; + + std::array<char, PATH_MAX> path_buffer{}; + const uint32_t path_size = + file_spec.GetPath(path_buffer.data(), path_buffer.size()); + + protocol::CompileUnit result; + result.id = unit.GetIDInModule(); + result.compileUnitPath = std::string(path_buffer.data(), path_size); + return result; +} + std::optional<protocol::Module> CreateModule(const lldb::SBTarget &target, lldb::SBModule &module, bool id_only) { diff --git a/lldb/tools/lldb-dap/ProtocolUtils.h b/lldb/tools/lldb-dap/ProtocolUtils.h index c7742c04c2c68..13d46ccb1e1b3 100644 --- a/lldb/tools/lldb-dap/ProtocolUtils.h +++ b/lldb/tools/lldb-dap/ProtocolUtils.h @@ -17,6 +17,7 @@ #include "Protocol/ProtocolTypes.h" #include "lldb/API/SBAddress.h" +#include "lldb/API/SBCompileUnit.h" #include "lldb/lldb-types.h" namespace lldb_dap { @@ -41,6 +42,9 @@ std::optional<protocol::Module> CreateModule(const lldb::SBTarget &target, lldb::SBModule &module, bool id_only = false); +std::optional<protocol::CompileUnit> +CreateCompileUnit(const lldb::SBCompileUnit &unit); + /// Create a "Source" JSON object as described in the debug adapter definition. /// /// \param[in] file diff --git a/lldb/unittests/DAP/ProtocolRequestsTest.cpp b/lldb/unittests/DAP/ProtocolRequestsTest.cpp index 601a6ab21f0a7..af0d69f3a9ac4 100644 --- a/lldb/unittests/DAP/ProtocolRequestsTest.cpp +++ b/lldb/unittests/DAP/ProtocolRequestsTest.cpp @@ -251,6 +251,12 @@ TEST(ProtocolRequestsTest, CompileUnitsArguments) { parse<CompileUnitsArguments>(R"({"moduleId": "42"})"); ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); EXPECT_EQ(expected->moduleId, "42"); + EXPECT_THAT(expected->compileUnitIds, testing::IsEmpty()); + + expected = parse<CompileUnitsArguments>( + R"({"moduleId": "42", "compileUnitIds": [3, 9]})"); + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_THAT(expected->compileUnitIds, testing::ElementsAre(3u, 9u)); // Check required keys. EXPECT_THAT_EXPECTED(parse<CompileUnitsArguments>(R"({})"), @@ -259,15 +265,17 @@ TEST(ProtocolRequestsTest, CompileUnitsArguments) { TEST(ProtocolRequestsTest, CompileUnitsResponseBody) { CompileUnitsResponseBody body; - body.compileUnits = {{"main.cpp"}, {"util.cpp"}}; + body.compileUnits = {{1, "main.cpp"}, {2, "util.cpp"}}; // Check required keys. Expected<json::Value> expected = parse(R"({ "compileUnits": [ { + "id": 1, "compileUnitPath": "main.cpp" }, { + "id": 2, "compileUnitPath": "util.cpp" } ] diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp index 996b6e3ec6ba5..54a94fb842d45 100644 --- a/lldb/unittests/DAP/ProtocolTypesTest.cpp +++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp @@ -1270,6 +1270,24 @@ TEST(ProtocolTypesTest, StackFrame) { ASSERT_THAT_EXPECTED(expected_frame, llvm::Succeeded()); EXPECT_EQ(PrettyPrint(*expected_frame), PrettyPrint(frame)); + + frame.id = 3; + frame.canRestart = false; + frame.instructionPointerReference = LLDB_INVALID_ADDRESS; + frame.presentationHint = StackFrame::ePresentationHintNone; + frame.moduleId = "2E6A5E9A-1D0C-3B2C-9C3E-8A6F0B1D2E3F"; + frame.compileUnitId = 7; + expected_frame = parse(R"({ + "id": 3, + "name": "foo", + "line": 0, + "column": 0, + "moduleId": "2E6A5E9A-1D0C-3B2C-9C3E-8A6F0B1D2E3F", + "compileUnitId": 7 + })"); + + ASSERT_THAT_EXPECTED(expected_frame, llvm::Succeeded()); + EXPECT_EQ(PrettyPrint(*expected_frame), PrettyPrint(frame)); } TEST(ProtocolTypesTest, DAPSession) { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
