Author: Sergei Druzhkov Date: 2026-08-17T12:28:42+03:00 New Revision: 4593ff73d0878f29b06c0d8fbc7f2ef2970f806a
URL: https://github.com/llvm/llvm-project/commit/4593ff73d0878f29b06c0d8fbc7f2ef2970f806a DIFF: https://github.com/llvm/llvm-project/commit/4593ff73d0878f29b06c0d8fbc7f2ef2970f806a.diff LOG: [lldb-dap] Preserve watchpoints from console (#215228) We should not delete watchpoints created via LLDB console when processing DAP `setDataBreakpoints` request. Added: Modified: lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py lldb/tools/lldb-dap/DAP.h lldb/tools/lldb-dap/Handler/SetDataBreakpointsRequestHandler.cpp lldb/tools/lldb-dap/Watchpoint.cpp lldb/tools/lldb-dap/Watchpoint.h Removed: ################################################################################ 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 7357a6dcd6365..f675c207066ce 100644 --- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py @@ -5,8 +5,11 @@ 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 DataBreakpoint, LaunchArgs - +from lldbsuite.test.tools.lldb_dap.types import ( + DataBreakpoint, + LaunchArgs, + StoppedReason, +) @requireNotWasm("data breakpoints map to watchpoints") class TestDAP_setDataBreakpoints(DAPTestCaseBase): @@ -182,6 +185,159 @@ def test_functionality(self): stop_event = session.continue_to_breakpoint(self.expect_not_none(bp_cond.id)) self.assertEqual(session.top_frame_from(stop_event).locals["x"].value, "10") + @skipIfWindows + def test_console_watchpoint_preserved(self): + """Test setDataBreakpoints must not delete watchpoints created via the console.""" + source = "main.cpp" + program = self.getBuildArtifact("a.out") + session = self.build_and_create_session() + first_loop_break_line = line_number(source, "// first loop breakpoint") + with session.configure(LaunchArgs(program)) as ctx: + session.resolve_source_breakpoints(source, [first_loop_break_line]) + stop_event = session.verify_stopped_on_breakpoint(after=ctx.process_event) + top_frame = session.top_frame_from(stop_event) + + # Create a watchpoint via the LLDB console. + resp_body = session.evaluate("`watchpoint set variable x", context="repl") + session.verify_evaluate(resp_body, matches=r".*Watchpoint created.*") + resp_body = session.evaluate("`watchpoint list", context="repl") + session.verify_evaluate(resp_body, matches=r".*Watchpoint 1:.*") + + # Set a data breakpoint via DAP. + arr = top_frame.locals["arr"] + arr_var_ref = self.expect_not_none(arr.variablesReference) + response_arr_2 = session.data_breakpoint_info( + "[2]", arr_var_ref, top_frame.frame.id + ) + arr_2_data_id = self.expect_not_none(response_arr_2.body.dataId) + set_response = session.set_data_breakpoints( + [DataBreakpoint(dataId=arr_2_data_id, accessType="write")] + ) + [bp_arr_2] = set_response.body.breakpoints + self.assertTrue(bp_arr_2.verified) + + resp_body = session.evaluate("`watchpoint list", context="repl") + session.verify_evaluate(resp_body, matches=r".*Watchpoint 1:.*") + + session.set_data_breakpoints([]) + resp_body = session.evaluate("`watchpoint list", context="repl") + session.verify_evaluate(resp_body, matches=r".*Watchpoint 1:.*") + + # Verify watchpoint from console. + stop_event = session.continue_to_next_stop( + exp_reason=StoppedReason.DATA_BREAKPOINT + ) + self.assertEqual(session.top_frame_from(stop_event).locals["x"].value, "2") + + session.evaluate("`watchpoint delete 1", context="repl") + session.continue_to_exit() + + @skipIfWindows + def test_hit_count_preserved(self): + """Test setDataBreakpoints preserves hit counts of existing watchpoints.""" + source = "main.cpp" + program = self.getBuildArtifact("a.out") + session = self.build_and_create_session() + first_loop_break_line = line_number(source, "// first loop breakpoint") + with session.configure(LaunchArgs(program)) as ctx: + session.resolve_source_breakpoints(source, [first_loop_break_line]) + stop_event = session.verify_stopped_on_breakpoint(after=ctx.process_event) + + second_loop_break_line = line_number(source, "// second loop breakpoint") + breakpoint_ids = session.resolve_source_breakpoints( + source, [second_loop_break_line] + ) + stop_event = session.continue_to_any_breakpoint(breakpoint_ids) + + top_frame_ctx = session.top_frame_from(stop_event) + frame_id = top_frame_ctx.frame.id + locals_ref = top_frame_ctx.locals.variablesReference + + response_x = session.data_breakpoint_info("x", locals_ref, frame_id) + arr = top_frame_ctx.locals["arr"] + arr_var_ref = self.expect_not_none(arr.variablesReference) + response_arr_2 = session.data_breakpoint_info("[2]", arr_var_ref, frame_id) + + x_data_id = self.expect_not_none(response_x.body.dataId) + arr_2_data_id = self.expect_not_none(response_arr_2.body.dataId) + + set_response = session.set_data_breakpoints( + [DataBreakpoint(dataId=x_data_id, accessType="write")] + ) + [bp_x] = set_response.body.breakpoints + self.assertTrue(bp_x.verified) + x_bp_id = self.expect_not_none(bp_x.id) + + # Hit the watchpoint on `x` twice. + stop_event = session.continue_to_breakpoint(x_bp_id) + self.assertEqual(session.top_frame_from(stop_event).locals["x"].value, "2") + stop_event = session.continue_to_breakpoint(x_bp_id) + self.assertEqual(session.top_frame_from(stop_event).locals["x"].value, "3") + + resp_body = session.evaluate("`watchpoint list -v", context="repl") + session.verify_evaluate(resp_body, matches=r"hit_count = 2\s") + + # Set additional data breakpoint on diff erent variable. + set_response = session.set_data_breakpoints( + [ + DataBreakpoint(dataId=x_data_id, accessType="write"), + DataBreakpoint(dataId=arr_2_data_id, accessType="write"), + ] + ) + [bp_x2, bp_arr_2] = set_response.body.breakpoints + self.assertTrue(bp_x2.verified) + self.assertTrue(bp_arr_2.verified) + self.assertEqual(bp_x2.id, x_bp_id) + + resp_body = session.evaluate("`watchpoint list -v", context="repl") + session.verify_evaluate(resp_body, matches=r"hit_count = 2\s") + + session.set_data_breakpoints([]) + session.continue_to_exit() + + @skipIfWindows + def test_type_change_recreates(self): + """Test setDataBreakpoints recreates watchpoint in case of changing type.""" + source = "main.cpp" + program = self.getBuildArtifact("a.out") + session = self.build_and_create_session() + first_loop_break_line = line_number(source, "// first loop breakpoint") + with session.configure(LaunchArgs(program)) as ctx: + session.resolve_source_breakpoints(source, [first_loop_break_line]) + stop_event = session.verify_stopped_on_breakpoint(after=ctx.process_event) + + top_frame_ctx = session.top_frame_from(stop_event) + frame_id = top_frame_ctx.frame.id + locals_ref = top_frame_ctx.locals.variablesReference + response_x = session.data_breakpoint_info("x", locals_ref, frame_id) + x_data_id = self.expect_not_none(response_x.body.dataId) + + set_response = session.set_data_breakpoints( + [DataBreakpoint(dataId=x_data_id, accessType="write")] + ) + [bp_write] = set_response.body.breakpoints + self.assertTrue(bp_write.verified) + write_id = self.expect_not_none(bp_write.id) + + stop_event = session.continue_to_breakpoint(write_id) + self.assertEqual(session.top_frame_from(stop_event).locals["x"].value, "2") + + resp_body = session.evaluate("`watchpoint list -v", context="repl") + session.verify_evaluate(resp_body, matches=r"hit_count = 1\s") + + set_response = session.set_data_breakpoints( + [DataBreakpoint(dataId=x_data_id, accessType="readWrite")] + ) + [bp_rw] = set_response.body.breakpoints + self.assertTrue(bp_rw.verified) + self.assertNotEqual(bp_rw.id, write_id) + + resp_body = session.evaluate("`watchpoint list -v", context="repl") + session.verify_evaluate(resp_body, matches=r"hit_count = 0\s") + + session.set_data_breakpoints([]) + session.continue_to_exit() + @skipIfWindows def test_bytes(self): """Tests setting data breakpoints on memory range.""" diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 3e2b1d4782147..68a401919b3be 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -22,6 +22,7 @@ #include "SourceBreakpoint.h" #include "Transport.h" #include "Variables.h" +#include "Watchpoint.h" #include "lldb/API/SBBroadcaster.h" #include "lldb/API/SBCommandInterpreter.h" #include "lldb/API/SBDebugger.h" @@ -63,6 +64,7 @@ typedef std::map<std::pair<uint32_t, uint32_t>, SourceBreakpoint> typedef llvm::StringMap<FunctionBreakpoint> FunctionBreakpointMap; typedef llvm::DenseMap<lldb::addr_t, InstructionBreakpoint> InstructionBreakpointMap; +typedef llvm::DenseMap<lldb::addr_t, Watchpoint> WatchpointMap; using AdapterFeature = protocol::AdapterFeature; using ClientFeature = protocol::ClientFeature; @@ -107,6 +109,7 @@ struct DAP final : public DAPTransport::MessageHandler { FunctionBreakpointMap function_breakpoints; InstructionBreakpointMap instruction_breakpoints; std::vector<ExceptionBreakpoint> exception_breakpoints; + WatchpointMap data_breakpoints; /// Map step in target id to list of function targets that user can choose. llvm::DenseMap<lldb::addr_t, std::string> step_in_targets; diff --git a/lldb/tools/lldb-dap/Handler/SetDataBreakpointsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/SetDataBreakpointsRequestHandler.cpp index 1caaa23bf06f6..f6b1dec7b6cf9 100644 --- a/lldb/tools/lldb-dap/Handler/SetDataBreakpointsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/SetDataBreakpointsRequestHandler.cpp @@ -11,7 +11,8 @@ #include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" #include "Watchpoint.h" -#include <set> +#include "llvm/ADT/DenseSet.h" +#include <algorithm> namespace lldb_dap { @@ -23,26 +24,62 @@ namespace lldb_dap { llvm::Expected<protocol::SetDataBreakpointsResponseBody> SetDataBreakpointsRequestHandler::Run( const protocol::SetDataBreakpointsArguments &args) const { - std::vector<protocol::Breakpoint> response_breakpoints; - - dap.target.DeleteAllWatchpoints(); std::vector<Watchpoint> watchpoints; + watchpoints.reserve(args.breakpoints.size()); for (const auto &bp : args.breakpoints) watchpoints.emplace_back(dap, bp); + llvm::DenseSet<lldb::addr_t> outdated( + llvm::from_range, llvm::make_first_range(dap.data_breakpoints)); + + std::vector<protocol::Breakpoint> response_breakpoints; + response_breakpoints.reserve(watchpoints.size()); // If two watchpoints start at the same address, the latter overwrite the // former. So, we only enable those at first-seen addresses when iterating // backward. - std::set<lldb::addr_t> addresses; - for (auto iter = watchpoints.rbegin(); iter != watchpoints.rend(); ++iter) { - if (addresses.count(iter->GetAddress()) == 0) { - iter->SetWatchpoint(); - addresses.insert(iter->GetAddress()); + llvm::DenseSet<lldb::addr_t> addresses; + for (auto it = watchpoints.rbegin(); it != watchpoints.rend(); ++it) { + const lldb::addr_t addr = it->GetAddress(); + if (addresses.contains(addr)) { + response_breakpoints.push_back(it->ToProtocolBreakpoint()); + continue; } + addresses.insert(addr); + outdated.erase(addr); + + auto existing = dap.data_breakpoints.find(addr); + if (existing == dap.data_breakpoints.end()) { + // Set the new one. + it->SetWatchpoint(); + dap.data_breakpoints.try_emplace(addr, *it); + response_breakpoints.push_back(it->ToProtocolBreakpoint()); + } else if (existing->second.HasSameSizeAndType(*it)) { + // Update existing. + existing->second.UpdateBreakpoint(*it); + response_breakpoints.push_back(existing->second.ToProtocolBreakpoint()); + } else { + // Delete existing and set the new one. + if (lldb::watch_id_t watch_id = existing->second.GetID(); + watch_id != LLDB_INVALID_WATCH_ID) + dap.target.DeleteWatchpoint(watch_id); + dap.data_breakpoints.erase(existing); + it->SetWatchpoint(); + dap.data_breakpoints.try_emplace(addr, *it); + response_breakpoints.push_back(it->ToProtocolBreakpoint()); + } + } + + for (lldb::addr_t addr : outdated) { + auto it = dap.data_breakpoints.find(addr); + if (it == dap.data_breakpoints.end()) + continue; + if (lldb::watch_id_t watch_id = it->second.GetID(); + watch_id != LLDB_INVALID_WATCH_ID) + dap.target.DeleteWatchpoint(watch_id); + dap.data_breakpoints.erase(it); } - for (auto wp : watchpoints) - response_breakpoints.push_back(wp.ToProtocolBreakpoint()); + std::reverse(response_breakpoints.begin(), response_breakpoints.end()); return protocol::SetDataBreakpointsResponseBody{ std::move(response_breakpoints)}; } diff --git a/lldb/tools/lldb-dap/Watchpoint.cpp b/lldb/tools/lldb-dap/Watchpoint.cpp index f9512104128d2..23e1e73cd22a8 100644 --- a/lldb/tools/lldb-dap/Watchpoint.cpp +++ b/lldb/tools/lldb-dap/Watchpoint.cpp @@ -60,4 +60,15 @@ void Watchpoint::SetWatchpoint() { if (!m_hit_condition.empty()) SetHitCondition(); } + +bool Watchpoint::HasSameSizeAndType(const Watchpoint &wp) const { + if (m_size != wp.m_size) + return false; + if (m_options.GetWatchpointTypeRead() != wp.m_options.GetWatchpointTypeRead()) + return false; + if (m_options.GetWatchpointTypeWrite() != + wp.m_options.GetWatchpointTypeWrite()) + return false; + return true; +} } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Watchpoint.h b/lldb/tools/lldb-dap/Watchpoint.h index d943e1218bdcd..c094027dd7b65 100644 --- a/lldb/tools/lldb-dap/Watchpoint.h +++ b/lldb/tools/lldb-dap/Watchpoint.h @@ -34,6 +34,10 @@ class Watchpoint : public BreakpointBase { lldb::addr_t GetAddress() const { return m_addr; } + lldb::watch_id_t GetID() { return m_wp.GetID(); } + + bool HasSameSizeAndType(const Watchpoint &wp) const; + protected: lldb::addr_t m_addr; size_t m_size; _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
