[v2,8/9] Implement gdb.execute_mi

Message ID 20230404-dap-loaded-sources-v2-8-93f229095e03@adacore.com
State New
Headers
Series Implement the DAP "loadedSources" request |

Commit Message

Tom Tromey May 18, 2023, 8:18 p.m. UTC
  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(+)
  

Comments

Eli Zaretskii May 19, 2023, 5:37 a.m. UTC | #1
> 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>
  
Simon Marchi May 29, 2023, 3:54 p.m. UTC | #2
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
  
Tom Tromey June 2, 2023, 7:19 p.m. UTC | #3
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
  
Tom Tromey June 8, 2023, 8:16 p.m. UTC | #4
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
  

Patch

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")