[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

[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 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 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 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

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 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