Re: [PATCH] Fix gdb crash with tui

[Hui Zhu <teawater@gmail.com>]

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;