[RFC,3/8] Create MI commands using python.

Message ID 20190418152337.6376-4-jan.vrany@fit.cvut.cz
State New
Headers show
Series
  • Create MI commands using python
Related show

Commit Message

Jan Vrany April 18, 2019, 3:23 p.m.
From: Didier Nadeau <didier.nadeau@gmail.com>


This commit allows an user to create custom MI commands using Python
similarly to what is possible for Python CLI commands.

A new subclass of mi_command is defined for Python MI commands,
mi_command_py. A new file, py-micmd.c contains the logic for Python
MI commands.

gdb/ChangeLog
2017-02-10  Didier Nadeau  <didier.nadeau@gmail.com>

    * Makefile.in (SUBDIR_PYTHON_SRCS): Add py-micmd.c.
    * mi/mi-cmds.c (insert_mi_cmd_entry): Remove static.
    (mi_command_py::mi_command_py): New function.
    (mi_command_py::invoke): New function.
    * mi/mi-cmds.h (mi_command_py): New class.
    (insert_mi_cmd_entry): New declaration.
    * python/py-micmd.c: New file
    (micmdpy_object): New struct.
    (micmdpy_object): New typedef.
    (parse_mi_result): New function.
    (py_mi_invoke): New function.
    (micmdpy_parse_command_name): New function.
    (micmdpy_init): New function.
    (gdbpy_initialize_micommands): New function.
    * python/py-micmd.h: New file
    (py_mi_invoke): New function.
    * python/python-internal.h
    (gdbpy_initialize_micommands): New declaration.
    * python/python.c
    (_initialize_python): New call to gdbpy_initialize_micommands.
---
 gdb/Makefile.in              |   1 +
 gdb/mi/mi-cmds.c             |  27 +++-
 gdb/mi/mi-cmds.h             |  19 +++
 gdb/python/py-micmd.c        | 290 +++++++++++++++++++++++++++++++++++
 gdb/python/py-micmd.h        |   6 +
 gdb/python/python-internal.h |   2 +
 gdb/python/python.c          |   3 +-
 7 files changed, 344 insertions(+), 4 deletions(-)
 create mode 100644 gdb/python/py-micmd.c
 create mode 100644 gdb/python/py-micmd.h

-- 
2.20.1

Comments

Tom Tromey April 25, 2019, 7:42 p.m. | #1
>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:


Lots of nits on this one, mostly I think because it was converted from
C?  Anything, nothing very major, just lots of little things.

Jan> +mi_command_py::mi_command_py (const char *name, int *suppress_notification,
Jan> +			      void *object)
Jan> +  : mi_command (name, suppress_notification),
Jan> +    pyobj (object)
Jan> +{}

I think it would be better to put this sort of thing somewhere in the
Python code, so that it can use gdbpy_ref<> rather than "void *".

Jan> +  py_mi_invoke (this->pyobj, parse->argv, parse->argc);

This sort of indirection wouldn't be needed then either.

Jan> +class mi_command_py : public mi_command
Jan> +{
Jan> +  public:
Jan> +    mi_command_py (const char *name, int *suppress_notification,
Jan> +		   void *object);
Jan> +    void invoke (struct mi_parse *parse) override;
Jan> +
Jan> +  private:
Jan> +    void *pyobj;
Jan> +
Jan> +};

Extra blank line before the "}".

Jan> diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
Jan> new file mode 100644
Jan> index 0000000000..ee612e2bc5
Jan> --- /dev/null
Jan> +++ b/gdb/python/py-micmd.c
Jan> @@ -0,0 +1,290 @@
Jan> +/* gdb MI commands implemented in Python  */
Jan> +
Jan> +#include <string.h>

Files need a copyright header.

Jan> +#include "defs.h"

defs.h must come first.  I wonder if string.h is needed at all.

Jan> +/* If the command invoked returns a list, this function parses it and create an
Jan> +   appropriate MI out output.
Jan> +
Jan> +   The returned values must be Python string, and can be contained within Python
Jan> +   lists and dictionaries. It is possible to have a multiple levels of lists
Jan> +   and/or dictionaries.  */
Jan> +
Jan> +static void
Jan> +parse_mi_result (PyObject *result, const char *field_name)
Jan> +{
Jan> +  struct ui_out *uiout = current_uiout;
Jan> +
Jan> +  if(!field_name)

gdb+gnu style would be:  "if (field_name == nullptr)"

Jan> +  if (PyString_Check(result))

Need a space before open parens.
There are a few instances of this.

Jan> +    {
Jan> +      const char *string = gdbpy_obj_to_string (result).release ();
Jan> +      uiout->field_string (field_name, string);
Jan> +      xfree ( (void *)string);

Better to take advantage of the unique pointer, like:

   gdb::unique_xmalloc_ptr<char> string = gdbpy_obj_to_string (result);
   uiout->field_string (field_name, string.get ());

However, this code is ignoring Python errors.  Probably this function
should use one of the standard conventions and return -1 on error.

Jan> +	  //struct cleanup *cleanup_item = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);

There are a couple of these; remove them.

Jan> +	  item = PyList_GetItem (result, i);

This can fail, so it needs a check.

Jan> +  else if ( PyDict_Check (result))

There's an extra space after paren.

Jan> +	  char *key_string = gdbpy_obj_to_string (key).release ();

Error checking.

Jan> +	  xfree ( (void *) key_string);

Again, no need to do this explicitly.

Jan> +  PyObject *argobj, *result,  **strings;

Extra space.

Jan> +  if (! obj)
Jan> +    error(_("Invalid invocation of Python micommand object."));

This could probably be improved somehow, for instance mentioning the MI
command name.  I think that's sort of standard in MI commands, so this
should follow it in all the errors, I think.

Jan> +  strings = (PyObject **) malloc (sizeof(PyObject *) * argc);

Not sure this is needed, see below.  But if it is it should use XNEWVEC
or xmalloc.

Jan> +  for (i = 0; i < argc; ++i)
Jan> +    {
Jan> +      strings[i] = PyUnicode_Decode (argv[i], strlen(argv[i]), host_charset (), NULL);
Jan> +      if (PyList_SetItem (argobj, i, strings[i]) != 0)

PyList_SetItem steals a reference, so I think the ref-counting in this
function is incorrect.  But, since it steals a reference, I think
"strings" isn't needed.

Jan> +	{
Jan> +	  error (_("Failed to create the python arguments list."));

This will leak references.  However if you use gdbpy_ref<>, you should
be ok.

Jan> +  /* Skip preceding whitespaces. */
Jan> +  /* Find first character of the final word.  */

Not sure if the first comment documents something that should be there
and is not, or if it should just be removed.

Probably the latter, I think it's fine to require command names to be
valid.  I'd probably skip stripping the trailing whitespace as well.

Jan> +  cmd_name = micmdpy_parse_command_name (name);
Jan> +  if (! cmd_name)
Jan> +    return -1;

Like this could just give an error if the NAME isn't a valid MI command
name, and not bother with trying to fix it up, or copy it.

Jan> +    PyErr_Format (except.reason == RETURN_QUIT
Jan> +		    ? PyExc_KeyboardInterrupt : PyExc_RuntimeError,
Jan> +		    "%s", except.message);

I think GDB_PY_SET_HANDLE_EXCEPTION is better for this.

Jan> +/* Initialize the MI command object.  */
Jan> +
Jan> +int
Jan> +gdbpy_initialize_micommands(void)

Don't need (void) any more.

Jan> +++ b/gdb/python/py-micmd.h
Jan> @@ -0,0 +1,6 @@
Jan> +#ifndef PY_MICMDS_H
Jan> +#define PY_MICMDS_H
Jan> +
Jan> +void py_mi_invoke (void *py_obj, char **argv, int argc);
Jan> +

I'd just stick this in python-internal.h and not have a new header.

thanks,
Tom

Patch

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 0f49578360..28866962bc 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -382,6 +382,7 @@  SUBDIR_PYTHON_SRCS = \
 	python/py-instruction.c \
 	python/py-lazy-string.c \
 	python/py-linetable.c \
+	python/py-micmd.c \
 	python/py-newobjfileevent.c \
 	python/py-objfile.c \
 	python/py-param.c \
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 566d6c1885..c4332e59cf 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -23,6 +23,7 @@ 
 #include "mi-cmds.h"
 #include "mi-main.h"
 #include "mi-parse.h"
+#include "python/py-micmd.h"
 #include <map>
 #include <string>
 #include <memory>
@@ -31,10 +32,9 @@ 
 
 static std::map<std::string, mi_cmd_up> mi_cmd_table;
 
-/* Insert a new mi-command into the command table.  Return true if
-   insertion was successful.  */
+/* See mi-cmds.h.  */
 
-static bool
+bool
 insert_mi_cmd_entry (mi_cmd_up command)
 {
   gdb_assert (command != NULL);
@@ -131,6 +131,27 @@  mi_command_cli::invoke (struct mi_parse *parse)
 			  parse->args);
 }
 
+mi_command_py::mi_command_py (const char *name, int *suppress_notification,
+			      void *object)
+  : mi_command (name, suppress_notification),
+    pyobj (object)
+{}
+
+void
+mi_command_py::invoke (struct mi_parse *parse)
+{
+  std::unique_ptr<scoped_restore_tmpl <int>> restore
+      = do_suppress_notification ();
+
+  mi_parse_argv (parse->args, parse);
+
+  if (parse->argv == NULL)
+      error (_("Problem parsing arguments: %s %s"), parse->command, parse->args);
+
+  py_mi_invoke (this->pyobj, parse->argv, parse->argc);
+
+}
+
 /* Initialize the available MI commands.  */
 
 static void
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index eae023c5da..a89e265de7 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -179,6 +179,20 @@  class mi_command_cli : public mi_command
     int m_args_p;
 };
 
+/* MI command implemented on top of a Python command.  */
+
+class mi_command_py : public mi_command
+{
+  public:
+    mi_command_py (const char *name, int *suppress_notification,
+		   void *object);
+    void invoke (struct mi_parse *parse) override;
+
+  private:
+    void *pyobj;
+
+};
+
 typedef std::unique_ptr<mi_command> mi_cmd_up;
 
 /* Lookup a command in the MI command table.  */
@@ -190,4 +204,9 @@  extern int mi_debug_p;
 
 extern void mi_execute_command (const char *cmd, int from_tty);
 
+/* Insert a new mi-command into the command table.  Return true if
+   insertion was successful.  */
+
+extern bool insert_mi_cmd_entry (mi_cmd_up command);
+
 #endif /* MI_MI_CMDS_H */
diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
new file mode 100644
index 0000000000..ee612e2bc5
--- /dev/null
+++ b/gdb/python/py-micmd.c
@@ -0,0 +1,290 @@ 
+/* gdb MI commands implemented in Python  */
+
+#include <string.h>
+
+#include "defs.h"
+#include "arch-utils.h"
+#include "value.h"
+#include "python-internal.h"
+#include "charset.h"
+#include "gdbcmd.h"
+#include "cli/cli-decode.h"
+#include "completer.h"
+#include "language.h"
+#include "mi/mi-cmds.h"
+
+struct micmdpy_object
+{
+  PyObject_HEAD
+};
+
+typedef struct micmdpy_object micmdpy_object;
+
+static PyObject *invoke_cst;
+
+extern PyTypeObject micmdpy_object_type
+  CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("micmdpy_object");
+
+/* If the command invoked returns a list, this function parses it and create an
+   appropriate MI out output.
+
+   The returned values must be Python string, and can be contained within Python
+   lists and dictionaries. It is possible to have a multiple levels of lists
+   and/or dictionaries.  */
+
+static void
+parse_mi_result (PyObject *result, const char *field_name)
+{
+  struct ui_out *uiout = current_uiout;
+
+  if(!field_name)
+    field_name = "default";
+
+  if (PyString_Check(result))
+    {
+      const char *string = gdbpy_obj_to_string (result).release ();
+      uiout->field_string (field_name, string);
+      xfree ( (void *)string);
+    }
+  else if (PyList_Check (result))
+    {
+      PyObject *item;
+      Py_ssize_t i = 0;      
+      ui_out_emit_list list_emitter (uiout, field_name);
+      for(i = 0; i < PyList_GET_SIZE (result); ++i)
+	{
+	  //struct cleanup *cleanup_item = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+          ui_out_emit_tuple tuple_emitter (uiout, NULL);
+	  item = PyList_GetItem (result, i);
+	  parse_mi_result (item, NULL);
+	  //do_cleanups (cleanup_item);
+	}      
+    }
+  else if ( PyDict_Check (result))
+    {
+      PyObject *key, *value;
+      Py_ssize_t pos = 0;
+      while ( PyDict_Next (result, &pos, &key, &value) )
+	{
+	  char *key_string = gdbpy_obj_to_string (key).release ();
+	  parse_mi_result (value, key_string);
+	  xfree ( (void *) key_string);
+	}
+    }
+}
+
+/* Called from mi_command_py's invoke to invoke the command.  */
+
+void
+py_mi_invoke (void *py_obj, char **argv, int argc)
+{
+  micmdpy_object *obj = (micmdpy_object *) py_obj;
+  PyObject *argobj, *result,  **strings;
+  int i;
+
+  gdbpy_enter enter_py (get_current_arch (), current_language);
+
+  if (! obj)
+    error(_("Invalid invocation of Python micommand object."));
+
+  if (! PyObject_HasAttr ((PyObject *) obj, invoke_cst))
+    {
+      error (_("Python command object missing 'invoke' method."));
+    }
+
+  strings = (PyObject **) malloc (sizeof(PyObject *) * argc);
+  argobj = PyList_New (argc);
+  if ( !argobj)
+    {
+      gdbpy_print_stack ();
+      error (_("Failed to create the python arguments list."));
+    }
+
+  for (i = 0; i < argc; ++i)
+    {
+      strings[i] = PyUnicode_Decode (argv[i], strlen(argv[i]), host_charset (), NULL);
+      if (PyList_SetItem (argobj, i, strings[i]) != 0)
+	{
+	  error (_("Failed to create the python arguments list."));
+	}
+    }
+
+  result = PyObject_CallMethodObjArgs ((PyObject *) obj, invoke_cst, argobj,
+				       NULL);
+
+  if (result)
+    {
+      parse_mi_result (result, NULL);
+      Py_DECREF (result);
+    }
+
+  Py_DECREF (argobj);
+  for (i = 0; i < argc; ++i)
+    {
+      Py_DECREF (strings[i]);
+    }
+  free (strings);
+}
+
+/* Parse the name of the MI command to register.
+
+   This function returns the xmalloc()d name of the new command. On error
+   sets the Python error and returns NULL.  */
+
+static char *
+micmdpy_parse_command_name (const char *name)
+{
+  int len = strlen (name);
+  int i, lastchar;
+  char *result;
+
+  /* Skip trailing whitespaces. */
+  for (i = len - 1; i >= 0 && (name[i] == ' ' || name[i] == '\t'); --i)
+    ;
+  if (i < 0)
+    {
+      PyErr_SetString (PyExc_RuntimeError, _("No command name found."));
+      return NULL;
+    }
+  lastchar = i;
+
+  /* Skip preceding whitespaces. */
+  /* Find first character of the final word.  */
+   for (; i > 0 && (isalnum (name[i - 1])
+ 		   || name[i - 1] == '-'
+ 		   || name[i - 1] == '_');
+        --i)
+     ;
+   /* Skip the first dash to have to command name only.
+    * i.e. -thread-info -> thread-info
+    */
+   if(name[i] == '-' && i < len - 2)
+     i++;
+
+   if( i == lastchar)
+    {
+      PyErr_SetString (PyExc_RuntimeError, _("No command name found."));
+      return NULL;
+    }
+
+  result = (char *) xmalloc (lastchar - i + 2);
+  memcpy(result, &name[i], lastchar - i + 1);
+  result[lastchar - i + 1] = '\0';
+
+  return result;
+}
+
+/* Object initializer; sets up gdb-side structures for MI command.
+
+   Use: __init__(NAME).
+
+   NAME is the name of the MI command to register. It should starts with a dash
+   as a traditional MI command does.  */
+
+static int
+micmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
+{
+  const char *name;
+  char *cmd_name;
+
+  if(! PyArg_ParseTuple (args, "s", &name))
+    return -1;
+
+  cmd_name = micmdpy_parse_command_name (name);
+  if (! cmd_name)
+    return -1;
+
+  Py_INCREF (self);
+
+  try
+  {
+    mi_command *micommand = new mi_command_py (cmd_name, NULL, (void *) self);
+
+    bool result = insert_mi_cmd_entry (mi_cmd_up (micommand));
+
+    if (! result)
+      {
+	PyErr_Format (PyExc_RuntimeError,
+		      _("Unable to insert command. The name might already be in use."));
+	return -1;
+      }
+  }
+  catch (const gdb_exception &except)
+  {
+    xfree (cmd_name);
+    Py_DECREF (self);
+    PyErr_Format (except.reason == RETURN_QUIT
+		    ? PyExc_KeyboardInterrupt : PyExc_RuntimeError,
+		    "%s", except.message);
+    return -1;
+  }
+
+  xfree (cmd_name);
+  return 0;
+}
+
+/* Initialize the MI command object.  */
+
+int
+gdbpy_initialize_micommands(void)
+{
+  micmdpy_object_type.tp_new = PyType_GenericNew;
+  if (PyType_Ready (&micmdpy_object_type) < 0)
+    return -1;
+
+  if (gdb_pymodule_addobject (gdb_module, "MICommand",
+  			      (PyObject *) &micmdpy_object_type) < 0)
+      return -1;
+
+  invoke_cst = PyString_FromString("invoke");
+  if (invoke_cst == NULL)
+    return -1;
+  
+  return 0;
+}
+
+static PyMethodDef micmdpy_object_methods[] =
+    {
+	{ 0 }
+    };
+
+PyTypeObject micmdpy_object_type =
+{
+  PyVarObject_HEAD_INIT (NULL, 0)
+  "gdb.MICommand",		  /*tp_name*/
+  sizeof (micmdpy_object),	  /*tp_basicsize*/
+  0,				  /*tp_itemsize*/
+  0,				  /*tp_dealloc*/
+  0,				  /*tp_print*/
+  0,				  /*tp_getattr*/
+  0,				  /*tp_setattr*/
+  0,				  /*tp_compare*/
+  0,				  /*tp_repr*/
+  0,				  /*tp_as_number*/
+  0,				  /*tp_as_sequence*/
+  0,				  /*tp_as_mapping*/
+  0,				  /*tp_hash */
+  0,				  /*tp_call*/
+  0,				  /*tp_str*/
+  0,				  /*tp_getattro*/
+  0,				  /*tp_setattro*/
+  0,				  /*tp_as_buffer*/
+  Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /*tp_flags*/
+  "GDB mi-command object",		  /* tp_doc */
+  0,				  /* tp_traverse */
+  0,				  /* tp_clear */
+  0,				  /* tp_richcompare */
+  0,				  /* tp_weaklistoffset */
+  0,				  /* tp_iter */
+  0,				  /* tp_iternext */
+  micmdpy_object_methods,	  /* tp_methods */
+  0,				  /* tp_members */
+  0,				  /* tp_getset */
+  0,				  /* tp_base */
+  0,				  /* tp_dict */
+  0,				  /* tp_descr_get */
+  0,				  /* tp_descr_set */
+  0,				  /* tp_dictoffset */
+  micmdpy_init,			  /* tp_init */
+  0,				  /* tp_alloc */
+};
diff --git a/gdb/python/py-micmd.h b/gdb/python/py-micmd.h
new file mode 100644
index 0000000000..98f5305003
--- /dev/null
+++ b/gdb/python/py-micmd.h
@@ -0,0 +1,6 @@ 
+#ifndef PY_MICMDS_H
+#define PY_MICMDS_H
+
+void py_mi_invoke (void *py_obj, char **argv, int argc);
+
+#endif
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 449926ca87..26606b4b36 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -539,6 +539,8 @@  int gdbpy_initialize_xmethods (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 int gdbpy_initialize_unwind (void)
   CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
+int gdbpy_initialize_micommands (void)
+  CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION;
 
 /* A wrapper for PyErr_Fetch that handles reference counting for the
    caller.  */
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 4dad8ec10d..5184b99e49 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1698,7 +1698,8 @@  do_start_initialization ()
       || gdbpy_initialize_event () < 0
       || gdbpy_initialize_arch () < 0
       || gdbpy_initialize_xmethods () < 0
-      || gdbpy_initialize_unwind () < 0)
+      || gdbpy_initialize_unwind () < 0
+      || gdbpy_initialize_micommands () < 0)
     return false;
 
 #define GDB_PY_DEFINE_EVENT_TYPE(name, py_name, doc, base)	\