[v2,3/5] Create MI commands using python.
Commit Message
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
* Makefile.in (SUBDIR_PYTHON_SRCS): Add py-micmd.c.
* mi/mi-cmds.c (insert_mi_cmd_entry): Remove static.
(mi_cmd_table): Remove static.
* mi/mi-cmds.h (insert_mi_cmd_entry): New declaration.
(mi_cmd_table): New declaration.
* python/py-micmd.c: New file
(parse_mi_result): New function.
(micmdpy_init): New function.
(gdbpy_initialize_micommands): New function.
(mi_command_py): New class.
* python/py-micmd.h: New file
(micmdpy_object): New struct.
(micmdpy_object): New typedef.
(mi_command_py): New class.
* python/python-internal.h
(gdbpy_initialize_micommands): New declaration.
* python/python.c
(_initialize_python): New call to gdbpy_initialize_micommands.
(finalize_python): Finalize Python MI commands.
---
gdb/ChangeLog | 23 +++
gdb/Makefile.in | 1 +
gdb/mi/mi-cmds.c | 7 +-
gdb/mi/mi-cmds.h | 9 ++
gdb/python/py-micmd.c | 300 +++++++++++++++++++++++++++++++++++
gdb/python/py-micmd.h | 51 ++++++
gdb/python/python-internal.h | 2 +
gdb/python/python.c | 13 +-
8 files changed, 401 insertions(+), 5 deletions(-)
create mode 100644 gdb/python/py-micmd.c
create mode 100644 gdb/python/py-micmd.h
Comments
On 2019-05-14 7:24 a.m., Jan Vrany wrote:
> 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
>
> * Makefile.in (SUBDIR_PYTHON_SRCS): Add py-micmd.c.
> * mi/mi-cmds.c (insert_mi_cmd_entry): Remove static.
> (mi_cmd_table): Remove static.
> * mi/mi-cmds.h (insert_mi_cmd_entry): New declaration.
> (mi_cmd_table): New declaration.
> * python/py-micmd.c: New file
> (parse_mi_result): New function.
> (micmdpy_init): New function.
> (gdbpy_initialize_micommands): New function.
> (mi_command_py): New class.
> * python/py-micmd.h: New file
> (micmdpy_object): New struct.
> (micmdpy_object): New typedef.
> (mi_command_py): New class.
> * python/python-internal.h
> (gdbpy_initialize_micommands): New declaration.
> * python/python.c
> (_initialize_python): New call to gdbpy_initialize_micommands.
> (finalize_python): Finalize Python MI commands.
> ---
> gdb/ChangeLog | 23 +++
> gdb/Makefile.in | 1 +
> gdb/mi/mi-cmds.c | 7 +-
> gdb/mi/mi-cmds.h | 9 ++
> gdb/python/py-micmd.c | 300 +++++++++++++++++++++++++++++++++++
> gdb/python/py-micmd.h | 51 ++++++
> gdb/python/python-internal.h | 2 +
> gdb/python/python.c | 13 +-
> 8 files changed, 401 insertions(+), 5 deletions(-)
> create mode 100644 gdb/python/py-micmd.c
> create mode 100644 gdb/python/py-micmd.h
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 03eb42555e..4cbe97002b 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,26 @@
> +2019-05-02 Didier Nadeau <didier.nadeau@gmail.com>
> + Jan Vrany <jan.vrany@fit.cvut.cz>
> +
> + * Makefile.in (SUBDIR_PYTHON_SRCS): Add py-micmd.c.
> + * mi/mi-cmds.c (insert_mi_cmd_entry): Remove static.
> + (mi_cmd_table): Remove static.
> + * mi/mi-cmds.h (insert_mi_cmd_entry): New declaration.
> + (mi_cmd_table): New declaration.
> + * python/py-micmd.c: New file
> + (parse_mi_result): New function.
> + (micmdpy_init): New function.
> + (gdbpy_initialize_micommands): New function.
> + (mi_command_py): New class.
> + * python/py-micmd.h: New file
> + (micmdpy_object): New struct.
> + (micmdpy_object): New typedef.
> + (mi_command_py): New class.
> + * python/python-internal.h
> + (gdbpy_initialize_micommands): New declaration.
> + * python/python.c
> + (_initialize_python): New call to gdbpy_initialize_micommands.
> + (finalize_python): Finalize Python MI commands.
> +
> 2019-05-02 Didier Nadeau <didier.nadeau@gmail.com>
> Jan Vrany <jan.vrany@fit.cvut.cz>
>
> 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 44f3813fdc..7e1d2c3f2d 100644
> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -28,12 +28,11 @@
>
> /* MI command table (built at run time). */
>
> -static std::map<std::string, mi_cmd_up> mi_cmd_table;
> +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);
> diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
> index fc432a16b9..a2757bae20 100644
> --- a/gdb/mi/mi-cmds.h
> +++ b/gdb/mi/mi-cmds.h
> @@ -22,6 +22,8 @@
> #ifndef MI_MI_CMDS_H
> #define MI_MI_CMDS_H
>
> +#include <map>
> +
> enum print_values {
> PRINT_NO_VALUES,
> PRINT_ALL_VALUES,
> @@ -190,9 +192,16 @@ typedef std::unique_ptr<mi_command> mi_cmd_up;
>
> extern mi_command *mi_cmd_lookup (const char *command);
>
> +extern std::map<std::string, mi_cmd_up> mi_cmd_table;
> +
> /* Debug flag */
> 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..c8d429bb4b
> --- /dev/null
> +++ b/gdb/python/py-micmd.c
> @@ -0,0 +1,300 @@
> +/* MI Command Set for GDB, the GNU debugger.
> +
> + Copyright (C) 2019 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/>. */
> +
> +/* gdb MI commands implemented in Python */
> +
> +#include "defs.h"
> +#include "python-internal.h"
> +#include "python/py-micmd.h"
> +#include "arch-utils.h"
> +#include "charset.h"
> +#include "language.h"
> +
> +#include <string>
> +
> +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 (PyString_Check (result))
> + {
> + goto generic;
Hmm is this goto really necessary? Can't you just remove this if, and execution will
go directly in the else? Or is a string considered a sequence by PySequence_Check?
In any case, there's gotta be a better way to organize this that doesn't require the goto.
> + }
> + else if (PyDict_Check (result))
> + {
> + PyObject *key, *value;
> + Py_ssize_t pos = 0;
> + ui_out_emit_tuple tuple_emitter (uiout, field_name);
> + while (PyDict_Next (result, &pos, &key, &value))
> + {
> + gdb::unique_xmalloc_ptr<char> key_string (gdbpy_obj_to_string (key));
When parsing a dict, I think it would be good to enforce that keys are strings, I
don't really see the use case to allow arbitrary objects there. In the MI, keys
will be strings, and I don't really see the use case of letting the user do
return {
MyObject(): "foo"
}
where the str(MyObject()) will be used as the key. By validating that keys are
strings, I think we can help the user catch errors earlier in development.
> + parse_mi_result (value, key_string.get ());
> + }
> + }
> + else if (PySequence_Check (result))
> + {
> + PyObject *item;
> + Py_ssize_t i = 0;
Declare these variables in the most specific scope possible (the for loop header for i,
in the for loop for item).
> + ui_out_emit_list list_emitter (uiout, field_name);
> + for (i = 0; i < PySequence_Size (result); ++i)
> + {
> + item = PySequence_GetItem (result, i);
PySequence_GetItem returns a new reference, which must be dropped.
Also, you could probably use PySequence_ITEM which will be a bit faster. It should be
safe, since we know result is a PySequence and we don't use negative indices.
> + parse_mi_result (item, NULL);
> + }
> + }
> + else if (PyIter_Check (result))
> + {
> + gdbpy_ref<> item;
> + ui_out_emit_list list_emitter (uiout, field_name);
> + while (item.reset (PyIter_Next (result)), item != nullptr)
> + {
> + parse_mi_result (item.get (), NULL);
> + }
Remove extra curly braces.
> + }
> + else
> + {
> + generic:
> + gdb::unique_xmalloc_ptr<char> string (gdbpy_obj_to_string (result));
> + uiout->field_string (field_name, string.get ());
> + }
> +}
> +
> +/* Object initializer; sets up gdb-side structures for MI command.
> +
> + Use: __init__(NAME).
> +
> + NAME is the name of the MI command to register. It must start with a dash
> + as a traditional MI commands do. */
"as traditional MI commands", drop the "a"
> +
> +static int
> +micmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
> +{
> + const char *name;
> +
> + if (!PyArg_ParseTuple (args, "s", &name))
> + return -1;
> +
> + /* Validate command name */
> + const int name_len = strlen (name);
> + if (name_len < 2)
This should probably name_len == 0, to match the error message.
> + {
> + PyErr_Format (PyExc_RuntimeError, _("MI command name is empty"));
> + return -1;
> + }
> + else if ((name[0] != '-') || !isalnum (name[1]))
> + {
> + PyErr_Format (PyExc_RuntimeError,
> + _("MI command name does not start with '-'"
> + " followed by a letter or digit"));
> + return -1;
> + }
Could all these error messages be just calls to error(), handled by the catch below?
We would then raise some gdb.error exceptions instead of RuntimeError, maybe it's desirable?
> + else
> + for (int i = 2; i < name_len; i++)
> + {
> + if (!isalnum (name[i]) && name[i] != '-')
> + {
> + PyErr_Format (PyExc_RuntimeError,
> + _("MI command name contains invalid character: %c"),
> + name[i]);
> + return -1;
> + }
> + }
> +
> + gdbpy_ref<> self_ref = gdbpy_ref<>::new_reference (self);
> + try
> + {
> + mi_command *micommand = new mi_command_py (name + 1, NULL, self_ref);
> +
> + bool result = insert_mi_cmd_entry (std::move (mi_cmd_up (micommand)));
Make micommand a mi_cmd_up directly:
mi_cmd_up micommand (new mi_command_py (...));
> +
> + if (!result)
> + {
> + PyErr_Format (
> + PyExc_RuntimeError,
> + _("Unable to insert command. The name might already be in use."));
> + return -1;
> + }
> + }
> + catch (const gdb_exception &except)
> + {
> + GDB_PY_SET_HANDLE_EXCEPTION (except);
> + }
> +
> + return 0;
> +}
> +
> +mi_command_py::mi_command_py (const char *name, int *suppress_notification,
> + gdbpy_ref<> 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);
> +
> + PyObject *obj = this->pyobj.get ();
> + int i;
> +
> + gdbpy_enter enter_py (get_current_arch (), current_language);
> +
> + if (!obj)
> + error (_("-%s: invalid invocation of Python micommand object."),
> + name ().c_str ());
Can this condition happen because of a mistake by the user, or it would only be because
of a logic error in GDB? If it's the latter, it should be a gdb_assert (obj != nullptr);
> +
> + if (!PyObject_HasAttr (obj, invoke_cst))
> + {
> + error (_("-%s: python command object missing 'invoke' method."),
> + name ().c_str ());
> + }
I would make that check (presence of invoke method) when the command is registered. It would
help users catch mistakes earlier. Here, it could just be a gdb_assert to confirm it exists.
> +
> + gdbpy_ref<> argobj (PyList_New (parse->argc));
> + if (argobj == nullptr)
> + {
> + gdbpy_print_stack ();
> + error (_("-%s: failed to create the python arguments list."),
> + name ().c_str ());
> + }
It would be nice to be consistent in how we write "Python" in our messages to the user.
I suggest "Python", with the capital letter, as it's how the language is named.
> +
> + for (i = 0; i < parse->argc; ++i)
Declare i in the for loop header.
> + {
> + /* Since PyList_SetItem steals the reference, we don't use
> + * gdbpy_ref<> to hold on arg string. */
> + PyObject *str = PyUnicode_Decode (parse->argv[i], strlen (parse->argv[i]),
> + host_charset (), NULL);
> + if (PyList_SetItem (argobj.get (), i, str) != 0)
Actually, I would suggest using gdbpy_ref<>, but then releasing it:
PyList_SetItem (..., str.release ());
It illustrates better the transfer of ownership, and there's no window where
the reference is unmanaged.
Just wondering, because I know that unicode handling is very different between
Python 2 and 3: did you test with both Python versions?
> + {
> + error (_("-%s: failed to create the python arguments list."),
> + name ().c_str ());
> + }
> + }
> +
> + gdb_assert (PyErr_Occurred () == NULL);
> + gdbpy_ref<> result (
> + PyObject_CallMethodObjArgs (obj, invoke_cst, argobj.get (), NULL));
> + if (PyErr_Occurred () != NULL)
> + {
> + gdbpy_err_fetch ex;
> + gdb::unique_xmalloc_ptr<char> ex_msg (ex.to_string ());
> +
> + if (ex_msg == NULL || *ex_msg == '\0')
> + error (_("-%s: failed to execute command"), name ().c_str ());
> + else
> + error (_("-%s: %s"), name ().c_str (), ex_msg.get ());
> + }
> + else if (result != nullptr)
> + {
> + if (Py_None != result)
> + parse_mi_result (result.get (), "result");
> + }
> + else
> + {
> + error (
> + _("-%s: command invoke() method returned NULL but no python exception "
> + "is set"),
> + name ().c_str ());
I am curious, did you actually hit this case, where no Python exception was
raised and invoke returned NULL? I thought the two were pretty coupled
(if return value is NULL, it means there's an exception raise and vice versa).
> + }
> +}
> +
> +void mi_command_py::finalize ()
> +{
> + this->pyobj.reset (nullptr);
> +}
> +
> +/* Initialize the MI command object. */
> +
> +int
> +gdbpy_initialize_micommands ()
> +{
> + 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..418de41a10
> --- /dev/null
> +++ b/gdb/python/py-micmd.h
> @@ -0,0 +1,51 @@
> +/* MI Command Set for GDB, the GNU debugger.
> +
> + Copyright (C) 2019 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/>. */
> +
> +#ifndef PY_MICMDS_H
> +#define PY_MICMDS_H
> +
> +#include "mi/mi-cmds.h"
> +#include "mi/mi-parse.h"
> +#include "python-internal.h"
> +#include "python/py-ref.h"
> +
> +struct micmdpy_object
> +{
> + PyObject_HEAD
> +};
> +
> +typedef struct micmdpy_object micmdpy_object;
> +
> +/* MI command implemented on top of a Python command. */
I find this description a bit misleading, what's a Python command?
The only thing "Python command" can refer to is the gdb.Command class, use
to implement CLI commands in Python, but these Python MI commands are not
related. I would suggest instead:
/* MI command implemented in Python. */
> +class mi_command_py : public mi_command
> +{
> +public:
> + mi_command_py (const char *name, int *suppress_notification,
> + gdbpy_ref<> object);
If the suppress_notification parameter is not used for Python commands, you can remove it.
Could you document the constructor? The OBJECT parameter in particular is a bit opaque.
> + void invoke (struct mi_parse *parse) override;
This should be private, like in the previous patch.
> +
> + /* This is called just before shutting down a Python interpreter
> + to release python object implementing the command */
> + void finalize ();
> +
> +private:
> + gdbpy_ref<> pyobj;
> +};
> +
> +#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..ee89c603a5 100644
> --- a/gdb/python/python.c
> +++ b/gdb/python/python.c
> @@ -36,6 +36,8 @@
> #include <ctype.h>
> #include "location.h"
> #include "ser-event.h"
> +#include "mi/mi-cmds.h"
> +#include "py-micmd.h"
>
> /* Declared constants and enum for python stack printing. */
> static const char python_excp_none[] = "none";
> @@ -1564,6 +1566,14 @@ finalize_python (void *ignore)
> python_gdbarch = target_gdbarch ();
> python_language = current_language;
>
> + for (auto const& name_and_cmd : mi_cmd_table)
Use:
for (const auto &name_and_cmd : mi_cmd_table)
> + {
> + mi_command *cmd = name_and_cmd.second.get ();
> + mi_command_py *cmd_py = dynamic_cast<mi_command_py*> (cmd);
Don't align variables like this, use regular formatting.
> + if (cmd_py != nullptr)
> + cmd_py->finalize ();
> + }
> +
> Py_Finalize ();
>
> restore_active_ext_lang (previous_active);
> @@ -1698,7 +1708,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) \
>
Simon
Hi,
thanks a lot! I agree with most of comments and fixed
them already in upcoming v3 patch.
For the rest, sae below.
> > > + {
> > > + /* Since PyList_SetItem steals the reference, we don't use
> > > + * gdbpy_ref<> to hold on arg string. */
> > > + PyObject *str = PyUnicode_Decode (parse->argv[i], strlen (parse->argv[i]),
> > > + host_charset (), NULL);
> > > + if (PyList_SetItem (argobj.get (), i, str) != 0)
> >
> > Just wondering, because I know that unicode handling is very different between
> > Python 2 and 3: did you test with both Python versions?
> >
No. I will do that before sending v3.
> > > + gdbpy_ref<> result (
> > > + PyObject_CallMethodObjArgs (obj, invoke_cst, argobj.get (), NULL));
> > > + if (PyErr_Occurred () != NULL)
> > > + {
> > > + gdbpy_err_fetch ex;
> > > + gdb::unique_xmalloc_ptr<char> ex_msg (ex.to_string ());
> > > +
> > > + if (ex_msg == NULL || *ex_msg == '\0')
> > > + error (_("-%s: failed to execute command"), name ().c_str ());
> > > + else
> > > + error (_("-%s: %s"), name ().c_str (), ex_msg.get ());
> > > + }
> > > + else if (result != nullptr)
> > > + {
> > > + if (Py_None != result)
> > > + parse_mi_result (result.get (), "result");
> > > + }
> > > + else
> > > + {
> > > + error (
> > > + _("-%s: command invoke() method returned NULL but no python exception "
> > > + "is set"),
> > > + name ().c_str ());
> >
> > I am curious, did you actually hit this case, where no Python exception was
> > raised and invoke returned NULL? I thought the two were pretty coupled
> > (if return value is NULL, it means there's an exception raise and vice versa).
> >
Yes, it looks like. I guess I took this code from Didier. I removed the else
branch in v3.
> > > + void invoke (struct mi_parse *parse) override;
> >
> > This should be private, like in the previous patch.
> >
Ah, this is "bigger" problem, we should override
do_invoke() which is declared protected in superclass. Again,fixed in v3.
Thanks a lot!
Jan
@@ -1,3 +1,26 @@
+2019-05-02 Didier Nadeau <didier.nadeau@gmail.com>
+ Jan Vrany <jan.vrany@fit.cvut.cz>
+
+ * Makefile.in (SUBDIR_PYTHON_SRCS): Add py-micmd.c.
+ * mi/mi-cmds.c (insert_mi_cmd_entry): Remove static.
+ (mi_cmd_table): Remove static.
+ * mi/mi-cmds.h (insert_mi_cmd_entry): New declaration.
+ (mi_cmd_table): New declaration.
+ * python/py-micmd.c: New file
+ (parse_mi_result): New function.
+ (micmdpy_init): New function.
+ (gdbpy_initialize_micommands): New function.
+ (mi_command_py): New class.
+ * python/py-micmd.h: New file
+ (micmdpy_object): New struct.
+ (micmdpy_object): New typedef.
+ (mi_command_py): New class.
+ * python/python-internal.h
+ (gdbpy_initialize_micommands): New declaration.
+ * python/python.c
+ (_initialize_python): New call to gdbpy_initialize_micommands.
+ (finalize_python): Finalize Python MI commands.
+
2019-05-02 Didier Nadeau <didier.nadeau@gmail.com>
Jan Vrany <jan.vrany@fit.cvut.cz>
@@ -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 \
@@ -28,12 +28,11 @@
/* MI command table (built at run time). */
-static std::map<std::string, mi_cmd_up> mi_cmd_table;
+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);
@@ -22,6 +22,8 @@
#ifndef MI_MI_CMDS_H
#define MI_MI_CMDS_H
+#include <map>
+
enum print_values {
PRINT_NO_VALUES,
PRINT_ALL_VALUES,
@@ -190,9 +192,16 @@ typedef std::unique_ptr<mi_command> mi_cmd_up;
extern mi_command *mi_cmd_lookup (const char *command);
+extern std::map<std::string, mi_cmd_up> mi_cmd_table;
+
/* Debug flag */
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 */
new file mode 100644
@@ -0,0 +1,300 @@
+/* MI Command Set for GDB, the GNU debugger.
+
+ Copyright (C) 2019 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/>. */
+
+/* gdb MI commands implemented in Python */
+
+#include "defs.h"
+#include "python-internal.h"
+#include "python/py-micmd.h"
+#include "arch-utils.h"
+#include "charset.h"
+#include "language.h"
+
+#include <string>
+
+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 (PyString_Check (result))
+ {
+ goto generic;
+ }
+ else if (PyDict_Check (result))
+ {
+ PyObject *key, *value;
+ Py_ssize_t pos = 0;
+ ui_out_emit_tuple tuple_emitter (uiout, field_name);
+ while (PyDict_Next (result, &pos, &key, &value))
+ {
+ gdb::unique_xmalloc_ptr<char> key_string (gdbpy_obj_to_string (key));
+ parse_mi_result (value, key_string.get ());
+ }
+ }
+ else if (PySequence_Check (result))
+ {
+ PyObject *item;
+ Py_ssize_t i = 0;
+ ui_out_emit_list list_emitter (uiout, field_name);
+ for (i = 0; i < PySequence_Size (result); ++i)
+ {
+ item = PySequence_GetItem (result, i);
+ parse_mi_result (item, NULL);
+ }
+ }
+ else if (PyIter_Check (result))
+ {
+ gdbpy_ref<> item;
+ ui_out_emit_list list_emitter (uiout, field_name);
+ while (item.reset (PyIter_Next (result)), item != nullptr)
+ {
+ parse_mi_result (item.get (), NULL);
+ }
+ }
+ else
+ {
+ generic:
+ gdb::unique_xmalloc_ptr<char> string (gdbpy_obj_to_string (result));
+ uiout->field_string (field_name, string.get ());
+ }
+}
+
+/* Object initializer; sets up gdb-side structures for MI command.
+
+ Use: __init__(NAME).
+
+ NAME is the name of the MI command to register. It must start with a dash
+ as a traditional MI commands do. */
+
+static int
+micmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
+{
+ const char *name;
+
+ if (!PyArg_ParseTuple (args, "s", &name))
+ return -1;
+
+ /* Validate command name */
+ const int name_len = strlen (name);
+ if (name_len < 2)
+ {
+ PyErr_Format (PyExc_RuntimeError, _("MI command name is empty"));
+ return -1;
+ }
+ else if ((name[0] != '-') || !isalnum (name[1]))
+ {
+ PyErr_Format (PyExc_RuntimeError,
+ _("MI command name does not start with '-'"
+ " followed by a letter or digit"));
+ return -1;
+ }
+ else
+ for (int i = 2; i < name_len; i++)
+ {
+ if (!isalnum (name[i]) && name[i] != '-')
+ {
+ PyErr_Format (PyExc_RuntimeError,
+ _("MI command name contains invalid character: %c"),
+ name[i]);
+ return -1;
+ }
+ }
+
+ gdbpy_ref<> self_ref = gdbpy_ref<>::new_reference (self);
+ try
+ {
+ mi_command *micommand = new mi_command_py (name + 1, NULL, self_ref);
+
+ bool result = insert_mi_cmd_entry (std::move (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)
+ {
+ GDB_PY_SET_HANDLE_EXCEPTION (except);
+ }
+
+ return 0;
+}
+
+mi_command_py::mi_command_py (const char *name, int *suppress_notification,
+ gdbpy_ref<> 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);
+
+ PyObject *obj = this->pyobj.get ();
+ int i;
+
+ gdbpy_enter enter_py (get_current_arch (), current_language);
+
+ if (!obj)
+ error (_("-%s: invalid invocation of Python micommand object."),
+ name ().c_str ());
+
+ if (!PyObject_HasAttr (obj, invoke_cst))
+ {
+ error (_("-%s: python command object missing 'invoke' method."),
+ name ().c_str ());
+ }
+
+ gdbpy_ref<> argobj (PyList_New (parse->argc));
+ if (argobj == nullptr)
+ {
+ gdbpy_print_stack ();
+ error (_("-%s: failed to create the python arguments list."),
+ name ().c_str ());
+ }
+
+ for (i = 0; i < parse->argc; ++i)
+ {
+ /* Since PyList_SetItem steals the reference, we don't use
+ * gdbpy_ref<> to hold on arg string. */
+ PyObject *str = PyUnicode_Decode (parse->argv[i], strlen (parse->argv[i]),
+ host_charset (), NULL);
+ if (PyList_SetItem (argobj.get (), i, str) != 0)
+ {
+ error (_("-%s: failed to create the python arguments list."),
+ name ().c_str ());
+ }
+ }
+
+ gdb_assert (PyErr_Occurred () == NULL);
+ gdbpy_ref<> result (
+ PyObject_CallMethodObjArgs (obj, invoke_cst, argobj.get (), NULL));
+ if (PyErr_Occurred () != NULL)
+ {
+ gdbpy_err_fetch ex;
+ gdb::unique_xmalloc_ptr<char> ex_msg (ex.to_string ());
+
+ if (ex_msg == NULL || *ex_msg == '\0')
+ error (_("-%s: failed to execute command"), name ().c_str ());
+ else
+ error (_("-%s: %s"), name ().c_str (), ex_msg.get ());
+ }
+ else if (result != nullptr)
+ {
+ if (Py_None != result)
+ parse_mi_result (result.get (), "result");
+ }
+ else
+ {
+ error (
+ _("-%s: command invoke() method returned NULL but no python exception "
+ "is set"),
+ name ().c_str ());
+ }
+}
+
+void mi_command_py::finalize ()
+{
+ this->pyobj.reset (nullptr);
+}
+
+/* Initialize the MI command object. */
+
+int
+gdbpy_initialize_micommands ()
+{
+ 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 */
+};
new file mode 100644
@@ -0,0 +1,51 @@
+/* MI Command Set for GDB, the GNU debugger.
+
+ Copyright (C) 2019 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/>. */
+
+#ifndef PY_MICMDS_H
+#define PY_MICMDS_H
+
+#include "mi/mi-cmds.h"
+#include "mi/mi-parse.h"
+#include "python-internal.h"
+#include "python/py-ref.h"
+
+struct micmdpy_object
+{
+ PyObject_HEAD
+};
+
+typedef struct micmdpy_object micmdpy_object;
+
+/* 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,
+ gdbpy_ref<> object);
+ void invoke (struct mi_parse *parse) override;
+
+ /* This is called just before shutting down a Python interpreter
+ to release python object implementing the command */
+ void finalize ();
+
+private:
+ gdbpy_ref<> pyobj;
+};
+
+#endif
@@ -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. */
@@ -36,6 +36,8 @@
#include <ctype.h>
#include "location.h"
#include "ser-event.h"
+#include "mi/mi-cmds.h"
+#include "py-micmd.h"
/* Declared constants and enum for python stack printing. */
static const char python_excp_none[] = "none";
@@ -1564,6 +1566,14 @@ finalize_python (void *ignore)
python_gdbarch = target_gdbarch ();
python_language = current_language;
+ for (auto const& name_and_cmd : mi_cmd_table)
+ {
+ mi_command *cmd = name_and_cmd.second.get ();
+ mi_command_py *cmd_py = dynamic_cast<mi_command_py*> (cmd);
+ if (cmd_py != nullptr)
+ cmd_py->finalize ();
+ }
+
Py_Finalize ();
restore_active_ext_lang (previous_active);
@@ -1698,7 +1708,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) \