Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 00/10] Mildly better refcount safety for Python
@ 2026-02-20 21:03 Tom Tromey
  2026-02-20 21:03 ` [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object Tom Tromey
                   ` (10 more replies)
  0 siblings, 11 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This series converts a number of gdb-to-Python converion functions to
return a gdbpy_ref<> rather than returning a PyObject* and then simply
documenting that a new reference is returned.

The hope is that this will make refcount mistakes less likely.

Regression tested on x86-64 Fedora 43.

Signed-off-by: Tom Tromey <tromey@adacore.com>
---
Tom Tromey (10):
      Return gdbpy_ref<> from symtab_and_line_to_sal_object
      Return gdbpy_ref<> from symbol_to_symbol_object
      Return gdbpy_ref<> from symtab_to_symtab_object
      Return gdbpy_ref<> from block_to_block_object
      Return gdbpy_ref<> from value_to_value_object
      Return gdbpy_ref<> from type_to_type_object
      Return gdbpy_ref<> from frame_info_to_frame_object
      Return gdbpy_ref<> from symtab_to_linetable_object
      Return gdbpy_ref<> from gdbarch_to_arch_object
      Return gdbpy_ref<> from gdbpy_registry::lookup

 gdb/python/py-arch.c             |  8 ++---
 gdb/python/py-block.c            | 26 ++++++--------
 gdb/python/py-disasm.c           |  2 +-
 gdb/python/py-finishbreakpoint.c |  6 ++--
 gdb/python/py-frame.c            | 47 ++++++++++++-------------
 gdb/python/py-framefilter.c      |  2 +-
 gdb/python/py-function.c         |  2 +-
 gdb/python/py-inferior.c         |  2 +-
 gdb/python/py-infevents.c        |  2 +-
 gdb/python/py-lazy-string.c      |  6 ++--
 gdb/python/py-linetable.c        |  4 +--
 gdb/python/py-objfile.c          |  4 +--
 gdb/python/py-prettyprint.c      |  4 +--
 gdb/python/py-progspace.c        |  6 ++--
 gdb/python/py-record-btrace.c    |  6 ++--
 gdb/python/py-stopevent.c        |  2 +-
 gdb/python/py-symbol.c           | 70 +++++++++++++++++--------------------
 gdb/python/py-symtab.c           | 26 +++++++-------
 gdb/python/py-type.c             | 59 ++++++++++++++++----------------
 gdb/python/py-unwind.c           | 15 ++++----
 gdb/python/py-value.c            | 74 +++++++++++++++++++---------------------
 gdb/python/py-xmethods.c         | 18 +++++-----
 gdb/python/python-internal.h     | 27 +++++++--------
 gdb/python/python.c              | 16 ++++-----
 24 files changed, 207 insertions(+), 227 deletions(-)
---
base-commit: e16c7848007e4faa071a6483556e69f365e9eb69
change-id: 20260220-python-safety-minor-9eeaef26c39a

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-21  1:58   ` Simon Marchi
  2026-02-20 21:03 ` [PATCH 02/10] Return gdbpy_ref<> from symbol_to_symbol_object Tom Tromey
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes symtab_and_line_to_sal_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
---
 gdb/python/py-frame.c         | 4 ++--
 gdb/python/py-progspace.c     | 4 ++--
 gdb/python/py-record-btrace.c | 4 ++--
 gdb/python/py-symtab.c        | 4 ++--
 gdb/python/py-unwind.c        | 4 ++--
 gdb/python/python-internal.h  | 2 +-
 gdb/python/python.c           | 8 ++++----
 7 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index ab7883b4e73..8b50307c099 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -466,7 +466,7 @@ static PyObject *
 frapy_find_sal (PyObject *self, PyObject *args)
 {
   frame_info_ptr frame;
-  PyObject *sal_obj = NULL;   /* Initialize to appease gcc warning.  */
+  gdbpy_ref<> sal_obj;
 
   try
     {
@@ -480,7 +480,7 @@ frapy_find_sal (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return sal_obj;
+  return sal_obj.release ();
 }
 
 /* Implementation of gdb.Frame.read_var_value (self, variable,
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 19f5e533b0a..8b5ada2dc2e 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -534,7 +534,7 @@ static PyObject *
 pspy_find_pc_line (PyObject *o, PyObject *args)
 {
   CORE_ADDR pc;
-  PyObject *result = NULL; /* init for gcc -Wall */
+  gdbpy_ref<> result;
   PyObject *pc_obj;
   pspace_object *self = (pspace_object *) o;
 
@@ -560,7 +560,7 @@ pspy_find_pc_line (PyObject *o, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Implementation of is_valid (self) -> Boolean.
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 3fcf653f4ba..90d9ecd95a8 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -205,7 +205,7 @@ PyObject *
 recpy_bt_insn_sal (PyObject *self, void *closure)
 {
   const btrace_insn * const insn = btrace_insn_from_recpy_insn (self);
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   if (insn == NULL)
     return NULL;
@@ -219,7 +219,7 @@ recpy_bt_insn_sal (PyObject *self, void *closure)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Implementation of RecordInstruction.pc [int] for btrace.
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 2dca0083277..820542932ba 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -390,7 +390,7 @@ symtab_to_symtab_object (struct symtab *symtab)
 
 /* Create a new symtab and line (gdb.Symtab_and_line) object
    that encapsulates the symtab_and_line structure from GDB.  */
-PyObject *
+gdbpy_ref<>
 symtab_and_line_to_sal_object (struct symtab_and_line sal)
 {
   sal_object *sal_obj;
@@ -399,7 +399,7 @@ symtab_and_line_to_sal_object (struct symtab_and_line sal)
   if (sal_obj != nullptr)
     set_sal (sal_obj, sal);
 
-  return (PyObject *) sal_obj;
+  return gdbpy_ref<> (sal_obj);
 }
 
 /* Return struct symtab_and_line reference that is wrapped by this
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index d4c35e17f14..96e11137be6 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -613,7 +613,7 @@ pending_framepy_find_sal (PyObject *self, PyObject *args)
 
   PENDING_FRAMEPY_REQUIRE_VALID (pending_frame);
 
-  PyObject *sal_obj = nullptr;
+  gdbpy_ref<> sal_obj;
 
   try
     {
@@ -627,7 +627,7 @@ pending_framepy_find_sal (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return sal_obj;
+  return sal_obj.release ();
 }
 
 /* Implement PendingFrame.block().  Return a gdb.Block for the pending
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 8925714f3c1..feabf270464 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -495,7 +495,7 @@ gdb::unique_xmalloc_ptr<char> gdbpy_parse_command_name
 PyObject *gdbpy_register_tui_window (PyObject *self, PyObject *args,
 				     PyObject *kw);
 
-PyObject *symtab_and_line_to_sal_object (struct symtab_and_line sal);
+gdbpy_ref<> symtab_and_line_to_sal_object (struct symtab_and_line sal);
 PyObject *symtab_to_symtab_object (struct symtab *symtab);
 PyObject *symbol_to_symbol_object (struct symbol *sym);
 PyObject *block_to_block_object (const struct block *block,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 79b5d24d1cf..d2a77890072 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1020,11 +1020,11 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
 	return NULL;
       for (size_t i = 0; i < sals.size (); ++i)
 	{
-	  PyObject *obj = symtab_and_line_to_sal_object (sals[i]);
-	  if (obj == NULL)
-	    return NULL;
+	  gdbpy_ref<> obj = symtab_and_line_to_sal_object (sals[i]);
+	  if (obj == nullptr)
+	    return nullptr;
 
-	  PyTuple_SetItem (result.get (), i, obj);
+	  PyTuple_SetItem (result.get (), i, obj.release ());
 	}
     }
   else

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 02/10] Return gdbpy_ref<> from symbol_to_symbol_object
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
  2026-02-20 21:03 ` [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-21  2:28   ` Simon Marchi
  2026-02-20 21:03 ` [PATCH 03/10] Return gdbpy_ref<> from symtab_to_symtab_object Tom Tromey
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes symbol_to_symbol_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
---
 gdb/python/py-block.c            |  6 ++---
 gdb/python/py-finishbreakpoint.c |  2 +-
 gdb/python/py-frame.c            |  2 +-
 gdb/python/py-objfile.c          |  4 ++--
 gdb/python/py-record-btrace.c    |  2 +-
 gdb/python/py-symbol.c           | 51 +++++++++++++++++-----------------------
 gdb/python/py-unwind.c           |  2 +-
 gdb/python/python-internal.h     |  2 +-
 8 files changed, 32 insertions(+), 39 deletions(-)

diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index bebbb32d844..718aca17829 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -127,7 +127,7 @@ blpy_get_function (PyObject *self, void *closure)
 
   sym = block->function ();
   if (sym)
-    return symbol_to_symbol_object (sym);
+    return symbol_to_symbol_object (sym).release ();
 
   Py_RETURN_NONE;
 }
@@ -274,7 +274,7 @@ blpy_getitem (PyObject *self, PyObject *key)
   for (struct symbol *sym : block_iterator_range (block, &lookup_name))
     {
       /* Just stop at the first match */
-      return symbol_to_symbol_object (sym);
+      return symbol_to_symbol_object (sym).release ();
     }
 
   PyErr_SetObject (PyExc_KeyError, key);
@@ -412,7 +412,7 @@ blpy_block_syms_iternext (PyObject *self)
       return NULL;
     }
 
-  return symbol_to_symbol_object (sym);
+  return symbol_to_symbol_object (sym).release ();
 }
 
 static void
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 14e9f18c863..43dffd64adc 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -268,7 +268,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 		  PyErr_Clear ();
 
 		  self_bpfinish->func_symbol
-		    = symbol_to_symbol_object (function);
+		    = symbol_to_symbol_object (function).release ();
 		  PyErr_Clear ();
 		}
 	    }
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 8b50307c099..e98edd52cfb 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -351,7 +351,7 @@ frapy_function (PyObject *self, PyObject *args)
     }
 
   if (sym)
-    return symbol_to_symbol_object (sym);
+    return symbol_to_symbol_object (sym).release ();
 
   Py_RETURN_NONE;
 }
diff --git a/gdb/python/py-objfile.c b/gdb/python/py-objfile.c
index 36acdb06a25..16584f7c8f8 100644
--- a/gdb/python/py-objfile.c
+++ b/gdb/python/py-objfile.c
@@ -477,7 +477,7 @@ objfpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw)
       if (sym == nullptr)
 	Py_RETURN_NONE;
 
-      return symbol_to_symbol_object (sym);
+      return symbol_to_symbol_object (sym).release ();
     }
   catch (const gdb_exception &except)
     {
@@ -512,7 +512,7 @@ objfpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
       if (sym == nullptr)
 	Py_RETURN_NONE;
 
-      return symbol_to_symbol_object (sym);
+      return symbol_to_symbol_object (sym).release ();
     }
   catch (const gdb_exception &except)
     {
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index 90d9ecd95a8..1974dd3e939 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -354,7 +354,7 @@ recpy_bt_func_symbol (PyObject *self, void *closure)
   if (func->sym == NULL)
     Py_RETURN_NONE;
 
-  return symbol_to_symbol_object (func->sym);
+  return symbol_to_symbol_object (func->sym).release ();
 }
 
 /* Implementation of RecordFunctionSegment.instructions [list] for btrace.
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 736774bd94f..d51f4823315 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -336,7 +336,7 @@ set_symbol (symbol_object *obj, struct symbol *symbol)
 
 /* Create a new symbol object (gdb.Symbol) that encapsulates the struct
    symbol object from GDB.  */
-PyObject *
+gdbpy_ref<>
 symbol_to_symbol_object (struct symbol *sym)
 {
   symbol_object *sym_obj;
@@ -348,13 +348,13 @@ symbol_to_symbol_object (struct symbol *sym)
   else
     sym_obj = sympy_registry.lookup (sym->arch (), sym);
   if (sym_obj != nullptr)
-    return (PyObject*)sym_obj;
+    return gdbpy_ref<> (sym_obj);
 
   sym_obj = PyObject_New (symbol_object, &symbol_object_type);
   if (sym_obj)
     set_symbol (sym_obj, sym);
 
-  return (PyObject *) sym_obj;
+  return gdbpy_ref<> (sym_obj);
 }
 
 /* Return the symbol that is wrapped by this symbol object.  */
@@ -409,7 +409,7 @@ gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw)
   const char *name;
   static const char *keywords[] = { "name", "block", "domain", NULL };
   struct symbol *symbol = NULL;
-  PyObject *block_obj = NULL, *sym_obj, *bool_obj;
+  PyObject *block_obj = NULL, *bool_obj;
   const struct block *block = NULL;
 
   if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!i", keywords, &name,
@@ -448,18 +448,16 @@ gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw)
   if (ret_tuple == NULL)
     return NULL;
 
+  gdbpy_ref<> sym_obj;
   if (symbol)
     {
       sym_obj = symbol_to_symbol_object (symbol);
-      if (!sym_obj)
-	return NULL;
+      if (sym_obj == nullptr)
+	return nullptr;
     }
   else
-    {
-      sym_obj = Py_None;
-      Py_INCREF (Py_None);
-    }
-  PyTuple_SET_ITEM (ret_tuple.get (), 0, sym_obj);
+    sym_obj = gdbpy_ref<>::new_reference (Py_None);
+  PyTuple_SET_ITEM (ret_tuple.get (), 0, sym_obj.release ());
 
   bool_obj = PyBool_FromLong (is_a_field_of_this.type != NULL);
   PyTuple_SET_ITEM (ret_tuple.get (), 1, bool_obj);
@@ -477,7 +475,6 @@ gdbpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw)
   const char *name;
   static const char *keywords[] = { "name", "domain", NULL };
   struct symbol *symbol = NULL;
-  PyObject *sym_obj;
 
   if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
 					&domain))
@@ -493,19 +490,17 @@ gdbpy_lookup_global_symbol (PyObject *self, PyObject *args, PyObject *kw)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
+  gdbpy_ref<> sym_obj;
   if (symbol)
     {
       sym_obj = symbol_to_symbol_object (symbol);
-      if (!sym_obj)
-	return NULL;
+      if (sym_obj == nullptr)
+	return nullptr;
     }
   else
-    {
-      sym_obj = Py_None;
-      Py_INCREF (Py_None);
-    }
+    sym_obj = gdbpy_ref<>::new_reference (Py_None);
 
-  return sym_obj;
+  return sym_obj.release ();
 }
 
 /* Implementation of
@@ -518,7 +513,6 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
   int domain = VAR_DOMAIN;
   static const char *keywords[] = { "name", "domain", NULL };
   struct symbol *symbol = NULL;
-  PyObject *sym_obj;
 
   if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|i", keywords, &name,
 					&domain))
@@ -561,19 +555,17 @@ gdbpy_lookup_static_symbol (PyObject *self, PyObject *args, PyObject *kw)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
+  gdbpy_ref<> sym_obj;
   if (symbol)
     {
       sym_obj = symbol_to_symbol_object (symbol);
-      if (!sym_obj)
-	return NULL;
+      if (sym_obj == nullptr)
+	return nullptr;
     }
   else
-    {
-      sym_obj = Py_None;
-      Py_INCREF (Py_None);
-    }
+    sym_obj = gdbpy_ref<>::new_reference (Py_None);
 
-  return sym_obj;
+  return sym_obj.release ();
 }
 
 /* Implementation of
@@ -622,9 +614,10 @@ gdbpy_lookup_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
 
 		  if (symbol != nullptr)
 		    {
-		      PyObject *sym_obj = symbol_to_symbol_object (symbol);
+		      gdbpy_ref<> sym_obj = symbol_to_symbol_object (symbol);
 		      if (sym_obj == nullptr
-			  || PyList_Append (return_list.get (), sym_obj) == -1)
+			  || PyList_Append (return_list.get (),
+					    sym_obj.get ()) == -1)
 			return false;
 		    }
 		}
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 96e11137be6..a71aaac7335 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -696,7 +696,7 @@ pending_framepy_function (PyObject *self, PyObject *args)
     }
 
   if (sym != nullptr)
-    return symbol_to_symbol_object (sym);
+    return symbol_to_symbol_object (sym).release ();
 
   Py_RETURN_NONE;
 }
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index feabf270464..979bf2498da 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -497,7 +497,7 @@ PyObject *gdbpy_register_tui_window (PyObject *self, PyObject *args,
 
 gdbpy_ref<> symtab_and_line_to_sal_object (struct symtab_and_line sal);
 PyObject *symtab_to_symtab_object (struct symtab *symtab);
-PyObject *symbol_to_symbol_object (struct symbol *sym);
+gdbpy_ref<> symbol_to_symbol_object (struct symbol *sym);
 PyObject *block_to_block_object (const struct block *block,
 				 struct objfile *objfile);
 PyObject *value_to_value_object (struct value *v);

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 03/10] Return gdbpy_ref<> from symtab_to_symtab_object
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
  2026-02-20 21:03 ` [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object Tom Tromey
  2026-02-20 21:03 ` [PATCH 02/10] Return gdbpy_ref<> from symbol_to_symbol_object Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-20 21:03 ` [PATCH 04/10] Return gdbpy_ref<> from block_to_block_object Tom Tromey
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes symtab_to_symtab_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
---
 gdb/python/py-symbol.c       | 2 +-
 gdb/python/py-symtab.c       | 8 ++++----
 gdb/python/python-internal.h | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index d51f4823315..492c386b7ee 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -86,7 +86,7 @@ sympy_get_symtab (PyObject *self, void *closure)
   if (!symbol->is_objfile_owned ())
     Py_RETURN_NONE;
 
-  return symtab_to_symtab_object (symbol->symtab ());
+  return symtab_to_symtab_object (symbol->symtab ()).release ();
 }
 
 static PyObject *
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 820542932ba..3b05721e616 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -299,7 +299,7 @@ salpy_get_symtab (PyObject *self, void *closure)
   if (sal->symtab == nullptr)
     Py_RETURN_NONE;
   else
-    return symtab_to_symtab_object (sal->symtab);
+    return symtab_to_symtab_object (sal->symtab).release ();
 }
 
 /* Implementation of gdb.Symtab_and_line.is_valid (self) -> Boolean.
@@ -366,7 +366,7 @@ set_symtab (symtab_object *obj, struct symtab *symtab)
 
 /* Create a new symbol table (gdb.Symtab) object that encapsulates the
    symtab structure from GDB.  */
-PyObject *
+gdbpy_ref<>
 symtab_to_symtab_object (struct symtab *symtab)
 {
   symtab_object *symtab_obj;
@@ -378,14 +378,14 @@ symtab_to_symtab_object (struct symtab *symtab)
       symtab_obj = stpy_registry.lookup (symtab->compunit ()->objfile (),
 					 symtab);
       if (symtab_obj != nullptr)
-	return (PyObject*)symtab_obj;
+	return gdbpy_ref<> (symtab_obj);
     }
 
   symtab_obj = PyObject_New (symtab_object, &symtab_object_type);
   if (symtab_obj)
     set_symtab (symtab_obj, symtab);
 
-  return (PyObject *) symtab_obj;
+  return gdbpy_ref<> (symtab_obj);
 }
 
 /* Create a new symtab and line (gdb.Symtab_and_line) object
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 979bf2498da..8d101e8afb6 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -496,7 +496,7 @@ PyObject *gdbpy_register_tui_window (PyObject *self, PyObject *args,
 				     PyObject *kw);
 
 gdbpy_ref<> symtab_and_line_to_sal_object (struct symtab_and_line sal);
-PyObject *symtab_to_symtab_object (struct symtab *symtab);
+gdbpy_ref<> symtab_to_symtab_object (struct symtab *symtab);
 gdbpy_ref<> symbol_to_symbol_object (struct symbol *sym);
 PyObject *block_to_block_object (const struct block *block,
 				 struct objfile *objfile);

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 04/10] Return gdbpy_ref<> from block_to_block_object
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
                   ` (2 preceding siblings ...)
  2026-02-20 21:03 ` [PATCH 03/10] Return gdbpy_ref<> from symtab_to_symtab_object Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-20 21:03 ` [PATCH 05/10] Return gdbpy_ref<> from value_to_value_object Tom Tromey
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes block_to_block_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
---
 gdb/python/py-block.c        | 20 +++++++-------------
 gdb/python/py-frame.c        |  3 ++-
 gdb/python/py-progspace.c    |  2 +-
 gdb/python/py-symtab.c       |  6 ++++--
 gdb/python/py-unwind.c       |  3 ++-
 gdb/python/python-internal.h |  4 ++--
 6 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/gdb/python/py-block.c b/gdb/python/py-block.c
index 718aca17829..263819e1292 100644
--- a/gdb/python/py-block.c
+++ b/gdb/python/py-block.c
@@ -143,7 +143,7 @@ blpy_get_superblock (PyObject *self, void *closure)
 
   super_block = block->superblock ();
   if (super_block)
-    return block_to_block_object (super_block, self_obj->objfile);
+    return block_to_block_object (super_block, self_obj->objfile).release ();
 
   Py_RETURN_NONE;
 }
@@ -168,7 +168,7 @@ blpy_get_subblocks (PyObject *self, void *closure)
     {
       if (each->superblock () == block)
 	{
-	  gdbpy_ref<> item (block_to_block_object (each, cu->objfile ()));
+	  gdbpy_ref<> item = block_to_block_object (each, cu->objfile ());
 
 	  if (item.get () == nullptr
 	      || PyList_Append (list.get (), item.get ()) == -1)
@@ -192,9 +192,7 @@ blpy_get_global_block (PyObject *self, void *closure)
 
   global_block = block->global_block ();
 
-  return block_to_block_object (global_block,
-				self_obj->objfile);
-
+  return block_to_block_object (global_block, self_obj->objfile).release ();
 }
 
 /* Return the static block associated to this block.  Return None
@@ -214,7 +212,7 @@ blpy_get_static_block (PyObject *self, void *closure)
 
   static_block = block->static_block ();
 
-  return block_to_block_object (static_block, self_obj->objfile);
+  return block_to_block_object (static_block, self_obj->objfile).release ();
 }
 
 /* Implementation of gdb.Block.is_global (self) -> Boolean.
@@ -333,7 +331,7 @@ blpy_dealloc (PyObject *obj)
 
 /* Create a new block object (gdb.Block) that encapsulates the struct
    block object from GDB.  */
-PyObject *
+gdbpy_ref<>
 block_to_block_object (const struct block *block, struct objfile *objfile)
 {
   htab_t table = blpy_objfile_data_key.get (objfile);
@@ -348,11 +346,7 @@ block_to_block_object (const struct block *block, struct objfile *objfile)
   block_object *result = (block_object *) htab_find_with_hash (table, block,
 							       hash);
   if (result != nullptr)
-    {
-      PyObject *py_result = (PyObject *) result;
-      Py_INCREF (py_result);
-      return py_result;
-    }
+    return gdbpy_ref<>::new_reference ((PyObject *) result);
 
   result = PyObject_New (block_object, &block_object_type);
   if (result == nullptr)
@@ -364,7 +358,7 @@ block_to_block_object (const struct block *block, struct objfile *objfile)
   void **slot = htab_find_slot_with_hash (table, block, hash, INSERT);
   *slot = result;
 
-  return (PyObject *) result;
+  return gdbpy_ref<> (result);
 }
 
 /* Return struct block reference that is wrapped by this object.  */
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index e98edd52cfb..d9af3639d29 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -323,7 +323,8 @@ frapy_block (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  return block_to_block_object (block, fn_block->function ()->objfile ());
+  return block_to_block_object (block,
+				fn_block->function ()->objfile ()).release ();
 }
 
 
diff --git a/gdb/python/py-progspace.c b/gdb/python/py-progspace.c
index 8b5ada2dc2e..587bcf1b8c3 100644
--- a/gdb/python/py-progspace.c
+++ b/gdb/python/py-progspace.c
@@ -522,7 +522,7 @@ pspy_block_for_pc (PyObject *o, PyObject *args)
     Py_RETURN_NONE;
 
   if (block)
-    return block_to_block_object (block, cust->objfile ());
+    return block_to_block_object (block, cust->objfile ()).release ();
 
   Py_RETURN_NONE;
 }
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 3b05721e616..550f287a6ee 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -188,7 +188,8 @@ stpy_global_block (PyObject *self, PyObject *args)
   blockvector = symtab->compunit ()->blockvector ();
   const struct block *block = blockvector->global_block ();
 
-  return block_to_block_object (block, symtab->compunit ()->objfile ());
+  return block_to_block_object (block,
+				symtab->compunit ()->objfile ()).release ();
 }
 
 /* Return the STATIC_BLOCK of the underlying symtab.  */
@@ -204,7 +205,8 @@ stpy_static_block (PyObject *self, PyObject *args)
   blockvector = symtab->compunit ()->blockvector ();
   const struct block *block = blockvector->static_block ();
 
-  return block_to_block_object (block, symtab->compunit ()->objfile ());
+  return block_to_block_object (block,
+				symtab->compunit ()->objfile ()).release ();
 }
 
 /* Implementation of gdb.Symtab.linetable (self) -> gdb.LineTable.
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index a71aaac7335..5465d3d6cf9 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -666,7 +666,8 @@ pending_framepy_block (PyObject *self, PyObject *args)
       return nullptr;
     }
 
-  return block_to_block_object (block, fn_block->function ()->objfile ());
+  return block_to_block_object (block,
+				fn_block->function ()->objfile ()).release ();
 }
 
 /* Implement gdb.PendingFrame.function().  Return a gdb.Symbol
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 8d101e8afb6..de4226868dc 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -498,8 +498,8 @@ PyObject *gdbpy_register_tui_window (PyObject *self, PyObject *args,
 gdbpy_ref<> symtab_and_line_to_sal_object (struct symtab_and_line sal);
 gdbpy_ref<> symtab_to_symtab_object (struct symtab *symtab);
 gdbpy_ref<> symbol_to_symbol_object (struct symbol *sym);
-PyObject *block_to_block_object (const struct block *block,
-				 struct objfile *objfile);
+gdbpy_ref<> block_to_block_object (const struct block *block,
+				   struct objfile *objfile);
 PyObject *value_to_value_object (struct value *v);
 PyObject *type_to_type_object (struct type *);
 PyObject *frame_info_to_frame_object (const frame_info_ptr &frame);

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 05/10] Return gdbpy_ref<> from value_to_value_object
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
                   ` (3 preceding siblings ...)
  2026-02-20 21:03 ` [PATCH 04/10] Return gdbpy_ref<> from block_to_block_object Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-20 21:03 ` [PATCH 06/10] Return gdbpy_ref<> from type_to_type_object Tom Tromey
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes value_to_value_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
---
 gdb/python/py-finishbreakpoint.c |  4 +--
 gdb/python/py-frame.c            |  8 ++---
 gdb/python/py-function.c         |  2 +-
 gdb/python/py-lazy-string.c      |  4 +--
 gdb/python/py-prettyprint.c      |  4 +--
 gdb/python/py-stopevent.c        |  2 +-
 gdb/python/py-symbol.c           |  4 +--
 gdb/python/py-type.c             |  6 ++--
 gdb/python/py-unwind.c           |  4 +--
 gdb/python/py-value.c            | 70 +++++++++++++++++++---------------------
 gdb/python/py-xmethods.c         | 16 ++++-----
 gdb/python/python-internal.h     |  2 +-
 gdb/python/python.c              |  4 +--
 13 files changed, 64 insertions(+), 66 deletions(-)

diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index 43dffd64adc..3370bb02580 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -122,7 +122,7 @@ bpfinishpy_pre_stop_hook (struct gdbpy_breakpoint_object *bp_obj)
 
       if (ret)
 	{
-	  self_finishbp->return_value = value_to_value_object (ret);
+	  self_finishbp->return_value = value_to_value_object (ret).release ();
 	  if (!self_finishbp->return_value)
 	      gdbpy_print_stack ();
 	}
@@ -264,7 +264,7 @@ bpfinishpy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 		  /* Ignore Python errors at this stage.  */
 		  value *func_value = read_var_value (function, NULL, frame);
 		  self_bpfinish->function_value
-		    = value_to_value_object (func_value);
+		    = value_to_value_object (func_value).release ();
 		  PyErr_Clear ();
 
 		  self_bpfinish->func_symbol
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index d9af3639d29..68c2628c297 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -257,7 +257,7 @@ static PyObject *
 frapy_read_register (PyObject *self, PyObject *args, PyObject *kw)
 {
   PyObject *pyo_reg_id;
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
 
   static const char *keywords[] = { "register", nullptr };
   if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, &pyo_reg_id))
@@ -289,7 +289,7 @@ frapy_read_register (PyObject *self, PyObject *args, PyObject *kw)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Implementation of gdb.Frame.block (self) -> gdb.Block.
@@ -557,7 +557,7 @@ frapy_read_var (PyObject *self, PyObject *args, PyObject *kw)
       return NULL;
     }
 
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
   try
     {
       FRAPY_REQUIRE_VALID (self, frame);
@@ -571,7 +571,7 @@ frapy_read_var (PyObject *self, PyObject *args, PyObject *kw)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Select this frame.  */
diff --git a/gdb/python/py-function.c b/gdb/python/py-function.c
index ee60f08b65c..3bb81527a9c 100644
--- a/gdb/python/py-function.c
+++ b/gdb/python/py-function.c
@@ -45,7 +45,7 @@ convert_values_to_python (int argc, struct value **argv)
 
   for (i = 0; i < argc; ++i)
     {
-      gdbpy_ref<> elt (value_to_value_object (argv[i]));
+      gdbpy_ref<> elt = value_to_value_object (argv[i]);
       if (elt == NULL)
 	return NULL;
       PyTuple_SetItem (result.get (), i, elt.release ());
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index 58cdea69f56..fe248f6e78f 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -109,7 +109,7 @@ stpy_convert_to_value (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
   try
     {
       scoped_value_mark free_values;
@@ -149,7 +149,7 @@ stpy_convert_to_value (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 static void
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index 6900bf64585..0aa11d29094 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -591,7 +591,7 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
 
   gdbpy_enter enter_py (gdbarch, language);
 
-  gdbpy_ref<> val_obj (value_to_value_object (value));
+  gdbpy_ref<> val_obj = value_to_value_object (value);
   if (val_obj == NULL)
     {
       print_stack_unless_memory_error (stream);
@@ -662,7 +662,7 @@ apply_varobj_pretty_printer (PyObject *printer_obj,
 gdbpy_ref<>
 gdbpy_get_varobj_pretty_printer (struct value *value)
 {
-  gdbpy_ref<> val_obj (value_to_value_object (value));
+  gdbpy_ref<> val_obj = value_to_value_object (value);
   if (val_obj == NULL)
     return NULL;
 
diff --git a/gdb/python/py-stopevent.c b/gdb/python/py-stopevent.c
index 351c0479ee6..cefcfdd0761 100644
--- a/gdb/python/py-stopevent.c
+++ b/gdb/python/py-stopevent.c
@@ -85,7 +85,7 @@ py_print_bpstat (bpstat *bs, enum gdb_signal stop_signal)
      there's no API to add generic Python objects to a py_ui_out.  */
   if (return_value != nullptr)
     {
-      gdbpy_ref<> val (value_to_value_object (return_value));
+      gdbpy_ref<> val = value_to_value_object (return_value);
       if (val == nullptr)
 	return nullptr;
       if (PyDict_SetItemString (dict.get (), "finish-value", val.get ()) < 0)
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 492c386b7ee..80c7705aa68 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -282,7 +282,7 @@ sympy_value (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
   try
     {
       if (frame_obj != NULL)
@@ -308,7 +308,7 @@ sympy_value (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Given a symbol, and a symbol_object that has previously been
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index d1ebe852eaa..017937c1c0a 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1045,7 +1045,7 @@ typy_template_argument (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
   try
     {
       scoped_value_mark free_values;
@@ -1057,7 +1057,7 @@ typy_template_argument (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* __repr__ implementation for gdb.Type.  */
@@ -1236,7 +1236,7 @@ typy_optimized_out (PyObject *self, PyObject *args)
   struct type *type = ((type_object *) self)->type;
 
   scoped_value_mark free_values;
-  return value_to_value_object (value::allocate_optimized_out (type));
+  return value_to_value_object (value::allocate_optimized_out (type)).release ();
 }
 
 /* Return a gdb.Field object for the field named by the argument.  */
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 5465d3d6cf9..872cd00e84f 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -480,7 +480,7 @@ pending_framepy_read_register (PyObject *self, PyObject *args, PyObject *kw)
   if (!gdbpy_parse_register_id (pending_frame->gdbarch, pyo_reg_id, &regnum))
     return nullptr;
 
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
   try
     {
       scoped_value_mark free_values;
@@ -504,7 +504,7 @@ pending_framepy_read_register (PyObject *self, PyObject *args, PyObject *kw)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Implement PendingFrame.is_valid().  Return True if this pending frame
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index e19be1d2701..c0fcfb882e1 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -243,7 +243,7 @@ gdbpy_preserve_values (const struct extension_language_defn *extlang,
 static PyObject *
 valpy_dereference (PyObject *self, PyObject *args)
 {
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   try
     {
@@ -258,7 +258,7 @@ valpy_dereference (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Given a value of a pointer type or a reference type, return the value
@@ -272,7 +272,7 @@ valpy_dereference (PyObject *self, PyObject *args)
 static PyObject *
 valpy_referenced_value (PyObject *self, PyObject *args)
 {
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   try
     {
@@ -301,7 +301,7 @@ valpy_referenced_value (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Return a value which is a reference to the value.  */
@@ -309,7 +309,7 @@ valpy_referenced_value (PyObject *self, PyObject *args)
 static PyObject *
 valpy_reference_value (PyObject *self, PyObject *args, enum type_code refcode)
 {
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   try
     {
@@ -324,7 +324,7 @@ valpy_reference_value (PyObject *self, PyObject *args, enum type_code refcode)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 static PyObject *
@@ -344,7 +344,7 @@ valpy_rvalue_reference_value (PyObject *self, PyObject *args)
 static PyObject *
 valpy_to_array (PyObject *self, PyObject *args)
 {
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
 
   try
     {
@@ -352,10 +352,7 @@ valpy_to_array (PyObject *self, PyObject *args)
       struct type *type = check_typedef (val->type ());
 
       if (type->code () == TYPE_CODE_ARRAY)
-	{
-	  result = self;
-	  Py_INCREF (result);
-	}
+	result = gdbpy_ref<>::new_reference (self);
       else
 	{
 	  val = value_to_array (val);
@@ -370,7 +367,7 @@ valpy_to_array (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Return a "const" qualified version of the value.  */
@@ -378,7 +375,7 @@ valpy_to_array (PyObject *self, PyObject *args)
 static PyObject *
 valpy_const_value (PyObject *self, PyObject *args)
 {
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   try
     {
@@ -394,7 +391,7 @@ valpy_const_value (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Return "&value".  */
@@ -411,7 +408,7 @@ valpy_get_address (PyObject *self, void *closure)
 	  scoped_value_mark free_values;
 
 	  res_val = value_addr (val_obj->value);
-	  val_obj->address = value_to_value_object (res_val);
+	  val_obj->address = value_to_value_object (res_val).release ();
 	}
       catch (const gdb_exception_forced_quit &except)
 	{
@@ -807,7 +804,7 @@ valpy_format_string (PyObject *self, PyObject *args, PyObject *kw)
 static PyObject *
 valpy_do_cast (PyObject *self, PyObject *args, enum exp_opcode op)
 {
-  PyObject *type_obj, *result = NULL;
+  PyObject *type_obj;
   struct type *type;
 
   if (! PyArg_ParseTuple (args, "O", &type_obj))
@@ -821,6 +818,7 @@ valpy_do_cast (PyObject *self, PyObject *args, enum exp_opcode op)
       return NULL;
     }
 
+  gdbpy_ref<> result;
   try
     {
       struct value *val = ((value_object *) self)->value;
@@ -844,7 +842,7 @@ valpy_do_cast (PyObject *self, PyObject *args, enum exp_opcode op)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Implementation of the "cast" method.  */
@@ -1020,7 +1018,7 @@ valpy_getitem (PyObject *self, PyObject *key)
   gdb::unique_xmalloc_ptr<char> field;
   struct type *base_class_type = NULL, *field_type = NULL;
   long bitpos = -1;
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   if (gdbpy_is_string (key))
     {
@@ -1153,7 +1151,7 @@ valpy_getitem (PyObject *self, PyObject *key)
       return gdbpy_handle_gdb_exception (nullptr, ex);
     }
 
-  return result;
+  return result.release ();
 }
 
 static int
@@ -1173,7 +1171,7 @@ valpy_call (PyObject *self, PyObject *args, PyObject *keywords)
   struct value *function = ((value_object *) self)->value;
   struct value **vargs = NULL;
   struct type *ftype = NULL;
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   try
     {
@@ -1241,7 +1239,7 @@ valpy_call (PyObject *self, PyObject *args, PyObject *keywords)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Called by the Python interpreter to obtain string representation
@@ -1439,10 +1437,10 @@ enum valpy_opcode
    of applying the operation specified by OPCODE to the given
    arguments.  Throws a GDB exception on error.  */
 
-static PyObject *
+static gdbpy_ref<>
 valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
 {
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   struct value *arg1, *arg2;
   struct value *res_val = NULL;
@@ -1565,7 +1563,7 @@ valpy_binop_throw (enum valpy_opcode opcode, PyObject *self, PyObject *other)
 static PyObject *
 valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
 {
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   try
     {
@@ -1576,7 +1574,7 @@ valpy_binop (enum valpy_opcode opcode, PyObject *self, PyObject *other)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 static PyObject *
@@ -1628,7 +1626,7 @@ valpy_power (PyObject *self, PyObject *other, PyObject *unused)
 static PyObject *
 valpy_negative (PyObject *self)
 {
-  PyObject *result = NULL;
+  gdbpy_ref<> result;
 
   try
     {
@@ -1644,13 +1642,13 @@ valpy_negative (PyObject *self)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 static PyObject *
 valpy_positive (PyObject *self)
 {
-  return value_to_value_object (((value_object *) self)->value);
+  return value_to_value_object (((value_object *) self)->value).release ();
 }
 
 static PyObject *
@@ -1713,7 +1711,7 @@ valpy_nonzero (PyObject *self)
 static PyObject *
 valpy_invert (PyObject *self)
 {
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
 
   try
     {
@@ -1726,7 +1724,7 @@ valpy_invert (PyObject *self)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Implements left shift for value objects.  */
@@ -1972,7 +1970,7 @@ valpy_float (PyObject *self)
 
 /* Returns an object for a value, without releasing it from the
    all_values chain.  */
-PyObject *
+gdbpy_ref<>
 value_to_value_object (struct value *val)
 {
   value_object *val_obj;
@@ -1990,7 +1988,7 @@ value_to_value_object (struct value *val)
       note_value (val_obj);
     }
 
-  return (PyObject *) val_obj;
+  return gdbpy_ref<> (val_obj);
 }
 
 /* Returns a borrowed reference to the struct value corresponding to
@@ -2103,7 +2101,7 @@ gdbpy_history (PyObject *self, PyObject *args)
   if (!PyArg_ParseTuple (args, "i", &i))
     return NULL;
 
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
   try
     {
       scoped_value_mark free_values;
@@ -2115,7 +2113,7 @@ gdbpy_history (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Add a gdb.Value into GDB's history, and return (as an integer) the
@@ -2163,7 +2161,7 @@ gdbpy_convenience_variable (PyObject *self, PyObject *args)
   if (!PyArg_ParseTuple (args, "s", &varname))
     return NULL;
 
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
   bool found = false;
   try
     {
@@ -2190,7 +2188,7 @@ gdbpy_convenience_variable (PyObject *self, PyObject *args)
   if (result == nullptr && !found)
     Py_RETURN_NONE;
 
-  return result;
+  return result.release ();
 }
 
 /* Set the value of a convenience variable.  */
diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
index de6c9d0b07d..64f9807cfee 100644
--- a/gdb/python/py-xmethods.c
+++ b/gdb/python/py-xmethods.c
@@ -444,7 +444,7 @@ python_xmethod_worker::do_get_result_type (value *obj,
       if (!types_equal (obj_type, this_type))
 	obj = value_cast (this_type, obj);
     }
-  gdbpy_ref<> py_value_obj (value_to_value_object (obj));
+  gdbpy_ref<> py_value_obj = value_to_value_object (obj);
   if (py_value_obj == NULL)
     {
       gdbpy_print_stack ();
@@ -464,14 +464,14 @@ python_xmethod_worker::do_get_result_type (value *obj,
 
   for (i = 0; i < args.size (); i++)
     {
-      PyObject *py_value_arg = value_to_value_object (args[i]);
+      gdbpy_ref<> py_value_arg = value_to_value_object (args[i]);
 
-      if (py_value_arg == NULL)
+      if (py_value_arg == nullptr)
 	{
 	  gdbpy_print_stack ();
 	  return EXT_LANG_RC_ERROR;
 	}
-      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg);
+      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg.release ());
     }
 
   gdbpy_ref<> py_result_type
@@ -529,7 +529,7 @@ python_xmethod_worker::invoke (struct value *obj,
       if (!types_equal (obj_type, this_type))
 	obj = value_cast (this_type, obj);
     }
-  gdbpy_ref<> py_value_obj (value_to_value_object (obj));
+  gdbpy_ref<> py_value_obj = value_to_value_object (obj);
   if (py_value_obj == NULL)
     {
       gdbpy_print_stack ();
@@ -549,15 +549,15 @@ python_xmethod_worker::invoke (struct value *obj,
 
   for (i = 0; i < args.size (); i++)
     {
-      PyObject *py_value_arg = value_to_value_object (args[i]);
+      gdbpy_ref<> py_value_arg = value_to_value_object (args[i]);
 
-      if (py_value_arg == NULL)
+      if (py_value_arg == nullptr)
 	{
 	  gdbpy_print_stack ();
 	  error (_("Error while executing Python code."));
 	}
 
-      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg);
+      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg.release ());
     }
 
   gdbpy_ref<> py_result (PyObject_CallObject (m_py_worker,
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index de4226868dc..c7e812d6893 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -500,7 +500,7 @@ gdbpy_ref<> symtab_to_symtab_object (struct symtab *symtab);
 gdbpy_ref<> symbol_to_symbol_object (struct symbol *sym);
 gdbpy_ref<> block_to_block_object (const struct block *block,
 				   struct objfile *objfile);
-PyObject *value_to_value_object (struct value *v);
+gdbpy_ref<> value_to_value_object (struct value *v);
 PyObject *type_to_type_object (struct type *);
 PyObject *frame_info_to_frame_object (const frame_info_ptr &frame);
 PyObject *symtab_to_linetable_object (PyObject *symtab);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index d2a77890072..aec2c7559a8 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1073,7 +1073,7 @@ gdbpy_parse_and_eval (PyObject *self, PyObject *args, PyObject *kw)
 	flags |= PARSER_LEAVE_BLOCK_ALONE;
     }
 
-  PyObject *result = nullptr;
+  gdbpy_ref<> result;
   try
     {
       scoped_value_mark free_values;
@@ -1094,7 +1094,7 @@ gdbpy_parse_and_eval (PyObject *self, PyObject *args, PyObject *kw)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return result;
+  return result.release ();
 }
 
 /* Implementation of gdb.invalidate_cached_frames.  */

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 06/10] Return gdbpy_ref<> from type_to_type_object
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
                   ` (4 preceding siblings ...)
  2026-02-20 21:03 ` [PATCH 05/10] Return gdbpy_ref<> from value_to_value_object Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-20 21:03 ` [PATCH 07/10] Return gdbpy_ref<> from frame_info_to_frame_object Tom Tromey
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes type_to_type_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
---
 gdb/python/py-arch.c         |  4 ++--
 gdb/python/py-lazy-string.c  |  2 +-
 gdb/python/py-symbol.c       |  2 +-
 gdb/python/py-type.c         | 45 ++++++++++++++++++++++----------------------
 gdb/python/py-value.c        |  4 ++--
 gdb/python/py-xmethods.c     |  2 +-
 gdb/python/python-internal.h |  2 +-
 gdb/python/python.c          |  2 +-
 8 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index eb4a7c63530..cf740821dfa 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -314,7 +314,7 @@ archpy_integer_type (PyObject *self, PyObject *args, PyObject *kw)
       return nullptr;
     }
 
-  return type_to_type_object (type);
+  return type_to_type_object (type).release ();
 }
 
 /* Implementation of gdb.void_type.  */
@@ -324,7 +324,7 @@ archpy_void_type (PyObject *self, PyObject *args)
   struct gdbarch *gdbarch;
   ARCHPY_REQUIRE_VALID (self, gdbarch);
 
-  return type_to_type_object (builtin_type (gdbarch)->builtin_void);
+  return type_to_type_object (builtin_type (gdbarch)->builtin_void).release ();
 }
 
 /* __repr__ implementation for gdb.Architecture.  */
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index fe248f6e78f..26998064206 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -235,7 +235,7 @@ gdbpy_create_lazy_string_object (CORE_ADDR address, long length,
     str_obj->encoding = NULL;
   else
     str_obj->encoding = xstrdup (encoding);
-  str_obj->type = type_to_type_object (type);
+  str_obj->type = type_to_type_object (type).release ();
 
   return (PyObject *) str_obj;
 }
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 80c7705aa68..7b6164fb20d 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -73,7 +73,7 @@ sympy_get_type (PyObject *self, void *closure)
       return Py_None;
     }
 
-  return type_to_type_object (symbol->type ());
+  return type_to_type_object (symbol->type ()).release ();
 }
 
 static PyObject *
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 017937c1c0a..8f8f6e326a8 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -132,7 +132,7 @@ convert_field (struct type *type, int field)
   if (result == NULL)
     return NULL;
 
-  gdbpy_ref<> arg (type_to_type_object (type));
+  gdbpy_ref<> arg = type_to_type_object (type);
   if (arg == NULL)
     return NULL;
   if (PyObject_SetAttrString (result.get (), "parent_type", arg.get ()) < 0)
@@ -202,7 +202,7 @@ convert_field (struct type *type, int field)
   if (type->field (field).type () == NULL)
     arg = gdbpy_ref<>::new_reference (Py_None);
   else
-    arg.reset (type_to_type_object (type->field (field).type ()));
+    arg = type_to_type_object (type->field (field).type ());
   if (arg == NULL)
     return NULL;
   if (PyObject_SetAttrString (result.get (), "type", arg.get ()) < 0)
@@ -283,7 +283,7 @@ typy_fields_items (PyObject *self, enum gdbpy_iter_kind kind)
   gdbpy_ref<> type_holder;
   if (checked_type != type)
     {
-      type_holder.reset (type_to_type_object (checked_type));
+      type_holder = type_to_type_object (checked_type);
       if (type_holder == nullptr)
 	return nullptr;
       py_type = type_holder.get ();
@@ -489,7 +489,7 @@ typy_strip_typedefs (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return type_to_type_object (type);
+  return type_to_type_object (type).release ();
 }
 
 /* Strip typedefs and pointers/reference from a type.  Then check that
@@ -580,7 +580,7 @@ typy_array_1 (PyObject *self, PyObject *args, int is_vector)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return type_to_type_object (array);
+  return type_to_type_object (array).release ();
 }
 
 /* Return an array type.  */
@@ -614,7 +614,7 @@ typy_pointer (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return type_to_type_object (type);
+  return type_to_type_object (type).release ();
 }
 
 /* Return the range of a type represented by SELF.  The return type is
@@ -686,7 +686,7 @@ typy_reference (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return type_to_type_object (type);
+  return type_to_type_object (type).release ();
 }
 
 /* Return a Type object which represents the target type of SELF.  */
@@ -702,7 +702,7 @@ typy_target (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  return type_to_type_object (type->target_type ());
+  return type_to_type_object (type->target_type ()).release ();
 }
 
 /* Return a const-qualified type variant.  */
@@ -720,7 +720,7 @@ typy_const (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return type_to_type_object (type);
+  return type_to_type_object (type).release ();
 }
 
 /* Return a volatile-qualified type variant.  */
@@ -738,7 +738,7 @@ typy_volatile (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return type_to_type_object (type);
+  return type_to_type_object (type).release ();
 }
 
 /* Return an unqualified type variant.  */
@@ -756,7 +756,7 @@ typy_unqualified (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return type_to_type_object (type);
+  return type_to_type_object (type).release ();
 }
 
 /* Return the size of the type represented by SELF, in bytes.  */
@@ -978,7 +978,7 @@ typy_legacy_template_argument (struct type *type, const struct block *block,
   if (! argtype)
     return NULL;
 
-  return type_to_type_object (argtype);
+  return type_to_type_object (argtype).release ();
 }
 
 static PyObject *
@@ -1037,7 +1037,7 @@ typy_template_argument (PyObject *self, PyObject *args)
 
   sym = TYPE_TEMPLATE_ARGUMENT (type, argno);
   if (sym->loc_class () == LOC_TYPEDEF)
-    return type_to_type_object (sym->type ());
+    return type_to_type_object (sym->type ()).release ();
   else if (sym->loc_class () == LOC_OPTIMIZED_OUT)
     {
       PyErr_Format (PyExc_RuntimeError,
@@ -1423,7 +1423,7 @@ typy_iterator_dealloc (PyObject *obj)
 }
 
 /* Create a new Type referring to TYPE.  */
-PyObject *
+gdbpy_ref<>
 type_to_type_object (struct type *type)
 {
   type_object *type_obj;
@@ -1450,14 +1450,13 @@ type_to_type_object (struct type *type)
   else
     type_obj = typy_registry.lookup (type->arch_owner (), type);
 
-  if (type_obj != nullptr)
-    return (PyObject*)type_obj;
-
-  type_obj = PyObject_New (type_object, &type_object_type);
-  if (type_obj)
-    set_type (type_obj, type);
-
-  return (PyObject *) type_obj;
+  if (type_obj == nullptr)
+    {
+      type_obj = PyObject_New (type_object, &type_object_type);
+      if (type_obj)
+	set_type (type_obj, type);
+    }
+  return gdbpy_ref<> (type_obj);
 }
 
 struct type *
@@ -1499,7 +1498,7 @@ gdbpy_lookup_type (PyObject *self, PyObject *args, PyObject *kw)
   if (! type)
     return NULL;
 
-  return type_to_type_object (type);
+  return type_to_type_object (type).release ();
 }
 
 static int
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index c0fcfb882e1..a5b2303728c 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -432,7 +432,7 @@ valpy_get_type (PyObject *self, void *closure)
 {
   value_object *obj = (value_object *) self;
 
-  return type_to_type_object (obj->value->type ());
+  return type_to_type_object (obj->value->type ()).release ();
 }
 
 /* Return dynamic type of the value.  */
@@ -484,7 +484,7 @@ valpy_get_dynamic_type (PyObject *self, void *closure)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return type_to_type_object (type);
+  return type_to_type_object (type).release ();
 }
 
 /* Implementation of gdb.Value.lazy_string ([encoding] [, length]) ->
diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
index 64f9807cfee..1fafafb4d24 100644
--- a/gdb/python/py-xmethods.c
+++ b/gdb/python/py-xmethods.c
@@ -123,7 +123,7 @@ gdbpy_get_matching_xmethod_workers
 
   gdbpy_enter enter_py;
 
-  gdbpy_ref<> py_type (type_to_type_object (obj_type));
+  gdbpy_ref<> py_type = type_to_type_object (obj_type);
   if (py_type == NULL)
     {
       gdbpy_print_stack ();
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index c7e812d6893..6fa37cf85bd 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -501,7 +501,7 @@ gdbpy_ref<> symbol_to_symbol_object (struct symbol *sym);
 gdbpy_ref<> block_to_block_object (const struct block *block,
 				   struct objfile *objfile);
 gdbpy_ref<> value_to_value_object (struct value *v);
-PyObject *type_to_type_object (struct type *);
+gdbpy_ref<> type_to_type_object (struct type *);
 PyObject *frame_info_to_frame_object (const frame_info_ptr &frame);
 PyObject *symtab_to_linetable_object (PyObject *symtab);
 gdbpy_ref<> pspace_to_pspace_object (struct program_space *);
diff --git a/gdb/python/python.c b/gdb/python/python.c
index aec2c7559a8..9862d1a726f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -2105,7 +2105,7 @@ gdbpy_apply_type_printers (const struct extension_language_defn *extlang,
 
   gdbpy_enter enter_py;
 
-  gdbpy_ref<> type_obj (type_to_type_object (type));
+  gdbpy_ref<> type_obj = type_to_type_object (type);
   if (type_obj == NULL)
     {
       gdbpy_print_stack ();

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 07/10] Return gdbpy_ref<> from frame_info_to_frame_object
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
                   ` (5 preceding siblings ...)
  2026-02-20 21:03 ` [PATCH 06/10] Return gdbpy_ref<> from type_to_type_object Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-20 21:03 ` [PATCH 08/10] Return gdbpy_ref<> from symtab_to_linetable_object Tom Tromey
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes frame_info_to_frame_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
---
 gdb/python/py-frame.c        | 28 +++++++++++-----------------
 gdb/python/py-framefilter.c  |  2 +-
 gdb/python/py-infevents.c    |  2 +-
 gdb/python/python-internal.h |  2 +-
 4 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 68c2628c297..c0defbdc7ef 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -360,7 +360,7 @@ frapy_function (PyObject *self, PyObject *args)
 /* Convert a frame_info struct to a Python Frame object.
    Sets a Python exception and returns NULL on error.  */
 
-PyObject *
+gdbpy_ref<>
 frame_info_to_frame_object (const frame_info_ptr &frame)
 {
   gdbpy_ref<frame_object> frame_obj (PyObject_New (frame_object,
@@ -393,7 +393,7 @@ frame_info_to_frame_object (const frame_info_ptr &frame)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return (PyObject *) frame_obj.release ();
+  return gdbpy_ref<> (frame_obj.release ());
 }
 
 /* Implementation of gdb.Frame.older (self) -> gdb.Frame.
@@ -404,7 +404,6 @@ static PyObject *
 frapy_older (PyObject *self, PyObject *args)
 {
   frame_info_ptr frame, prev = NULL;
-  PyObject *prev_obj = NULL;   /* Initialize to appease gcc warning.  */
 
   try
     {
@@ -417,15 +416,13 @@ frapy_older (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
+  gdbpy_ref<> prev_obj;
   if (prev)
     prev_obj = frame_info_to_frame_object (prev);
   else
-    {
-      Py_INCREF (Py_None);
-      prev_obj = Py_None;
-    }
+    prev_obj = gdbpy_ref<>::new_reference (Py_None);
 
-  return prev_obj;
+  return prev_obj.release ();
 }
 
 /* Implementation of gdb.Frame.newer (self) -> gdb.Frame.
@@ -436,7 +433,6 @@ static PyObject *
 frapy_newer (PyObject *self, PyObject *args)
 {
   frame_info_ptr frame, next = NULL;
-  PyObject *next_obj = NULL;   /* Initialize to appease gcc warning.  */
 
   try
     {
@@ -449,15 +445,13 @@ frapy_newer (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
+  gdbpy_ref<> next_obj;
   if (next)
     next_obj = frame_info_to_frame_object (next);
   else
-    {
-      Py_INCREF (Py_None);
-      next_obj = Py_None;
-    }
+    next_obj = gdbpy_ref<>::new_reference (Py_None);
 
-  return next_obj;
+  return next_obj.release ();
 }
 
 /* Implementation of gdb.Frame.find_sal (self) -> gdb.Symtab_and_line.
@@ -660,7 +654,7 @@ frapy_static_link (PyObject *self, PyObject *args)
   if (link == nullptr)
     Py_RETURN_NONE;
 
-  return frame_info_to_frame_object (link);
+  return frame_info_to_frame_object (link).release ();
 }
 
 /* Implementation of gdb.newest_frame () -> gdb.Frame.
@@ -680,7 +674,7 @@ gdbpy_newest_frame (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return frame_info_to_frame_object (frame);
+  return frame_info_to_frame_object (frame).release ();
 }
 
 /* Implementation of gdb.selected_frame () -> gdb.Frame.
@@ -700,7 +694,7 @@ gdbpy_selected_frame (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return frame_info_to_frame_object (frame);
+  return frame_info_to_frame_object (frame).release ();
 }
 
 /* Implementation of gdb.stop_reason_string (Integer) -> String.
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 7e54286abbc..26172d9f80e 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1088,7 +1088,7 @@ static PyObject *
 bootstrap_python_frame_filters (const frame_info_ptr &frame,
 				int frame_low, int frame_high)
 {
-  gdbpy_ref<> frame_obj (frame_info_to_frame_object (frame));
+  gdbpy_ref<> frame_obj = frame_info_to_frame_object (frame);
   if (frame_obj == NULL)
     return NULL;
 
diff --git a/gdb/python/py-infevents.c b/gdb/python/py-infevents.c
index 58c29de130e..1b27987d039 100644
--- a/gdb/python/py-infevents.c
+++ b/gdb/python/py-infevents.c
@@ -68,7 +68,7 @@ create_register_changed_event_object (const frame_info_ptr &frame,
   if (event == NULL)
     return NULL;
 
-  gdbpy_ref<> frame_obj (frame_info_to_frame_object (frame));
+  gdbpy_ref<> frame_obj = frame_info_to_frame_object (frame);
   if (frame_obj == NULL)
     return NULL;
 
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 6fa37cf85bd..0b816db196f 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -502,7 +502,7 @@ gdbpy_ref<> block_to_block_object (const struct block *block,
 				   struct objfile *objfile);
 gdbpy_ref<> value_to_value_object (struct value *v);
 gdbpy_ref<> type_to_type_object (struct type *);
-PyObject *frame_info_to_frame_object (const frame_info_ptr &frame);
+gdbpy_ref<> frame_info_to_frame_object (const frame_info_ptr &frame);
 PyObject *symtab_to_linetable_object (PyObject *symtab);
 gdbpy_ref<> pspace_to_pspace_object (struct program_space *);
 PyObject *pspy_get_printers (PyObject *, void *);

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 08/10] Return gdbpy_ref<> from symtab_to_linetable_object
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
                   ` (6 preceding siblings ...)
  2026-02-20 21:03 ` [PATCH 07/10] Return gdbpy_ref<> from frame_info_to_frame_object Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-20 21:03 ` [PATCH 09/10] Return gdbpy_ref<> from gdbarch_to_arch_object Tom Tromey
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes symtab_to_linetable_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
---
 gdb/python/py-linetable.c    | 4 ++--
 gdb/python/py-symtab.c       | 2 +-
 gdb/python/python-internal.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/python/py-linetable.c b/gdb/python/py-linetable.c
index 384ced68a32..2afa9b033e7 100644
--- a/gdb/python/py-linetable.c
+++ b/gdb/python/py-linetable.c
@@ -77,7 +77,7 @@ get_symtab (PyObject *linetable)
 /* Helper function to create a line table object that wraps a
    gdb.Symtab object.  */
 
-PyObject *
+gdbpy_ref<>
 symtab_to_linetable_object (PyObject *symtab)
 {
   linetable_object *ltable;
@@ -88,7 +88,7 @@ symtab_to_linetable_object (PyObject *symtab)
       ltable->symtab = symtab;
       Py_INCREF (symtab);
     }
-  return (PyObject *) ltable;
+  return gdbpy_ref<> (ltable);
 }
 
 /* Internal helper function to build a line table object from a line
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index 550f287a6ee..ed5abac1372 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -220,7 +220,7 @@ stpy_get_linetable (PyObject *self, PyObject *args)
 
   STPY_REQUIRE_VALID (self, symtab);
 
-  return symtab_to_linetable_object (self);
+  return symtab_to_linetable_object (self).release ();
 }
 
 static PyObject *
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 0b816db196f..85f7f1f4dd1 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -503,7 +503,7 @@ gdbpy_ref<> block_to_block_object (const struct block *block,
 gdbpy_ref<> value_to_value_object (struct value *v);
 gdbpy_ref<> type_to_type_object (struct type *);
 gdbpy_ref<> frame_info_to_frame_object (const frame_info_ptr &frame);
-PyObject *symtab_to_linetable_object (PyObject *symtab);
+gdbpy_ref<> symtab_to_linetable_object (PyObject *symtab);
 gdbpy_ref<> pspace_to_pspace_object (struct program_space *);
 PyObject *pspy_get_printers (PyObject *, void *);
 PyObject *pspy_get_frame_filters (PyObject *, void *);

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 09/10] Return gdbpy_ref<> from gdbarch_to_arch_object
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
                   ` (7 preceding siblings ...)
  2026-02-20 21:03 ` [PATCH 08/10] Return gdbpy_ref<> from symtab_to_linetable_object Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-20 21:03 ` [PATCH 10/10] Return gdbpy_ref<> from gdbpy_registry::lookup Tom Tromey
  2026-02-21  2:31 ` [PATCH 00/10] Mildly better refcount safety for Python Simon Marchi
  10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes gdbarch_to_arch_object to return a gdbpy_ref<>,
using the type system to convey that a new reference is always
returned.
---
 gdb/python/py-arch.c         | 4 ++--
 gdb/python/py-disasm.c       | 2 +-
 gdb/python/py-frame.c        | 2 +-
 gdb/python/py-inferior.c     | 2 +-
 gdb/python/py-unwind.c       | 2 +-
 gdb/python/python-internal.h | 2 +-
 gdb/python/python.c          | 2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/gdb/python/py-arch.c b/gdb/python/py-arch.c
index cf740821dfa..f40d7da1763 100644
--- a/gdb/python/py-arch.c
+++ b/gdb/python/py-arch.c
@@ -84,7 +84,7 @@ gdbpy_is_architecture (PyObject *obj)
    Returns a new reference to the arch_object associated as data with
    GDBARCH.  */
 
-PyObject *
+gdbpy_ref<>
 gdbarch_to_arch_object (struct gdbarch *gdbarch)
 {
   PyObject *new_ref = arch_object_data.get (gdbarch);
@@ -97,7 +97,7 @@ gdbarch_to_arch_object (struct gdbarch *gdbarch)
   /* new_ref could be NULL if creation failed.  */
   Py_XINCREF (new_ref);
 
-  return new_ref;
+  return gdbpy_ref<> (new_ref);
 }
 
 /* Implementation of gdb.Architecture.name (self) -> String.
diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 9f5d0e2fdcb..748ffd5e6a3 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -699,7 +699,7 @@ disasmpy_info_architecture (PyObject *self, void *closure)
 {
   disasm_info_object *obj = (disasm_info_object *) self;
   DISASMPY_DISASM_INFO_REQUIRE_VALID (obj);
-  return gdbarch_to_arch_object (obj->gdbarch);
+  return gdbarch_to_arch_object (obj->gdbarch).release ();
 }
 
 /* Implement DisassembleInfo.progspace attribute.  Return the
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index c0defbdc7ef..4d625f05ced 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -201,7 +201,7 @@ frapy_arch (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  return gdbarch_to_arch_object (obj->gdbarch);
+  return gdbarch_to_arch_object (obj->gdbarch).release ();
 }
 
 /* Implementation of gdb.Frame.unwind_stop_reason (self) -> Integer.
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 53f3344429d..fd9da27b92e 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -778,7 +778,7 @@ infpy_architecture (PyObject *self, PyObject *args)
 
   INFPY_REQUIRE_VALID (inf);
 
-  return gdbarch_to_arch_object (inf->inferior->arch ());
+  return gdbarch_to_arch_object (inf->inferior->arch ()).release ();
 }
 
 /* Implement repr() for gdb.Inferior.  */
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 872cd00e84f..fbb9263844c 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -766,7 +766,7 @@ pending_framepy_architecture (PyObject *self, PyObject *args)
 
   PENDING_FRAMEPY_REQUIRE_VALID (pending_frame);
 
-  return gdbarch_to_arch_object (pending_frame->gdbarch);
+  return gdbarch_to_arch_object (pending_frame->gdbarch).release ();
 }
 
 /* Implementation of PendingFrame.level (self) -> Integer.  */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 85f7f1f4dd1..1f0964f24d3 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -517,7 +517,7 @@ PyObject *objfpy_get_frame_unwinders (PyObject *, void *);
 PyObject *objfpy_get_xmethods (PyObject *, void *);
 PyObject *gdbpy_lookup_objfile (PyObject *self, PyObject *args, PyObject *kw);
 
-PyObject *gdbarch_to_arch_object (struct gdbarch *gdbarch);
+gdbpy_ref<> gdbarch_to_arch_object (struct gdbarch *gdbarch);
 PyObject *gdbpy_all_architecture_names (PyObject *self, PyObject *args);
 
 PyObject *gdbpy_new_register_descriptor_iterator (struct gdbarch *gdbarch,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 9862d1a726f..8739864a861 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1421,7 +1421,7 @@ gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
       return {};
     }
 
-  gdbpy_ref<> gdbarch_arg (gdbarch_to_arch_object (gdbarch));
+  gdbpy_ref<> gdbarch_arg = gdbarch_to_arch_object (gdbarch);
   if (gdbarch_arg == nullptr)
     {
       gdbpy_print_stack ();

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 10/10] Return gdbpy_ref<> from gdbpy_registry::lookup
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
                   ` (8 preceding siblings ...)
  2026-02-20 21:03 ` [PATCH 09/10] Return gdbpy_ref<> from gdbarch_to_arch_object Tom Tromey
@ 2026-02-20 21:03 ` Tom Tromey
  2026-02-21  2:31 ` [PATCH 00/10] Mildly better refcount safety for Python Simon Marchi
  10 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-20 21:03 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

This changes gdbpy_registry::lookup to return a gdbpy_ref<>, using the
type system to convey that a new reference is always returned.
---
 gdb/python/py-symbol.c       | 13 ++++++-------
 gdb/python/py-symtab.c       |  8 ++++----
 gdb/python/py-type.c         | 16 ++++++++--------
 gdb/python/python-internal.h |  7 +++----
 4 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 7b6164fb20d..e8ff2dfe28a 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -339,18 +339,17 @@ set_symbol (symbol_object *obj, struct symbol *symbol)
 gdbpy_ref<>
 symbol_to_symbol_object (struct symbol *sym)
 {
-  symbol_object *sym_obj;
-
   /* Look if there's already a gdb.Symbol object for given SYMBOL
      and if so, return it.  */
+  gdbpy_ref<> result;
   if (sym->is_objfile_owned ())
-    sym_obj = sympy_registry.lookup (sym->objfile (), sym);
+    result = sympy_registry.lookup (sym->objfile (), sym);
   else
-    sym_obj = sympy_registry.lookup (sym->arch (), sym);
-  if (sym_obj != nullptr)
-    return gdbpy_ref<> (sym_obj);
+    result = sympy_registry.lookup (sym->arch (), sym);
+  if (result != nullptr)
+    return result;
 
-  sym_obj = PyObject_New (symbol_object, &symbol_object_type);
+  symbol_object *sym_obj = PyObject_New (symbol_object, &symbol_object_type);
   if (sym_obj)
     set_symbol (sym_obj, sym);
 
diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
index ed5abac1372..27d6c4beba9 100644
--- a/gdb/python/py-symtab.c
+++ b/gdb/python/py-symtab.c
@@ -377,10 +377,10 @@ symtab_to_symtab_object (struct symtab *symtab)
      and if so, return it.  */
   if (symtab != nullptr)
     {
-      symtab_obj = stpy_registry.lookup (symtab->compunit ()->objfile (),
-					 symtab);
-      if (symtab_obj != nullptr)
-	return gdbpy_ref<> (symtab_obj);
+      gdbpy_ref<> result
+	= stpy_registry.lookup (symtab->compunit ()->objfile (), symtab);
+      if (result != nullptr)
+	return result;
     }
 
   symtab_obj = PyObject_New (symtab_object, &symtab_object_type);
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 8f8f6e326a8..b6807801f7e 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -1426,8 +1426,6 @@ typy_iterator_dealloc (PyObject *obj)
 gdbpy_ref<>
 type_to_type_object (struct type *type)
 {
-  type_object *type_obj;
-
   try
     {
       /* Try not to let stub types leak out to Python.  */
@@ -1445,18 +1443,20 @@ type_to_type_object (struct type *type)
 
   /* Look if there's already a gdb.Type object for given TYPE
      and if so, return it.  */
+  gdbpy_ref<> result;
   if (type->is_objfile_owned ())
-    type_obj = typy_registry.lookup (type->objfile_owner (), type);
+    result = typy_registry.lookup (type->objfile_owner (), type);
   else
-    type_obj = typy_registry.lookup (type->arch_owner (), type);
+    result = typy_registry.lookup (type->arch_owner (), type);
 
-  if (type_obj == nullptr)
+  if (result == nullptr)
     {
-      type_obj = PyObject_New (type_object, &type_object_type);
-      if (type_obj)
+      type_object *type_obj = PyObject_New (type_object, &type_object_type);
+      if (type_obj != nullptr)
 	set_type (type_obj, type);
+      result = gdbpy_ref<> (type_obj);
     }
-  return gdbpy_ref<> (type_obj);
+  return result;
 }
 
 struct type *
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 1f0964f24d3..fdd353ffbeb 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -1191,14 +1191,13 @@ class gdbpy_registry
   }
 
   /* Lookup pre-existing Python object for given VAL.  Return such object
-     if found, otherwise return NULL.  This method always returns new
-     reference.  */
+     if found, otherwise return NULL.  */
   template <typename O>
-  obj_type *lookup (O *owner, val_type *val) const
+  gdbpy_ref<> lookup (O *owner, val_type *val) const
   {
     obj_type *obj = get_storage (owner)->lookup (val);
     Py_XINCREF (static_cast<PyObject *> (obj));
-    return obj;
+    return gdbpy_ref<> (obj);
   }
 
 private:

-- 
2.53.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object
  2026-02-20 21:03 ` [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object Tom Tromey
@ 2026-02-21  1:58   ` Simon Marchi
  2026-02-21  2:13     ` Simon Marchi
  2026-02-23 12:28     ` Tom Tromey
  0 siblings, 2 replies; 18+ messages in thread
From: Simon Marchi @ 2026-02-21  1:58 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches



On 2026-02-20 16:03, Tom Tromey wrote:
> This changes symtab_and_line_to_sal_object to return a gdbpy_ref<>,
> using the type system to convey that a new reference is always
> returned.

I noted some stylistic suggestions below.  It's a bit subjective so it's
up to you really.  I suppose that the same patterns happen in all the
patches.

> @@ -560,7 +560,7 @@ pspy_find_pc_line (PyObject *o, PyObject *args)
>        return gdbpy_handle_gdb_exception (nullptr, except);
>      }
>  
> -  return result;
> +  return result.release ();

In these cases I think I would return directly from the
symtab_and_line_to_sal_object call:

    return symtab_and_line_to_sal_object (sal).release ();

> diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
> index 2dca0083277..820542932ba 100644
> --- a/gdb/python/py-symtab.c
> +++ b/gdb/python/py-symtab.c
> @@ -390,7 +390,7 @@ symtab_to_symtab_object (struct symtab *symtab)
>  
>  /* Create a new symtab and line (gdb.Symtab_and_line) object
>     that encapsulates the symtab_and_line structure from GDB.  */
> -PyObject *
> +gdbpy_ref<>
>  symtab_and_line_to_sal_object (struct symtab_and_line sal)
>  {
>    sal_object *sal_obj;
> @@ -399,7 +399,7 @@ symtab_and_line_to_sal_object (struct symtab_and_line sal)
>    if (sal_obj != nullptr)
>      set_sal (sal_obj, sal);
>  
> -  return (PyObject *) sal_obj;
> +  return gdbpy_ref<> (sal_obj);

I think you could be even more pedantic and change sal_obj to be of type
gdbpy_ref<>.  This way, the ref is managed from the very edge of the
Python API.

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object
  2026-02-21  1:58   ` Simon Marchi
@ 2026-02-21  2:13     ` Simon Marchi
  2026-02-21 22:33       ` Tom Tromey
  2026-02-23 12:28     ` Tom Tromey
  1 sibling, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2026-02-21  2:13 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches



On 2026-02-20 20:58, Simon Marchi wrote:
> 
> 
> On 2026-02-20 16:03, Tom Tromey wrote:
>> This changes symtab_and_line_to_sal_object to return a gdbpy_ref<>,
>> using the type system to convey that a new reference is always
>> returned.
> 
> I noted some stylistic suggestions below.  It's a bit subjective so it's
> up to you really.  I suppose that the same patterns happen in all the
> patches.
> 
>> @@ -560,7 +560,7 @@ pspy_find_pc_line (PyObject *o, PyObject *args)
>>        return gdbpy_handle_gdb_exception (nullptr, except);
>>      }
>>  
>> -  return result;
>> +  return result.release ();
> 
> In these cases I think I would return directly from the
> symtab_and_line_to_sal_object call:
> 
>     return symtab_and_line_to_sal_object (sal).release ();
> 
>> diff --git a/gdb/python/py-symtab.c b/gdb/python/py-symtab.c
>> index 2dca0083277..820542932ba 100644
>> --- a/gdb/python/py-symtab.c
>> +++ b/gdb/python/py-symtab.c
>> @@ -390,7 +390,7 @@ symtab_to_symtab_object (struct symtab *symtab)
>>  
>>  /* Create a new symtab and line (gdb.Symtab_and_line) object
>>     that encapsulates the symtab_and_line structure from GDB.  */
>> -PyObject *
>> +gdbpy_ref<>
>>  symtab_and_line_to_sal_object (struct symtab_and_line sal)
>>  {
>>    sal_object *sal_obj;
>> @@ -399,7 +399,7 @@ symtab_and_line_to_sal_object (struct symtab_and_line sal)
>>    if (sal_obj != nullptr)
>>      set_sal (sal_obj, sal);
>>  
>> -  return (PyObject *) sal_obj;
>> +  return gdbpy_ref<> (sal_obj);
> 
> I think you could be even more pedantic and change sal_obj to be of type
> gdbpy_ref<>.  This way, the ref is managed from the very edge of the
> Python API.

Regarding this, I thought this would work, but it doesn't:

  gdbpy_ref<sal_object> sal_obj (PyObject_New (sal_object, &sal_object_type));
  if (sal_obj != nullptr)
    set_sal (sal_obj.get (), sal);

  return sal_obj;

I don't know if there is a magic way to make gdbpy_ref<sal_object>
implicitly convertible to gdbpy_ref<>.

I tried changing the return type of symtab_and_line_to_sal_object to be
gdbpy_ref<sal_object> instead.  I don't think it would be a bad thing,
it would convey what type of object it is exactly.  But this would require moving the
definition of struct sal_object in some header file, otherwise the other
files don't know that `sal_object *` is convertible to `PyObject *`,
when they try to pass the released value to the Python API.

Anyway, what you have here is fine, it is definitely an improvement over
the current state.

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 02/10] Return gdbpy_ref<> from symbol_to_symbol_object
  2026-02-20 21:03 ` [PATCH 02/10] Return gdbpy_ref<> from symbol_to_symbol_object Tom Tromey
@ 2026-02-21  2:28   ` Simon Marchi
  2026-02-21 22:35     ` Tom Tromey
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2026-02-21  2:28 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches



On 2026-02-20 16:03, Tom Tromey wrote:
> @@ -622,9 +614,10 @@ gdbpy_lookup_static_symbols (PyObject *self, PyObject *args, PyObject *kw)
>  
>  		  if (symbol != nullptr)
>  		    {
> -		      PyObject *sym_obj = symbol_to_symbol_object (symbol);
> +		      gdbpy_ref<> sym_obj = symbol_to_symbol_object (symbol);
>  		      if (sym_obj == nullptr
> -			  || PyList_Append (return_list.get (), sym_obj) == -1)
> +			  || PyList_Append (return_list.get (),
> +					    sym_obj.get ()) == -1)

I asked Claude to review your comments, and it said:

    Bonus finding: The gdbpy_lookup_static_symbols function in
    py-symbol.c had a pre-existing reference leak — the old code passed
    the raw PyObject * from symbol_to_symbol_object directly to
    PyList_Append (which does its own incref) without decrementing.  The
    new code stores it in a gdbpy_ref<> and passes .get(), fixing the
    leak.

It seems true.  It's really hard to remember which functions steal
references and which functions get their own new reference.  Good job on
your part to call .get() here and not .release().  I wish we had a C++
layer around the Python API to make that clear through the type
system...

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 00/10] Mildly better refcount safety for Python
  2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
                   ` (9 preceding siblings ...)
  2026-02-20 21:03 ` [PATCH 10/10] Return gdbpy_ref<> from gdbpy_registry::lookup Tom Tromey
@ 2026-02-21  2:31 ` Simon Marchi
  10 siblings, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2026-02-21  2:31 UTC (permalink / raw)
  To: Tom Tromey, gdb-patches



On 2026-02-20 16:03, Tom Tromey wrote:
> This series converts a number of gdb-to-Python converion functions to
> return a gdbpy_ref<> rather than returning a PyObject* and then simply
> documenting that a new reference is returned.
> 
> The hope is that this will make refcount mistakes less likely.
> 
> Regression tested on x86-64 Fedora 43.
> 
> Signed-off-by: Tom Tromey <tromey@adacore.com>

I don't know if someone else wants to review this, but I think this is
good.  I think it's good if the references are managed from the point
where we get them from the Python API to the point where we release them
to the Python API.

I gave it a quick look, since it's very mechanical and I probably
wouldn't spot mistakes anyway.  But I asked Claude to review and it said
it looked good.  It even noted that you fixed a ref leak in the symbol
patch.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object
  2026-02-21  2:13     ` Simon Marchi
@ 2026-02-21 22:33       ` Tom Tromey
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-21 22:33 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I don't know if there is a magic way to make gdbpy_ref<sal_object>
Simon> implicitly convertible to gdbpy_ref<>.

I sent a series for this today.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 02/10] Return gdbpy_ref<> from symbol_to_symbol_object
  2026-02-21  2:28   ` Simon Marchi
@ 2026-02-21 22:35     ` Tom Tromey
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-21 22:35 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> It seems true.  It's really hard to remember which functions steal
Simon> references and which functions get their own new reference.  Good job on
Simon> your part to call .get() here and not .release().  I wish we had a C++
Simon> layer around the Python API to make that clear through the type
Simon> system...

I've been working on some patches that do just this.
It's complicated to fully wrap everything nicely, though.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object
  2026-02-21  1:58   ` Simon Marchi
  2026-02-21  2:13     ` Simon Marchi
@ 2026-02-23 12:28     ` Tom Tromey
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Tromey @ 2026-02-23 12:28 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, gdb-patches

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> -  return result;
>> +  return result.release ();

Simon> In these cases I think I would return directly from the
Simon> symtab_and_line_to_sal_object call:

Simon>     return symtab_and_line_to_sal_object (sal).release ();

I made this change, except in one spot where it would negatively affect
the formatting.

Tom

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2026-02-23 12:30 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-20 21:03 [PATCH 00/10] Mildly better refcount safety for Python Tom Tromey
2026-02-20 21:03 ` [PATCH 01/10] Return gdbpy_ref<> from symtab_and_line_to_sal_object Tom Tromey
2026-02-21  1:58   ` Simon Marchi
2026-02-21  2:13     ` Simon Marchi
2026-02-21 22:33       ` Tom Tromey
2026-02-23 12:28     ` Tom Tromey
2026-02-20 21:03 ` [PATCH 02/10] Return gdbpy_ref<> from symbol_to_symbol_object Tom Tromey
2026-02-21  2:28   ` Simon Marchi
2026-02-21 22:35     ` Tom Tromey
2026-02-20 21:03 ` [PATCH 03/10] Return gdbpy_ref<> from symtab_to_symtab_object Tom Tromey
2026-02-20 21:03 ` [PATCH 04/10] Return gdbpy_ref<> from block_to_block_object Tom Tromey
2026-02-20 21:03 ` [PATCH 05/10] Return gdbpy_ref<> from value_to_value_object Tom Tromey
2026-02-20 21:03 ` [PATCH 06/10] Return gdbpy_ref<> from type_to_type_object Tom Tromey
2026-02-20 21:03 ` [PATCH 07/10] Return gdbpy_ref<> from frame_info_to_frame_object Tom Tromey
2026-02-20 21:03 ` [PATCH 08/10] Return gdbpy_ref<> from symtab_to_linetable_object Tom Tromey
2026-02-20 21:03 ` [PATCH 09/10] Return gdbpy_ref<> from gdbarch_to_arch_object Tom Tromey
2026-02-20 21:03 ` [PATCH 10/10] Return gdbpy_ref<> from gdbpy_registry::lookup Tom Tromey
2026-02-21  2:31 ` [PATCH 00/10] Mildly better refcount safety for Python Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox