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

[Hui Zhu <hui_zhu@mentor.com>]

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");
>   	}
>
>