[PATCH] Fix handling of terminal escape sequences in TUI
Tom Tromey
tom@tromey.com
Sat Jun 14 16:02:48 GMT 2025
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
---
gdb/source.c | 3 +-
gdb/testsuite/gdb.tui/esc-match.exp | 48 +++++++++++++++++++++
gdb/testsuite/gdb.tui/esc-match.py | 26 ++++++++++++
gdb/tui/tui-file.c | 6 +--
gdb/tui/tui-file.h | 11 +++--
gdb/tui/tui-winsource.c | 6 ++-
gdb/ui-file.c | 66 +++++++++++++++++++++++++++--
gdb/ui-file.h | 46 +++++++++++++++++---
gdb/ui-style.c | 43 +++++++++++++------
gdb/ui-style.h | 23 +++++++---
gdb/utils.c | 5 ++-
11 files changed, 245 insertions(+), 38 deletions(-)
create mode 100644 gdb/testsuite/gdb.tui/esc-match.exp
create mode 100644 gdb/testsuite/gdb.tui/esc-match.py
diff --git a/gdb/source.c b/gdb/source.c
index 13cb8d6de8a..59627d1b734 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1428,7 +1428,8 @@ print_source_lines_base (struct symtab *s, int line, int stopline,
int skip_bytes;
char c = *iter;
- if (c == '\033' && skip_ansi_escape (iter, &skip_bytes))
+ if (c == '\033' && (examine_ansi_escape (iter, &skip_bytes)
+ == ansi_escape_result::MATCHED))
iter += skip_bytes;
else if (c >= 0 && c < 040 && c != '\t')
break;
diff --git a/gdb/testsuite/gdb.tui/esc-match.exp b/gdb/testsuite/gdb.tui/esc-match.exp
new file mode 100644
index 00000000000..db78ebe352f
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/esc-match.exp
@@ -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"
diff --git a/gdb/testsuite/gdb.tui/esc-match.py b/gdb/testsuite/gdb.tui/esc-match.py
new file mode 100644
index 00000000000..78160020145
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/esc-match.py
@@ -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)
diff --git a/gdb/tui/tui-file.c b/gdb/tui/tui-file.c
index 39aee9f32e2..df6f503e283 100644
--- a/gdb/tui/tui-file.c
+++ b/gdb/tui/tui-file.c
@@ -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 ();
}
diff --git a/gdb/tui/tui-file.h b/gdb/tui/tui-file.h
index dbd6fa91da5..b6dc0587f33 100644
--- a/gdb/tui/tui-file.h
+++ b/gdb/tui/tui-file.h
@@ -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. */
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index a545c4870e1..f6f3560dc18 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -79,7 +79,9 @@ tui_copy_source_line (const char **ptr, int *length)
int skip_bytes;
c = *lineptr;
- if (c == '\033' && skip_ansi_escape (lineptr, &skip_bytes))
+ if (c == '\033'
+ && (examine_ansi_escape (lineptr, &skip_bytes)
+ == ansi_escape_result::MATCHED))
{
/* We always have to preserve escapes. */
result.append (lineptr, lineptr + skip_bytes);
@@ -271,7 +273,7 @@ tui_source_window_base::puts_to_pad_with_skip (const char *string, int skip)
gdb_assert (*next == '\033');
int n_read;
- if (skip_ansi_escape (next, &n_read))
+ if (examine_ansi_escape (next, &n_read) == ansi_escape_result::MATCHED)
{
std::string copy (next, n_read);
tui_puts (copy.c_str (), w);
diff --git a/gdb/ui-file.c b/gdb/ui-file.c
index f86b6b171f2..fae27932daa 100644
--- a/gdb/ui-file.c
+++ b/gdb/ui-file.c
@@ -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,16 +417,69 @@ 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;
- if (!skip_ansi_escape (esc, &n_read))
+ 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')
+ {
+ const char *esc = strchr (buf, '\033');
+ if (esc == nullptr)
+ break;
+
+ int n_read = 0;
+ if (examine_ansi_escape (esc, &n_read) != ansi_escape_result::MATCHED)
++esc;
this->stdio_file::write (buf, esc - buf);
@@ -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)
{
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 3919e528da9..1219bde0a75 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -362,24 +362,58 @@ class tee_file : public ui_file
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. */
diff --git a/gdb/ui-style.c b/gdb/ui-style.c
index b8d73abdc87..c8f2e11759a 100644
--- a/gdb/ui-style.c
+++ b/gdb/ui-style.c
@@ -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. */
diff --git a/gdb/ui-style.h b/gdb/ui-style.h
index 77a175d73ed..bfa1602a350 100644
--- a/gdb/ui-style.h
+++ b/gdb/ui-style.h
@@ -354,11 +354,24 @@ struct ui_file_style
bool m_reverse = false;
};
-/* 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. */
+/* 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 OK is returned, then N_READ is updated to reflect
+ the number of chars read from BUF. */
-extern bool skip_ansi_escape (const char *buf, int *n_read);
+extern ansi_escape_result examine_ansi_escape (const char *buf, int *n_read);
#endif /* GDB_UI_STYLE_H */
diff --git a/gdb/utils.c b/gdb/utils.c
index 4f48e15b7eb..7c4ff628192 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1680,7 +1680,8 @@ pager_file::puts (const char *linebuffer)
linebuffer++;
}
else if (*linebuffer == '\033'
- && skip_ansi_escape (linebuffer, &skip_bytes))
+ && (examine_ansi_escape (linebuffer, &skip_bytes)
+ == ansi_escape_result::MATCHED))
{
m_wrap_buffer.append (linebuffer, skip_bytes);
/* Note that we don't consider this a character, so we
@@ -1788,7 +1789,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 ());
}
--
2.49.0
More information about the Gdb-patches
mailing list