From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4546 invoked by alias); 18 Feb 2013 13:09:17 -0000 Received: (qmail 4504 invoked by uid 22791); 18 Feb 2013 13:09:15 -0000 X-SWARE-Spam-Status: No, hits=-3.8 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 18 Feb 2013 13:09:09 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1U7QSy-0005Is-FU from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Mon, 18 Feb 2013 05:09:08 -0800 Received: from SVR-ORW-FEM-04.mgc.mentorg.com ([147.34.97.41]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Mon, 18 Feb 2013 05:09:08 -0800 Received: from qiyao.dyndns.org.dyndns.org (147.34.91.1) by svr-orw-fem-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server id 14.1.289.1; Mon, 18 Feb 2013 05:09:07 -0800 From: Yao Qi To: Subject: [RFC] PR 15075 dprintf interferes with "next" Date: Mon, 18 Feb 2013 13:09:00 -0000 Message-ID: <1361192891-29341-1-git-send-email-yao@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes 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: 2013-02/txt/msg00451.txt.bz2 Hi, I'd like to get your comments on this patch, as I think my patch is hacky but I am unable to figure out a correct one. This patch fixes more problems than PR 15075 mentioned, they are, - When step a source line by "next" command, and there is a dprintf on that line, "next" command will behave like "continue". - Two dprintf are set on the same address. Only one printf is executed. - dprintf and regular breakpoint are set on the same address, inferior doesn't stop. In order to fix these problems, I don't append "continue" command to dprintf breakpoint, and after all commands of all breakpoints (caused the stop) are executed, execute "continue" command if 1) the inferior is not proceeded by these commands 2) and the stop is only caused by dprintf (no other types of breakpoint) 3) the inferior is not in the state of stepping a source line or an instruction (to fix PR 15075). My patch fixes all the three known problems, but I think it is still hacky. The printf stuff is implemented as command, it is convenient to move them to the target side. I tried to change the bpstat saying that "don't stop", but the command can't be executed. We have to keep bpstat saying that "stop" for dprintf. I also thought about Tom's suggestion about Python "Breakpoint.stop" API, but "Breakpoint.stop" acts as condition instead of command, so I don't go that way. Known issue: from the user's perspective, inferior stops at the dprintf breakpoint if dprintf breakpoint and regular breakpoint are set at the same address. I'll fix it if my patch is on the right track. gdb: 2013-02-18 Yao Qi * breakpoint.c (bpstat_do_actions_1): Change argument from "bpstat *" to "struc thread_info *". (bpstat_do_actions_1): Execute "continue" command if BS has dprintf breakpoint only. (bpstat_do_actions): Caller update. (update_dprintf_command_list): Don't append "continue" command to the command of dprintf breakpoint. --- gdb/breakpoint.c | 68 ++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 49 insertions(+), 19 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index fb57a57..878ab08 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -4263,20 +4263,26 @@ command_line_is_silent (struct command_line *cmd) } /* Execute all the commands associated with all the breakpoints at - this location. Any of these commands could cause the process to - proceed beyond this point, etc. We look out for such changes by - checking the global "breakpoint_proceeded" after each command. + the location where THREAD stops. Any of these commands could cause + the process to proceed beyond this point, etc. We look out for + such changes by checking the global "breakpoint_proceeded" after + each command. Returns true if a breakpoint command resumed the inferior. In that case, it is the caller's responsibility to recall it again with the bpstat of the current thread. */ static int -bpstat_do_actions_1 (bpstat *bsp) +bpstat_do_actions_1 (struct thread_info *thread) { + bpstat *bsp = &thread->control.stop_bpstat; bpstat bs; struct cleanup *old_chain; int again = 0; + /* Breakpoint dprintf in list BS. */ + int bkpt_dprintf = 0; + /* Breakpoint other than dprintf in list BS. */ + int bkpt_other_than_dprintf = 0; /* Avoid endless recursion if a `source' command is contained in bs->commands. */ @@ -4298,6 +4304,13 @@ bpstat_do_actions_1 (bpstat *bsp) struct command_line *cmd; struct cleanup *this_cmd_tree_chain; + if (bs->breakpoint_at != NULL) + { + if (bs->breakpoint_at->type == bp_dprintf) + bkpt_dprintf = 1; + else + bkpt_other_than_dprintf = 1; + } /* Take ownership of the BSP's command tree, if it has one. The command tree could legitimately contain commands like @@ -4356,6 +4369,33 @@ bpstat_do_actions_1 (bpstat *bsp) break; } } + + /* Inferior is not proceed by commands. */ + if (!breakpoint_proceeded + /* No breakpoint of type other than dprintf. If there is + breakpoint of other types in BS, the inferior really needs + stopped. Don't execute command "continue". */ + && !bkpt_other_than_dprintf && bkpt_dprintf + && (0 != strcmp (dprintf_style, dprintf_style_agent)) + /* GDB is not stepping a line. When GDB is stepping a source + line, don't execute command "continue", otherwise "next" will + behave like "continue", which is not expected. */ + && !(thread->control.step_range_start >= 1 + && thread->control.step_range_end >= 1)) + { + struct command_line *cont_cmd_line + = xmalloc (sizeof (struct command_line)); + + cont_cmd_line->control_type = simple_control; + cont_cmd_line->body_count = 0; + cont_cmd_line->body_list = NULL; + cont_cmd_line->next = NULL; + cont_cmd_line->line = xstrdup ("continue");; + + execute_control_command (cont_cmd_line); + if (!target_can_async_p ()) + again = 1; + } do_cleanups (old_chain); return again; } @@ -4374,7 +4414,7 @@ bpstat_do_actions (void) and only return when it is stopped at the next breakpoint, we keep doing breakpoint actions until it returns false to indicate the inferior was not resumed. */ - if (!bpstat_do_actions_1 (&inferior_thread ()->control.stop_bpstat)) + if (!bpstat_do_actions_1 (inferior_thread ())) break; discard_cleanups (cleanup_if_error); @@ -8947,25 +8987,15 @@ update_dprintf_command_list (struct breakpoint *b) _("Invalid dprintf style.")); gdb_assert (printf_line != NULL); - /* Manufacture a printf/continue sequence. */ + /* Manufacture a printf command. */ { - struct command_line *printf_cmd_line, *cont_cmd_line = NULL; - - if (strcmp (dprintf_style, dprintf_style_agent) != 0) - { - cont_cmd_line = xmalloc (sizeof (struct command_line)); - cont_cmd_line->control_type = simple_control; - cont_cmd_line->body_count = 0; - cont_cmd_line->body_list = NULL; - cont_cmd_line->next = NULL; - cont_cmd_line->line = xstrdup ("continue"); - } + struct command_line *printf_cmd_line + = xmalloc (sizeof (struct command_line)); - printf_cmd_line = xmalloc (sizeof (struct command_line)); printf_cmd_line->control_type = simple_control; printf_cmd_line->body_count = 0; printf_cmd_line->body_list = NULL; - printf_cmd_line->next = cont_cmd_line; + printf_cmd_line->next = NULL; printf_cmd_line->line = printf_line; breakpoint_set_commands (b, printf_cmd_line); -- 1.7.7.6