[PATCH] Guile: temporary breakpoints

[PATCH] Guile: temporary breakpoints

[George Barrett]

Adds API to the Guile bindings for creating temporary breakpoints and
querying whether an existing breakpoint object is temporary. This is
effectively a transliteration of the Python implementation.

It's worth noting that the added `is_temporary' flag is ignored in the
watchpoint registration path. This replicates the behaviour of the
Python implementation, but might be a bit surprising for users.

gdb/ChangeLog:

2021-04-26  George Barrett  <bob@bob131.so>

	* guile/scm-breakpoint.c (gdbscm_breakpoint_object::spec): Add
	is_temporary field.
	(temporary_keyword): Add keyword object for make-breakpoint
	argument parsing.
	(gdbscm_make_breakpoint): Accept #:temporary keyword argument
	and store the value in the allocated object's
	spec.is_temporary.
	(gdbscm_register_breakpoint_x): Pass the breakpoint's
	spec.is_temporary value to create_breakpoint.
	(gdbscm_breakpoint_temporary): Add breakpoint-temporary?
	procedure implementation.
	(breakpoint_functions::make-breakpoint): Update documentation
	string and fix a typo.
	(breakpoint_functions::breakpoint-temporary?): Add
	breakpoint-temporary? procedure.
	(gdbscm_initialize_breakpoints): Initialise temporary_keyword
	variable.

gdb/doc/ChangeLog:

2021-04-26  George Barrett  <bob@bob131.so>

	* guile.texi (Breakpoints In Guile): Update make-breakpoint
	documentation to reflect new #:temporary argument.
	Add documentation for new breakpoint-temporary? procedure.

gdb/testsuite/ChangeLog:

2021-04-26  George Barrett  <bob@bob131.so>

	* gdb.guile/scm-breakpoint.exp: Add additional tests for
	temporary breakpoints.
---
 gdb/doc/guile.texi                         | 16 +++++++-
 gdb/guile/scm-breakpoint.c                 | 43 ++++++++++++++++++----
 gdb/testsuite/gdb.guile/scm-breakpoint.exp | 35 ++++++++++++++++++
 3 files changed, 85 insertions(+), 9 deletions(-)

diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
index 762a82a08c5..649dac559ad 100644
--- a/gdb/doc/guile.texi
+++ b/gdb/doc/guile.texi
@@ -2947,7 +2947,7 @@ The following breakpoint-related procedures are provided by the
 @code{(gdb)} module:
 
 @c TODO: line length
-@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]}
+@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]} @r{[}#:temporary temporary@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,
@@ -2973,6 +2973,11 @@ registered, nor will it be listed in the output from @code{info breakpoints}
 If an internal flag is not provided, the breakpoint is visible
 (non-internal).
 
+The optional @var{temporary} argument makes the breakpoint a temporary
+breakpoint.  Temporary breakpoints are deleted after they have been hit,
+after which the Guile breakpoint is no longer usable (although it may be
+re-registered with @code{register-breakpoint!}).
+
 When a watchpoint is created, @value{GDBN} will try to create a
 hardware assisted watchpoint.  If successful, the type of the watchpoint
 is changed from @code{BP_WATCHPOINT} to @code{BP_HARDWARE_WATCHPOINT}
@@ -3065,6 +3070,15 @@ Return the breakpoint's number --- the identifier used by
 the user to manipulate the breakpoint.
 @end deffn
 
+@deffn {Scheme Procedure} breakpoint-temporary? breakpoint
+Return @code{#t} if the breakpoint was created as a temporary
+breakpoint.  Temporary breakpoints are automatically deleted after that
+breakpoint has been hit.  Calling this function, and all other functions
+other than @code{breakpoint-valid?} and @code{register-breakpoint!},
+will result in an error after the breakpoint has been hit (as it has
+been automatically deleted).
+@end deffn
+
 @deffn {Scheme Procedure} breakpoint-type breakpoint
 Return the breakpoint's type --- the identifier used to
 determine the actual breakpoint type or use-case.
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index af63893461b..aa9b2ba0d33 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -69,6 +69,9 @@ typedef struct gdbscm_breakpoint_object
 
     /* Non-zero if the breakpoint is an "internal" breakpoint.  */
     int is_internal;
+
+    /* Non-zero if the breakpoint is temporary.  */
+    int is_temporary;
   } spec;
 
   /* The breakpoint number according to gdb.
@@ -103,6 +106,7 @@ static SCM pending_breakpoint_scm = SCM_BOOL_F;
 static SCM type_keyword;
 static SCM wp_class_keyword;
 static SCM internal_keyword;
+static SCM temporary_keyword;
 \f
 /* Administrivia for breakpoint smobs.  */
 
@@ -329,7 +333,7 @@ bpscm_get_valid_breakpoint_smob_arg_unsafe (SCM self, int arg_pos,
 /* Breakpoint methods.  */
 
 /* (make-breakpoint string [#:type integer] [#:wp-class integer]
-    [#:internal boolean) -> <gdb:breakpoint>
+    [#:internal boolean] [#:temporary boolean]) -> <gdb:breakpoint>
 
    The result is the <gdb:breakpoint> Scheme object.
    The breakpoint is not available to be used yet, however.
@@ -339,22 +343,26 @@ static SCM
 gdbscm_make_breakpoint (SCM location_scm, SCM rest)
 {
   const SCM keywords[] = {
-    type_keyword, wp_class_keyword, internal_keyword, SCM_BOOL_F
+    type_keyword, wp_class_keyword, internal_keyword,
+    temporary_keyword, SCM_BOOL_F
   };
   char *s;
   char *location;
-  int type_arg_pos = -1, access_type_arg_pos = -1, internal_arg_pos = -1;
+  int type_arg_pos = -1, access_type_arg_pos = -1,
+      internal_arg_pos = -1, temporary_arg_pos = -1;
   enum bptype type = bp_breakpoint;
   enum target_hw_bp_type access_type = hw_write;
   int internal = 0;
+  int temporary = 0;
   SCM result;
   breakpoint_smob *bp_smob;
 
-  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iit",
+  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iitt",
 			      location_scm, &location, rest,
 			      &type_arg_pos, &type,
 			      &access_type_arg_pos, &access_type,
-			      &internal_arg_pos, &internal);
+			      &internal_arg_pos, &internal,
+			      &temporary_arg_pos, &temporary);
 
   result = bpscm_make_breakpoint_smob ();
   bp_smob = (breakpoint_smob *) SCM_SMOB_DATA (result);
@@ -397,6 +405,7 @@ gdbscm_make_breakpoint (SCM location_scm, SCM rest)
   bp_smob->spec.type = type;
   bp_smob->spec.access_type = access_type;
   bp_smob->spec.is_internal = internal;
+  bp_smob->spec.is_temporary = temporary;
 
   return result;
 }
@@ -432,6 +441,7 @@ gdbscm_register_breakpoint_x (SCM self)
   try
     {
       int internal = bp_smob->spec.is_internal;
+      int temporary = bp_smob->spec.is_temporary;
 
       switch (bp_smob->spec.type)
 	{
@@ -442,7 +452,7 @@ gdbscm_register_breakpoint_x (SCM self)
 	    create_breakpoint (get_current_arch (),
 			       eloc.get (), NULL, -1, NULL, false,
 			       0,
-			       0, bp_breakpoint,
+			       temporary, bp_breakpoint,
 			       0,
 			       AUTO_BOOLEAN_TRUE,
 			       ops,
@@ -1029,6 +1039,18 @@ gdbscm_breakpoint_number (SCM self)
 
   return scm_from_long (bp_smob->number);
 }
+
+/* (breakpoint-temporary? <gdb:breakpoint>) -> boolean */
+
+static SCM
+gdbscm_breakpoint_temporary (SCM self)
+{
+  breakpoint_smob *bp_smob
+    = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
+
+  return scm_from_bool (bp_smob->bp->disposition == disp_del ||
+			bp_smob->bp->disposition == disp_del_at_next_stop);
+}
 \f
 /* Return TRUE if "stop" has been set for this breakpoint.
 
@@ -1159,9 +1181,9 @@ static const scheme_function breakpoint_functions[] =
 Create a GDB breakpoint object.\n\
 \n\
   Arguments:\n\
-    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>]\n\
+    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>] [#:temporary <bool>]\n\
   Returns:\n\
-    <gdb:breakpoint object" },
+    <gdb:breakpoint> object" },
 
   { "register-breakpoint!", 1, 0, 0,
     as_a_scm_t_subr (gdbscm_register_breakpoint_x),
@@ -1190,6 +1212,10 @@ Return #t if the breakpoint has not been deleted from GDB." },
     "\
 Return the breakpoint's number." },
 
+  { "breakpoint-temporary?", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_temporary),
+    "\
+Return #t if the breakpoint is a temporary breakpoint." },
+
   { "breakpoint-type", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_type),
     "\
 Return the type of the breakpoint." },
@@ -1331,4 +1357,5 @@ gdbscm_initialize_breakpoints (void)
   type_keyword = scm_from_latin1_keyword ("type");
   wp_class_keyword = scm_from_latin1_keyword ("wp-class");
   internal_keyword = scm_from_latin1_keyword ("internal");
+  temporary_keyword = scm_from_latin1_keyword ("temporary");
 }
diff --git a/gdb/testsuite/gdb.guile/scm-breakpoint.exp b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
index 071a6f66f7e..1fa4711ea23 100644
--- a/gdb/testsuite/gdb.guile/scm-breakpoint.exp
+++ b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
@@ -487,6 +487,40 @@ proc test_bkpt_registration {} {
     }
 }
 
+proc_with_prefix test_bkpt_temporary { } {
+    global srcfile testfile hex decimal
+
+    with_test_prefix test_bkpt_temporary {
+	# Start with a fresh gdb.
+	clean_restart ${testfile}
+
+	if ![gdb_guile_runto_main] then {
+	    fail "cannot run to main."
+	    return 0
+	}
+	delete_breakpoints
+
+	set ibp_location [gdb_get_line_number "Break at multiply."]
+	gdb_scm_test_silent_cmd "guile (define ibp (make-breakpoint \"$ibp_location\" #:temporary #t))" \
+	    "create temporary breakpoint"
+	gdb_scm_test_silent_cmd "guile (register-breakpoint! ibp)" \
+	    "register ibp"
+	gdb_test "info breakpoints" \
+	    "2.*breakpoint.*del.*scm-breakpoint\.c:$ibp_location.*" \
+	    "check info breakpoints shows breakpoint with temporary status"
+	gdb_test "guile (print (breakpoint-location ibp))" "scm-breakpoint\.c:$ibp_location*" \
+	    "check temporary breakpoint location"
+	gdb_test "guile (print (breakpoint-temporary? ibp))" "#t" \
+	    "check breakpoint temporary status"
+	gdb_continue_to_breakpoint "Break at multiply." \
+	    ".*$srcfile:$ibp_location.*"
+	gdb_test "guile (print (breakpoint-temporary? ibp))" "Invalid object: <gdb:breakpoint>.*" \
+	    "check temporary breakpoint is deleted after being hit"
+	gdb_test "info breakpoints" "No breakpoints or watchpoints.*" \
+	    "check info breakpoints shows temporary breakpoint is deleted"
+    }
+}
+
 proc test_bkpt_address {} {
     global decimal srcfile
 
@@ -529,5 +563,6 @@ test_watchpoints
 test_bkpt_internal
 test_bkpt_eval_funcs
 test_bkpt_registration
+test_bkpt_temporary
 test_bkpt_address
 test_bkpt_probe
-- 
2.30.2

[PING] [PATCH] Guile: temporary breakpoints

[George Barrett]

Would someone mind reviewing this?

Thanks.

On Mon, Apr 26, 2021 at 04:56:56AM +1000, George Barrett wrote:
> Adds API to the Guile bindings for creating temporary breakpoints and
> querying whether an existing breakpoint object is temporary. This is
> effectively a transliteration of the Python implementation.
> 
> It's worth noting that the added `is_temporary' flag is ignored in the
> watchpoint registration path. This replicates the behaviour of the
> Python implementation, but might be a bit surprising for users.
> 
> gdb/ChangeLog:
> 
> 2021-04-26  George Barrett  <bob@bob131.so>
> 
> 	* guile/scm-breakpoint.c (gdbscm_breakpoint_object::spec): Add
> 	is_temporary field.
> 	(temporary_keyword): Add keyword object for make-breakpoint
> 	argument parsing.
> 	(gdbscm_make_breakpoint): Accept #:temporary keyword argument
> 	and store the value in the allocated object's
> 	spec.is_temporary.
> 	(gdbscm_register_breakpoint_x): Pass the breakpoint's
> 	spec.is_temporary value to create_breakpoint.
> 	(gdbscm_breakpoint_temporary): Add breakpoint-temporary?
> 	procedure implementation.
> 	(breakpoint_functions::make-breakpoint): Update documentation
> 	string and fix a typo.
> 	(breakpoint_functions::breakpoint-temporary?): Add
> 	breakpoint-temporary? procedure.
> 	(gdbscm_initialize_breakpoints): Initialise temporary_keyword
> 	variable.
> 
> gdb/doc/ChangeLog:
> 
> 2021-04-26  George Barrett  <bob@bob131.so>
> 
> 	* guile.texi (Breakpoints In Guile): Update make-breakpoint
> 	documentation to reflect new #:temporary argument.
> 	Add documentation for new breakpoint-temporary? procedure.
> 
> gdb/testsuite/ChangeLog:
> 
> 2021-04-26  George Barrett  <bob@bob131.so>
> 
> 	* gdb.guile/scm-breakpoint.exp: Add additional tests for
> 	temporary breakpoints.
> ---
>  gdb/doc/guile.texi                         | 16 +++++++-
>  gdb/guile/scm-breakpoint.c                 | 43 ++++++++++++++++++----
>  gdb/testsuite/gdb.guile/scm-breakpoint.exp | 35 ++++++++++++++++++
>  3 files changed, 85 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
> index 762a82a08c5..649dac559ad 100644
> --- a/gdb/doc/guile.texi
> +++ b/gdb/doc/guile.texi
> @@ -2947,7 +2947,7 @@ The following breakpoint-related procedures are provided by the
>  @code{(gdb)} module:
>  
>  @c TODO: line length
> -@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]}
> +@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]} @r{[}#:temporary temporary@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,
> @@ -2973,6 +2973,11 @@ registered, nor will it be listed in the output from @code{info breakpoints}
>  If an internal flag is not provided, the breakpoint is visible
>  (non-internal).
>  
> +The optional @var{temporary} argument makes the breakpoint a temporary
> +breakpoint.  Temporary breakpoints are deleted after they have been hit,
> +after which the Guile breakpoint is no longer usable (although it may be
> +re-registered with @code{register-breakpoint!}).
> +
>  When a watchpoint is created, @value{GDBN} will try to create a
>  hardware assisted watchpoint.  If successful, the type of the watchpoint
>  is changed from @code{BP_WATCHPOINT} to @code{BP_HARDWARE_WATCHPOINT}
> @@ -3065,6 +3070,15 @@ Return the breakpoint's number --- the identifier used by
>  the user to manipulate the breakpoint.
>  @end deffn
>  
> +@deffn {Scheme Procedure} breakpoint-temporary? breakpoint
> +Return @code{#t} if the breakpoint was created as a temporary
> +breakpoint.  Temporary breakpoints are automatically deleted after that
> +breakpoint has been hit.  Calling this function, and all other functions
> +other than @code{breakpoint-valid?} and @code{register-breakpoint!},
> +will result in an error after the breakpoint has been hit (as it has
> +been automatically deleted).
> +@end deffn
> +
>  @deffn {Scheme Procedure} breakpoint-type breakpoint
>  Return the breakpoint's type --- the identifier used to
>  determine the actual breakpoint type or use-case.
> diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
> index af63893461b..aa9b2ba0d33 100644
> --- a/gdb/guile/scm-breakpoint.c
> +++ b/gdb/guile/scm-breakpoint.c
> @@ -69,6 +69,9 @@ typedef struct gdbscm_breakpoint_object
>  
>      /* Non-zero if the breakpoint is an "internal" breakpoint.  */
>      int is_internal;
> +
> +    /* Non-zero if the breakpoint is temporary.  */
> +    int is_temporary;
>    } spec;
>  
>    /* The breakpoint number according to gdb.
> @@ -103,6 +106,7 @@ static SCM pending_breakpoint_scm = SCM_BOOL_F;
>  static SCM type_keyword;
>  static SCM wp_class_keyword;
>  static SCM internal_keyword;
> +static SCM temporary_keyword;
>  \f
>  /* Administrivia for breakpoint smobs.  */
>  
> @@ -329,7 +333,7 @@ bpscm_get_valid_breakpoint_smob_arg_unsafe (SCM self, int arg_pos,
>  /* Breakpoint methods.  */
>  
>  /* (make-breakpoint string [#:type integer] [#:wp-class integer]
> -    [#:internal boolean) -> <gdb:breakpoint>
> +    [#:internal boolean] [#:temporary boolean]) -> <gdb:breakpoint>
>  
>     The result is the <gdb:breakpoint> Scheme object.
>     The breakpoint is not available to be used yet, however.
> @@ -339,22 +343,26 @@ static SCM
>  gdbscm_make_breakpoint (SCM location_scm, SCM rest)
>  {
>    const SCM keywords[] = {
> -    type_keyword, wp_class_keyword, internal_keyword, SCM_BOOL_F
> +    type_keyword, wp_class_keyword, internal_keyword,
> +    temporary_keyword, SCM_BOOL_F
>    };
>    char *s;
>    char *location;
> -  int type_arg_pos = -1, access_type_arg_pos = -1, internal_arg_pos = -1;
> +  int type_arg_pos = -1, access_type_arg_pos = -1,
> +      internal_arg_pos = -1, temporary_arg_pos = -1;
>    enum bptype type = bp_breakpoint;
>    enum target_hw_bp_type access_type = hw_write;
>    int internal = 0;
> +  int temporary = 0;
>    SCM result;
>    breakpoint_smob *bp_smob;
>  
> -  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iit",
> +  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iitt",
>  			      location_scm, &location, rest,
>  			      &type_arg_pos, &type,
>  			      &access_type_arg_pos, &access_type,
> -			      &internal_arg_pos, &internal);
> +			      &internal_arg_pos, &internal,
> +			      &temporary_arg_pos, &temporary);
>  
>    result = bpscm_make_breakpoint_smob ();
>    bp_smob = (breakpoint_smob *) SCM_SMOB_DATA (result);
> @@ -397,6 +405,7 @@ gdbscm_make_breakpoint (SCM location_scm, SCM rest)
>    bp_smob->spec.type = type;
>    bp_smob->spec.access_type = access_type;
>    bp_smob->spec.is_internal = internal;
> +  bp_smob->spec.is_temporary = temporary;
>  
>    return result;
>  }
> @@ -432,6 +441,7 @@ gdbscm_register_breakpoint_x (SCM self)
>    try
>      {
>        int internal = bp_smob->spec.is_internal;
> +      int temporary = bp_smob->spec.is_temporary;
>  
>        switch (bp_smob->spec.type)
>  	{
> @@ -442,7 +452,7 @@ gdbscm_register_breakpoint_x (SCM self)
>  	    create_breakpoint (get_current_arch (),
>  			       eloc.get (), NULL, -1, NULL, false,
>  			       0,
> -			       0, bp_breakpoint,
> +			       temporary, bp_breakpoint,
>  			       0,
>  			       AUTO_BOOLEAN_TRUE,
>  			       ops,
> @@ -1029,6 +1039,18 @@ gdbscm_breakpoint_number (SCM self)
>  
>    return scm_from_long (bp_smob->number);
>  }
> +
> +/* (breakpoint-temporary? <gdb:breakpoint>) -> boolean */
> +
> +static SCM
> +gdbscm_breakpoint_temporary (SCM self)
> +{
> +  breakpoint_smob *bp_smob
> +    = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> +
> +  return scm_from_bool (bp_smob->bp->disposition == disp_del ||
> +			bp_smob->bp->disposition == disp_del_at_next_stop);
> +}
>  \f
>  /* Return TRUE if "stop" has been set for this breakpoint.
>  
> @@ -1159,9 +1181,9 @@ static const scheme_function breakpoint_functions[] =
>  Create a GDB breakpoint object.\n\
>  \n\
>    Arguments:\n\
> -    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>]\n\
> +    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>] [#:temporary <bool>]\n\
>    Returns:\n\
> -    <gdb:breakpoint object" },
> +    <gdb:breakpoint> object" },
>  
>    { "register-breakpoint!", 1, 0, 0,
>      as_a_scm_t_subr (gdbscm_register_breakpoint_x),
> @@ -1190,6 +1212,10 @@ Return #t if the breakpoint has not been deleted from GDB." },
>      "\
>  Return the breakpoint's number." },
>  
> +  { "breakpoint-temporary?", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_temporary),
> +    "\
> +Return #t if the breakpoint is a temporary breakpoint." },
> +
>    { "breakpoint-type", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_type),
>      "\
>  Return the type of the breakpoint." },
> @@ -1331,4 +1357,5 @@ gdbscm_initialize_breakpoints (void)
>    type_keyword = scm_from_latin1_keyword ("type");
>    wp_class_keyword = scm_from_latin1_keyword ("wp-class");
>    internal_keyword = scm_from_latin1_keyword ("internal");
> +  temporary_keyword = scm_from_latin1_keyword ("temporary");
>  }
> diff --git a/gdb/testsuite/gdb.guile/scm-breakpoint.exp b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> index 071a6f66f7e..1fa4711ea23 100644
> --- a/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> +++ b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> @@ -487,6 +487,40 @@ proc test_bkpt_registration {} {
>      }
>  }
>  
> +proc_with_prefix test_bkpt_temporary { } {
> +    global srcfile testfile hex decimal
> +
> +    with_test_prefix test_bkpt_temporary {
> +	# Start with a fresh gdb.
> +	clean_restart ${testfile}
> +
> +	if ![gdb_guile_runto_main] then {
> +	    fail "cannot run to main."
> +	    return 0
> +	}
> +	delete_breakpoints
> +
> +	set ibp_location [gdb_get_line_number "Break at multiply."]
> +	gdb_scm_test_silent_cmd "guile (define ibp (make-breakpoint \"$ibp_location\" #:temporary #t))" \
> +	    "create temporary breakpoint"
> +	gdb_scm_test_silent_cmd "guile (register-breakpoint! ibp)" \
> +	    "register ibp"
> +	gdb_test "info breakpoints" \
> +	    "2.*breakpoint.*del.*scm-breakpoint\.c:$ibp_location.*" \
> +	    "check info breakpoints shows breakpoint with temporary status"
> +	gdb_test "guile (print (breakpoint-location ibp))" "scm-breakpoint\.c:$ibp_location*" \
> +	    "check temporary breakpoint location"
> +	gdb_test "guile (print (breakpoint-temporary? ibp))" "#t" \
> +	    "check breakpoint temporary status"
> +	gdb_continue_to_breakpoint "Break at multiply." \
> +	    ".*$srcfile:$ibp_location.*"
> +	gdb_test "guile (print (breakpoint-temporary? ibp))" "Invalid object: <gdb:breakpoint>.*" \
> +	    "check temporary breakpoint is deleted after being hit"
> +	gdb_test "info breakpoints" "No breakpoints or watchpoints.*" \
> +	    "check info breakpoints shows temporary breakpoint is deleted"
> +    }
> +}
> +
>  proc test_bkpt_address {} {
>      global decimal srcfile
>  
> @@ -529,5 +563,6 @@ test_watchpoints
>  test_bkpt_internal
>  test_bkpt_eval_funcs
>  test_bkpt_registration
> +test_bkpt_temporary
>  test_bkpt_address
>  test_bkpt_probe
> -- 
> 2.30.2

[PING**2] [PATCH] Guile: temporary breakpoints

[George Barrett]

Ping

On Wed, May 12, 2021 at 04:23:04AM +1000, George Barrett wrote:
> Would someone mind reviewing this?
> 
> Thanks.
> 
> On Mon, Apr 26, 2021 at 04:56:56AM +1000, George Barrett wrote:
> > Adds API to the Guile bindings for creating temporary breakpoints and
> > querying whether an existing breakpoint object is temporary. This is
> > effectively a transliteration of the Python implementation.
> > 
> > It's worth noting that the added `is_temporary' flag is ignored in the
> > watchpoint registration path. This replicates the behaviour of the
> > Python implementation, but might be a bit surprising for users.
> > 
> > gdb/ChangeLog:
> > 
> > 2021-04-26  George Barrett  <bob@bob131.so>
> > 
> > 	* guile/scm-breakpoint.c (gdbscm_breakpoint_object::spec): Add
> > 	is_temporary field.
> > 	(temporary_keyword): Add keyword object for make-breakpoint
> > 	argument parsing.
> > 	(gdbscm_make_breakpoint): Accept #:temporary keyword argument
> > 	and store the value in the allocated object's
> > 	spec.is_temporary.
> > 	(gdbscm_register_breakpoint_x): Pass the breakpoint's
> > 	spec.is_temporary value to create_breakpoint.
> > 	(gdbscm_breakpoint_temporary): Add breakpoint-temporary?
> > 	procedure implementation.
> > 	(breakpoint_functions::make-breakpoint): Update documentation
> > 	string and fix a typo.
> > 	(breakpoint_functions::breakpoint-temporary?): Add
> > 	breakpoint-temporary? procedure.
> > 	(gdbscm_initialize_breakpoints): Initialise temporary_keyword
> > 	variable.
> > 
> > gdb/doc/ChangeLog:
> > 
> > 2021-04-26  George Barrett  <bob@bob131.so>
> > 
> > 	* guile.texi (Breakpoints In Guile): Update make-breakpoint
> > 	documentation to reflect new #:temporary argument.
> > 	Add documentation for new breakpoint-temporary? procedure.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> > 2021-04-26  George Barrett  <bob@bob131.so>
> > 
> > 	* gdb.guile/scm-breakpoint.exp: Add additional tests for
> > 	temporary breakpoints.
> > ---
> >  gdb/doc/guile.texi                         | 16 +++++++-
> >  gdb/guile/scm-breakpoint.c                 | 43 ++++++++++++++++++----
> >  gdb/testsuite/gdb.guile/scm-breakpoint.exp | 35 ++++++++++++++++++
> >  3 files changed, 85 insertions(+), 9 deletions(-)
> > 
> > diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
> > index 762a82a08c5..649dac559ad 100644
> > --- a/gdb/doc/guile.texi
> > +++ b/gdb/doc/guile.texi
> > @@ -2947,7 +2947,7 @@ The following breakpoint-related procedures are provided by the
> >  @code{(gdb)} module:
> >  
> >  @c TODO: line length
> > -@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]}
> > +@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]} @r{[}#:temporary temporary@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,
> > @@ -2973,6 +2973,11 @@ registered, nor will it be listed in the output from @code{info breakpoints}
> >  If an internal flag is not provided, the breakpoint is visible
> >  (non-internal).
> >  
> > +The optional @var{temporary} argument makes the breakpoint a temporary
> > +breakpoint.  Temporary breakpoints are deleted after they have been hit,
> > +after which the Guile breakpoint is no longer usable (although it may be
> > +re-registered with @code{register-breakpoint!}).
> > +
> >  When a watchpoint is created, @value{GDBN} will try to create a
> >  hardware assisted watchpoint.  If successful, the type of the watchpoint
> >  is changed from @code{BP_WATCHPOINT} to @code{BP_HARDWARE_WATCHPOINT}
> > @@ -3065,6 +3070,15 @@ Return the breakpoint's number --- the identifier used by
> >  the user to manipulate the breakpoint.
> >  @end deffn
> >  
> > +@deffn {Scheme Procedure} breakpoint-temporary? breakpoint
> > +Return @code{#t} if the breakpoint was created as a temporary
> > +breakpoint.  Temporary breakpoints are automatically deleted after that
> > +breakpoint has been hit.  Calling this function, and all other functions
> > +other than @code{breakpoint-valid?} and @code{register-breakpoint!},
> > +will result in an error after the breakpoint has been hit (as it has
> > +been automatically deleted).
> > +@end deffn
> > +
> >  @deffn {Scheme Procedure} breakpoint-type breakpoint
> >  Return the breakpoint's type --- the identifier used to
> >  determine the actual breakpoint type or use-case.
> > diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
> > index af63893461b..aa9b2ba0d33 100644
> > --- a/gdb/guile/scm-breakpoint.c
> > +++ b/gdb/guile/scm-breakpoint.c
> > @@ -69,6 +69,9 @@ typedef struct gdbscm_breakpoint_object
> >  
> >      /* Non-zero if the breakpoint is an "internal" breakpoint.  */
> >      int is_internal;
> > +
> > +    /* Non-zero if the breakpoint is temporary.  */
> > +    int is_temporary;
> >    } spec;
> >  
> >    /* The breakpoint number according to gdb.
> > @@ -103,6 +106,7 @@ static SCM pending_breakpoint_scm = SCM_BOOL_F;
> >  static SCM type_keyword;
> >  static SCM wp_class_keyword;
> >  static SCM internal_keyword;
> > +static SCM temporary_keyword;
> >  \f
> >  /* Administrivia for breakpoint smobs.  */
> >  
> > @@ -329,7 +333,7 @@ bpscm_get_valid_breakpoint_smob_arg_unsafe (SCM self, int arg_pos,
> >  /* Breakpoint methods.  */
> >  
> >  /* (make-breakpoint string [#:type integer] [#:wp-class integer]
> > -    [#:internal boolean) -> <gdb:breakpoint>
> > +    [#:internal boolean] [#:temporary boolean]) -> <gdb:breakpoint>
> >  
> >     The result is the <gdb:breakpoint> Scheme object.
> >     The breakpoint is not available to be used yet, however.
> > @@ -339,22 +343,26 @@ static SCM
> >  gdbscm_make_breakpoint (SCM location_scm, SCM rest)
> >  {
> >    const SCM keywords[] = {
> > -    type_keyword, wp_class_keyword, internal_keyword, SCM_BOOL_F
> > +    type_keyword, wp_class_keyword, internal_keyword,
> > +    temporary_keyword, SCM_BOOL_F
> >    };
> >    char *s;
> >    char *location;
> > -  int type_arg_pos = -1, access_type_arg_pos = -1, internal_arg_pos = -1;
> > +  int type_arg_pos = -1, access_type_arg_pos = -1,
> > +      internal_arg_pos = -1, temporary_arg_pos = -1;
> >    enum bptype type = bp_breakpoint;
> >    enum target_hw_bp_type access_type = hw_write;
> >    int internal = 0;
> > +  int temporary = 0;
> >    SCM result;
> >    breakpoint_smob *bp_smob;
> >  
> > -  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iit",
> > +  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iitt",
> >  			      location_scm, &location, rest,
> >  			      &type_arg_pos, &type,
> >  			      &access_type_arg_pos, &access_type,
> > -			      &internal_arg_pos, &internal);
> > +			      &internal_arg_pos, &internal,
> > +			      &temporary_arg_pos, &temporary);
> >  
> >    result = bpscm_make_breakpoint_smob ();
> >    bp_smob = (breakpoint_smob *) SCM_SMOB_DATA (result);
> > @@ -397,6 +405,7 @@ gdbscm_make_breakpoint (SCM location_scm, SCM rest)
> >    bp_smob->spec.type = type;
> >    bp_smob->spec.access_type = access_type;
> >    bp_smob->spec.is_internal = internal;
> > +  bp_smob->spec.is_temporary = temporary;
> >  
> >    return result;
> >  }
> > @@ -432,6 +441,7 @@ gdbscm_register_breakpoint_x (SCM self)
> >    try
> >      {
> >        int internal = bp_smob->spec.is_internal;
> > +      int temporary = bp_smob->spec.is_temporary;
> >  
> >        switch (bp_smob->spec.type)
> >  	{
> > @@ -442,7 +452,7 @@ gdbscm_register_breakpoint_x (SCM self)
> >  	    create_breakpoint (get_current_arch (),
> >  			       eloc.get (), NULL, -1, NULL, false,
> >  			       0,
> > -			       0, bp_breakpoint,
> > +			       temporary, bp_breakpoint,
> >  			       0,
> >  			       AUTO_BOOLEAN_TRUE,
> >  			       ops,
> > @@ -1029,6 +1039,18 @@ gdbscm_breakpoint_number (SCM self)
> >  
> >    return scm_from_long (bp_smob->number);
> >  }
> > +
> > +/* (breakpoint-temporary? <gdb:breakpoint>) -> boolean */
> > +
> > +static SCM
> > +gdbscm_breakpoint_temporary (SCM self)
> > +{
> > +  breakpoint_smob *bp_smob
> > +    = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> > +
> > +  return scm_from_bool (bp_smob->bp->disposition == disp_del ||
> > +			bp_smob->bp->disposition == disp_del_at_next_stop);
> > +}
> >  \f
> >  /* Return TRUE if "stop" has been set for this breakpoint.
> >  
> > @@ -1159,9 +1181,9 @@ static const scheme_function breakpoint_functions[] =
> >  Create a GDB breakpoint object.\n\
> >  \n\
> >    Arguments:\n\
> > -    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>]\n\
> > +    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>] [#:temporary <bool>]\n\
> >    Returns:\n\
> > -    <gdb:breakpoint object" },
> > +    <gdb:breakpoint> object" },
> >  
> >    { "register-breakpoint!", 1, 0, 0,
> >      as_a_scm_t_subr (gdbscm_register_breakpoint_x),
> > @@ -1190,6 +1212,10 @@ Return #t if the breakpoint has not been deleted from GDB." },
> >      "\
> >  Return the breakpoint's number." },
> >  
> > +  { "breakpoint-temporary?", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_temporary),
> > +    "\
> > +Return #t if the breakpoint is a temporary breakpoint." },
> > +
> >    { "breakpoint-type", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_type),
> >      "\
> >  Return the type of the breakpoint." },
> > @@ -1331,4 +1357,5 @@ gdbscm_initialize_breakpoints (void)
> >    type_keyword = scm_from_latin1_keyword ("type");
> >    wp_class_keyword = scm_from_latin1_keyword ("wp-class");
> >    internal_keyword = scm_from_latin1_keyword ("internal");
> > +  temporary_keyword = scm_from_latin1_keyword ("temporary");
> >  }
> > diff --git a/gdb/testsuite/gdb.guile/scm-breakpoint.exp b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> > index 071a6f66f7e..1fa4711ea23 100644
> > --- a/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> > +++ b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> > @@ -487,6 +487,40 @@ proc test_bkpt_registration {} {
> >      }
> >  }
> >  
> > +proc_with_prefix test_bkpt_temporary { } {
> > +    global srcfile testfile hex decimal
> > +
> > +    with_test_prefix test_bkpt_temporary {
> > +	# Start with a fresh gdb.
> > +	clean_restart ${testfile}
> > +
> > +	if ![gdb_guile_runto_main] then {
> > +	    fail "cannot run to main."
> > +	    return 0
> > +	}
> > +	delete_breakpoints
> > +
> > +	set ibp_location [gdb_get_line_number "Break at multiply."]
> > +	gdb_scm_test_silent_cmd "guile (define ibp (make-breakpoint \"$ibp_location\" #:temporary #t))" \
> > +	    "create temporary breakpoint"
> > +	gdb_scm_test_silent_cmd "guile (register-breakpoint! ibp)" \
> > +	    "register ibp"
> > +	gdb_test "info breakpoints" \
> > +	    "2.*breakpoint.*del.*scm-breakpoint\.c:$ibp_location.*" \
> > +	    "check info breakpoints shows breakpoint with temporary status"
> > +	gdb_test "guile (print (breakpoint-location ibp))" "scm-breakpoint\.c:$ibp_location*" \
> > +	    "check temporary breakpoint location"
> > +	gdb_test "guile (print (breakpoint-temporary? ibp))" "#t" \
> > +	    "check breakpoint temporary status"
> > +	gdb_continue_to_breakpoint "Break at multiply." \
> > +	    ".*$srcfile:$ibp_location.*"
> > +	gdb_test "guile (print (breakpoint-temporary? ibp))" "Invalid object: <gdb:breakpoint>.*" \
> > +	    "check temporary breakpoint is deleted after being hit"
> > +	gdb_test "info breakpoints" "No breakpoints or watchpoints.*" \
> > +	    "check info breakpoints shows temporary breakpoint is deleted"
> > +    }
> > +}
> > +
> >  proc test_bkpt_address {} {
> >      global decimal srcfile
> >  
> > @@ -529,5 +563,6 @@ test_watchpoints
> >  test_bkpt_internal
> >  test_bkpt_eval_funcs
> >  test_bkpt_registration
> > +test_bkpt_temporary
> >  test_bkpt_address
> >  test_bkpt_probe
> > -- 
> > 2.30.2

[PING**3] [PATCH] Guile: temporary breakpoints

[George Barrett]

On Wed, May 19, 2021 at 03:15:28PM +1000, George Barrett wrote:
> Ping
> 
> On Wed, May 12, 2021 at 04:23:04AM +1000, George Barrett wrote:
> > Would someone mind reviewing this?
> > 
> > Thanks.
> > 
> > On Mon, Apr 26, 2021 at 04:56:56AM +1000, George Barrett wrote:
> > > Adds API to the Guile bindings for creating temporary breakpoints and
> > > querying whether an existing breakpoint object is temporary. This is
> > > effectively a transliteration of the Python implementation.
> > > 
> > > It's worth noting that the added `is_temporary' flag is ignored in the
> > > watchpoint registration path. This replicates the behaviour of the
> > > Python implementation, but might be a bit surprising for users.
> > > 
> > > gdb/ChangeLog:
> > > 
> > > 2021-04-26  George Barrett  <bob@bob131.so>
> > > 
> > > 	* guile/scm-breakpoint.c (gdbscm_breakpoint_object::spec): Add
> > > 	is_temporary field.
> > > 	(temporary_keyword): Add keyword object for make-breakpoint
> > > 	argument parsing.
> > > 	(gdbscm_make_breakpoint): Accept #:temporary keyword argument
> > > 	and store the value in the allocated object's
> > > 	spec.is_temporary.
> > > 	(gdbscm_register_breakpoint_x): Pass the breakpoint's
> > > 	spec.is_temporary value to create_breakpoint.
> > > 	(gdbscm_breakpoint_temporary): Add breakpoint-temporary?
> > > 	procedure implementation.
> > > 	(breakpoint_functions::make-breakpoint): Update documentation
> > > 	string and fix a typo.
> > > 	(breakpoint_functions::breakpoint-temporary?): Add
> > > 	breakpoint-temporary? procedure.
> > > 	(gdbscm_initialize_breakpoints): Initialise temporary_keyword
> > > 	variable.
> > > 
> > > gdb/doc/ChangeLog:
> > > 
> > > 2021-04-26  George Barrett  <bob@bob131.so>
> > > 
> > > 	* guile.texi (Breakpoints In Guile): Update make-breakpoint
> > > 	documentation to reflect new #:temporary argument.
> > > 	Add documentation for new breakpoint-temporary? procedure.
> > > 
> > > gdb/testsuite/ChangeLog:
> > > 
> > > 2021-04-26  George Barrett  <bob@bob131.so>
> > > 
> > > 	* gdb.guile/scm-breakpoint.exp: Add additional tests for
> > > 	temporary breakpoints.
> > > ---
> > >  gdb/doc/guile.texi                         | 16 +++++++-
> > >  gdb/guile/scm-breakpoint.c                 | 43 ++++++++++++++++++----
> > >  gdb/testsuite/gdb.guile/scm-breakpoint.exp | 35 ++++++++++++++++++
> > >  3 files changed, 85 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
> > > index 762a82a08c5..649dac559ad 100644
> > > --- a/gdb/doc/guile.texi
> > > +++ b/gdb/doc/guile.texi
> > > @@ -2947,7 +2947,7 @@ The following breakpoint-related procedures are provided by the
> > >  @code{(gdb)} module:
> > >  
> > >  @c TODO: line length
> > > -@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]}
> > > +@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]} @r{[}#:temporary temporary@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,
> > > @@ -2973,6 +2973,11 @@ registered, nor will it be listed in the output from @code{info breakpoints}
> > >  If an internal flag is not provided, the breakpoint is visible
> > >  (non-internal).
> > >  
> > > +The optional @var{temporary} argument makes the breakpoint a temporary
> > > +breakpoint.  Temporary breakpoints are deleted after they have been hit,
> > > +after which the Guile breakpoint is no longer usable (although it may be
> > > +re-registered with @code{register-breakpoint!}).
> > > +
> > >  When a watchpoint is created, @value{GDBN} will try to create a
> > >  hardware assisted watchpoint.  If successful, the type of the watchpoint
> > >  is changed from @code{BP_WATCHPOINT} to @code{BP_HARDWARE_WATCHPOINT}
> > > @@ -3065,6 +3070,15 @@ Return the breakpoint's number --- the identifier used by
> > >  the user to manipulate the breakpoint.
> > >  @end deffn
> > >  
> > > +@deffn {Scheme Procedure} breakpoint-temporary? breakpoint
> > > +Return @code{#t} if the breakpoint was created as a temporary
> > > +breakpoint.  Temporary breakpoints are automatically deleted after that
> > > +breakpoint has been hit.  Calling this function, and all other functions
> > > +other than @code{breakpoint-valid?} and @code{register-breakpoint!},
> > > +will result in an error after the breakpoint has been hit (as it has
> > > +been automatically deleted).
> > > +@end deffn
> > > +
> > >  @deffn {Scheme Procedure} breakpoint-type breakpoint
> > >  Return the breakpoint's type --- the identifier used to
> > >  determine the actual breakpoint type or use-case.
> > > diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
> > > index af63893461b..aa9b2ba0d33 100644
> > > --- a/gdb/guile/scm-breakpoint.c
> > > +++ b/gdb/guile/scm-breakpoint.c
> > > @@ -69,6 +69,9 @@ typedef struct gdbscm_breakpoint_object
> > >  
> > >      /* Non-zero if the breakpoint is an "internal" breakpoint.  */
> > >      int is_internal;
> > > +
> > > +    /* Non-zero if the breakpoint is temporary.  */
> > > +    int is_temporary;
> > >    } spec;
> > >  
> > >    /* The breakpoint number according to gdb.
> > > @@ -103,6 +106,7 @@ static SCM pending_breakpoint_scm = SCM_BOOL_F;
> > >  static SCM type_keyword;
> > >  static SCM wp_class_keyword;
> > >  static SCM internal_keyword;
> > > +static SCM temporary_keyword;
> > >  \f
> > >  /* Administrivia for breakpoint smobs.  */
> > >  
> > > @@ -329,7 +333,7 @@ bpscm_get_valid_breakpoint_smob_arg_unsafe (SCM self, int arg_pos,
> > >  /* Breakpoint methods.  */
> > >  
> > >  /* (make-breakpoint string [#:type integer] [#:wp-class integer]
> > > -    [#:internal boolean) -> <gdb:breakpoint>
> > > +    [#:internal boolean] [#:temporary boolean]) -> <gdb:breakpoint>
> > >  
> > >     The result is the <gdb:breakpoint> Scheme object.
> > >     The breakpoint is not available to be used yet, however.
> > > @@ -339,22 +343,26 @@ static SCM
> > >  gdbscm_make_breakpoint (SCM location_scm, SCM rest)
> > >  {
> > >    const SCM keywords[] = {
> > > -    type_keyword, wp_class_keyword, internal_keyword, SCM_BOOL_F
> > > +    type_keyword, wp_class_keyword, internal_keyword,
> > > +    temporary_keyword, SCM_BOOL_F
> > >    };
> > >    char *s;
> > >    char *location;
> > > -  int type_arg_pos = -1, access_type_arg_pos = -1, internal_arg_pos = -1;
> > > +  int type_arg_pos = -1, access_type_arg_pos = -1,
> > > +      internal_arg_pos = -1, temporary_arg_pos = -1;
> > >    enum bptype type = bp_breakpoint;
> > >    enum target_hw_bp_type access_type = hw_write;
> > >    int internal = 0;
> > > +  int temporary = 0;
> > >    SCM result;
> > >    breakpoint_smob *bp_smob;
> > >  
> > > -  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iit",
> > > +  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iitt",
> > >  			      location_scm, &location, rest,
> > >  			      &type_arg_pos, &type,
> > >  			      &access_type_arg_pos, &access_type,
> > > -			      &internal_arg_pos, &internal);
> > > +			      &internal_arg_pos, &internal,
> > > +			      &temporary_arg_pos, &temporary);
> > >  
> > >    result = bpscm_make_breakpoint_smob ();
> > >    bp_smob = (breakpoint_smob *) SCM_SMOB_DATA (result);
> > > @@ -397,6 +405,7 @@ gdbscm_make_breakpoint (SCM location_scm, SCM rest)
> > >    bp_smob->spec.type = type;
> > >    bp_smob->spec.access_type = access_type;
> > >    bp_smob->spec.is_internal = internal;
> > > +  bp_smob->spec.is_temporary = temporary;
> > >  
> > >    return result;
> > >  }
> > > @@ -432,6 +441,7 @@ gdbscm_register_breakpoint_x (SCM self)
> > >    try
> > >      {
> > >        int internal = bp_smob->spec.is_internal;
> > > +      int temporary = bp_smob->spec.is_temporary;
> > >  
> > >        switch (bp_smob->spec.type)
> > >  	{
> > > @@ -442,7 +452,7 @@ gdbscm_register_breakpoint_x (SCM self)
> > >  	    create_breakpoint (get_current_arch (),
> > >  			       eloc.get (), NULL, -1, NULL, false,
> > >  			       0,
> > > -			       0, bp_breakpoint,
> > > +			       temporary, bp_breakpoint,
> > >  			       0,
> > >  			       AUTO_BOOLEAN_TRUE,
> > >  			       ops,
> > > @@ -1029,6 +1039,18 @@ gdbscm_breakpoint_number (SCM self)
> > >  
> > >    return scm_from_long (bp_smob->number);
> > >  }
> > > +
> > > +/* (breakpoint-temporary? <gdb:breakpoint>) -> boolean */
> > > +
> > > +static SCM
> > > +gdbscm_breakpoint_temporary (SCM self)
> > > +{
> > > +  breakpoint_smob *bp_smob
> > > +    = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> > > +
> > > +  return scm_from_bool (bp_smob->bp->disposition == disp_del ||
> > > +			bp_smob->bp->disposition == disp_del_at_next_stop);
> > > +}
> > >  \f
> > >  /* Return TRUE if "stop" has been set for this breakpoint.
> > >  
> > > @@ -1159,9 +1181,9 @@ static const scheme_function breakpoint_functions[] =
> > >  Create a GDB breakpoint object.\n\
> > >  \n\
> > >    Arguments:\n\
> > > -    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>]\n\
> > > +    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>] [#:temporary <bool>]\n\
> > >    Returns:\n\
> > > -    <gdb:breakpoint object" },
> > > +    <gdb:breakpoint> object" },
> > >  
> > >    { "register-breakpoint!", 1, 0, 0,
> > >      as_a_scm_t_subr (gdbscm_register_breakpoint_x),
> > > @@ -1190,6 +1212,10 @@ Return #t if the breakpoint has not been deleted from GDB." },
> > >      "\
> > >  Return the breakpoint's number." },
> > >  
> > > +  { "breakpoint-temporary?", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_temporary),
> > > +    "\
> > > +Return #t if the breakpoint is a temporary breakpoint." },
> > > +
> > >    { "breakpoint-type", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_type),
> > >      "\
> > >  Return the type of the breakpoint." },
> > > @@ -1331,4 +1357,5 @@ gdbscm_initialize_breakpoints (void)
> > >    type_keyword = scm_from_latin1_keyword ("type");
> > >    wp_class_keyword = scm_from_latin1_keyword ("wp-class");
> > >    internal_keyword = scm_from_latin1_keyword ("internal");
> > > +  temporary_keyword = scm_from_latin1_keyword ("temporary");
> > >  }
> > > diff --git a/gdb/testsuite/gdb.guile/scm-breakpoint.exp b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> > > index 071a6f66f7e..1fa4711ea23 100644
> > > --- a/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> > > +++ b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> > > @@ -487,6 +487,40 @@ proc test_bkpt_registration {} {
> > >      }
> > >  }
> > >  
> > > +proc_with_prefix test_bkpt_temporary { } {
> > > +    global srcfile testfile hex decimal
> > > +
> > > +    with_test_prefix test_bkpt_temporary {
> > > +	# Start with a fresh gdb.
> > > +	clean_restart ${testfile}
> > > +
> > > +	if ![gdb_guile_runto_main] then {
> > > +	    fail "cannot run to main."
> > > +	    return 0
> > > +	}
> > > +	delete_breakpoints
> > > +
> > > +	set ibp_location [gdb_get_line_number "Break at multiply."]
> > > +	gdb_scm_test_silent_cmd "guile (define ibp (make-breakpoint \"$ibp_location\" #:temporary #t))" \
> > > +	    "create temporary breakpoint"
> > > +	gdb_scm_test_silent_cmd "guile (register-breakpoint! ibp)" \
> > > +	    "register ibp"
> > > +	gdb_test "info breakpoints" \
> > > +	    "2.*breakpoint.*del.*scm-breakpoint\.c:$ibp_location.*" \
> > > +	    "check info breakpoints shows breakpoint with temporary status"
> > > +	gdb_test "guile (print (breakpoint-location ibp))" "scm-breakpoint\.c:$ibp_location*" \
> > > +	    "check temporary breakpoint location"
> > > +	gdb_test "guile (print (breakpoint-temporary? ibp))" "#t" \
> > > +	    "check breakpoint temporary status"
> > > +	gdb_continue_to_breakpoint "Break at multiply." \
> > > +	    ".*$srcfile:$ibp_location.*"
> > > +	gdb_test "guile (print (breakpoint-temporary? ibp))" "Invalid object: <gdb:breakpoint>.*" \
> > > +	    "check temporary breakpoint is deleted after being hit"
> > > +	gdb_test "info breakpoints" "No breakpoints or watchpoints.*" \
> > > +	    "check info breakpoints shows temporary breakpoint is deleted"
> > > +    }
> > > +}
> > > +
> > >  proc test_bkpt_address {} {
> > >      global decimal srcfile
> > >  
> > > @@ -529,5 +563,6 @@ test_watchpoints
> > >  test_bkpt_internal
> > >  test_bkpt_eval_funcs
> > >  test_bkpt_registration
> > > +test_bkpt_temporary
> > >  test_bkpt_address
> > >  test_bkpt_probe
> > > -- 
> > > 2.30.2

[PING**4] [PATCH] Guile: temporary breakpoints

[George Barrett]

On Fri, May 28, 2021 at 02:36:16AM +1000, George Barrett wrote:
> On Wed, May 19, 2021 at 03:15:28PM +1000, George Barrett wrote:
> > Ping
> > 
> > On Wed, May 12, 2021 at 04:23:04AM +1000, George Barrett wrote:
> > > Would someone mind reviewing this?
> > > 
> > > Thanks.
> > > 
> > > On Mon, Apr 26, 2021 at 04:56:56AM +1000, George Barrett wrote:
> > > > Adds API to the Guile bindings for creating temporary breakpoints and
> > > > querying whether an existing breakpoint object is temporary. This is
> > > > effectively a transliteration of the Python implementation.
> > > > 
> > > > It's worth noting that the added `is_temporary' flag is ignored in the
> > > > watchpoint registration path. This replicates the behaviour of the
> > > > Python implementation, but might be a bit surprising for users.
> > > > 
> > > > gdb/ChangeLog:
> > > > 
> > > > 2021-04-26  George Barrett  <bob@bob131.so>
> > > > 
> > > > 	* guile/scm-breakpoint.c (gdbscm_breakpoint_object::spec): Add
> > > > 	is_temporary field.
> > > > 	(temporary_keyword): Add keyword object for make-breakpoint
> > > > 	argument parsing.
> > > > 	(gdbscm_make_breakpoint): Accept #:temporary keyword argument
> > > > 	and store the value in the allocated object's
> > > > 	spec.is_temporary.
> > > > 	(gdbscm_register_breakpoint_x): Pass the breakpoint's
> > > > 	spec.is_temporary value to create_breakpoint.
> > > > 	(gdbscm_breakpoint_temporary): Add breakpoint-temporary?
> > > > 	procedure implementation.
> > > > 	(breakpoint_functions::make-breakpoint): Update documentation
> > > > 	string and fix a typo.
> > > > 	(breakpoint_functions::breakpoint-temporary?): Add
> > > > 	breakpoint-temporary? procedure.
> > > > 	(gdbscm_initialize_breakpoints): Initialise temporary_keyword
> > > > 	variable.
> > > > 
> > > > gdb/doc/ChangeLog:
> > > > 
> > > > 2021-04-26  George Barrett  <bob@bob131.so>
> > > > 
> > > > 	* guile.texi (Breakpoints In Guile): Update make-breakpoint
> > > > 	documentation to reflect new #:temporary argument.
> > > > 	Add documentation for new breakpoint-temporary? procedure.
> > > > 
> > > > gdb/testsuite/ChangeLog:
> > > > 
> > > > 2021-04-26  George Barrett  <bob@bob131.so>
> > > > 
> > > > 	* gdb.guile/scm-breakpoint.exp: Add additional tests for
> > > > 	temporary breakpoints.
> > > > ---
> > > >  gdb/doc/guile.texi                         | 16 +++++++-
> > > >  gdb/guile/scm-breakpoint.c                 | 43 ++++++++++++++++++----
> > > >  gdb/testsuite/gdb.guile/scm-breakpoint.exp | 35 ++++++++++++++++++
> > > >  3 files changed, 85 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
> > > > index 762a82a08c5..649dac559ad 100644
> > > > --- a/gdb/doc/guile.texi
> > > > +++ b/gdb/doc/guile.texi
> > > > @@ -2947,7 +2947,7 @@ The following breakpoint-related procedures are provided by the
> > > >  @code{(gdb)} module:
> > > >  
> > > >  @c TODO: line length
> > > > -@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]}
> > > > +@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]} @r{[}#:temporary temporary@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,
> > > > @@ -2973,6 +2973,11 @@ registered, nor will it be listed in the output from @code{info breakpoints}
> > > >  If an internal flag is not provided, the breakpoint is visible
> > > >  (non-internal).
> > > >  
> > > > +The optional @var{temporary} argument makes the breakpoint a temporary
> > > > +breakpoint.  Temporary breakpoints are deleted after they have been hit,
> > > > +after which the Guile breakpoint is no longer usable (although it may be
> > > > +re-registered with @code{register-breakpoint!}).
> > > > +
> > > >  When a watchpoint is created, @value{GDBN} will try to create a
> > > >  hardware assisted watchpoint.  If successful, the type of the watchpoint
> > > >  is changed from @code{BP_WATCHPOINT} to @code{BP_HARDWARE_WATCHPOINT}
> > > > @@ -3065,6 +3070,15 @@ Return the breakpoint's number --- the identifier used by
> > > >  the user to manipulate the breakpoint.
> > > >  @end deffn
> > > >  
> > > > +@deffn {Scheme Procedure} breakpoint-temporary? breakpoint
> > > > +Return @code{#t} if the breakpoint was created as a temporary
> > > > +breakpoint.  Temporary breakpoints are automatically deleted after that
> > > > +breakpoint has been hit.  Calling this function, and all other functions
> > > > +other than @code{breakpoint-valid?} and @code{register-breakpoint!},
> > > > +will result in an error after the breakpoint has been hit (as it has
> > > > +been automatically deleted).
> > > > +@end deffn
> > > > +
> > > >  @deffn {Scheme Procedure} breakpoint-type breakpoint
> > > >  Return the breakpoint's type --- the identifier used to
> > > >  determine the actual breakpoint type or use-case.
> > > > diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
> > > > index af63893461b..aa9b2ba0d33 100644
> > > > --- a/gdb/guile/scm-breakpoint.c
> > > > +++ b/gdb/guile/scm-breakpoint.c
> > > > @@ -69,6 +69,9 @@ typedef struct gdbscm_breakpoint_object
> > > >  
> > > >      /* Non-zero if the breakpoint is an "internal" breakpoint.  */
> > > >      int is_internal;
> > > > +
> > > > +    /* Non-zero if the breakpoint is temporary.  */
> > > > +    int is_temporary;
> > > >    } spec;
> > > >  
> > > >    /* The breakpoint number according to gdb.
> > > > @@ -103,6 +106,7 @@ static SCM pending_breakpoint_scm = SCM_BOOL_F;
> > > >  static SCM type_keyword;
> > > >  static SCM wp_class_keyword;
> > > >  static SCM internal_keyword;
> > > > +static SCM temporary_keyword;
> > > >  \f
> > > >  /* Administrivia for breakpoint smobs.  */
> > > >  
> > > > @@ -329,7 +333,7 @@ bpscm_get_valid_breakpoint_smob_arg_unsafe (SCM self, int arg_pos,
> > > >  /* Breakpoint methods.  */
> > > >  
> > > >  /* (make-breakpoint string [#:type integer] [#:wp-class integer]
> > > > -    [#:internal boolean) -> <gdb:breakpoint>
> > > > +    [#:internal boolean] [#:temporary boolean]) -> <gdb:breakpoint>
> > > >  
> > > >     The result is the <gdb:breakpoint> Scheme object.
> > > >     The breakpoint is not available to be used yet, however.
> > > > @@ -339,22 +343,26 @@ static SCM
> > > >  gdbscm_make_breakpoint (SCM location_scm, SCM rest)
> > > >  {
> > > >    const SCM keywords[] = {
> > > > -    type_keyword, wp_class_keyword, internal_keyword, SCM_BOOL_F
> > > > +    type_keyword, wp_class_keyword, internal_keyword,
> > > > +    temporary_keyword, SCM_BOOL_F
> > > >    };
> > > >    char *s;
> > > >    char *location;
> > > > -  int type_arg_pos = -1, access_type_arg_pos = -1, internal_arg_pos = -1;
> > > > +  int type_arg_pos = -1, access_type_arg_pos = -1,
> > > > +      internal_arg_pos = -1, temporary_arg_pos = -1;
> > > >    enum bptype type = bp_breakpoint;
> > > >    enum target_hw_bp_type access_type = hw_write;
> > > >    int internal = 0;
> > > > +  int temporary = 0;
> > > >    SCM result;
> > > >    breakpoint_smob *bp_smob;
> > > >  
> > > > -  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iit",
> > > > +  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iitt",
> > > >  			      location_scm, &location, rest,
> > > >  			      &type_arg_pos, &type,
> > > >  			      &access_type_arg_pos, &access_type,
> > > > -			      &internal_arg_pos, &internal);
> > > > +			      &internal_arg_pos, &internal,
> > > > +			      &temporary_arg_pos, &temporary);
> > > >  
> > > >    result = bpscm_make_breakpoint_smob ();
> > > >    bp_smob = (breakpoint_smob *) SCM_SMOB_DATA (result);
> > > > @@ -397,6 +405,7 @@ gdbscm_make_breakpoint (SCM location_scm, SCM rest)
> > > >    bp_smob->spec.type = type;
> > > >    bp_smob->spec.access_type = access_type;
> > > >    bp_smob->spec.is_internal = internal;
> > > > +  bp_smob->spec.is_temporary = temporary;
> > > >  
> > > >    return result;
> > > >  }
> > > > @@ -432,6 +441,7 @@ gdbscm_register_breakpoint_x (SCM self)
> > > >    try
> > > >      {
> > > >        int internal = bp_smob->spec.is_internal;
> > > > +      int temporary = bp_smob->spec.is_temporary;
> > > >  
> > > >        switch (bp_smob->spec.type)
> > > >  	{
> > > > @@ -442,7 +452,7 @@ gdbscm_register_breakpoint_x (SCM self)
> > > >  	    create_breakpoint (get_current_arch (),
> > > >  			       eloc.get (), NULL, -1, NULL, false,
> > > >  			       0,
> > > > -			       0, bp_breakpoint,
> > > > +			       temporary, bp_breakpoint,
> > > >  			       0,
> > > >  			       AUTO_BOOLEAN_TRUE,
> > > >  			       ops,
> > > > @@ -1029,6 +1039,18 @@ gdbscm_breakpoint_number (SCM self)
> > > >  
> > > >    return scm_from_long (bp_smob->number);
> > > >  }
> > > > +
> > > > +/* (breakpoint-temporary? <gdb:breakpoint>) -> boolean */
> > > > +
> > > > +static SCM
> > > > +gdbscm_breakpoint_temporary (SCM self)
> > > > +{
> > > > +  breakpoint_smob *bp_smob
> > > > +    = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> > > > +
> > > > +  return scm_from_bool (bp_smob->bp->disposition == disp_del ||
> > > > +			bp_smob->bp->disposition == disp_del_at_next_stop);
> > > > +}
> > > >  \f
> > > >  /* Return TRUE if "stop" has been set for this breakpoint.
> > > >  
> > > > @@ -1159,9 +1181,9 @@ static const scheme_function breakpoint_functions[] =
> > > >  Create a GDB breakpoint object.\n\
> > > >  \n\
> > > >    Arguments:\n\
> > > > -    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>]\n\
> > > > +    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>] [#:temporary <bool>]\n\
> > > >    Returns:\n\
> > > > -    <gdb:breakpoint object" },
> > > > +    <gdb:breakpoint> object" },
> > > >  
> > > >    { "register-breakpoint!", 1, 0, 0,
> > > >      as_a_scm_t_subr (gdbscm_register_breakpoint_x),
> > > > @@ -1190,6 +1212,10 @@ Return #t if the breakpoint has not been deleted from GDB." },
> > > >      "\
> > > >  Return the breakpoint's number." },
> > > >  
> > > > +  { "breakpoint-temporary?", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_temporary),
> > > > +    "\
> > > > +Return #t if the breakpoint is a temporary breakpoint." },
> > > > +
> > > >    { "breakpoint-type", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_type),
> > > >      "\
> > > >  Return the type of the breakpoint." },
> > > > @@ -1331,4 +1357,5 @@ gdbscm_initialize_breakpoints (void)
> > > >    type_keyword = scm_from_latin1_keyword ("type");
> > > >    wp_class_keyword = scm_from_latin1_keyword ("wp-class");
> > > >    internal_keyword = scm_from_latin1_keyword ("internal");
> > > > +  temporary_keyword = scm_from_latin1_keyword ("temporary");
> > > >  }
> > > > diff --git a/gdb/testsuite/gdb.guile/scm-breakpoint.exp b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> > > > index 071a6f66f7e..1fa4711ea23 100644
> > > > --- a/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> > > > +++ b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
> > > > @@ -487,6 +487,40 @@ proc test_bkpt_registration {} {
> > > >      }
> > > >  }
> > > >  
> > > > +proc_with_prefix test_bkpt_temporary { } {
> > > > +    global srcfile testfile hex decimal
> > > > +
> > > > +    with_test_prefix test_bkpt_temporary {
> > > > +	# Start with a fresh gdb.
> > > > +	clean_restart ${testfile}
> > > > +
> > > > +	if ![gdb_guile_runto_main] then {
> > > > +	    fail "cannot run to main."
> > > > +	    return 0
> > > > +	}
> > > > +	delete_breakpoints
> > > > +
> > > > +	set ibp_location [gdb_get_line_number "Break at multiply."]
> > > > +	gdb_scm_test_silent_cmd "guile (define ibp (make-breakpoint \"$ibp_location\" #:temporary #t))" \
> > > > +	    "create temporary breakpoint"
> > > > +	gdb_scm_test_silent_cmd "guile (register-breakpoint! ibp)" \
> > > > +	    "register ibp"
> > > > +	gdb_test "info breakpoints" \
> > > > +	    "2.*breakpoint.*del.*scm-breakpoint\.c:$ibp_location.*" \
> > > > +	    "check info breakpoints shows breakpoint with temporary status"
> > > > +	gdb_test "guile (print (breakpoint-location ibp))" "scm-breakpoint\.c:$ibp_location*" \
> > > > +	    "check temporary breakpoint location"
> > > > +	gdb_test "guile (print (breakpoint-temporary? ibp))" "#t" \
> > > > +	    "check breakpoint temporary status"
> > > > +	gdb_continue_to_breakpoint "Break at multiply." \
> > > > +	    ".*$srcfile:$ibp_location.*"
> > > > +	gdb_test "guile (print (breakpoint-temporary? ibp))" "Invalid object: <gdb:breakpoint>.*" \
> > > > +	    "check temporary breakpoint is deleted after being hit"
> > > > +	gdb_test "info breakpoints" "No breakpoints or watchpoints.*" \
> > > > +	    "check info breakpoints shows temporary breakpoint is deleted"
> > > > +    }
> > > > +}
> > > > +
> > > >  proc test_bkpt_address {} {
> > > >      global decimal srcfile
> > > >  
> > > > @@ -529,5 +563,6 @@ test_watchpoints
> > > >  test_bkpt_internal
> > > >  test_bkpt_eval_funcs
> > > >  test_bkpt_registration
> > > > +test_bkpt_temporary
> > > >  test_bkpt_address
> > > >  test_bkpt_probe
> > > > -- 
> > > > 2.30.2

Re: [PING**4] [PATCH] Guile: temporary breakpoints

[Keith Seitz]

Hi,

I apologize that no one has yet reviewed your patch, which certainly seems
like something we would want. Feature parity between Python and Guile can
only be good for users.

I've nearly no experience with Guile, but looking this over, this patch looks
pretty straightforward, but I do have some small suggestions/requests below.

Otherwise, I would like to recommend that a maintainer approve this.
[I am not someone that can grant permission to push to the repo.]

Thanks again for the patch *and* your patience.

Keith

On 6/8/21 12:57 PM, George Barrett via Gdb-patches wrote:
>>>>> diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi
>>>>> index 762a82a08c5..649dac559ad 100644
>>>>> --- a/gdb/doc/guile.texi
>>>>> +++ b/gdb/doc/guile.texi
>>>>> @@ -2947,7 +2947,7 @@ The following breakpoint-related procedures are provided by the
>>>>>  @code{(gdb)} module:
>>>>>  
>>>>>  @c TODO: line length
>>>>> -@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]}
>>>>> +@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]} @r{[}#:temporary temporary@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,
>>>>> @@ -2973,6 +2973,11 @@ registered, nor will it be listed in the output from @code{info breakpoints}
>>>>>  If an internal flag is not provided, the breakpoint is visible
>>>>>  (non-internal).
>>>>>  
>>>>> +The optional @var{temporary} argument makes the breakpoint a temporary
>>>>> +breakpoint.  Temporary breakpoints are deleted after they have been hit,
>>>>> +after which the Guile breakpoint is no longer usable (although it may be
>>>>> +re-registered with @code{register-breakpoint!}).
>>>>> +
>>>>>  When a watchpoint is created, @value{GDBN} will try to create a
>>>>>  hardware assisted watchpoint.  If successful, the type of the watchpoint
>>>>>  is changed from @code{BP_WATCHPOINT} to @code{BP_HARDWARE_WATCHPOINT}
>>>>> @@ -3065,6 +3070,15 @@ Return the breakpoint's number --- the identifier used by
>>>>>  the user to manipulate the breakpoint.
>>>>>  @end deffn
>>>>>  
>>>>> +@deffn {Scheme Procedure} breakpoint-temporary? breakpoint
>>>>> +Return @code{#t} if the breakpoint was created as a temporary
>>>>> +breakpoint.  Temporary breakpoints are automatically deleted after that
                                                                     ^^^^^^^^^

"after that" is awkward. Since you've kept consistency with using the plural
for this and the above, consider using "they" (and adjusting remainder of
sentence).

>>>>> +breakpoint has been hit.  Calling this function, and all other functions
>>>>> +other than @code{breakpoint-valid?} and @code{register-breakpoint!},
>>>>> +will result in an error after the breakpoint has been hit (as it has
>>>>> +been automatically deleted).

[pedantic] "as" != "because". Maybe "since"?

>>>>> +@end deffn
>>>>> +
>>>>>  @deffn {Scheme Procedure} breakpoint-type breakpoint
>>>>>  Return the breakpoint's type --- the identifier used to
>>>>>  determine the actual breakpoint type or use-case.
>>>>> diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
>>>>> index af63893461b..aa9b2ba0d33 100644
>>>>> --- a/gdb/guile/scm-breakpoint.c
>>>>> +++ b/gdb/guile/scm-breakpoint.c
>>>>> @@ -69,6 +69,9 @@ typedef struct gdbscm_breakpoint_object
>>>>>  
>>>>>      /* Non-zero if the breakpoint is an "internal" breakpoint.  */
>>>>>      int is_internal;
>>>>> +
>>>>> +    /* Non-zero if the breakpoint is temporary.  */
>>>>> +    int is_temporary;
>>>>>    } spec;
>>>>>  
>>>>>    /* The breakpoint number according to gdb.
>>>>> @@ -103,6 +106,7 @@ static SCM pending_breakpoint_scm = SCM_BOOL_F;
>>>>>  static SCM type_keyword;
>>>>>  static SCM wp_class_keyword;
>>>>>  static SCM internal_keyword;
>>>>> +static SCM temporary_keyword;
>>>>>  \f
>>>>>  /* Administrivia for breakpoint smobs.  */
>>>>>  
>>>>> @@ -329,7 +333,7 @@ bpscm_get_valid_breakpoint_smob_arg_unsafe (SCM self, int arg_pos,
>>>>>  /* Breakpoint methods.  */
>>>>>  
>>>>>  /* (make-breakpoint string [#:type integer] [#:wp-class integer]
>>>>> -    [#:internal boolean) -> <gdb:breakpoint>
>>>>> +    [#:internal boolean] [#:temporary boolean]) -> <gdb:breakpoint>
>>>>>  
>>>>>     The result is the <gdb:breakpoint> Scheme object.
>>>>>     The breakpoint is not available to be used yet, however.
>>>>> @@ -339,22 +343,26 @@ static SCM
>>>>>  gdbscm_make_breakpoint (SCM location_scm, SCM rest)
>>>>>  {
>>>>>    const SCM keywords[] = {
>>>>> -    type_keyword, wp_class_keyword, internal_keyword, SCM_BOOL_F
>>>>> +    type_keyword, wp_class_keyword, internal_keyword,
>>>>> +    temporary_keyword, SCM_BOOL_F
>>>>>    };
>>>>>    char *s;
>>>>>    char *location;
>>>>> -  int type_arg_pos = -1, access_type_arg_pos = -1, internal_arg_pos = -1;
>>>>> +  int type_arg_pos = -1, access_type_arg_pos = -1,
>>>>> +      internal_arg_pos = -1, temporary_arg_pos = -1;
>>>>>    enum bptype type = bp_breakpoint;
>>>>>    enum target_hw_bp_type access_type = hw_write;
>>>>>    int internal = 0;
>>>>> +  int temporary = 0;
>>>>>    SCM result;
>>>>>    breakpoint_smob *bp_smob;
>>>>>  
>>>>> -  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iit",
>>>>> +  gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iitt",
>>>>>  			      location_scm, &location, rest,
>>>>>  			      &type_arg_pos, &type,
>>>>>  			      &access_type_arg_pos, &access_type,
>>>>> -			      &internal_arg_pos, &internal);
>>>>> +			      &internal_arg_pos, &internal,
>>>>> +			      &temporary_arg_pos, &temporary);
>>>>>  
>>>>>    result = bpscm_make_breakpoint_smob ();
>>>>>    bp_smob = (breakpoint_smob *) SCM_SMOB_DATA (result);
>>>>> @@ -397,6 +405,7 @@ gdbscm_make_breakpoint (SCM location_scm, SCM rest)
>>>>>    bp_smob->spec.type = type;
>>>>>    bp_smob->spec.access_type = access_type;
>>>>>    bp_smob->spec.is_internal = internal;
>>>>> +  bp_smob->spec.is_temporary = temporary;
>>>>>  
>>>>>    return result;
>>>>>  }
>>>>> @@ -432,6 +441,7 @@ gdbscm_register_breakpoint_x (SCM self)
>>>>>    try
>>>>>      {
>>>>>        int internal = bp_smob->spec.is_internal;
>>>>> +      int temporary = bp_smob->spec.is_temporary;
>>>>>  
>>>>>        switch (bp_smob->spec.type)
>>>>>  	{
>>>>> @@ -442,7 +452,7 @@ gdbscm_register_breakpoint_x (SCM self)
>>>>>  	    create_breakpoint (get_current_arch (),
>>>>>  			       eloc.get (), NULL, -1, NULL, false,
>>>>>  			       0,
>>>>> -			       0, bp_breakpoint,
>>>>> +			       temporary, bp_breakpoint,
>>>>>  			       0,
>>>>>  			       AUTO_BOOLEAN_TRUE,
>>>>>  			       ops,
>>>>> @@ -1029,6 +1039,18 @@ gdbscm_breakpoint_number (SCM self)
>>>>>  
>>>>>    return scm_from_long (bp_smob->number);
>>>>>  }
>>>>> +
>>>>> +/* (breakpoint-temporary? <gdb:breakpoint>) -> boolean */
>>>>> +
>>>>> +static SCM
>>>>> +gdbscm_breakpoint_temporary (SCM self)
>>>>> +{
>>>>> +  breakpoint_smob *bp_smob
>>>>> +    = bpscm_get_valid_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>>>>> +
>>>>> +  return scm_from_bool (bp_smob->bp->disposition == disp_del ||
>>>>> +			bp_smob->bp->disposition == disp_del_at_next_stop);
>>>>> +}
>>>>>  \f
>>>>>  /* Return TRUE if "stop" has been set for this breakpoint.
>>>>>  
>>>>> @@ -1159,9 +1181,9 @@ static const scheme_function breakpoint_functions[] =
>>>>>  Create a GDB breakpoint object.\n\
>>>>>  \n\
>>>>>    Arguments:\n\
>>>>> -    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>]\n\
>>>>> +    location [#:type <type>] [#:wp-class <wp-class>] [#:internal <bool>] [#:temporary <bool>]\n\
>>>>>    Returns:\n\
>>>>> -    <gdb:breakpoint object" },
>>>>> +    <gdb:breakpoint> object" },
>>>>>  
>>>>>    { "register-breakpoint!", 1, 0, 0,
>>>>>      as_a_scm_t_subr (gdbscm_register_breakpoint_x),
>>>>> @@ -1190,6 +1212,10 @@ Return #t if the breakpoint has not been deleted from GDB." },
>>>>>      "\
>>>>>  Return the breakpoint's number." },
>>>>>  
>>>>> +  { "breakpoint-temporary?", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_temporary),
>>>>> +    "\
>>>>> +Return #t if the breakpoint is a temporary breakpoint." },
>>>>> +
>>>>>    { "breakpoint-type", 1, 0, 0, as_a_scm_t_subr (gdbscm_breakpoint_type),
>>>>>      "\
>>>>>  Return the type of the breakpoint." },
>>>>> @@ -1331,4 +1357,5 @@ gdbscm_initialize_breakpoints (void)
>>>>>    type_keyword = scm_from_latin1_keyword ("type");
>>>>>    wp_class_keyword = scm_from_latin1_keyword ("wp-class");
>>>>>    internal_keyword = scm_from_latin1_keyword ("internal");
>>>>> +  temporary_keyword = scm_from_latin1_keyword ("temporary");
>>>>>  }
>>>>> diff --git a/gdb/testsuite/gdb.guile/scm-breakpoint.exp b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
>>>>> index 071a6f66f7e..1fa4711ea23 100644
>>>>> --- a/gdb/testsuite/gdb.guile/scm-breakpoint.exp
>>>>> +++ b/gdb/testsuite/gdb.guile/scm-breakpoint.exp
>>>>> @@ -487,6 +487,40 @@ proc test_bkpt_registration {} {
>>>>>      }
>>>>>  }
>>>>>  
>>>>> +proc_with_prefix test_bkpt_temporary { } {
>>>>> +    global srcfile testfile hex decimal
>>>>> +
>>>>> +    with_test_prefix test_bkpt_temporary {
>>>>> +	# Start with a fresh gdb.
>>>>> +	clean_restart ${testfile}
>>>>> +
>>>>> +	if ![gdb_guile_runto_main] then {
>>>>> +	    fail "cannot run to main."

For consistency, please use "if {expr} {".

This result should probably be 'untested "cannot run to main"' instead
of simply a fail. [Taken straight from gdb.base/template.exp.]

>>>>> +	    return 0
>>>>> +	}
>>>>> +	delete_breakpoints
>>>>> +
>>>>> +	set ibp_location [gdb_get_line_number "Break at multiply."]
>>>>> +	gdb_scm_test_silent_cmd "guile (define ibp (make-breakpoint \"$ibp_location\" #:temporary #t))" \
>>>>> +	    "create temporary breakpoint"
>>>>> +	gdb_scm_test_silent_cmd "guile (register-breakpoint! ibp)" \
>>>>> +	    "register ibp"
>>>>> +	gdb_test "info breakpoints" \
>>>>> +	    "2.*breakpoint.*del.*scm-breakpoint\.c:$ibp_location.*" \
>>>>> +	    "check info breakpoints shows breakpoint with temporary status"
>>>>> +	gdb_test "guile (print (breakpoint-location ibp))" "scm-breakpoint\.c:$ibp_location*" \
>>>>> +	    "check temporary breakpoint location"
>>>>> +	gdb_test "guile (print (breakpoint-temporary? ibp))" "#t" \
>>>>> +	    "check breakpoint temporary status"
>>>>> +	gdb_continue_to_breakpoint "Break at multiply." \
>>>>> +	    ".*$srcfile:$ibp_location.*"
>>>>> +	gdb_test "guile (print (breakpoint-temporary? ibp))" "Invalid object: <gdb:breakpoint>.*" \
>>>>> +	    "check temporary breakpoint is deleted after being hit"
>>>>> +	gdb_test "info breakpoints" "No breakpoints or watchpoints.*" \
>>>>> +	    "check info breakpoints shows temporary breakpoint is deleted"
>>>>> +    }
>>>>> +}
>>>>> +
>>>>>  proc test_bkpt_address {} {
>>>>>      global decimal srcfile
>>>>>  
>>>>> @@ -529,5 +563,6 @@ test_watchpoints
>>>>>  test_bkpt_internal
>>>>>  test_bkpt_eval_funcs
>>>>>  test_bkpt_registration
>>>>> +test_bkpt_temporary
>>>>>  test_bkpt_address
>>>>>  test_bkpt_probe
>>>>> -- 
>>>>> 2.30.2
> 

Re: [PING**4] [PATCH] Guile: temporary breakpoints

[Simon Marchi]

On 2021-06-08 5:00 p.m., Keith Seitz via Gdb-patches wrote:
> Hi,
> 
> I apologize that no one has yet reviewed your patch, which certainly seems
> like something we would want. Feature parity between Python and Guile can
> only be good for users.
> 
> I've nearly no experience with Guile, but looking this over, this patch looks
> pretty straightforward, but I do have some small suggestions/requests below.
> 
> Otherwise, I would like to recommend that a maintainer approve this.
> [I am not someone that can grant permission to push to the repo.]
> 
> Thanks again for the patch *and* your patience.
> 
> Keith

Thanks for looking at it Keith, it helps a lot.

>>>>>> +proc_with_prefix test_bkpt_temporary { } {
>>>>>> +    global srcfile testfile hex decimal
>>>>>> +
>>>>>> +    with_test_prefix test_bkpt_temporary {
>>>>>> +	# Start with a fresh gdb.
>>>>>> +	clean_restart ${testfile}
>>>>>> +
>>>>>> +	if ![gdb_guile_runto_main] then {
>>>>>> +	    fail "cannot run to main."
> 
> For consistency, please use "if {expr} {".
> 
> This result should probably be 'untested "cannot run to main"' instead
> of simply a fail. [Taken straight from gdb.base/template.exp.]

Perhaps it was a mistake to use untested in gdb.base/template.exp?

There are more instances of using "fail":


    $ ag -A 2 runto_main | grep untested | wc -l
    198
    $ ag -A 2 runto_main | grep fail | wc -l
    555

... and it makes more sense to me to use "fail".  If I introduce a bug
that prevents running to main to work, untested would be easier to miss
than fail.

Otherwise, I don't have the original context to quote but where it says:

  return scm_from_bool (bp_smob->bp->disposition == disp_del ||
			bp_smob->bp->disposition == disp_del_at_next_stop);

put the || operator on the next line:

  return scm_from_bool (bp_smob->bp->disposition == disp_del
			|| bp_smob->bp->disposition == disp_del_at_next_stop);


Other than that, the patch LGTM after taking Keith's feedback in
consideration.

Simon

Re: [PING**4] [PATCH] Guile: temporary breakpoints

[Simon Marchi]

On 2021-06-08 10:21 p.m., Simon Marchi via Gdb-patches wrote:
> On 2021-06-08 5:00 p.m., Keith Seitz via Gdb-patches wrote:
>> Hi,
>>
>> I apologize that no one has yet reviewed your patch, which certainly seems
>> like something we would want. Feature parity between Python and Guile can
>> only be good for users.
>>
>> I've nearly no experience with Guile, but looking this over, this patch looks
>> pretty straightforward, but I do have some small suggestions/requests below.
>>
>> Otherwise, I would like to recommend that a maintainer approve this.
>> [I am not someone that can grant permission to push to the repo.]
>>
>> Thanks again for the patch *and* your patience.
>>
>> Keith
> 
> Thanks for looking at it Keith, it helps a lot.
> 
>>>>>>> +proc_with_prefix test_bkpt_temporary { } {
>>>>>>> +    global srcfile testfile hex decimal
>>>>>>> +
>>>>>>> +    with_test_prefix test_bkpt_temporary {
>>>>>>> +	# Start with a fresh gdb.
>>>>>>> +	clean_restart ${testfile}
>>>>>>> +
>>>>>>> +	if ![gdb_guile_runto_main] then {
>>>>>>> +	    fail "cannot run to main."
>>
>> For consistency, please use "if {expr} {".
>>
>> This result should probably be 'untested "cannot run to main"' instead
>> of simply a fail. [Taken straight from gdb.base/template.exp.]
> 
> Perhaps it was a mistake to use untested in gdb.base/template.exp?
> 
> There are more instances of using "fail":
> 
> 
>     $ ag -A 2 runto_main | grep untested | wc -l
>     198
>     $ ag -A 2 runto_main | grep fail | wc -l
>     555
> 
> ... and it makes more sense to me to use "fail".  If I introduce a bug
> that prevents running to main to work, untested would be easier to miss
> than fail.
> 
> Otherwise, I don't have the original context to quote but where it says:
> 
>   return scm_from_bool (bp_smob->bp->disposition == disp_del ||
> 			bp_smob->bp->disposition == disp_del_at_next_stop);
> 
> put the || operator on the next line:
> 
>   return scm_from_bool (bp_smob->bp->disposition == disp_del
> 			|| bp_smob->bp->disposition == disp_del_at_next_stop);
> 
> 
> Other than that, the patch LGTM after taking Keith's feedback in
> consideration.
> 
> Simon
> 

Oh also, we probably need a short NEWS entry for this.

Simon

Re: [PING**4] [PATCH] Guile: temporary breakpoints

[George Barrett]

Ack to all. I also just noticed that I used 'function' instead of 'procedure'
in the Guile documentation, so I've changed that too. v2 coming shortly.

Thanks