* [PATCH] Guile: temporary breakpoints
@ 2021-04-25 18:56 George Barrett
2021-05-11 18:23 ` [PING] " George Barrett
0 siblings, 1 reply; 9+ messages in thread
From: George Barrett @ 2021-04-25 18:56 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PING] [PATCH] Guile: temporary breakpoints
2021-04-25 18:56 [PATCH] Guile: temporary breakpoints George Barrett
@ 2021-05-11 18:23 ` George Barrett
2021-05-19 5:15 ` [PING**2] " George Barrett
0 siblings, 1 reply; 9+ messages in thread
From: George Barrett @ 2021-05-11 18:23 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PING**2] [PATCH] Guile: temporary breakpoints
2021-05-11 18:23 ` [PING] " George Barrett
@ 2021-05-19 5:15 ` George Barrett
2021-05-27 16:36 ` [PING**3] " George Barrett
0 siblings, 1 reply; 9+ messages in thread
From: George Barrett @ 2021-05-19 5:15 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PING**3] [PATCH] Guile: temporary breakpoints
2021-05-19 5:15 ` [PING**2] " George Barrett
@ 2021-05-27 16:36 ` George Barrett
2021-06-08 19:57 ` [PING**4] " George Barrett
0 siblings, 1 reply; 9+ messages in thread
From: George Barrett @ 2021-05-27 16:36 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 9+ messages in thread* [PING**4] [PATCH] Guile: temporary breakpoints
2021-05-27 16:36 ` [PING**3] " George Barrett
@ 2021-06-08 19:57 ` George Barrett
2021-06-08 21:00 ` Keith Seitz
0 siblings, 1 reply; 9+ messages in thread
From: George Barrett @ 2021-06-08 19:57 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PING**4] [PATCH] Guile: temporary breakpoints
2021-06-08 19:57 ` [PING**4] " George Barrett
@ 2021-06-08 21:00 ` Keith Seitz
2021-06-09 2:21 ` Simon Marchi
0 siblings, 1 reply; 9+ messages in thread
From: Keith Seitz @ 2021-06-08 21:00 UTC (permalink / raw)
To: George Barrett, gdb-patches
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PING**4] [PATCH] Guile: temporary breakpoints
2021-06-08 21:00 ` Keith Seitz
@ 2021-06-09 2:21 ` Simon Marchi
2021-06-09 2:25 ` Simon Marchi
0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2021-06-09 2:21 UTC (permalink / raw)
To: Keith Seitz, George Barrett, gdb-patches
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PING**4] [PATCH] Guile: temporary breakpoints
2021-06-09 2:21 ` Simon Marchi
@ 2021-06-09 2:25 ` Simon Marchi
2021-06-09 2:42 ` George Barrett
0 siblings, 1 reply; 9+ messages in thread
From: Simon Marchi @ 2021-06-09 2:25 UTC (permalink / raw)
To: Keith Seitz, George Barrett, gdb-patches
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-06-09 2:43 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-25 18:56 [PATCH] Guile: temporary breakpoints George Barrett
2021-05-11 18:23 ` [PING] " George Barrett
2021-05-19 5:15 ` [PING**2] " George Barrett
2021-05-27 16:36 ` [PING**3] " George Barrett
2021-06-08 19:57 ` [PING**4] " George Barrett
2021-06-08 21:00 ` Keith Seitz
2021-06-09 2:21 ` Simon Marchi
2021-06-09 2:25 ` Simon Marchi
2021-06-09 2:42 ` George Barrett
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).