[PATCH 0/3] Implement DAP "source" request

[PATCH 0/3] Implement DAP "source" request

[Tom Tromey]

This series refactors some source / sourceReference handling in the
DAP implementation and then implements the "source" request.

---
Tom Tromey (3):
      Introduce sourceReference handling in DAP
      Handle Source in DAP breakpointLocations
      Implement DAP "source" request

 gdb/python/lib/gdb/dap/breakpoint.py | 10 ++---
 gdb/python/lib/gdb/dap/bt.py         |  9 +---
 gdb/python/lib/gdb/dap/locations.py  | 12 ++----
 gdb/python/lib/gdb/dap/sources.py    | 82 +++++++++++++++++++++++++++++++++---
 gdb/testsuite/gdb.dap/basic-dap.exp  |  3 --
 gdb/testsuite/gdb.dap/sources.c      | 22 ++++++++++
 gdb/testsuite/gdb.dap/sources.exp    | 52 +++++++++++++++++++++++
 7 files changed, 158 insertions(+), 32 deletions(-)
---
base-commit: 249d4715e41061b6bd2d26df20ae274e6478f972
change-id: 20230728-dap-source-refs-e07c9993c6dd

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/3] Introduce sourceReference handling in DAP

[Tom Tromey]

This changes the gdb DAP implementation to emit a real
sourceReference, rather than emitting 0.  Sources are tracked in some
maps in sources.py, and a new helper function is introduced to compute
the "Source" object that can be sent to the client.
---
 gdb/python/lib/gdb/dap/breakpoint.py | 10 +++------
 gdb/python/lib/gdb/dap/bt.py         |  9 ++------
 gdb/python/lib/gdb/dap/sources.py    | 42 ++++++++++++++++++++++++++++++------
 3 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 27745ebfd2c..9c214b2bf14 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -21,6 +21,7 @@ import re
 from typing import Optional, Sequence
 
 from .server import request, capability, send_event
+from .sources import make_source
 from .startup import send_gdb_with_response, in_gdb_thread, log_stack
 from .typecheck import type_check
 
@@ -48,15 +49,10 @@ def breakpoint_descriptor(bp):
         # multiple locations.  See
         # https://github.com/microsoft/debug-adapter-protocol/issues/13
         loc = bp.locations[0]
-        (basename, line) = loc.source
+        (filename, line) = loc.source
         result.update(
             {
-                "source": {
-                    "name": os.path.basename(basename),
-                    # We probably don't need this but it doesn't hurt to
-                    # be explicit.
-                    "sourceReference": 0,
-                },
+                "source": make_source(filename, os.path.basename(filename)),
                 "line": line,
                 "instructionReference": hex(loc.address),
             }
diff --git a/gdb/python/lib/gdb/dap/bt.py b/gdb/python/lib/gdb/dap/bt.py
index 975c88f8208..34d05cbaac0 100644
--- a/gdb/python/lib/gdb/dap/bt.py
+++ b/gdb/python/lib/gdb/dap/bt.py
@@ -20,6 +20,7 @@ from gdb.frames import frame_iterator
 from .frames import frame_id
 from .modules import module_id
 from .server import request, capability
+from .sources import make_source
 from .startup import send_gdb_with_response, in_gdb_thread
 from .state import set_thread
 
@@ -59,13 +60,7 @@ def _backtrace(thread_id, levels, startFrame):
             newframe["line"] = line
         filename = current_frame.filename()
         if filename is not None:
-            newframe["source"] = {
-                "name": os.path.basename(filename),
-                "path": filename,
-                # We probably don't need this but it doesn't hurt
-                # to be explicit.
-                "sourceReference": 0,
-            }
+            newframe["source"] = make_source(filename, os.path.basename(filename))
         frames.append(newframe)
     # Note that we do not calculate totalFrames here.  Its absence
     # tells the client that it may simply ask for frames until a
diff --git a/gdb/python/lib/gdb/dap/sources.py b/gdb/python/lib/gdb/dap/sources.py
index af7313ca2f9..50b5909ed1d 100644
--- a/gdb/python/lib/gdb/dap/sources.py
+++ b/gdb/python/lib/gdb/dap/sources.py
@@ -19,16 +19,46 @@ from .server import request, capability
 from .startup import send_gdb_with_response, in_gdb_thread
 
 
+# The next available source reference ID.  Must be greater than 0.
+_next_source = 1
+
+# Map from full paths to Source dictionaries.
+_source_map = {}
+
+# Map from a source reference ID back to the same Source that is
+# stored in _source_map.
+_id_map = {}
+
+
+@in_gdb_thread
+def make_source(fullname, filename):
+    """Return the Source for a given file name.
+
+    FULLNAME is the full name.  This is used as the key.
+    FILENAME is the base name.
+    """
+    global _source_map
+    if fullname in _source_map:
+        result = _source_map[fullname]
+    else:
+        global _next_source
+        result = {
+            "name": filename,
+            "path": fullname,
+            "sourceReference": _next_source,
+        }
+        _source_map[fullname] = result
+        global _id_map
+        _id_map[_next_source] = result
+        _next_source += 1
+    return result
+
+
 @in_gdb_thread
 def _sources():
     result = []
     for elt in gdb.execute_mi("-file-list-exec-source-files")["files"]:
-        result.append(
-            {
-                "name": elt["file"],
-                "path": elt["fullname"],
-            }
-        )
+        result.append(make_source(elt["fullname"], elt["file"]))
     return {
         "sources": result,
     }

-- 
2.40.1

[PATCH 2/3] Handle Source in DAP breakpointLocations

[Tom Tromey]

This changes the DAP breakpointLocations request to accept a Source
and to decode it properly.
---
 gdb/python/lib/gdb/dap/locations.py | 12 ++++--------
 gdb/python/lib/gdb/dap/sources.py   | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/locations.py b/gdb/python/lib/gdb/dap/locations.py
index 594f1bad45b..a299e8da959 100644
--- a/gdb/python/lib/gdb/dap/locations.py
+++ b/gdb/python/lib/gdb/dap/locations.py
@@ -19,11 +19,13 @@ import gdb
 from typing import Optional
 
 from .server import capability, request
+from .sources import decode_source
 from .startup import in_gdb_thread, send_gdb_with_response
 
 
 @in_gdb_thread
-def _find_lines(filename, start_line, end_line):
+def _find_lines(source, start_line, end_line):
+    filename = decode_source(source)
     lines = set()
     for entry in gdb.execute_mi("-symbol-list-lines", filename)["lines"]:
         line = entry["line"]
@@ -44,10 +46,4 @@ def _find_lines(filename, start_line, end_line):
 def breakpoint_locations(*, source, line: int, endLine: Optional[int] = None, **extra):
     if endLine is None:
         endLine = line
-    if "path" in source:
-        filename = source["path"]
-    elif "name" in source:
-        filename = source["name"]
-    else:
-        raise Exception("")
-    return send_gdb_with_response(lambda: _find_lines(filename, line, endLine))
+    return send_gdb_with_response(lambda: _find_lines(source, line, endLine))
diff --git a/gdb/python/lib/gdb/dap/sources.py b/gdb/python/lib/gdb/dap/sources.py
index 50b5909ed1d..806352836db 100644
--- a/gdb/python/lib/gdb/dap/sources.py
+++ b/gdb/python/lib/gdb/dap/sources.py
@@ -54,6 +54,22 @@ def make_source(fullname, filename):
     return result
 
 
+@in_gdb_thread
+def decode_source(source):
+    """Decode a Source object.
+
+    Finds and returns the filename of a given Source object."""
+    if "path" in source:
+        return source["path"]
+    if "sourceReference" not in source:
+        raise Exception("either 'path' or 'sourceReference' must appear in Source")
+    ref = source["sourceReference"]
+    global _id_map
+    if ref not in _id_map:
+        raise Exception("no sourceReference " + str(ref))
+    return _id_map[ref]["path"]
+
+
 @in_gdb_thread
 def _sources():
     result = []

-- 
2.40.1

[PATCH 3/3] Implement DAP "source" request

[Tom Tromey]

This implements the DAP "source" request.  I renamed the
"loadedSources" function from "sources" to "loaded_sources" to avoid
any confusion.  I also moved the loadedSources test to the new
sources.exp.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30691
---
 gdb/python/lib/gdb/dap/sources.py   | 24 ++++++++++++++++-
 gdb/testsuite/gdb.dap/basic-dap.exp |  3 ---
 gdb/testsuite/gdb.dap/sources.c     | 22 ++++++++++++++++
 gdb/testsuite/gdb.dap/sources.exp   | 52 +++++++++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/sources.py b/gdb/python/lib/gdb/dap/sources.py
index 806352836db..7fa1ae430c8 100644
--- a/gdb/python/lib/gdb/dap/sources.py
+++ b/gdb/python/lib/gdb/dap/sources.py
@@ -82,5 +82,27 @@ def _sources():
 
 @request("loadedSources")
 @capability("supportsLoadedSourcesRequest")
-def sources(**extra):
+def loaded_sources(**extra):
     return send_gdb_with_response(_sources)
+
+
+# This helper is needed because we must only access the globals here
+# from the gdb thread.
+@in_gdb_thread
+def _get_source(source):
+    filename = decode_source(source)
+    with open(filename) as f:
+        content = f.read()
+    return {
+        "content": content,
+    }
+
+
+@request("source")
+def source(*, source=None, sourceReference: int, **extra):
+    # The 'sourceReference' parameter is required by the spec, but is
+    # for backward compatibility, which I take to mean that the
+    # 'source' is preferred.
+    if source is None:
+        source = {"sourceReference": sourceReference}
+    return send_gdb_with_response(lambda: _get_source(source))
diff --git a/gdb/testsuite/gdb.dap/basic-dap.exp b/gdb/testsuite/gdb.dap/basic-dap.exp
index ef3c535f6a2..b0f8e624610 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.exp
+++ b/gdb/testsuite/gdb.dap/basic-dap.exp
@@ -206,7 +206,4 @@ set obj [dap_check_request_and_response "command repl" \
 set response [lindex $obj 0]
 gdb_assert {[dict get $response body result] == 23}
 
-set obj [dap_check_request_and_response sources loadedSources]
-gdb_assert {[string first basic-dap.c $obj] != -1}
-
 dap_shutdown
diff --git a/gdb/testsuite/gdb.dap/sources.c b/gdb/testsuite/gdb.dap/sources.c
new file mode 100644
index 00000000000..599d1bd9f77
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/sources.c
@@ -0,0 +1,22 @@
+/* Copyright 2023 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/>.  */
+
+int
+main ()
+{
+  /* Distinguishing comment.  */
+}
diff --git a/gdb/testsuite/gdb.dap/sources.exp b/gdb/testsuite/gdb.dap/sources.exp
new file mode 100644
index 00000000000..8983b4d94e6
--- /dev/null
+++ b/gdb/testsuite/gdb.dap/sources.exp
@@ -0,0 +1,52 @@
+# Copyright 2023 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/>.
+
+# Test 'sources' and 'loadedSources'.
+
+require allow_dap_tests
+
+load_lib dap-support.exp
+
+standard_testfile
+
+if {[build_executable ${testfile}.exp $testfile] == -1} {
+    return
+}
+
+if {[dap_launch $testfile {} {} 1] == ""} {
+    return
+}
+
+set obj [dap_check_request_and_response loadedSources loadedSources]
+set ref 0
+foreach src [dict get [lindex $obj 0] body sources] {
+    if {[file tail [dict get $src name]] == "sources.c"} {
+	set ref [dict get $src sourceReference]
+    }	
+}
+
+if {$ref == 0} {
+    fail "sources.c in loadedSources"
+} else {
+    pass "sources.c in loadedSources"
+
+    set obj [dap_check_request_and_response "get source" source \
+		 [format {o sourceReference [i %d]} $ref]]
+    set text [dict get [lindex $obj 0] body content]
+    gdb_assert {[string first "Distinguishing comment" $text] != -1}
+}
+
+
+dap_shutdown

-- 
2.40.1