Re: [PATCH] Fix gdb crash with tui

[Hui Zhu <teawater@gmail.com>]

Not sure the prev backtrace for crash is right, so I post new one:
#0  0x0000000000000000 in ?? ()
#1  0x0000000000770a0e in rl_callback_read_char () at
../../src/readline/callback.c:220
#2  0x000000000061da5d in rl_callback_read_char_wrapper
(client_data=0x0) at ../../src/gdb/event-top.c:163
#3  0x000000000061decd in stdin_event_handler (error=0,
client_data=0x0) at ../../src/gdb/event-top.c:371
#4  0x000000000061c9e9 in handle_file_event (data=...) at
../../src/gdb/event-loop.c:768
#5  0x000000000061beaf in process_event () at ../../src/gdb/event-loop.c:342
#6  0x000000000061bf76 in gdb_do_one_event () at ../../src/gdb/event-loop.c:406
#7  0x000000000061bfc7 in start_event_loop () at ../../src/gdb/event-loop.c:431
#8  0x000000000061da87 in cli_command_loop () at ../../src/gdb/event-top.c:176
#9  0x00000000006141f7 in current_interp_command_loop () at
../../src/gdb/interps.c:331
#10 0x0000000000614c97 in captured_command_loop (data=0x0) at
../../src/gdb/main.c:256
#11 0x0000000000612f42 in catch_errors (func=0x614c7c
<captured_command_loop>, func_args=0x0, errstring=0x94875f "",
    mask=6) at ../../src/gdb/exceptions.c:546
#12 0x0000000000616098 in captured_main (data=0x7fffa80c5cc0) at
../../src/gdb/main.c:1033
#13 0x0000000000612f42 in catch_errors (func=0x614f2d <captured_main>,
func_args=0x7fffa80c5cc0, errstring=0x94875f "",
    mask=6) at ../../src/gdb/exceptions.c:546
#14 0x00000000006160ce in gdb_main (args=0x7fffa80c5cc0) at
../../src/gdb/main.c:1042
#15 0x000000000045b7cf in main (argc=2, argv=0x7fffa80c5dc8) at
../../src/gdb/gdb.c:34

Thanks,
Hui

On Tue, Mar 12, 2013 at 11:14 AM, Hui Zhu <teawater@gmail.com> wrote:
> On Tue, Mar 12, 2013 at 3:25 AM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
>> On Sat, 09 Mar 2013 15:13:34 +0100, Hui Zhu wrote:
>>> 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 <UP> some times.
>>> Keep click <Down> some times.
>>> Then you can get "---Type <return> to continue, or q <return> to quit---"
>>> Click <return>.
>>> Then the GDB crash.
>>>
>>> I think this issue is this part should not output "---Type <return> to
>>> continue, or q <return> to quit---".
>>
>> The patch is really not acceptable, there may be some memory corruption which
>> gets only hidden by the patch.
>>
>> I do not get a crash and not even that prompt.  Could you provide a backtrace?
>> Or even to run parent GDB under valgrind?
>>
>> When I ran it under valgrind I got:
>> ==22920== Source and destination overlap in strcpy(0xefbaed0, 0xefbaed0)
>> ==22920==    at 0x4C2B322: strcpy (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==22920==    by 0x653E33: tui_set_source_content (tui-source.c:225)
>> ==22920==    by 0x6582C3: tui_update_source_window_as_is (tui-winsource.c:99)
>> ==22920==    by 0x658276: tui_update_source_window (tui-winsource.c:81)
>> ==22920==    by 0x654E47: tui_show_frame_info (tui-stack.c:406)
>> ==22920==    by 0x659ABF: tui_enable (tui.c:423)
>>
>> With the debug hook below showing strcpy(sameptr,sameptr).
>>
>> Couldn't this patch (best without the 3rd debug hunk) fix your problem?
>> But maybe it is really unrelated.
>
> After I patch this patch, GDB still crash:
> #0  0x0000000000000000 in ?? ()
> #1  0x0000000000770976 in rl_callback_read_char () at
> ../../src/readline/callback.c:220
> #2  0x000000000061d9c5 in rl_callback_read_char_wrapper
> (client_data=0x0) at ../../src/gdb/event-top.c:163
> #3  0x000000000061de35 in stdin_event_handler (error=0,
> client_data=0x0) at ../../src/gdb/event-top.c:371
> #4  0x000000000061c951 in handle_file_event (data=...) at
> ../../src/gdb/event-loop.c:768
> #5  0x000000000061be17 in process_event () at ../../src/gdb/event-loop.c:342
> #6  0x000000000061bede in gdb_do_one_event () at ../../src/gdb/event-loop.c:406
> #7  0x000000000061bf2f in start_event_loop () at ../../src/gdb/event-loop.c:431
> #8  0x000000000061d9ef in cli_command_loop () at ../../src/gdb/event-top.c:176
> #9  0x000000000061415f in current_interp_command_loop () at
> ../../src/gdb/interps.c:331
> #10 0x0000000000614bff in captured_command_loop (data=0x0) at
> ../../src/gdb/main.c:256
> #11 0x0000000000612eaa in catch_errors (func=0x614be4
> <captured_command_loop>, func_args=0x0, errstring=0x9486bf "",
>     mask=6) at ../../src/gdb/exceptions.c:546
> #12 0x0000000000616000 in captured_main (data=0x7fff57836570) at
> ../../src/gdb/main.c:1033
> #13 0x0000000000612eaa in catch_errors (func=0x614e95 <captured_main>,
> func_args=0x7fff57836570, errstring=0x9486bf "",
>     mask=6) at ../../src/gdb/exceptions.c:546
> #14 0x0000000000616036 in gdb_main (args=0x7fff57836570) at
> ../../src/gdb/main.c:1042
> #15 0x000000000045b7cf in main (argc=2, argv=0x7fff57836678) at
> ../../src/gdb/gdb.c:34
>
> And I think the reason is when push <up> and <down> in tui mode, there
> should not show "---Type <return> to continue, or q <return> to
> quit---".
>
> If we just fix this crash, there will be a lot of "---Type <return> to
> continue, or q <return> to quit---". when push <up> and <down>.
>
> And this is the backtrace that when tui output it:
> #0  prompt_for_continue () at ../../src/gdb/utils.c:1863
> #1  0x000000000071b2ce in fputs_maybe_filtered (linebuffer=0x142b890
> "../../src/gdb/main.c", stream=0x136c110, filter=1)
>     at ../../src/gdb/utils.c:2137
> #2  0x000000000071b7b8 in vfprintf_maybe_filtered (stream=0x136c110,
> format=0x97c1de "%s", args=0x7fffef19b388, filter=1)
>     at ../../src/gdb/utils.c:2324
> #3  0x000000000071b7f3 in vfprintf_filtered (stream=0x136c110,
> format=0x97c1de "%s", args=0x7fffef19b388)
>     at ../../src/gdb/utils.c:2332
> #4  0x00000000006dcd17 in out_field_fmt (uiout=0x12692b0, fldno=146,
> fldname=0x9303c4 "file", format=0x97c1de "%s")
>     at ../../src/gdb/cli-out.c:334
> #5  0x00000000006dc977 in cli_field_string (uiout=0x12692b0,
> fldno=146, width=0, align=ui_noalign,
>     fldname=0x9303c4 "file", string=0x159e390 "../../src/gdb/main.c")
> at ../../src/gdb/cli-out.c:209
> #6  0x000000000052df90 in tui_field_string (uiout=0x12692b0,
> fldno=146, width=0, align=ui_noalign,
>     fldname=0x9303c4 "file", string=0x159e390 "../../src/gdb/main.c")
> at ../../src/gdb/tui/tui-out.c:99
> #7  0x00000000006dbb4a in uo_field_string (uiout=0x12692b0, fldno=146,
> width=0, align=ui_noalign,
>     fldname=0x9303c4 "file", string=0x159e390 "../../src/gdb/main.c")
> at ../../src/gdb/ui-out.c:854
> #8  0x00000000006db474 in ui_out_field_string (uiout=0x12692b0,
> fldname=0x9303c4 "file",
>     string=0x159e390 "../../src/gdb/main.c") at ../../src/gdb/ui-out.c:544
> #9  0x00000000005a9a3f in print_source_lines_base (s=0x1863fc0,
> line=985, stopline=986, flags=PRINT_SOURCE_LINES_NOERROR)
>     at ../../src/gdb/source.c:1347
> #10 0x00000000005a9ddc in print_source_lines (s=0x1863fc0, line=985,
> stopline=986, flags=(unknown: 0))
>     at ../../src/gdb/source.c:1442
> #11 0x000000000052fe6a in tui_vertical_source_scroll
> (scroll_direction=BACKWARD_SCROLL, num_to_scroll=1)
>     at ../../src/gdb/tui/tui-source.c:385
> #12 0x000000000053160c in tui_scroll_backward
> (win_to_scroll=0x1d6a6c0, num_to_scroll=1)
>     at ../../src/gdb/tui/tui-win.c:538
> #13 0x0000000000528b65 in tui_dispatch_ctrl_char (ch=259) at
> ../../src/gdb/tui/tui-command.c:118
> #14 0x000000000052c57f in tui_getc (fp=0x7f67f2dee340
> <_IO_2_1_stdin_>) at ../../src/gdb/tui/tui-io.c:692
> #15 0x00000000007702d7 in rl_read_key () at ../../src/readline/input.c:448
> ---Type <return> to continue, or q <return> to quit---
> #16 0x0000000000756c08 in readline_internal_char () at
> ../../src/readline/readline.c:517
> #17 0x00000000007708e9 in rl_callback_read_char () at
> ../../src/readline/callback.c:201
> #18 0x000000000061d9c5 in rl_callback_read_char_wrapper
> (client_data=0x0) at ../../src/gdb/event-top.c:163
> #19 0x000000000061de35 in stdin_event_handler (error=0,
> client_data=0x0) at ../../src/gdb/event-top.c:371
> #20 0x000000000061c951 in handle_file_event (data=...) at
> ../../src/gdb/event-loop.c:768
> #21 0x000000000061be17 in process_event () at ../../src/gdb/event-loop.c:342
> #22 0x000000000061bede in gdb_do_one_event () at ../../src/gdb/event-loop.c:406
>
> Thanks,
> Hui
>
>>
>>
>> Thanks,
>> Jan
>>
>>
>> gdb/
>> 2013-03-11  Jan Kratochvil  <jan.kratochvil@redhat.com>
>>
>>         * tui/tui-source.c (tui_set_source_content): Allocate and free SRC_LINE
>>         always.
>>
>> diff --git a/gdb/tui/tui-source.c b/gdb/tui/tui-source.c
>> index e599382..41e7aa6 100644
>> --- a/gdb/tui/tui-source.c
>> +++ b/gdb/tui/tui-source.c
>> @@ -116,9 +116,7 @@ tui_set_source_content (struct symtab *s,
>>                   src->gdbarch = get_objfile_arch (s->objfile);
>>                   src->start_line_or_addr.loa = LOA_LINE;
>>                   cur_line_no = src->start_line_or_addr.u.line_no = line_no;
>> -                 if (offset > 0)
>> -                   src_line = (char *) xmalloc (
>> -                                          (threshold + 1) * sizeof (char));
>> +                 src_line = xmalloc (threshold + 1);
>>                   while (cur_line < nlines)
>>                     {
>>                       struct tui_win_element *element
>> @@ -128,10 +126,6 @@ tui_set_source_content (struct symtab *s,
>>                       /* Get the first character in the line.  */
>>                       c = fgetc (stream);
>>
>> -                     if (offset == 0)
>> -                       src_line = ((struct tui_win_element *)
>> -                                  TUI_SRC_WIN->generic.content[
>> -                                       cur_line])->which_element.source.line;
>>                       /* Init the line with the line number.  */
>>                       sprintf (src_line, "%-6d", cur_line_no);
>>                       cur_len = strlen (src_line);
>> @@ -222,9 +216,20 @@ tui_set_source_content (struct symtab *s,
>>                       /* Now copy the line taking the offset into
>>                          account.  */
>>                       if (strlen (src_line) > offset)
>> +{
>> +char *a=((struct tui_win_element *)
>> +                                TUI_SRC_WIN->generic.content[cur_line])->which_element.source.line;
>> +char *b=&src_line[offset];
>> +size_t l=strlen(b)+1;
>> +if (a==b
>> +||(a<b&&a+l>b)
>> +||(b<a&&b+l>a)
>> +)
>> +sleep(0);
>>                         strcpy (((struct tui_win_element *)
>>                                  TUI_SRC_WIN->generic.content[cur_line])->which_element.source.line,
>>                                 &src_line[offset]);
>> +}
>>                       else
>>                         ((struct tui_win_element *)
>>                          TUI_SRC_WIN->generic.content[
>> @@ -232,8 +237,7 @@ tui_set_source_content (struct symtab *s,
>>                       cur_line++;
>>                       cur_line_no++;
>>                     }
>> -                 if (offset > 0)
>> -                   xfree (src_line);
>> +                 xfree (src_line);
>>                   fclose (stream);
>>                   TUI_SRC_WIN->generic.content_size = nlines;
>>                   ret = TUI_SUCCESS;