[binutils-gdb] Avoid spurious breakpoint-setting failure in DAP

Tom Tromey tromey@sourceware.org
Tue Sep 12 16:37:09 GMT 2023


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f8ab027008863bd06513053ea9f84e1116c1cf73

commit f8ab027008863bd06513053ea9f84e1116c1cf73
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Sep 5 06:55:54 2023 -0600

    Avoid spurious breakpoint-setting failure in DAP
    
    A user pointed out that if a DAP setBreakpoints request has a 'source'
    field in a SourceBreakpoint object, then the gdb DAP implementation
    will throw an exception.
    
    While SourceBreakpoint does not allow 'source' in the spec, it seems
    better to me to accept it.  I don't think we should fully go down the
    "Postel's Law" path -- after all, we have the type-checker -- but at
    the same time, if we do send errors, they should be intentional and
    not artifacts of the implementation.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30820

Diff:
---
 gdb/python/lib/gdb/dap/breakpoint.py | 8 +++++++-
 gdb/testsuite/gdb.dap/basic-dap.exp  | 9 +++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 76ff129ddbc..1622ae69023 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -273,7 +273,13 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     if "path" not in source:
         result = []
     else:
-        specs = [_rewrite_src_breakpoint(source=source, **bp) for bp in breakpoints]
+        # Setting 'source' in BP avoids any Python error if BP already
+        # has a 'source' parameter.  Setting this isn't in the spec,
+        # but it is better to be safe.  See PR dap/30820.
+        specs = []
+        for bp in breakpoints:
+            bp["source"] = source
+            specs.append(_rewrite_src_breakpoint(**bp))
         # Be sure to include the path in the key, so that we only
         # clear out breakpoints coming from this same source.
         key = "source:" + source["path"]
diff --git a/gdb/testsuite/gdb.dap/basic-dap.exp b/gdb/testsuite/gdb.dap/basic-dap.exp
index 7dfc759baac..98ca739ccab 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.exp
+++ b/gdb/testsuite/gdb.dap/basic-dap.exp
@@ -87,10 +87,15 @@ foreach entry [dict get [lindex $obj 0] body breakpoints] {
     incr first_line
 }
 
+# Note that in this request, we add a 'source' field to the
+# SourceBreakpoint object.  This isn't in the spec but it once caused
+# an incorrect exception in the Python code.  See PR dap/30820.
 set obj [dap_check_request_and_response "reset breakpoint by line number" \
 	     setBreakpoints \
-	     [format {o source [o path [%s]] breakpoints [a [o line [i %d]]]} \
-		  [list s $srcfile] $line]]
+	     [format {o source [o path [%s]] \
+			  breakpoints [a [o source [o path [%s]] \
+					      line [i %d]]]} \
+		  [list s $srcfile] [list s $srcfile] $line]]
 set new_line_bpno [dap_get_breakpoint_number $obj]
 gdb_assert {$new_line_bpno == $line_bpno} "re-setting kept same breakpoint number"


More information about the Gdb-cvs mailing list