[PATCH] Wrap (and poison) PyObject_CallFunctionObjArgs

[Tom Tromey <tom@tromey.com>]

This patch adds a new wrapper for PyObject_CallFunctionObjArgs.  The
wrapper function knows how to unwrap gdbpy_ref<> and
gdbpy_borrowed_ref<>.  It rejects null arguments, and it automatically
supplies the trailing NULL required by PyObject_CallFunctionObjArgs.
Finally, PyObject_CallFunctionObjArgs is poisoned to avoid introducing
new calls.

The idea behind this change is that passing anything other than
PyObject* to this function will cause failures; this patch turns
silent failures (like passing a gdbpy_borrowed_ref<>) into a
compile-time failure.
---
 gdb/python/py-disasm.c        |  5 +--
 gdb/python/py-event.c         | 11 +++----
 gdb/python/py-framefilter.c   |  9 +++--
 gdb/python/py-prettyprint.c   |  3 +-
 gdb/python/py-record-btrace.c |  8 ++---
 gdb/python/py-tui.c           |  6 ++--
 gdb/python/py-unwind.c        |  2 +-
 gdb/python/python-internal.h  | 62 +++++++++++++++++++++++++++++++++++
 gdb/python/python.c           | 42 ++++++++++--------------
 gdb/varobj.c                  |  8 +++--
 10 files changed, 101 insertions(+), 55 deletions(-)

diff --git a/gdb/python/py-disasm.c b/gdb/python/py-disasm.c
index 48a2a4f6f8b..d99c899cf38 100644
--- a/gdb/python/py-disasm.c
+++ b/gdb/python/py-disasm.c
@@ -1233,10 +1233,7 @@ gdbpy_print_insn (struct gdbarch *gdbarch, CORE_ADDR memaddr,
 
   /* Call into the registered disassembler to (possibly) perform the
      disassembly.  */
-  gdbpy_ref<> result
-    (PyObject_CallFunctionObjArgs (hook.get (),
-				   disasm_info.get (),
-				   nullptr));
+  gdbpy_ref<> result = gdbpy_object_call_function_obj_args (hook, disasm_info);
 
   if (result == nullptr)
     {
diff --git a/gdb/python/py-event.c b/gdb/python/py-event.c
index e75ca91c784..27230bdfdb9 100644
--- a/gdb/python/py-event.c
+++ b/gdb/python/py-event.c
@@ -83,12 +83,11 @@ evpy_emit_event (gdbpy_opt_borrowed_ref<> event,
       if (func == NULL)
 	return -1;
 
-      /* This local exists so we don't pass an object through
-	 '...'.  */
-      PyObject *ev = event;
-      gdbpy_ref<> func_result (PyObject_CallFunctionObjArgs (func, ev,
-							     nullptr));
-
+      gdbpy_ref<> func_result;
+      if (event == nullptr)
+	func_result = gdbpy_object_call_function_obj_args (func);
+      else
+	func_result = gdbpy_object_call_function_obj_args (func, event);
       if (func_result == NULL)
 	{
 	  /* Print the trace here, but keep going -- we want to try to
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index 2777b944097..b4734d73d45 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -1103,11 +1103,10 @@ bootstrap_python_frame_filters (const frame_info_ptr &frame,
   if (py_frame_high == NULL)
     return NULL;
 
-  gdbpy_ref<> iterable (PyObject_CallFunctionObjArgs (sort_func.get (),
-						      frame_obj.get (),
-						      py_frame_low.get (),
-						      py_frame_high.get (),
-						      NULL));
+  gdbpy_ref<> iterable = gdbpy_object_call_function_obj_args (sort_func,
+							      frame_obj,
+							      py_frame_low,
+							      py_frame_high);
   if (iterable == NULL)
     return NULL;
 
diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index ce6ed699f3e..a7cbb6e045c 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -78,8 +78,7 @@ search_pp_list (PyObject *list, PyObject *value)
 	    continue;
 	}
 
-      gdbpy_ref<> printer (PyObject_CallFunctionObjArgs (function, value,
-							 NULL));
+      gdbpy_ref<> printer = gdbpy_object_call_function_obj_args (function, value);
       if (printer == NULL)
 	return NULL;
       else if (printer != Py_None)
diff --git a/gdb/python/py-record-btrace.c b/gdb/python/py-record-btrace.c
index a73167a5bdd..9249a340e06 100644
--- a/gdb/python/py-record-btrace.c
+++ b/gdb/python/py-record-btrace.c
@@ -826,11 +826,9 @@ recpy_call_filter (const uint64_t payload, std::optional<uint64_t> ip,
   else
     py_ip = gdb_py_object_from_ulongest (*ip);
 
-  gdbpy_ref<> py_result (PyObject_CallFunctionObjArgs ((PyObject *) ptw_filter,
-							py_payload.get (),
-							py_ip.get (),
-							nullptr));
-
+  gdbpy_ref<> py_result
+    = gdbpy_object_call_function_obj_args ((PyObject *) ptw_filter,
+					   py_payload, py_ip);
   if (py_result == nullptr)
     {
       gdbpy_print_stack ();
diff --git a/gdb/python/py-tui.c b/gdb/python/py-tui.c
index 465fbd2fe6d..9ecfff0ecf6 100644
--- a/gdb/python/py-tui.c
+++ b/gdb/python/py-tui.c
@@ -418,10 +418,8 @@ gdbpy_tui_window_maker::operator() (const char *win_name)
      which, this method should not be called.  */
   gdb_assert (m_constr != nullptr);
 
-  gdbpy_ref<> user_window
-    (PyObject_CallFunctionObjArgs (m_constr.get (),
-				   wrapper.get (),
-				   nullptr));
+  gdbpy_ref<> user_window = gdbpy_object_call_function_obj_args (m_constr,
+								 wrapper);
   if (user_window == nullptr)
     {
       gdbpy_print_stack ();
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 345203f8633..65bbd406510 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -898,7 +898,7 @@ frame_unwind_python::sniff (const frame_info_ptr &this_frame,
 
   /* A (gdb.UnwindInfo, str) tuple, or None.  */
   gdbpy_ref<> pyo_execute_ret
-    (PyObject_CallFunctionObjArgs (pyo_execute.get (), pfo.get (), nullptr));
+    = gdbpy_object_call_function_obj_args (pyo_execute, pfo);
   if (pyo_execute_ret == nullptr)
     {
       /* If the unwinder is cancelled due to a Ctrl-C, then propagate
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 2e8f35729cd..b08f2343f15 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -212,6 +212,68 @@ gdbpy_call_method (const gdbpy_ref<> &o, const char *method, Args... args)
 # define PyObject_CallMethod POISONED_PyObject_CallMethod
 #endif
 
+namespace detail
+{
+
+/* These are helpers for gdbpy_object_call_function_obj_args.  Each
+   function takes a single argument and returns a non-NULL
+   PyObject*.  */
+
+static inline PyObject *
+unwrap_ref (PyObject *val)
+{
+  gdb_assert (val != nullptr);
+  return val;
+}
+
+template<typename T>
+PyObject *
+unwrap_ref (const gdbpy_ref<T> &val)
+{
+  gdb_assert (val != nullptr);
+  return val.get ();
+}
+
+template<typename T>
+PyObject *
+unwrap_ref (gdbpy_borrowed_ref<T> val)
+{
+  /* Note that VAL cannot be nullptr here by construction.  */
+  return (PyObject *) val;
+}
+
+}
+
+/* A wrapper for PyObject_CallFunctionObjArgs that takes various kinds
+   of gdb wrappers, in addition to "PyObject *".  This variant does
+   not allow NULL arguments.  While PyObject_CallFunctionObjArgs
+   requires a trailing NULL, this function does not -- it supplies the
+   required trailing NULL on its own.
+
+   As a safety measure, no argument may be NULL.  While this may be
+   slightly inconvenient at times (you can't early-terminate the
+   arguments, you have to add a special case at the call site), it
+   avoids bugs where early termination was unintentional.  */
+template<typename Arg, typename... Args>
+static inline gdbpy_ref<>
+gdbpy_object_call_function_obj_args (Arg &&fn, Args && ...args)
+{
+  PyObject *result
+    = PyObject_CallFunctionObjArgs (detail::unwrap_ref (fn),
+				    detail::unwrap_ref (args)...,
+				    nullptr);
+  return gdbpy_ref<> (result);
+}
+
+/* Poison PyObject_CallFunctionObjArgs.  The typesafe wrapper
+   gdbpy_objects_call_function_obj_args should be used instead.  */
+#undef PyObject_CallFunctionObjArgs
+#ifdef __GNUC__
+# pragma GCC poison PyObject_CallFunctionObjArgs
+#else
+# define PyObject_CallFunctionObjArgs POISONED_PyObject_CallFunctionObjArgs
+#endif
+
 /* The 'name' parameter of PyErr_NewException was missing the 'const'
    qualifier in Python <= 3.4.  Hence, we wrap it in a function to
    avoid errors when compiled with -Werror.  */
diff --git a/gdb/python/python.c b/gdb/python/python.c
index df9f7c69006..1996915501a 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -1262,8 +1262,7 @@ gdbpy_before_prompt_hook (const struct extension_language_defn *extlang,
 	    }
 
 	  gdbpy_ref<> result
-	    (PyObject_CallFunctionObjArgs (hook.get (), current_prompt.get (),
-					   NULL));
+	    = gdbpy_object_call_function_obj_args (hook, current_prompt);
 	  if (result == NULL)
 	    {
 	      gdbpy_print_stack ();
@@ -1364,11 +1363,10 @@ gdbpy_colorize (const std::string &filename, const std::string &contents,
      contents (a bytes object).  This function should return either a bytes
      object, the same contents with styling applied, or None to indicate
      that no styling should be performed.  */
-  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
-						    fname_arg.get (),
-						    contents_arg.get (),
-						    lang_arg.get (),
-						    nullptr));
+  gdbpy_ref<> result = gdbpy_object_call_function_obj_args (hook,
+							    fname_arg,
+							    contents_arg,
+							    lang_arg);
   if (result == nullptr)
     {
       gdbpy_print_stack ();
@@ -1433,10 +1431,9 @@ gdbpy_colorize_disasm (const std::string &content, gdbarch *gdbarch)
       return {};
     }
 
-  gdbpy_ref<> result (PyObject_CallFunctionObjArgs (hook.get (),
-						    content_arg.get (),
-						    gdbarch_arg.get (),
-						    nullptr));
+  gdbpy_ref<> result = gdbpy_object_call_function_obj_args (hook,
+							    content_arg,
+							    gdbarch_arg);
   if (result == nullptr)
     {
       gdbpy_print_stack ();
@@ -1903,8 +1900,7 @@ gdbpy_handle_missing_debuginfo (const struct extension_language_defn *extlang,
 
   /* Call the function, passing in the Python objfile object.  */
   gdbpy_ref<> pyo_execute_ret
-    (PyObject_CallFunctionObjArgs (pyo_handler.get (), pyo_objfile.get (),
-				   nullptr));
+    = gdbpy_object_call_function_obj_args (pyo_handler, pyo_objfile);
   if (pyo_execute_ret == nullptr)
     {
       /* If the handler is cancelled due to a Ctrl-C, then propagate
@@ -2003,9 +1999,8 @@ gdbpy_find_objfile_from_buildid (const struct extension_language_defn *extlang,
 
   /* Call the function, passing in the Python objfile object.  */
   gdbpy_ref<> pyo_execute_ret
-    (PyObject_CallFunctionObjArgs (pyo_handler.get (), pyo_pspace.get (),
-				   pyo_buildid.get (), pyo_filename.get (),
-				   nullptr));
+    = gdbpy_object_call_function_obj_args (pyo_handler, pyo_pspace,
+					   pyo_buildid, pyo_filename);
   if (pyo_execute_ret == nullptr)
     {
       /* If the handler is cancelled due to a Ctrl-C, then propagate
@@ -2055,8 +2050,6 @@ static void
 gdbpy_start_type_printers (const struct extension_language_defn *extlang,
 			   struct ext_lang_type_printers *ext_printers)
 {
-  PyObject *printers_obj = NULL;
-
   if (!gdb_python_initialized)
     return;
 
@@ -2077,11 +2070,11 @@ gdbpy_start_type_printers (const struct extension_language_defn *extlang,
       return;
     }
 
-  printers_obj = PyObject_CallFunctionObjArgs (func.get (), (char *) NULL);
-  if (printers_obj == NULL)
+  gdbpy_ref<> printers_obj = gdbpy_object_call_function_obj_args (func);
+  if (printers_obj == nullptr)
     gdbpy_print_stack ();
   else
-    ext_printers->py_type_printers = printers_obj;
+    ext_printers->py_type_printers = printers_obj.release ();
 }
 
 /* If TYPE is recognized by some type printer, store in *PRETTIED_TYPE
@@ -2130,10 +2123,9 @@ gdbpy_apply_type_printers (const struct extension_language_defn *extlang,
       return EXT_LANG_RC_ERROR;
     }
 
-  gdbpy_ref<> result_obj (PyObject_CallFunctionObjArgs (func.get (),
-							printers_obj,
-							type_obj.get (),
-							(char *) NULL));
+  gdbpy_ref<> result_obj = gdbpy_object_call_function_obj_args (func,
+								printers_obj,
+								type_obj);
   if (result_obj == NULL)
     {
       gdbpy_print_stack ();
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 812de2b27ba..85ae1a21da6 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -467,11 +467,13 @@ varobj_delete (struct varobj *var, bool only_children)
 static PyObject *
 instantiate_pretty_printer (PyObject *constructor, struct value *value)
 {
-  gdbpy_ref<> val_obj (value_to_value_object (value));
+  gdbpy_ref<> val_obj = value_to_value_object (value);
   if (val_obj == nullptr)
-    return NULL;
+    return nullptr;
 
-  return PyObject_CallFunctionObjArgs (constructor, val_obj.get (), NULL);
+  gdbpy_ref<> result = gdbpy_object_call_function_obj_args (constructor,
+							    val_obj);
+  return result.release ();
 }
 
 #endif
-- 
2.49.0