From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7753 invoked by alias); 14 Mar 2013 12:33:33 -0000 Received: (qmail 7743 invoked by uid 22791); 14 Mar 2013 12:33:31 -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; Thu, 14 Mar 2013 12:33:21 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2ECXI3B002025 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Thu, 14 Mar 2013 08:33:18 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r2ECXFv5005493; Thu, 14 Mar 2013 08:33:16 -0400 Message-ID: <5141C38B.1080204@redhat.com> Date: Thu, 14 Mar 2013 12:33: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: Jan Kratochvil CC: Hui Zhu , gdb-patches ml , Joel Brobecker Subject: Re: [patch+7.6] [TUI] Fix scrolling crash 7.6 regression [Re: [PATCH] Fix gdb crash with tui] References: <513F7592.2080902@redhat.com> <20130313185456.GA18563@host2.jankratochvil.net> In-Reply-To: <20130313185456.GA18563@host2.jankratochvil.net> 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/msg00623.txt.bz2 On 03/13/2013 06:54 PM, Jan Kratochvil wrote: > On Tue, 12 Mar 2013 19:36:02 +0100, Pedro Alves wrote: >> 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" > typo: "fullname" >> string? If so, that may have caused a TUI regression. > > I was verifying print_source_lines_base is surprisingly really the only case > from which the output is caught by tui_field_string. tui_field_string > together with tui_field_int required that "line" precedes "file" on the same > line. While every other GDB output normally prints "line" only after "file" > is output. (Currently everything is s/"file"/"fullname"/.) OK. Man, this is really gross. >> but the patch also made it so that tui_field_string is called >> twice: once for "file", and another for "filename". And "file", > typo: "fullname" >> 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, > > That's true but I expect there has to be output a lot of other garbage like > "\tin " or "\n" so I did not consider "file" to be significant. I guess the > same crash could happen before just after much more scrollings. Hmm, I'm not seeing how. tui_show_source does no other output to gdb_stdout. With output of regular commands, e.g., enable the TUI, and do "set height 1", you should see that pagination works as expected. Well, I do see how, from a distance, haven't investigated fully. With "set height 1", and a series of background steps "s&", I could get GDB to crash in a similar way. >> and fills up the pagination, ultimately causing the pagination prompt and >> the crash as consequence of that being unexpected. > > I still do not have the crash reproducible, I even tried to tune stty size. The TUI resets the terminal height. This only triggers after column wrapping (since the file name has no \n.). What do "show height" and "show width" show for you after you enable the TUI? If I start GDB on one terminal $ ./gdb someprogram (gdb) show height Number of lines gdb thinks are in a page is 15. (gdb) show colu Undefined show command: "colu". Try "help show". (gdb) show width Number of characters gdb thinks are in a line is 172. And then on another terminal attach to that gdb, and do: (gdb) watch chars_printed Hardware watchpoint 1: chars_printed (gdb) watch lines_printed Hardware watchpoint 2: lines_printed (gdb) commands 1-2 Type commands for breakpoint(s) 1-2, one per line. End with a line saying just "end". >continue >end (gdb) set pagination off (gdb) b prompt_for_continue Breakpoint 3 at 0x6dc212: file ../../src/gdb/utils.c, line 1870. (gdb) c Continuing. Eventually, I see: ... Old value = 171 New value = 172 fputs_maybe_filtered (linebuffer=0x33f8f60 "../../src/gdb/gdb.c", stream=0x2268590, filter=1) at ../../src/gdb/utils.c:2120 2120 lineptr++; Hardware watchpoint 5: chars_printed Old value = 172 New value = 0 fputs_maybe_filtered (linebuffer=0x33f8f60 "../../src/gdb/gdb.c", stream=0x2268590, filter=1) at ../../src/gdb/utils.c:2128 2128 lines_printed++; Hardware watchpoint 6: lines_printed Old value = 13 New value = 14 fputs_maybe_filtered (linebuffer=0x33f8f60 "../../src/gdb/gdb.c", stream=0x2268590, filter=1) at ../../src/gdb/utils.c:2132 2132 if (wrap_column) Breakpoint 3, prompt_for_continue () at ../../src/gdb/utils.c:1870 1870 gettimeofday (&prompt_started, NULL); (gdb) > 2013-03-13 Hui Zhu > Jan Kratochvil > > * source.c (print_source_lines_base): Suppress "file" for TUI. This is fine with me. Please check it in. > - /* TUI expects the "fullname" field. While it is > - !ui_out_is_mi_like_p compared to CLI it is !ui_source_list. */ > + /* CLI expects only the "file" field. TUI expects only the > + "fullname" field (and TUI does break if "file" is printed). > + MI expects both the fields. ui_source_list is set only for CLI, s/both the fields/both fields/ I'm wondering about whether this "fullname" handling gross-ness in the TUI is really necessary. #0 tui_show_source (fullname=0x1a1b420 "/home/pedro/gdb/mygit/src/gdb/gdb.c", line=16) at ../../src/gdb/tui/tui.c:537 #1 0x00000000004f7655 in tui_field_string (uiout=0xd6d5a0, fldno=3, width=0, align=ui_noalign, fldname=0x879449 "fullname", string= 0x1a1b420 "/home/pedro/gdb/mygit/src/gdb/gdb.c") at ../../src/gdb/tui/tui-out.c:92 #2 0x000000000069dd9d in uo_field_string (uiout=0xd6d5a0, fldno=3, width=0, align=ui_noalign, fldname=0x879449 "fullname", string= 0x1a1b420 "/home/pedro/gdb/mygit/src/gdb/gdb.c") at ../../src/gdb/ui-out.c:854 #3 0x000000000069d6f4 in ui_out_field_string (uiout=0xd6d5a0, fldname=0x879449 "fullname", string=0x1a1b420 "/home/pedro/gdb/mygit/src/gdb/gdb.c") at ../../src/gdb/ui-out.c:544 #4 0x0000000000570fe6 in print_source_lines_base (s=0x15fa010, line=16, stopline=17, flags=PRINT_SOURCE_LINES_NOERROR) at ../../src/gdb/source.c:1357 #5 0x000000000057131f in print_source_lines (s=0x15fa010, line=16, stopline=17, flags=(unknown: 0)) at ../../src/gdb/source.c:1442 #6 0x00000000004f943e in tui_vertical_source_scroll (scroll_direction=BACKWARD_SCROLL, num_to_scroll=1) at ../../src/gdb/tui/tui-source.c:385 #7 0x00000000004faa89 in tui_scroll_backward (win_to_scroll=0x20755e0, num_to_scroll=1) at ../../src/gdb/tui/tui-win.c:538 #8 0x00000000004f242c in tui_dispatch_ctrl_char (ch=259) at ../../src/gdb/tui/tui-command.c:118 #9 0x00000000004f5ce5 in tui_getc (fp=0x3d261b1340) at ../../src/gdb/tui/tui-io.c:692 #10 0x000000000072fa31 in rl_read_key () at ../../src/readline/input.c:448 At frame #6, we were in the TUI code, decided we need to show the source, and called into the core's print_source_lines, only to end up in tui_show_source (which AFAICS prints directly to the curses window), in the TUI again, through a contorted hack. It seems like we should be able to bypass all that and go from frame #6 to #1 directly or almost directly. Haven't tried that yet (for >7.6) ... Thanks, -- Pedro Alves