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