From patchwork Wed Jun 4 04:53:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 1280 Received: (qmail 10392 invoked by alias); 4 Jun 2014 04:54:12 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 10379 invoked by uid 89); 4 Jun 2014 04:54:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pb0-f49.google.com Received: from mail-pb0-f49.google.com (HELO mail-pb0-f49.google.com) (209.85.160.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 04 Jun 2014 04:54:09 +0000 Received: by mail-pb0-f49.google.com with SMTP id jt11so6432753pbb.36 for ; Tue, 03 Jun 2014 21:54:07 -0700 (PDT) X-Received: by 10.68.189.137 with SMTP id gi9mr59059957pbc.79.1401857647218; Tue, 03 Jun 2014 21:54:07 -0700 (PDT) Received: from seba.sebabeach.org.gmail.com (173-13-178-50-sfba.hfc.comcastbusiness.net. [173.13.178.50]) by mx.google.com with ESMTPSA id it4sm4680932pbc.39.2014.06.03.21.54.06 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 03 Jun 2014 21:54:06 -0700 (PDT) From: Doug Evans To: ludo@gnu.org, eliz@gnu.org, gdb-patches@sourceware.org Subject: [PATCH, doc RFA] Split create-breakpoint! into make-breakpoint, register-breakpoint! Date: Tue, 03 Jun 2014 21:53:48 -0700 Message-ID: MIME-Version: 1.0 X-IsSubscribed: yes Hi. In keeping with the splitting of object creation and registration into two separate pieces, this does the same for breakpoints. "Consistency Is Good". There hasn't been an FSF release of gdb+guile yet so I think it's ok to remove create-breakpoint! here. 2014-06-03 Doug Evans * guile/scm-breakpoint.c (struct gdbscm_breakpoint_object): New members is_scheme_bkpt, spec. (bpscm_make_breakpoint_smob): Initialize new members. (gdbscm_create_breakpoint_x): Split into two ... (gdbscm_make_breakpoint, gdbscm_register_breakpoint_x): New functions. (bpscm_breakpoint_deleted): Reset breakpoint number and stop function. (scheme_function breakpoint_functions): Update. testsuite/ * gdb.guile/scm-breakpoint.exp: Update. Add tests for breakpoint registration. doc/ * guile.texi (Breakpoints In Guile): Update. diff --git a/gdb/doc/guile.texi b/gdb/doc/guile.texi index 3f8c4e4..4675cb2 100644 --- a/gdb/doc/guile.texi +++ b/gdb/doc/guile.texi @@ -2899,18 +2899,30 @@ object will be @code{#f} and 0 respectively. @tindex Breakpoints in Guile are represented by objects of type -@code{}. +@code{}. New breakpoints can be created with the +@code{make-breakpoint} Guile function, and then added to @value{GDBN} with the +@code{register-breakpoint!} Guile function. +This two-step approach is taken to separate out the side-effect of adding +the breakpoint to @value{GDBN} from @code{make-breakpoint}. + +Support is also provided to view and manipulate breakpoints created +outside of Guile. The following breakpoint-related procedures are provided by the @code{(gdb)} module: @c TODO: line length -@deffn {Scheme Procedure} create-breakpoint! location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]} -Create a new breakpoint according to @var{spec}, a string naming the +@deffn {Scheme Procedure} make-breakpoint location @r{[}#:type type@r{]} @r{[}#:wp-class wp-class@r{]} @r{[}#:internal internal@r{]} +Create a new breakpoint at @var{location}, a string naming the location of the breakpoint, or an expression that defines a watchpoint. The contents can be any location recognized by the @code{break} command, or in the case of a watchpoint, by the @code{watch} command. +The breakpoint is initially marked as @samp{invalid}. +The breakpoint is not usable until it has been registered with @value{GDBN} +with @code{register-breakpoint!}, at which point it becomes @samp{valid}. +The result is the @code{} object representing the breakpoint. + The optional @var{type} denotes the breakpoint to create. This argument can be either @code{BP_BREAKPOINT} or @code{BP_WATCHPOINT}, and defaults to @code{BP_BREAKPOINT}. @@ -2921,7 +2933,7 @@ not provided, it is assumed to be a @code{WP_WRITE} class. The optional @var{internal} argument allows the breakpoint to become invisible to the user. The breakpoint will neither be reported when -created, nor will it be listed in the output from @code{info breakpoints} +registered, nor will it be listed in the output from @code{info breakpoints} (but will be listed with the @code{maint info breakpoints} command). If an internal flag is not provided, the breakpoint is visible (non-internal). @@ -2972,10 +2984,24 @@ Read/Write watchpoint. @end deffn +@deffn {Scheme Procedure} register-breakpoint! breakpoint +Add @var{breakpoint}, a @code{} object, to @value{GDBN}'s +list of breakpoints. The breakpoint must have been created with +@code{make-breakpoint}. One cannot register breakpoints that have been +created outside of Guile. Once a breakpoint is registered it becomes +@samp{valid}. +It is an error to register an already registered breakpoint. +The result is unspecified. +@end deffn + @deffn {Scheme Procedure} breakpoint-delete! breakpoint -Permanently delete @var{breakpoint}. This also invalidates the -Guile @var{breakpoint} object. Any further attempt to access the -object will throw an exception. +Remove @var{breakpoint} from @value{GDBN}'s list of breakpoints. +This also invalidates the Guile @var{breakpoint} object. +Any further attempt to access the object will throw an exception. + +If @var{breakpoint} was created from Guile with @code{make-breakpoint} +it may be re-registered with @value{GDBN}, in which case the breakpoint +becomes valid again. @end deffn @deffn {Scheme Procedure} breakpoints @@ -2990,6 +3016,8 @@ and @code{#f} otherwise. @deffn {Scheme Procedure} breakpoint-valid? breakpoint Return @code{#t} if @var{breakpoint} is valid, @code{#f} otherwise. +Breakpoints created with @code{make-breakpoint} are marked as invalid +until they are registered with @value{GDBN} with @code{register-breakpoint!}. A @code{} object can become invalid if the user deletes the breakpoint. In this case, the object still exists, but the underlying breakpoint does not. In the cases of @@ -3129,7 +3157,8 @@ Example @code{stop} implementation: (define (my-stop? bkpt) (let ((int-val (parse-and-eval "foo"))) (value=? int-val 3))) -(define bkpt (create-breakpoint! "main.c:42")) +(define bkpt (make-breakpoint "main.c:42")) +(register-breakpoint! bkpt) (set-breakpoint-stop! bkpt my-stop?) @end smallexample @end deffn diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c index 7edb97d..05e46c0 100644 --- a/gdb/guile/scm-breakpoint.c +++ b/gdb/guile/scm-breakpoint.c @@ -34,18 +34,51 @@ #include "guile-internal.h" /* The smob. - N.B.: The name of this struct is known to breakpoint.h. */ + N.B.: The name of this struct is known to breakpoint.h. + + Note: Breakpoints are added to gdb using a two step process: + 1) Call make-breakpoint to create a object. + 2) Call register-breakpoint! to add the breakpoint to gdb. + It is done this way so that the constructor, make-breakpoint, doesn't have + any side-effects. This means that the smob needs to store everything + that was passed to make-breakpoint. */ typedef struct gdbscm_breakpoint_object { /* This always appears first. */ gdb_smob base; + /* Non-zero if this breakpoint was created with make-breakpoint. */ + int is_scheme_bkpt; + + /* For breakpoints created with make-breakpoint, these are the parameters + that were passed to make-breakpoint. These values are not used except + to register the breakpoint with GDB. */ + struct + { + /* The string representation of the breakpoint. + Space for this lives in GC space. */ + char *location; + + /* The kind of breakpoint. + At the moment this can only be one of bp_breakpoint, bp_watchpoint. */ + enum bptype type; + + /* If a watchpoint, the kind of watchpoint. */ + enum target_hw_bp_type access_type; + + /* Non-zero if the breakpoint is an "internal" breakpoint. */ + int is_internal; + } spec; + /* The breakpoint number according to gdb. + For breakpoints created from Scheme, this has the value -1 until the + breakpoint is registered with gdb. This is recorded here because BP will be NULL when deleted. */ int number; - /* The gdb breakpoint object, or NULL if the breakpoint has been deleted. */ + /* The gdb breakpoint object, or NULL if the breakpoint has not been + registered yet, or has been deleted. */ struct breakpoint *bp; /* Backlink to our containing smob. @@ -171,8 +204,8 @@ bpscm_make_breakpoint_smob (void) scm_gc_malloc (sizeof (breakpoint_smob), breakpoint_smob_name); SCM bp_scm; + memset (bp_smob, 0, sizeof (*bp_smob)); bp_smob->number = -1; - bp_smob->bp = NULL; bp_smob->stop = SCM_BOOL_F; bp_scm = scm_new_smob (breakpoint_smob_tag, (scm_t_bits) bp_smob); bp_smob->containing_scm = bp_scm; @@ -293,42 +326,111 @@ bpscm_get_valid_breakpoint_smob_arg_unsafe (SCM self, int arg_pos, /* Breakpoint methods. */ -/* (create-breakpoint! string [#:type integer] [#:wp-class integer] - [#:internal boolean) -> */ +/* (make-breakpoint string [#:type integer] [#:wp-class integer] + [#:internal boolean) -> + + The result is the Scheme object. + The breakpoint is not available to be used yet, however. + It must still be added to gdb with register-breakpoint!. */ static SCM -gdbscm_create_breakpoint_x (SCM spec_scm, SCM rest) +gdbscm_make_breakpoint (SCM location_scm, SCM rest) { const SCM keywords[] = { type_keyword, wp_class_keyword, internal_keyword, SCM_BOOL_F }; - char *spec; + char *s; + char *location; int type_arg_pos = -1, access_type_arg_pos = -1, internal_arg_pos = -1; int type = bp_breakpoint; int access_type = hw_write; int internal = 0; SCM result; - volatile struct gdb_exception except; + breakpoint_smob *bp_smob; gdbscm_parse_function_args (FUNC_NAME, SCM_ARG1, keywords, "s#iit", - spec_scm, &spec, rest, + location_scm, &location, rest, &type_arg_pos, &type, &access_type_arg_pos, &access_type, &internal_arg_pos, &internal); result = bpscm_make_breakpoint_smob (); - pending_breakpoint_scm = result; + bp_smob = (breakpoint_smob *) SCM_SMOB_DATA (result); + + s = location; + location = gdbscm_gc_xstrdup (s); + xfree (s); + + switch (type) + { + case bp_breakpoint: + if (access_type_arg_pos > 0) + { + gdbscm_misc_error (FUNC_NAME, access_type_arg_pos, + scm_from_int (access_type), + _("access type with breakpoint is not allowed")); + } + break; + case bp_watchpoint: + switch (access_type) + { + case hw_write: + case hw_access: + case hw_read: + break; + default: + gdbscm_out_of_range_error (FUNC_NAME, access_type_arg_pos, + scm_from_int (access_type), + _("invalid watchpoint class")); + } + break; + default: + gdbscm_out_of_range_error (FUNC_NAME, access_type_arg_pos, + scm_from_int (type), + _("invalid breakpoint type")); + } + + bp_smob->is_scheme_bkpt = 1; + bp_smob->spec.location = location; + bp_smob->spec.type = type; + bp_smob->spec.access_type = access_type; + bp_smob->spec.is_internal = internal; + + return result; +} + +/* (register-breakpoint! ) -> unspecified + + It is an error to register a breakpoint created outside of Guile, + or an already-registered breakpoint. */ + +static SCM +gdbscm_register_breakpoint_x (SCM self) +{ + breakpoint_smob *bp_smob + = bpscm_get_breakpoint_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME); + volatile struct gdb_exception except; + + /* We only support registering breakpoints created with make-breakpoint. */ + if (!bp_smob->is_scheme_bkpt) + scm_misc_error (FUNC_NAME, _("not a Scheme breakpoint"), SCM_EOL); + + if (bpscm_is_valid (bp_smob)) + scm_misc_error (FUNC_NAME, _("breakpoint is already registered"), SCM_EOL); + + pending_breakpoint_scm = self; TRY_CATCH (except, RETURN_MASK_ALL) { - struct cleanup *cleanup = make_cleanup (xfree, spec); + char *location = bp_smob->spec.location; + int internal = bp_smob->spec.is_internal; - switch (type) + switch (bp_smob->spec.type) { case bp_breakpoint: { create_breakpoint (get_current_arch (), - spec, NULL, -1, NULL, + location, NULL, -1, NULL, 0, 0, bp_breakpoint, 0, @@ -339,27 +441,27 @@ gdbscm_create_breakpoint_x (SCM spec_scm, SCM rest) } case bp_watchpoint: { + enum target_hw_bp_type access_type = bp_smob->spec.access_type; + if (access_type == hw_write) - watch_command_wrapper (spec, 0, internal); + watch_command_wrapper (location, 0, internal); else if (access_type == hw_access) - awatch_command_wrapper (spec, 0, internal); + awatch_command_wrapper (location, 0, internal); else if (access_type == hw_read) - rwatch_command_wrapper (spec, 0, internal); + rwatch_command_wrapper (location, 0, internal); else - error (_("Invalid watchpoint access type")); + gdb_assert_not_reached ("invalid access type"); break; } default: - error (_("Invalid breakpoint type")); + gdb_assert_not_reached ("invalid breakpoint type"); } - - do_cleanups (cleanup); } /* Ensure this gets reset, even if there's an error. */ pending_breakpoint_scm = SCM_BOOL_F; GDBSCM_HANDLE_GDB_EXCEPTION (except); - return result; + return SCM_UNSPECIFIED; } /* (breakpoint-delete! ) -> unspecified @@ -999,6 +1101,8 @@ bpscm_breakpoint_deleted (struct breakpoint *b) if (bp_smob) { bp_smob->bp = NULL; + bp_smob->number = -1; + bp_smob->stop = SCM_BOOL_F; scm_gc_unprotect_object (bp_smob->containing_scm); } } @@ -1024,12 +1128,18 @@ static const scheme_integer_constant breakpoint_integer_constants[] = static const scheme_function breakpoint_functions[] = { - { "create-breakpoint!", 1, 0, 1, gdbscm_create_breakpoint_x, + { "make-breakpoint", 1, 0, 1, gdbscm_make_breakpoint, "\ -Create and install a GDB breakpoint object.\n\ +Create a GDB breakpoint object.\n\ \n\ Arguments:\n\ - location [#:type ] [#:wp-class ] [#:internal ]" }, + location [#:type ] [#:wp-class ] [#:internal ]\n\ + Returns:\n\ + object with GDB." }, { "breakpoint-delete!", 1, 0, 0, gdbscm_breakpoint_delete_x, "\ diff --git a/gdb/testsuite/gdb.guile/scm-breakpoint.exp b/gdb/testsuite/gdb.guile/scm-breakpoint.exp index fd7c970..4701969 100644 --- a/gdb/testsuite/gdb.guile/scm-breakpoint.exp +++ b/gdb/testsuite/gdb.guile/scm-breakpoint.exp @@ -116,8 +116,10 @@ proc test_bkpt_deletion { } { # Test breakpoints are deleted correctly. set deltst_location [gdb_get_line_number "Break at multiply."] set end_location [gdb_get_line_number "Break at end."] - gdb_scm_test_silent_cmd "guile (define dp1 (create-breakpoint! \"$deltst_location\"))" \ + gdb_scm_test_silent_cmd "guile (define dp1 (make-breakpoint \"$deltst_location\"))" \ "create deltst breakpoint" + gdb_scm_test_silent_cmd "guile (register-breakpoint! dp1)" \ + "register dp1" gdb_breakpoint [gdb_get_line_number "Break at end."] gdb_scm_test_silent_cmd "guile (define del-list (breakpoints))" \ "get breakpoint list 4" @@ -128,7 +130,7 @@ proc test_bkpt_deletion { } { gdb_scm_test_silent_cmd "guile (breakpoint-delete! dp1)" \ "delete breakpoint" gdb_test "guile (print (breakpoint-number dp1))" \ - "ERROR: .*: Invalid object: in position 1: #.*" \ + "ERROR: .*: Invalid object: in position 1: #.*" \ "check breakpoint invalidated" gdb_scm_test_silent_cmd "guile (set! del-list (breakpoints))" \ "get breakpoint list 5" @@ -151,8 +153,10 @@ proc test_bkpt_cond_and_cmds { } { # Test conditional setting. set bp_location1 [gdb_get_line_number "Break at multiply."] - gdb_scm_test_silent_cmd "guile (define bp1 (create-breakpoint! \"$bp_location1\"))" \ + gdb_scm_test_silent_cmd "guile (define bp1 (make-breakpoint \"$bp_location1\"))" \ "create multiply breakpoint" + gdb_scm_test_silent_cmd "guile (register-breakpoint! bp1)" \ + "register bp1" gdb_continue_to_breakpoint "Break at multiply." gdb_scm_test_silent_cmd "guile (set-breakpoint-condition! bp1 \"i == 5\")" \ "set condition" @@ -199,8 +203,10 @@ proc test_bkpt_invisible { } { # Test invisible breakpoints. delete_breakpoints set ibp_location [gdb_get_line_number "Break at multiply."] - gdb_scm_test_silent_cmd "guile (define vbp (create-breakpoint! \"$ibp_location\" #:internal #f))" \ + gdb_scm_test_silent_cmd "guile (define vbp1 (make-breakpoint \"$ibp_location\" #:internal #f))" \ "create visible breakpoint" + gdb_scm_test_silent_cmd "guile (register-breakpoint! vbp1)" \ + "register vbp1" gdb_scm_test_silent_cmd "guile (define vbp (car (breakpoints)))" \ "get visible breakpoint" gdb_test "guile (print vbp)" \ @@ -214,8 +220,10 @@ proc test_bkpt_invisible { } { "scm-breakpoint\.c:$ibp_location.*" \ "check info breakpoints shows visible breakpoints" delete_breakpoints - gdb_scm_test_silent_cmd "guile (define ibp (create-breakpoint! \"$ibp_location\" #:internal #t))" \ + gdb_scm_test_silent_cmd "guile (define ibp (make-breakpoint \"$ibp_location\" #:internal #t))" \ "create invisible breakpoint" + gdb_scm_test_silent_cmd "guile (register-breakpoint! ibp)" \ + "register ibp" gdb_test "guile (print ibp)" \ "= #" \ "check invisible bp obj exists" @@ -247,8 +255,10 @@ proc test_watchpoints { } { return } - gdb_scm_test_silent_cmd "guile (define wp1 (create-breakpoint! \"result\" #:type BP_WATCHPOINT #:wp-class WP_WRITE))" \ + gdb_scm_test_silent_cmd "guile (define wp1 (make-breakpoint \"result\" #:type BP_WATCHPOINT #:wp-class WP_WRITE))" \ "create watchpoint" + gdb_scm_test_silent_cmd "guile (register-breakpoint! wp1)" \ + "register wp1" gdb_test "continue" \ ".*\[Ww\]atchpoint.*result.*Old value = 0.*New value = 25.*main.*" \ "test watchpoint write" @@ -272,8 +282,10 @@ proc test_bkpt_internal { } { delete_breakpoints - gdb_scm_test_silent_cmd "guile (define wp1 (create-breakpoint! \"result\" #:type BP_WATCHPOINT #:wp-class WP_WRITE #:internal #t))" \ + gdb_scm_test_silent_cmd "guile (define wp1 (make-breakpoint \"result\" #:type BP_WATCHPOINT #:wp-class WP_WRITE #:internal #t))" \ "create invisible watchpoint" + gdb_scm_test_silent_cmd "guile (register-breakpoint! wp1)" \ + "register wp1" gdb_test "info breakpoints" \ "No breakpoints or watchpoints.*" \ "check info breakpoints does not show invisible watchpoint" @@ -303,6 +315,11 @@ proc test_bkpt_eval_funcs { } { delete_breakpoints + # Define create-breakpoint! as a convenient wrapper around + # make-breakpoint, register-breakpoint! + gdb_test_no_output "guile (define (create-breakpoint! . args) (let ((bp (apply make-breakpoint args))) (register-breakpoint! bp) bp))" \ + "define create-breakpoint!" + gdb_test_multiline "data collection breakpoint 1" \ "guile" "" \ "(define (make-bp-data) (cons 0 0))" "" \ @@ -429,6 +446,47 @@ proc test_bkpt_eval_funcs { } { } } +proc test_bkpt_registration {} { + global srcfile testfile + + with_test_prefix "test_bkpt_registration" { + # Start with a fresh gdb. + clean_restart ${testfile} + + if ![gdb_guile_runto_main] { + return + } + + # Initially there should be one breakpoint: main. + gdb_scm_test_silent_cmd "guile (define blist (breakpoints))" \ + "get breakpoint list 1" + gdb_test "guile (register-breakpoint! (car blist))" \ + "ERROR: .*: not a Scheme breakpoint.*" \ + "try to register a non-guile breakpoint" + + set bp_location1 [gdb_get_line_number "Break at multiply."] + gdb_scm_test_silent_cmd "guile (define bp1 (make-breakpoint \"$bp_location1\"))" \ + "create multiply breakpoint" + gdb_test "guile (print (breakpoint-valid? bp1))" \ + "= #f" "breakpoint invalid after creation" + gdb_scm_test_silent_cmd "guile (register-breakpoint! bp1)" \ + "register bp1" + gdb_test "guile (print (breakpoint-valid? bp1))" \ + "= #t" "breakpoint valid after registration" + gdb_test "guile (register-breakpoint! bp1)" \ + "ERROR: .*: breakpoint is already registered.*" \ + "re-register already registered bp1" + gdb_scm_test_silent_cmd "guile (breakpoint-delete! bp1)" \ + "delete registered breakpoint" + gdb_test "guile (print (breakpoint-valid? bp1))" \ + "= #f" "breakpoint invalid after deletion" + gdb_scm_test_silent_cmd "guile (register-breakpoint! bp1)" \ + "re-register bp1" + gdb_test "guile (print (breakpoint-valid? bp1))" \ + "= #t" "breakpoint valid after re-registration" + } +} + test_bkpt_basic test_bkpt_deletion test_bkpt_cond_and_cmds @@ -436,3 +494,4 @@ test_bkpt_invisible test_watchpoints test_bkpt_internal test_bkpt_eval_funcs +test_bkpt_registration