From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12492 invoked by alias); 12 Mar 2013 18:36:15 -0000 Received: (qmail 12286 invoked by uid 22791); 12 Mar 2013 18:36:13 -0000 X-SWARE-Spam-Status: No, hits=-8.9 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,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; Tue, 12 Mar 2013 18:36:07 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2CIa4vp011981 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 12 Mar 2013 14:36:04 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r2CIa21u024867; Tue, 12 Mar 2013 14:36:03 -0400 Message-ID: <513F7592.2080902@redhat.com> Date: Tue, 12 Mar 2013 18:36:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130219 Thunderbird/17.0.3 MIME-Version: 1.0 To: Hui Zhu CC: gdb-patches ml , Joel Brobecker Subject: Re: [PATCH] Fix gdb crash with tui References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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-03/txt/msg00539.txt.bz2 On 03/09/2013 02:13 PM, Hui Zhu wrote: > Hi, > > I got crash when I use tui. The steps to reproduce is: > gdb gdb > b gdb_main > r > Ctrl-x A change to TUI mode. > Keep click some times. > Keep click some times. > Then you can get "---Type to continue, or q to quit---" > Click . > Then the GDB crash. > > I think this issue is this part should not output "---Type to > continue, or q to quit---". > So I make a patch to let tui doesn't output "file" just output "fullname". > Not sure is a good enough but this issue is fixed. > > I suggest we fix this issue before next release because it will crash the GDB. > > Thanks, > Hui > > 2013-03-09 Hui Zhu > > * source.c (print_source_lines_base): Doesn't output "file". > Hmm. print_source_lines_base does: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ { ui_out_field_int (uiout, "line", line); ui_out_text (uiout, "\tin "); ui_out_field_string (uiout, "file", symtab_to_filename_for_display (s)); /* TUI expects the "fullname" field. While it is !ui_out_is_mi_like_p compared to CLI it is !ui_source_list. */ if (ui_out_is_mi_like_p (uiout) || !ui_out_test_flags (uiout, ui_source_list)) { const char *fullname = symtab_to_fullname (s); ui_out_field_string (uiout, "fullname", fullname); } ui_out_text (uiout, "\n"); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ And tui_field_string does: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /* Other cli_field_* end up here so alignment and field separators are both handled by tui_field_string. */ static void tui_field_string (struct ui_out *uiout, int fldno, int width, enum ui_align align, const char *fldname, const char *string) { tui_out_data *data = ui_out_data (uiout); if (data->base.suppress_output) return; if (fldname && data->line > 0 && strcmp (fldname, "fullname") == 0) { data->start_of_line ++; if (data->line > 0) { tui_show_source (string, data->line); } return; } data->start_of_line++; (*cli_ui_out_impl.field_string) (uiout, fldno, width, align, fldname, string); } ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ That "fullname" handling is gross... It's there since forever though. This: 703eecdd98022d08b362292ff79ac4087d1406de, Author: Jan Kratochvil Date: Sun Feb 3 16:16:40 2013 +0000 gdb/ * source.c (print_source_lines_base): Print for TUI also "fullname". ... * tui/tui-out.c (tui_field_string): Check for "fullname" now. Did: --- a/gdb/tui/tui-out.c +++ b/gdb/tui/tui-out.c @@ -84,7 +84,7 @@ tui_field_string (struct ui_out *uiout, if (data->base.suppress_output) return; - if (fldname && data->line > 0 && strcmp (fldname, "file") == 0) + if (fldname && data->line > 0 && strcmp (fldname, "fullname") == 0) { So before, tui-out had the hack to call tui_show_source whenever a string "file" was being output. Are there any other cases where we print a "file" string, but not a "filename" string? If so, that may have caused a TUI regression. but the patch also made it so that tui_field_string is called twice: once for "file", and another for "filename". And "file", having to special handling, causes tui_field_string to reach: if (fldname && data->line > 0 && strcmp (fldname, "fullname") == 0) { .,.. } // ... this .... // ###### data->start_of_line++; (*cli_ui_out_impl.field_string) (uiout, fldno, width, align, fldname, string); } And call the cli's field_string output, which goes the the console window, which I guess causes the flashes I see under valgrind, and fills up the pagination, ultimately causing the pagination prompt and the crash as consequence of that being unexpected. Rolling back before that patch, the bug goes away. Another bug that this caused (or rather another manifestation of the bug), is that when you scroll up/down, you see the highlighted line disappear rather than following the scroll. Before the patch it worked correctly. It seems one way to fix it would be revert these two hunks: gdb/ * source.c (print_source_lines_base): Print for TUI also "fullname". ... * tui/tui-out.c (tui_field_string): Check for "fullname" now. But I don't know what motivated that change in the first place. Another idea would be to make tui_field_string ignore "file" strings. Just doing any of these does fix the issue with the highlighted current line disappearing, so there's something else that needs fixing in addition. Yet another that came to mind when I saw the explicit "fullname" check in tui_field_string would make to come up with new ui_out_field_file and ui_out_field_filename methods (would also get rid of the ui_out_is_mi_like_p check in print_source_lines_base), instead of using tui_field_string for file names, which tui would implement by ignoring one, and making the other refresh the source window, though that still feels a little bit hacky (aren't there situations we print a file name but don't want a TUI source refresh?). Maybe some deeper fix could be possible. But I'd go with a simple fix for now. I'm not planing of addressing this further myself for now, but I'd like it fixed in 7.6, being a TUI user myself. Hui, could you create a PR in bugzilla so we can track this? -- Pedro Alves