https://github.com/satyajanga updated https://github.com/llvm/llvm-project/pull/130841
>From 55d3b7d0ecf103cc97942c1406926853c35356c1 Mon Sep 17 00:00:00 2001 From: satya janga <satyaja...@fb.com> Date: Tue, 11 Mar 2025 13:39:08 -0700 Subject: [PATCH 1/3] Make breakpoint stop reason more accurate --- .../test/tools/lldb-dap/lldbdap_testcase.py | 3 ++- lldb/tools/lldb-dap/DAP.cpp | 26 +++++++++++++++++++ lldb/tools/lldb-dap/DAP.h | 12 ++++++--- lldb/tools/lldb-dap/JSONUtils.cpp | 12 ++++++--- 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py index 70b04b051e0ec..5faf83ca826a0 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py @@ -86,6 +86,7 @@ def verify_breakpoint_hit(self, breakpoint_ids): if ( body["reason"] != "breakpoint" and body["reason"] != "instruction breakpoint" + and body["reason"] != "function breakpoint" ): continue if "description" not in body: @@ -100,7 +101,7 @@ def verify_breakpoint_hit(self, breakpoint_ids): # location. description = body["description"] for breakpoint_id in breakpoint_ids: - match_desc = "breakpoint %s." % (breakpoint_id) + match_desc = "%s %s." % (body["reason"], breakpoint_id) if match_desc in description: return self.assertTrue(False, "breakpoint not hit") diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index edd3b31be8ff7..485c420dc59e8 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1128,6 +1128,32 @@ DAP::GetInstructionBPFromStopReason(lldb::SBThread &thread) { return inst_bp; } +FunctionBreakpoint *DAP::GetFunctionBPFromStopReason(lldb::SBThread &thread) { + const auto num = thread.GetStopReasonDataCount(); + FunctionBreakpoint *func_bp = nullptr; + for (size_t i = 0; i < num; i += 2) { + // thread.GetStopReasonDataAtIndex(i) will return the bp ID and + // thread.GetStopReasonDataAtIndex(i+1) will return the location + // within that breakpoint. We only care about the bp ID so we can + // see if this is an function breakpoint that is getting hit. + lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i); + func_bp = GetFunctionBreakPoint(bp_id); + // If any breakpoint is not an function breakpoint, then stop and + // report this as a normal breakpoint + if (func_bp == nullptr) + return nullptr; + } + return func_bp; +} + +FunctionBreakpoint *DAP::GetFunctionBreakPoint(const lldb::break_id_t bp_id) { + for (auto &bp : function_breakpoints) { + if (bp.second.bp.GetID() == bp_id) + return &bp.second; + } + return nullptr; +} + lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) { switch (variablesReference) { case VARREF_LOCALS: diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 3ff1992b61f5b..ea72b41d02516 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -125,21 +125,21 @@ struct Variables { struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {}; + explicit StartDebuggingRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit ReplModeRequestHandler(DAP &d) : dap(d) {}; + explicit ReplModeRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct SendEventRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit SendEventRequestHandler(DAP &d) : dap(d) {}; + explicit SendEventRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; @@ -383,6 +383,12 @@ struct DAP { InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread); + FunctionBreakpoint *GetFunctionBPFromStopReason(lldb::SBThread &thread); + + FunctionBreakpoint *GetFunctionBreakPoint(const lldb::break_id_t bp_id); + + void WaitWorkerThreadsToExit(); + private: // Send the JSON in "json_str" to the "out" stream. Correctly send the // "Content-Length:" field followed by the length, followed by the raw diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 932145b1799bd..cd0d518cd4f7d 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -967,17 +967,23 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, body.try_emplace("reason", "exception"); EmplaceSafeString(body, "description", exc_bp->label); } else { + std::string reason = "breakpoint"; InstructionBreakpoint *inst_bp = dap.GetInstructionBPFromStopReason(thread); if (inst_bp) { - body.try_emplace("reason", "instruction breakpoint"); + reason = "instruction breakpoint"; } else { - body.try_emplace("reason", "breakpoint"); + FunctionBreakpoint *function_bp = + dap.GetFunctionBPFromStopReason(thread); + if (function_bp) { + reason = "function breakpoint"; + } } + body.try_emplace("reason", reason); lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(0); lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(1); std::string desc_str = - llvm::formatv("breakpoint {0}.{1}", bp_id, bp_loc_id); + llvm::formatv("{0} {1}.{2}", reason, bp_id, bp_loc_id); body.try_emplace("hitBreakpointIds", llvm::json::Array{llvm::json::Value(bp_id)}); EmplaceSafeString(body, "description", desc_str); >From 4bcddcf9cebccb56fd3264e3436d6c16fc772d7d Mon Sep 17 00:00:00 2001 From: satya janga <satyaja...@fb.com> Date: Tue, 11 Mar 2025 13:39:08 -0700 Subject: [PATCH 2/3] Make breakpoint stop reason more accurate --- .../TestDAP_setDataBreakpoints.py | 32 ++++++++++ lldb/tools/lldb-dap/DAP.cpp | 58 ------------------- lldb/tools/lldb-dap/DAP.h | 54 ++++++++++++++++- .../Handler/ExceptionInfoRequestHandler.cpp | 3 +- lldb/tools/lldb-dap/JSONUtils.cpp | 19 ++++-- 5 files changed, 100 insertions(+), 66 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py index a542a318050dd..f5ff971261b6d 100644 --- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py @@ -171,3 +171,35 @@ def test_functionality(self): self.continue_to_next_stop() x_val = self.dap_server.get_local_variable_value("x") self.assertEqual(x_val, "10") + + @skipIfWindows + def test_breakpoint_reason(self): + """Tests setting data breakpoints on variable. Verify that the breakpoint has the correct reason and description in the stopped event.""" + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + source = "main.cpp" + first_loop_break_line = line_number(source, "// first loop breakpoint") + self.set_source_breakpoints(source, [first_loop_break_line]) + self.continue_to_next_stop() + self.dap_server.get_local_variables() + # Test write watchpoints on x, arr[2] + response_x = self.dap_server.request_dataBreakpointInfo(1, "x") + # Test response from dataBreakpointInfo request. + self.assertEqual(response_x["body"]["dataId"].split("/")[1], "4") + self.assertEqual(response_x["body"]["accessTypes"], self.accessTypes) + dataBreakpoints = [ + {"dataId": response_x["body"]["dataId"], "accessType": "write"}, + ] + set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints) + self.assertEqual( + set_response["body"]["breakpoints"], + [{"verified": True}], + ) + + stopped_events = self.continue_to_next_stop() + self.assertEqual(len(stopped_events), 1) + stopped_event = stopped_events[0] + self.assertEqual(stopped_event["body"]["reason"], "data breakpoint") + self.assertEqual(stopped_event["body"]["description"], "data breakpoint 1.1") + x_val = self.dap_server.get_local_variable_value("x") + self.assertEqual(x_val, "2") diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 485c420dc59e8..4929205a4bfe9 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -183,17 +183,6 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const std::string &filter) { return nullptr; } -ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const lldb::break_id_t bp_id) { - // See comment in the other GetExceptionBreakpoint(). - PopulateExceptionBreakpoints(); - - for (auto &bp : *exception_breakpoints) { - if (bp.bp.GetID() == bp_id) - return &bp; - } - return nullptr; -} - llvm::Error DAP::ConfigureIO(std::FILE *overrideOut, std::FILE *overrideErr) { in = lldb::SBFile(std::fopen(DEV_NULL, "r"), /*transfer_ownership=*/true); @@ -475,27 +464,6 @@ DAP::SendFormattedOutput(OutputType o, const char *format, ...) { o, llvm::StringRef(buffer, std::min<int>(actual_length, sizeof(buffer)))); } -ExceptionBreakpoint *DAP::GetExceptionBPFromStopReason(lldb::SBThread &thread) { - const auto num = thread.GetStopReasonDataCount(); - // Check to see if have hit an exception breakpoint and change the - // reason to "exception", but only do so if all breakpoints that were - // hit are exception breakpoints. - ExceptionBreakpoint *exc_bp = nullptr; - for (size_t i = 0; i < num; i += 2) { - // thread.GetStopReasonDataAtIndex(i) will return the bp ID and - // thread.GetStopReasonDataAtIndex(i+1) will return the location - // within that breakpoint. We only care about the bp ID so we can - // see if this is an exception breakpoint that is getting hit. - lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i); - exc_bp = GetExceptionBreakpoint(bp_id); - // If any breakpoint is not an exception breakpoint, then stop and - // report this as a normal breakpoint - if (exc_bp == nullptr) - return nullptr; - } - return exc_bp; -} - lldb::SBThread DAP::GetLLDBThread(const llvm::json::Object &arguments) { auto tid = GetInteger<int64_t>(arguments, "threadId") .value_or(LLDB_INVALID_THREAD_ID); @@ -1128,32 +1096,6 @@ DAP::GetInstructionBPFromStopReason(lldb::SBThread &thread) { return inst_bp; } -FunctionBreakpoint *DAP::GetFunctionBPFromStopReason(lldb::SBThread &thread) { - const auto num = thread.GetStopReasonDataCount(); - FunctionBreakpoint *func_bp = nullptr; - for (size_t i = 0; i < num; i += 2) { - // thread.GetStopReasonDataAtIndex(i) will return the bp ID and - // thread.GetStopReasonDataAtIndex(i+1) will return the location - // within that breakpoint. We only care about the bp ID so we can - // see if this is an function breakpoint that is getting hit. - lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i); - func_bp = GetFunctionBreakPoint(bp_id); - // If any breakpoint is not an function breakpoint, then stop and - // report this as a normal breakpoint - if (func_bp == nullptr) - return nullptr; - } - return func_bp; -} - -FunctionBreakpoint *DAP::GetFunctionBreakPoint(const lldb::break_id_t bp_id) { - for (auto &bp : function_breakpoints) { - if (bp.second.bp.GetID() == bp_id) - return &bp.second; - } - return nullptr; -} - lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) { switch (variablesReference) { case VARREF_LOCALS: diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index ea72b41d02516..0a42d2be4bd5e 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -379,9 +379,59 @@ struct DAP { void SetThreadFormat(llvm::StringRef format); - InstructionBreakpoint *GetInstructionBreakpoint(const lldb::break_id_t bp_id); + template <typename BreakpointType> + BreakpointType *GetBreakpointFromStopReason(lldb::SBThread &thread) { + // Check to see if have hit the <BreakpointType> breakpoint and change the + // reason accordingly, but only do so if all breakpoints that were + // hit are of <BreakpointType>. + const auto num = thread.GetStopReasonDataCount(); + BreakpointType *bp = nullptr; + for (size_t i = 0; i < num; i += 2) { + lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(i); + // If any breakpoint is not the <BreakpointType>, then stop and + // report this as a normal breakpoint + bp = GetBreakpoint<BreakpointType>(bp_id); + if (bp == nullptr) + return nullptr; + } + return bp; + } + + template <typename BreakpointType> + BreakpointType *GetBreakpoint(const lldb::break_id_t bp_id); + + template <> + FunctionBreakpoint * + GetBreakpoint<FunctionBreakpoint>(const lldb::break_id_t bp_id) { + for (auto &bp : function_breakpoints) { + if (bp.second.bp.GetID() == bp_id) + return &bp.second; + } + return nullptr; + } - InstructionBreakpoint *GetInstructionBPFromStopReason(lldb::SBThread &thread); + template <> + InstructionBreakpoint * + GetBreakpoint<InstructionBreakpoint>(const lldb::break_id_t bp_id) { + for (auto &bp : instruction_breakpoints) { + if (bp.second.bp.GetID() == bp_id) + return &bp.second; + } + return nullptr; + } + + template <> + ExceptionBreakpoint * + GetBreakpoint<ExceptionBreakpoint>(const lldb::break_id_t bp_id) { + // See comment in the other GetExceptionBreakpoint(). + PopulateExceptionBreakpoints(); + + for (auto &bp : *exception_breakpoints) { + if (bp.bp.GetID() == bp_id) + return &bp; + } + return nullptr; + } FunctionBreakpoint *GetFunctionBPFromStopReason(lldb::SBThread &thread); diff --git a/lldb/tools/lldb-dap/Handler/ExceptionInfoRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ExceptionInfoRequestHandler.cpp index 2f4d4efd1b189..974b340bb960c 100644 --- a/lldb/tools/lldb-dap/Handler/ExceptionInfoRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ExceptionInfoRequestHandler.cpp @@ -123,7 +123,8 @@ void ExceptionInfoRequestHandler::operator()( if (stopReason == lldb::eStopReasonSignal) body.try_emplace("exceptionId", "signal"); else if (stopReason == lldb::eStopReasonBreakpoint) { - ExceptionBreakpoint *exc_bp = dap.GetExceptionBPFromStopReason(thread); + ExceptionBreakpoint *exc_bp = + dap.GetBreakpointFromStopReason<ExceptionBreakpoint>(thread); if (exc_bp) { EmplaceSafeString(body, "exceptionId", exc_bp->filter); EmplaceSafeString(body, "description", exc_bp->label); diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index cd0d518cd4f7d..1561c73ff5f2a 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -962,19 +962,20 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, body.try_emplace("reason", "step"); break; case lldb::eStopReasonBreakpoint: { - ExceptionBreakpoint *exc_bp = dap.GetExceptionBPFromStopReason(thread); + ExceptionBreakpoint *exc_bp = + dap.GetBreakpointFromStopReason<ExceptionBreakpoint>(thread); if (exc_bp) { body.try_emplace("reason", "exception"); EmplaceSafeString(body, "description", exc_bp->label); } else { - std::string reason = "breakpoint"; + llvm::StringRef reason = "breakpoint"; InstructionBreakpoint *inst_bp = - dap.GetInstructionBPFromStopReason(thread); + dap.GetBreakpointFromStopReason<InstructionBreakpoint>(thread); if (inst_bp) { reason = "instruction breakpoint"; } else { FunctionBreakpoint *function_bp = - dap.GetFunctionBPFromStopReason(thread); + dap.GetBreakpointFromStopReason<FunctionBreakpoint>(thread); if (function_bp) { reason = "function breakpoint"; } @@ -989,7 +990,15 @@ llvm::json::Value CreateThreadStopped(DAP &dap, lldb::SBThread &thread, EmplaceSafeString(body, "description", desc_str); } } break; - case lldb::eStopReasonWatchpoint: + case lldb::eStopReasonWatchpoint: { + // Assuming that all watch points are data breakpoints. + body.try_emplace("reason", "data breakpoint"); + lldb::break_id_t bp_id = thread.GetStopReasonDataAtIndex(0); + lldb::break_id_t bp_loc_id = thread.GetStopReasonDataAtIndex(1); + std::string desc_str = + llvm::formatv("data breakpoint {0}.{1}", bp_id, bp_loc_id); + EmplaceSafeString(body, "description", desc_str); + } break; case lldb::eStopReasonInstrumentation: body.try_emplace("reason", "breakpoint"); break; >From 9ebf1d0b0bff31da7777ec4f62ec0251fce6a3d3 Mon Sep 17 00:00:00 2001 From: satya janga <satyaja...@fb.com> Date: Tue, 11 Mar 2025 13:39:08 -0700 Subject: [PATCH 3/3] Make breakpoint stop reason more accurate --- lldb/tools/lldb-dap/DAP.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 0a42d2be4bd5e..b1fe64a1b72a5 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -125,21 +125,21 @@ struct Variables { struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit StartDebuggingRequestHandler(DAP &d) : dap(d){}; + explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit ReplModeRequestHandler(DAP &d) : dap(d){}; + explicit ReplModeRequestHandler(DAP &d) : dap(d) {}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct SendEventRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit SendEventRequestHandler(DAP &d) : dap(d){}; + explicit SendEventRequestHandler(DAP &d) : dap(d) {}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits