[PATCH 0/4] Convert py-frame.c to Python safety API

[PATCH 0/4] Convert py-frame.c to Python safety API

[Tom Tromey]

This series converts py-frame.c to the Python safety API.

I've split it into multiple parts to make it perhaps a bit easier to
review.  Often I find these patches pretty hard to read, since they
involve converting a bunch of functions into methods, making them
fairly messy.

A few FIXME comments are added; as with other cases, these can all be
removed once the conversion is complete.  At the current rate this may
be a while.

Tested on x86-64 Fedora 40.

Signed-off-by: Tom Tromey <tom@tromey.com>
---
Tom Tromey (4):
      Basic safety conversion of py-frame.c
      Convert gdbpy_newest_frame and gdbpy_selected_frame to safety API
      Convert gdbpy_frame_stop_reason_string to the safety API
      Convert frapy_richcompare to safety API

 gdb/python/py-frame.c                 | 852 +++++++++++++---------------------
 gdb/python/py-inferior.c              |  24 +-
 gdb/python/py-safety.h                |  53 +++
 gdb/python/python-internal.h          |   7 +-
 gdb/python/python.c                   |  12 +-
 gdb/testsuite/gdb.python/py-frame.exp |   3 +
 6 files changed, 408 insertions(+), 543 deletions(-)
---
base-commit: 07ee9e316ab6d53b5ed1aae8fe7448a559abe4be
change-id: 20260819-python-safety-frame-c795afcc47df

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

[PATCH 1/4] Basic safety conversion of py-frame.c

[Tom Tromey]

This converts the bulk of py-frame.c to the new Python safety API.
---
 gdb/python/py-frame.c | 764 +++++++++++++++++++-------------------------------
 1 file changed, 289 insertions(+), 475 deletions(-)

diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index afa89f0d112..5c5ff8cf0de 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -32,22 +32,125 @@ struct frame_object : public PyObject
 {
   struct frame_id frame_id;
   struct gdbarch *gdbarch;
+
+  /* Marks that the FRAME_ID member actually holds the ID of the frame next
+     to this, and not this frames' ID itself.  This is a hack to permit Python
+     frame objects which represent invalid frames (i.e., the last frame_info
+     in a corrupt stack).  The problem arises from the fact that this code
+     relies on FRAME_ID to uniquely identify a frame, which is not always true
+     for the last "frame" in a corrupt stack (it can have a null ID, or the same
+     ID as the  previous frame).  Whenever get_prev_frame returns NULL, we
+     record the frame_id of the next frame and set FRAME_ID_IS_NEXT to 1.  */
+  int frame_id_is_next;
+
+  /* Return the associated frame.  Throws an exception on error.  */
+  frame_info_ptr require_frame ()
+  {
+    frame_info_ptr frame = frame_object_to_frame_info (this);
+    /* FIXME: Python safety.  frame_object_to_frame_info should be
+       converted, but the callers aren't ready yet.  */
+    if (frame == nullptr)
+      gdbpy_err_format (PyExc_RuntimeError, _("Frame is invalid."));
+    return frame;
+  }
+
+  /* Called by the Python interpreter to obtain string representation
+     of the object.  */
+  gdbpy_ref<> str ()
+  {
+    return gdbpy_unicode_from_string (frame_id.to_string ());
+  }
+
+  /* Implement repr() for gdb.Frame.  */
+  gdbpy_ref<> repr ();
+
+  /* Implementation of gdb.Frame.is_valid (self) -> Boolean.  Returns
+     True if the frame corresponding to the frame_id of this object
+     still exists in the inferior.  */
+  bool is_valid ()
+  {
+    return frame_object_to_frame_info (this) != nullptr;
+  }
+
+  /* Implementation of gdb.Frame.name (self) -> String.
+     Returns the name of the function corresponding to this frame.  */
+  gdb::unique_xmalloc_ptr<char> name ();
+
+  /* Implementation of gdb.Frame.type (self) -> Integer.
+     Returns the frame type, namely one of the gdb.*_FRAME constants.  */
+  ULONGEST type ();
+
+  /* Implementation of gdb.Frame.architecture (self) -> gdb.Architecture.
+     Returns the frame's architecture as a gdb.Architecture object.  */
+  gdbpy_ref<> arch ();
+
+  /* Implementation of gdb.Frame.unwind_stop_reason (self) -> Integer.
+     Returns one of the gdb.FRAME_UNWIND_* constants.  */
+  int unwind_stop_reason ();
+
+  /* Implementation of gdb.Frame.pc (self) -> Long.
+     Returns the frame's resume address.  */
+  ULONGEST pc ();
+
+  /* Implementation of gdb.Frame.read_register (self, register) -> gdb.Value.
+     Returns the value of a register in this frame.  */
+  gdbpy_ref<> read_register (gdbpy_borrowed_ref<> args,
+			     gdbpy_opt_borrowed_ref<> kw);
+
+  /* Implementation of gdb.Frame.block (self) -> gdb.Block.
+     Returns the frame's code block.  */
+  gdbpy_ref<> block ();
+
+  /* Implementation of gdb.Frame.function (self) -> gdb.Symbol.
+     Returns the symbol for the function corresponding to this frame.  */
+  gdbpy_ref<> function ();
+
+  /* Implementation of gdb.Frame.older (self) -> gdb.Frame.
+     Returns the frame immediately older (outer) to this frame, or None if
+     there isn't one.  */
+  gdbpy_ref<> older ();
+
+  /* Implementation of gdb.Frame.newer (self) -> gdb.Frame.
+     Returns the frame immediately newer (inner) to this frame, or None if
+     there isn't one.  */
+  gdbpy_ref<> newer ();
+
+  /* Implementation of gdb.Frame.find_sal (self) -> gdb.Symtab_and_line.
+     Returns the frame's symtab and line.  */
+  gdbpy_ref<> find_sal ();
+
+  /* Implementation of gdb.Frame.read_var_value (self, variable,
+     [block]) -> gdb.Value.  If the optional block argument is provided
+     start the search from that block, otherwise search from the frame's
+     current block (determined by examining the resume address of the
+     frame).  The variable argument must be a string or an instance of a
+     gdb.Symbol.  The block argument must be an instance of gdb.Block.  Returns
+     NULL on error, with a python exception set.  */
+  gdbpy_ref<> read_var (gdbpy_borrowed_ref<> args,
+			gdbpy_opt_borrowed_ref<> kw);
+
+  /* Select this frame.  */
+  void select ();
+
+  /* The stack frame level for this frame.  */
+  int level ();
+
+  /* The language for this frame.  */
+  const char *language ();
+
+  /* The static link for this frame.  */
+  gdbpy_ref<> static_link ();
+
+  static PyTypeObject *corresponding_object_type;
 };
 
 static_assert (gdb::is_python_allocatable_v<frame_object>);
 
-/* Require a valid frame.  This must be called inside a TRY_CATCH, or
-   another context in which a gdb exception is allowed.  */
-#define FRAPY_REQUIRE_VALID(frame_obj, frame)		\
-    do {						\
-      frame = frame_object_to_frame_info (frame_obj);	\
-      if (frame == NULL)				\
-	error (_("Frame is invalid."));			\
-    } while (0)
-
 /* Returns the frame_info object corresponding to the given Python Frame
    object.  If the frame doesn't exist anymore (the frame id doesn't
    correspond to any frame in the inferior), returns NULL.  */
+/* FIXME: Python safety.  This function should be converted, but the
+   callers aren't ready yet.  */
 
 frame_info_ptr
 frame_object_to_frame_info (PyObject *obj)
@@ -62,291 +165,127 @@ frame_object_to_frame_info (PyObject *obj)
   return frame;
 }
 
-/* Called by the Python interpreter to obtain string representation
-   of the object.  */
-
-static PyObject *
-frapy_str (PyObject *self)
-{
-  const frame_id &fid = ((frame_object *) self)->frame_id;
-  return PyUnicode_FromString (fid.to_string ().c_str ());
-}
-
-/* Implement repr() for gdb.Frame.  */
-
-static PyObject *
-frapy_repr (PyObject *self)
+gdbpy_ref<>
+frame_object::repr ()
 {
-  frame_object *frame_obj = (frame_object *) self;
-  frame_info_ptr f_info = frame_find_by_id (frame_obj->frame_id);
+  frame_info_ptr f_info = frame_find_by_id (frame_id);
   if (f_info == nullptr)
-    return gdb_py_invalid_object_repr (self);
-
-  const frame_id &fid = frame_obj->frame_id;
-  return PyUnicode_FromFormat ("<%s level=%d frame-id=%s>",
-			       gdbpy_py_obj_tp_name (self).c_str (),
-			       frame_relative_level (f_info),
-			       fid.to_string ().c_str ());
+    /* FIXME: Python safety.  gdb_py_invalid_object_repr should
+       throw on error. */
+    return gdbpy_ref<> (gdb_py_invalid_object_repr (this));
+
+  return gdbpy_unicode_from_format ("<%s level=%d frame-id=%s>",
+				    gdbpy_py_obj_tp_name (this).c_str (),
+				    frame_relative_level (f_info),
+				    frame_id.to_string ().c_str ());
 }
 
-/* Implementation of gdb.Frame.is_valid (self) -> Boolean.
-   Returns True if the frame corresponding to the frame_id of this
-   object still exists in the inferior.  */
-
-static PyObject *
-frapy_is_valid (PyObject *self, PyObject *args)
+gdb::unique_xmalloc_ptr<char>
+frame_object::name ()
 {
-  frame_info_ptr frame = NULL;
-
-  try
-    {
-      frame = frame_object_to_frame_info (self);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  if (frame == NULL)
-    return py_false ().release ();
-
-  return py_true ().release ();
-}
-
-/* Implementation of gdb.Frame.name (self) -> String.
-   Returns the name of the function corresponding to this frame.  */
-
-static PyObject *
-frapy_name (PyObject *self, PyObject *args)
-{
-  frame_info_ptr frame;
-  gdb::unique_xmalloc_ptr<char> name;
+  frame_info_ptr frame = require_frame ();
   enum language lang;
-  PyObject *result;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
-
-      name = find_frame_funname (frame, &lang, NULL);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  if (name)
-    {
-      result = PyUnicode_Decode (name.get (), strlen (name.get ()),
-				 host_charset (), NULL);
-    }
-  else
-    {
-      result = py_none ().release ();
-    }
-
-  return result;
+  return find_frame_funname (frame, &lang, nullptr);
 }
 
-/* Implementation of gdb.Frame.type (self) -> Integer.
-   Returns the frame type, namely one of the gdb.*_FRAME constants.  */
-
-static PyObject *
-frapy_type (PyObject *self, PyObject *args)
+ULONGEST
+frame_object::type ()
 {
-  frame_info_ptr frame;
-  enum frame_type type = NORMAL_FRAME;/* Initialize to appease gcc warning.  */
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
-
-      type = get_frame_type (frame);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  return gdb_py_object_from_longest (type).release ();
+  frame_info_ptr frame = require_frame ();
+  return get_frame_type (frame);
 }
 
-/* Implementation of gdb.Frame.architecture (self) -> gdb.Architecture.
-   Returns the frame's architecture as a gdb.Architecture object.  */
-
-static PyObject *
-frapy_arch (PyObject *self, PyObject *args)
+gdbpy_ref<>
+frame_object::arch ()
 {
-  frame_info_ptr frame = NULL;    /* Initialize to appease gcc warning.  */
-  frame_object *obj = (frame_object *) self;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  return gdbarch_to_arch_object (obj->gdbarch).release ();
+  require_frame ();
+  return gdbarch_to_arch_object (gdbarch);
 }
 
-/* Implementation of gdb.Frame.unwind_stop_reason (self) -> Integer.
-   Returns one of the gdb.FRAME_UNWIND_* constants.  */
-
-static PyObject *
-frapy_unwind_stop_reason (PyObject *self, PyObject *args)
+int
+frame_object::unwind_stop_reason ()
 {
-  frame_info_ptr frame = NULL;    /* Initialize to appease gcc warning.  */
-  enum unwind_stop_reason stop_reason;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  stop_reason = get_frame_unwind_stop_reason (frame);
-
-  return gdb_py_object_from_longest (stop_reason).release ();
+  frame_info_ptr frame = require_frame ();
+  return get_frame_unwind_stop_reason (frame);
 }
 
-/* Implementation of gdb.Frame.pc (self) -> Long.
-   Returns the frame's resume address.  */
-
-static PyObject *
-frapy_pc (PyObject *self, PyObject *args)
+ULONGEST
+frame_object::pc ()
 {
-  CORE_ADDR pc = 0;	      /* Initialize to appease gcc warning.  */
-  frame_info_ptr frame;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
-
-      pc = get_frame_pc (frame);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  return gdb_py_object_from_ulongest (pc).release ();
+  frame_info_ptr frame = require_frame ();
+  return get_frame_pc (frame);
 }
 
-/* Implementation of gdb.Frame.read_register (self, register) -> gdb.Value.
-   Returns the value of a register in this frame.  */
-
-static PyObject *
-frapy_read_register (PyObject *self, PyObject *args, PyObject *kw)
+gdbpy_ref<>
+frame_object::read_register (gdbpy_borrowed_ref<> args,
+			     gdbpy_opt_borrowed_ref<> kw)
 {
   PyObject *pyo_reg_id;
-  gdbpy_ref<> result;
 
   static const char *keywords[] = { "register", nullptr };
-  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "O", keywords, &pyo_reg_id))
-    return nullptr;
-
-  try
-    {
-      scoped_value_mark free_values;
-      frame_info_ptr frame;
-      int regnum;
-
-      FRAPY_REQUIRE_VALID (self, frame);
-
-      if (!gdbpy_parse_register_id (get_frame_arch (frame), pyo_reg_id,
-				    &regnum))
-	return nullptr;
+  gdbpy_arg_parse_tuple_and_keywords (args, kw, "O", keywords, &pyo_reg_id);
 
-      gdb_assert (regnum >= 0);
-      value *val
-	= value_of_register (regnum, get_next_frame_sentinel_okay (frame));
+  scoped_value_mark free_values;
+  frame_info_ptr frame = require_frame ();
 
-      if (val == NULL)
-	PyErr_SetString (PyExc_ValueError, _("Can't read register."));
-      else
-	result = value_to_value_object (val);
-    }
-  catch (const gdb_exception &except)
+  int regnum;
+  if (!gdbpy_parse_register_id (get_frame_arch (frame), pyo_reg_id, &regnum))
     {
-      return gdbpy_handle_gdb_exception (nullptr, except);
+      /* FIXME: Python safety.  gdbpy_parse_register_id should throw
+	 on error.  */
+      throw gdb_python_exception ();
     }
 
-  return result.release ();
-}
+  gdb_assert (regnum >= 0);
+  value *val
+    = value_of_register (regnum, get_next_frame_sentinel_okay (frame));
 
-/* Implementation of gdb.Frame.block (self) -> gdb.Block.
-   Returns the frame's code block.  */
+  if (val == nullptr)
+    gdbpy_err_set_string (PyExc_ValueError, _("Can't read register."));
 
-static PyObject *
-frapy_block (PyObject *self, PyObject *args)
-{
-  frame_info_ptr frame;
-  const struct block *block = NULL, *fn_block;
+  return value_to_value_object (val);
+}
 
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
-      block = get_frame_block (frame, NULL);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
+gdbpy_ref<>
+frame_object::block ()
+{
+  frame_info_ptr frame = require_frame ();
+  const struct block *block = get_frame_block (frame, nullptr);
 
+  const struct block *fn_block;
   for (fn_block = block;
        fn_block != NULL && fn_block->function () == NULL;
        fn_block = fn_block->superblock ())
     ;
 
   if (block == NULL || fn_block == NULL || fn_block->function () == NULL)
-    {
-      PyErr_SetString (PyExc_RuntimeError,
-		       _("Cannot locate block for frame."));
-      return NULL;
-    }
+    gdbpy_err_set_string (PyExc_RuntimeError,
+			  _("Cannot locate block for frame."));
 
-  return block_to_block_object (block,
-				fn_block->function ()->objfile ()).release ();
+  return block_to_block_object (block, fn_block->function ()->objfile ());
 }
 
 
-/* Implementation of gdb.Frame.function (self) -> gdb.Symbol.
-   Returns the symbol for the function corresponding to this frame.  */
-
-static PyObject *
-frapy_function (PyObject *self, PyObject *args)
+gdbpy_ref<>
+frame_object::function ()
 {
-  struct symbol *sym = NULL;
-  frame_info_ptr frame;
-
-  try
-    {
-      enum language funlang;
+  frame_info_ptr frame = require_frame ();
 
-      FRAPY_REQUIRE_VALID (self, frame);
+  struct symbol *sym = nullptr;
+  enum language funlang;
+  gdb::unique_xmalloc_ptr<char> funname
+    = find_frame_funname (frame, &funlang, &sym);
 
-      gdb::unique_xmalloc_ptr<char> funname
-	= find_frame_funname (frame, &funlang, &sym);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  if (sym)
-    return symbol_to_symbol_object (sym).release ();
+  if (sym != nullptr)
+    return symbol_to_symbol_object (sym);
 
-  return py_none ().release ();
+  return py_none ();
 }
 
 /* Convert a frame_info struct to a Python Frame object.
    Sets a Python exception and returns NULL on error.  */
+/* FIXME: Python safety.  This function should be converted, but the
+   callers aren't ready yet.  */
 
 gdbpy_ref<>
 frame_info_to_frame_object (const frame_info_ptr &frame)
@@ -369,106 +308,51 @@ frame_info_to_frame_object (const frame_info_ptr &frame)
   return frame_obj;
 }
 
-/* Implementation of gdb.Frame.older (self) -> gdb.Frame.
-   Returns the frame immediately older (outer) to this frame, or None if
-   there isn't one.  */
-
-static PyObject *
-frapy_older (PyObject *self, PyObject *args)
+gdbpy_ref<>
+frame_object::older ()
 {
-  frame_info_ptr frame, prev = NULL;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
+  frame_info_ptr frame = require_frame ();
+  frame_info_ptr prev = get_prev_frame (frame);
 
-      prev = get_prev_frame (frame);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  gdbpy_ref<> prev_obj;
   if (prev)
-    prev_obj = frame_info_to_frame_object (prev);
-  else
-    prev_obj = py_none ();
+    return frame_info_to_frame_object (prev);
 
-  return prev_obj.release ();
+  return py_none ();
 }
 
-/* Implementation of gdb.Frame.newer (self) -> gdb.Frame.
-   Returns the frame immediately newer (inner) to this frame, or None if
-   there isn't one.  */
-
-static PyObject *
-frapy_newer (PyObject *self, PyObject *args)
+gdbpy_ref<>
+frame_object::newer ()
 {
-  frame_info_ptr frame, next = NULL;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
-
-      next = get_next_frame (frame);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
+  frame_info_ptr frame = require_frame ();
+  frame_info_ptr next = get_next_frame (frame);
 
-  gdbpy_ref<> next_obj;
   if (next)
-    next_obj = frame_info_to_frame_object (next);
-  else
-    next_obj = py_none ();
+    return frame_info_to_frame_object (next);
 
-  return next_obj.release ();
+  return py_none ();
 }
 
-/* Implementation of gdb.Frame.find_sal (self) -> gdb.Symtab_and_line.
-   Returns the frame's symtab and line.  */
-
-static PyObject *
-frapy_find_sal (PyObject *self, PyObject *args)
+gdbpy_ref<>
+frame_object::find_sal ()
 {
-  frame_info_ptr frame;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
-
-      symtab_and_line sal = find_frame_sal (frame);
-      return symtab_and_line_to_sal_object (sal).release ();
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
+  frame_info_ptr frame = require_frame ();
+  symtab_and_line sal = find_frame_sal (frame);
+  return symtab_and_line_to_sal_object (sal);
 }
 
-/* Implementation of gdb.Frame.read_var_value (self, variable,
-   [block]) -> gdb.Value.  If the optional block argument is provided
-   start the search from that block, otherwise search from the frame's
-   current block (determined by examining the resume address of the
-   frame).  The variable argument must be a string or an instance of a
-   gdb.Symbol.  The block argument must be an instance of gdb.Block.  Returns
-   NULL on error, with a python exception set.  */
-static PyObject *
-frapy_read_var (PyObject *self, PyObject *args, PyObject *kw)
+gdbpy_ref<>
+frame_object::read_var (gdbpy_borrowed_ref<> args,
+			gdbpy_opt_borrowed_ref<> kw)
 {
-  frame_info_ptr frame;
   PyObject *sym_obj, *block_obj = NULL;
-  struct symbol *var = NULL;	/* gcc-4.3.2 false warning.  */
-  const struct block *block = NULL;
 
   static const char *keywords[] = { "variable", "block", nullptr };
-  if (!gdb_PyArg_ParseTupleAndKeywords (args, kw, "O|O!", keywords,
-					&sym_obj, &block_object_type,
-					&block_obj))
-    return nullptr;
+  gdbpy_arg_parse_tuple_and_keywords (args, kw, "O|O!", keywords,
+				      &sym_obj, &block_object_type,
+				      &block_obj);
 
+  const struct block *block = NULL;
+  struct symbol *var = NULL;	/* gcc-4.3.2 false warning.  */
   if (PyObject_TypeCheck (sym_obj, &symbol_object_type))
     var = symbol_object_to_symbol (sym_obj);
   else if (gdbpy_is_string (sym_obj))
@@ -476,8 +360,10 @@ frapy_read_var (PyObject *self, PyObject *args, PyObject *kw)
       gdb::unique_xmalloc_ptr<char>
 	var_name (python_string_to_target_string (sym_obj));
 
-      if (!var_name)
-	return NULL;
+      /* FIXME: Python safety.  python_string_to_target_string should
+	 throw on error.  */
+      if (var_name == nullptr)
+	throw gdb_python_exception ();
 
       if (block_obj != nullptr)
 	{
@@ -488,139 +374,65 @@ frapy_read_var (PyObject *self, PyObject *args, PyObject *kw)
 	  gdb_assert (block != nullptr);
 	}
 
-      try
-	{
-	  struct block_symbol lookup_sym;
-	  FRAPY_REQUIRE_VALID (self, frame);
-
-	  if (!block)
-	    block = get_frame_block (frame, NULL);
-	  lookup_sym = lookup_symbol (var_name.get (), block,
-				      SEARCH_VFT, nullptr);
-	  var = lookup_sym.symbol;
-	  block = lookup_sym.block;
-	}
-      catch (const gdb_exception &except)
-	{
-	  return gdbpy_handle_gdb_exception (nullptr, except);
-	}
+      frame_info_ptr frame = require_frame ();
 
-      if (!var)
-	{
-	  PyErr_Format (PyExc_ValueError,
-			_("Variable '%s' not found."), var_name.get ());
+      if (!block)
+	block = get_frame_block (frame, NULL);
+      block_symbol lookup_sym = lookup_symbol (var_name.get (), block,
+					       SEARCH_VFT, nullptr);
+      var = lookup_sym.symbol;
+      block = lookup_sym.block;
 
-	  return NULL;
-	}
+      if (var == nullptr)
+	gdbpy_err_format (PyExc_ValueError,
+			  _("Variable '%s' not found."), var_name.get ());
     }
   else
-    {
-      PyErr_Format (PyExc_TypeError,
-		    _("argument 1 must be gdb.Symbol or str, not %s"),
-		    gdbpy_py_obj_tp_name (sym_obj).c_str ());
-      return NULL;
-    }
-
-  gdbpy_ref<> result;
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, frame);
-
-      scoped_value_mark free_values;
-      struct value *val = read_var_value (var, block, frame);
-      result = value_to_value_object (val);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  return result.release ();
+    gdbpy_err_format (PyExc_TypeError,
+		      _("argument 1 must be gdb.Symbol or str, not %s"),
+		      gdbpy_py_obj_tp_name (sym_obj).c_str ());
+
+  frame_info_ptr frame = require_frame ();
+  scoped_value_mark free_values;
+  struct value *val = read_var_value (var, block, frame);
+  return value_to_value_object (val);
 }
 
-/* Select this frame.  */
-
-static PyObject *
-frapy_select (PyObject *self, PyObject *args)
+void
+frame_object::select ()
 {
-  frame_info_ptr fi;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, fi);
-
-      select_frame (fi);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  return py_none ().release ();
+  frame_info_ptr fi = require_frame ();
+  select_frame (fi);
 }
 
-/* The stack frame level for this frame.  */
-
-static PyObject *
-frapy_level (PyObject *self, PyObject *args)
+int
+frame_object::level ()
 {
-  frame_info_ptr fi;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, fi);
-
-      return gdb_py_object_from_longest (frame_relative_level (fi)).release ();
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
+  frame_info_ptr fi = require_frame ();
+  return frame_relative_level (fi);
 }
 
-/* The language for this frame.  */
-
-static PyObject *
-frapy_language (PyObject *self, PyObject *args)
+const char *
+frame_object::language ()
 {
-  try
-    {
-      frame_info_ptr fi;
-      FRAPY_REQUIRE_VALID (self, fi);
+  frame_info_ptr fi = require_frame ();
 
-      enum language lang = get_frame_language (fi);
-      const language_defn *lang_def = language_def (lang);
+  enum language lang = get_frame_language (fi);
+  const language_defn *lang_def = language_def (lang);
 
-      return host_string_to_python_string (lang_def->name ()).release ();
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
+  return lang_def->name ();
 }
 
-/* The static link for this frame.  */
-
-static PyObject *
-frapy_static_link (PyObject *self, PyObject *args)
+gdbpy_ref<>
+frame_object::static_link ()
 {
-  frame_info_ptr link;
-
-  try
-    {
-      FRAPY_REQUIRE_VALID (self, link);
-
-      link = frame_follow_static_link (link);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
+  frame_info_ptr link = require_frame ();
+  link = frame_follow_static_link (link);
 
   if (link == nullptr)
-    return py_none ().release ();
+    return py_none ();
 
-  return frame_info_to_frame_object (link).release ();
+  return frame_info_to_frame_object (link);
 }
 
 /* Implementation of gdb.newest_frame () -> gdb.Frame.
@@ -712,6 +524,8 @@ frapy_richcompare (PyObject *self, PyObject *other, int op)
   return py_false ().release ();
 }
 
+PyTypeObject *frame_object::corresponding_object_type = &frame_object_type;
+
 /* Sets up the Frame API in the gdb module.  */
 
 static int
@@ -750,54 +564,54 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_frames);
 \f
 
 static PyMethodDef frame_object_methods[] = {
-  { "is_valid", frapy_is_valid, METH_NOARGS,
+  noargs_method<frame_object, &frame_object::is_valid> ("is_valid",
     "is_valid () -> Boolean.\n\
-Return true if this frame is valid, false if not." },
-  { "name", frapy_name, METH_NOARGS,
+Return true if this frame is valid, false if not."),
+  noargs_method<frame_object, &frame_object::name> ("name",
     "name () -> String.\n\
-Return the function name of the frame, or None if it can't be determined." },
-  { "type", frapy_type, METH_NOARGS,
+Return the function name of the frame, or None if it can't be determined."),
+  noargs_method<frame_object, &frame_object::type> ("type",
     "type () -> Integer.\n\
-Return the type of the frame." },
-  { "architecture", frapy_arch, METH_NOARGS,
+Return the type of the frame."),
+  noargs_method<frame_object, &frame_object::arch> ("architecture",
     "architecture () -> gdb.Architecture.\n\
-Return the architecture of the frame." },
-  { "unwind_stop_reason", frapy_unwind_stop_reason, METH_NOARGS,
-    "unwind_stop_reason () -> Integer.\n\
-Return the reason why it's not possible to find frames older than this." },
-  { "pc", frapy_pc, METH_NOARGS,
+Return the architecture of the frame."),
+  noargs_method<frame_object, &frame_object::unwind_stop_reason>
+    ("unwind_stop_reason",
+     "unwind_stop_reason () -> Integer.\n\
+Return the reason why it's not possible to find frames older than this."),
+  noargs_method<frame_object, &frame_object::pc> ("pc",
     "pc () -> Long.\n\
-Return the frame's resume address." },
-  { "read_register", (PyCFunction) frapy_read_register,
-    METH_VARARGS | METH_KEYWORDS,
+Return the frame's resume address."),
+  varargs_method<frame_object, &frame_object::read_register> ("read_register",
     "read_register (register_name) -> gdb.Value\n\
-Return the value of the register in the frame." },
-  { "block", frapy_block, METH_NOARGS,
+Return the value of the register in the frame."),
+  noargs_method<frame_object, &frame_object::block> ("block",
     "block () -> gdb.Block.\n\
-Return the frame's code block." },
-  { "function", frapy_function, METH_NOARGS,
+Return the frame's code block."),
+  noargs_method<frame_object, &frame_object::function> ("function",
     "function () -> gdb.Symbol.\n\
-Returns the symbol for the function corresponding to this frame." },
-  { "older", frapy_older, METH_NOARGS,
+Returns the symbol for the function corresponding to this frame."),
+  noargs_method<frame_object, &frame_object::older> ("older",
     "older () -> gdb.Frame.\n\
-Return the frame that called this frame." },
-  { "newer", frapy_newer, METH_NOARGS,
+Return the frame that called this frame."),
+  noargs_method<frame_object, &frame_object::newer> ("newer",
     "newer () -> gdb.Frame.\n\
-Return the frame called by this frame." },
-  { "find_sal", frapy_find_sal, METH_NOARGS,
+Return the frame called by this frame."),
+  noargs_method<frame_object, &frame_object::find_sal> ("find_sal",
     "find_sal () -> gdb.Symtab_and_line.\n\
-Return the frame's symtab and line." },
-  { "read_var", (PyCFunction) frapy_read_var, METH_VARARGS | METH_KEYWORDS,
+Return the frame's symtab and line."),
+  varargs_method<frame_object, &frame_object::read_var> ("read_var",
     "read_var (variable) -> gdb.Value.\n\
-Return the value of the variable in this frame." },
-  { "select", frapy_select, METH_NOARGS,
-    "Select this frame as the user's current frame." },
-  { "level", frapy_level, METH_NOARGS,
-    "The stack level of this frame." },
-  { "language", frapy_language, METH_NOARGS,
-    "The language of this frame." },
-  { "static_link", frapy_static_link, METH_NOARGS,
-    "The static link of this frame, or None." },
+Return the value of the variable in this frame."),
+  noargs_method<frame_object, &frame_object::select> ("select",
+    "Select this frame as the user's current frame."),
+  noargs_method<frame_object, &frame_object::level> ("level",
+    "The stack level of this frame."),
+  noargs_method<frame_object, &frame_object::language> ("language",
+    "The language of this frame."),
+  noargs_method<frame_object, &frame_object::static_link> ("static_link",
+    "The static link of this frame, or None."),
   {NULL}  /* Sentinel */
 };
 
@@ -811,13 +625,13 @@ PyTypeObject frame_object_type = {
   0,				  /* tp_getattr */
   0,				  /* tp_setattr */
   0,				  /* tp_compare */
-  frapy_repr,			  /* tp_repr */
+  wrap_tp_callback<frame_object, &frame_object::repr>, /* tp_repr */
   0,				  /* tp_as_number */
   0,				  /* tp_as_sequence */
   0,				  /* tp_as_mapping */
   0,				  /* tp_hash  */
   0,				  /* tp_call */
-  frapy_str,			  /* tp_str */
+  wrap_tp_callback<frame_object, &frame_object::str>, /* tp_str */
   0,				  /* tp_getattro */
   0,				  /* tp_setattro */
   0,				  /* tp_as_buffer */

-- 
2.49.0

[PATCH 2/4] Convert gdbpy_newest_frame and gdbpy_selected_frame to safety API

[Tom Tromey]

This convert gdbpy_newest_frame and gdbpy_selected_frame to the Python
safety API.  Some extra work was needed in py-inferior.c; note that
the code there is "temporary" -- once event emission is converted, the
try/catch can be removed.  Also, a new noargs_function wrapper was
needed.
---
 gdb/python/py-frame.c        | 44 ++++++++++++++++----------------------------
 gdb/python/py-inferior.c     | 24 ++++++++++++++++++------
 gdb/python/py-safety.h       | 21 +++++++++++++++++++++
 gdb/python/python-internal.h |  4 ++--
 gdb/python/python.c          |  8 ++++----
 5 files changed, 61 insertions(+), 40 deletions(-)

diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 5c5ff8cf0de..3910f19ef83 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -438,41 +438,29 @@ frame_object::static_link ()
 /* Implementation of gdb.newest_frame () -> gdb.Frame.
    Returns the newest frame object.  */
 
-PyObject *
-gdbpy_newest_frame (PyObject *self, PyObject *args)
+gdbpy_ref<>
+gdbpy_newest_frame ()
 {
-  frame_info_ptr frame = NULL;
-
-  try
-    {
-      frame = get_current_frame ();
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  return frame_info_to_frame_object (frame).release ();
+  /* FIXME: Python safety.  Convert frame_info_to_frame_object.  */
+  gdbpy_ref<> result = frame_info_to_frame_object (get_current_frame ());
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
 }
 
 /* Implementation of gdb.selected_frame () -> gdb.Frame.
    Returns the selected frame object.  */
 
-PyObject *
-gdbpy_selected_frame (PyObject *self, PyObject *args)
+gdbpy_ref<>
+gdbpy_selected_frame ()
 {
-  frame_info_ptr frame = NULL;
-
-  try
-    {
-      frame = get_selected_frame ("No frame is currently selected.");
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (nullptr, except);
-    }
-
-  return frame_info_to_frame_object (frame).release ();
+  frame_info_ptr frame
+    = get_selected_frame ("No frame is currently selected.");
+  /* FIXME: Python safety.  Convert frame_info_to_frame_object.  */
+  gdbpy_ref<> result = frame_info_to_frame_object (frame);
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
 }
 
 /* Implementation of gdb.stop_reason_string (Integer) -> String.
diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index 426aec31e9e..780f7271c09 100644
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -1023,13 +1023,25 @@ python_context_changed (user_selected_what selection)
     }
 
   gdbpy_ref<> frame_obj;
-  if (has_stack_frames ())
-    frame_obj = gdbpy_ref<> (gdbpy_selected_frame (nullptr, nullptr));
-  else
-    frame_obj = py_none ();
-
-  if (frame_obj == nullptr)
+  /* FIXME: Python safety.  Eventually this function will be converted
+     and this try/catch can be removed.  */
+  try
+    {
+      if (has_stack_frames ())
+	frame_obj = gdbpy_selected_frame ();
+      else
+	frame_obj = py_none ();
+    }
+  catch (const gdb_python_exception &e)
+    {
+      gdbpy_print_stack ();
+      return;
+    }
+  catch (const gdb_exception &exc)
     {
+      /* This is a bit roundabout but we're going to be deleting this
+	 code someday anyway.  */
+      (void) gdbpy_handle_gdb_exception (nullptr, exc);
       gdbpy_print_stack ();
       return;
     }
diff --git a/gdb/python/py-safety.h b/gdb/python/py-safety.h
index 3294f38c8b6..06324868817 100644
--- a/gdb/python/py-safety.h
+++ b/gdb/python/py-safety.h
@@ -233,6 +233,27 @@ varargs_wrapper (PyObject *self, PyObject *args, PyObject *kw)
 
 } /* namespace safety_details */
 
+/* Create a PyMethodDef for a no-argument function.  It takes the
+   underlying function F as template parameters, and the name and
+   documentation as arguments.  The function F is wrapped to call
+   to_python and to catch exceptions per the safety protocol.  F
+   should not accept any arguments.  */
+template<auto F>
+constexpr PyMethodDef
+noargs_function (const char *name, const char *doc)
+{
+  using namespace safety_details;
+  return {
+    name,
+    [] (PyObject *self, PyObject *args) -> PyObject *
+    {
+      return wrapped_function<F> ();
+    },
+    METH_NOARGS,
+    doc,
+  };
+}
+
 /* Create a PyMethodDef for a no-argument method.  It takes the
    underlying class C and a pointer-to-method M as template
    parameters, and the name and documentation as arguments.  The
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 5529e9fd4d7..76ccdc6b0c6 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -477,8 +477,8 @@ gdbpy_ref<> gdbpy_lookup_static_symbols (gdbpy_borrowed_ref<> args,
 PyObject *gdbpy_start_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_current_recording (PyObject *self, PyObject *args);
 PyObject *gdbpy_stop_recording (PyObject *self, PyObject *args);
-PyObject *gdbpy_newest_frame (PyObject *self, PyObject *args);
-PyObject *gdbpy_selected_frame (PyObject *self, PyObject *args);
+gdbpy_ref<> gdbpy_newest_frame ();
+gdbpy_ref<> gdbpy_selected_frame ();
 PyObject *gdbpy_lookup_type (PyObject *self, PyObject *args, PyObject *kw);
 int gdbpy_is_field (PyObject *obj);
 PyObject *gdbpy_create_lazy_string_object (CORE_ADDR address, long length,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 14c243b135e..72c2d7bb10d 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -3159,12 +3159,12 @@ Arguments (also strings) are passed to the command." },
   { "current_objfile", gdbpy_get_current_objfile, METH_NOARGS,
     "Return the current Objfile being loaded, or None." },
 
-  { "newest_frame", gdbpy_newest_frame, METH_NOARGS,
+  noargs_function<gdbpy_newest_frame> ("newest_frame",
     "newest_frame () -> gdb.Frame.\n\
-Return the newest frame object." },
-  { "selected_frame", gdbpy_selected_frame, METH_NOARGS,
+Return the newest frame object."),
+  noargs_function<gdbpy_selected_frame> ("selected_frame",
     "selected_frame () -> gdb.Frame.\n\
-Return the selected frame object." },
+Return the selected frame object."),
   { "frame_stop_reason_string", gdbpy_frame_stop_reason_string, METH_VARARGS,
     "stop_reason_string (Integer) -> String.\n\
 Return a string explaining unwind stop reason." },

-- 
2.49.0

[PATCH 3/4] Convert gdbpy_frame_stop_reason_string to the safety API

[Tom Tromey]

This converts gdbpy_frame_stop_reason_string to the Python safety API.

This changes the function to accept keyword arguments as well,
following the outcome of an earlier discussion.  A new test is added
for this.
---
 gdb/python/py-frame.c                 | 20 ++++++++------------
 gdb/python/python-internal.h          |  3 ++-
 gdb/python/python.c                   |  4 ++--
 gdb/testsuite/gdb.python/py-frame.exp |  3 +++
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 3910f19ef83..4dd174cf5ae 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -466,24 +466,20 @@ gdbpy_selected_frame ()
 /* Implementation of gdb.stop_reason_string (Integer) -> String.
    Return a string explaining the unwind stop reason.  */
 
-PyObject *
-gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
+const char *
+gdbpy_frame_stop_reason_string (gdbpy_borrowed_ref<> args,
+				gdbpy_opt_borrowed_ref<> kw)
 {
   int reason;
-  const char *str;
 
-  if (!PyArg_ParseTuple (args, "i", &reason))
-    return NULL;
+  static const char *keywords[] = { "reason", nullptr };
+  gdbpy_arg_parse_tuple_and_keywords (args, kw, "i", keywords, &reason);
 
   if (reason < UNWIND_FIRST || reason > UNWIND_LAST)
-    {
-      PyErr_SetString (PyExc_ValueError,
-		       _("Invalid frame stop reason."));
-      return NULL;
-    }
+    gdbpy_err_set_string (PyExc_ValueError,
+			  _("Invalid frame stop reason."));
 
-  str = unwind_stop_reason_to_string ((enum unwind_stop_reason) reason);
-  return PyUnicode_Decode (str, strlen (str), host_charset (), NULL);
+  return unwind_stop_reason_to_string ((enum unwind_stop_reason) reason);
 }
 
 /* Implements the equality comparison for Frame objects.
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 76ccdc6b0c6..5796b80037a 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -465,7 +465,8 @@ extern PyObject *gdbpy_history_count (PyObject *self, PyObject *args);
 PyObject *gdbpy_convenience_variable (PyObject *self, PyObject *args);
 PyObject *gdbpy_set_convenience_variable (PyObject *self, PyObject *args);
 PyObject *gdbpy_breakpoints (PyObject *, PyObject *);
-PyObject *gdbpy_frame_stop_reason_string (PyObject *, PyObject *);
+const char *gdbpy_frame_stop_reason_string (gdbpy_borrowed_ref<> args,
+					    gdbpy_opt_borrowed_ref<> kw);
 gdbpy_ref<> gdbpy_lookup_symbol (gdbpy_borrowed_ref<> args,
 				 gdbpy_opt_borrowed_ref<> kw);
 gdbpy_ref<> gdbpy_lookup_global_symbol (gdbpy_borrowed_ref<> args,
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 72c2d7bb10d..84e7c21377a 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -3165,9 +3165,9 @@ Return the newest frame object."),
   noargs_function<gdbpy_selected_frame> ("selected_frame",
     "selected_frame () -> gdb.Frame.\n\
 Return the selected frame object."),
-  { "frame_stop_reason_string", gdbpy_frame_stop_reason_string, METH_VARARGS,
+  varargs_function<gdbpy_frame_stop_reason_string> ("frame_stop_reason_string",
     "stop_reason_string (Integer) -> String.\n\
-Return a string explaining unwind stop reason." },
+Return a string explaining unwind stop reason."),
 
   { "start_recording", gdbpy_start_recording, METH_VARARGS,
     "start_recording ([method] [, format]) -> gdb.Record.\n\
diff --git a/gdb/testsuite/gdb.python/py-frame.exp b/gdb/testsuite/gdb.python/py-frame.exp
index aebec651964..30692ed6dd8 100644
--- a/gdb/testsuite/gdb.python/py-frame.exp
+++ b/gdb/testsuite/gdb.python/py-frame.exp
@@ -137,6 +137,9 @@ gdb_test "python print ('result = %s' % (f0.type () == gdb.NORMAL_FRAME))" " = T
 gdb_test "python print ('result = %s' % (f0.unwind_stop_reason () == gdb.FRAME_UNWIND_NO_REASON))" \
     " = True" "test Frame.unwind_stop_reason"
 gdb_test "python print ('result = %s' % gdb.frame_stop_reason_string (gdb.FRAME_UNWIND_INNER_ID))" " = previous frame inner to this frame \\(corrupt stack\\?\\)" "test gdb.frame_stop_reason_string"
+gdb_test "python print ('result = %s' % gdb.frame_stop_reason_string (reason=gdb.FRAME_UNWIND_INNER_ID))" \
+    " = previous frame inner to this frame \\(corrupt stack\\?\\)" \
+    "test gdb.frame_stop_reason_string with keyword"
 gdb_test "python print ('result = %s' % f0.pc ())" " = ${::decimal}" "test Frame.pc"
 gdb_test "python print ('result = %s' % (f0.older () == f1))" " = True" "test Frame.older"
 gdb_test "python print ('result = %s' % (f1.newer () == f0))" " = True" "test Frame.newer"

-- 
2.49.0

[PATCH 4/4] Convert frapy_richcompare to safety API

[Tom Tromey]

This converts frapy_richcompare to the Python safety API.  A new
wrap_richcompare template function is added.  As 'richcompare' can
return three results (or throw), the wrapped method returns a
std::optional<bool>; this is documented by the wrapper.

I considered a specialization of wrap_richcompare that automatically
ensures that the compared-to value is of the same class as 'this' --
this would be useful in a few (but not all) spots in gdb.  However
this seemed like a refinement that could easily be added later.

This also removes a part of a comment that I think is incorrect.
---
 gdb/python/py-frame.c  | 24 +++++++++++-------------
 gdb/python/py-safety.h | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index 4dd174cf5ae..4aedd7db563 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -141,6 +141,9 @@ struct frame_object : public PyObject
   /* The static link for this frame.  */
   gdbpy_ref<> static_link ();
 
+  /* Implementation of the Python richcompare API.  */
+  std::optional<bool> richcompare (gdbpy_borrowed_ref<> other, int op);
+
   static PyTypeObject *corresponding_object_type;
 };
 
@@ -482,30 +485,25 @@ gdbpy_frame_stop_reason_string (gdbpy_borrowed_ref<> args,
   return unwind_stop_reason_to_string ((enum unwind_stop_reason) reason);
 }
 
-/* Implements the equality comparison for Frame objects.
-   All other comparison operators will throw a TypeError Python exception,
-   as they aren't valid for frames.  */
+/* Implements the equality comparison for Frame objects.  */
 
-static PyObject *
-frapy_richcompare (PyObject *self, PyObject *other, int op)
+std::optional<bool>
+frame_object::richcompare (gdbpy_borrowed_ref<> other, int op)
 {
   int result;
 
   if (!PyObject_TypeCheck (other, &frame_object_type)
       || (op != Py_EQ && op != Py_NE))
-    return py_notimplemented ().release ();
+    return std::nullopt;
 
-  frame_object *self_frame = (frame_object *) self;
-  frame_object *other_frame = (frame_object *) other;
+  frame_object *other_frame = other;
 
-  if (self_frame->frame_id == other_frame->frame_id)
+  if (frame_id == other_frame->frame_id)
     result = Py_EQ;
   else
     result = Py_NE;
 
-  if (op == result)
-    return py_true ().release ();
-  return py_false ().release ();
+  return op == result;
 }
 
 PyTypeObject *frame_object::corresponding_object_type = &frame_object_type;
@@ -623,7 +621,7 @@ PyTypeObject frame_object_type = {
   "GDB frame object",		  /* tp_doc */
   0,				  /* tp_traverse */
   0,				  /* tp_clear */
-  frapy_richcompare,		  /* tp_richcompare */
+  wrap_richcompare<frame_object, &frame_object::richcompare>, /* tp_richcompare */
   0,				  /* tp_weaklistoffset */
   0,				  /* tp_iter */
   0,				  /* tp_iternext */
diff --git a/gdb/python/py-safety.h b/gdb/python/py-safety.h
index 06324868817..e2cd8bc3f30 100644
--- a/gdb/python/py-safety.h
+++ b/gdb/python/py-safety.h
@@ -364,4 +364,36 @@ wrap_setter (PyObject *arg, PyObject *value, void *closure)
   return 0;
 }
 
+/* A function that wraps a richcompare method.
+
+   A Python tp_richcompare function can either raise an exception,
+   return True or False, or return "not implemented".  The wrapped
+   method must return a std::optional<bool>, which allows all these
+   results: exceptions are simply thrown, true and false are ordinary
+   returns, and the return of an empty optional means "not
+   implemented".  */
+template<typename C, std::optional<bool> (C::*M) (gdbpy_borrowed_ref<>, int)>
+PyObject *
+wrap_richcompare (PyObject *arg, PyObject *value, int op)
+{
+  using namespace safety_details;
+  try
+    {
+      C *self = static_cast<C *> (arg);
+      std::optional<bool> result = (self->*M) (value, op);
+      if (result.has_value ())
+	return to_python (*result);
+      return py_notimplemented ().release ();
+    }
+  catch (const gdb_python_exception &pye)
+    {
+      gdb_assert (PyErr_Occurred () != nullptr);
+      return nullptr;
+    }
+  catch (const gdb_exception &exc)
+    {
+      return gdbpy_handle_gdb_exception (nullptr, exc);
+    }
+}
+
 #endif /* GDB_PYTHON_PY_SAFETY_H */

-- 
2.49.0