Fix handling of terminal escape sequences in TUI

A user noticed that if the remote sends terminal escape sequences from
the "monitor" command, then these will not be correctly displayed when
in TUI mode.

I tracked this down to remote.c emitting one character at a time --
something the TUI output functions did not handle correctly.

I decided in the end to fix in this in the ui-file layer, because the
same bug seems to affect logging and, as is evidenced by the test case
in this patch, Python output in TUI mode.

The idea is simple: buffer escape sequences until they are either
complete or cannot possibly be recognized by gdb.

Regression tested on x86-64 Fedora 40.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=14126
Approved-By: Andrew Burgess <aburgess@redhat.com>
This commit is contained in:
Tom Tromey 2025-06-13 12:54:16 -06:00
parent b054ff6042
commit 7b18593a9e
9 changed files with 243 additions and 29 deletions

View file

@ -0,0 +1,48 @@
# Copyright 2025 Free Software Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# Test that the ANSI escape sequence matcher works
# character-by-character.
load_lib gdb-python.exp
require allow_python_tests allow_tui_tests
tuiterm_env
Term::clean_restart 24 80
set remote_python_file [gdb_remote_download host \
${srcdir}/${subdir}/esc-match.py]
gdb_test_no_output "source ${remote_python_file}" \
"source esc-match.py"
if {![Term::enter_tui]} {
unsupported "TUI not supported"
return
}
Term::command "python print_it()"
Term::dump_screen
set text [Term::get_all_lines]
# We should not see the control sequence here.
gdb_assert {![regexp -- "\\\[35;1mOUTPUT\\\[m" $text]} \
"output visible without control sequences"
# Also check the styling.
set text [Term::get_region 0 1 78 23 "\n" true]
gdb_assert {[regexp -- "<fg:magenta>.*OUTPUT" $text]} \
"output is magenta"

View file

@ -0,0 +1,26 @@
# Copyright (C) 2025 Free Software Foundation, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import sys
# Some text to print that includes styling.
OUT = "\033[35;1mOUTPUT\033[m"
def print_it():
# Print to stderr avoids any buffering, showing the bug.
for c in OUT:
print(c, end="", file=sys.stderr)
print(file=sys.stderr)

View file

@ -21,7 +21,7 @@
#include "tui/tui-command.h"
void
tui_file::puts (const char *linebuffer)
tui_file::do_puts (const char *linebuffer)
{
tui_puts (linebuffer);
if (!m_buffered)
@ -29,7 +29,7 @@ tui_file::puts (const char *linebuffer)
}
void
tui_file::write (const char *buf, long length_buf)
tui_file::do_write (const char *buf, long length_buf)
{
tui_write (buf, length_buf);
if (!m_buffered)
@ -41,5 +41,5 @@ tui_file::flush ()
{
if (m_buffered)
tui_cmd_win ()->refresh_window ();
stdio_file::flush ();
escape_buffering_file::flush ();
}

View file

@ -23,18 +23,21 @@
/* A STDIO-like output stream for the TUI. */
class tui_file : public stdio_file
class tui_file : public escape_buffering_file
{
public:
tui_file (FILE *stream, bool buffered)
: stdio_file (stream),
: escape_buffering_file (stream),
m_buffered (buffered)
{}
void write (const char *buf, long length_buf) override;
void puts (const char *) override;
void flush () override;
protected:
void do_write (const char *buf, long length_buf) override;
void do_puts (const char *) override;
private:
/* True if this stream is buffered. */

View file

@ -408,7 +408,7 @@ tee_file::can_emit_style_escape ()
/* See ui-file.h. */
void
no_terminal_escape_file::write (const char *buf, long length_buf)
escape_buffering_file::write (const char *buf, long length_buf)
{
std::string copy (buf, length_buf);
this->puts (copy.c_str ());
@ -417,7 +417,60 @@ no_terminal_escape_file::write (const char *buf, long length_buf)
/* See ui-file.h. */
void
no_terminal_escape_file::puts (const char *buf)
escape_buffering_file::puts (const char *buf)
{
std::string local_buffer;
if (!m_buffer.empty ())
{
gdb_assert (m_buffer[0] == '\033');
m_buffer += buf;
/* If we need to keep buffering, we'll handle that below. */
local_buffer = std::move (m_buffer);
buf = local_buffer.c_str ();
}
while (*buf != '\0')
{
const char *esc = strchr (buf, '\033');
if (esc == nullptr)
break;
/* First, write out any prefix. */
if (esc > buf)
{
do_write (buf, esc - buf);
buf = esc;
}
int n_read = 0;
ansi_escape_result seen = examine_ansi_escape (esc, &n_read);
if (seen == ansi_escape_result::INCOMPLETE)
{
/* Start buffering. */
m_buffer = buf;
return;
}
else if (seen == ansi_escape_result::NO_MATCH)
{
/* Just emit the ESC . */
n_read = 1;
}
else
gdb_assert (seen == ansi_escape_result::MATCHED);
do_write (esc, n_read);
buf += n_read;
}
/* If there is any data remaining in BUF, we can flush it now. */
if (*buf != '\0')
do_puts (buf);
}
/* See ui-file.h. */
void
no_terminal_escape_file::do_puts (const char *buf)
{
while (*buf != '\0')
{
@ -437,6 +490,13 @@ no_terminal_escape_file::puts (const char *buf)
this->stdio_file::write (buf, strlen (buf));
}
void
no_terminal_escape_file::do_write (const char *buf, long len)
{
std::string copy (buf, len);
do_puts (copy.c_str ());
}
void
timestamped_file::write (const char *buf, long len)
{

View file

@ -362,24 +362,58 @@ private:
ui_file *m_two;
};
/* A ui_file implementation that buffers terminal escape sequences.
Note that this does not buffer in general -- it only buffers when
an incomplete but potentially recognizable escape sequence is
started. */
class escape_buffering_file : public stdio_file
{
public:
using stdio_file::stdio_file;
/* Like the stdio_file methods but these forward to do_write and
do_puts, respectively. */
void write (const char *buf, long length_buf) override final;
void puts (const char *linebuffer) override final;
/* This class does not override 'flush'. While it does have an
internal buffer, it does not really make sense to flush the
buffer until an escape sequence has been fully processed. */
protected:
/* Called to output some text. If the text contains a recognizable
terminal escape sequence, then it is guaranteed to be complete.
"Recognizable" here means that examine_ansi_escape did not return
INCOMPLETE. */
virtual void do_puts (const char *buf) = 0;
virtual void do_write (const char *buf, long len) = 0;
private:
/* Buffer used only for incomplete escape sequences. */
std::string m_buffer;
};
/* A ui_file implementation that filters out terminal escape
sequences. */
class no_terminal_escape_file : public stdio_file
class no_terminal_escape_file : public escape_buffering_file
{
public:
no_terminal_escape_file ()
{
}
/* Like the stdio_file methods, but these filter out terminal escape
sequences. */
void write (const char *buf, long length_buf) override;
void puts (const char *linebuffer) override;
void emit_style_escape (const ui_file_style &style) override
{
}
protected:
void do_puts (const char *linebuffer) override;
void do_write (const char *buf, long len) override;
};
/* A base class for ui_file types that wrap another ui_file. */

View file

@ -21,12 +21,14 @@
#include "gdbsupport/gdb_regex.h"
/* A regular expression that is used for matching ANSI terminal escape
sequences. */
sequences. Note that this will actually match any prefix of such a
sequence. This property is used so that other code can buffer
incomplete sequences as needed. */
static const char ansi_regex_text[] =
/* Introduction. */
"^\033\\["
#define DATA_SUBEXP 1
/* Introduction. Only the escape character is truly required. */
"^\033(\\["
#define DATA_SUBEXP 2
/* Capture parameter and intermediate bytes. */
"("
/* Parameter bytes. */
@ -36,12 +38,12 @@ static const char ansi_regex_text[] =
/* End the first capture. */
")"
/* The final byte. */
#define FINAL_SUBEXP 2
"([\x40-\x7e])";
#define FINAL_SUBEXP 3
"([\x40-\x7e]))?";
/* The number of subexpressions to allocate space for, including the
"0th" whole match subexpression. */
#define NUM_SUBEXPRESSIONS 3
#define NUM_SUBEXPRESSIONS 4
/* The compiled form of ansi_regex_text. */
@ -371,6 +373,15 @@ ui_file_style::parse (const char *buf, size_t *n_read)
*n_read = 0;
return false;
}
/* If the final subexpression did not match, then that means there
was an incomplete sequence. These are ignored here. */
if (subexps[FINAL_SUBEXP].rm_so == -1)
{
*n_read = 0;
return false;
}
/* Other failures mean the regexp is broken. */
gdb_assert (match == 0);
/* The regexp is anchored. */
@ -527,17 +538,25 @@ ui_file_style::parse (const char *buf, size_t *n_read)
/* See ui-style.h. */
bool
skip_ansi_escape (const char *buf, int *n_read)
ansi_escape_result
examine_ansi_escape (const char *buf, int *n_read)
{
gdb_assert (*buf == '\033');
regmatch_t subexps[NUM_SUBEXPRESSIONS];
int match = ansi_regex.exec (buf, ARRAY_SIZE (subexps), subexps, 0);
if (match == REG_NOMATCH || buf[subexps[FINAL_SUBEXP].rm_so] != 'm')
return false;
if (match == REG_NOMATCH)
return ansi_escape_result::NO_MATCH;
if (subexps[FINAL_SUBEXP].rm_so == -1)
return ansi_escape_result::INCOMPLETE;
if (buf[subexps[FINAL_SUBEXP].rm_so] != 'm')
return ansi_escape_result::NO_MATCH;
*n_read = subexps[FINAL_SUBEXP].rm_eo;
return true;
return ansi_escape_result::MATCHED;
}
/* See ui-style.h. */

View file

@ -354,11 +354,35 @@ private:
bool m_reverse = false;
};
/* Possible results for checking an ANSI escape sequence. */
enum class ansi_escape_result
{
/* The escape sequence is definitely not recognizable. */
NO_MATCH,
/* The escape sequence might be recognizable with more input. */
INCOMPLETE,
/* The escape sequence is definitely recognizable. */
MATCHED,
};
/* Examine an ANSI escape sequence in BUF. BUF must begin with an ESC
character. Return a value indicating whether the sequence was
recognizable. If MATCHED is returned, then N_READ is updated to
reflect the number of chars read from BUF. */
extern ansi_escape_result examine_ansi_escape (const char *buf, int *n_read);
/* Skip an ANSI escape sequence in BUF. BUF must begin with an ESC
character. Return true if an escape sequence was successfully
skipped; false otherwise. If an escape sequence was skipped,
N_READ is updated to reflect the number of chars read from BUF. */
extern bool skip_ansi_escape (const char *buf, int *n_read);
static inline bool
skip_ansi_escape (const char *buf, int *n_read)
{
return examine_ansi_escape (buf, n_read) == ansi_escape_result::MATCHED;
}
#endif /* GDB_UI_STYLE_H */

View file

@ -1809,7 +1809,7 @@ void
pager_file::write (const char *buf, long length_buf)
{
/* We have to make a string here because the pager uses
skip_ansi_escape, which requires NUL-termination. */
examine_ansi_escape, which requires NUL-termination. */
std::string str (buf, length_buf);
this->puts (str.c_str ());
}