[1/3] gdb/python: break dependencies between gdbpy_initialize_* functions

Message ID 5353d718bda4d304a38d1b29ae7ce6d455614acd.1664729134.git.aburgess@redhat.com
State Committed
Commit 66bd1b294d8e5b460d6b9c645d2db529f4c441de
Headers
Series New mechanism to initialise Python components in GDB |

Commit Message

Andrew Burgess Oct. 2, 2022, 4:53 p.m. UTC
  In a later commit in this series I will propose removing all of the
explicit gdbpy_initialize_* calls from python.c and replace these
calls with a more generic mechanism.

One of the side effects of this generic mechanism is that the order in
which the various Python sub-systems within GDB are initialized is no
longer guaranteed.

On the whole I don't think this matters, most of the sub-systems are
independent of each other, though testing did reveal a few places
where we did have dependencies, though I don't think those
dependencies were explicitly documented in a comment anywhere.

This commit removes the first dependency issue, with this and the next
commit, all of the implicit inter-sub-system dependencies will be
replaced by explicit dependencies, which will allow me to, I think,
clean up how the sub-systems are initialized.

The dependency is around the py_insn_type.  This type is setup in
gdbpy_initialize_instruction and used in gdbpy_initialize_record.
Rather than depend on the calls to these two functions being in a
particular order, in this commit I propose adding a new function
py_insn_get_insn_type.  This function will take care of setting up the
py_insn_type type and calling PyType_Ready.  This helper function can
be called from gdbpy_initialize_record and
gdbpy_initialize_instruction, and the py_insn_type will be initialized
just once.

To me this is better, the dependency is now really obvious, but also,
we no longer care in which order gdbpy_initialize_record and
gdbpy_initialize_instruction are called.

There should be no user visible changes after this commit.
---
 gdb/python/py-instruction.c | 41 +++++++++++++++++++++++++++++--------
 gdb/python/py-instruction.h | 13 ++++++++----
 gdb/python/py-record.c      |  2 +-
 3 files changed, 42 insertions(+), 14 deletions(-)
  

Comments

Tom Tromey Oct. 14, 2022, 5:06 p.m. UTC | #1
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> To me this is better, the dependency is now really obvious, but also,
Andrew> we no longer care in which order gdbpy_initialize_record and
Andrew> gdbpy_initialize_instruction are called.

I agree, I think this should go in.  Thank you.

Tom
  
Andrew Burgess Oct. 20, 2022, 3:58 p.m. UTC | #2
Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> To me this is better, the dependency is now really obvious, but also,
> Andrew> we no longer care in which order gdbpy_initialize_record and
> Andrew> gdbpy_initialize_instruction are called.
>
> I agree, I think this should go in.  Thank you.

Thanks, I've pushed this patch.

Andrew
  

Patch

diff --git a/gdb/python/py-instruction.c b/gdb/python/py-instruction.c
index e1ad49a5143..fee5bba4015 100644
--- a/gdb/python/py-instruction.c
+++ b/gdb/python/py-instruction.c
@@ -20,7 +20,9 @@ 
 #include "defs.h"
 #include "py-instruction.h"
 
-/* See py-instruction.h.  */
+/* Python type object for the abstract gdb.Instruction class.  This class
+   contains getters for four elements: "pc" (int), "data" (buffer), "decode"
+   (str) and "size" (int) that must be overridden by sub classes.  */
 
 PyTypeObject py_insn_type = {
   PyVarObject_HEAD_INIT (NULL, 0)
@@ -51,17 +53,38 @@  static gdb_PyGetSetDef py_insn_getset[] =
   {NULL}
 };
 
+/* See py-instruction.h.  */
+
+PyTypeObject *
+py_insn_get_insn_type ()
+{
+  if (py_insn_type.tp_new == nullptr)
+    {
+      py_insn_type.tp_new = PyType_GenericNew;
+      py_insn_type.tp_flags = Py_TPFLAGS_DEFAULT;
+      py_insn_type.tp_basicsize = sizeof (py_insn_obj);
+      py_insn_type.tp_name = "gdb.Instruction";
+      py_insn_type.tp_doc = "GDB instruction object";
+      py_insn_type.tp_getset = py_insn_getset;
+
+      if (PyType_Ready (&py_insn_type) < 0)
+	{
+	  /* Reset the tp_new field so any subsequent calls to this
+	     function will retry to make the type ready.  */
+	  py_insn_type.tp_new = nullptr;
+	  return nullptr;
+	}
+    }
+
+  return &py_insn_type;
+}
+
 /* Sets up the gdb.Instruction type.  */
 
 int
 gdbpy_initialize_instruction (void)
 {
-  py_insn_type.tp_new = PyType_GenericNew;
-  py_insn_type.tp_flags = Py_TPFLAGS_DEFAULT;
-  py_insn_type.tp_basicsize = sizeof (py_insn_obj);
-  py_insn_type.tp_name = "gdb.Instruction";
-  py_insn_type.tp_doc = "GDB instruction object";
-  py_insn_type.tp_getset = py_insn_getset;
-
-  return PyType_Ready (&py_insn_type);
+  if (py_insn_get_insn_type () == nullptr)
+    return -1;
+  return 0;
 }
diff --git a/gdb/python/py-instruction.h b/gdb/python/py-instruction.h
index 59f0893e641..1e602636a39 100644
--- a/gdb/python/py-instruction.h
+++ b/gdb/python/py-instruction.h
@@ -22,9 +22,14 @@ 
 
 #include "python-internal.h"
 
-/* Python type object for the abstract gdb.Instruction class.  This class
-   contains getters for four elements: "pc" (int), "data" (buffer), "decode"
-   (str) and "size" (int) that must be overridden by sub classes.  */
-extern PyTypeObject py_insn_type;
+/* Return a pointer to the py_insn_type object (see py-instruction.c), but
+   ensure that PyType_Ready has been called for the type first.  If the
+   PyType_Ready call is successful then subsequent calls to this function
+   will not call PyType_Ready, the type pointer will just be returned.
+
+   If the PyType_Ready call is not successful then nullptr is returned and
+   subsequent calls to this function will call PyType_Ready again.  */
+
+extern PyTypeObject *py_insn_get_insn_type ();
 
 #endif /* PYTHON_PY_INSTRUCTION_H */
diff --git a/gdb/python/py-record.c b/gdb/python/py-record.c
index 51084dfac72..fd5cfee84eb 100644
--- a/gdb/python/py-record.c
+++ b/gdb/python/py-record.c
@@ -563,7 +563,7 @@  gdbpy_initialize_record (void)
   recpy_insn_type.tp_getset = recpy_insn_getset;
   recpy_insn_type.tp_richcompare = recpy_element_richcompare;
   recpy_insn_type.tp_hash = recpy_element_hash;
-  recpy_insn_type.tp_base = &py_insn_type;
+  recpy_insn_type.tp_base = py_insn_get_insn_type ();
 
   recpy_func_type.tp_new = PyType_GenericNew;
   recpy_func_type.tp_flags = Py_TPFLAGS_DEFAULT;