diff mbox series

[1/4] gdb/dap: check for breakpoint source before unpacking

Message ID 20230901180323.22328-5-greg@gpanders.com
State New
Headers
Series DAP fixups |

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

Commit Message

Gregory Anders Sept. 1, 2023, 5:55 p.m. UTC 
  Not all breakpoints have a source location. For example, a breakpoint
set on a raw address will have only the "address" field populated, but
"source" will be None, which leads to a RuntimeError when attempting to
unpack the filename and line number.

Before attempting to unpack the filename and line number from the
breakpoint, ensure that the source information is not None. Also
populate the source and line information separately from the
"instructionReference" field, so that breakpoints that include only an
address are still included.
---
 gdb/python/lib/gdb/dap/breakpoint.py | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comments

Tom Tromey Sept. 1, 2023, 7:05 p.m. UTC | #1 
>>>>> Gregory Anders via Gdb-patches <gdb-patches@sourceware.org> writes:

Thank you for the patch.

> Not all breakpoints have a source location. For example, a breakpoint
> set on a raw address will have only the "address" field populated, but
> "source" will be None, which leads to a RuntimeError when attempting to
> unpack the filename and line number.

This one could probably use a test case.

I guess the failure mode is setting a breakpoint on a function without
debuginfo?  If so maybe the test is as simple as having
gdb/testsuite/gdb.dap/bt-nodebug.exp set a breakpoint on the
"no_debug_info" function, I suppose just before dap_shutdown.

Anyway I think the patch itself looks fine.

Tom
Gregory Anders Sept. 1, 2023, 7:18 p.m. UTC | #2 
On Fri, 01 Sep 2023 13:05 -0600, Tom Tromey wrote:
>This one could probably use a test case.
>
>I guess the failure mode is setting a breakpoint on a function without
>debuginfo?  If so maybe the test is as simple as having
>gdb/testsuite/gdb.dap/bt-nodebug.exp set a breakpoint on the
>"no_debug_info" function, I suppose just before dap_shutdown.

I actually stumbled on this on accident. I was starting GDB with a
.gdbinit file that sets a breakpoint by address (specifically, an
exception handler in the vector table, so something like b *0xf4). That
breakpoint had address info, but no source file or line information, and
would trigger an error which this patch corrects.

I'll look into writing up a test case.
diff mbox series

Patch

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 76ff129d..0ebb9597 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -106,14 +106,18 @@  def _breakpoint_descriptor(bp):
         # multiple locations.  See
         # https://github.com/microsoft/debug-adapter-protocol/issues/13
         loc = bp.locations[0]
-        (filename, line) = loc.source
-        result.update(
-            {
-                "source": make_source(filename, os.path.basename(filename)),
-                "line": line,
-                "instructionReference": hex(loc.address),
-            }
-        )
+        if loc.source:
+            (filename, line) = loc.source
+            result.update(
+                {
+                    "source": make_source(filename, os.path.basename(filename)),
+                    "line": line,
+                }
+            )
+
+        if loc.address:
+            result["instructionReference"] = hex(loc.address),
+
         path = loc.fullname
         if path is not None:
             result["source"]["path"] = path