Have DAP handle multiple breakpoints at same location

A user pointed out that if multiple breakpoints are set at the same
spot, in DAP mode, then changing the breakpoints won't reset all of
them.

The problem here is that the breakpoint map only stores a single
breakpoint, so if two breakpoints have the same key, only one will be
stored.  Then, when breakpoints are changed, the "missing" breakpoint
will not be deleted.

The fix is to change the map to store a list of breakpoints.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33467
Reviewed-By: Ciaran Woodward <ciaranwoodward@xmos.com>

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 390a20946ce634c9aefd5f69dec29796fe372604..060e4d8878cb3f3ba3fbaa8c48f127cc05b32ba7 100644 (file)
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -14,6 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import re
+from collections import defaultdict
 from contextlib import contextmanager
 
 # These are deprecated in 3.9, but required in older versions.
@@ -92,9 +93,9 @@ gdb.events.breakpoint_deleted.connect(_bp_deleted)
 
 # Map from the breakpoint "kind" (like "function") to a second map, of
 # breakpoints of that type.  The second map uses the breakpoint spec
-# as a key, and the gdb.Breakpoint itself as a value.  This is used to
-# implement the clearing behavior specified by the protocol, while
-# allowing for reuse when a breakpoint can be kept.
+# as a key, and a list of gdb.Breakpoint objects as a value.  This is
+# used to implement the clearing behavior specified by the protocol,
+# while allowing for reuse when a breakpoint can be kept.
 _breakpoint_map = {}
 
 
@@ -153,7 +154,7 @@ def _set_breakpoints(kind, specs, creator):
         saved_map = _breakpoint_map[kind]
     else:
         saved_map = {}
-    _breakpoint_map[kind] = {}
+    _breakpoint_map[kind] = defaultdict(list)
     result = []
     with suppress_new_breakpoint_event():
         for spec in specs:
@@ -170,8 +171,8 @@ def _set_breakpoints(kind, specs, creator):
             # report these as an "unverified" breakpoint.
             bp = None
             try:
-                if keyspec in saved_map:
-                    bp = saved_map.pop(keyspec)
+                if keyspec in saved_map and len(saved_map[keyspec]) > 0:
+                    bp = saved_map[keyspec].pop()
                 else:
                     bp = creator(**spec)
 
@@ -184,7 +185,7 @@ def _set_breakpoints(kind, specs, creator):
                     )
 
                 # Reaching this spot means success.
-                _breakpoint_map[kind][keyspec] = bp
+                _breakpoint_map[kind][keyspec].append(bp)
                 result.append(_breakpoint_descriptor(bp))
             # Exceptions other than gdb.error are possible here.
             except Exception as e:
@@ -205,8 +206,9 @@ def _set_breakpoints(kind, specs, creator):
                 )
 
         # Delete any breakpoints that were not reused.
-        for entry in saved_map.values():
-            entry.delete()
+        for sub_map in saved_map.values():
+            for entry in sub_map:
+                entry.delete()
     return result
 
 

diff --git a/gdb/testsuite/gdb.dap/multi-break.c b/gdb/testsuite/gdb.dap/multi-break.c
new file mode 100644 (file)
index 0000000..e57a3df
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/multi-break.c
@@ -0,0 +1,38 @@
+/* Copyright 2025 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+void
+do_nothing ()
+{
+}
+
+void
+never_called ()
+{
+  /* EXTRA */
+}
+
+int
+main ()
+{
+  int i;
+
+  for (i = 0; i < 10; ++i)
+    do_nothing ();             /* STOP */
+
+  return 0;
+}

diff --git a/gdb/testsuite/gdb.dap/multi-break.exp b/gdb/testsuite/gdb.dap/multi-break.exp
new file mode 100644 (file)
index 0000000..631f9de
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/multi-break.exp
@@ -0,0 +1,64 @@
+# Copyright 2025 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Proper handling of multiple breakpoints on one line.
+
+require allow_dap_tests
+
+load_lib dap-support.exp
+
+standard_testfile
+
+if {[build_executable ${testfile}.exp $testfile] == -1} {
+    return
+}
+
+if {[dap_initialize] == ""} {
+    return
+}
+
+set launch_id [dap_launch $testfile]
+
+set line [gdb_get_line_number "STOP"]
+set extra [gdb_get_line_number "EXTRA"]
+dap_check_request_and_response "set two breakpoints by line number" \
+    setBreakpoints \
+    [format {o source [o path [%s]] \
+                breakpoints [a [o line [i %d]] [o line [i %d]] [o line [i %d]]]} \
+        [list s $srcfile] $line $line $extra]
+
+dap_check_request_and_response "configurationDone" configurationDone
+
+dap_check_response "launch response" launch $launch_id
+
+dap_wait_for_event_and_check "inferior started" thread "body reason" started
+
+dap_wait_for_event_and_check "stopped at breakpoint" stopped \
+    "body reason" breakpoint
+
+# Now clear the breakpoints on the "STOP" line, leaving just the
+# "EXTRA" one.
+dap_check_request_and_response "clear most breakpoints" \
+    setBreakpoints \
+    [format {o source [o path [%s]] breakpoints [a [o line [i %d]]]} \
+        [list s $srcfile] $extra]
+
+dap_check_request_and_response "continue" continue \
+    {o threadId [i 1]}
+
+# Check that the breakpoint did not cause a stop.
+dap_wait_for_event_and_check "inferior exited" exited
+
+dap_shutdown