diff options
| -rw-r--r-- | gdb/mi/mi-cmds.c | 14 | ||||
| -rw-r--r-- | gdb/mi/mi-cmds.h | 7 | ||||
| -rw-r--r-- | gdb/python/lib/gdb/__init__.py | 4 | ||||
| -rw-r--r-- | gdb/python/py-micmd.c | 232 | ||||
| -rw-r--r-- | gdb/python/python-internal.h | 1 | ||||
| -rw-r--r-- | gdb/python/python.c | 2 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.python/py-mi-cmd.exp | 53 |
7 files changed, 105 insertions, 208 deletions
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c index 60fec0a..0571469 100644 --- a/gdb/mi/mi-cmds.c +++ b/gdb/mi/mi-cmds.c @@ -128,6 +128,20 @@ remove_mi_cmd_entry (const std::string &name) return true; } +/* See mi-cmds.h. */ + +void +remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback) +{ + for (auto it = mi_cmd_table.cbegin (); it != mi_cmd_table.cend (); ) + { + if (callback (it->second.get ())) + it = mi_cmd_table.erase (it); + else + ++it; + } +} + /* Create and register a new MI command with an MI specific implementation. NAME must name an MI command that does not already exist, otherwise an assertion will trigger. */ diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h index 05b702f..9ffb11b 100644 --- a/gdb/mi/mi-cmds.h +++ b/gdb/mi/mi-cmds.h @@ -22,6 +22,7 @@ #ifndef MI_MI_CMDS_H #define MI_MI_CMDS_H +#include "gdbsupport/function-view.h" #include "gdbsupport/gdb_optional.h" #include "mi/mi-main.h" @@ -218,5 +219,11 @@ extern bool insert_mi_cmd_entry (mi_command_up command); extern bool remove_mi_cmd_entry (const std::string &name); +/* Call CALLBACK for each registered MI command. Remove commands for which + CALLBACK returns true. */ + +using remove_mi_cmd_entries_ftype + = gdb::function_view<bool (mi_command *)>; +extern void remove_mi_cmd_entries (remove_mi_cmd_entries_ftype callback); #endif /* MI_MI_CMDS_H */ diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py index 3294583..5f63bce 100644 --- a/gdb/python/lib/gdb/__init__.py +++ b/gdb/python/lib/gdb/__init__.py @@ -82,10 +82,6 @@ frame_filters = {} # Initial frame unwinders. frame_unwinders = [] -# Dictionary containing all user created MI commands, the key is the -# command name, and the value is the gdb.MICommand object. -_mi_commands = {} - def _execute_unwinders(pending_frame): """Internal function called from GDB to execute all unwinders. diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c index 399e89b..a194798 100644 --- a/gdb/python/py-micmd.c +++ b/gdb/python/py-micmd.c @@ -88,9 +88,10 @@ struct mi_command_py : public mi_command mi_command_py (const char *name, micmdpy_object *object) : mi_command (name, nullptr), - m_pyobj (object) + m_pyobj (gdbpy_ref<micmdpy_object>::new_reference (object)) { pymicmd_debug_printf ("this = %p", this); + m_pyobj->mi_command = this; } ~mi_command_py () @@ -117,21 +118,42 @@ struct mi_command_py : public mi_command then always returns true. */ static void validate_installation (micmdpy_object *cmd_obj); - /* Update m_pyobj to NEW_PYOBJ. The pointer from M_PYOBJ that points + /* Update M_PYOBJ to NEW_PYOBJ. The pointer from M_PYOBJ that points back to this object is swapped with the pointer in NEW_PYOBJ, which must be nullptr, so that NEW_PYOBJ now points back to this object. - Additionally our parent's name string is stored in m_pyobj, so we + Additionally our parent's name string is stored in M_PYOBJ, so we swap the name string with NEW_PYOBJ. - Before this call m_pyobj is the Python object representing this MI + Before this call M_PYOBJ is the Python object representing this MI command object. After this call has completed, NEW_PYOBJ now represents this MI command object. */ void swap_python_object (micmdpy_object *new_pyobj) { + /* Current object has a backlink, new object doesn't have a backlink. */ + gdb_assert (m_pyobj->mi_command != nullptr); gdb_assert (new_pyobj->mi_command == nullptr); + + /* Clear the current M_PYOBJ's backlink, set NEW_PYOBJ's backlink. */ std::swap (new_pyobj->mi_command, m_pyobj->mi_command); + + /* Both object have names. */ + gdb_assert (m_pyobj->mi_command_name != nullptr); + gdb_assert (new_pyobj->mi_command_name != nullptr); + + /* mi_command::m_name is the string owned by the current object. */ + gdb_assert (m_pyobj->mi_command_name == this->name ()); + + /* The name in mi_command::m_name is owned by the current object. Rather + than changing the value of mi_command::m_name (which is not accessible + from here) to point to the name owned by the new object, swap the names + of the two objects, since we know they are identical strings. */ + gdb_assert (strcmp (new_pyobj->mi_command_name, + m_pyobj->mi_command_name) == 0); std::swap (new_pyobj->mi_command_name, m_pyobj->mi_command_name); - m_pyobj = new_pyobj; + + /* Take a reference to the new object, drop the reference to the current + object. */ + m_pyobj = gdbpy_ref<micmdpy_object>::new_reference (new_pyobj); } /* Called when the MI command is invoked. */ @@ -139,9 +161,11 @@ struct mi_command_py : public mi_command private: /* The Python object representing this MI command. */ - micmdpy_object *m_pyobj; + gdbpy_ref<micmdpy_object> m_pyobj; }; +using mi_command_py_up = std::unique_ptr<mi_command_py>; + extern PyTypeObject micmdpy_object_type CPYCHECKER_TYPE_OBJECT_FOR_TYPEDEF ("micmdpy_object"); @@ -321,8 +345,6 @@ mi_command_py::invoke (struct mi_parse *parse) const if (parse->argv == nullptr) error (_("Problem parsing arguments: %s %s"), parse->command, parse->args); - PyObject *obj = (PyObject *) this->m_pyobj; - gdb_assert (obj != nullptr); gdbpy_enter enter_py; @@ -341,9 +363,11 @@ mi_command_py::invoke (struct mi_parse *parse) const gdbpy_handle_exception (); } + gdb_assert (this->m_pyobj != nullptr); gdb_assert (PyErr_Occurred () == nullptr); - gdbpy_ref<> result (PyObject_CallMethodObjArgs (obj, invoke_cst, - argobj.get (), nullptr)); + gdbpy_ref<> result + (PyObject_CallMethodObjArgs ((PyObject *) this->m_pyobj.get (), invoke_cst, + argobj.get (), nullptr)); if (result == nullptr) gdbpy_handle_exception (); @@ -367,32 +391,13 @@ mi_command_py::validate_installation (micmdpy_object *cmd_obj) gdb_assert (cmd->m_pyobj == cmd_obj); } -/* Return a reference to the gdb._mi_commands dictionary. If the - dictionary can't be found for any reason then nullptr is returned, and - a Python exception will be set. */ +/* Return CMD as an mi_command_py if it is a Python MI command, else + nullptr. */ -static gdbpy_ref<> -micmdpy_global_command_dictionary () +static mi_command_py * +as_mi_command_py (mi_command *cmd) { - if (gdb_python_module == nullptr) - { - PyErr_SetString (PyExc_RuntimeError, _("unable to find gdb module")); - return nullptr; - } - - gdbpy_ref<> mi_cmd_dict (PyObject_GetAttrString (gdb_python_module, - "_mi_commands")); - if (mi_cmd_dict == nullptr) - return nullptr; - - if (!PyDict_Check (mi_cmd_dict.get ())) - { - PyErr_SetString (PyExc_RuntimeError, - _("gdb._mi_commands is not a dictionary as expected")); - return nullptr; - } - - return mi_cmd_dict; + return dynamic_cast<mi_command_py *> (cmd); } /* Uninstall OBJ, making the MI command represented by OBJ unavailable for @@ -409,41 +414,13 @@ micmdpy_uninstall_command (micmdpy_object *obj) pymicmd_debug_printf ("name = %s", obj->mi_command_name); - /* Remove the command from the internal MI table of commands, this will - cause the c++ object to be deleted, which will clear the mi_command - member variable within the Python object. */ - remove_mi_cmd_entry (obj->mi_command->name ()); + /* Remove the command from the internal MI table of commands. This will + cause the mi_command_py object to be deleted, which will clear the + backlink in OBJ. */ + bool removed = remove_mi_cmd_entry (obj->mi_command->name ()); + gdb_assert (removed); gdb_assert (obj->mi_command == nullptr); - gdbpy_ref<> mi_cmd_dict = micmdpy_global_command_dictionary (); - if (mi_cmd_dict == nullptr) - return -1; - - /* Grab the name for this command. */ - gdbpy_ref<> name_obj - = host_string_to_python_string (obj->mi_command_name); - if (name_obj == nullptr) - return -1; - - /* Lookup the gdb.MICommand object in the dictionary of all Python MI - commands, this is gdb._mi_command, and remove it. */ - PyObject *curr = PyDict_GetItemWithError (mi_cmd_dict.get (), - name_obj.get ()); - - /* Did we encounter an error? Failing to find the object in the - dictionary isn't an error, that's fine. */ - if (curr == nullptr && PyErr_Occurred ()) - return -1; - - /* Did we find this command in the gdb._mi_commands dictionary? If so, - then remove it. */ - if (curr != nullptr) - { - /* Yes we did! Remove it. */ - if (PyDict_DelItem (mi_cmd_dict.get (), name_obj.get ()) < 0) - return -1; - } - return 0; } @@ -467,96 +444,36 @@ micmdpy_install_command (micmdpy_object *obj) pymicmd_debug_printf ("name = %s", obj->mi_command_name); - gdbpy_ref<> mi_cmd_dict = micmdpy_global_command_dictionary (); - if (mi_cmd_dict == nullptr) - return -1; - - /* Look up this command name in the gdb._mi_commands dictionary, a - command with this name may already exist. */ - gdbpy_ref<> name_obj - = host_string_to_python_string (obj->mi_command_name); + /* Look up this command name in MI_COMMANDS, a command with this name may + already exist. */ + mi_command *cmd = mi_cmd_lookup (obj->mi_command_name); + mi_command_py *cmd_py = as_mi_command_py (cmd); - PyObject *curr = PyDict_GetItemWithError (mi_cmd_dict.get (), - name_obj.get ()); - if (curr == nullptr && PyErr_Occurred ()) - return -1; - if (curr != nullptr) + if (cmd != nullptr && cmd_py == nullptr) { - /* There is a command with this name already in the gdb._mi_commands - dictionary. First, validate that the object in the dictionary is - of the expected type, just in case something weird has happened. */ - if (!PyObject_IsInstance (curr, (PyObject *) &micmdpy_object_type)) - { - PyErr_SetString (PyExc_RuntimeError, - _("unexpected object in gdb._mi_commands dictionary")); - return -1; - } - - /* To get to this function OBJ should not be installed, which should - mean OBJ is not in the gdb._mi_commands dictionary. If we find - that OBJ is the thing in the dictionary, then something weird is - going on, we should throw an error. */ - micmdpy_object *other = (micmdpy_object *) curr; - if (other == obj || other->mi_command == nullptr) - { - PyErr_SetString (PyExc_RuntimeError, - _("uninstalled command found in gdb._mi_commands dictionary")); - return -1; - } - - /* All Python mi command object should always have a name set. */ - gdb_assert (other->mi_command_name != nullptr); - - /* We always insert commands into the gdb._mi_commands dictionary - using their name as a key, if this check fails then the dictionary - is in some weird state. */ - if (other->mi_command_name != other->mi_command->name () - || strcmp (other->mi_command_name, obj->mi_command_name) != 0) - { - PyErr_SetString (PyExc_RuntimeError, - _("gdb._mi_commands dictionary is corrupted")); - return -1; - } + /* There is already an MI command registered with that name, and it's not + a Python one. Forbid replacing a non-Python MI command. */ + PyErr_SetString (PyExc_RuntimeError, + _("unable to add command, name is already in use")); + return -1; + } - /* Switch the state of the c++ object held in the MI command table - so that it now references OBJ. After this action the old Python - object that used to be referenced from the MI command table will - now show as uninstalled, while the new Python object will show as - installed. */ - other->mi_command->swap_python_object (obj); - - gdb_assert (other->mi_command == nullptr); - gdb_assert (obj->mi_command != nullptr); - gdb_assert (obj->mi_command->name () == obj->mi_command_name); - - /* Remove the previous Python object from the gdb._mi_commands - dictionary, we'll install the new object below. */ - if (PyDict_DelItem (mi_cmd_dict.get (), name_obj.get ()) < 0) - return -1; + if (cmd_py != nullptr) + { + /* There is already a Python MI command registered with that name, swap + in the new gdb.MICommand implementation. */ + cmd_py->swap_python_object (obj); } else { - /* There's no Python object for this command name in the - gdb._mi_commands dictionary from which we can steal an existing - object already held in the MI commands table, and so, we now - create a new c++ object, and install it into the MI table. */ - obj->mi_command = new mi_command_py (obj->mi_command_name, obj); - mi_command_up micommand (obj->mi_command); + /* There's no MI command registered with that name at all, create one. */ + mi_command_py_up mi_cmd (new mi_command_py (obj->mi_command_name, obj)); /* Add the command to the gdb internal MI command table. */ - bool result = insert_mi_cmd_entry (std::move (micommand)); - if (!result) - { - PyErr_SetString (PyExc_RuntimeError, - _("unable to add command, name may already be in use")); - return -1; - } + bool result = insert_mi_cmd_entry (std::move (mi_cmd)); + gdb_assert (result); } - /* Finally, add the Python object to the gdb._mi_commands dictionary. */ - if (PyDict_SetItem (mi_cmd_dict.get (), name_obj.get (), (PyObject *) obj) < 0) - return -1; - return 0; } @@ -662,11 +579,10 @@ micmdpy_dealloc (PyObject *obj) (cmd->mi_command_name == nullptr ? "(null)" : cmd->mi_command_name)); - /* Remove the command from the MI command table if needed. This will - cause the mi_command_py object to be deleted, which, in turn, will - clear the cmd->mi_command member variable, hence the assert. */ - if (cmd->mi_command != nullptr) - remove_mi_cmd_entry (cmd->mi_command->name ()); + /* As the mi_command_py object holds a reference to the micmdpy_object, + the only way the dealloc function can be called is if the mi_command_py + object has been deleted, in which case the following assert will + hold. */ gdb_assert (cmd->mi_command == nullptr); /* Free the memory that holds the command name. */ @@ -698,6 +614,18 @@ gdbpy_initialize_micommands () return 0; } +void +gdbpy_finalize_micommands () +{ + /* mi_command_py objects hold references to micmdpy_object objects. They must + be dropped before the Python interpreter is finalized. Do so by removing + those MI command entries, thus deleting the mi_command_py objects. */ + remove_mi_cmd_entries ([] (mi_command *cmd) + { + return as_mi_command_py (cmd) != nullptr; + }); +} + /* Get the gdb.MICommand.name attribute, returns a string, the name of this MI command. */ diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h index 083c4db..098d913 100644 --- a/gdb/python/python-internal.h +++ b/gdb/python/python-internal.h @@ -564,6 +564,7 @@ int gdbpy_initialize_connection () CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION; int gdbpy_initialize_micommands (void) CPYCHECKER_NEGATIVE_RESULT_SETS_EXCEPTION; +void gdbpy_finalize_micommands (); /* 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 9795588..91636ef 100644 --- a/gdb/python/python.c +++ b/gdb/python/python.c @@ -1795,6 +1795,8 @@ finalize_python (void *ignore) (void) PyGILState_Ensure (); gdbpy_enter::finalize (); + gdbpy_finalize_micommands (); + Py_Finalize (); gdb_python_initialized = false; diff --git a/gdb/testsuite/gdb.python/py-mi-cmd.exp b/gdb/testsuite/gdb.python/py-mi-cmd.exp index c102efb..300ab95 100644 --- a/gdb/testsuite/gdb.python/py-mi-cmd.exp +++ b/gdb/testsuite/gdb.python/py-mi-cmd.exp @@ -289,31 +289,6 @@ mi_gdb_test "-aa" \ ".*\\^done,zzz={msg=\"message three\"}" \ "call the -aa command looking for message three" -# Remove the gdb._mi_commands dictionary, then try to register a new -# command. -mi_gdb_test "python del(gdb._mi_commands)" ".*\\^done" -mi_gdb_test "python pycmd3('-hello', 'Hello', 'msg')" \ - [multi_line \ - ".*" \ - "&\"AttributeError: module 'gdb' has no attribute '_mi_commands'..\"" \ - "&\"Error while executing Python code\\...\"" \ - "\\^error,msg=\"Error while executing Python code\\.\""] \ - "register a command with no gdb._mi_commands available" - -# Set gdb._mi_commands to be something other than a dictionary, and -# try to register a command. -mi_gdb_test "python gdb._mi_commands = 'string'" ".*\\^done" -mi_gdb_test "python pycmd3('-hello', 'Hello', 'msg')" \ - [multi_line \ - ".*" \ - "&\"RuntimeError: gdb._mi_commands is not a dictionary as expected..\"" \ - "&\"Error while executing Python code\\...\"" \ - "\\^error,msg=\"Error while executing Python code\\.\""] \ - "register a command when gdb._mi_commands is not a dictionary" - -# Restore gdb._mi_commands to a dictionary. -mi_gdb_test "python gdb._mi_commands = {}" ".*\\^done" - # Try to register a command object that is missing an invoke method. # This is accepted, but will give an error when the user tries to run # the command. @@ -346,37 +321,11 @@ mi_gdb_test "-hello" \ "\\^error,msg=\"Error occurred in Python: 'str' object is not callable\""] \ "execute command with invoke set to a string" -# Further checking of corruption to the gdb._mi_commands dictionary. -# -# First, insert an object of the wrong type, then try to register an -# MI command that will go into that same dictionary slot. -mi_gdb_test "python gdb._mi_commands\['blah'\] = 'blah blah blah'" ".*\\^done" -mi_gdb_test "python pycmd2('-blah')" \ - [multi_line \ - ".*" \ - "&\"RuntimeError: unexpected object in gdb\\._mi_commands dictionary..\"" \ - "&\"Error while executing Python code\\...\"" \ - "\\^error,msg=\"Error while executing Python code\\.\""] \ - "hit unexpected object in gdb._mi_commands dictionary" - -# Next, create a command, uninstall it, then force the command back -# into the dictionary. -mi_gdb_test "python cmd = pycmd2('-foo')" ".*\\^done" -mi_gdb_test "python cmd.installed = False" ".*\\^done" -mi_gdb_test "python gdb._mi_commands\['foo'\] = cmd" ".*\\^done" -mi_gdb_test "python cmd.installed = True" \ - [multi_line \ - ".*" \ - "&\"RuntimeError: uninstalled command found in gdb\\._mi_commands dictionary..\"" \ - "&\"Error while executing Python code\\...\"" \ - "\\^error,msg=\"Error while executing Python code\\.\""] \ - "found uninstalled command in gdb._mi_commands dictionary" - # Try to create a new MI command that uses the name of a builtin MI command. mi_gdb_test "python cmd = pycmd2('-data-disassemble')" \ [multi_line \ ".*" \ - "&\"RuntimeError: unable to add command, name may already be in use..\"" \ + "&\"RuntimeError: unable to add command, name is already in use..\"" \ "&\"Error while executing Python code\\...\"" \ "\\^error,msg=\"Error while executing Python code\\.\""] \ "try to register a command that replaces -data-disassemble" |