From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27497 invoked by alias); 27 Aug 2002 00:50:43 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 27458 invoked from network); 27 Aug 2002 00:50:42 -0000 Received: from unknown (HELO cygnus.com) (205.180.83.203) by sources.redhat.com with SMTP; 27 Aug 2002 00:50:42 -0000 Received: from redhat.com (reddwarf.sfbay.redhat.com [172.16.24.50]) by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id RAA09255; Mon, 26 Aug 2002 17:43:44 -0700 (PDT) Message-ID: <3D6ACCDD.CAECD28A@redhat.com> Date: Mon, 26 Aug 2002 17:54:00 -0000 From: Michael Snyder Organization: Red Hat, Inc. X-Accept-Language: en MIME-Version: 1.0 To: brobecker@gnat.com CC: gdb-patches@sources.redhat.com, dhoward@redhat.com Subject: [PATCH] GDB/622, GDB/644, bp deletion and cmds Content-Type: multipart/mixed; boundary="------------9800C3F4EACD05CEA9BE6F5F" X-SW-Source: 2002-08/txt/msg00880.txt.bz2 This is a multi-part message in MIME format. --------------9800C3F4EACD05CEA9BE6F5F Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-length: 661 OK, I think I have an elegant solution for this. I'm going to use Joel's copy_command_lines function, and Joel's commands.exp tests, but here's what we'll do in breakpoint.c: 1) bpstat_stop_status will make a copy of the bp commands and put it in the bpstat (instead of merely copying the pointer). 2) bpstat_clear, bpstat_clear_actions, and bpstat_do_actions will free the command lines instead of simply discarding them. Since all bpstat commands are copies instead of pointers, they won't be affected by the deletion of the breakpoint -- and we get to plug a memory leak as a bonus. Here's the patch as checked in (tested on linux, no regressions): --------------9800C3F4EACD05CEA9BE6F5F Content-Type: text/plain; charset=us-ascii; name="joel.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="joel.diff" Content-length: 10284 Index: defs.h =================================================================== RCS file: /cvs/src/src/gdb/defs.h,v retrieving revision 1.94 diff -p -r1.94 defs.h *** defs.h 1 Aug 2002 17:18:32 -0000 1.94 --- defs.h 27 Aug 2002 00:43:32 -0000 *************** struct command_line *** 653,658 **** --- 653,659 ---- extern struct command_line *read_command_lines (char *, int); extern void free_command_lines (struct command_line **); + extern struct command_line *copy_command_lines (struct command_line *); /* To continue the execution commands when running gdb asynchronously. A continuation structure contains a pointer to a function to be called Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.82 diff -p -r1.82 breakpoint.c *** breakpoint.c 23 Aug 2002 20:49:38 -0000 1.82 --- breakpoint.c 27 Aug 2002 00:43:32 -0000 *************** bpstat_clear (bpstat *bsp) *** 1763,1768 **** --- 1763,1769 ---- q = p->next; if (p->old_val != NULL) value_free (p->old_val); + free_command_lines (&p->commands); xfree (p); p = q; } *************** bpstat_clear_actions (bpstat bs) *** 1875,1881 **** { for (; bs != NULL; bs = bs->next) { ! bs->commands = NULL; if (bs->old_val != NULL) { value_free (bs->old_val); --- 1876,1882 ---- { for (; bs != NULL; bs = bs->next) { ! free_command_lines (&bs->commands); if (bs->old_val != NULL) { value_free (bs->old_val); *************** top: *** 1944,1954 **** to look at, so start over. */ goto top; else ! bs->commands = NULL; } ! ! executing_breakpoint_commands = 0; ! discard_cleanups (old_chain); } /* This is the normal print function for a bpstat. In the future, --- 1945,1953 ---- to look at, so start over. */ goto top; else ! free_command_lines (&bs->commands); } ! do_cleanups (old_chain); } /* This is the normal print function for a bpstat. In the future, *************** bpstat_stop_status (CORE_ADDR *pc, int n *** 2730,2736 **** /* We will stop here */ if (b->disposition == disp_disable) b->enable_state = bp_disabled; ! bs->commands = b->commands; if (b->silent) bs->print = 0; if (bs->commands && --- 2729,2735 ---- /* We will stop here */ if (b->disposition == disp_disable) b->enable_state = bp_disabled; ! bs->commands = copy_command_lines (b->commands); if (b->silent) bs->print = 0; if (bs->commands && *************** delete_breakpoint (struct breakpoint *bp *** 6787,6800 **** if (bs->breakpoint_at == bpt) { bs->breakpoint_at = NULL; - - /* we'd call bpstat_clear_actions, but that free's stuff and due - to the multiple pointers pointing to one item with no - reference counts found anywhere through out the bpstat's (how - do you spell fragile?), we don't want to free things twice -- - better a memory leak than a corrupt malloc pool! */ - bs->commands = NULL; bs->old_val = NULL; } /* On the chance that someone will soon try again to delete this same bp, we mark it as deleted before freeing its storage. */ --- 6786,6793 ---- if (bs->breakpoint_at == bpt) { bs->breakpoint_at = NULL; bs->old_val = NULL; + /* bs->commands will be freed later. */ } /* On the chance that someone will soon try again to delete this same bp, we mark it as deleted before freeing its storage. */ Index: cli/cli-script.c =================================================================== RCS file: /cvs/src/src/gdb/cli/cli-script.c,v retrieving revision 1.13 diff -p -r1.13 cli-script.c *** cli/cli-script.c 30 Jul 2002 13:45:14 -0000 1.13 --- cli/cli-script.c 27 Aug 2002 00:43:32 -0000 *************** make_cleanup_free_command_lines (struct *** 1012,1017 **** --- 1012,1047 ---- { return make_cleanup (do_free_command_lines_cleanup, arg); } + + struct command_line * + copy_command_lines (struct command_line *cmds) + { + struct command_line *result = NULL; + + if (cmds) + { + result = (struct command_line *) xmalloc (sizeof (struct command_line)); + + result->next = copy_command_lines (cmds->next); + result->line = xstrdup (cmds->line); + result->control_type = cmds->control_type; + result->body_count = cmds->body_count; + if (cmds->body_count > 0) + { + int i; + + result->body_list = (struct command_line **) + xmalloc (sizeof (struct command_line *) * cmds->body_count); + + for (i = 0; i < cmds->body_count; i++) + result->body_list[i] = copy_command_lines (cmds->body_list[i]); + } + else + result->body_list = NULL; + } + + return result; + } static void validate_comname (char *comname) Index: testsuite/gdb.base/commands.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.base/commands.exp,v retrieving revision 1.10 diff -p -r1.10 commands.exp *** testsuite/gdb.base/commands.exp 13 Dec 2001 22:42:23 -0000 1.10 --- testsuite/gdb.base/commands.exp 27 Aug 2002 00:43:32 -0000 *************** proc deprecated_command_test {} { *** 440,446 **** --- 440,559 ---- "deprecate with no arguments" } + proc bp_deleted_in_command_test {} { + global gdb_prompt + + gdb_test "set args 1" "" "set args in bp_deleted_in_command_test" + delete_breakpoints + # Create a breakpoint, and associate a command-list to it, with + # one command that deletes this breakpoint. + gdb_test "break factorial" \ + "Breakpoint \[0-9\]+ at .*: file .*/run.c, line \[0-9\]+\." \ + "breakpoint in bp_deleted_in_command_test" + + send_gdb "commands\n" + gdb_expect { + -re "Type commands for when breakpoint .* is hit, one per line.*>" { + pass "begin commands in bp_deleted_in_command_test" + } + -re "$gdb_prompt $" {fail "begin commands in bp_deleted_in_command_test"} + timeout {fail "(timeout) begin commands bp_deleted_in_command_test"} + } + send_gdb "silent\n" + gdb_expect { + -re ">" {pass "add silent command"} + -re "$gdb_prompt $" {fail "add silent command"} + timeout {fail "(timeout) add silent command"} + } + send_gdb "clear factorial\n" + gdb_expect { + -re ">" {pass "add clear command"} + -re "$gdb_prompt $" {fail "add clear command"} + timeout {fail "(timeout) add clear command"} } + send_gdb "printf \"factorial command-list executed\\n\"\n" + gdb_expect { + -re ">" {pass "add printf command"} + -re "$gdb_prompt $" {fail "add printf command"} + timeout {fail "(timeout) add printf command"} + } + send_gdb "cont\n" + gdb_expect { + -re ">" {pass "add cont command"} + -re "$gdb_prompt $" {fail "add cont command"} + timeout {fail "(timeout) add cont command"} } + send_gdb "end\n" + gdb_expect { + -re "$gdb_prompt $" {pass "end commands"} + timeout {fail "(timeout) end commands"} + } + + gdb_run_cmd + gdb_expect { + -re ".*factorial command-list executed.*1.*Program exited normally.*$gdb_prompt $" { + pass "run factorial until breakpoint" + } + -re ".*$gdb_prompt $" { + fail "run factorial until breakpoint" + } + default { fail "(timeout) run factorial until breakpoint" } + timeout { fail "(timeout) run factorial until breakpoint" } + } + } + + proc temporary_breakpoint_commands {} { + global gdb_prompt + + gdb_test "set args 1" "" "set args in temporary_breakpoint_commands" + delete_breakpoints + + # Create a temporary breakpoint, and associate a commands list to it. + # This test will verify that this commands list is executed when the + # breakpoint is hit. + gdb_test "tbreak factorial" \ + "Breakpoint \[0-9\]+ at .*: file .*/run.c, line \[0-9\]+\." \ + "breakpoint in temporary_breakpoint_commands" + + send_gdb "commands\n" + gdb_expect { + -re "Type commands for when breakpoint .* is hit, one per line.*>" { + pass "begin commands in bp_deleted_in_command_test" + } + -re "$gdb_prompt $" {fail "begin commands in bp_deleted_in_command_test"} + timeout {fail "(timeout) begin commands bp_deleted_in_command_test"} + } + send_gdb "silent\n" + gdb_expect { + -re ">" {pass "add silent tbreak command"} + -re "$gdb_prompt $" {fail "add silent tbreak command"} + timeout {fail "(timeout) add silent tbreak command"} + } + send_gdb "printf \"factorial tbreak commands executed\\n\"\n" + gdb_expect { + -re ">" {pass "add printf tbreak command"} + -re "$gdb_prompt $" {fail "add printf tbreak command"} + timeout {fail "(timeout) add printf tbreak command"} + } + send_gdb "cont\n" + gdb_expect { + -re ">" {pass "add cont tbreak command"} + -re "$gdb_prompt $" {fail "add cont tbreak command"} + timeout {fail "(timeout) add cont tbreak command"} } + send_gdb "end\n" + gdb_expect { + -re "$gdb_prompt $" {pass "end tbreak commands"} + timeout {fail "(timeout) end tbreak commands"} + } + + gdb_run_cmd + gdb_expect { + -re ".*factorial tbreak commands executed.*1.*Program exited normally.*" { + pass "run factorial until temporary breakpoint" + } + timeout { fail "(timeout) run factorial until temporary breakpoint" } + } + } + gdbvar_simple_if_test gdbvar_simple_while_test gdbvar_complex_if_while_test *************** user_defined_command_test *** 454,456 **** --- 567,571 ---- watchpoint_command_test test_command_prompt_position deprecated_command_test + bp_deleted_in_command_test + temporary_breakpoint_commands --------------9800C3F4EACD05CEA9BE6F5F--