Avoid spurious breakpoint-setting failure in DAP
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
Commit Message
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
---
gdb/python/lib/gdb/dap/breakpoint.py | 8 +++++++-
gdb/testsuite/gdb.dap/basic-dap.exp | 9 +++++++--
2 files changed, 14 insertions(+), 3 deletions(-)
Comments
>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:
Tom> A user pointed out that if a DAP setBreakpoints request has a 'source'
Tom> field in a SourceBreakpoint object, then the gdb DAP implementation
Tom> will throw an exception.
Tom> While SourceBreakpoint does not allow 'source' in the spec, it seems
Tom> better to me to accept it. I don't think we should fully go down the
Tom> "Postel's Law" path -- after all, we have the type-checker -- but at
Tom> the same time, if we do send errors, they should be intentional and
Tom> not artifacts of the implementation.
Tom> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30820
This one is just a simple DAP-specific bug fix, so I'm checking it in.
Tom
@@ -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"]
@@ -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"