gdb/python: remove gdb._mi_commands dict

The motivation for this patch is the fact that py-micmd.c doesn't build
with Python 2, due to PyDict_GetItemWithError being a Python 3-only
function:

      CXX    python/py-micmd.o
    /home/smarchi/src/binutils-gdb/gdb/python/py-micmd.c: In function ‘int micmdpy_uninstall_command(micmdpy_object*)’:
    /home/smarchi/src/binutils-gdb/gdb/python/py-micmd.c:430:20: error: ‘PyDict_GetItemWithError’ was not declared in this scope; did you mean ‘PyDict_GetItemString’?
      430 |   PyObject *curr = PyDict_GetItemWithError (mi_cmd_dict.get (),
          |                    ^~~~~~~~~~~~~~~~~~~~~~~
          |                    PyDict_GetItemString

A first solution to fix this would be to try to replace
PyDict_GetItemWithError equivalent Python 2 code.  But I looked at why
we are doing this in the first place: it is to maintain the
`gdb._mi_commands` Python dictionary that we use as a `name ->
gdb.MICommand object` map.  Since the `gdb._mi_commands` dictionary is
never actually used in Python, it seems like a lot of trouble to use a
Python object for this.

My first idea was to replace it with a C++ map
(std::unordered_map<std::string, gdbpy_ref<micmdpy_object>>).  While
implementing this, I realized we don't really need this map at all.  The
mi_command_py objects registered in the main MI command table can own
their backing micmdpy_object (that's a gdb.MICommand, but seen from the
C++ code).  To know whether an mi_command is an mi_command_py, we can
use a dynamic cast.  Since there's one less data structure to maintain,
there are less chances of messing things up.

 - Change mi_command_py::m_pyobj to a gdbpy_ref, the mi_command_py is
   now what keeps the MICommand alive.
 - Set micmdpy_object::mi_command in the constructor of mi_command_py.
   If mi_command_py manages setting/clearing that field in
   swap_python_object, I think it makes sense that it also takes care of
   setting it initially.
 - Move a bunch of checks from micmdpy_install_command to
   swap_python_object, and make them gdb_asserts.
 - In micmdpy_install_command, start by doing an mi_cmd_lookup.  This is
   needed to know whether there's a Python MI command already registered
   with that name.  But we can already tell if there's a non-Python
   command registered with that name.  Return an error if that happens,
   rather than waiting for insert_mi_cmd_entry to fail.  Change the
   error message to "name is already in use" rather than "may already be
   in use", since it's more precise.

I asked Andrew about the original intent of using a Python dictionary
object to hold the command objects.  The reason was to make sure the
objects get destroyed when the Python runtime gets finalized, not later.
Holding the objects in global C++ data structures and not doing anything
more means that the held Python objects will be decref'd after the
Python interpreter has been finalized.  That's not desirable.  I tried
it and it indeed segfaults.

Handle this by adding a gdbpy_finalize_micommands function called in
finalize_python.  This is the mirror of gdbpy_initialize_micommands
called in do_start_initialization.  In there, delete all Python MI
commands.  I think it makes sense to do it this way: if it was somehow
possible to unload Python support from GDB in the middle of a session
we'd want to unregister any Python MI command.  Otherwise, these MI
commands would be backed with a stale PyObject or simply nothing.

Delete tests that were related to `gdb._mi_commands`.

Co-Authored-By: Andrew Burgess <aburgess@redhat.com>
Change-Id: I060d5ebc7a096c67487998a8a4ca1e8e56f12cd3

1 files changed, 1 insertions, 52 deletions

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"