[doc,RFA] Split create-breakpoint! into make-breakpoint, register-breakpoint!

Message ID m3egz5cm1v.fsf@sspiff.org
State New, archived
Headers

Commit Message

Doug Evans June 4, 2014, 4:53 a.m. UTC
  Hi.
In keeping with the splitting of object creation and registration into
two separate pieces, this does the same for breakpoints.
"Consistency Is Good".

There hasn't been an FSF release of gdb+guile yet so I think it's ok
to remove create-breakpoint! here.


2014-06-03  Doug Evans  <xdje42@gmail.com>

	* guile/scm-breakpoint.c (struct gdbscm_breakpoint_object): New members
	is_scheme_bkpt, spec.
	(bpscm_make_breakpoint_smob): Initialize new members.
	(gdbscm_create_breakpoint_x): Split into two ...
	(gdbscm_make_breakpoint, gdbscm_register_breakpoint_x): New functions.
	(bpscm_breakpoint_deleted): Reset breakpoint number and stop function.
	(scheme_function breakpoint_functions): Update.

	testsuite/
	* gdb.guile/scm-breakpoint.exp: Update.
	Add tests for breakpoint registration.

	doc/
	* guile.texi (Breakpoints In Guile): Update.
  

Comments

Eli Zaretskii June 4, 2014, 7:05 a.m. UTC | #1
> From: Doug Evans <xdje42@gmail.com>
> Date: Tue, 03 Jun 2014 21:53:48 -0700
> 
> In keeping with the splitting of object creation and registration into
> two separate pieces, this does the same for breakpoints.
> "Consistency Is Good".
> 
> There hasn't been an FSF release of gdb+guile yet so I think it's ok
> to remove create-breakpoint! here.

OK for the documentation part, but is delete-breakpoint a better name
than breakpoint-delete?.

A question: how does one discard breakpoint objects so that they no
longer exist?  It seems that breakpoint-delete! only "deregisters" the
breakpoint, but there's no way to undo the effect of the
create-breakpoint procedure.

Thanks.
  
Ludovic Courtès June 4, 2014, 8:29 a.m. UTC | #2
Eli Zaretskii <eliz@gnu.org> skribis:

>> From: Doug Evans <xdje42@gmail.com>
>> Date: Tue, 03 Jun 2014 21:53:48 -0700
>> 
>> In keeping with the splitting of object creation and registration into
>> two separate pieces, this does the same for breakpoints.
>> "Consistency Is Good".
>> 
>> There hasn't been an FSF release of gdb+guile yet so I think it's ok
>> to remove create-breakpoint! here.
>
> OK for the documentation part, but is delete-breakpoint a better name
> than breakpoint-delete?.

I prefer ‘delete-breakpoint!’.

> A question: how does one discard breakpoint objects so that they no
> longer exist?  It seems that breakpoint-delete! only "deregisters" the
> breakpoint, but there's no way to undo the effect of the
> create-breakpoint procedure.

They just get garbage collected eventually.

Ludo’.
  
Ludovic Courtès June 4, 2014, 8:30 a.m. UTC | #3
Doug Evans <xdje42@gmail.com> skribis:

> In keeping with the splitting of object creation and registration into
> two separate pieces, this does the same for breakpoints.
> "Consistency Is Good".

That makes sense.

> There hasn't been an FSF release of gdb+guile yet so I think it's ok
> to remove create-breakpoint! here.

Yup.

> 2014-06-03  Doug Evans  <xdje42@gmail.com>
>
> 	* guile/scm-breakpoint.c (struct gdbscm_breakpoint_object): New members
> 	is_scheme_bkpt, spec.
> 	(bpscm_make_breakpoint_smob): Initialize new members.
> 	(gdbscm_create_breakpoint_x): Split into two ...
> 	(gdbscm_make_breakpoint, gdbscm_register_breakpoint_x): New functions.
> 	(bpscm_breakpoint_deleted): Reset breakpoint number and stop function.
> 	(scheme_function breakpoint_functions): Update.
>
> 	testsuite/
> 	* gdb.guile/scm-breakpoint.exp: Update.
> 	Add tests for breakpoint registration.
>
> 	doc/
> 	* guile.texi (Breakpoints In Guile): Update.

LGTM.

Ludo’.
  
Eli Zaretskii June 4, 2014, 9:27 a.m. UTC | #4
> From: ludo@gnu.org (Ludovic Courtès)
> Cc: Doug Evans <xdje42@gmail.com>,  gdb-patches@sourceware.org
> Date: Wed, 04 Jun 2014 10:29:14 +0200
> 
> Eli Zaretskii <eliz@gnu.org> skribis:
> 
> >> From: Doug Evans <xdje42@gmail.com>
> >> Date: Tue, 03 Jun 2014 21:53:48 -0700
> >> 
> >> In keeping with the splitting of object creation and registration into
> >> two separate pieces, this does the same for breakpoints.
> >> "Consistency Is Good".
> >> 
> >> There hasn't been an FSF release of gdb+guile yet so I think it's ok
> >> to remove create-breakpoint! here.
> >
> > OK for the documentation part, but is delete-breakpoint a better name
> > than breakpoint-delete?.
> 
> I prefer ‘delete-breakpoint!’.

Fine with me.

(Btw, what exactly is the principle behind the usage of the trailing
exclam?)

> > A question: how does one discard breakpoint objects so that they no
> > longer exist?  It seems that breakpoint-delete! only "deregisters" the
> > breakpoint, but there's no way to undo the effect of the
> > create-breakpoint procedure.
> 
> They just get garbage collected eventually.

That's what I suspected.  But then we cannot say in the manual that a
breakpoint deleted by breakpoint-delete! can later be re-registered,
can we?  Because if the object was GCed, we cannot register it, can
we?

Actually, there's a more serious problem here: what if the object
created by create-breakpoint is GCed _before_ we register it the first
time?  GC can strike whenever it feels like, so we _must_ explain that
a Guile variable that holds the breakpoint object should never go out
of scope before it is used to register the breakpoint.  Am I missing
something?
  
Ludovic Courtès June 4, 2014, 1:47 p.m. UTC | #5
Eli Zaretskii <eliz@gnu.org> skribis:

>> From: ludo@gnu.org (Ludovic Courtès)
>> Cc: Doug Evans <xdje42@gmail.com>,  gdb-patches@sourceware.org
>> Date: Wed, 04 Jun 2014 10:29:14 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> skribis:
>> 
>> >> From: Doug Evans <xdje42@gmail.com>
>> >> Date: Tue, 03 Jun 2014 21:53:48 -0700
>> >> 
>> >> In keeping with the splitting of object creation and registration into
>> >> two separate pieces, this does the same for breakpoints.
>> >> "Consistency Is Good".
>> >> 
>> >> There hasn't been an FSF release of gdb+guile yet so I think it's ok
>> >> to remove create-breakpoint! here.
>> >
>> > OK for the documentation part, but is delete-breakpoint a better name
>> > than breakpoint-delete?.
>> 
>> I prefer ‘delete-breakpoint!’.
>
> Fine with me.
>
> (Btw, what exactly is the principle behind the usage of the trailing
> exclam?)

It denotes procedures that mutate one of their arguments or some global
state (here it mutates an implicit list of registered breakpoints.)

>> > A question: how does one discard breakpoint objects so that they no
>> > longer exist?  It seems that breakpoint-delete! only "deregisters" the
>> > breakpoint, but there's no way to undo the effect of the
>> > create-breakpoint procedure.
>> 
>> They just get garbage collected eventually.
>
> That's what I suspected.  But then we cannot say in the manual that a
> breakpoint deleted by breakpoint-delete! can later be re-registered,
> can we?  Because if the object was GCed, we cannot register it, can
> we?

If it’s GC’d, that means user code no longer holds a reference to it, so
it cannot register it.

If the user holds a reference to it, then it is not GC’d.

Consider this:

  (define b (make-breakpoint ...))

  (register-breakpoint! b)
  ;; ...
  (delete-breakpoint! b)
  ;; ...
  (register-breakpoint! b))

This is fine: the breakpoint object won’t be GC’d because it’s
referenced from global variable ‘b’.

> Actually, there's a more serious problem here: what if the object
> created by create-breakpoint is GCed _before_ we register it the first
> time?  GC can strike whenever it feels like, so we _must_ explain that
> a Guile variable that holds the breakpoint object should never go out
> of scope before it is used to register the breakpoint.  Am I missing
> something?

Yes, if it goes out of scope, then by definitions it means that it
cannot be registered because we no longer have a handle on it.

For instance:

  (make-breakpoint ...)  ; ignore return value

  ;; Here I can’t register the above breakpoint because I don’t have
  ;; a reference on it.

  (gc)                   ; the breakpoint above is reclaimed

Ludo’.
  

Patch

diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
index 3f8c4e4..4675cb2 100644
--- a/gdb/doc/guile.texi
+++ b/gdb/doc/guile.texi
@@ -2899,18 +2899,30 @@  object will be @code{#f} and 0 respectively.
 @tindex <gdb:breakpoint>
 
 Breakpoints in Guile are represented by objects of type
-@code{<gdb:breakpoint>}.
+@code{<gdb:breakpoint>}.  New breakpoints can be created with the
+@code{make-breakpoint} Guile function, and then added to @value{GDBN} with the
+@code{register-breakpoint!} Guile function.
+This two-step approach is taken to separate out the side-effect of adding
+the breakpoint to @value{GDBN} from @code{make-breakpoint}.
+
+Support is also provided to view and manipulate breakpoints created
+outside of Guile.
 
 The following breakpoint-related procedures are provided by the
 @code{(gdb)} module:
 
 @c TODO: line length
-@deffn {Scheme Procedure} create-breakpoint! location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]}
-Create a new breakpoint according to @var{spec}, a string naming the
+@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]}
+Create a new breakpoint at @var{location}, a string naming the
 location of the breakpoint, or an expression that defines a watchpoint.
 The contents can be any location recognized by the @code{break} command,
 or in the case of a watchpoint, by the @code{watch} command.
 
+The breakpoint is initially marked as @samp{invalid}.
+The breakpoint is not usable until it has been registered with @value{GDBN}
+with @code{register-breakpoint!}, at which point it becomes @samp{valid}.
+The result is the @code{<gdb:breakpoint>} object representing the breakpoint.
+
 The optional @var{type} denotes the breakpoint to create.
 This argument can be either @code{BP_BREAKPOINT} or @code{BP_WATCHPOINT},
 and defaults to @code{BP_BREAKPOINT}.
@@ -2921,7 +2933,7 @@  not provided, it is assumed to be a @code{WP_WRITE} class.
 
 The optional @var{internal} argument allows the breakpoint to become
 invisible to the user.  The breakpoint will neither be reported when
-created, nor will it be listed in the output from @code{info breakpoints}
+registered, nor will it be listed in the output from @code{info breakpoints}
 (but will be listed with the @code{maint info breakpoints} command).
 If an internal flag is not provided, the breakpoint is visible
 (non-internal).
@@ -2972,10 +2984,24 @@  Read/Write watchpoint.
 
 @end deffn
 
+@deffn {Scheme Procedure} register-breakpoint! breakpoint
+Add @var{breakpoint}, a @code{<gdb:breakpoint>} object, to @value{GDBN}'s
+list of breakpoints.  The breakpoint must have been created with
+@code{make-breakpoint}.  One cannot register breakpoints that have been
+created outside of Guile.  Once a breakpoint is registered it becomes
+@samp{valid}.
+It is an error to register an already registered breakpoint.
+The result is unspecified.
+@end deffn
+
 @deffn {Scheme Procedure} breakpoint-delete! breakpoint
-Permanently delete @var{breakpoint}.  This also invalidates the
-Guile @var{breakpoint} object.  Any further attempt to access the
-object will throw an exception.
+Remove @var{breakpoint} from @value{GDBN}'s list of breakpoints.
+This also invalidates the Guile @var{breakpoint} object.
+Any further attempt to access the object will throw an exception.
+
+If @var{breakpoint} was created from Guile with @code{make-breakpoint}
+it may be re-registered with @value{GDBN}, in which case the breakpoint
+becomes valid again.
 @end deffn
 
 @deffn {Scheme Procedure} breakpoints
@@ -2990,6 +3016,8 @@  and @code{#f} otherwise.
 
 @deffn {Scheme Procedure} breakpoint-valid? breakpoint
 Return @code{#t} if @var{breakpoint} is valid, @code{#f} otherwise.
+Breakpoints created with @code{make-breakpoint} are marked as invalid
+until they are registered with @value{GDBN} with @code{register-breakpoint!}.
 A @code{<gdb:breakpoint>} object can become invalid
 if the user deletes the breakpoint.  In this case, the object still
 exists, but the underlying breakpoint does not.  In the cases of
@@ -3129,7 +3157,8 @@  Example @code{stop} implementation:
 (define (my-stop? bkpt)
   (let ((int-val (parse-and-eval "foo")))
     (value=? int-val 3)))
-(define bkpt (create-breakpoint! "main.c:42"))
+(define bkpt (make-breakpoint "main.c:42"))
+(register-breakpoint! bkpt)
 (set-breakpoint-stop! bkpt my-stop?)
 @end smallexample
 @end deffn
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index 7edb97d..05e46c0 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -34,18 +34,51 @@ 
 #include "guile-internal.h"
 
 /* The <gdb:breakpoint> smob.
-   N.B.: The name of this struct is known to breakpoint.h.  */
+   N.B.: The name of this struct is known to breakpoint.h.
+
+   Note: Breakpoints are added to gdb using a two step process:
+   1) Call make-breakpoint to create a <gdb:breakpoint> object.
+   2) Call register-breakpoint! to add the breakpoint to gdb.
+   It is done this way so that the constructor, make-breakpoint, doesn't have
+   any side-effects.  This means that the smob needs to store everything
+   that was passed to make-breakpoint.  */
 
 typedef struct gdbscm_breakpoint_object
 {
   /* This always appears first.  */
   gdb_smob base;
 
+  /* Non-zero if this breakpoint was created with make-breakpoint.  */
+  int is_scheme_bkpt;
+
+  /* For breakpoints created with make-breakpoint, these are the parameters
+     that were passed to make-breakpoint.  These values are not used except
+     to register the breakpoint with GDB.  */
+  struct
+  {
+    /* The string representation of the breakpoint.
+       Space for this lives in GC space.  */
+    char *location;
+
+    /* The kind of breakpoint.
+       At the moment this can only be one of bp_breakpoint, bp_watchpoint.  */
+    enum bptype type;
+
+    /* If a watchpoint, the kind of watchpoint.  */
+    enum target_hw_bp_type access_type;
+
+    /* Non-zero if the breakpoint is an "internal" breakpoint.  */
+    int is_internal;
+  } spec;
+
   /* The breakpoint number according to gdb.
+     For breakpoints created from Scheme, this has the value -1 until the
+     breakpoint is registered with gdb.
      This is recorded here because BP will be NULL when deleted.  */
   int number;
 
-  /* The gdb breakpoint object, or NULL if the breakpoint has been deleted.  */
+  /* The gdb breakpoint object, or NULL if the breakpoint has not been
+     registered yet, or has been deleted.  */
   struct breakpoint *bp;
 
   /* Backlink to our containing <gdb:breakpoint> smob.
@@ -171,8 +204,8 @@  bpscm_make_breakpoint_smob (void)
     scm_gc_malloc (sizeof (breakpoint_smob), breakpoint_smob_name);
   SCM bp_scm;
 
+  memset (bp_smob, 0, sizeof (*bp_smob));
   bp_smob->number = -1;
-  bp_smob->bp = NULL;
   bp_smob->stop = SCM_BOOL_F;
   bp_scm = scm_new_smob (breakpoint_smob_tag, (scm_t_bits) bp_smob);
   bp_smob->containing_scm = bp_scm;
@@ -293,42 +326,111 @@  bpscm_get_valid_breakpoint_smob_arg_unsafe (SCM self, int arg_pos,
 
 /* Breakpoint methods.  */
 
-/* (create-breakpoint! string [#:type integer] [#:wp-class integer]
-    [#:internal boolean) -> <gdb:breakpoint> */
+/* (make-breakpoint string [#:type integer] [#:wp-class integer]
+    [#:internal boolean) -> <gdb:breakpoint>
+
+   The result is the <gdb:breakpoint> Scheme object.
+   The breakpoint is not available to be used yet, however.
+   It must still be added to gdb with register-breakpoint!.  */
 
 static SCM
-gdbscm_create_breakpoint_x (SCM spec_scm, SCM rest)
+gdbscm_make_breakpoint (SCM location_scm, SCM rest)
 {
   const SCM keywords[] = {
     type_keyword, wp_class_keyword, internal_keyword, SCM_BOOL_F
   };
-  char *spec;
+  char *s;
+  char *location;
   int type_arg_pos = -1, access_type_arg_pos = -1, internal_arg_pos = -1;
   int type = bp_breakpoint;
   int access_type = hw_write;
   int internal = 0;
   SCM result;
-  volatile struct gdb_exception except;
+  breakpoint_smob *bp_smob;
 
   gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iit",
-			      spec_scm, &spec, rest,
+			      location_scm, &location, rest,
 			      &type_arg_pos, &type,
 			      &access_type_arg_pos, &access_type,
 			      &internal_arg_pos, &internal);
 
   result = bpscm_make_breakpoint_smob ();
-  pending_breakpoint_scm = result;
+  bp_smob = (breakpoint_smob *) SCM_SMOB_DATA (result);
+
+  s = location;
+  location = gdbscm_gc_xstrdup (s);
+  xfree (s);
+
+  switch (type)
+    {
+    case bp_breakpoint:
+      if (access_type_arg_pos > 0)
+	{
+	  gdbscm_misc_error (FUNC_NAME, access_type_arg_pos,
+			     scm_from_int (access_type),
+			     _("access type with breakpoint is not allowed"));
+	}
+      break;
+    case bp_watchpoint:
+      switch (access_type)
+	{
+	case hw_write:
+	case hw_access:
+	case hw_read:
+	  break;
+	default:
+	  gdbscm_out_of_range_error (FUNC_NAME, access_type_arg_pos,
+				     scm_from_int (access_type),
+				     _("invalid watchpoint class"));
+	}
+      break;
+    default:
+      gdbscm_out_of_range_error (FUNC_NAME, access_type_arg_pos,
+				 scm_from_int (type),
+				 _("invalid breakpoint type"));
+    }
+
+  bp_smob->is_scheme_bkpt = 1;
+  bp_smob->spec.location = location;
+  bp_smob->spec.type = type;
+  bp_smob->spec.access_type = access_type;
+  bp_smob->spec.is_internal = internal;
+
+  return result;
+}
+
+/* (register-breakpoint! <gdb:breakpoint>) -> unspecified
+
+   It is an error to register a breakpoint created outside of Guile,
+   or an already-registered breakpoint.  */
+
+static SCM
+gdbscm_register_breakpoint_x (SCM self)
+{
+  breakpoint_smob *bp_smob
+    = bpscm_get_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
+  volatile struct gdb_exception except;
+
+  /* We only support registering breakpoints created with make-breakpoint.  */
+  if (!bp_smob->is_scheme_bkpt)
+    scm_misc_error (FUNC_NAME, _("not a Scheme breakpoint"), SCM_EOL);
+
+  if (bpscm_is_valid (bp_smob))
+    scm_misc_error (FUNC_NAME, _("breakpoint is already registered"), SCM_EOL);
+
+  pending_breakpoint_scm = self;
 
   TRY_CATCH (except, RETURN_MASK_ALL)
     {
-      struct cleanup *cleanup = make_cleanup (xfree, spec);
+      char *location = bp_smob->spec.location;
+      int internal = bp_smob->spec.is_internal;
 
-      switch (type)
+      switch (bp_smob->spec.type)
 	{
 	case bp_breakpoint:
 	  {
 	    create_breakpoint (get_current_arch (),
-			       spec, NULL, -1, NULL,
+			       location, NULL, -1, NULL,
 			       0,
 			       0, bp_breakpoint,
 			       0,
@@ -339,27 +441,27 @@  gdbscm_create_breakpoint_x (SCM spec_scm, SCM rest)
 	  }
 	case bp_watchpoint:
 	  {
+	    enum target_hw_bp_type access_type = bp_smob->spec.access_type;
+
 	    if (access_type == hw_write)
-	      watch_command_wrapper (spec, 0, internal);
+	      watch_command_wrapper (location, 0, internal);
 	    else if (access_type == hw_access)
-	      awatch_command_wrapper (spec, 0, internal);
+	      awatch_command_wrapper (location, 0, internal);
 	    else if (access_type == hw_read)
-	      rwatch_command_wrapper (spec, 0, internal);
+	      rwatch_command_wrapper (location, 0, internal);
 	    else
-	      error (_("Invalid watchpoint access type"));
+	      gdb_assert_not_reached ("invalid access type");
 	    break;
 	  }
 	default:
-	  error (_("Invalid breakpoint type"));
+	  gdb_assert_not_reached ("invalid breakpoint type");
 	}
-
-      do_cleanups (cleanup);
     }
   /* Ensure this gets reset, even if there's an error.  */
   pending_breakpoint_scm = SCM_BOOL_F;
   GDBSCM_HANDLE_GDB_EXCEPTION (except);
 
-  return result;
+  return SCM_UNSPECIFIED;
 }
 
 /* (breakpoint-delete! <gdb:breakpoint>) -> unspecified
@@ -999,6 +1101,8 @@  bpscm_breakpoint_deleted (struct breakpoint *b)
       if (bp_smob)
 	{
 	  bp_smob->bp = NULL;
+	  bp_smob->number = -1;
+	  bp_smob->stop = SCM_BOOL_F;
 	  scm_gc_unprotect_object (bp_smob->containing_scm);
 	}
     }
@@ -1024,12 +1128,18 @@  static const scheme_integer_constant breakpoint_integer_constants[] =
 
 static const scheme_function breakpoint_functions[] =
 {
-  { "create-breakpoint!", 1, 0, 1, gdbscm_create_breakpoint_x,
+  { "make-breakpoint", 1, 0, 1, gdbscm_make_breakpoint,
     "\
-Create and install a GDB breakpoint object.\n\
+Create a GDB breakpoint object.\n\
 \n\
   Arguments:\n\
-     location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>]" },
+    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>]\n\
+  Returns:\n\
+    <gdb:breakpoint object" },
+
+  { "register-breakpoint!", 1, 0, 0, gdbscm_register_breakpoint_x,
+    "\
+Register a <gdb:breakpoint> object with GDB." },
 
   { "breakpoint-delete!", 1, 0, 0, gdbscm_breakpoint_delete_x,
     "\
diff --git a/gdb/testsuite/gdb.guile/scm-breakpoint.exp b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
index fd7c970..4701969 100644
--- a/gdb/testsuite/gdb.guile/scm-breakpoint.exp
+++ b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
@@ -116,8 +116,10 @@  proc test_bkpt_deletion { } {
 	# Test breakpoints are deleted correctly.
 	set deltst_location [gdb_get_line_number "Break at multiply."]
 	set end_location [gdb_get_line_number "Break at end."]
-	gdb_scm_test_silent_cmd  "guile (define dp1 (create-breakpoint! \"$deltst_location\"))" \
+	gdb_scm_test_silent_cmd  "guile (define dp1 (make-breakpoint \"$deltst_location\"))" \
 	    "create deltst breakpoint"
+	gdb_scm_test_silent_cmd "guile (register-breakpoint! dp1)" \
+	    "register dp1"
 	gdb_breakpoint [gdb_get_line_number "Break at end."]
 	gdb_scm_test_silent_cmd "guile (define del-list (breakpoints))" \
 	    "get breakpoint list 4"
@@ -128,7 +130,7 @@  proc test_bkpt_deletion { } {
 	gdb_scm_test_silent_cmd  "guile (breakpoint-delete! dp1)" \
 	    "delete breakpoint"
 	gdb_test "guile (print (breakpoint-number dp1))" \
-	    "ERROR: .*: Invalid object: <gdb:breakpoint> in position 1: #<gdb:breakpoint #2>.*" \
+	    "ERROR: .*: Invalid object: <gdb:breakpoint> in position 1: #<gdb:breakpoint #-1>.*" \
 	    "check breakpoint invalidated"
 	gdb_scm_test_silent_cmd "guile (set! del-list (breakpoints))" \
 	    "get breakpoint list 5"
@@ -151,8 +153,10 @@  proc test_bkpt_cond_and_cmds { } {
 
 	# Test conditional setting.
 	set bp_location1 [gdb_get_line_number "Break at multiply."]
-	gdb_scm_test_silent_cmd  "guile (define bp1 (create-breakpoint! \"$bp_location1\"))" \
+	gdb_scm_test_silent_cmd  "guile (define bp1 (make-breakpoint \"$bp_location1\"))" \
 	    "create multiply breakpoint"
+	gdb_scm_test_silent_cmd "guile (register-breakpoint! bp1)" \
+	    "register bp1"
 	gdb_continue_to_breakpoint "Break at multiply."
 	gdb_scm_test_silent_cmd  "guile (set-breakpoint-condition! bp1 \"i == 5\")" \
 	    "set condition"
@@ -199,8 +203,10 @@  proc test_bkpt_invisible { } {
 	# Test invisible breakpoints.
 	delete_breakpoints
 	set ibp_location [gdb_get_line_number "Break at multiply."]
-	gdb_scm_test_silent_cmd  "guile (define vbp (create-breakpoint! \"$ibp_location\" #:internal #f))" \
+	gdb_scm_test_silent_cmd  "guile (define vbp1 (make-breakpoint \"$ibp_location\" #:internal #f))" \
 	    "create visible breakpoint"
+	gdb_scm_test_silent_cmd "guile (register-breakpoint! vbp1)" \
+	    "register vbp1"
 	gdb_scm_test_silent_cmd "guile (define vbp (car (breakpoints)))" \
 	    "get visible breakpoint"
 	gdb_test "guile (print vbp)" \
@@ -214,8 +220,10 @@  proc test_bkpt_invisible { } {
 	    "scm-breakpoint\.c:$ibp_location.*" \
 	    "check info breakpoints shows visible breakpoints"
 	delete_breakpoints
-	gdb_scm_test_silent_cmd  "guile (define ibp (create-breakpoint! \"$ibp_location\" #:internal #t))" \
+	gdb_scm_test_silent_cmd  "guile (define ibp (make-breakpoint \"$ibp_location\" #:internal #t))" \
 	    "create invisible breakpoint"
+	gdb_scm_test_silent_cmd "guile (register-breakpoint! ibp)" \
+	    "register ibp"
 	gdb_test "guile (print ibp)" \
 	    "= #<gdb:breakpoint #-$decimal BP_BREAKPOINT enabled noisy hit:0 ignore:0 @.*scm-breakpoint.c:$ibp_location>" \
 	    "check invisible bp obj exists"
@@ -247,8 +255,10 @@  proc test_watchpoints { } {
 	    return
 	}
 
-	gdb_scm_test_silent_cmd  "guile (define wp1 (create-breakpoint! \"result\" #:type BP_WATCHPOINT #:wp-class WP_WRITE))" \
+	gdb_scm_test_silent_cmd  "guile (define wp1 (make-breakpoint \"result\" #:type BP_WATCHPOINT #:wp-class WP_WRITE))" \
 	    "create watchpoint"
+	gdb_scm_test_silent_cmd "guile (register-breakpoint! wp1)" \
+	    "register wp1"
 	gdb_test "continue" \
 	    ".*\[Ww\]atchpoint.*result.*Old value = 0.*New value = 25.*main.*" \
 	    "test watchpoint write"
@@ -272,8 +282,10 @@  proc test_bkpt_internal { } {
 
 	delete_breakpoints
 
-	gdb_scm_test_silent_cmd  "guile (define wp1 (create-breakpoint! \"result\" #:type BP_WATCHPOINT #:wp-class WP_WRITE #:internal #t))" \
+	gdb_scm_test_silent_cmd  "guile (define wp1 (make-breakpoint \"result\" #:type BP_WATCHPOINT #:wp-class WP_WRITE #:internal #t))" \
 	    "create invisible watchpoint"
+	gdb_scm_test_silent_cmd "guile (register-breakpoint! wp1)" \
+	    "register wp1"
 	gdb_test "info breakpoints" \
 	    "No breakpoints or watchpoints.*" \
 	    "check info breakpoints does not show invisible watchpoint"
@@ -303,6 +315,11 @@  proc test_bkpt_eval_funcs { } {
 
 	delete_breakpoints
 
+	# Define create-breakpoint! as a convenient wrapper around
+	# make-breakpoint, register-breakpoint!
+	gdb_test_no_output "guile (define (create-breakpoint! . args) (let ((bp (apply make-breakpoint args))) (register-breakpoint! bp) bp))" \
+	    "define create-breakpoint!"
+
 	gdb_test_multiline "data collection breakpoint 1" \
 	    "guile" "" \
 	    "(define (make-bp-data) (cons 0 0))" "" \
@@ -429,6 +446,47 @@  proc test_bkpt_eval_funcs { } {
     }
 }
 
+proc test_bkpt_registration {} {
+    global srcfile testfile
+
+    with_test_prefix "test_bkpt_registration" {
+	# Start with a fresh gdb.
+	clean_restart ${testfile}
+
+	if ![gdb_guile_runto_main] {
+	    return
+	}
+
+	# Initially there should be one breakpoint: main.
+	gdb_scm_test_silent_cmd "guile (define blist (breakpoints))" \
+	    "get breakpoint list 1"
+	gdb_test "guile (register-breakpoint! (car blist))" \
+	    "ERROR: .*: not a Scheme breakpoint.*" \
+	    "try to register a non-guile breakpoint"
+
+	set bp_location1 [gdb_get_line_number "Break at multiply."]
+	gdb_scm_test_silent_cmd  "guile (define bp1 (make-breakpoint \"$bp_location1\"))" \
+	    "create multiply breakpoint"
+	gdb_test "guile (print (breakpoint-valid? bp1))" \
+	    "= #f" "breakpoint invalid after creation"
+	gdb_scm_test_silent_cmd "guile (register-breakpoint! bp1)" \
+	    "register bp1"
+	gdb_test "guile (print (breakpoint-valid? bp1))" \
+	    "= #t" "breakpoint valid after registration"
+	gdb_test "guile (register-breakpoint! bp1)" \
+	    "ERROR: .*: breakpoint is already registered.*" \
+	    "re-register already registered bp1"
+	gdb_scm_test_silent_cmd "guile (breakpoint-delete! bp1)" \
+	    "delete registered breakpoint"
+	gdb_test "guile (print (breakpoint-valid? bp1))" \
+	    "= #f" "breakpoint invalid after deletion"
+	gdb_scm_test_silent_cmd "guile (register-breakpoint! bp1)" \
+	    "re-register bp1"
+	gdb_test "guile (print (breakpoint-valid? bp1))" \
+	    "= #t" "breakpoint valid after re-registration"
+    }
+}
+
 test_bkpt_basic
 test_bkpt_deletion
 test_bkpt_cond_and_cmds
@@ -436,3 +494,4 @@  test_bkpt_invisible
 test_watchpoints
 test_bkpt_internal
 test_bkpt_eval_funcs
+test_bkpt_registration