[PATCH] Fix gdb crash with tui

[PATCH] Fix gdb crash with tui

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 713 bytes --]

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 <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---".
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  <hui_zhu@mentor.com>

	* source.c (print_source_lines_base): Doesn't output "file".

[-- Attachment #2: fix-tui-crash.txt --]
[-- Type: text/plain, Size: 817 bytes --]

--- a/source.c
+++ b/source.c
@@ -1344,18 +1344,16 @@ print_source_lines_base (struct symtab *
 	{
 	  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", symtab_to_fullname (s));
+	  else
+	    ui_out_field_string (uiout, "file",
+				 symtab_to_filename_for_display (s));
 
-	      ui_out_field_string (uiout, "fullname", fullname);
-	    }
 	  ui_out_text (uiout, "\n");
 	}
 

Re: [PATCH] Fix gdb crash with tui

[Jan Kratochvil]

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.


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;

Re: [PATCH] Fix gdb crash with tui

[Hui Zhu]

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;

Re: [PATCH] Fix gdb crash with tui

[Hui Zhu]

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;

Re: [PATCH] Fix gdb crash with tui

[Jan Kratochvil]

On Tue, 12 Mar 2013 13:21:44 +0100, Hui Zhu wrote:
> Not sure the prev backtrace for crash is right, so I post new one:

BTW don't you have a recipe for the reproducibility?  Don't you have some
unusual terminal size (stty size) for example?  Which distro?


Jan

Re: [PATCH] Fix gdb crash with tui

[Hui Zhu]

On Tue, Mar 12, 2013 at 8:36 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Tue, 12 Mar 2013 13:21:44 +0100, Hui Zhu wrote:
>> Not sure the prev backtrace for crash is right, so I post new one:
>
> BTW don't you have a recipe for the reproducibility?  Don't you have some
> unusual terminal size (stty size) for example?  Which distro?
>
>
> Jan

Ubuntu 12.04.  I can got this crash even if in simple text terminal.
I am trying to reproduce this issue in fedora.

Thanks,
Hui

Re: [PATCH] Fix gdb crash with tui

[Hui Zhu]

Hi Jan,

I got crash in fedora 13, this is the backtrace:

#0  0x0000000000000000 in ?? ()
#1  0x000000000076b353 in rl_callback_read_char () at
../../gdb/readline/callback.c:220
#2  0x000000000061a771 in rl_callback_read_char_wrapper
(client_data=0x0) at ../../gdb/gdb/event-top.c:163
#3  0x000000000061ab0b in stdin_event_handler (error=0,
client_data=0x0) at ../../gdb/gdb/event-top.c:371
#4  0x00000000006196f9 in handle_file_event (data=...) at
../../gdb/gdb/event-loop.c:768
#5  0x0000000000618bb4 in process_event () at ../../gdb/gdb/event-loop.c:342
#6  0x0000000000618c7a in gdb_do_one_event () at ../../gdb/gdb/event-loop.c:406
#7  0x0000000000618ccb in start_event_loop () at ../../gdb/gdb/event-loop.c:431
#8  0x000000000061a79b in cli_command_loop () at ../../gdb/gdb/event-top.c:176
#9  0x0000000000611112 in current_interp_command_loop () at
../../gdb/gdb/interps.c:331
#10 0x0000000000611b41 in captured_command_loop (data=0x0) at
../../gdb/gdb/main.c:256
#11 0x000000000060fe50 in catch_errors (func=0x611b26
<captured_command_loop>, func_args=0x0, errstring=0x9433d4 "",
    mask=6) at ../../gdb/gdb/exceptions.c:546
#12 0x0000000000612fb5 in captured_main (data=0x7fff099dcc90) at
../../gdb/gdb/main.c:1033
#13 0x000000000060fe50 in catch_errors (func=0x611dff <captured_main>,
func_args=0x7fff099dcc90, errstring=0x9433d4 "",
    mask=6) at ../../gdb/gdb/exceptions.c:546
#14 0x0000000000612feb in gdb_main (args=0x7fff099dcc90) at
../../gdb/gdb/main.c:1042
#15 0x000000000045d832 in main (argc=2, argv=0x7fff099dcd98) at
../../gdb/gdb/gdb.c:34

Thanks,
Hui

On Tue, Mar 12, 2013 at 9:19 PM, Hui Zhu <teawater@gmail.com> wrote:
> On Tue, Mar 12, 2013 at 8:36 PM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
>> On Tue, 12 Mar 2013 13:21:44 +0100, Hui Zhu wrote:
>>> Not sure the prev backtrace for crash is right, so I post new one:
>>
>> BTW don't you have a recipe for the reproducibility?  Don't you have some
>> unusual terminal size (stty size) for example?  Which distro?
>>
>>
>> Jan
>
> Ubuntu 12.04.  I can got this crash even if in simple text terminal.
> I am trying to reproduce this issue in fedora.
>
> Thanks,
> Hui

Re: [PATCH] Fix gdb crash with tui

[Pedro Alves]

On 03/12/2013 12:36 PM, Jan Kratochvil wrote:
> On Tue, 12 Mar 2013 13:21:44 +0100, Hui Zhu wrote:
>> Not sure the prev backtrace for crash is right, so I post new one:
> 
> BTW don't you have a recipe for the reproducibility?  Don't you have some
> unusual terminal size (stty size) for example?  Which distro?
> 
> 
> Jan

I just reproduced it on my Fedora 17 box with current mainline.

$ stty size
45 172

To reproduce, I just did the steps in the OP.

$ ./gdb ./gdb
(gdb) start

"c-x a" to flip to TUI mode.

repeat:
 press arrow up a bit, so the source display scrolls a bit up.
 press arrow down a bit, so the source display scrolls a bit down.
 goto repeat until

 "---Type <return> to continue, or q <return> to quit---"

appears in TUI's console.  Then press return.

(top-gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x000000000072ea9a in rl_callback_read_char () at ../../src/readline/callback.c:220
#2  0x00000000005e1c9d in rl_callback_read_char_wrapper (client_data=0x0) at ../../src/gdb/event-top.c:163
#3  0x00000000005e207e in stdin_event_handler (error=0, client_data=0x0) at ../../src/gdb/event-top.c:371
#4  0x00000000005e0c22 in handle_file_event (data=...) at ../../src/gdb/event-loop.c:768
#5  0x00000000005e00cb in process_event () at ../../src/gdb/event-loop.c:342
#6  0x00000000005e0192 in gdb_do_one_event () at ../../src/gdb/event-loop.c:406
#7  0x00000000005e01e3 in start_event_loop () at ../../src/gdb/event-loop.c:431
#8  0x00000000005e1cc7 in cli_command_loop () at ../../src/gdb/event-top.c:176
#9  0x00000000005d8768 in current_interp_command_loop () at ../../src/gdb/interps.c:331
#10 0x00000000005d91a6 in captured_command_loop (data=0x0) at ../../src/gdb/main.c:256
#11 0x00000000005d7512 in catch_errors (func=0x5d918b <captured_command_loop>, func_args=0x0, errstring=0x88f8d4 "", mask=6) at ../../src/gdb/exceptions.c:546
#12 0x00000000005da5b9 in captured_main (data=0x7fff21b3eea0) at ../../src/gdb/main.c:1033
#13 0x00000000005d7512 in catch_errors (func=0x5d9442 <captured_main>, func_args=0x7fff21b3eea0, errstring=0x88f8d4 "", mask=6) at ../../src/gdb/exceptions.c:546
#14 0x00000000005da5ef in gdb_main (args=0x7fff21b3eea0) at ../../src/gdb/main.c:1042
#15 0x000000000045a00a in main (argc=2, argv=0x7fff21b3efa8) at ../../src/gdb/gdb.c:34
(top-gdb)

-- 
Pedro Alves

Re: [PATCH] Fix gdb crash with tui

[Pedro Alves]

[-- Attachment #1: Type: text/plain, Size: 1476 bytes --]

On 03/12/2013 04:03 PM, Pedro Alves wrote:
> On 03/12/2013 12:36 PM, Jan Kratochvil wrote:
>> On Tue, 12 Mar 2013 13:21:44 +0100, Hui Zhu wrote:
>>> Not sure the prev backtrace for crash is right, so I post new one:
>>
>> BTW don't you have a recipe for the reproducibility?  Don't you have some
>> unusual terminal size (stty size) for example?  Which distro?
>>
>>
>> Jan
> 
> I just reproduced it on my Fedora 17 box with current mainline.
> 
> $ stty size
> 45 172
> 
> To reproduce, I just did the steps in the OP.
> 
> $ ./gdb ./gdb
> (gdb) start
> 
> "c-x a" to flip to TUI mode.
> 
> repeat:
>  press arrow up a bit, so the source display scrolls a bit up.
>  press arrow down a bit, so the source display scrolls a bit down.
>  goto repeat until
> 
>  "---Type <return> to continue, or q <return> to quit---"
> 
> appears in TUI's console.  Then press return.

Valgrind log appended (w/ valgrind --log-file=valgrind.log ./gdb ./gdb).

Suspecting this to be some rounding error, I tried with
44 columns, but I get the same.

Running under valgrind, however, since things run much
slower, it's noticeable that GDB occasionally prints the
source file name at the wrong place -- we see it
flashing under the cursor in the console window.

I don't know if that's the source file name that's supposed
to go to the top left, or if it's an extra rogue source file
name that should not be printed at all in the tui.

I don't see that flash without valgrind.

-- 
Pedro Alves


[-- Attachment #2: valgrind.log.gz --]
[-- Type: application/x-gzip, Size: 4999 bytes --]

Re: [PATCH] Fix gdb crash with tui

[Pedro Alves]

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

Re: [PATCH] Fix gdb crash with tui

[Pedro Alves]

On 03/12/2013 06:36 PM, Pedro Alves wrote:
> 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.

FAOD, I meant s/does fix/does NOT fix/.

-- 
Pedro Alves

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

[Jan Kratochvil]

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"/.)


> 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.


> 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.


> 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 is an unrelated bug but regressed by the same patch.  Going to post a patch
for it as a second one.


> But I don't know what motivated that change in the first place.

The motivation was to fix incorrect TUI handling of source files with the same
basename but different dirname, as was demonstrated in:
	[patchv2 8/11] TUI: source "file" -> "fullname"
	http://sourceware.org/ml/gdb-patches/2013-01/msg00665.html
	Message-ID: <20130127223625.GI15252@host2.jankratochvil.net>


I am unable to test this specific patch, I at least understand how the
original Hui's patch should have worked.  But it regressed MI output so I have
fixed that.

No regressions on {x86_64,x86_64-m32,i686}-fedora19pre-linux-gnu but that does
not say much for TUI.


Thanks,
Jan


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.

diff --git a/gdb/source.c b/gdb/source.c
index f5949e6..828d953 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1344,11 +1344,15 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
 	{
 	  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.  */
+	  /* 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,
+	     not for TUI.  */
+	  if (ui_out_is_mi_like_p (uiout)
+	      || ui_out_test_flags (uiout, ui_source_list))
+	    ui_out_field_string (uiout, "file",
+				 symtab_to_filename_for_display (s));
 	  if (ui_out_is_mi_like_p (uiout)
 	      || !ui_out_test_flags (uiout, ui_source_list))
 	    {
@@ -1356,6 +1360,7 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
 
 	      ui_out_field_string (uiout, "fullname", fullname);
 	    }
+
 	  ui_out_text (uiout, "\n");
 	}
 

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

[Jan Kratochvil]

On Tue, 12 Mar 2013 19:36:02 +0100, Pedro Alves wrote:
> 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.

That is the second patch for a regression by me.

#0  __memset_sse2 () at ../sysdeps/x86_64/memset.S:360
#1  in freehook (ptr=0x3a61ba0, caller=0x879de9 <xfree+31>) at mcheck.c:193
#2  in xfree (ptr=0x3a61ba0) at ./common/common-utils.c:108
#3  in find_and_open_source (filename=0x23c2010 "gdb.c", dirname=0x289cec0 "/home/jkratoch/redhat/gdb-clean/gdb", fullname=0x289cde0) at source.c:1008
#4  in open_source_file (s=0x289cd80) at source.c:1088
#5  in tui_set_source_content (s=0x289cd80, line_no=16, noerror=0) at ./tui/tui-source.c:61
#6  in tui_update_source_window_as_is (win_info=0x3a51a80, gdbarch=0x22d0000, s=0x289cd80, line_or_addr=..., noerror=0) at ./tui/tui-winsource.c:99
#7  in tui_show_symtab_source (gdbarch=0x22d0000, s=0x289cd80, line=..., noerror=0) at ./tui/tui-source.c:339 
#8  in tui_update_source_windows_with_line (s=0x289cd80, line=16) at ./tui/tui-winsource.c:201
#9  in tui_show_source (fullname=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c", line=16) at ./tui/tui.c:542
#10 in tui_field_string (uiout=0x2296dc0, fldno=3, width=0, align=ui_noalign, fldname=0xfa09d9 "fullname", string=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c") at ./tui/tui-out.c:92
#11 in uo_field_string (uiout=0x2296dc0, fldno=3, width=0, align=ui_noalign, fldname=0xfa09d9 "fullname", string=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c") at ui-out.c:854
#12 in ui_out_field_string (uiout=0x2296dc0, fldname=0xfa09d9 "fullname", string=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c") at ui-out.c:544
#13 in print_source_lines_base (s=0x289cd80, line=16, stopline=17, flags=PRINT_SOURCE_LINES_NOERROR) at source.c:1357
#14 in print_source_lines (s=0x289cd80, line=16, stopline=17, flags=(unknown: 0)) at source.c:1442
#15 in tui_vertical_source_scroll (scroll_direction=BACKWARD_SCROLL, num_to_scroll=1) at ./tui/tui-source.c:393
#16 in tui_scroll_backward (win_to_scroll=0x3a51a80, num_to_scroll=1) at ./tui/tui-win.c:538
#17 in tui_dispatch_ctrl_char (ch=259) at ./tui/tui-command.c:118
#18 in tui_getc (fp=0x7ffff663b660 <_IO_2_1_stdin_>) at ./tui/tui-io.c:692
#19 in rl_read_key () at input.c:448


I did not expect this may happen...

No regressions on {x86_64,x86_64-m32,i686}-fedora19pre-linux-gnu but it does
not say much for TUI.


Thanks,
Jan


2013-03-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* source.c (print_source_lines_base): Make a local copy of
	symtab_to_fullname.

diff --git a/gdb/source.c b/gdb/source.c
index 828d953..201e848 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1355,11 +1355,17 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
 				 symtab_to_filename_for_display (s));
 	  if (ui_out_is_mi_like_p (uiout)
 	      || !ui_out_test_flags (uiout, ui_source_list))
-	    {
-	      const char *fullname = symtab_to_fullname (s);
+ 	    {
+	      const char *s_fullname = symtab_to_fullname (s);
+	      char *local_fullname;
 
-	      ui_out_field_string (uiout, "fullname", fullname);
-	    }
+	      /* ui_out_field_string may free S_FULLNAME by calling
+	         open_source_file for it again.  */
+	      local_fullname = alloca (strlen (s_fullname) + 1);
+	      strcpy (local_fullname, s_fullname);
+
+	      ui_out_field_string (uiout, "fullname", local_fullname);
+ 	    }
 
 	  ui_out_text (uiout, "\n");
 	}

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

[Hui Zhu]

Hi Jan,

After patch this patch.  My issue is fixed.
Thanks.

Best,
Hui

On 03/14/13 02:54, 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"/.)
>
>
>> 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.
>
>
>> 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.
>
>
>> 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 is an unrelated bug but regressed by the same patch.  Going to post a patch
> for it as a second one.
>
>
>> But I don't know what motivated that change in the first place.
>
> The motivation was to fix incorrect TUI handling of source files with the same
> basename but different dirname, as was demonstrated in:
> 	[patchv2 8/11] TUI: source "file" -> "fullname"
> 	http://sourceware.org/ml/gdb-patches/2013-01/msg00665.html
> 	Message-ID: <20130127223625.GI15252@host2.jankratochvil.net>
>
>
> I am unable to test this specific patch, I at least understand how the
> original Hui's patch should have worked.  But it regressed MI output so I have
> fixed that.
>
> No regressions on {x86_64,x86_64-m32,i686}-fedora19pre-linux-gnu but that does
> not say much for TUI.
>
>
> Thanks,
> Jan
>
>
> 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.
>
> diff --git a/gdb/source.c b/gdb/source.c
> index f5949e6..828d953 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -1344,11 +1344,15 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
>   	{
>   	  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.  */
> +	  /* 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,
> +	     not for TUI.  */
> +	  if (ui_out_is_mi_like_p (uiout)
> +	      || ui_out_test_flags (uiout, ui_source_list))
> +	    ui_out_field_string (uiout, "file",
> +				 symtab_to_filename_for_display (s));
>   	  if (ui_out_is_mi_like_p (uiout)
>   	      || !ui_out_test_flags (uiout, ui_source_list))
>   	    {
> @@ -1356,6 +1360,7 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
>
>   	      ui_out_field_string (uiout, "fullname", fullname);
>   	    }
> +
>   	  ui_out_text (uiout, "\n");
>   	}
>
>

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

[Hui Zhu]

Hi Jan,

After patch this patch.  My issue is fixed.
Thanks.

Best,
Hui

On 03/14/13 02:55, Jan Kratochvil wrote:
> On Tue, 12 Mar 2013 19:36:02 +0100, Pedro Alves wrote:
>> 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.
>
> That is the second patch for a regression by me.
>
> #0  __memset_sse2 () at ../sysdeps/x86_64/memset.S:360
> #1  in freehook (ptr=0x3a61ba0, caller=0x879de9 <xfree+31>) at mcheck.c:193
> #2  in xfree (ptr=0x3a61ba0) at ./common/common-utils.c:108
> #3  in find_and_open_source (filename=0x23c2010 "gdb.c", dirname=0x289cec0 "/home/jkratoch/redhat/gdb-clean/gdb", fullname=0x289cde0) at source.c:1008
> #4  in open_source_file (s=0x289cd80) at source.c:1088
> #5  in tui_set_source_content (s=0x289cd80, line_no=16, noerror=0) at ./tui/tui-source.c:61
> #6  in tui_update_source_window_as_is (win_info=0x3a51a80, gdbarch=0x22d0000, s=0x289cd80, line_or_addr=..., noerror=0) at ./tui/tui-winsource.c:99
> #7  in tui_show_symtab_source (gdbarch=0x22d0000, s=0x289cd80, line=..., noerror=0) at ./tui/tui-source.c:339
> #8  in tui_update_source_windows_with_line (s=0x289cd80, line=16) at ./tui/tui-winsource.c:201
> #9  in tui_show_source (fullname=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c", line=16) at ./tui/tui.c:542
> #10 in tui_field_string (uiout=0x2296dc0, fldno=3, width=0, align=ui_noalign, fldname=0xfa09d9 "fullname", string=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c") at ./tui/tui-out.c:92
> #11 in uo_field_string (uiout=0x2296dc0, fldno=3, width=0, align=ui_noalign, fldname=0xfa09d9 "fullname", string=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c") at ui-out.c:854
> #12 in ui_out_field_string (uiout=0x2296dc0, fldname=0xfa09d9 "fullname", string=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c") at ui-out.c:544
> #13 in print_source_lines_base (s=0x289cd80, line=16, stopline=17, flags=PRINT_SOURCE_LINES_NOERROR) at source.c:1357
> #14 in print_source_lines (s=0x289cd80, line=16, stopline=17, flags=(unknown: 0)) at source.c:1442
> #15 in tui_vertical_source_scroll (scroll_direction=BACKWARD_SCROLL, num_to_scroll=1) at ./tui/tui-source.c:393
> #16 in tui_scroll_backward (win_to_scroll=0x3a51a80, num_to_scroll=1) at ./tui/tui-win.c:538
> #17 in tui_dispatch_ctrl_char (ch=259) at ./tui/tui-command.c:118
> #18 in tui_getc (fp=0x7ffff663b660 <_IO_2_1_stdin_>) at ./tui/tui-io.c:692
> #19 in rl_read_key () at input.c:448
>
>
> I did not expect this may happen...
>
> No regressions on {x86_64,x86_64-m32,i686}-fedora19pre-linux-gnu but it does
> not say much for TUI.
>
>
> Thanks,
> Jan
>
>
> 2013-03-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
> 	* source.c (print_source_lines_base): Make a local copy of
> 	symtab_to_fullname.
>
> diff --git a/gdb/source.c b/gdb/source.c
> index 828d953..201e848 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -1355,11 +1355,17 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
>   				 symtab_to_filename_for_display (s));
>   	  if (ui_out_is_mi_like_p (uiout)
>   	      || !ui_out_test_flags (uiout, ui_source_list))
> -	    {
> -	      const char *fullname = symtab_to_fullname (s);
> + 	    {
> +	      const char *s_fullname = symtab_to_fullname (s);
> +	      char *local_fullname;
>
> -	      ui_out_field_string (uiout, "fullname", fullname);
> -	    }
> +	      /* ui_out_field_string may free S_FULLNAME by calling
> +	         open_source_file for it again.  */
> +	      local_fullname = alloca (strlen (s_fullname) + 1);
> +	      strcpy (local_fullname, s_fullname);
> +
> +	      ui_out_field_string (uiout, "fullname", local_fullname);
> + 	    }
>
>   	  ui_out_text (uiout, "\n");
>   	}
>

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

[Pedro Alves]

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

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

[Pedro Alves]

On 03/13/2013 06:55 PM, Jan Kratochvil wrote:
> On Tue, 12 Mar 2013 19:36:02 +0100, Pedro Alves wrote:
>> 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.
> 
> That is the second patch for a regression by me.

Thanks.

> #0  __memset_sse2 () at ../sysdeps/x86_64/memset.S:360
> #1  in freehook (ptr=0x3a61ba0, caller=0x879de9 <xfree+31>) at mcheck.c:193
> #2  in xfree (ptr=0x3a61ba0) at ./common/common-utils.c:108
> #3  in find_and_open_source (filename=0x23c2010 "gdb.c", dirname=0x289cec0 "/home/jkratoch/redhat/gdb-clean/gdb", fullname=0x289cde0) at source.c:1008
> #4  in open_source_file (s=0x289cd80) at source.c:1088
> #5  in tui_set_source_content (s=0x289cd80, line_no=16, noerror=0) at ./tui/tui-source.c:61
> #6  in tui_update_source_window_as_is (win_info=0x3a51a80, gdbarch=0x22d0000, s=0x289cd80, line_or_addr=..., noerror=0) at ./tui/tui-winsource.c:99
> #7  in tui_show_symtab_source (gdbarch=0x22d0000, s=0x289cd80, line=..., noerror=0) at ./tui/tui-source.c:339 
> #8  in tui_update_source_windows_with_line (s=0x289cd80, line=16) at ./tui/tui-winsource.c:201
> #9  in tui_show_source (fullname=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c", line=16) at ./tui/tui.c:542
> #10 in tui_field_string (uiout=0x2296dc0, fldno=3, width=0, align=ui_noalign, fldname=0xfa09d9 "fullname", string=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c") at ./tui/tui-out.c:92
> #11 in uo_field_string (uiout=0x2296dc0, fldno=3, width=0, align=ui_noalign, fldname=0xfa09d9 "fullname", string=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c") at ui-out.c:854
> #12 in ui_out_field_string (uiout=0x2296dc0, fldname=0xfa09d9 "fullname", string=0x3a61ba0 "\225\225\225\225\225\225\225\225ratoch/redhat/gdb-clean/gdb/gdb.c") at ui-out.c:544
> #13 in print_source_lines_base (s=0x289cd80, line=16, stopline=17, flags=PRINT_SOURCE_LINES_NOERROR) at source.c:1357
> #14 in print_source_lines (s=0x289cd80, line=16, stopline=17, flags=(unknown: 0)) at source.c:1442
> #15 in tui_vertical_source_scroll (scroll_direction=BACKWARD_SCROLL, num_to_scroll=1) at ./tui/tui-source.c:393
> #16 in tui_scroll_backward (win_to_scroll=0x3a51a80, num_to_scroll=1) at ./tui/tui-win.c:538
> #17 in tui_dispatch_ctrl_char (ch=259) at ./tui/tui-command.c:118
> #18 in tui_getc (fp=0x7ffff663b660 <_IO_2_1_stdin_>) at ./tui/tui-io.c:692
> #19 in rl_read_key () at input.c:448
> 
> 
> I did not expect this may happen...

Took me a bit to realize why this was a problem, and
why the copy fixed it.  For the archives, the issue is
here:

void
tui_show_source (const char *fullname, int line)
{
  struct symtab_and_line cursal = get_current_source_symtab_and_line ();

  /* Make sure that the source window is displayed.  */
  tui_add_win_to_layout (SRC_WIN);

  tui_update_source_windows_with_line (cursal.symtab, line);
  tui_update_locator_fullname (fullname);
}

Where tui_update_locator_fullname is using a dangling
pointer -- FULLNAME ends up released by
tui_update_source_windows_with_line, as seen in the backtrace above.

> 2013-03-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* source.c (print_source_lines_base): Make a local copy of
> 	symtab_to_fullname.

This is fine with me.

> @@ -1355,11 +1355,17 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
>  				 symtab_to_filename_for_display (s));
>  	  if (ui_out_is_mi_like_p (uiout)
>  	      || !ui_out_test_flags (uiout, ui_source_list))
> -	    {
> -	      const char *fullname = symtab_to_fullname (s);
> + 	    {
> +	      const char *s_fullname = symtab_to_fullname (s);
> +	      char *local_fullname;
>  
> -	      ui_out_field_string (uiout, "fullname", fullname);
> -	    }
> +	      /* ui_out_field_string may free S_FULLNAME by calling
> +	         open_source_file for it again.  */

I wouldn't mind a mention of the TUI here to make future
readers a bit less surprised:

	      /* ui_out_field_string may free S_FULLNAME by calling
	         open_source_file for it again.  See e.g.,
                 tui_field_string->tui_show_source.  */


> +	      local_fullname = alloca (strlen (s_fullname) + 1);
> +	      strcpy (local_fullname, s_fullname);
> +
> +	      ui_out_field_string (uiout, "fullname", local_fullname);
> + 	    }
>  
>  	  ui_out_text (uiout, "\n");
>  	}

-- 
Pedro Alves

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

[Jan Kratochvil]

On Thu, 14 Mar 2013 13:33:15 +0100, Pedro Alves wrote:
> On 03/13/2013 06:54 PM, Jan Kratochvil wrote:
> > 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.

When I investigate it I agree there is no accidental output, I thought it is:

print_source_lines_base:
         ui_out_field_int (uiout, "line", line);
         ui_out_text (uiout, "\tin ");
         ui_out_field_string (uiout, "file",
                              symtab_to_filename_for_display (s));

and TUI:
  tui_ui_out_impl.text = tui_text;
tui_text:
  if (data->line > 0)
    {
      if (strchr (string, '\n') != 0)
        {
          data->line = -1;
          data->start_of_line = 0;
        }
      return;
    }

That "\tin " is discarded thanks to that 'return' and 'data->line > 0'.

This was the mistake I made in the patch...


> > -	  /* 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/

done


> I'm wondering about whether this "fullname" handling gross-ness in the
> TUI is really necessary.

Probably not but I was fixing absolute pathnames across GDB, not fixing TUI.
I am still not done with the absolute pathnames task.


Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-03/msg00133.html
and for 7.6:
	http://sourceware.org/ml/gdb-cvs/2013-03/msg00134.html


Thanks,
Jan

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

[Jan Kratochvil]

On Thu, 14 Mar 2013 13:53:16 +0100, Pedro Alves wrote:
> I wouldn't mind a mention of the TUI here to make future
> readers a bit less surprised:
> 
> 	      /* ui_out_field_string may free S_FULLNAME by calling
> 	         open_source_file for it again.  See e.g.,
>                  tui_field_string->tui_show_source.  */

done


Checked in:
	http://sourceware.org/ml/gdb-cvs/2013-03/msg00135.html
and for 7.6:
	http://sourceware.org/ml/gdb-cvs/2013-03/msg00136.html


Thanks,
Jan

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

[Pedro Alves]

On 03/14/2013 02:40 PM, Jan Kratochvil wrote:
> 
>> > I'm wondering about whether this "fullname" handling gross-ness in the
>> > TUI is really necessary.
>> > (...)
>> > Haven't tried that yet (for >7.6) ...

> Probably not but I was fixing absolute pathnames across GDB, not fixing TUI.
> I am still not done with the absolute pathnames task.

I did not suggest you do it yourself.

> Checked in:
> 	http://sourceware.org/ml/gdb-cvs/2013-03/msg00133.html
> and for 7.6:
> 	http://sourceware.org/ml/gdb-cvs/2013-03/msg00134.html

Thanks.

-- 
Pedro Alves