[PATCH 0/5] Use Python safety API for breakpoint locations

[PATCH 0/5] Use Python safety API for breakpoint locations

[Tom Tromey]

This series changes the breakpoint location code to use the Python
safety API.

Tested on x86-64 Fedora 40.

Signed-off-by: Tom Tromey <tom@tromey.com>
---
Tom Tromey (5):
      Rewrite gdb.BreakpointLocation repr method
      Move two macros from python-internal.h to py-breakpoint.c
      Add require_valid to Python breakpoint and breakpoint location
      Rewrite breakpoint location getter methods
      Rewrite gdb.BreakpointLocation setter

 gdb/python/py-breakpoint.c   | 371 ++++++++++++++++++++++---------------------
 gdb/python/py-ref.h          |   7 +
 gdb/python/py-wrappers.h     |  10 ++
 gdb/python/python-internal.h |  26 +--
 4 files changed, 209 insertions(+), 205 deletions(-)
---
base-commit: 692a398424b5468c17843b68faefa26169d961a1
change-id: 20261014-python-safety-breakpoint-6f2868b31b69

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

[PATCH 1/5] Rewrite gdb.BreakpointLocation repr method

[Tom Tromey]

This rewrites the gdb.BreakpointLocation repr method to use the Python
safety API.
---
 gdb/python/py-breakpoint.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index b1615db1be3..cb18fec11f9 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -44,6 +44,9 @@ struct gdbpy_breakpoint_location_object : public PyObject
 
   /* An owning reference to the location's breakpoint owner.  */
   gdbpy_breakpoint_object *owner;
+
+  /* Implementation of 'repr'.  */
+  gdbpy_ref<> repr ();
 };
 
 static_assert (gdb::is_python_allocatable_v<gdbpy_breakpoint_location_object>);
@@ -1716,40 +1719,41 @@ bplocpy_dealloc (PyObject *py_self)
 
 /* __repr__ implementation for gdb.BreakpointLocation.  */
 
-static PyObject *
-bplocpy_repr (PyObject *py_self)
+gdbpy_ref<>
+gdbpy_breakpoint_location_object::repr ()
 {
-  const auto self = (gdbpy_breakpoint_location_object *) py_self;
-  if (self->owner == nullptr || self->owner->bp == nullptr
-      || self->owner->bp != self->bp_loc->owner)
-    return gdb_py_invalid_object_repr (py_self);
+  if (owner == nullptr || owner->bp == nullptr || owner->bp != bp_loc->owner)
+    /* FIXME: Python safety.  gdb_py_invalid_object_repr ought to
+       throw on error, and return gdbpy_ref<>, but currently does
+       not.  */
+    return gdbpy_ref<> (gdb_py_invalid_object_repr (this));
 
-  const auto enabled = self->bp_loc->enabled ? "enabled" : "disabled";
+  const auto enabled = bp_loc->enabled ? "enabled" : "disabled";
 
   std::string str (enabled);
 
   str += string_printf (" address=%s",
-			paddress (self->bp_loc->owner->gdbarch,
-				  self->bp_loc->address));
+			paddress (bp_loc->owner->gdbarch,
+				  bp_loc->address));
 
-  if (self->bp_loc->requested_address != self->bp_loc->address)
+  if (bp_loc->requested_address != bp_loc->address)
     str += string_printf (" requested_address=%s",
-			  paddress (self->bp_loc->owner->gdbarch,
-				    self->bp_loc->requested_address));
-  if (self->bp_loc->symtab != nullptr)
-    str += string_printf (" source=%s:%d", self->bp_loc->symtab->filename (),
-			  self->bp_loc->line_number);
+			  paddress (bp_loc->owner->gdbarch,
+				    bp_loc->requested_address));
+  if (bp_loc->symtab != nullptr)
+    str += string_printf (" source=%s:%d", bp_loc->symtab->filename (),
+			  bp_loc->line_number);
 
-  const auto fn_name = self->bp_loc->function_name.get ();
+  const auto fn_name = bp_loc->function_name.get ();
   if (fn_name != nullptr)
     {
       str += " in ";
       str += fn_name;
     }
 
-  return PyUnicode_FromFormat ("<%s %s>",
-			       gdbpy_py_obj_tp_name (py_self).c_str (),
-			       str.c_str ());
+  return gdbpy_unicode_from_format ("<%s %s>",
+				    gdbpy_py_obj_tp_name (this).c_str (),
+				    str.c_str ());
 }
 
 /* Attribute get/set Python definitions. */
@@ -1783,7 +1787,8 @@ PyTypeObject breakpoint_location_object_type =
   0,					/*tp_getattr*/
   0,					/*tp_setattr*/
   0,					/*tp_compare*/
-  bplocpy_repr,                        /*tp_repr*/
+  wrap_tp_callback<gdbpy_breakpoint_location_object,
+		   &gdbpy_breakpoint_location_object::repr>, /*tp_repr*/
   0,					/*tp_as_number*/
   0,					/*tp_as_sequence*/
   0,					/*tp_as_mapping*/

-- 
2.49.0

[PATCH 2/5] Move two macros from python-internal.h to py-breakpoint.c

[Tom Tromey]

I found a couple of macros in python-internal.h that don't need to be.
This moves them into py-breakpoint.c.
---
 gdb/python/py-breakpoint.c   | 22 ++++++++++++++++++++++
 gdb/python/python-internal.h | 23 -----------------------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index cb18fec11f9..8781c17838b 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -37,6 +37,28 @@
 
 extern PyTypeObject breakpoint_location_object_type;
 
+/* Require that BREAKPOINT be a valid breakpoint ID; throw a Python
+   exception if it is invalid.  */
+#define BPPY_REQUIRE_VALID(Breakpoint)                                  \
+    do {                                                                \
+      if ((Breakpoint)->bp == NULL)                                     \
+	return PyErr_Format (PyExc_RuntimeError,                        \
+			     _("Breakpoint %d is invalid."),            \
+			     (Breakpoint)->number);                     \
+    } while (0)
+
+/* Require that BREAKPOINT be a valid breakpoint ID; throw a Python
+   exception if it is invalid.  This macro is for use in setter functions.  */
+#define BPPY_SET_REQUIRE_VALID(Breakpoint)                              \
+    do {                                                                \
+      if ((Breakpoint)->bp == NULL)                                     \
+	{                                                               \
+	  PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
+			(Breakpoint)->number);                          \
+	  return -1;                                                    \
+	}                                                               \
+    } while (0)
+
 struct gdbpy_breakpoint_location_object : public PyObject
 {
   /* An owning reference to the gdb breakpoint location object.  */
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 763504dfc46..55e5e8eaabe 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -431,29 +431,6 @@ struct gdbpy_breakpoint_object : public PyObject
 
 static_assert (gdb::is_python_allocatable_v<gdbpy_breakpoint_object>);
 
-/* Require that BREAKPOINT be a valid breakpoint ID; throw a Python
-   exception if it is invalid.  */
-#define BPPY_REQUIRE_VALID(Breakpoint)                                  \
-    do {                                                                \
-      if ((Breakpoint)->bp == NULL)                                     \
-	return PyErr_Format (PyExc_RuntimeError,                        \
-			     _("Breakpoint %d is invalid."),            \
-			     (Breakpoint)->number);                     \
-    } while (0)
-
-/* Require that BREAKPOINT be a valid breakpoint ID; throw a Python
-   exception if it is invalid.  This macro is for use in setter functions.  */
-#define BPPY_SET_REQUIRE_VALID(Breakpoint)                              \
-    do {                                                                \
-      if ((Breakpoint)->bp == NULL)                                     \
-	{                                                               \
-	  PyErr_Format (PyExc_RuntimeError, _("Breakpoint %d is invalid."), \
-			(Breakpoint)->number);                          \
-	  return -1;                                                    \
-	}                                                               \
-    } while (0)
-
-
 /* Variables used to pass information between the Breakpoint
    constructor and the breakpoint-created hook function.  */
 extern gdbpy_breakpoint_object *bppy_pending_object;

-- 
2.49.0

[PATCH 3/5] Add require_valid to Python breakpoint and breakpoint location

[Tom Tromey]

This adds a require_valid method to the Python breakpoint and
breakpoint location objects.  These will be used in subsequent
patches.

Note that the breakpoint location implementation checks that the
breakpoint is valid.  This was done by all users of the macro, so it
seemed more convenient to do this here.
---
 gdb/python/py-breakpoint.c   | 17 +++++++++++++++++
 gdb/python/python-internal.h |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 8781c17838b..40d1244abcf 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -69,6 +69,15 @@ struct gdbpy_breakpoint_location_object : public PyObject
 
   /* Implementation of 'repr'.  */
   gdbpy_ref<> repr ();
+
+  /* If this breakpoint location is not valid, throw an exception.  */
+  void require_valid () const
+  {
+    owner->require_valid ();
+    if (owner->bp != bp_loc->owner)
+      return gdbpy_err_format (PyExc_RuntimeError,
+			       _("Breakpoint location is invalid."));
+  }
 };
 
 static_assert (gdb::is_python_allocatable_v<gdbpy_breakpoint_location_object>);
@@ -159,6 +168,14 @@ static struct pybp_code pybp_watch_types[] =
   {NULL} /* Sentinel.  */
 };
 
+void
+gdbpy_breakpoint_object::require_valid () const
+{
+  if (bp == nullptr)
+    gdbpy_err_format (PyExc_RuntimeError, _("Breakpoint %d is invalid."),
+		      number);
+}
+
 /* Python function which checks the validity of a breakpoint object.  */
 static PyObject *
 bppy_is_valid (PyObject *self, PyObject *args)
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 55e5e8eaabe..2d990ee6ac1 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -427,6 +427,9 @@ struct gdbpy_breakpoint_object : public PyObject
 
   /* 1 is this is a FinishBreakpoint object, 0 otherwise.  */
   int is_finish_bp;
+
+  /* If this breakpoint is not valid, throw an exception.  */
+  void require_valid () const;
 };
 
 static_assert (gdb::is_python_allocatable_v<gdbpy_breakpoint_object>);

-- 
2.49.0

[PATCH 4/5] Rewrite breakpoint location getter methods

[Tom Tromey]

This rewrites the breakpoint location getter methods to use the Python
safety API.
---
 gdb/python/py-breakpoint.c | 231 +++++++++++++++++++++------------------------
 1 file changed, 110 insertions(+), 121 deletions(-)

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 40d1244abcf..b39093ebd81 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -67,6 +67,52 @@ struct gdbpy_breakpoint_location_object : public PyObject
   /* An owning reference to the location's breakpoint owner.  */
   gdbpy_breakpoint_object *owner;
 
+  /* Python function to test whether or not the breakpoint location is
+     enabled.  */
+  bool get_enabled () const
+  {
+    require_valid ();
+    return bp_loc->enabled;
+  }
+
+  /* Python function to get owner of breakpoint location, which is of
+     type gdb.Breakpoint.  */
+  gdbpy_ref<> get_owner () const
+  {
+    require_valid ();
+    return gdbpy_ref<>::new_reference (owner);
+  }
+
+  /* Python function to get address of breakpoint location.  */
+  CORE_ADDR get_address () const
+  {
+    require_valid ();
+    return bp_loc->address;
+  }
+
+  /* Python function to get the function name of where this location
+     was set.  */
+  const char *get_function () const
+  {
+    require_valid ();
+    return bp_loc->function_name.get ();
+  }
+
+  /* Return the full filename of the location.  */
+  const char *get_fullname () const
+  {
+    require_valid ();
+    if (bp_loc->symtab == nullptr)
+      return nullptr;
+    return bp_loc->symtab->fullname ();
+  }
+
+  /* Accessor to get the source location.  */
+  gdbpy_ref<> get_source_location () const;
+
+  /* Accessor to get the thread groups.  */
+  gdbpy_ref<> get_thread_groups () const;
+
   /* Implementation of 'repr'.  */
   gdbpy_ref<> repr ();
 
@@ -82,15 +128,6 @@ struct gdbpy_breakpoint_location_object : public PyObject
 
 static_assert (gdb::is_python_allocatable_v<gdbpy_breakpoint_location_object>);
 
-/* Require that BREAKPOINT and LOCATION->OWNER are the same; throw a Python
-   exception if they are not.  */
-#define BPLOCPY_REQUIRE_VALID(Breakpoint, Location)                         \
-    do {                                                                    \
-      if ((Breakpoint)->bp != (Location)->bp_loc->owner)                    \
-	return PyErr_Format (PyExc_RuntimeError,                            \
-			     _("Breakpoint location is invalid."));	    \
-    } while (0)
-
 /* Require that BREAKPOINT and LOCATION->OWNER are the same; throw a Python
    exception if they are not.  This macro is for use in setter functions.  */
 #define BPLOCPY_SET_REQUIRE_VALID(Breakpoint, Location)                     \
@@ -1604,45 +1641,6 @@ bplocpy_set_enabled (PyObject *py_self, PyObject *newvalue, void *closure)
   return 0;
 }
 
-/* Python function to test whether or not the breakpoint location is enabled.  */
-
-static PyObject *
-bplocpy_get_enabled (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-
-  if (self->bp_loc->enabled)
-    return py_true ().release ();
-  else
-    return py_false ().release ();
-}
-
-/* Python function to get address of breakpoint location.  */
-
-static PyObject *
-bplocpy_get_address (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  return gdb_py_object_from_ulongest (self->bp_loc->address).release ();
-}
-
-/* Python function to get owner of breakpoint location, which
-   is of type gdb.Breakpoint.  */
-
-static PyObject *
-bplocpy_get_owner (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  Py_INCREF (self->owner);
-  return self->owner;
-}
-
 /* Attempt to get fully resolved file path for symtab.  */
 
 static gdbpy_ref<>
@@ -1650,99 +1648,69 @@ bploc_filepath (struct symtab *bploc_symtab)
 {
   /* The exception is not ours to handle.  We should always
      return some string value and filename is never null.  */
+  const char *filename = nullptr;
   try
     {
-      const char *full = symtab_to_fullname (bploc_symtab);
-      if (full)
-	return host_string_to_python_string (full);
+      filename = symtab_to_fullname (bploc_symtab);
     }
   catch (const gdb_exception &except)
     {
     }
 
-  return host_string_to_python_string (bploc_symtab->filename ());
+  if (filename == nullptr)
+    filename = bploc_symtab->filename ();
+  gdb_assert (filename != nullptr);
+
+  gdbpy_ref<> result = host_string_to_python_string (filename);
+  /* FIXME: Python safety: host_string_to_python_string should
+     throw.  */
+  if (result == nullptr)
+    throw gdb_python_exception ();
+  return result;
 }
 
 /* Python function to get the source file name path and line number
    where this breakpoint location was set.  */
 
-static PyObject *
-bplocpy_get_source_location (PyObject *py_self, void *closure)
+gdbpy_ref<>
+gdbpy_breakpoint_location_object::get_source_location () const
 {
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  if (self->bp_loc->symtab)
+  require_valid ();
+  if (bp_loc->symtab != nullptr)
     {
-      gdbpy_ref<> tup (PyTuple_New (2));
-      if (tup == nullptr)
-	return nullptr;
-      gdbpy_ref<> filename = bploc_filepath (self->bp_loc->symtab);
-      if (filename == nullptr)
-	return nullptr;
-      auto line = gdb_py_object_from_ulongest (self->bp_loc->line_number);
+      gdbpy_ref<> tup = gdbpy_tuple_new (2);
+      gdbpy_ref<> filename = bploc_filepath (bp_loc->symtab);
+      auto line = gdb_py_object_from_ulongest (bp_loc->line_number);
+      /* FIXME: Python safety: gdb_py_object_from_ulongest should
+	 throw.  */
       if (line == nullptr)
-	return nullptr;
-      if (PyTuple_SetItem (tup.get (), 0, filename.release ()) == -1
-	  || PyTuple_SetItem (tup.get (), 1, line.release ()) == -1)
-	return nullptr;
-      return tup.release ();
+	throw gdb_python_exception ();
+      gdbpy_tuple_set_item (tup, 0, std::move (filename));
+      gdbpy_tuple_set_item (tup, 1, std::move (line));
+      return tup;
     }
-  else
-    return py_none ().release ();
-}
 
-/* Python function to get the function name of where this location was set.  */
-
-static PyObject *
-bplocpy_get_function (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  const auto fn_name = self->bp_loc->function_name.get ();
-  if (fn_name != nullptr)
-    return host_string_to_python_string (fn_name).release ();
-  return py_none ().release ();
+  return py_none ();
 }
 
-static PyObject *
-bplocpy_get_thread_groups (PyObject *py_self, void *closure)
+gdbpy_ref<>
+gdbpy_breakpoint_location_object::get_thread_groups () const
 {
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  gdbpy_ref<> list (PyList_New (0));
-  if (list == nullptr)
-    return nullptr;
+  require_valid ();
+  gdbpy_ref<> list = gdbpy_list_new (0);
   for (inferior *inf : all_inferiors ())
     {
-      if (inf->pspace == self->bp_loc->pspace)
+      if (inf->pspace == bp_loc->pspace)
 	{
 	  gdbpy_ref<> num = gdb_py_object_from_ulongest (inf->num);
+	  /* FIXME: Python safety: gdb_py_object_from_ulongest should
+	     throw.  */
 	  if (num == nullptr)
-	    return nullptr;
-	  if (PyList_Append (list.get (), num.get ()) != 0)
-	    return nullptr;
+	    throw gdb_python_exception ();
+	  gdbpy_list_append (list, num);
 	}
     }
-  return list.release ();
-}
-
-static PyObject *
-bplocpy_get_fullname (PyObject *py_self, void *closure)
-{
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_REQUIRE_VALID (self->owner);
-  BPLOCPY_REQUIRE_VALID (self->owner, self);
-  const auto symtab = self->bp_loc->symtab;
-  if (symtab != nullptr && symtab->fullname () != nullptr)
-    {
-      gdbpy_ref<> fullname
-	= host_string_to_python_string (symtab->fullname ());
-      return fullname.release ();
-    }
-  return py_none ().release ();
+  return list;
 }
 
 /* De-allocation function to be called for the Python object.  */
@@ -1798,19 +1766,40 @@ gdbpy_breakpoint_location_object::repr ()
 /* Attribute get/set Python definitions. */
 
 static gdb_PyGetSetDef bp_location_object_getset[] = {
-  { "enabled", bplocpy_get_enabled, bplocpy_set_enabled,
+  { "enabled",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_enabled>,
+    bplocpy_set_enabled,
     "Boolean telling whether the breakpoint is enabled.", NULL },
-  { "owner", bplocpy_get_owner, NULL,
+  { "owner",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_owner>,
+    nullptr,
     "Get the breakpoint owner object", NULL },
-  { "address", bplocpy_get_address, NULL,
+  { "address",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_address>,
+    nullptr,
     "Get address of where this location was set", NULL},
-  { "source", bplocpy_get_source_location, NULL,
+  { "source",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_source_location>,
+    nullptr,
     "Get file and line number of where this location was set", NULL},
-  { "function", bplocpy_get_function, NULL,
+  { "function",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_function>,
+    nullptr,
     "Get function of where this location was set", NULL },
-  { "fullname", bplocpy_get_fullname, NULL,
+  { "fullname",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_fullname>,
+    nullptr,
     "Get fullname of where this location was set", NULL },
-  { "thread_groups", bplocpy_get_thread_groups, NULL,
+  { "thread_groups",
+    wrap_getter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::get_thread_groups>,
+    nullptr,
     "Get thread groups where this location is in", NULL },
   { NULL }  /* Sentinel.  */
 };

-- 
2.49.0

[PATCH 5/5] Rewrite gdb.BreakpointLocation setter

[Tom Tromey]

This rewrites the one gdb.BreakpointLocation setter function to use
the Python safety API.
---
 gdb/python/py-breakpoint.c | 56 +++++++++++++---------------------------------
 gdb/python/py-ref.h        |  7 ++++++
 gdb/python/py-wrappers.h   | 10 +++++++++
 3 files changed, 32 insertions(+), 41 deletions(-)

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index b39093ebd81..f6d3ffc01f5 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -75,6 +75,10 @@ struct gdbpy_breakpoint_location_object : public PyObject
     return bp_loc->enabled;
   }
 
+  /* Python function to set the enabled state of a breakpoint
+     location.  */
+  void set_enabled (gdbpy_opt_borrowed_ref<> newvalue);
+
   /* Python function to get owner of breakpoint location, which is of
      type gdb.Breakpoint.  */
   gdbpy_ref<> get_owner () const
@@ -128,18 +132,6 @@ struct gdbpy_breakpoint_location_object : public PyObject
 
 static_assert (gdb::is_python_allocatable_v<gdbpy_breakpoint_location_object>);
 
-/* Require that BREAKPOINT and LOCATION->OWNER are the same; throw a Python
-   exception if they are not.  This macro is for use in setter functions.  */
-#define BPLOCPY_SET_REQUIRE_VALID(Breakpoint, Location)                     \
-    do {                                                                    \
-      if ((Breakpoint)->bp != (Location)->bp_loc->owner)                    \
-	{                                                                   \
-	  PyErr_Format (PyExc_RuntimeError,                                 \
-			_("Breakpoint location is invalid."));		    \
-	  return -1;                                                        \
-	}                                                                   \
-    } while (0)
-
 /* Debugging of Python breakpoints.  */
 
 static bool pybp_debug;
@@ -1606,39 +1598,20 @@ GDBPY_INITIALIZE_FILE (gdbpy_initialize_breakpoint_locations);
 
 /* Python function to set the enabled state of a breakpoint location.  */
 
-static int
-bplocpy_set_enabled (PyObject *py_self, PyObject *newvalue, void *closure)
+void
+gdbpy_breakpoint_location_object::set_enabled
+     (gdbpy_opt_borrowed_ref<> newvalue)
 {
-  auto *self = (gdbpy_breakpoint_location_object *) py_self;
-  BPPY_SET_REQUIRE_VALID (self->owner);
-  BPLOCPY_SET_REQUIRE_VALID (self->owner, self);
+  require_valid ();
 
   if (newvalue == nullptr)
-    {
-      PyErr_SetString (PyExc_TypeError,
-		       _("Cannot delete 'enabled' attribute."));
-      return -1;
-    }
+    gdbpy_err_set_string (PyExc_TypeError,
+			  _("Cannot delete 'enabled' attribute."));
   else if (!PyBool_Check (newvalue))
-    {
-      PyErr_SetString (PyExc_TypeError,
-		       _("The value of 'enabled' must be a boolean."));
-      return -1;
-    }
+    gdbpy_err_set_string (PyExc_TypeError,
+			  _("The value of 'enabled' must be a boolean."));
 
-  int cmp = PyObject_IsTrue (newvalue);
-  if (cmp < 0)
-    return -1;
-
-  try
-    {
-      enable_disable_bp_location (self->bp_loc, cmp == 1);
-    }
-  catch (const gdb_exception &except)
-    {
-      return gdbpy_handle_gdb_exception (-1, except);
-    }
-  return 0;
+  enable_disable_bp_location (bp_loc, gdbpy_object_is_true (newvalue));
 }
 
 /* Attempt to get fully resolved file path for symtab.  */
@@ -1769,7 +1742,8 @@ static gdb_PyGetSetDef bp_location_object_getset[] = {
   { "enabled",
     wrap_getter<gdbpy_breakpoint_location_object,
 		&gdbpy_breakpoint_location_object::get_enabled>,
-    bplocpy_set_enabled,
+    wrap_setter<gdbpy_breakpoint_location_object,
+		&gdbpy_breakpoint_location_object::set_enabled>,
     "Boolean telling whether the breakpoint is enabled.", NULL },
   { "owner",
     wrap_getter<gdbpy_breakpoint_location_object,
diff --git a/gdb/python/py-ref.h b/gdb/python/py-ref.h
index 3d0373b7001..3e8838b3849 100644
--- a/gdb/python/py-ref.h
+++ b/gdb/python/py-ref.h
@@ -108,6 +108,13 @@ class gdbpy_borrowed_ref : public gdbpy_opt_borrowed_ref<T>
     gdb_assert (this->m_obj != nullptr);
   }
 
+  template<typename U>
+  gdbpy_borrowed_ref (const gdbpy_opt_borrowed_ref<U> &ref)
+    : gdbpy_opt_borrowed_ref<T> (ref)
+  {
+    gdb_assert (this->m_obj != nullptr);
+  }
+
   gdbpy_borrowed_ref (std::nullptr_t) = delete;
 
   /* Allow a (checked) conversion to any subclass of T.  */
diff --git a/gdb/python/py-wrappers.h b/gdb/python/py-wrappers.h
index 5b1353872cc..fdb11d2a1fa 100644
--- a/gdb/python/py-wrappers.h
+++ b/gdb/python/py-wrappers.h
@@ -358,4 +358,14 @@ gdbpy_sequence_concat (gdbpy_borrowed_ref<> first, gdbpy_borrowed_ref<> second)
   return result;
 }
 
+/* Wrapper for PyObject_IsTrue.  */
+static inline bool
+gdbpy_object_is_true (gdbpy_borrowed_ref<> val)
+{
+  int cmp = PyObject_IsTrue (val);
+  if (cmp < 0)
+    throw gdb_python_exception ();
+  return cmp;
+}
+
 #endif /* GDB_PYTHON_PY_WRAPPERS_H */

-- 
2.49.0