[PATCH v2 0/9] Implement the DAP "loadedSources" request

[PATCH v2 0/9] Implement the DAP "loadedSources" request

[Tom Tromey]

This series implements the DAP "loadedSources" request.  However, it
does so by exposing MI commands to Python, with MI output being
converted to Python objects via a new ui_out implementation.

Regression tested on x86-64 Fedora 36.

---
Changes in v2:
- MI commands must be prefixed with '-'
- Link to v1: https://inbox.sourceware.org/gdb-patches/20230404-dap-loaded-sources-v1-0-75c796bd644b@adacore.com

---
Tom Tromey (9):
      Use field_signed from Python MI commands
      Use member initializers in mi_parse
      Use accessor for mi_parse::args
      Change mi_parse_argv to a method
      Introduce "static constructor" for mi_parse
      Introduce mi_parse helper methods
      Add second mi_parse constructor
      Implement gdb.execute_mi
      Implement DAP loadedSources request

 gdb/Makefile.in                         |   1 +
 gdb/NEWS                                |   3 +
 gdb/data-directory/Makefile.in          |   1 +
 gdb/doc/python.texi                     |  30 ++++
 gdb/mi/mi-cmds.c                        |   6 +-
 gdb/mi/mi-cmds.h                        |   5 +
 gdb/mi/mi-main.c                        |  19 +-
 gdb/mi/mi-parse.c                       | 196 ++++++++++++++++-----
 gdb/mi/mi-parse.h                       |  81 ++++++---
 gdb/python/lib/gdb/dap/__init__.py      |   1 +
 gdb/python/lib/gdb/dap/sources.py       |  40 +++++
 gdb/python/py-mi.c                      | 298 ++++++++++++++++++++++++++++++++
 gdb/python/py-micmd.c                   |  20 ++-
 gdb/python/python-internal.h            |   5 +
 gdb/python/python.c                     |   5 +
 gdb/testsuite/gdb.dap/basic-dap.exp     |   3 +
 gdb/testsuite/gdb.python/py-exec-mi.exp |  32 ++++
 gdb/testsuite/gdb.python/py-mi-cmd.py   |  27 +++
 18 files changed, 697 insertions(+), 76 deletions(-)
---
base-commit: c96452ad168cf42ad42f0d57214dddb38d5fae88
change-id: 20230404-dap-loaded-sources-5d01323a1240

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

[PATCH v2 1/9] Use field_signed from Python MI commands

[Tom Tromey]

If an MI command written in Python includes a number in its output,
currently that is simply emitted as a string.  However, it's
convenient for a later patch if these are emitted using field_signed.
This does not make a difference to ordinary MI clients.
---
 gdb/python/py-micmd.c        | 15 +++++++++++++++
 gdb/python/python-internal.h |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
index e86807d049f..d7d95918cb4 100644
--- a/gdb/python/py-micmd.c
+++ b/gdb/python/py-micmd.c
@@ -293,6 +293,21 @@ serialize_mi_result_1 (PyObject *result, const char *field_name)
     }
   else
     {
+      if (PyLong_Check (result))
+	{
+	  int overflow = 0;
+	  gdb_py_longest val = gdb_py_long_as_long_and_overflow (result,
+								 &overflow);
+	  if (PyErr_Occurred () != nullptr)
+	    gdbpy_handle_exception ();
+	  if (overflow == 0)
+	    {
+	      uiout->field_signed (field_name, val);
+	      return;
+	    }
+	  /* Fall through to the string case on overflow.  */
+	}
+
       gdb::unique_xmalloc_ptr<char> string (gdbpy_obj_to_string (result));
       if (string == nullptr)
 	gdbpy_handle_exception ();
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index dbd33570a78..1142e0e739d 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -110,6 +110,7 @@
 typedef PY_LONG_LONG gdb_py_longest;
 typedef unsigned PY_LONG_LONG gdb_py_ulongest;
 #define gdb_py_long_as_ulongest PyLong_AsUnsignedLongLong
+#define gdb_py_long_as_long_and_overflow PyLong_AsLongLongAndOverflow
 
 #else /* HAVE_LONG_LONG */
 
@@ -118,6 +119,7 @@ typedef unsigned PY_LONG_LONG gdb_py_ulongest;
 typedef long gdb_py_longest;
 typedef unsigned long gdb_py_ulongest;
 #define gdb_py_long_as_ulongest PyLong_AsUnsignedLong
+#define gdb_py_long_as_long_and_overflow PyLong_AsLongAndOverflow
 
 #endif /* HAVE_LONG_LONG */
 

-- 
2.40.0

[PATCH v2 2/9] Use member initializers in mi_parse

[Tom Tromey]

This changes mi_parse to use member initializers rather than a
constructor.  This is easier to follow.
---
 gdb/mi/mi-parse.c | 17 -----------------
 gdb/mi/mi-parse.h | 28 ++++++++++++++--------------
 2 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index 737ec57eddd..bda554a568e 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -208,23 +208,6 @@ mi_parse_argv (const char *args, struct mi_parse *parse)
     }
 }
 
-mi_parse::mi_parse ()
-  : op (MI_COMMAND),
-    command (NULL),
-    token (NULL),
-    cmd (NULL),
-    cmd_start (NULL),
-    args (NULL),
-    argv (NULL),
-    argc (0),
-    all (0),
-    thread_group (-1),
-    thread (-1),
-    frame (-1),
-    language (language_unknown)
-{
-}
-
 mi_parse::~mi_parse ()
 {
   xfree (command);
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index e34796988a4..75bb36b1c77 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -41,27 +41,27 @@ enum mi_command_type
 
 struct mi_parse
   {
-    mi_parse ();
+    mi_parse () = default;
     ~mi_parse ();
 
     DISABLE_COPY_AND_ASSIGN (mi_parse);
 
-    enum mi_command_type op;
-    char *command;
-    char *token;
-    const struct mi_command *cmd;
-    struct mi_timestamp *cmd_start;
-    char *args;
-    char **argv;
-    int argc;
-    int all;
-    int thread_group; /* At present, the same as inferior number.  */
-    int thread;
-    int frame;
+    enum mi_command_type op = MI_COMMAND;
+    char *command = nullptr;
+    char *token = nullptr;
+    const struct mi_command *cmd = nullptr;
+    struct mi_timestamp *cmd_start = nullptr;
+    char *args = nullptr;
+    char **argv = nullptr;
+    int argc = 0;
+    int all = 0;
+    int thread_group = -1; /* At present, the same as inferior number.  */
+    int thread = -1;
+    int frame = -1;
 
     /* The language that should be used to evaluate the MI command.
        Ignored if set to language_unknown.  */
-    enum language language;
+    enum language language = language_unknown;
   };
 
 /* Attempts to parse CMD returning a ``struct mi_parse''.  If CMD is

-- 
2.40.0

[PATCH v2 3/9] Use accessor for mi_parse::args

[Tom Tromey]

This changes mi_parse::args to be a private member, retrieved via
accessor.  It also changes this member to be a std::string.  This
makes it simpler for a subsequent patch to implement different
behavior for argument parsing.
---
 gdb/mi/mi-cmds.c      |  6 +++---
 gdb/mi/mi-main.c      |  2 +-
 gdb/mi/mi-parse.c     |  3 +--
 gdb/mi/mi-parse.h     | 13 ++++++++++++-
 gdb/python/py-micmd.c |  5 +++--
 5 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 284453db85d..ca8c633e218 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -49,11 +49,11 @@ struct mi_command_mi : public mi_command
      with arguments contained within PARSE.  */
   void invoke (struct mi_parse *parse) const override
   {
-    mi_parse_argv (parse->args, parse);
+    mi_parse_argv (parse->args (), parse);
 
     if (parse->argv == nullptr)
       error (_("Problem parsing arguments: %s %s"), parse->command,
-	     parse->args);
+	     parse->args ());
 
     this->m_argv_function (parse->command, parse->argv, parse->argc);
   }
@@ -87,7 +87,7 @@ struct mi_command_cli : public mi_command
      is passed through to the CLI function as its argument string.  */
   void invoke (struct mi_parse *parse) const override
   {
-    const char *args = m_args_p ? parse->args : nullptr;
+    const char *args = m_args_p ? parse->args () : nullptr;
     mi_execute_cli_command (m_cli_name, m_args_p, args);
   }
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 840663e0fb4..aaebce40fac 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1813,7 +1813,7 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
       if (mi_debug_p)
 	gdb_printf (gdb_stdlog,
 		    " token=`%s' command=`%s' args=`%s'\n",
-		    context->token, context->command, context->args);
+		    context->token, context->command, context->args ());
 
       mi_cmd_execute (context);
 
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index bda554a568e..bf3b534e590 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -212,7 +212,6 @@ mi_parse::~mi_parse ()
 {
   xfree (command);
   xfree (token);
-  xfree (args);
   freeargv (argv);
 }
 
@@ -346,7 +345,7 @@ mi_parse (const char *cmd, char **token)
     }
 
   /* Save the rest of the arguments for the command.  */
-  parse->args = xstrdup (chp);
+  parse->set_args (chp);
 
   /* Fully parsed, flag as an MI command.  */
   parse->op = MI_COMMAND;
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index 75bb36b1c77..d4ac3f002e4 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -46,12 +46,19 @@ struct mi_parse
 
     DISABLE_COPY_AND_ASSIGN (mi_parse);
 
+    /* Return the full argument string, as used by commands which are
+       implemented as CLI commands.  */
+    const char *args () const
+    { return m_args.c_str (); }
+
+    void set_args (const char *args)
+    { m_args = args; }
+
     enum mi_command_type op = MI_COMMAND;
     char *command = nullptr;
     char *token = nullptr;
     const struct mi_command *cmd = nullptr;
     struct mi_timestamp *cmd_start = nullptr;
-    char *args = nullptr;
     char **argv = nullptr;
     int argc = 0;
     int all = 0;
@@ -62,6 +69,10 @@ struct mi_parse
     /* The language that should be used to evaluate the MI command.
        Ignored if set to language_unknown.  */
     enum language language = language_unknown;
+
+  private:
+
+    std::string m_args;
   };
 
 /* Attempts to parse CMD returning a ``struct mi_parse''.  If CMD is
diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
index d7d95918cb4..88d52db2202 100644
--- a/gdb/python/py-micmd.c
+++ b/gdb/python/py-micmd.c
@@ -355,10 +355,11 @@ mi_command_py::invoke (struct mi_parse *parse) const
 
   pymicmd_debug_printf ("this = %p, name = %s", this, name ());
 
-  mi_parse_argv (parse->args, parse);
+  mi_parse_argv (parse->args (), parse);
 
   if (parse->argv == nullptr)
-    error (_("Problem parsing arguments: %s %s"), parse->command, parse->args);
+    error (_("Problem parsing arguments: %s %s"), parse->command,
+	   parse->args ());
 
 
   gdbpy_enter enter_py;

-- 
2.40.0

[PATCH v2 4/9] Change mi_parse_argv to a method

[Tom Tromey]

This changes mi_parse_argv to be a method of mi_parse.  This is just a
minor cleanup.
---
 gdb/mi/mi-cmds.c      | 2 +-
 gdb/mi/mi-parse.c     | 8 ++++----
 gdb/mi/mi-parse.h     | 7 +++----
 gdb/python/py-micmd.c | 2 +-
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index ca8c633e218..f8cae4131d8 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -49,7 +49,7 @@ struct mi_command_mi : public mi_command
      with arguments contained within PARSE.  */
   void invoke (struct mi_parse *parse) const override
   {
-    mi_parse_argv (parse->args (), parse);
+    parse->parse_argv ();
 
     if (parse->argv == nullptr)
       error (_("Problem parsing arguments: %s %s"), parse->command,
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index bf3b534e590..f077eb36a7c 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -107,9 +107,9 @@ mi_parse_escape (const char **string_ptr)
 }
 
 void
-mi_parse_argv (const char *args, struct mi_parse *parse)
+mi_parse::parse_argv ()
 {
-  const char *chp = args;
+  const char *chp = m_args.get ();
   int argc = 0;
   char **argv = XNEWVEC (char *, argc + 1);
 
@@ -124,8 +124,8 @@ mi_parse_argv (const char *args, struct mi_parse *parse)
       switch (*chp)
 	{
 	case '\0':
-	  parse->argv = argv;
-	  parse->argc = argc;
+	  this->argv = argv;
+	  this->argc = argc;
 	  return;
 	case '"':
 	  {
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index d4ac3f002e4..edb61547354 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -46,6 +46,9 @@ struct mi_parse
 
     DISABLE_COPY_AND_ASSIGN (mi_parse);
 
+    /* Split the arguments into argc/argv and store the result.  */
+    void parse_argv ();
+
     /* Return the full argument string, as used by commands which are
        implemented as CLI commands.  */
     const char *args () const
@@ -90,8 +93,4 @@ extern std::unique_ptr<struct mi_parse> mi_parse (const char *cmd,
 
 enum print_values mi_parse_print_values (const char *name);
 
-/* Split ARGS into argc/argv and store the result in PARSE.  */
-
-extern void mi_parse_argv (const char *args, struct mi_parse *parse);
-
 #endif /* MI_MI_PARSE_H */
diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
index 88d52db2202..7027210d0d8 100644
--- a/gdb/python/py-micmd.c
+++ b/gdb/python/py-micmd.c
@@ -355,7 +355,7 @@ mi_command_py::invoke (struct mi_parse *parse) const
 
   pymicmd_debug_printf ("this = %p, name = %s", this, name ());
 
-  mi_parse_argv (parse->args (), parse);
+  parse->parse_argv ();
 
   if (parse->argv == nullptr)
     error (_("Problem parsing arguments: %s %s"), parse->command,

-- 
2.40.0

[PATCH v2 5/9] Introduce "static constructor" for mi_parse

[Tom Tromey]

Change the mi_parse function to be a static method of mi_parse.  This
lets us remove the 'set_args' setter function.
---
 gdb/mi/mi-main.c  |  2 +-
 gdb/mi/mi-parse.c |  6 +++---
 gdb/mi/mi-parse.h | 28 +++++++++++++---------------
 3 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index aaebce40fac..b430115daea 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1919,7 +1919,7 @@ mi_execute_command (const char *cmd, int from_tty)
 
   try
     {
-      command = mi_parse (cmd, &token);
+      command = mi_parse::make (cmd, &token);
     }
   catch (const gdb_exception &exception)
     {
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index f077eb36a7c..b7c5a8cecdf 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -109,7 +109,7 @@ mi_parse_escape (const char **string_ptr)
 void
 mi_parse::parse_argv ()
 {
-  const char *chp = m_args.get ();
+  const char *chp = m_args.c_str ();
   int argc = 0;
   char **argv = XNEWVEC (char *, argc + 1);
 
@@ -216,7 +216,7 @@ mi_parse::~mi_parse ()
 }
 
 std::unique_ptr<struct mi_parse>
-mi_parse (const char *cmd, char **token)
+mi_parse::make (const char *cmd, char **token)
 {
   const char *chp;
 
@@ -345,7 +345,7 @@ mi_parse (const char *cmd, char **token)
     }
 
   /* Save the rest of the arguments for the command.  */
-  parse->set_args (chp);
+  parse->m_args = chp;
 
   /* Fully parsed, flag as an MI command.  */
   parse->op = MI_COMMAND;
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index edb61547354..6f1da6e6eb5 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -41,7 +41,17 @@ enum mi_command_type
 
 struct mi_parse
   {
-    mi_parse () = default;
+    /* Attempts to parse CMD returning a ``struct mi_parse''.  If CMD is
+       invalid, an exception is thrown.  For an MI_COMMAND COMMAND, ARGS
+       and OP are initialized.  Un-initialized fields are zero.  *TOKEN is
+       set to the token, even if an exception is thrown.  It is allocated
+       with xmalloc; it must either be freed with xfree, or assigned to
+       the TOKEN field of the resultant mi_parse object, to be freed by
+       mi_parse_free.  */
+
+    static std::unique_ptr<struct mi_parse> make (const char *cmd,
+						  char **token);
+
     ~mi_parse ();
 
     DISABLE_COPY_AND_ASSIGN (mi_parse);
@@ -54,9 +64,6 @@ struct mi_parse
     const char *args () const
     { return m_args.c_str (); }
 
-    void set_args (const char *args)
-    { m_args = args; }
-
     enum mi_command_type op = MI_COMMAND;
     char *command = nullptr;
     char *token = nullptr;
@@ -75,20 +82,11 @@ struct mi_parse
 
   private:
 
+    mi_parse () = default;
+
     std::string m_args;
   };
 
-/* Attempts to parse CMD returning a ``struct mi_parse''.  If CMD is
-   invalid, an exception is thrown.  For an MI_COMMAND COMMAND, ARGS
-   and OP are initialized.  Un-initialized fields are zero.  *TOKEN is
-   set to the token, even if an exception is thrown.  It is allocated
-   with xmalloc; it must either be freed with xfree, or assigned to
-   the TOKEN field of the resultant mi_parse object, to be freed by
-   mi_parse_free.  */
-
-extern std::unique_ptr<struct mi_parse> mi_parse (const char *cmd,
-						  char **token);
-
 /* Parse a string argument into a print_values value.  */
 
 enum print_values mi_parse_print_values (const char *name);

-- 
2.40.0

[PATCH v2 6/9] Introduce mi_parse helper methods

[Tom Tromey]

This introduces some helper methods for mi_parse that handle some of
the details of parsing.  This approach lets us reuse them later.
---
 gdb/mi/mi-parse.c | 69 +++++++++++++++++++++++++++++++++++++++++--------------
 gdb/mi/mi-parse.h |  9 ++++++++
 2 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index b7c5a8cecdf..09207164291 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -215,6 +215,54 @@ mi_parse::~mi_parse ()
   freeargv (argv);
 }
 
+/* See mi-parse.h.  */
+
+void
+mi_parse::set_thread_group (const char *arg, char **endp)
+{
+  if (thread_group != -1)
+    error (_("Duplicate '--thread-group' option"));
+  if (*arg != 'i')
+    error (_("Invalid thread group id"));
+  arg += 1;
+  thread_group = strtol (arg, endp, 10);
+}
+
+/* See mi-parse.h.  */
+
+void
+mi_parse::set_thread (const char *arg, char **endp)
+{
+  if (thread != -1)
+    error (_("Duplicate '--thread' option"));
+  thread = strtol (arg, endp, 10);
+}
+
+/* See mi-parse.h.  */
+
+void
+mi_parse::set_frame (const char *arg, char **endp)
+{
+  if (frame != -1)
+    error (_("Duplicate '--frame' option"));
+  frame = strtol (arg, endp, 10);
+}
+
+/* See mi-parse.h.  */
+
+void
+mi_parse::set_language (const char *arg, const char **endp)
+{
+  std::string lang_name = extract_arg (&arg);
+
+  language = language_enum (lang_name.c_str ());
+  if (language == language_unknown)
+    error (_("Invalid --language argument: %s"), lang_name.c_str ());
+
+  if (endp != nullptr)
+    *endp = arg;
+}
+
 std::unique_ptr<struct mi_parse>
 mi_parse::make (const char *cmd, char **token)
 {
@@ -295,13 +343,8 @@ mi_parse::make (const char *cmd, char **token)
 	  char *endp;
 
 	  option = "--thread-group";
-	  if (parse->thread_group != -1)
-	    error (_("Duplicate '--thread-group' option"));
 	  chp += tgs;
-	  if (*chp != 'i')
-	    error (_("Invalid thread group id"));
-	  chp += 1;
-	  parse->thread_group = strtol (chp, &endp, 10);
+	  parse->set_thread_group (chp, &endp);
 	  chp = endp;
 	}
       else if (strncmp (chp, "--thread ", ts) == 0)
@@ -309,10 +352,8 @@ mi_parse::make (const char *cmd, char **token)
 	  char *endp;
 
 	  option = "--thread";
-	  if (parse->thread != -1)
-	    error (_("Duplicate '--thread' option"));
 	  chp += ts;
-	  parse->thread = strtol (chp, &endp, 10);
+	  parse->set_thread (chp, &endp);
 	  chp = endp;
 	}
       else if (strncmp (chp, "--frame ", fs) == 0)
@@ -320,21 +361,15 @@ mi_parse::make (const char *cmd, char **token)
 	  char *endp;
 
 	  option = "--frame";
-	  if (parse->frame != -1)
-	    error (_("Duplicate '--frame' option"));
 	  chp += fs;
-	  parse->frame = strtol (chp, &endp, 10);
+	  parse->set_frame (chp, &endp);
 	  chp = endp;
 	}
       else if (strncmp (chp, "--language ", ls) == 0)
 	{
 	  option = "--language";
 	  chp += ls;
-	  std::string lang_name = extract_arg (&chp);
-
-	  parse->language = language_enum (lang_name.c_str ());
-	  if (parse->language == language_unknown)
-	    error (_("Invalid --language argument: %s"), lang_name.c_str ());
+	  parse->set_language (chp, &chp);
 	}
       else
 	break;
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index 6f1da6e6eb5..19c41f23ed6 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -84,6 +84,15 @@ struct mi_parse
 
     mi_parse () = default;
 
+    /* Helper methods for parsing arguments.  Each takes the argument
+       to be parsed.  It will either set a member of this object, or
+       throw an exception on error.  In each case, *ENDP, if non-NULL,
+       will be updated to just after the argument text.  */
+    void set_thread_group (const char *arg, char **endp);
+    void set_thread (const char *arg, char **endp);
+    void set_frame (const char *arg, char **endp);
+    void set_language (const char *arg, const char **endp);
+
     std::string m_args;
   };
 

-- 
2.40.0

[PATCH v2 7/9] Add second mi_parse constructor

[Tom Tromey]

This adds a second mi_parse constructor.  This constructor takes a
command name and vector of arguments, and does not do any escape
processing.  This also changes mi_parse::args to handle parse objects
created this new way.
---
 gdb/mi/mi-parse.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/mi/mi-parse.h | 12 +++++--
 2 files changed, 107 insertions(+), 2 deletions(-)

diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index 09207164291..a113d4d48da 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -109,6 +109,11 @@ mi_parse_escape (const char **string_ptr)
 void
 mi_parse::parse_argv ()
 {
+  /* If arguments were already computed (or were supplied at
+     construction), then there's no need to re-compute them.  */
+  if (argv != nullptr)
+    return;
+
   const char *chp = m_args.c_str ();
   int argc = 0;
   char **argv = XNEWVEC (char *, argc + 1);
@@ -217,6 +222,27 @@ mi_parse::~mi_parse ()
 
 /* See mi-parse.h.  */
 
+const char *
+mi_parse::args ()
+{
+  /* If args were already computed, or if there is no pre-computed
+     argv, just return the args.  */
+  if (!m_args.empty () || argv == nullptr)
+    return  m_args.c_str ();
+
+  /* Compute args from argv.  */
+  for (int i = 0; i < argc; ++i)
+    {
+      if (!m_args.empty ())
+	m_args += " ";
+      m_args += argv[i];
+    }
+
+  return m_args.c_str ();
+}
+
+/* See mi-parse.h.  */
+
 void
 mi_parse::set_thread_group (const char *arg, char **endp)
 {
@@ -387,6 +413,77 @@ mi_parse::make (const char *cmd, char **token)
   return parse;
 }
 
+/* See mi-parse.h.  */
+
+std::unique_ptr<struct mi_parse>
+mi_parse::make (gdb::unique_xmalloc_ptr<char> command,
+		std::vector<gdb::unique_xmalloc_ptr<char>> args)
+{
+  std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
+
+  parse->command = command.release ();
+  parse->token = xstrdup ("");
+
+  if (parse->command[0] != '-')
+    throw_error (UNDEFINED_COMMAND_ERROR,
+		 _("MI command '%s' does not start with '-'"),
+		 parse->command);
+
+  /* Find the command in the MI table.  */
+  parse->cmd = mi_cmd_lookup (parse->command + 1);
+  if (parse->cmd == NULL)
+    throw_error (UNDEFINED_COMMAND_ERROR,
+		 _("Undefined MI command: %s"), parse->command);
+
+  /* This over-allocates slightly, but it seems unimportant.  */
+  parse->argv = XCNEWVEC (char *, args.size () + 1);
+
+  for (size_t i = 0; i < args.size (); ++i)
+    {
+      const char *chp = args[i].get ();
+
+      /* See if --all is the last token in the input.  */
+      if (strcmp (chp, "--all") == 0)
+	{
+	  parse->all = 1;
+	}
+      else if (strcmp (chp, "--thread-group") == 0)
+	{
+	  ++i;
+	  if (i == args.size ())
+	    error ("No argument to '--thread-group'");
+	  parse->set_thread_group (args[i].get (), nullptr);
+	}
+      else if (strcmp (chp, "--thread") == 0)
+	{
+	  ++i;
+	  if (i == args.size ())
+	    error ("No argument to '--thread'");
+	  parse->set_thread (args[i].get (), nullptr);
+	}
+      else if (strcmp (chp, "--frame") == 0)
+	{
+	  ++i;
+	  if (i == args.size ())
+	    error ("No argument to '--frame'");
+	  parse->set_frame (args[i].get (), nullptr);
+	}
+      else if (strcmp (chp, "--language") == 0)
+	{
+	  ++i;
+	  if (i == args.size ())
+	    error ("No argument to '--language'");
+	  parse->set_language (args[i].get (), nullptr);
+	}
+      else
+	parse->argv[parse->argc++] = args[i].release ();
+    }
+
+  /* Fully parsed, flag as an MI command.  */
+  parse->op = MI_COMMAND;
+  return parse;
+}
+
 enum print_values
 mi_parse_print_values (const char *name)
 {
diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
index 19c41f23ed6..6373543529b 100644
--- a/gdb/mi/mi-parse.h
+++ b/gdb/mi/mi-parse.h
@@ -52,6 +52,15 @@ struct mi_parse
     static std::unique_ptr<struct mi_parse> make (const char *cmd,
 						  char **token);
 
+    /* Create an mi_parse object given the command name and a vector
+       of arguments.  Unlike with the other constructor, here the
+       arguments are treated "as is" -- no escape processing is
+       done.  */
+
+    static std::unique_ptr<struct mi_parse> make
+	 (gdb::unique_xmalloc_ptr<char> command,
+	  std::vector<gdb::unique_xmalloc_ptr<char>> args);
+
     ~mi_parse ();
 
     DISABLE_COPY_AND_ASSIGN (mi_parse);
@@ -61,8 +70,7 @@ struct mi_parse
 
     /* Return the full argument string, as used by commands which are
        implemented as CLI commands.  */
-    const char *args () const
-    { return m_args.c_str (); }
+    const char *args ();
 
     enum mi_command_type op = MI_COMMAND;
     char *command = nullptr;

-- 
2.40.0

[PATCH v2 8/9] Implement gdb.execute_mi

[Tom Tromey]

This adds a new Python function, gdb.execute_mi, that can be used to
invoke an MI command but get the output as a Python object, rather
than a string.  This is done by implementing a new ui_out subclass
that builds a Python object.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=11688
---
 gdb/Makefile.in                         |   1 +
 gdb/NEWS                                |   3 +
 gdb/doc/python.texi                     |  30 ++++
 gdb/mi/mi-cmds.h                        |   5 +
 gdb/mi/mi-main.c                        |  15 ++
 gdb/python/py-mi.c                      | 298 ++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h            |   3 +
 gdb/python/python.c                     |   5 +
 gdb/testsuite/gdb.python/py-exec-mi.exp |  32 ++++
 gdb/testsuite/gdb.python/py-mi-cmd.py   |  27 +++
 10 files changed, 419 insertions(+)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 14b5dd0bad6..d909786792c 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -414,6 +414,7 @@ SUBDIR_PYTHON_SRCS = \
 	python/py-lazy-string.c \
 	python/py-linetable.c \
 	python/py-membuf.c \
+	python/py-mi.c \
 	python/py-micmd.c \
 	python/py-newobjfileevent.c \
 	python/py-objfile.c \
diff --git a/gdb/NEWS b/gdb/NEWS
index b82114d80b0..3ea7237d022 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -192,6 +192,9 @@ info main
      - Changes are backwards compatible, the older API can still be
        used to disassemble instructions without styling.
 
+  ** New function gdb.execute_mi(COMMAND, [ARG]...), that invokes a
+     GDB/MI command and returns the output as a Python dictionary.
+
 *** Changes in GDB 13
 
 * MI version 1 is deprecated, and will be removed in GDB 14.
diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
index d93ee55690e..b2f3cb4874b 100644
--- a/gdb/doc/python.texi
+++ b/gdb/doc/python.texi
@@ -4584,6 +4584,36 @@ commands have been added:
 (@value{GDBP})
 @end smallexample
 
+Conversely, it is possible to execute @sc{gdb/mi} commands from
+Python, with the results being a Python object and not a
+specially-formatted string.  This is done with the
+@code{gdb.execute_mi} function.
+
+@findex gdb.execute_mi
+@defun gdb.execute_mi (command @r{[}, arg @r{]}@dots{})
+Invoke a @sc{gdb/mi} command.  @var{command} is the name of the
+command, a string.  The arguments, @var{arg}, are passed to the
+command.  Each argument must also be a string.
+
+This function returns a Python dictionary whose contents reflect the
+corresponding @sc{GDB/MI} command's output.  Refer to the
+documentation for these commands for details.  Lists are represented
+as Python lists, and tuples are represented as Python dictionaries.
+
+If the command fails, it will raise a Python exception.
+@end defun
+
+Here is how this works using the commands from the example above:
+
+@smallexample
+(@value{GDBP}) python print(gdb.execute_mi("-echo-dict", "abc", "def", "ghi"))
+@{'dict': @{'argv': ['abc', 'def', 'ghi']@}@}
+(@value{GDBP}) python print(gdb.execute_mi("-echo-list", "abc", "def", "ghi"))
+@{'list': ['abc', 'def', 'ghi']@}
+(@value{GDBP}) python print(gdb.execute_mi("-echo-string", "abc", "def", "ghi"))
+@{'string': 'abc, def, ghi'@}
+@end smallexample
+
 @node Parameters In Python
 @subsubsection Parameters In Python
 
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index d867c298df9..c62b134fe3d 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -206,6 +206,11 @@ extern mi_command *mi_cmd_lookup (const char *command);
 
 extern void mi_execute_command (const char *cmd, int from_tty);
 
+/* Execute an MI command given an already-constructed parse
+   object.  */
+
+extern void mi_execute_command (mi_parse *context);
+
 /* Insert COMMAND into the global mi_cmd_table.  Return false if
    COMMAND->name already exists in mi_cmd_table, in which case COMMAND will
    not have been added to mi_cmd_table.  Otherwise, return true, and
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index b430115daea..f3b7490e089 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1965,6 +1965,21 @@ mi_execute_command (const char *cmd, int from_tty)
     }
 }
 
+/* See mi-cmds.h.  */
+
+void
+mi_execute_command (mi_parse *context)
+{
+  if (context->op != MI_COMMAND)
+    error (_("Command is not an MI command"));
+
+  scoped_restore save_token = make_scoped_restore (&current_token,
+						   context->token);
+  scoped_restore save_debug = make_scoped_restore (&mi_debug_p, 0);
+
+  mi_cmd_execute (context);
+}
+
 /* Captures the current user selected context state, that is the current
    thread and frame.  Later we can then check if the user selected context
    has changed at all.  */
diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
new file mode 100644
index 00000000000..0fcd57844e7
--- /dev/null
+++ b/gdb/python/py-mi.c
@@ -0,0 +1,298 @@
+/* Python interface to MI commands
+
+   Copyright (C) 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/>.  */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "ui-out.h"
+#include "mi/mi-parse.h"
+
+/* A ui_out subclass that creates a Python object based on the data
+   that is passed in.  */
+
+class py_ui_out : public ui_out
+{
+public:
+
+  py_ui_out ()
+    : ui_out (fix_multi_location_breakpoint_output
+	      | fix_breakpoint_script_output)
+  {
+    do_begin (ui_out_type_tuple, nullptr);
+  }
+
+  bool can_emit_style_escape () const override
+  { return false; }
+
+  bool do_is_mi_like_p () const override
+  { return true; }
+
+  /* Return the Python object that was created.  If a Python error
+     occurred during the processing, set the Python error and return
+     nullptr.  */
+  PyObject *result ()
+  {
+    if (m_error.has_value ())
+      {
+	m_error->restore ();
+	return nullptr;
+      }
+    return current ().obj.release ();
+  }
+
+protected:
+
+  void do_progress_end () override { }
+  void do_progress_start () override { }
+  void do_progress_notify (const std::string &, const char *, double, double)
+    override
+  { }
+
+  void do_table_begin (int nbrofcols, int nr_rows, const char *tblid) override
+  {
+    do_begin (ui_out_type_list, tblid);
+  }
+  void do_table_body () override
+  { }
+  void do_table_end () override
+  {
+    do_end (ui_out_type_list);
+  }
+  void do_table_header (int width, ui_align align,
+			const std::string &col_name,
+			const std::string &col_hdr) override
+  { }
+
+  void do_begin (ui_out_type type, const char *id) override;
+  void do_end (ui_out_type type) override;
+
+  void do_field_signed (int fldno, int width, ui_align align,
+			const char *fldname, LONGEST value) override;
+  void do_field_unsigned (int fldno, int width, ui_align align,
+			  const char *fldname, ULONGEST value) override;
+
+  void do_field_skip (int fldno, int width, ui_align align,
+		      const char *fldname) override
+  { }
+
+  void do_field_string (int fldno, int width, ui_align align,
+			const char *fldname, const char *string,
+			const ui_file_style &style) override;
+  void do_field_fmt (int fldno, int width, ui_align align,
+		     const char *fldname, const ui_file_style &style,
+		     const char *format, va_list args) override
+    ATTRIBUTE_PRINTF (7, 0);
+
+  void do_spaces (int numspaces) override
+  { }
+
+  void do_text (const char *string) override
+  { }
+
+  void do_message (const ui_file_style &style,
+		   const char *format, va_list args)
+    override ATTRIBUTE_PRINTF (3,0)
+  { }
+
+  void do_wrap_hint (int indent) override
+  { }
+
+  void do_flush () override
+  { }
+
+  void do_redirect (struct ui_file *outstream) override
+  { }
+
+private:
+
+  /* When constructing Python objects, this class keeps a stack of
+     objects being constructed.  Each such object has this type.  */
+  struct object_desc
+  {
+    /* Name of the field (or empty for lists) that this object will
+       eventually become.  */
+    std::string field_name;
+    /* The object under construction.  */
+    gdbpy_ref<> obj;
+    /* The type of structure being created.  Note that tables are
+       treated as lists here.  */
+    ui_out_type type;
+  };
+
+  /* The stack of objects being created.  */
+  std::vector<object_desc> m_objects;
+
+  /* If an error occurred, this holds the exception information for
+     use by the 'release' method.  */
+  gdb::optional<gdbpy_err_fetch> m_error;
+
+  /* Return a reference to the object under construction.  */
+  object_desc &current ()
+  { return m_objects.back (); }
+
+  /* Add a new field to the current object under construction.  */
+  void add_field (const char *name, const gdbpy_ref<> &obj);
+};
+
+void
+py_ui_out::add_field (const char *name, const gdbpy_ref<> &obj)
+{
+  if (obj == nullptr)
+    {
+      m_error.emplace ();
+      return;
+    }
+
+  object_desc &desc = current ();
+  if (desc.type == ui_out_type_list)
+    {
+      if (PyList_Append (desc.obj.get (), obj.get ()) < 0)
+	m_error.emplace ();
+    }
+  else
+    {
+      if (PyDict_SetItemString (desc.obj.get (), name, obj.get ()) < 0)
+	m_error.emplace ();
+    }
+}
+
+void
+py_ui_out::do_begin (ui_out_type type, const char *id)
+{
+  if (m_error.has_value ())
+    return;
+
+  gdbpy_ref<> new_obj (type == ui_out_type_list
+		       ? PyList_New (0)
+		       : PyDict_New ());
+  if (new_obj == nullptr)
+    {
+      m_error.emplace ();
+      return;
+    }
+
+  object_desc new_desc;
+  if (id != nullptr)
+    new_desc.field_name = id;
+  new_desc.obj = std::move (new_obj);
+  new_desc.type = type;
+
+  m_objects.push_back (std::move (new_desc));
+}
+
+void
+py_ui_out::do_end (ui_out_type type)
+{
+  if (m_error.has_value ())
+    return;
+
+  object_desc new_obj = std::move (current ());
+  m_objects.pop_back ();
+  add_field (new_obj.field_name.c_str (), new_obj.obj);
+}
+
+void
+py_ui_out::do_field_signed (int fldno, int width, ui_align align,
+			    const char *fldname, LONGEST value)
+{
+  if (m_error.has_value ())
+    return;
+
+  gdbpy_ref<> val = gdb_py_object_from_longest (value);
+  add_field (fldname, val);
+}
+
+void
+py_ui_out::do_field_unsigned (int fldno, int width, ui_align align,
+			    const char *fldname, ULONGEST value)
+{
+  if (m_error.has_value ())
+    return;
+
+  gdbpy_ref<> val = gdb_py_object_from_ulongest (value);
+  add_field (fldname, val);
+}
+
+void
+py_ui_out::do_field_string (int fldno, int width, ui_align align,
+			    const char *fldname, const char *string,
+			    const ui_file_style &style)
+{
+  if (m_error.has_value ())
+    return;
+
+  gdbpy_ref<> val = host_string_to_python_string (string);
+  add_field (fldname, val);
+}
+
+void
+py_ui_out::do_field_fmt (int fldno, int width, ui_align align,
+			 const char *fldname, const ui_file_style &style,
+			 const char *format, va_list args)
+{
+  if (m_error.has_value ())
+    return;
+
+  std::string str = string_vprintf (format, args);
+  do_field_string (fldno, width, align, fldname, str.c_str (), style);
+}
+
+/* Implementation of the gdb.execute_mi command.  */
+
+PyObject *
+gdbpy_execute_mi_command (PyObject *self, PyObject *args, PyObject *kw)
+{
+  gdb::unique_xmalloc_ptr<char> mi_command;
+  std::vector<gdb::unique_xmalloc_ptr<char>> arg_strings;
+
+  Py_ssize_t n_args = PyTuple_Size (args);
+  if (n_args < 0)
+    return nullptr;
+
+  for (Py_ssize_t i = 0; i < n_args; ++i)
+    {
+      /* Note this returns a borrowed reference.  */
+      PyObject *arg = PyTuple_GetItem (args, i);
+      if (arg == nullptr)
+	return nullptr;
+      gdb::unique_xmalloc_ptr<char> str = python_string_to_host_string (arg);
+      if (str == nullptr)
+	return nullptr;
+      if (i == 0)
+	mi_command = std::move (str);
+      else
+	arg_strings.push_back (std::move (str));
+    }
+
+  py_ui_out uiout;
+
+  try
+    {
+      scoped_restore save_uiout = make_scoped_restore (&current_uiout, &uiout);
+      std::unique_ptr<struct mi_parse> parser
+	= mi_parse::make (std::move (mi_command), std::move (arg_strings));
+      mi_execute_command (parser.get ());
+    }
+  catch (const gdb_exception &except)
+    {
+      gdbpy_convert_exception (except);
+      return nullptr;
+    }
+
+  return uiout.result ();
+}
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 1142e0e739d..93217375cc5 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -483,6 +483,9 @@ struct symtab_and_line *sal_object_to_symtab_and_line (PyObject *obj);
 frame_info_ptr frame_object_to_frame_info (PyObject *frame_obj);
 struct gdbarch *arch_object_to_gdbarch (PyObject *obj);
 
+extern PyObject *gdbpy_execute_mi_command (PyObject *self, PyObject *args,
+					   PyObject *kw);
+
 /* Convert Python object OBJ to a program_space pointer.  OBJ must be a
    gdb.Progspace reference.  Return nullptr if the gdb.Progspace is not
    valid (see gdb.Progspace.is_valid), otherwise return the program_space
diff --git a/gdb/python/python.c b/gdb/python/python.c
index fd5a920cbdb..9703d6ef900 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -2484,6 +2484,11 @@ PyMethodDef python_GdbMethods[] =
 Evaluate command, a string, as a gdb CLI command.  Optionally returns\n\
 a Python String containing the output of the command if to_string is\n\
 set to True." },
+  { "execute_mi", (PyCFunction) gdbpy_execute_mi_command,
+    METH_VARARGS | METH_KEYWORDS,
+    "execute_mi (command, arg...) -> dictionary\n\
+Evaluate command, a string, as a gdb MI command.\n\
+Arguments (also strings) are passed to the command." },
   { "parameter", gdbpy_parameter, METH_VARARGS,
     "Return a gdb parameter's value" },
 
diff --git a/gdb/testsuite/gdb.python/py-exec-mi.exp b/gdb/testsuite/gdb.python/py-exec-mi.exp
new file mode 100644
index 00000000000..09c4877870c
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-exec-mi.exp
@@ -0,0 +1,32 @@
+# Copyright (C) 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 gdb.execute_mi.
+
+load_lib gdb-python.exp
+require allow_python_tests
+
+clean_restart
+
+gdb_test_no_output "source ${srcdir}/${subdir}/py-mi-cmd.py" \
+    "load python file"
+
+gdb_test "python run_execute_mi_tests()" "PASS"
+
+# Be sure to test a command implemented as CLI command, as those fetch
+# the args.
+gdb_test_no_output "python gdb.execute_mi('-exec-arguments', 'a', 'b', 'c')" \
+    "set arguments"
+
+gdb_test "show args" ".*\"a b c\"."
diff --git a/gdb/testsuite/gdb.python/py-mi-cmd.py b/gdb/testsuite/gdb.python/py-mi-cmd.py
index c7bf5b7226f..2c86c909858 100644
--- a/gdb/testsuite/gdb.python/py-mi-cmd.py
+++ b/gdb/testsuite/gdb.python/py-mi-cmd.py
@@ -118,3 +118,30 @@ def free_invoke(obj, args):
 # these as a Python function which is then called from the exp script.
 def run_exception_tests():
     print("PASS")
+
+
+# Run some execute_mi tests.  This is easier to do from Python.
+def run_execute_mi_tests():
+    # Install the command.
+    cmd = pycmd1("-pycmd")
+    # Pass in a representative subset of the pycmd1 keys, and then
+    # check that the result via MI is the same as the result via a
+    # direct Python call.  Note that some results won't compare as
+    # equal -- for example, a Python MI command can return a tuple,
+    # but that will be translated to a Python list.
+    for name in ("int", "str", "dct"):
+        expect = cmd.invoke([name])
+        got = gdb.execute_mi("-pycmd", name)
+        if expect != got:
+            print("FAIL: saw " + repr(got) + ", but expected " + repr(expect))
+            return
+    ok = False
+    try:
+        gdb.execute_mi("-pycmd", "exp")
+    # Due to the "denaturation" problem, we have to expect a gdb.error
+    # here and not a gdb.GdbError.
+    except gdb.error:
+        ok = True
+    if not ok:
+        print("FAIL: did not throw exception")
+    print("PASS")

-- 
2.40.0

[PATCH v2 9/9] Implement DAP loadedSources request

[Tom Tromey]

This implements the DAP loadedSources request, using gdb.execute_mi to
avoid having to write another custom Python API.
---
 gdb/data-directory/Makefile.in      |  1 +
 gdb/python/lib/gdb/dap/__init__.py  |  1 +
 gdb/python/lib/gdb/dap/sources.py   | 40 +++++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.dap/basic-dap.exp |  3 +++
 4 files changed, 45 insertions(+)

diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 39979037245..a95c2d7ab37 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -101,6 +101,7 @@ PYTHON_FILE_LIST = \
 	gdb/dap/pause.py \
 	gdb/dap/scopes.py \
 	gdb/dap/server.py \
+	gdb/dap/sources.py \
 	gdb/dap/startup.py \
 	gdb/dap/state.py \
 	gdb/dap/threads.py \
diff --git a/gdb/python/lib/gdb/dap/__init__.py b/gdb/python/lib/gdb/dap/__init__.py
index 014fd086f4b..f07228e46ce 100644
--- a/gdb/python/lib/gdb/dap/__init__.py
+++ b/gdb/python/lib/gdb/dap/__init__.py
@@ -29,6 +29,7 @@ from . import memory
 from . import next
 from . import pause
 from . import scopes
+from . import sources
 from . import threads
 
 from .server import Server
diff --git a/gdb/python/lib/gdb/dap/sources.py b/gdb/python/lib/gdb/dap/sources.py
new file mode 100644
index 00000000000..af7313ca2f9
--- /dev/null
+++ b/gdb/python/lib/gdb/dap/sources.py
@@ -0,0 +1,40 @@
+# 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/>.
+
+import gdb
+
+from .server import request, capability
+from .startup import send_gdb_with_response, in_gdb_thread
+
+
+@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"],
+            }
+        )
+    return {
+        "sources": result,
+    }
+
+
+@request("loadedSources")
+@capability("supportsLoadedSourcesRequest")
+def sources(**extra):
+    return send_gdb_with_response(_sources)
diff --git a/gdb/testsuite/gdb.dap/basic-dap.exp b/gdb/testsuite/gdb.dap/basic-dap.exp
index 0026690ba44..f28239d8268 100644
--- a/gdb/testsuite/gdb.dap/basic-dap.exp
+++ b/gdb/testsuite/gdb.dap/basic-dap.exp
@@ -168,4 +168,7 @@ 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

-- 
2.40.0

Re: [PATCH v2 8/9] Implement gdb.execute_mi

[Eli Zaretskii]

> Date: Thu, 18 May 2023 14:18:14 -0600
> From: Tom Tromey via Gdb-patches <gdb-patches@sourceware.org>
> 
> This adds a new Python function, gdb.execute_mi, that can be used to
> invoke an MI command but get the output as a Python object, rather
> than a string.  This is done by implementing a new ui_out subclass
> that builds a Python object.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=11688
> ---
>  gdb/Makefile.in                         |   1 +
>  gdb/NEWS                                |   3 +
>  gdb/doc/python.texi                     |  30 ++++
>  gdb/mi/mi-cmds.h                        |   5 +
>  gdb/mi/mi-main.c                        |  15 ++
>  gdb/python/py-mi.c                      | 298 ++++++++++++++++++++++++++++++++
>  gdb/python/python-internal.h            |   3 +
>  gdb/python/python.c                     |   5 +
>  gdb/testsuite/gdb.python/py-exec-mi.exp |  32 ++++
>  gdb/testsuite/gdb.python/py-mi-cmd.py   |  27 +++
>  10 files changed, 419 insertions(+)

Thanks, the documentation parts are OK.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>

Re: [PATCH v2 0/9] Implement the DAP "loadedSources" request

[Tom Tromey]

>>>>> "Tom" == Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> This series implements the DAP "loadedSources" request.  However, it
Tom> does so by exposing MI commands to Python, with MI output being
Tom> converted to Python objects via a new ui_out implementation.

Tom> Regression tested on x86-64 Fedora 36.

I'm going to check this in now.

Tom

Re: [PATCH v2 8/9] Implement gdb.execute_mi

[Simon Marchi]

On 5/18/23 16:18, Tom Tromey via Gdb-patches wrote:
> This adds a new Python function, gdb.execute_mi, that can be used to
> invoke an MI command but get the output as a Python object, rather
> than a string.  This is done by implementing a new ui_out subclass
> that builds a Python object.

Hi Tom,

My patch here:

https://inbox.sourceware.org/gdb-patches/48eb5ee9-53e1-f1df-b424-d08342c7e857@simark.ca/T/#m94825956483bc1d340da33ad4712752d3326fecf

conflicts with this patch, so I am taking a closer look at
gdb.execute_mi.  I think some things are missing from the gdb.execute_mi
implementation, one way to expose some are to try:

  (gdb) python gdb.execute_mi('-gdb-exit')

First, I think that the new mi_execute_command function should install
the mi interpreter as the current interpreter (or the command
interpreter, I don't really understand the distinction) such as code
down mi_execute_command gets the mi_interp object when calling
current_interpreter.  There are not a lot of commands getting the
current interpreter, but -gdb-exit is one of them.  I tried fixing this
quickly, but it's not just a oneliner.  We need to ens

The other places that get the current interpreter are mostly in the
observers that output notifications.  So that got me wondering, what
should we do with any MI notification happening during the execution of
the MI command?  MI notifications are often suppressed during the
execution of MI commands, but I suspect that there might some legitimate
cases where notifications would go through.  I was wondering what to do
about them.  They are written directly to mi->raw_stdout, so they would
likely just appear on the terminal.  Should we just make them go to
/dev/null?  Should we accumulate them and return a list of notifications
as part of gdb.execute_mi's return value?  Should the caller of
gdb.execute_mi be able to provide a callback that gets called whenever
there is a notification?

The latter has the advantage that the Python code get notified
immediately when the notification is sent, rather than receiving them
all at once when the command's execution is complete.  For something
like -target-download, it might be interesting.  It also has the
advantage that it can be implemented later without breaking
gdb.execute_mi (just add an optional parameter).  Whereas the solution
of returning them as a list would require changing gdb.execute_mi now,
to avoid doing a breaking change later.

Simon

Re: [PATCH v2 8/9] Implement gdb.execute_mi

[Tom Tromey]

Simon> My patch here:

Simon> https://inbox.sourceware.org/gdb-patches/48eb5ee9-53e1-f1df-b424-d08342c7e857@simark.ca/T/#m94825956483bc1d340da33ad4712752d3326fecf

Simon> conflicts with this patch, so I am taking a closer look at
Simon> gdb.execute_mi.  I think some things are missing from the gdb.execute_mi
Simon> implementation [...]

I just wanted to let you know I haven't dropped this.
I didn't look at it much but I hope to investigate a bit next week.

Simon>   (gdb) python gdb.execute_mi('-gdb-exit')

Simon> First, I think that the new mi_execute_command function should install
Simon> the mi interpreter as the current interpreter (or the command
Simon> interpreter, I don't really understand the distinction) such as code
Simon> down mi_execute_command gets the mi_interp object when calling
Simon> current_interpreter.  There are not a lot of commands getting the
Simon> current interpreter, but -gdb-exit is one of them.  I tried fixing this
Simon> quickly, but it's not just a oneliner.

I wouldn't mind disallowing -gdb-exit from execute_mi.  That's a
pathological case anyway.

I looked for calls to current_interpreter in the MI code but I don't see
many... though who knows what the core of gdb is doing.  In fact the
-gdb-exit one is the only one that seems to be part of a command.

Tom

Re: [PATCH v2 8/9] Implement gdb.execute_mi

[Tom Tromey]

Simon> There are not a lot of commands getting the current interpreter,
Simon> but -gdb-exit is one of them.

I looked at this a little bit today.

I found 4 places doing this:

  struct mi_interp *mi = (struct mi_interp *) command_interp ();

A couple of these are, IMO, latent bugs.

For -gdb-exit in particular, it seems completely fine for me to simply
not print anything if the command interpreter is not an MI interpreter.
After all, gdb is about to exit, and the Python call isn't returning
anyway.

Some other spots should probably use gdb::checked_static_cast to find
bugs earlier.

Simon> The other places that get the current interpreter are mostly in the
Simon> observers that output notifications.  So that got me wondering, what
Simon> should we do with any MI notification happening during the execution of
Simon> the MI command?

I still haven't looked into this part.

Tom

Re: [PATCH v2 5/9] Introduce "static constructor" for mi_parse

[Andrew Burgess]

Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> writes:

> Change the mi_parse function to be a static method of mi_parse.  This
> lets us remove the 'set_args' setter function.

Hi Tom,

Sorry to respond to such an old thread, but I ran into the
mi_parse::make code today.  From your commit message I don't see the
connection for why we need a static mi_parse::make function in order to
get rid of the set_args method.

For reference, I switched the code back to using constructors (but
didn't add back set_args), and I'm not seeing any regressions in
gdb.mi/* and gdb.python/*.

Could you expand on why this change is needed?

Thanks,
Andrew


> ---
>  gdb/mi/mi-main.c  |  2 +-
>  gdb/mi/mi-parse.c |  6 +++---
>  gdb/mi/mi-parse.h | 28 +++++++++++++---------------
>  3 files changed, 17 insertions(+), 19 deletions(-)
>
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index aaebce40fac..b430115daea 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -1919,7 +1919,7 @@ mi_execute_command (const char *cmd, int from_tty)
>  
>    try
>      {
> -      command = mi_parse (cmd, &token);
> +      command = mi_parse::make (cmd, &token);
>      }
>    catch (const gdb_exception &exception)
>      {
> diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
> index f077eb36a7c..b7c5a8cecdf 100644
> --- a/gdb/mi/mi-parse.c
> +++ b/gdb/mi/mi-parse.c
> @@ -109,7 +109,7 @@ mi_parse_escape (const char **string_ptr)
>  void
>  mi_parse::parse_argv ()
>  {
> -  const char *chp = m_args.get ();
> +  const char *chp = m_args.c_str ();
>    int argc = 0;
>    char **argv = XNEWVEC (char *, argc + 1);
>  
> @@ -216,7 +216,7 @@ mi_parse::~mi_parse ()
>  }
>  
>  std::unique_ptr<struct mi_parse>
> -mi_parse (const char *cmd, char **token)
> +mi_parse::make (const char *cmd, char **token)
>  {
>    const char *chp;
>  
> @@ -345,7 +345,7 @@ mi_parse (const char *cmd, char **token)
>      }
>  
>    /* Save the rest of the arguments for the command.  */
> -  parse->set_args (chp);
> +  parse->m_args = chp;
>  
>    /* Fully parsed, flag as an MI command.  */
>    parse->op = MI_COMMAND;
> diff --git a/gdb/mi/mi-parse.h b/gdb/mi/mi-parse.h
> index edb61547354..6f1da6e6eb5 100644
> --- a/gdb/mi/mi-parse.h
> +++ b/gdb/mi/mi-parse.h
> @@ -41,7 +41,17 @@ enum mi_command_type
>  
>  struct mi_parse
>    {
> -    mi_parse () = default;
> +    /* Attempts to parse CMD returning a ``struct mi_parse''.  If CMD is
> +       invalid, an exception is thrown.  For an MI_COMMAND COMMAND, ARGS
> +       and OP are initialized.  Un-initialized fields are zero.  *TOKEN is
> +       set to the token, even if an exception is thrown.  It is allocated
> +       with xmalloc; it must either be freed with xfree, or assigned to
> +       the TOKEN field of the resultant mi_parse object, to be freed by
> +       mi_parse_free.  */
> +
> +    static std::unique_ptr<struct mi_parse> make (const char *cmd,
> +						  char **token);
> +
>      ~mi_parse ();
>  
>      DISABLE_COPY_AND_ASSIGN (mi_parse);
> @@ -54,9 +64,6 @@ struct mi_parse
>      const char *args () const
>      { return m_args.c_str (); }
>  
> -    void set_args (const char *args)
> -    { m_args = args; }
> -
>      enum mi_command_type op = MI_COMMAND;
>      char *command = nullptr;
>      char *token = nullptr;
> @@ -75,20 +82,11 @@ struct mi_parse
>  
>    private:
>  
> +    mi_parse () = default;
> +
>      std::string m_args;
>    };
>  
> -/* Attempts to parse CMD returning a ``struct mi_parse''.  If CMD is
> -   invalid, an exception is thrown.  For an MI_COMMAND COMMAND, ARGS
> -   and OP are initialized.  Un-initialized fields are zero.  *TOKEN is
> -   set to the token, even if an exception is thrown.  It is allocated
> -   with xmalloc; it must either be freed with xfree, or assigned to
> -   the TOKEN field of the resultant mi_parse object, to be freed by
> -   mi_parse_free.  */
> -
> -extern std::unique_ptr<struct mi_parse> mi_parse (const char *cmd,
> -						  char **token);
> -
>  /* Parse a string argument into a print_values value.  */
>  
>  enum print_values mi_parse_print_values (const char *name);
>
> -- 
> 2.40.0

Re: [PATCH v2 5/9] Introduce "static constructor" for mi_parse

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> Sorry to respond to such an old thread, but I ran into the
Andrew> mi_parse::make code today.  From your commit message I don't see the
Andrew> connection for why we need a static mi_parse::make function in order to
Andrew> get rid of the set_args method.

Andrew> For reference, I switched the code back to using constructors (but
Andrew> didn't add back set_args), and I'm not seeing any regressions in
Andrew> gdb.mi/* and gdb.python/*.

Andrew> Could you expand on why this change is needed?

I don't recall what exactly I was thinking when I wrote it.  It might
have been a leftover from some other attempt to use unique_ptr to ensure
there isn't a possibility of a leak.

I think it's fine to switch it to constructors if that is convenient for
you.

Tom

Re: [PATCH v2 5/9] Introduce "static constructor" for mi_parse

[Andrew Burgess]

Tom Tromey <tromey@adacore.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> Sorry to respond to such an old thread, but I ran into the
> Andrew> mi_parse::make code today.  From your commit message I don't see the
> Andrew> connection for why we need a static mi_parse::make function in order to
> Andrew> get rid of the set_args method.
>
> Andrew> For reference, I switched the code back to using constructors (but
> Andrew> didn't add back set_args), and I'm not seeing any regressions in
> Andrew> gdb.mi/* and gdb.python/*.
>
> Andrew> Could you expand on why this change is needed?
>
> I don't recall what exactly I was thinking when I wrote it.  It might
> have been a leftover from some other attempt to use unique_ptr to ensure
> there isn't a possibility of a leak.
>
> I think it's fine to switch it to constructors if that is convenient for
> you.

Thanks.  I'll CC you when I post the patch.  Just thought I'd ask first,
didn't want to look like I was just trying to revert your work :)

Thanks,
Andrew