This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch+7.6] [TUI] Fix scrolling crash 7.6 regression [Re: [PATCH] Fix gdb crash with tui]


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  <hui_zhu@mentor.com>
> 	    Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]