[PATCH v3 0/4] Python safety initial work

[PATCH v3 0/4] Python safety initial work

[Tom Tromey]

I rewrote my earlier Python safety series to use methods a while back.
And while I like the result, I got bogged down converting things, so I
thought I'd send some initial work, with the idea that if this looks
good, other code can be transformed in pieces.

Since this is extracted from a larger series, there is some code in
here that isn't currently used in-tree.  This is mainly true for the
gdb functions that wrap Python APIs, see patch 2.

Patch 4 shows what the end effect is, but the patch is somewhat hard
to read IMO.  So for instance now the 'title' getter for a TUI window
is just an ordinary method:

    const std::string &
    gdbpy_tui_window::title ()
    {
      require_valid ();
      return window->title ();
    }

Since it wants to only operate on a valid window, require_valid will
throw if that isn't the case.

Note there's no explicit error checking, no casts, no messing with
reference counts, and the return type is just a string.  All these
boilerplate details are handled by a template wrapper function.

Signed-off-by: Tom Tromey <tom@tromey.com>
---
Changes in v3:
- Updated per review
- Changed gdbpy_opt_borrowed_ref and gdbpy_borrowed_ref to templates
- Link to v2: https://inbox.sourceware.org/gdb-patches/20260515-python-safety-initial-v2-0-6129cadf258a@tromey.com

Changes in v2:
- Avoids Py_RETURN_* macros
- Link to v1: https://inbox.sourceware.org/gdb-patches/20260515-python-safety-initial-v1-0-8f155338df57@tromey.com

---
Tom Tromey (4):
      Add gdbpy_borrowed_ref
      Add wrappers for some Python APIs
      Add wrappers for Python implementation functions and methods
      Convert py-tui.c to the "python safety" approach

 gdb/python/py-ref.h          |  86 +++++++++++
 gdb/python/py-safety.h       | 346 +++++++++++++++++++++++++++++++++++++++++
 gdb/python/py-tui.c          | 209 +++++++++++--------------
 gdb/python/py-wrappers.h     | 361 +++++++++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h |   8 +-
 gdb/python/python.c          |   5 +-
 6 files changed, 890 insertions(+), 125 deletions(-)
---
base-commit: 1500a5d93a2ae02ee227d6b227acfeda9f6d4f6a
change-id: 20260515-python-safety-initial-422bf34a8ce3

Best regards,
-- 
Tom Tromey <tom@tromey.com>

[PATCH v3 1/4] Add gdbpy_borrowed_ref

[Tom Tromey]

This adds new gdbpy_opt_borrowed_ref and gdbpy_borrowed_ref classes.
These classes are primarily for code "documentation" purposes -- it
makes it clear to the reader that a given reference is borrowed.
However, they also add a tiny bit of safety, in that conversion to
gdbpy_ref<> will either be rejected (by the "opt" class) or acquire a
new reference.
---
 gdb/python/py-ref.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
index 3d2906fd7f5..3d0373b7001 100644
--- a/gdb/python/py-ref.h
+++ b/gdb/python/py-ref.h
@@ -21,6 +21,7 @@
 #define GDB_PYTHON_PY_REF_H
 
 #include "gdbsupport/gdb_ref_ptr.h"
+#include "gdbsupport/traits.h"
 #include "python-traits.h"
 
 /* A policy class for gdb::ref_ptr for Python reference counting.  */
@@ -42,6 +43,91 @@ struct gdbpy_ref_policy
 template<typename T = PyObject> using gdbpy_ref
   = gdb::ref_ptr<T, gdbpy_ref_policy>;
 
+/* A class representing an optional borrowed reference.  It is
+   "optional" because NULL is a valid value.
+
+   This is a simple wrapper for a pointer to PyObject or some subclass
+   of it.  Aside from documenting what the code does, the main
+   advantage of using this is that conversion to a gdbpy_ref<T> is
+   prevented.
+
+   An optional borrowed reference is only used in situations where
+   Python says NULL is valid.  For example, it is used as the type of
+   the "keywords" argument to a varargs method.  Most code should
+   prefer an ordinary gdbpy_borrowed_ref, see below.  */
+template<typename T = PyObject>
+class gdbpy_opt_borrowed_ref
+{
+public:
+
+  gdbpy_opt_borrowed_ref (T *obj)
+    : m_obj (obj)
+  {
+  }
+
+  template<typename U>
+  gdbpy_opt_borrowed_ref (const gdbpy_ref<U> &ref)
+    : m_obj (ref.get ())
+  {
+  }
+
+  operator T * () const
+  {
+    return m_obj;
+  }
+
+  operator gdbpy_ref<T> () = delete;
+
+protected:
+
+  T *m_obj;
+};
+
+/* A borrowed reference that is guaranteed not to be NULL.
+
+   Like gdbpy_opt_borrowed_ref, this mostly serves a documentary
+   purpose.  However, it also allows a checked cast to any subclass of
+   T, and conversion to a gdbpy_ref<T> will automatically acquire a
+   new reference -- a safety improvement over plain PyObject * or the
+   like.  */
+template<typename T = PyObject>
+class gdbpy_borrowed_ref : public gdbpy_opt_borrowed_ref<T>
+{
+public:
+
+  gdbpy_borrowed_ref (T *obj)
+    : gdbpy_opt_borrowed_ref<T> (obj)
+  {
+    gdb_assert (this->m_obj != nullptr);
+  }
+
+  template<typename U>
+  gdbpy_borrowed_ref (const gdbpy_ref<U> &ref)
+    : gdbpy_opt_borrowed_ref<T> (ref)
+  {
+    gdb_assert (this->m_obj != nullptr);
+  }
+
+  gdbpy_borrowed_ref (std::nullptr_t) = delete;
+
+  /* Allow a (checked) conversion to any subclass of T.  */
+  template<typename U,
+	   typename = gdb::Requires<std::is_convertible<U *, T *>>>
+  operator U * () const
+  {
+    gdb_assert (PyObject_TypeCheck (this->m_obj,
+				    U::corresponding_object_type));
+    return static_cast<U *> (this->m_obj);
+  }
+
+  /* When converting a borrowed reference to a gdbpy_ref<>, a new
+     reference is acquired.  */
+  operator gdbpy_ref<T> () const
+  {
+    return gdbpy_ref<T>::new_reference (this->m_obj);
+  }
+};
+
 /* A wrapper class for Python extension objects that have a __dict__ attribute.
 
    Any Python C object extension needing __dict__ should inherit from this

-- 
2.49.0

Re: [PATCH v3 1/4] Add gdbpy_borrowed_ref

[Matthieu Longo]

On 22/05/2026 00:27, Tom Tromey wrote:
> This adds new gdbpy_opt_borrowed_ref and gdbpy_borrowed_ref classes.
> These classes are primarily for code "documentation" purposes -- it
> makes it clear to the reader that a given reference is borrowed.
> However, they also add a tiny bit of safety, in that conversion to
> gdbpy_ref<> will either be rejected (by the "opt" class) or acquire a
> new reference.
> ---
>   gdb/python/py-ref.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 86 insertions(+)
> 
> diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
> index 3d2906fd7f5..3d0373b7001 100644
> --- a/gdb/python/py-ref.h
> +++ b/gdb/python/py-ref.h
> @@ -21,6 +21,7 @@
>   #define GDB_PYTHON_PY_REF_H
>   
>   #include "gdbsupport/gdb_ref_ptr.h"
> +#include "gdbsupport/traits.h"
>   #include "python-traits.h"
>   
>   /* A policy class for gdb::ref_ptr for Python reference counting.  */
> @@ -42,6 +43,91 @@ struct gdbpy_ref_policy
>   template<typename T = PyObject> using gdbpy_ref
>     = gdb::ref_ptr<T, gdbpy_ref_policy>;
>   
> +/* A class representing an optional borrowed reference.  It is
> +   "optional" because NULL is a valid value.
> +
> +   This is a simple wrapper for a pointer to PyObject or some subclass
> +   of it.  Aside from documenting what the code does, the main
> +   advantage of using this is that conversion to a gdbpy_ref<T> is
> +   prevented.
> +
> +   An optional borrowed reference is only used in situations where
> +   Python says NULL is valid.  For example, it is used as the type of
> +   the "keywords" argument to a varargs method.  Most code should
> +   prefer an ordinary gdbpy_borrowed_ref, see below.  */
> +template<typename T = PyObject>
> +class gdbpy_opt_borrowed_ref
> +{
> +public:
> +
> +  gdbpy_opt_borrowed_ref (T *obj)
> +    : m_obj (obj)
> +  {
> +  }
> +
> +  template<typename U>
> +  gdbpy_opt_borrowed_ref (const gdbpy_ref<U> &ref)
> +    : m_obj (ref.get ())
> +  {
> +  }
> +
> +  operator T * () const
> +  {
> +    return m_obj;
> +  }
> +
> +  operator gdbpy_ref<T> () = delete;
> +
> +protected:
> +
> +  T *m_obj;
> +};
> +
> +/* A borrowed reference that is guaranteed not to be NULL.
> +
> +   Like gdbpy_opt_borrowed_ref, this mostly serves a documentary
> +   purpose.  However, it also allows a checked cast to any subclass of
> +   T, and conversion to a gdbpy_ref<T> will automatically acquire a
> +   new reference -- a safety improvement over plain PyObject * or the
> +   like.  */
> +template<typename T = PyObject>
> +class gdbpy_borrowed_ref : public gdbpy_opt_borrowed_ref<T>
> +{
> +public:
> +
> +  gdbpy_borrowed_ref (T *obj)
> +    : gdbpy_opt_borrowed_ref<T> (obj)
> +  {
> +    gdb_assert (this->m_obj != nullptr);
> +  }
> +
> +  template<typename U>
> +  gdbpy_borrowed_ref (const gdbpy_ref<U> &ref)
> +    : gdbpy_opt_borrowed_ref<T> (ref)
> +  {
> +    gdb_assert (this->m_obj != nullptr);
> +  }
> +
> +  gdbpy_borrowed_ref (std::nullptr_t) = delete;
> +
> +  /* Allow a (checked) conversion to any subclass of T.  */
> +  template<typename U,
> +	   typename = gdb::Requires<std::is_convertible<U *, T *>>>
> +  operator U * () const
> +  {
> +    gdb_assert (PyObject_TypeCheck (this->m_obj,
> +				    U::corresponding_object_type));
> +    return static_cast<U *> (this->m_obj);
> +  }
> +
> +  /* When converting a borrowed reference to a gdbpy_ref<>, a new
> +     reference is acquired.  */
> +  operator gdbpy_ref<T> () const
> +  {
> +    return gdbpy_ref<T>::new_reference (this->m_obj);
> +  }
> +};
> +
>   /* A wrapper class for Python extension objects that have a __dict__ attribute.
>   
>      Any Python C object extension needing __dict__ should inherit from this
> 

Hi Tom,

Any idea when this patch could land on master ?

Matthieu

Re: [PATCH v3 1/4] Add gdbpy_borrowed_ref

[Tom Tromey]

>>>>> "Matthieu" == Matthieu Longo <matthieu.longo@arm.com> writes:

>> This adds new gdbpy_opt_borrowed_ref and gdbpy_borrowed_ref classes.
>> These classes are primarily for code "documentation" purposes -- it
>> makes it clear to the reader that a given reference is borrowed.
>> However, they also add a tiny bit of safety, in that conversion to
>> gdbpy_ref<> will either be rejected (by the "opt" class) or acquire a
>> new reference.

Matthieu> Any idea when this patch could land on master ?

It's hard to say.

For ordinary-ish patches, I tend to wait a couple of weeks and then just
check them in if they haven't been commented on.

For something like this, though, I'd normally wait and/or ping it until
there's some review; the difference being that a big change to how new
Python code should be written ought to have some buy-in.

I suppose I could land this particular one sooner.  That's a little
weird since it's not actually used by anything.  OTOH it unblocks stuff
you're doing.

Tom

Re: [PATCH v3 1/4] Add gdbpy_borrowed_ref

[Yury Khrustalev]

Hi Tom and Matthieu,

On Wed, Jun 03, 2026 at 12:19:37PM -0600, Tom Tromey wrote:
> >>>>> "Matthieu" == Matthieu Longo <matthieu.longo@arm.com> writes:
> 
> >> This adds new gdbpy_opt_borrowed_ref and gdbpy_borrowed_ref classes.
> >> These classes are primarily for code "documentation" purposes -- it
> >> makes it clear to the reader that a given reference is borrowed.
> >> However, they also add a tiny bit of safety, in that conversion to
> >> gdbpy_ref<> will either be rejected (by the "opt" class) or acquire a
> >> new reference.
> 
> Matthieu> Any idea when this patch could land on master ?
> 
> It's hard to say.
> 
> For ordinary-ish patches, I tend to wait a couple of weeks and then just
> check them in if they haven't been commented on.
> 
> For something like this, though, I'd normally wait and/or ping it until
> there's some review; the difference being that a big change to how new
> Python code should be written ought to have some buy-in.
> 
> I suppose I could land this particular one sooner.  That's a little
> weird since it's not actually used by anything.  OTOH it unblocks stuff
> you're doing.

I think this has been in review for a while. If there is nothing that
needs to be fixed in this patch, could we go ahead? There is a lot of
other work that is blocked and that we'd like to progress.

Thanks,
Yury

Re: [PATCH v3 1/4] Add gdbpy_borrowed_ref

[Matthieu Longo]

On 24/07/2026 13:02, Yury Khrustalev wrote:
> Hi Tom and Matthieu,
> 
> On Wed, Jun 03, 2026 at 12:19:37PM -0600, Tom Tromey wrote:
>>>>>>> "Matthieu" == Matthieu Longo <matthieu.longo@arm.com> writes:
>>
>>>> This adds new gdbpy_opt_borrowed_ref and gdbpy_borrowed_ref classes.
>>>> These classes are primarily for code "documentation" purposes -- it
>>>> makes it clear to the reader that a given reference is borrowed.
>>>> However, they also add a tiny bit of safety, in that conversion to
>>>> gdbpy_ref<> will either be rejected (by the "opt" class) or acquire a
>>>> new reference.
>>
>> Matthieu> Any idea when this patch could land on master ?
>>
>> It's hard to say.
>>
>> For ordinary-ish patches, I tend to wait a couple of weeks and then just
>> check them in if they haven't been commented on.
>>
>> For something like this, though, I'd normally wait and/or ping it until
>> there's some review; the difference being that a big change to how new
>> Python code should be written ought to have some buy-in.
>>
>> I suppose I could land this particular one sooner.  That's a little
>> weird since it's not actually used by anything.  OTOH it unblocks stuff
>> you're doing.
> 
> I think this has been in review for a while. If there is nothing that
> needs to be fixed in this patch, could we go ahead? There is a lot of
> other work that is blocked and that we'd like to progress.
> 
> Thanks,
> Yury
> 

FYI Tom committed this patch beginning of August.
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=4294f0662859f3e06ae731867de95673b68fc67e

Matthieu

Re: [PATCH v3 1/4] Add gdbpy_borrowed_ref

[Matthieu Longo]

On 22/05/2026 00:27, Tom Tromey wrote:
> This adds new gdbpy_opt_borrowed_ref and gdbpy_borrowed_ref classes.
> These classes are primarily for code "documentation" purposes -- it
> makes it clear to the reader that a given reference is borrowed.
> However, they also add a tiny bit of safety, in that conversion to
> gdbpy_ref<> will either be rejected (by the "opt" class) or acquire a
> new reference.
> ---
>  gdb/python/py-ref.h | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
> index 3d2906fd7f5..3d0373b7001 100644
> --- a/gdb/python/py-ref.h
> +++ b/gdb/python/py-ref.h
> @@ -21,6 +21,7 @@
>  #define GDB_PYTHON_PY_REF_H
>  
>  #include "gdbsupport/gdb_ref_ptr.h"
> +#include "gdbsupport/traits.h"
>  #include "python-traits.h"
>  
>  /* A policy class for gdb::ref_ptr for Python reference counting.  */
> @@ -42,6 +43,91 @@ struct gdbpy_ref_policy
>  template<typename T = PyObject> using gdbpy_ref
>    = gdb::ref_ptr<T, gdbpy_ref_policy>;
>  
> +/* A class representing an optional borrowed reference.  It is
> +   "optional" because NULL is a valid value.
> +
> +   This is a simple wrapper for a pointer to PyObject or some subclass
> +   of it.  Aside from documenting what the code does, the main
> +   advantage of using this is that conversion to a gdbpy_ref<T> is
> +   prevented.
> +
> +   An optional borrowed reference is only used in situations where
> +   Python says NULL is valid.  For example, it is used as the type of
> +   the "keywords" argument to a varargs method.  Most code should
> +   prefer an ordinary gdbpy_borrowed_ref, see below.  */
> +template<typename T = PyObject>
> +class gdbpy_opt_borrowed_ref
> +{
> +public:
> +
> +  gdbpy_opt_borrowed_ref (T *obj)
> +    : m_obj (obj)
> +  {
> +  }
> +
> +  template<typename U>
> +  gdbpy_opt_borrowed_ref (const gdbpy_ref<U> &ref)
> +    : m_obj (ref.get ())
> +  {
> +  }
> +
> +  operator T * () const

noexcept ?

> +  {
> +    return m_obj;
> +  }
> +
> +  operator gdbpy_ref<T> () = delete;

Is there any possibility to add a bool operator to avoid "if (my_ref == nullptr)" ?

Also, what about:
- T *operator-> () const noexcept
- T *get () const noexcept

> +
> +protected:
> +
> +  T *m_obj;
> +};
> +
> +/* A borrowed reference that is guaranteed not to be NULL.
> +
> +   Like gdbpy_opt_borrowed_ref, this mostly serves a documentary
> +   purpose.  However, it also allows a checked cast to any subclass of
> +   T, and conversion to a gdbpy_ref<T> will automatically acquire a
> +   new reference -- a safety improvement over plain PyObject * or the
> +   like.  */
> +template<typename T = PyObject>
> +class gdbpy_borrowed_ref : public gdbpy_opt_borrowed_ref<T>
> +{
> +public:
> +
> +  gdbpy_borrowed_ref (T *obj)
> +    : gdbpy_opt_borrowed_ref<T> (obj)
> +  {
> +    gdb_assert (this->m_obj != nullptr);
> +  }
> +
> +  template<typename U>
> +  gdbpy_borrowed_ref (const gdbpy_ref<U> &ref)
> +    : gdbpy_opt_borrowed_ref<T> (ref)
> +  {
> +    gdb_assert (this->m_obj != nullptr);
> +  }
> +
> +  gdbpy_borrowed_ref (std::nullptr_t) = delete;
> +
> +  /* Allow a (checked) conversion to any subclass of T.  */
> +  template<typename U,
> +	   typename = gdb::Requires<std::is_convertible<U *, T *>>>
> +  operator U * () const
> +  {
> +    gdb_assert (PyObject_TypeCheck (this->m_obj,
> +				    U::corresponding_object_type));
> +    return static_cast<U *> (this->m_obj);
> +  }
> +
> +  /* When converting a borrowed reference to a gdbpy_ref<>, a new
> +     reference is acquired.  */
> +  operator gdbpy_ref<T> () const
> +  {
> +    return gdbpy_ref<T>::new_reference (this->m_obj);
> +  }
> +};
> +
>  /* A wrapper class for Python extension objects that have a __dict__ attribute.
>  
>     Any Python C object extension needing __dict__ should inherit from this
> 

Hi Tom,

How can I help to make this patch be merged ?
Are you waiting for inputs from others maintainers ?

Matthieu

[PATCH v3 2/4] Add wrappers for some Python APIs

[Tom Tromey]

This adds some new functions that wrap Python APIs.  The wrapping
follows some proposed rules for Python safety in gdb:

* Functions returning a new reference return gdbpy_ref<>

* Errors are reported via exceptions, not special values

* Functions accepting a stolen reference take a gdbpy_ref<>&&
---
 gdb/python/py-wrappers.h     | 361 +++++++++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h |   2 +
 2 files changed, 363 insertions(+)

diff --git a/gdb/python/py-wrappers.h b/gdb/python/py-wrappers.h
new file mode 100644
index 00000000000..6c2b5e4d41e
--- /dev/null
+++ b/gdb/python/py-wrappers.h
@@ -0,0 +1,361 @@
+/* Wrappers for some Python safety.
+
+   Copyright (C) 2026 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 GDB_PYTHON_PY_WRAPPERS_H
+#define GDB_PYTHON_PY_WRAPPERS_H
+
+#include "py-ref.h"
+
+/* Gdb implements its own wrappers for many Python APIs.  This is done
+   in an attempt to be more safe.
+
+   In particular, in gdb:
+
+   - APIs returning a new reference will return gdbpy_ref<>.  This
+   makes reference counting errors less likely.
+
+   - APIs will throw an exception rather than return a special value
+   (NULL or -1).  This makes error checking simpler.
+
+   - APIs requiring a stolen reference take a gdbpy_ref<>&&, to make
+   reference counting errors less likely.
+
+   This file holds the currently-defined wrappers.  If new APIs are
+   needed, the normal approach is to add a wrapper here.
+
+   APIs here are named after the underlying Python function, but using
+   lower case and an "_" at each word break.  */
+
+/* The type of exception thrown when the Python exception has been
+   set.  */
+struct gdb_python_exception
+{
+  gdb_python_exception ()
+  {
+    gdb_assert (PyErr_Occurred ());
+  }
+};
+
+/* Wrapper for PyObject_New.  */
+template<typename T>
+gdbpy_ref<T>
+gdbpy_new ()
+{
+  gdbpy_ref<T> result (PyObject_New (T, T::corresponding_object_type));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyBool_FromLong.  */
+static inline gdbpy_ref<>
+gdbpy_bool_from_long (long value)
+{
+  /* This cannot fail.  */
+  return gdbpy_ref<> (PyBool_FromLong (value));
+}
+
+/* Wrapper for PyBytes_AsString.  */
+static inline char *
+gdbpy_bytes_as_string (gdbpy_borrowed_ref<> ref)
+{
+  char *result = PyBytes_AsString (ref);
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyBytes_AsStringAndSize.  */
+static inline void
+gdbpy_bytes_as_string_and_size (gdbpy_borrowed_ref<> ref,
+				char **buffer,
+				Py_ssize_t *length)
+{
+  if (PyBytes_AsStringAndSize (ref, buffer, length) == -1)
+    throw gdb_python_exception ();
+}
+
+/* Wrapper for PyBytes_FromString.  */
+static inline gdbpy_ref<>
+gdbpy_bytes_from_string (const char *str)
+{
+  gdb_assert (str != nullptr);
+  gdbpy_ref<> result (PyBytes_FromString (str));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyBytes_FromStringAndSize.  */
+static inline gdbpy_ref<>
+gdbpy_bytes_from_string_and_size (const char *str, Py_ssize_t len)
+{
+  /* Python allows STR==nullptr but it leaves the object
+     uninitialized, and I think we should avoid this in gdb.  */
+  gdb_assert (str != nullptr);
+  gdbpy_ref<> result (PyBytes_FromStringAndSize (str, len));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyBytes_Size.  */
+static inline Py_ssize_t
+gdbpy_bytes_size (gdbpy_borrowed_ref<> ref)
+{
+  Py_ssize_t result = PyBytes_Size (ref);
+  if (result == -1)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyList_New.  */
+static inline gdbpy_ref<>
+gdbpy_new_list (Py_ssize_t len)
+{
+  gdbpy_ref<> result (PyList_New (len));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyList_Append.  */
+static inline void
+gdbpy_list_append (gdbpy_borrowed_ref<> list, gdbpy_borrowed_ref<> val)
+{
+  if (PyList_Append (list, val) < 0)
+    throw gdb_python_exception ();
+}
+
+/* Wrapper for PyDict_New.  */
+static inline gdbpy_ref<>
+gdbpy_new_dict ()
+{
+  gdbpy_ref<> result (PyDict_New ());
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyDict_SetItemString.  */
+static inline void
+gdbpy_dict_set_item_string (gdbpy_borrowed_ref<> dict,
+			    const char *key,
+			    gdbpy_borrowed_ref<> value)
+{
+  if (PyDict_SetItemString (dict, key, value) != 0)
+    throw gdb_python_exception ();
+}
+
+/* Wrapper for PyDict_DelItemString.  */
+static inline void
+gdbpy_dict_del_item_string (gdbpy_borrowed_ref<> dict,
+			    const char *key)
+{
+  if (PyDict_DelItemString (dict, key) == -1)
+    throw gdb_python_exception ();
+}
+
+/* Wrapper for PyDict_GetItemWithError.  Note that this returns an
+   optional borrowed reference -- while it will throw an exception on
+   error, it will return NULL if the key is not in the dictionary.  */
+static inline gdbpy_opt_borrowed_ref<>
+gdbpy_dict_get_item_with_error (gdbpy_borrowed_ref<> dict,
+				gdbpy_borrowed_ref<> key)
+{
+  PyObject *result = PyDict_GetItemWithError (dict, key);
+  if (result == nullptr && PyErr_Occurred ())
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyDict_Keys.  */
+static inline gdbpy_ref<>
+gdbpy_dict_keys (gdbpy_borrowed_ref<> dict)
+{
+  gdbpy_ref<> result (PyDict_Keys (dict));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyUnicode_FromStringAndSize.  */
+static inline gdbpy_ref<>
+gdbpy_unicode_from_string (std::string_view str)
+{
+  gdbpy_ref<> result (PyUnicode_FromStringAndSize (str.data (), str.size ()));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyUnicode_FromFormatV.  A template function is used to
+   avoid issues with throwing across va_end.  */
+template<typename... Arg>
+gdbpy_ref<>
+gdbpy_unicode_from_format (const char *fmt, Arg... args)
+{
+  gdbpy_ref<> result (PyUnicode_FromFormat (fmt, args...));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyErr_SetString.  This always throws.  */
+[[noreturn]] static inline void
+gdbpy_err_set_string (gdbpy_borrowed_ref<> type, const char *str)
+{
+  PyErr_SetString (type, str);
+  throw gdb_python_exception ();
+}
+
+/* Wrapper for PyErr_Format.  This always throws.  */
+template<typename... Arg>
+[[noreturn]] void
+gdbpy_err_format (gdbpy_borrowed_ref<> type, const char *fmt, Arg... args)
+{
+  PyErr_Format (type, fmt, args...);
+  throw gdb_python_exception ();
+}
+
+/* Wrapper for gdb_PyArg_ParseTupleAndKeywords.  */
+template<typename... Arg>
+void
+gdbpy_arg_parse_tuple_and_keywords (gdbpy_borrowed_ref<> args,
+				    gdbpy_opt_borrowed_ref<> kw,
+				    const char *fmt,
+				    const char **keywords,
+				    Arg... outputs)
+{
+  /* It would be cool if callers could use references to the
+     out-parameters and also if gdbpy_borrowed_ref could be used for
+     those.  That requires some hairy template metaprogramming
+     though.  */
+  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, fmt, keywords, outputs...))
+    throw gdb_python_exception ();
+}
+
+/* Wrapper for PyArg_ParseTuple.  */
+template<typename... Arg>
+void
+gdbpy_arg_parse_tuple (gdbpy_borrowed_ref<> param, const char *format,
+		       Arg... args)
+{
+  if (!PyArg_ParseTuple (param, format, args...))
+    throw gdb_python_exception ();
+}
+
+/* Wrapper for PyLong_AsLong.  */
+static inline long
+gdbpy_long_as_long (gdbpy_borrowed_ref<> arg)
+{
+  long result = PyLong_AsLong (arg);
+  if (result == -1 && PyErr_Occurred ())
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyTuple_New.  */
+static inline gdbpy_ref<>
+gdbpy_tuple_new (Py_ssize_t len)
+{
+  gdbpy_ref<> result (PyTuple_New (len));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyTuple_GetItem.  */
+static inline gdbpy_borrowed_ref<>
+gdbpy_tuple_get_item (gdbpy_borrowed_ref<> tuple, Py_ssize_t pos)
+{
+  PyObject *result = PyTuple_GetItem (tuple, pos);
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PyTuple_SetItem.  */
+static inline void
+gdbpy_tuple_set_item (gdbpy_borrowed_ref<> tuple, Py_ssize_t pos,
+		      gdbpy_ref<> &&item)
+{
+  if (PyTuple_SetItem (tuple, pos, item.release ()) == -1)
+    throw gdb_python_exception ();
+}
+
+/* Wrapper for PyTuple_Size.  */
+static inline Py_ssize_t
+gdbpy_tuple_size (gdbpy_borrowed_ref<> tuple)
+{
+  Py_ssize_t result = PyTuple_Size (tuple);
+  if (result == -1)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PySequence_Size.  */
+static inline Py_ssize_t
+gdbpy_sequence_size (gdbpy_borrowed_ref<> seq)
+{
+  Py_ssize_t result = PySequence_Size (seq);
+  if (result == -1)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PySequence_GetItem.  */
+static inline gdbpy_ref<>
+gdbpy_sequence_get_item (gdbpy_borrowed_ref<> seq, Py_ssize_t i)
+{
+  gdbpy_ref<> result (PySequence_GetItem (seq, i));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PySequence_DelItem.  */
+static inline void
+gdbpy_sequence_del_item (gdbpy_borrowed_ref<> seq, Py_ssize_t i)
+{
+  if (PySequence_DelItem (seq, i) == -1)
+    throw gdb_python_exception ();
+}
+
+/* Wrapper for PySequence_List.  */
+static inline gdbpy_ref<>
+gdbpy_sequence_list (gdbpy_borrowed_ref<> seq)
+{
+  gdbpy_ref<> result (PySequence_List (seq));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+/* Wrapper for PySequence_Concat.  */
+static inline gdbpy_ref<>
+gdbpy_sequence_concat (gdbpy_borrowed_ref<> first, gdbpy_borrowed_ref<> second)
+{
+  gdbpy_ref<> result (PySequence_Concat (first, second));
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
+}
+
+#endif /* GDB_PYTHON_PY_WRAPPERS_H */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 82f3262ae07..3ec75ca08d5 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -1384,4 +1384,6 @@ py_notimplemented ()
 #undef Py_RETURN_FALSE
 #undef Py_RETURN_NOTIMPLEMENTED
 
+#include "py-wrappers.h"
+
 #endif /* GDB_PYTHON_PYTHON_INTERNAL_H */

-- 
2.49.0

[PATCH v3 3/4] Add wrappers for Python implementation functions and methods

[Tom Tromey]

This adds some wrappers for Python implementation functions and
methods, and a couple of new constexpr functions to create PyMethodDef
entries.  This provides a few safety benefits:

* The new-style API approach (see previous patch) is implemented by
  the wrapper.  That is, exceptions are caught here and transformed.

* The implementation functions can now return any reasonable type,
  with automatic conversion by the wrapper.

* The function API and the appropriate METH_* flags are handled
  together, avoiding any possible discrepancy.

This approach also means that we can modify the old rule that gdb
calls must be wrapped in a try/catch -- the try/catch is now provided
by the wrapper function, so the implementation can be written in a
more natural way.

Note that while this patch is usable as-is, it is not 100% complete,
in sense that there is still future work to do when converting other
parts of the gdb Python code.  For instance, there should be one more
wrapper for case where a method takes a single argument (though we
probably cannot use METH_O unfortunately).
---
 gdb/python/py-safety.h       | 346 +++++++++++++++++++++++++++++++++++++++++++
 gdb/python/python-internal.h |   1 +
 2 files changed, 347 insertions(+)

diff --git a/gdb/python/py-safety.h b/gdb/python/py-safety.h
new file mode 100644
index 00000000000..3294f38c8b6
--- /dev/null
+++ b/gdb/python/py-safety.h
@@ -0,0 +1,346 @@
+/* Wrappers for some Python safety.
+
+   Copyright (C) 2026 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 GDB_PYTHON_PY_SAFETY_H
+#define GDB_PYTHON_PY_SAFETY_H
+
+#include "gdbsupport/traits.h"
+#include "py-ref.h"
+#include "py-wrappers.h"
+#include "charset.h"
+
+/* This file holds wrapper templates for the various ways that gdb
+   code might be exposed to Python.  These wrappers are part of gdb's
+   "Python safety" approach -- utilities designed to try to prevent
+   refcount problems, missing error checks, and that also remove the
+   need to wrap calls into gdb in an explicit try/catch.
+
+   See py-wrappers.h for some more discussion of this.
+
+   Implementation functions and methods -- the stuff you write to
+   expose some part of gdb to Python -- are written in a certain
+   style.
+
+   An implementation function is a module-level function.  For example
+   something like 'gdb.register_window_type' is implemented as a
+   function.  An implementation method is a method of the gdb class
+   implementing a certain Python type; for example
+   'gdb.TuiWindow.width' is implemented via a method of
+   gdbpy_tui_window.
+
+   Each of these will accept gdbpy_borrowed_ref arguments (or in some
+   more limited situations, a gdbpy_opt_borrowed_ref) and return any
+   relevant type, which will be automatically converted (see the
+   'to_python' overloads below) to the correct Python type.  Note that
+   if the 'to_python' functions aren't appropriate in your case, you
+   can always use the escape hatch of returning a gdbpy_ref<> and
+   handling type conversion manually.
+
+   There are some constexpr functions below that wrap the
+   implementation methods, and create PyMethodDef objects and the
+   like.
+
+   Implementation methods are expected to use the wrappers in
+   py-wrappers.h and not generally call into Python directly.
+
+   Implementation details of the method-wrapping safety code are put
+   into this namespace, just to emphasize that these shouldn't be used
+   elsewhere.  Skip past the namespace to find the public APIs.  */
+namespace safety_details
+{
+/* Overloads of "to_python" are used by the safety wrappers to convert
+   a function's real return value to a Python object.  A new reference
+   will always be returned.  These are a detail of the method-wrapping
+   code.  Note that unlike the "safe" wrapper APIs that are commonly
+   used in gdb, these are all Python-facing and will return NULL on
+   error.  */
+
+static inline PyObject *
+to_python (bool value)
+{
+  return PyBool_FromLong (value);
+}
+
+template<typename T, typename = gdb::Requires<std::is_integral<T>>>
+static inline PyObject *
+to_python (T value)
+{
+  if constexpr (std::is_signed<T>::value)
+    return gdb_py_object_from_longest (value).release ();
+  else
+    return gdb_py_object_from_ulongest (value).release ();
+}
+
+static inline PyObject *
+to_python (const char *value)
+{
+  if (value == nullptr)
+    return py_none ().release ();
+  return PyUnicode_Decode (value, strlen (value), host_charset (), nullptr);
+}
+
+static inline PyObject *
+to_python (std::string &&value)
+{
+  return PyUnicode_Decode (value.c_str (), value.size (),
+			   host_charset (), nullptr);
+}
+
+static inline PyObject *
+to_python (const std::string &value)
+{
+  return PyUnicode_Decode (value.c_str (), value.size (),
+			   host_charset (), nullptr);
+}
+
+static inline PyObject *
+to_python (gdb::unique_xmalloc_ptr<char> &&value)
+{
+  if (value == nullptr)
+    return py_none ().release ();
+  return PyUnicode_Decode (value.get (), strlen (value.get ()),
+			   host_charset (), nullptr);
+}
+
+static inline PyObject *
+to_python (gdbpy_ref<> &&value)
+{
+  return value.release ();
+}
+
+/* An instantiation of this function is used when calling a gdb method
+   from Python.  It accepts some number of arguments (normally
+   gdbpy_borrowed_ref or gdbpy_opt_borrowed_ref), and then calls the
+   underlying function F.  Any exceptions are caught and converted,
+   and the return value of F is converted to a Python object as
+   appropriate.  */
+template<auto F, typename... Args>
+PyObject *
+wrapped_function (Args... args)
+{
+  try
+    {
+      using result_type = typename std::invoke_result_t<decltype (F), Args...>;
+
+      if constexpr (std::is_void_v<result_type>)
+	{
+	  F (args...);
+	  return py_none ().release ();
+	}
+      else
+	return to_python (F (args...));
+    }
+  catch (const gdb_python_exception &pye)
+    {
+      gdb_assert (PyErr_Occurred () != nullptr);
+      return nullptr;
+    }
+  catch (const gdb_exception &exc)
+    {
+      return gdbpy_handle_gdb_exception (nullptr, exc);
+    }
+}
+
+/* An instantiation of this function is used when calling a gdb method
+   from Python.  It accepts some number of arguments (normally
+   gdbpy_borrowed_ref or gdbpy_opt_borrowed_ref), and then calls the
+   method METH.  Any exceptions are caught and converted, and the
+   return value of METH is converted to a Python object as
+   appropriate.  */
+template<typename Class, typename Ret, typename... Args>
+PyObject *
+wrapped_method (Ret (Class::*meth) (Args...), Class *self, Args... args)
+{
+  try
+    {
+      if constexpr (std::is_void_v<Ret>)
+	{
+	  (self->*meth) (args...);
+	  return py_none ().release ();
+	}
+      else
+	return to_python ((self->*meth) (args...));
+    }
+  catch (const gdb_python_exception &pye)
+    {
+      gdb_assert (PyErr_Occurred () != nullptr);
+      return nullptr;
+    }
+  catch (const gdb_exception &exc)
+    {
+      return gdbpy_handle_gdb_exception (nullptr, exc);
+    }
+}
+
+/* A variant of wrapped_method that accepts a const method.  */
+template<typename Class, typename Ret, typename... Args>
+PyObject *
+wrapped_method (Ret (Class::*meth) (Args...) const, Class *self, Args... args)
+{
+  try
+    {
+      if constexpr (std::is_void_v<Ret>)
+	{
+	  (self->*meth) (args...);
+	  return py_none ().release ();
+	}
+      else
+	return to_python ((self->*meth) (args...));
+    }
+  catch (const gdb_python_exception &pye)
+    {
+      gdb_assert (PyErr_Occurred () != nullptr);
+      return nullptr;
+    }
+  catch (const gdb_exception &exc)
+    {
+      return gdbpy_handle_gdb_exception (nullptr, exc);
+    }
+}
+
+template<auto F>
+PyObject *
+fn_wrapper (PyObject *self, PyObject *args, PyObject *kw)
+{
+  return wrapped_function<F> (gdbpy_borrowed_ref<> (args),
+			      gdbpy_opt_borrowed_ref<> (kw));
+}
+
+template<typename C, auto M>
+PyObject *
+varargs_wrapper (PyObject *self, PyObject *args, PyObject *kw)
+{
+  return wrapped_method (M, static_cast<C *> (self),
+			 gdbpy_borrowed_ref<> (args),
+			 gdbpy_opt_borrowed_ref<> (kw));
+}
+
+} /* namespace safety_details */
+
+/* Create a PyMethodDef for a no-argument method.  It takes the
+   underlying class C and a pointer-to-method M as template
+   parameters, and the name and documentation as arguments.  The
+   method M is wrapped to call to_python and to catch exceptions per
+   the safety protocol.  */
+template<typename C, auto M>
+constexpr PyMethodDef
+noargs_method (const char *name, const char *doc)
+{
+  using namespace safety_details;
+  return {
+    name,
+    [] (PyObject *self, PyObject *args) -> PyObject *
+    {
+      return wrapped_method (M, static_cast<C *> (self));
+    },
+    METH_NOARGS,
+    doc,
+  };
+}
+
+/* This is used to create the PyMethodDef for a varargs function.  It
+   takes the underlying implementation function as a template
+   argument, and also arguments for the method name and documentation
+   string.
+
+   The underlying function should accept a gdbpy_borrowed_ref argument
+   (the arguments to the Python function), and then a
+   gdbpy_opt_borrowed_ref for the keywords.  The function can return
+   any type (see the to_python overloads); and should throw an
+   exception on error.  If gdb_python_exception is thrown, the Python
+   exception must already have been set.
+
+   The gdb policy is that varargs methods must also accept keywords,
+   and this is enforced here.  */
+template<auto F>
+constexpr PyMethodDef
+varargs_function (const char *name, const char *doc)
+{
+  using namespace safety_details;
+  return {
+    name,
+    (PyCFunction) fn_wrapper<F>,
+    /* gdb's rule is that varargs should also use keywords.  */
+    METH_VARARGS | METH_KEYWORDS,
+    doc,
+  };
+}
+
+/* Like a varargs function, but this implements a method on some
+   Python type that gdb provides.  The implementation class and a
+   pointer-to-method must be specified.  */
+template<typename C, auto M>
+constexpr PyMethodDef
+varargs_method (const char *name, const char *doc)
+{
+  using namespace safety_details;
+  return {
+    name,
+    (PyCFunction) varargs_wrapper<C, M>,
+    /* gdb's rule is that varargs should also use keywords.  */
+    METH_VARARGS | METH_KEYWORDS,
+    doc,
+  };
+}
+
+/* A function that wraps a "repr" or "str" method.  */
+template<typename C, auto M>
+PyObject *
+wrap_tp_callback (PyObject *arg)
+{
+  using namespace safety_details;
+  return wrapped_method (M, static_cast<C *> (arg));
+}
+
+/* A function that wraps a "get" method.  */
+template<typename C, auto M>
+PyObject *
+wrap_getter (PyObject *arg, void *closure)
+{
+  using namespace safety_details;
+  /* In gdb the closure argument is never used.  */
+  return wrapped_method (M, static_cast<C *> (arg));
+}
+
+/* A function that wraps a "set" method.  */
+template<typename C, void (C::*M) (gdbpy_opt_borrowed_ref<>)>
+int
+wrap_setter (PyObject *arg, PyObject *value, void *closure)
+{
+  using namespace safety_details;
+  try
+    {
+      C *self = static_cast<C *> (arg);
+      /* In gdb the closure argument is never used.  */
+      (self->*M) (gdbpy_opt_borrowed_ref<> (value));
+    }
+  catch (const gdb_python_exception &pye)
+    {
+      gdb_assert (PyErr_Occurred () != nullptr);
+      return -1;
+    }
+  catch (const gdb_exception &exc)
+    {
+      return gdbpy_handle_gdb_exception (-1, exc);
+    }
+
+  return 0;
+}
+
+#endif /* GDB_PYTHON_PY_SAFETY_H */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 3ec75ca08d5..c7a496899e0 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -1385,5 +1385,6 @@ py_notimplemented ()
 #undef Py_RETURN_NOTIMPLEMENTED
 
 #include "py-wrappers.h"
+#include "py-safety.h"
 
 #endif /* GDB_PYTHON_PYTHON_INTERNAL_H */

-- 
2.49.0

[PATCH v3 4/4] Convert py-tui.c to the "python safety" approach

[Tom Tromey]

This patch mostly converts py-tui.c to use the new Python safety code.

In particular:

* All methods of gdb.TuiWindow are now implemented as straightforward
  methods of gdbpy_tui_window.

* gdbpy_register_tui_window is converted and simply returns 'void'.

I converted this particular file because it was relatively
straightforward, while also demonstrating most of the features of the
new approach.  For example, explicit result checks aren't needed,
try/catch can be removed, and the methods are now written in a natural
style.

Note that more conversion remains to be done here:

* gdbpy_tui_enabled hasn't been converted and still does explicit
  checks.

* There's one explicit check in gdbpy_tui_window::set_title.  Fully
  implementing the safety approach means that some low-level things
  should eventually be converted to throw; but some work has to be
  deferred until a lot of the work is complete.
---
 gdb/python/py-tui.c          | 209 ++++++++++++++++++-------------------------
 gdb/python/python-internal.h |   5 +-
 gdb/python/python.c          |   5 +-
 3 files changed, 94 insertions(+), 125 deletions(-)

diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index ca0fb89ea49..5a36814debb 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -44,13 +44,40 @@ class tui_py_window;
 
 /* A PyObject representing a TUI window.  */
 
-struct gdbpy_tui_window: public PyObject
+struct gdbpy_tui_window : public PyObject
 {
   /* The TUI window, or nullptr if the window has been deleted.  */
   tui_py_window *window;
 
   /* Return true if this object is valid.  */
   bool is_valid () const;
+
+  /* Require that this object be valid.  Throws exception if not.  */
+  void require_valid () const
+  {
+    if (!is_valid ())
+      gdbpy_err_format (PyExc_RuntimeError, _("TUI window is invalid."));
+  }
+
+  /* Erase the TUI window.  */
+  void erase ();
+
+  /* Python function that writes some text to a TUI window.  */
+  void write (gdbpy_borrowed_ref<> args, gdbpy_opt_borrowed_ref<> kw);
+
+  /* Return the width of the TUI window.  */
+  int width () const;
+
+  /* Return the height of the TUI window.  */
+  int height () const;
+
+  /* Return the title of the TUI window.  */
+  const std::string &title () const;
+
+  /* Set the title of the TUI window.  */
+  void set_title (gdbpy_opt_borrowed_ref<> new_title);
+
+  static PyTypeObject *corresponding_object_type;
 };
 
 static_assert (gdb::is_python_allocatable_v<gdbpy_tui_window>);
@@ -408,173 +435,112 @@ gdbpy_tui_window_maker::operator() (const char *win_name)
 
 /* Implement "gdb.register_window_type".  */
 
-PyObject *
-gdbpy_register_tui_window (PyObject *self, PyObject *args, PyObject *kw)
+void
+gdbpy_register_tui_window (gdbpy_borrowed_ref<> args,
+			   gdbpy_opt_borrowed_ref<> kw)
 {
   static const char *keywords[] = { "name", "constructor", nullptr };
-
   const char *name;
   PyObject *cons_obj;
+  gdbpy_arg_parse_tuple_and_keywords (args, kw, "sO", keywords,
+				      &name, &cons_obj);
 
-  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "sO", keywords,
-					&name, &cons_obj))
-    return nullptr;
-
-  try
-    {
-      gdbpy_tui_window_maker constr (gdbpy_ref<>::new_reference (cons_obj));
-      tui_register_window (name, constr);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  return py_none ().release ();
+  gdbpy_tui_window_maker constr (gdbpy_ref<>::new_reference (cons_obj));
+  tui_register_window (name, constr);
 }
 
 \f
 
-/* Require that "Window" be a valid window.  */
-
-#define REQUIRE_WINDOW(Window)					\
-    do {							\
-      if (!(Window)->is_valid ())				\
-	return PyErr_Format (PyExc_RuntimeError,		\
-			     _("TUI window is invalid."));	\
-    } while (0)
-
-/* Require that "Window" be a valid window.  */
-
-#define REQUIRE_WINDOW_FOR_SETTER(Window)			\
-    do {							\
-      if (!(Window)->is_valid ())				\
-	{							\
-	  PyErr_Format (PyExc_RuntimeError,			\
-			_("TUI window is invalid."));		\
-	  return -1;						\
-	}							\
-    } while (0)
-
-/* Python function which checks the validity of a TUI window
-   object.  */
-static PyObject *
-gdbpy_tui_is_valid (PyObject *self, PyObject *args)
-{
-  gdbpy_tui_window *win = (gdbpy_tui_window *) self;
-
-  if (win->is_valid ())
-    return py_true ().release ();
-  return py_false ().release ();
-}
-
 /* Python function that erases the TUI window.  */
-static PyObject *
-gdbpy_tui_erase (PyObject *self, PyObject *args)
+void
+gdbpy_tui_window::erase ()
 {
-  gdbpy_tui_window *win = (gdbpy_tui_window *) self;
-
-  REQUIRE_WINDOW (win);
-
-  win->window->erase ();
-
-  return py_none ().release ();
+  require_valid ();
+  window->erase ();
 }
 
 /* Python function that writes some text to a TUI window.  */
-static PyObject *
-gdbpy_tui_write (PyObject *self, PyObject *args, PyObject *kw)
+void
+gdbpy_tui_window::write (gdbpy_borrowed_ref<> args,
+			 gdbpy_opt_borrowed_ref<> kw)
 {
+  require_valid ();
+
   static const char *keywords[] = { "string", "full_window", nullptr };
 
-  gdbpy_tui_window *win = (gdbpy_tui_window *) self;
   const char *text;
   int full_window = 0;
 
-  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords,
-					&text, &full_window))
-    return nullptr;
-
-  REQUIRE_WINDOW (win);
+  gdbpy_arg_parse_tuple_and_keywords (args, kw, "s|i", keywords,
+				      &text, &full_window);
 
-  win->window->output (text, full_window);
-
-  return py_none ().release ();
+  window->output (text, full_window);
 }
 
-/* Return the width of the TUI window.  */
-static PyObject *
-gdbpy_tui_width (PyObject *self, void *closure)
+int
+gdbpy_tui_window::width () const
 {
-  gdbpy_tui_window *win = (gdbpy_tui_window *) self;
-  REQUIRE_WINDOW (win);
-  gdbpy_ref<> result
-    = gdb_py_object_from_longest (win->window->viewport_width ());
-  return result.release ();
+  require_valid ();
+  return window->viewport_width ();
 }
 
-/* Return the height of the TUI window.  */
-static PyObject *
-gdbpy_tui_height (PyObject *self, void *closure)
+int
+gdbpy_tui_window::height () const
 {
-  gdbpy_tui_window *win = (gdbpy_tui_window *) self;
-  REQUIRE_WINDOW (win);
-  gdbpy_ref<> result
-    = gdb_py_object_from_longest (win->window->viewport_height ());
-  return result.release ();
+  require_valid ();
+  return window->viewport_height ();
 }
 
-/* Return the title of the TUI window.  */
-static PyObject *
-gdbpy_tui_title (PyObject *self, void *closure)
+const std::string &
+gdbpy_tui_window::title () const
 {
-  gdbpy_tui_window *win = (gdbpy_tui_window *) self;
-  REQUIRE_WINDOW (win);
-  return host_string_to_python_string (win->window->title ().c_str ()).release ();
+  require_valid ();
+  return window->title ();
 }
 
 /* Set the title of the TUI window.  */
-static int
-gdbpy_tui_set_title (PyObject *self, PyObject *newvalue, void *closure)
+void
+gdbpy_tui_window::set_title (gdbpy_opt_borrowed_ref<> new_title)
 {
-  gdbpy_tui_window *win = (gdbpy_tui_window *) self;
+  require_valid ();
 
-  REQUIRE_WINDOW_FOR_SETTER (win);
-
-  if (newvalue == nullptr)
-    {
-      PyErr_Format (PyExc_TypeError, _("Cannot delete \"title\" attribute."));
-      return -1;
-    }
+  if (new_title == nullptr)
+    gdbpy_err_format (PyExc_TypeError,
+		      _("Cannot delete \"title\" attribute."));
 
+  /* FIXME: Python safety.  Ideally python_string_to_host_string would
+     throw on error, but this can't be done until more of the code has
+     been converted.  */
   gdb::unique_xmalloc_ptr<char> value
-    = python_string_to_host_string (newvalue);
+    = python_string_to_host_string (new_title);
   if (value == nullptr)
-    return -1;
+    throw gdb_python_exception ();
 
-  win->window->set_title (value.get ());
-  return 0;
+  window->set_title (value.get ());
 }
 
 static gdb_PyGetSetDef tui_object_getset[] =
 {
-  { "width", gdbpy_tui_width, NULL, "Width of the window.", NULL },
-  { "height", gdbpy_tui_height, NULL, "Height of the window.", NULL },
-  { "title", gdbpy_tui_title, gdbpy_tui_set_title, "Title of the window.",
-    NULL },
-  { NULL }  /* Sentinel */
+  { "width", wrap_getter<gdbpy_tui_window, &gdbpy_tui_window::width>, nullptr,
+    "Width of the window.", nullptr },
+  { "height", wrap_getter<gdbpy_tui_window, &gdbpy_tui_window::height>, nullptr,
+    "Height of the window.", nullptr },
+  { "title", wrap_getter<gdbpy_tui_window, &gdbpy_tui_window::title>,
+    wrap_setter<gdbpy_tui_window, &gdbpy_tui_window::set_title>,
+    "Title of the window.", nullptr },
+  { nullptr }  /* Sentinel */
 };
 
 static PyMethodDef tui_object_methods[] =
 {
-  { "is_valid", gdbpy_tui_is_valid, METH_NOARGS,
+  noargs_method<gdbpy_tui_window, &gdbpy_tui_window::is_valid> ("is_valid",
     "is_valid () -> Boolean\n\
-Return true if this TUI window is valid, false if not." },
-  { "erase", gdbpy_tui_erase, METH_NOARGS,
-    "Erase the TUI window." },
-  { "write", (PyCFunction) gdbpy_tui_write, METH_VARARGS | METH_KEYWORDS,
-    "Append a string to the TUI window." },
-  { NULL } /* Sentinel.  */
+Return true if this TUI window is valid, false if not."),
+  noargs_method<gdbpy_tui_window, &gdbpy_tui_window::erase>
+    ("erase", "Erase the TUI window."),
+  varargs_method<gdbpy_tui_window, &gdbpy_tui_window::write> ("write",
+    "Append a string to the TUI window."),
+  { nullptr } /* Sentinel.  */
 };
 
 PyTypeObject gdbpy_tui_window_object_type =
@@ -618,6 +584,9 @@ PyTypeObject gdbpy_tui_window_object_type =
   0,				  /* tp_alloc */
 };
 
+PyTypeObject *gdbpy_tui_window::corresponding_object_type
+    = &gdbpy_tui_window_object_type;
+
 /* Called when TUI is enabled or disabled.  */
 
 static void
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index c7a496899e0..686ae63c315 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -498,8 +498,9 @@ gdb::unique_xmalloc_ptr<char> gdbpy_parse_command_name
   (const char *name, struct cmd_list_element ***base_list,
    struct cmd_list_element **start_list,
    struct cmd_list_element **prefix_cmd = nullptr);
-PyObject *gdbpy_register_tui_window (PyObject *self, PyObject *args,
-				     PyObject *kw);
+
+void gdbpy_register_tui_window (gdbpy_borrowed_ref<> args,
+				gdbpy_opt_borrowed_ref<> kw);
 
 gdbpy_ref<> symtab_and_line_to_sal_object (struct symtab_and_line sal);
 gdbpy_ref<> symtab_to_symtab_object (struct symtab *symtab);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index fd254a340d8..849154ff20c 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -3274,10 +3274,9 @@ or None if not set." },
 Set the value of the convenience variable $NAME." },
 
 #ifdef TUI
-  { "register_window_type", (PyCFunction) gdbpy_register_tui_window,
-    METH_VARARGS | METH_KEYWORDS,
+  varargs_function<gdbpy_register_tui_window> ("register_window_type",
     "register_window_type (NAME, CONSTRUCTOR) -> None\n\
-Register a TUI window constructor." },
+Register a TUI window constructor."),
 #endif	/* TUI */
 
   { "architecture_names", gdbpy_all_architecture_names, METH_NOARGS,

-- 
2.49.0

Re: [PATCH v3 0/4] Python safety initial work

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> I rewrote my earlier Python safety series to use methods a while back.
Tom> And while I like the result, I got bogged down converting things, so I
Tom> thought I'd send some initial work, with the idea that if this looks
Tom> good, other code can be transformed in pieces.

Ping.

Tom

Re: [PATCH v3 0/4] Python safety initial work

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
Tom> I rewrote my earlier Python safety series to use methods a while back.
Tom> And while I like the result, I got bogged down converting things, so I
Tom> thought I'd send some initial work, with the idea that if this looks
Tom> good, other code can be transformed in pieces.

Tom> Ping.

Ping again - any thoughts on this?

Tom

Re: [PATCH v3 0/4] Python safety initial work

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

On 7/19/26 7:16 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> Tom> I rewrote my earlier Python safety series to use methods a while back.
> Tom> And while I like the result, I got bogged down converting things, so I
> Tom> thought I'd send some initial work, with the idea that if this looks
> Tom> good, other code can be transformed in pieces.
> 
> Tom> Ping.
> 
> Ping again - any thoughts on this?
> 
> Tom

I applied the series and tried to use it.

I found this code:
...
       /* Note this returns a borrowed reference.  */
       PyObject *arg = PyTuple_GetItem (args, i);
...
and decided to try to convert all PyTuple_GetItem calls.  The result of 
that exercise is attached.

I ended up also touching the wrapper function:
...
  static inline gdbpy_borrowed_ref<>
  gdbpy_tuple_get_item (gdbpy_borrowed_ref<> tuple, Py_ssize_t pos)
  {
-  PyObject *result = PyTuple_GetItem (tuple, pos);
+  gdbpy_opt_borrowed_ref<> result = PyTuple_GetItem (tuple, pos);
    if (result == nullptr)
      throw gdb_python_exception ();
-  return result;
+  return (PyObject *)result;
  }
...
but perhaps you left that out intentionally?

Anyway, having done this exercise, I like the idea of making borrowed 
refs explicit in a type.

I haven't used the gdbpy_tuple_get_item wrapper, AFAICT it was not 
applicable.

Acked-By: Tom de Vries <tdevries@suse.de>

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-python-Use-gdbpy_borrowed_ref-for-PyTuple_GetIte.patch --]
[-- Type: text/x-patch, Size: 4186 bytes --]

From c14d61964e1288951ab2bf020d7d75de7b33aaa0 Mon Sep 17 00:00:00 2001
From: Tom de Vries <tdevries@suse.de>
Date: Tue, 21 Jul 2026 12:44:12 +0200
Subject: [PATCH] [gdb/python] Use gdbpy_borrowed_ref for PyTuple_GetItem

Use gdbpy_borrowed_ref and gdbpy_opt_borrowed_ref as return type of
PyTuple_GetItem.
---
 gdb/python/py-color.c    |  2 +-
 gdb/python/py-mi.c       |  3 +--
 gdb/python/py-unwind.c   | 10 +++++-----
 gdb/python/py-value.c    |  2 +-
 gdb/python/py-wrappers.h |  4 ++--
 5 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
index 29a42977adc..62360333f8b 100644
--- a/gdb/python/py-color.c
+++ b/gdb/python/py-color.c
@@ -231,7 +231,7 @@ colorpy_init (PyObject *self, PyObject *args, PyObject *kwds)
 	  uint8_t rgb[3];
 	  for (int i = 0; i < 3; ++i)
 	    {
-	      PyObject *item = PyTuple_GetItem (value_obj, i);
+	      gdbpy_borrowed_ref<> item = PyTuple_GetItem (value_obj, i);
 	      if (!PyLong_Check (item))
 		error (_("Item %d of an RGB tuple must be integer."), i);
 	      long item_value = -1;
diff --git a/gdb/python/py-mi.c b/gdb/python/py-mi.c
index 73daefecb20..5f14cefb134 100644
--- a/gdb/python/py-mi.c
+++ b/gdb/python/py-mi.c
@@ -152,8 +152,7 @@ gdbpy_execute_mi_command (PyObject *self, PyObject *args, PyObject *kw)
 
   for (Py_ssize_t i = 0; i < n_args; ++i)
     {
-      /* Note this returns a borrowed reference.  */
-      PyObject *arg = PyTuple_GetItem (args, i);
+      gdbpy_opt_borrowed_ref<> arg = PyTuple_GetItem (args, i);
       if (arg == nullptr)
 	return nullptr;
       gdb::unique_xmalloc_ptr<char> str = python_string_to_host_string (arg);
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 13eced4cecb..279ee922d4e 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -918,8 +918,8 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
 
   if (pyuw_debug)
     {
-      PyObject *pyo_unwinder_name = PyTuple_GetItem (pyo_execute_ret.get (), 1);
-      gdb_assert (pyo_unwinder_name != nullptr);
+      gdbpy_borrowed_ref<> pyo_unwinder_name
+	= PyTuple_GetItem (pyo_execute_ret.get (), 1);
       gdb::unique_xmalloc_ptr<char> name
 	= python_string_to_host_string (pyo_unwinder_name);
 
@@ -935,15 +935,15 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
     }
 
   /* Received UnwindInfo, cache data.  */
-  PyObject *pyo_unwind_info = PyTuple_GetItem (pyo_execute_ret.get (), 0);
-  gdb_assert (pyo_unwind_info != nullptr);
+  gdbpy_borrowed_ref<> pyo_unwind_info
+    = PyTuple_GetItem (pyo_execute_ret.get (), 0);
   if (!PyObject_TypeCheck (pyo_unwind_info, &unwind_info_object_type))
     error (_("an Unwinder should return gdb.UnwindInfo, not %s."),
 	   gdbpy_py_obj_tp_name (pyo_unwind_info).c_str ());
 
   {
     unwind_info_object *unwind_info =
-      (unwind_info_object *) pyo_unwind_info;
+      (unwind_info_object *) (PyObject *)pyo_unwind_info;
     int reg_count = unwind_info->saved_regs->size ();
 
     cached_frame
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 5b38110396e..0525f4b8bc9 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -1208,7 +1208,7 @@ valpy_call (PyObject *self, PyObject *args, PyObject *keywords)
       vargs = XALLOCAVEC (struct value *, args_count);
       for (i = 0; i < args_count; i++)
 	{
-	  PyObject *item = PyTuple_GetItem (args, i);
+	  gdbpy_opt_borrowed_ref<> item = PyTuple_GetItem (args, i);
 
 	  if (item == NULL)
 	    return NULL;
diff --git a/gdb/python/py-wrappers.h b/gdb/python/py-wrappers.h
index 6c2b5e4d41e..be0a0e31ef5 100644
--- a/gdb/python/py-wrappers.h
+++ b/gdb/python/py-wrappers.h
@@ -285,10 +285,10 @@ gdbpy_tuple_new (Py_ssize_t len)
 static inline gdbpy_borrowed_ref<>
 gdbpy_tuple_get_item (gdbpy_borrowed_ref<> tuple, Py_ssize_t pos)
 {
-  PyObject *result = PyTuple_GetItem (tuple, pos);
+  gdbpy_opt_borrowed_ref<> result = PyTuple_GetItem (tuple, pos);
   if (result == nullptr)
     throw gdb_python_exception ();
-  return result;
+  return (PyObject *)result;
 }
 
 /* Wrapper for PyTuple_SetItem.  */

base-commit: 9036979ff84d387211315801a2b0611f7b716ea0
-- 
2.51.0

Re: [PATCH v3 0/4] Python safety initial work

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> I found this code:
Tom> ...
Tom>       /* Note this returns a borrowed reference.  */
Tom>       PyObject *arg = PyTuple_GetItem (args, i);
Tom> ...
Tom> and decided to try to convert all PyTuple_GetItem calls.  The result
Tom> of that exercise is attached.

Tom> I ended up also touching the wrapper function:
Tom> ...
Tom>  static inline gdbpy_borrowed_ref<>
Tom>  gdbpy_tuple_get_item (gdbpy_borrowed_ref<> tuple, Py_ssize_t pos)
Tom>  {
Tom> -  PyObject *result = PyTuple_GetItem (tuple, pos);
Tom> +  gdbpy_opt_borrowed_ref<> result = PyTuple_GetItem (tuple, pos);
Tom>    if (result == nullptr)
Tom>      throw gdb_python_exception ();
Tom> -  return result;
Tom> +  return (PyObject *)result;
Tom>  }
Tom> ...
Tom> but perhaps you left that out intentionally?

Yeah, in this spot it didn't seem to really be necessary.
The idea of the wrapper file is to eventually isolate all "raw" calls to
the Python API.

Tom> I haven't used the gdbpy_tuple_get_item wrapper, AFAICT it was not
Tom> applicable.

The long term goal is to convert everything to call the wrappers.
This way exception handling will be enforced.

Tom

Re: [PATCH v3 0/4] Python safety initial work

[Tom de Vries]

On 8/7/26 9:48 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> I found this code:
> Tom> ...
> Tom>       /* Note this returns a borrowed reference.  */
> Tom>       PyObject *arg = PyTuple_GetItem (args, i);
> Tom> ...
> Tom> and decided to try to convert all PyTuple_GetItem calls.  The result
> Tom> of that exercise is attached.
> 
> Tom> I ended up also touching the wrapper function:
> Tom> ...
> Tom>  static inline gdbpy_borrowed_ref<>
> Tom>  gdbpy_tuple_get_item (gdbpy_borrowed_ref<> tuple, Py_ssize_t pos)
> Tom>  {
> Tom> -  PyObject *result = PyTuple_GetItem (tuple, pos);
> Tom> +  gdbpy_opt_borrowed_ref<> result = PyTuple_GetItem (tuple, pos);
> Tom>    if (result == nullptr)
> Tom>      throw gdb_python_exception ();
> Tom> -  return result;
> Tom> +  return (PyObject *)result;
> Tom>  }
> Tom> ...
> Tom> but perhaps you left that out intentionally?
> 
> Yeah, in this spot it didn't seem to really be necessary.
> The idea of the wrapper file is to eventually isolate all "raw" calls to
> the Python API.
> 
> Tom> I haven't used the gdbpy_tuple_get_item wrapper, AFAICT it was not
> Tom> applicable.
> 
> The long term goal is to convert everything to call the wrappers.
> This way exception handling will be enforced.

Ok, I've started over and converted valpy_call, now indeed using 
gdbpy_tuple_get_item.

Thanks,
- Tom

Re: [PATCH v3 0/4] Python safety initial work

[Tom Tromey]

Tom> Anyway, having done this exercise, I like the idea of making borrowed
Tom> refs explicit in a type.
...
Tom> Acked-By: Tom de Vries <tdevries@suse.de>

FWIW I am going to check this in now.

Tom