[pushed] gdb/python: introduce gdbpy_registry
Checks
Commit Message
This commit introduces new template class gdbpy_registry to simplify
Python object lifecycle management. As of now, each of the Python
object implementations contain its own (copy of) lifecycle management
code that is largely very similar. The aim of gdbpy_registry is to
factor out this code into a common (template) class in order to simplify
the code.
Approved-By: Tom Tromey <tom@tromey.com>
---
gdb/python/python-internal.h | 195 +++++++++++++++++++++++++++++++++++
1 file changed, 195 insertions(+)
Comments
On 3/19/25 22:22, Jan Vrany wrote:
> + template<typename O>
> + using StorageKey = typename registry<O>::key<Storage>;
> +
This breaks the build with gcc 9 (9.3.1).
Thanks,
- Tom
$ ( cd build/gdb; rm -f varobj.o; make CXX=g++-9 varobj.o )
CXX varobj.o
In file included from /data/vries/gdb/src/gdb/varobj.c:38:
/data/vries/gdb/src/gdb/python/python-internal.h:1211:47: error:
expected ‘;’ before ‘<’ token
1211 | using StorageKey = typename registry<O>::key<Storage>;
| ^
| ;
/data/vries/gdb/src/gdb/python/python-internal.h:1214:41: error:
‘StorageKey’ does not name a type; did you mean ‘Storage’?
1214 | Storage *get_storage (O *owner, const StorageKey<O> &key) const
| ^~~~~~~~~~
| Storage
/data/vries/gdb/src/gdb/python/python-internal.h:1214:51: error:
expected ‘,’ or ‘...’ before ‘<’ token
1214 | Storage *get_storage (O *owner, const StorageKey<O> &key) const
| ^
/data/vries/gdb/src/gdb/python/python-internal.h: In member function
‘Storage* gdbpy_registry<Storage>::get_storage(O*, int) const’:
/data/vries/gdb/src/gdb/python/python-internal.h:1216:18: error: ‘key’
was not declared in this scope
1216 | Storage *r = key.get (owner);
| ^~~
make: *** [Makefile:1968: varobj.o] Error 1
Hi,
On Thu, Mar 20, 2025 at 08:00:41AM +0100, Tom de Vries wrote:
> On 3/19/25 22:22, Jan Vrany wrote:
> >+ template<typename O>
> >+ using StorageKey = typename registry<O>::key<Storage>;
> >+
>
> This breaks the build with gcc 9 (9.3.1).
Same with gcc 8.5.0 on AlmaLinux release 8.10
See https://builder.sourceware.org/buildbot/#/builders/290/builds/3045
Cheers,
Mark
> $ ( cd build/gdb; rm -f varobj.o; make CXX=g++-9 varobj.o )
> CXX varobj.o
> In file included from /data/vries/gdb/src/gdb/varobj.c:38:
> /data/vries/gdb/src/gdb/python/python-internal.h:1211:47: error:
> expected ‘;’ before ‘<’ token
> 1211 | using StorageKey = typename registry<O>::key<Storage>;
> | ^
> | ;
> /data/vries/gdb/src/gdb/python/python-internal.h:1214:41: error:
> ‘StorageKey’ does not name a type; did you mean ‘Storage’?
> 1214 | Storage *get_storage (O *owner, const StorageKey<O> &key) const
> | ^~~~~~~~~~
> | Storage
> /data/vries/gdb/src/gdb/python/python-internal.h:1214:51: error:
> expected ‘,’ or ‘...’ before ‘<’ token
> 1214 | Storage *get_storage (O *owner, const StorageKey<O> &key) const
> | ^
> /data/vries/gdb/src/gdb/python/python-internal.h: In member function
> ‘Storage* gdbpy_registry<Storage>::get_storage(O*, int) const’:
> /data/vries/gdb/src/gdb/python/python-internal.h:1216:18: error:
> ‘key’ was not declared in this scope
> 1216 | Storage *r = key.get (owner);
> | ^~~
> make: *** [Makefile:1968: varobj.o] Error 1
>
On 3/20/25 11:15, Mark Wielaard wrote:
> Hi,
>
> On Thu, Mar 20, 2025 at 08:00:41AM +0100, Tom de Vries wrote:
>> On 3/19/25 22:22, Jan Vrany wrote:
>>> + template<typename O>
>>> + using StorageKey = typename registry<O>::key<Storage>;
>>> +
>>
>> This breaks the build with gcc 9 (9.3.1).
>
> Same with gcc 8.5.0 on AlmaLinux release 8.10
> See https://builder.sourceware.org/buildbot/#/builders/290/builds/3045
>
Hi Mark,
I've pushed a fix for this (
https://sourceware.org/pipermail/gdb-patches/2025-March/216505.html ).
FWIW, build and tested with gcc 7.5.0.
Thanks,
- Tom
> Cheers,
>
> Mark
>
>> $ ( cd build/gdb; rm -f varobj.o; make CXX=g++-9 varobj.o )
>> CXX varobj.o
>> In file included from /data/vries/gdb/src/gdb/varobj.c:38:
>> /data/vries/gdb/src/gdb/python/python-internal.h:1211:47: error:
>> expected ‘;’ before ‘<’ token
>> 1211 | using StorageKey = typename registry<O>::key<Storage>;
>> | ^
>> | ;
>> /data/vries/gdb/src/gdb/python/python-internal.h:1214:41: error:
>> ‘StorageKey’ does not name a type; did you mean ‘Storage’?
>> 1214 | Storage *get_storage (O *owner, const StorageKey<O> &key) const
>> | ^~~~~~~~~~
>> | Storage
>> /data/vries/gdb/src/gdb/python/python-internal.h:1214:51: error:
>> expected ‘,’ or ‘...’ before ‘<’ token
>> 1214 | Storage *get_storage (O *owner, const StorageKey<O> &key) const
>> | ^
>> /data/vries/gdb/src/gdb/python/python-internal.h: In member function
>> ‘Storage* gdbpy_registry<Storage>::get_storage(O*, int) const’:
>> /data/vries/gdb/src/gdb/python/python-internal.h:1216:18: error:
>> ‘key’ was not declared in this scope
>> 1216 | Storage *r = key.get (owner);
>> | ^~~
>> make: *** [Makefile:1968: varobj.o] Error 1
>>
Hi Tom,
On Thu, Mar 20, 2025 at 11:19:26AM +0100, Tom de Vries wrote:
> On 3/20/25 11:15, Mark Wielaard wrote:
> >On Thu, Mar 20, 2025 at 08:00:41AM +0100, Tom de Vries wrote:
> >>On 3/19/25 22:22, Jan Vrany wrote:
> >>>+ template<typename O>
> >>>+ using StorageKey = typename registry<O>::key<Storage>;
> >>>+
> >>
> >>This breaks the build with gcc 9 (9.3.1).
> >
> >Same with gcc 8.5.0 on AlmaLinux release 8.10
> >See https://builder.sourceware.org/buildbot/#/builders/290/builds/3045
>
> I've pushed a fix for this (
> https://sourceware.org/pipermail/gdb-patches/2025-March/216505.html
> ).
Thanks! The buildbot is green again:
https://builder.sourceware.org/buildbot/#/changes/71628
Cheers,
Mark
On Thu, 2025-03-20 at 11:19 +0100, Tom de Vries wrote:
> On 3/20/25 11:15, Mark Wielaard wrote:
> > Hi,
> >
> > On Thu, Mar 20, 2025 at 08:00:41AM +0100, Tom de Vries wrote:
> > > On 3/19/25 22:22, Jan Vrany wrote:
> > > > + template<typename O>
> > > > + using StorageKey = typename registry<O>::key<Storage>;
> > > > +
> > >
> > > This breaks the build with gcc 9 (9.3.1).
> >
> > Same with gcc 8.5.0 on AlmaLinux release 8.10
> > See
> > https://builder.sourceware.org/buildbot/#/builders/290/builds/3045
> >
>
> Hi Mark,
>
> I've pushed a fix for this (
> https://sourceware.org/pipermail/gdb-patches/2025-March/216505.html
> ).
Thanks a lot!
I'll try to add gcc-8.5 and gcc-9 to my testing setup.
Jan
>
> FWIW, build and tested with gcc 7.5.0.
>
> Thanks,
> - Tom
>
> > Cheers,
> >
> > Mark
> >
> > > $ ( cd build/gdb; rm -f varobj.o; make CXX=g++-9 varobj.o )
> > > CXX varobj.o
> > > In file included from /data/vries/gdb/src/gdb/varobj.c:38:
> > > /data/vries/gdb/src/gdb/python/python-internal.h:1211:47: error:
> > > expected ‘;’ before ‘<’ token
> > > 1211 | using StorageKey = typename registry<O>::key<Storage>;
> > > | ^
> > > | ;
> > > /data/vries/gdb/src/gdb/python/python-internal.h:1214:41: error:
> > > ‘StorageKey’ does not name a type; did you mean ‘Storage’?
> > > 1214 | Storage *get_storage (O *owner, const StorageKey<O>
> > > &key) const
> > > | ^~~~~~~~~~
> > > | Storage
> > > /data/vries/gdb/src/gdb/python/python-internal.h:1214:51: error:
> > > expected ‘,’ or ‘...’ before ‘<’ token
> > > 1214 | Storage *get_storage (O *owner, const StorageKey<O>
> > > &key) const
> > > | ^
> > > /data/vries/gdb/src/gdb/python/python-internal.h: In member
> > > function
> > > ‘Storage* gdbpy_registry<Storage>::get_storage(O*, int) const’:
> > > /data/vries/gdb/src/gdb/python/python-internal.h:1216:18: error:
> > > ‘key’ was not declared in this scope
> > > 1216 | Storage *r = key.get (owner);
> > > | ^~~
> > > make: *** [Makefile:1968: varobj.o] Error 1
> > >
@@ -22,6 +22,7 @@
#include "extension.h"
#include "extension-priv.h"
+#include "registry.h"
/* These WITH_* macros are defined by the CPython API checker that
comes with the Python plugin for GCC. See:
@@ -1145,4 +1146,198 @@ gdbpy_type_ready (PyTypeObject *type, PyObject *mod = nullptr)
# define PyType_Ready POISONED_PyType_Ready
#endif
+/* A class to manage lifecycle of Python objects for objects that are "owned"
+ by an objfile or a gdbarch. It keeps track of Python objects and when
+ the "owning" object (objfile or gdbarch) is about to be freed, ensures that
+ all Python objects "owned" by that object are properly invalidated.
+
+ The actual tracking of "owned" Python objects is handled externally
+ by storage class. Storage object is created for each owning object
+ on demand and it is deleted when owning object is about to be freed.
+
+ The storage class must provide two member types:
+
+ * obj_type - the type of Python object whose lifecycle is managed.
+ * val_type - the type of GDB structure the Python objects are
+ representing.
+
+ It must also provide following methods:
+
+ void add (obj_type *obj);
+ void remove (obj_type *obj);
+
+ Memoizing storage must in addition to method above provide:
+
+ obj_type *lookup (val_type *val);
+
+ Finally it must invalidate all registered Python objects upon deletion. */
+template <typename Storage>
+class gdbpy_registry
+{
+public:
+ using obj_type = typename Storage::obj_type;
+ using val_type = typename Storage::val_type;
+
+ /* Register Python object OBJ as being "owned" by OWNER. When OWNER is
+ about to be freed, OBJ will be invalidated. */
+ template <typename O>
+ void add (O *owner, obj_type *obj) const
+ {
+ get_storage (owner)->add (obj);
+ }
+
+ /* Unregister Python object OBJ. OBJ will no longer be invalidated when
+ OWNER is about to be be freed. */
+ template <typename O>
+ void remove (O *owner, obj_type *obj) const
+ {
+ get_storage (owner)->remove (obj);
+ }
+
+ /* Lookup pre-existing Python object for given VAL. Return such object
+ if found, otherwise return NULL. This method always returns new
+ reference. */
+ template <typename O>
+ obj_type *lookup (O *owner, val_type *val) const
+ {
+ obj_type *obj = get_storage (owner)->lookup (val);
+ Py_XINCREF (obj);
+ return obj;
+ }
+
+private:
+
+ template<typename O>
+ using StorageKey = typename registry<O>::key<Storage>;
+
+ template<typename O>
+ Storage *get_storage (O *owner, const StorageKey<O> &key) const
+ {
+ Storage *r = key.get (owner);
+ if (r == nullptr)
+ {
+ r = new Storage();
+ key.set (owner, r);
+ }
+ return r;
+ }
+
+ Storage *get_storage (struct objfile* objf) const
+ {
+ return get_storage (objf, m_key_for_objf);
+ }
+
+ Storage *get_storage (struct gdbarch* arch) const
+ {
+ return get_storage (arch, m_key_for_arch);
+ }
+
+ const registry<objfile>::key<Storage> m_key_for_objf;
+ const registry<gdbarch>::key<Storage> m_key_for_arch;
+};
+
+/* Default invalidator for Python objects. */
+template <typename P, typename V, V* P::*val_slot>
+struct gdbpy_default_invalidator
+{
+ void operator() (P *obj)
+ {
+ obj->*val_slot = nullptr;
+ }
+};
+
+/* A "storage" implementation suitable for temporary (on-demand) objects. */
+template <typename P,
+ typename V,
+ V* P::*val_slot,
+ typename Invalidator = gdbpy_default_invalidator<P, V, val_slot>>
+class gdbpy_tracking_registry_storage
+{
+public:
+ using obj_type = P;
+ using val_type = V;
+
+ void add (obj_type *obj)
+ {
+ gdb_assert (obj != nullptr && obj->*val_slot != nullptr);
+
+ m_objects.insert (obj);
+ }
+
+ void remove (obj_type *obj)
+ {
+ gdb_assert (obj != nullptr && obj->*val_slot != nullptr);
+ gdb_assert (m_objects.contains (obj));
+
+ m_objects.erase (obj);
+ }
+
+ ~gdbpy_tracking_registry_storage ()
+ {
+ Invalidator invalidate;
+ gdbpy_enter enter_py;
+
+ for (auto each : m_objects)
+ invalidate (each);
+ m_objects.clear ();
+ }
+
+protected:
+ gdb::unordered_set<obj_type *> m_objects;
+};
+
+/* A "storage" implementation suitable for memoized (interned) Python objects.
+
+ Python objects are memoized (interned) temporarily, meaning that when user
+ drops all their references the Python object is deallocated and removed
+ from storage.
+ */
+template <typename P,
+ typename V,
+ V* P::*val_slot,
+ typename Invalidator = gdbpy_default_invalidator<P, V, val_slot>>
+class gdbpy_memoizing_registry_storage
+{
+public:
+ using obj_type = P;
+ using val_type = V;
+
+ void add (obj_type *obj)
+ {
+ gdb_assert (obj != nullptr && obj->*val_slot != nullptr);
+
+ m_objects[obj->*val_slot] = obj;
+ }
+
+ void remove (obj_type *obj)
+ {
+ gdb_assert (obj != nullptr && obj->*val_slot != nullptr);
+ gdb_assert (m_objects.contains (obj->*val_slot));
+
+ m_objects.erase (obj->*val_slot);
+ }
+
+ obj_type *lookup (val_type *val) const
+ {
+ auto result = m_objects.find (val);
+ if (result != m_objects.end ())
+ return result->second;
+ else
+ return nullptr;
+ }
+
+ ~gdbpy_memoizing_registry_storage ()
+ {
+ Invalidator invalidate;
+ gdbpy_enter enter_py;
+
+ for (auto each : m_objects)
+ invalidate (each.second);
+ m_objects.clear ();
+ }
+
+protected:
+ gdb::unordered_map<val_type *, obj_type *> m_objects;
+};
+
#endif /* GDB_PYTHON_PYTHON_INTERNAL_H */