[PATCH v2 1/9] gdb: switch tuple object helpers to Python limited API equivalents

[Matthieu Longo <matthieu.longo@arm.com>]

* PyTuple_GET_ITEM -> PyTuple_GetItem
* PyTuple_SET_ITEM -> PyTuple_SetItem
* PyTuple_GET_SIZE -> PyTuple_Size

Unlike PyTuple_SET_ITEM(), PyTuple_SetItem() returns an integer: 0 on
success and -1 on error (e.g. IndexError). The existing code must therefore
be updated to handle this new behaviour.

Since processing now stops when PyTuple_SetItem() returns an error, some
resources (such as newly allocated tuples) must be properly deallocated in
error paths. To address this, this patch replaces the use of raw 'PyObject *'
pointers with gdbpy_ref<>, a reference-counted wrapper around 'PyObject *',
which automatically decrements the reference count on early exit.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=23830
---
 gdb/python/py-color.c     |  7 ++++---
 gdb/python/py-function.c  |  3 ++-
 gdb/python/py-inferior.c  | 16 +++++++---------
 gdb/python/py-infthread.c |  7 ++++---
 gdb/python/py-symbol.c    | 11 +++++++----
 gdb/python/py-type.c      |  5 +++--
 gdb/python/py-unwind.c    |  8 +++++---
 gdb/python/py-xmethods.c  | 24 ++++++++++++++++++------
 gdb/python/python.c       |  5 +++--
 9 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
index 0bec51b1e52..24589dba265 100644
--- a/gdb/python/py-color.c
+++ b/gdb/python/py-color.c
@@ -119,14 +119,15 @@ get_attr (PyObject *obj, PyObject *attr_name)
 	    return nullptr;
 	}
 
-      PyObject *comp = PyTuple_New (3);
+      gdbpy_ref<> comp (PyTuple_New (3));
       if (comp == nullptr)
 	return nullptr;
 
       for (int i = 0; i < 3; ++i)
-	PyTuple_SET_ITEM (comp, i, rgb_objects[i].release ());
+	if (PyTuple_SetItem (comp.get (), i, rgb_objects[i].release ()) < 0)
+	  return nullptr;
 
-      return comp;
+      return comp.release ();
     }
 
   return PyObject_GenericGetAttr (obj, attr_name);
diff --git a/gdb/python/py-function.c b/gdb/python/py-function.c
index 3bb81527a9c..23e0be0ea43 100644
--- a/gdb/python/py-function.c
+++ b/gdb/python/py-function.c
@@ -48,7 +48,8 @@ convert_values_to_python (int argc, struct value **argv)
       gdbpy_ref<> elt = value_to_value_object (argv[i]);
       if (elt == NULL)
 	return NULL;
-      PyTuple_SetItem (result.get (), i, elt.release ());
+      if (PyTuple_SetItem (result.get (), i, elt.release ()) < 0)
+	return nullptr;
     }
   return result;
 }
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index ab948d28ebe..ae309620e1f 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -382,7 +382,6 @@ infpy_threads (PyObject *self, PyObject *args)
 {
   int i = 0;
   inferior_object *inf_obj = (inferior_object *) self;
-  PyObject *tuple;
 
   INFPY_REQUIRE_VALID (inf_obj);
 
@@ -395,19 +394,18 @@ infpy_threads (PyObject *self, PyObject *args)
       return gdbpy_handle_gdb_exception (nullptr, except);
     }
 
-  tuple = PyTuple_New (inf_obj->threads->size ());
-  if (!tuple)
-    return NULL;
+  gdbpy_ref<> tuple (PyTuple_New (inf_obj->threads->size ()));
+  if (tuple == nullptr)
+    return nullptr;
 
   for (const thread_map_t::value_type &entry : *inf_obj->threads)
     {
-      PyObject *thr = (PyObject *) entry.second.get ();
-      Py_INCREF (thr);
-      PyTuple_SET_ITEM (tuple, i, thr);
-      i = i + 1;
+      auto thr = gdbpy_ref<>::new_reference ((PyObject *) entry.second.get ());
+      if (PyTuple_SetItem (tuple.get (), i++, thr.release ()) < 0)
+	return nullptr;
     }
 
-  return tuple;
+  return tuple.release ();
 }
 
 static PyObject *
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index d75742360d4..652355990ee 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -384,9 +384,10 @@ gdbpy_create_ptid_object (ptid_t ptid)
     return nullptr;
 
   /* Note that these steal references, hence the use of 'release'.  */
-  PyTuple_SET_ITEM (ret.get (), 0, pid_obj.release ());
-  PyTuple_SET_ITEM (ret.get (), 1, lwp_obj.release ());
-  PyTuple_SET_ITEM (ret.get (), 2, tid_obj.release ());
+  if (PyTuple_SetItem (ret.get (), 0, pid_obj.release ()) < 0
+      || PyTuple_SetItem (ret.get (), 1, lwp_obj.release ()) < 0
+      || PyTuple_SetItem (ret.get (), 2, tid_obj.release ()) < 0)
+    return nullptr;
 
   return ret;
 }
diff --git a/gdb/python/py-symbol.c b/gdb/python/py-symbol.c
index 15fcbeea9be..fe4d6dac000 100644
--- a/gdb/python/py-symbol.c
+++ b/gdb/python/py-symbol.c
@@ -402,7 +402,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, *bool_obj;
+  PyObject *block_obj = NULL;
   const struct block *block = NULL;
 
   if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "s|O!i", keywords, &name,
@@ -450,10 +450,13 @@ gdbpy_lookup_symbol (PyObject *self, PyObject *args, PyObject *kw)
     }
   else
     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);
+  if (PyTuple_SetItem (ret_tuple.get (), 0, sym_obj.release ()) < 0)
+    return nullptr;
+
+  gdbpy_ref<> bool_obj (PyBool_FromLong (is_a_field_of_this.type != NULL));
+  if (PyTuple_SetItem (ret_tuple.get (), 1, bool_obj.release ()) < 0)
+    return nullptr;
 
   return ret_tuple.release ();
 }
diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index b6807801f7e..7ba77ad1d4a 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -249,8 +249,9 @@ make_fielditem (struct type *type, int i, enum gdbpy_iter_kind kind)
 	gdbpy_ref<> item (PyTuple_New (2));
 	if (item == NULL)
 	  return NULL;
-	PyTuple_SET_ITEM (item.get (), 0, key.release ());
-	PyTuple_SET_ITEM (item.get (), 1, value.release ());
+	if (PyTuple_SetItem (item.get (), 0, key.release ()) < 0
+	    || PyTuple_SetItem (item.get (), 1, value.release ()) < 0)
+	  return nullptr;
 	return item;
       }
     case iter_keys:
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 09af3e7ca2c..9ffa382d093 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -899,11 +899,12 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
 
   /* Verify the return value of _execute_unwinders is a tuple of size 2.  */
   gdb_assert (PyTuple_Check (pyo_execute_ret.get ()));
-  gdb_assert (PyTuple_GET_SIZE (pyo_execute_ret.get ()) == 2);
+  gdb_assert (PyTuple_Size (pyo_execute_ret.get ()) == 2);
 
   if (pyuw_debug)
     {
-      PyObject *pyo_unwinder_name = PyTuple_GET_ITEM (pyo_execute_ret.get (), 1);
+      PyObject *pyo_unwinder_name = PyTuple_GetItem (pyo_execute_ret.get (), 1);
+      gdb_assert (pyo_unwinder_name != nullptr);
       gdb::unique_xmalloc_ptr<char> name
 	= python_string_to_host_string (pyo_unwinder_name);
 
@@ -919,7 +920,8 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
     }
 
   /* Received UnwindInfo, cache data.  */
-  PyObject *pyo_unwind_info = PyTuple_GET_ITEM (pyo_execute_ret.get (), 0);
+  PyObject *pyo_unwind_info = PyTuple_GetItem (pyo_execute_ret.get (), 0);
+  gdb_assert (pyo_unwind_info != nullptr);
   if (!PyObject_TypeCheck (pyo_unwind_info, &unwind_info_object_type))
     error (_("an Unwinder should return gdb.UnwindInfo, not %s."),
 	   Py_TYPE (pyo_unwind_info)->tp_name);
diff --git a/gdb/python/py-xmethods.c b/gdb/python/py-xmethods.c
index 1fafafb4d24..24c8f48e1bd 100644
--- a/gdb/python/py-xmethods.c
+++ b/gdb/python/py-xmethods.c
@@ -458,9 +458,13 @@ python_xmethod_worker::do_get_result_type (value *obj,
       return EXT_LANG_RC_ERROR;
     }
 
-  /* PyTuple_SET_ITEM steals the reference of the element, hence the
+  /* PyTuple_SetItem steals the reference of the element, hence the
      release.  */
-  PyTuple_SET_ITEM (py_arg_tuple.get (), 0, py_value_obj.release ());
+  if (PyTuple_SetItem (py_arg_tuple.get (), 0, py_value_obj.release ()) < 0)
+    {
+      gdbpy_print_stack ();
+      return EXT_LANG_RC_ERROR;
+    }
 
   for (i = 0; i < args.size (); i++)
     {
@@ -471,7 +475,12 @@ python_xmethod_worker::do_get_result_type (value *obj,
 	  gdbpy_print_stack ();
 	  return EXT_LANG_RC_ERROR;
 	}
-      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg.release ());
+      if (PyTuple_SetItem (py_arg_tuple.get (), i + 1,
+			   py_value_arg.release ()) < 0)
+	{
+	  gdbpy_print_stack ();
+	  return EXT_LANG_RC_ERROR;
+	}
     }
 
   gdbpy_ref<> py_result_type
@@ -543,9 +552,10 @@ python_xmethod_worker::invoke (struct value *obj,
       error (_("Error while executing Python code."));
     }
 
-  /* PyTuple_SET_ITEM steals the reference of the element, hence the
+  /* PyTuple_SetItem steals the reference of the element, hence the
      release.  */
-  PyTuple_SET_ITEM (py_arg_tuple.get (), 0, py_value_obj.release ());
+  if (PyTuple_SetItem (py_arg_tuple.get (), 0, py_value_obj.release ()) < 0)
+    return nullptr;
 
   for (i = 0; i < args.size (); i++)
     {
@@ -557,7 +567,9 @@ python_xmethod_worker::invoke (struct value *obj,
 	  error (_("Error while executing Python code."));
 	}
 
-      PyTuple_SET_ITEM (py_arg_tuple.get (), i + 1, py_value_arg.release ());
+      if (PyTuple_SetItem (py_arg_tuple.get (), i + 1,
+			   py_value_arg.release ()) < 0)
+	return nullptr;
     }
 
   gdbpy_ref<> py_result (PyObject_CallObject (m_py_worker,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 8739864a861..5474b8d644f 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1043,8 +1043,9 @@ gdbpy_decode_line (PyObject *self, PyObject *args)
   else
     unparsed = gdbpy_ref<>::new_reference (Py_None);
 
-  PyTuple_SetItem (return_result.get (), 0, unparsed.release ());
-  PyTuple_SetItem (return_result.get (), 1, result.release ());
+  if (PyTuple_SetItem (return_result.get (), 0, unparsed.release ()) < 0
+      || PyTuple_SetItem (return_result.get (), 1, result.release ()) < 0)
+    return nullptr;
 
   return return_result.release ();
 }
-- 
2.53.0