From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2159 invoked by alias); 24 Mar 2010 21:08:45 -0000 Received: (qmail 2142 invoked by uid 22791); 24 Mar 2010 21:08:42 -0000 X-SWARE-Spam-Status: No, hits=-7.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 24 Mar 2010 21:08:32 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o2OL8VGn007567 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 24 Mar 2010 17:08:31 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o2OL8Ui4015121; Wed, 24 Mar 2010 17:08:31 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id o2OL8TC0006697; Wed, 24 Mar 2010 17:08:30 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id 63C8837822D; Wed, 24 Mar 2010 15:08:28 -0600 (MDT) From: Tom Tromey To: gdb-patches@sourceware.org Subject: Re: [1/2] RFC: reference count breakpoint commands References: Reply-To: Tom Tromey Date: Wed, 24 Mar 2010 21:08:00 -0000 In-Reply-To: (Tom Tromey's message of "Tue, 09 Mar 2010 20:49:05 -0700") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-03/txt/msg00823.txt.bz2 >>>>> "Tom" == Tom Tromey writes: Tom> This adds reference counting for breakpoint commands. This removes a Tom> longstanding restriction: you can now change a breakpoint's commands Tom> while the commands are running. This patch also sets the stage for the Tom> next page. I am checking this in. I rebased this patch on top of Volodya's recent commits. The updated patch is appended. Tom 2010-03-24 Tom Tromey * breakpoint.h (struct counted_command_line): New struct. (struct breakpoint) : Change type. (struct bpstats) : Change type. : New field. * breakpoint.c (alloc_counted_command_line): New function. (incref_counted_command_line): Likewise. (decref_counted_command_line): Likewise. (do_cleanup_counted_command_line): Likewise. (make_cleanup_decref_counted_command_line): Likewise. (breakpoint_set_commands): Use decref_counted_command_line and alloc_counted_command_line. (commands_command): Don't error if breakpoint commands are executing. (commands_from_control_command): Likewise. (bpstat_free): Update. (bpstat_copy): Likewise. (bpstat_clear_actions): Likewise. (bpstat_do_actions_1): Likewise. (bpstat_stop_status): Likewise. (print_one_breakpoint_location): Likewise. (delete_breakpoint): Likewise. (bpstat_alloc): Initialize new field. (tracepoint_save_command): Update. * tracepoint.c (encode_actions): Update. (trace_dump_command): Update. Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.465 diff -u -r1.465 breakpoint.c --- breakpoint.c 23 Mar 2010 21:36:05 -0000 1.465 +++ breakpoint.c 24 Mar 2010 21:06:51 -0000 @@ -433,6 +433,63 @@ return (b->type == bp_tracepoint || b->type == bp_fast_tracepoint); } +/* Allocate a new counted_command_line with reference count of 1. + The new structure owns COMMANDS. */ + +static struct counted_command_line * +alloc_counted_command_line (struct command_line *commands) +{ + struct counted_command_line *result + = xmalloc (sizeof (struct counted_command_line)); + result->refc = 1; + result->commands = commands; + return result; +} + +/* Increment reference count. This does nothing if CMD is NULL. */ + +static void +incref_counted_command_line (struct counted_command_line *cmd) +{ + if (cmd) + ++cmd->refc; +} + +/* Decrement reference count. If the reference count reaches 0, + destroy the counted_command_line. Sets *CMDP to NULL. This does + nothing if *CMDP is NULL. */ + +static void +decref_counted_command_line (struct counted_command_line **cmdp) +{ + if (*cmdp) + { + if (--(*cmdp)->refc == 0) + { + free_command_lines (&(*cmdp)->commands); + xfree (*cmdp); + } + *cmdp = NULL; + } +} + +/* A cleanup function that calls decref_counted_command_line. */ + +static void +do_cleanup_counted_command_line (void *arg) +{ + decref_counted_command_line (arg); +} + +/* Create a cleanup that calls decref_counted_command_line on the + argument. */ + +static struct cleanup * +make_cleanup_decref_counted_command_line (struct counted_command_line **cmdp) +{ + return make_cleanup (do_cleanup_counted_command_line, cmdp); +} + /* Default address, symtab and line to put a breakpoint at for "break" command with no arg. if default_breakpoint_valid is zero, the other three are @@ -783,8 +840,8 @@ check_no_tracepoint_commands (commands); } - free_command_lines (&b->commands); - b->commands = commands; + decref_counted_command_line (&b->commands); + b->commands = alloc_counted_command_line (commands); breakpoints_changed (); observer_notify_breakpoint_modified (b->number); } @@ -803,13 +860,6 @@ int bnum; struct command_line *l; - /* If we allowed this, we would have problems with when to - free the storage, if we change the commands currently - being read from. */ - - if (executing_breakpoint_commands) - error (_("Can't use the \"commands\" command among a breakpoint's commands.")); - p = arg; bnum = get_number (&p); @@ -847,13 +897,6 @@ char *p; int bnum; - /* If we allowed this, we would have problems with when to - free the storage, if we change the commands currently - being read from. */ - - if (executing_breakpoint_commands) - error (_("Can't use the \"commands\" command among a breakpoint's commands.")); - /* An empty string for the breakpoint number means the last breakpoint, but get_number expects a NULL pointer. */ if (arg && !*arg) @@ -868,12 +911,13 @@ ALL_BREAKPOINTS (b) if (b->number == bnum) { - free_command_lines (&b->commands); + decref_counted_command_line (&b->commands); if (cmd->body_count != 1) error (_("Invalid \"commands\" block structure.")); /* We need to copy the commands because if/while will free the list after it finishes execution. */ - b->commands = copy_command_lines (cmd->body_list[0]); + b->commands + = alloc_counted_command_line (copy_command_lines (cmd->body_list[0])); breakpoints_changed (); observer_notify_breakpoint_modified (b->number); return simple_control; @@ -2697,7 +2741,7 @@ { if (bs->old_val != NULL) value_free (bs->old_val); - free_command_lines (&bs->commands); + decref_counted_command_line (&bs->commands); xfree (bs); } @@ -2739,8 +2783,7 @@ { tmp = (bpstat) xmalloc (sizeof (*tmp)); memcpy (tmp, bs, sizeof (*tmp)); - if (bs->commands != NULL) - tmp->commands = copy_command_lines (bs->commands); + incref_counted_command_line (tmp->commands); if (bs->old_val != NULL) { tmp->old_val = value_copy (bs->old_val); @@ -2841,7 +2884,7 @@ { for (; bs != NULL; bs = bs->next) { - free_command_lines (&bs->commands); + decref_counted_command_line (&bs->commands); if (bs->old_val != NULL) { value_free (bs->old_val); @@ -2907,6 +2950,7 @@ breakpoint_proceeded = 0; for (; bs != NULL; bs = bs->next) { + struct counted_command_line *ccmd; struct command_line *cmd; struct cleanup *this_cmd_tree_chain; @@ -2920,9 +2964,12 @@ commands are only executed once, we don't need to copy it; we can clear the pointer in the bpstat, and make sure we free the tree when we're done. */ - cmd = bs->commands; - bs->commands = 0; - this_cmd_tree_chain = make_cleanup_free_command_lines (&cmd); + ccmd = bs->commands; + bs->commands = NULL; + this_cmd_tree_chain + = make_cleanup_decref_counted_command_line (&ccmd); + cmd = bs->commands_left; + bs->commands_left = NULL; while (cmd != NULL) { @@ -3305,6 +3352,7 @@ bs->breakpoint_at = bl; /* If the condition is false, etc., don't do the commands. */ bs->commands = NULL; + bs->commands_left = NULL; bs->old_val = NULL; bs->print_it = print_it_normal; return bs; @@ -3942,15 +3990,17 @@ if (b->silent) bs->print = 0; bs->commands = b->commands; - if (bs->commands - && (strcmp ("silent", bs->commands->line) == 0 - || (xdb_commands && strcmp ("Q", - bs->commands->line) == 0))) + incref_counted_command_line (bs->commands); + bs->commands_left = bs->commands ? bs->commands->commands : NULL; + if (bs->commands_left + && (strcmp ("silent", bs->commands_left->line) == 0 + || (xdb_commands + && strcmp ("Q", + bs->commands_left->line) == 0))) { - bs->commands = bs->commands->next; + bs->commands_left = bs->commands_left->next; bs->print = 0; } - bs->commands = copy_command_lines (bs->commands); } /* Print nothing for this entry if we dont stop or dont print. */ @@ -4617,7 +4667,7 @@ ui_out_text (uiout, " hits\n"); } - l = b->commands; + l = b->commands ? b->commands->commands : NULL; if (!part_of_multiple && l) { struct cleanup *script_chain; @@ -9021,7 +9071,7 @@ break; } - free_command_lines (&bpt->commands); + decref_counted_command_line (&bpt->commands); xfree (bpt->cond_string); xfree (bpt->cond_exp); xfree (bpt->addr_string); @@ -10446,7 +10496,7 @@ ui_out_redirect (uiout, fp); TRY_CATCH (ex, RETURN_MASK_ERROR) { - print_command_lines (uiout, tp->commands, 2); + print_command_lines (uiout, tp->commands->commands, 2); } ui_out_redirect (uiout, NULL); Index: breakpoint.h =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.h,v retrieving revision 1.110 diff -u -r1.110 breakpoint.h --- breakpoint.h 23 Mar 2010 21:32:26 -0000 1.110 +++ breakpoint.h 24 Mar 2010 21:06:51 -0000 @@ -376,6 +376,17 @@ typedef struct bp_location *bp_location_p; DEF_VEC_P(bp_location_p); +/* A reference-counted struct command_line. This lets multiple + breakpoints share a single command list. */ +struct counted_command_line +{ + /* The reference count. */ + int refc; + + /* The command list. */ + struct command_line *commands; +}; + /* Note that the ->silent field is not currently used by any commands (though the code is in there if it was to be, and set_raw_breakpoint does set it to 0). I implemented it because I thought it would be @@ -414,7 +425,7 @@ be continued automatically before really stopping. */ int ignore_count; /* Chain of command lines to execute when this breakpoint is hit. */ - struct command_line *commands; + struct counted_command_line *commands; /* Stack depth (address of frame). If nonzero, break only if fp equals this. */ struct frame_id frame_id; @@ -698,8 +709,11 @@ bpstat next; /* Breakpoint that we are at. */ const struct bp_location *breakpoint_at; - /* Commands left to be done. */ - struct command_line *commands; + /* The associated command list. */ + struct counted_command_line *commands; + /* Commands left to be done. This points somewhere in + base_command. */ + struct command_line *commands_left; /* Old value associated with a watchpoint. */ struct value *old_val; Index: tracepoint.c =================================================================== RCS file: /cvs/src/src/gdb/tracepoint.c,v retrieving revision 1.154 diff -u -r1.154 tracepoint.c --- tracepoint.c 24 Mar 2010 19:37:06 -0000 1.154 +++ tracepoint.c 24 Mar 2010 21:06:55 -0000 @@ -1385,7 +1385,7 @@ gdbarch_virtual_frame_pointer (t->gdbarch, t->loc->address, &frame_reg, &frame_offset); - actions = t->commands; + actions = t->commands->commands; /* If there are default expressions to collect, make up a collect action and prepend to the action list to encode. Note that since @@ -1406,7 +1406,7 @@ { default_collect_action = xmalloc (sizeof (struct command_line)); make_cleanup (xfree, default_collect_action); - default_collect_action->next = t->commands; + default_collect_action->next = t->commands->commands; default_collect_action->line = line; actions = default_collect_action; } @@ -2332,7 +2332,7 @@ if (loc->address == regcache_read_pc (regcache)) stepping_frame = 0; - for (action = t->commands; action; action = action->next) + for (action = t->commands->commands; action; action = action->next) { struct cmd_list_element *cmd;