aboutsummaryrefslogtreecommitdiff
path: root/gdb
diff options
context:
space:
mode:
authorTom Tromey <tom@tromey.com>2025-06-13 12:54:16 -0600
committerTom Tromey <tom@tromey.com>2025-07-01 15:59:41 -0600
commit7b18593a9ef94694e2a16ac25133aa8c19dc6a87 (patch)
tree39ea1de8d6a15125c6def31548cbbff359fd6b3a /gdb
parentb054ff604253af016657d5c93e2f69dab14cc53a (diff)
downloadbinutils-7b18593a9ef94694e2a16ac25133aa8c19dc6a87.zip
binutils-7b18593a9ef94694e2a16ac25133aa8c19dc6a87.tar.gz
binutils-7b18593a9ef94694e2a16ac25133aa8c19dc6a87.tar.bz2
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>
Diffstat (limited to 'gdb')
-rw-r--r--gdb/testsuite/gdb.tui/esc-match.exp48
-rw-r--r--gdb/testsuite/gdb.tui/esc-match.py26
-rw-r--r--gdb/tui/tui-file.c6
-rw-r--r--gdb/tui/tui-file.h11
-rw-r--r--gdb/ui-file.c64
-rw-r--r--gdb/ui-file.h46
-rw-r--r--gdb/ui-style.c43
-rw-r--r--gdb/ui-style.h26
-rw-r--r--gdb/utils.c2
9 files changed, 243 insertions, 29 deletions
diff --git a/gdb/testsuite/gdb.tui/esc-match.exp b/gdb/testsuite/gdb.tui/esc-match.exp
new file mode 100644
index 0000000..db78ebe
--- /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 0000000..7816002
--- /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 39aee9f..df6f503 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 dbd6fa9..b6dc058 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/ui-file.c b/gdb/ui-file.c
index f86b6b1..2e5c06f 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,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')
{
@@ -438,6 +491,13 @@ no_terminal_escape_file::puts (const char *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)
{
if (debug_timestamp)
diff --git a/gdb/ui-file.h b/gdb/ui-file.h
index 3919e52..1219bde 100644
--- a/gdb/ui-file.h
+++ b/gdb/ui-file.h
@@ -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. */
diff --git a/gdb/ui-style.c b/gdb/ui-style.c
index b8d73ab..c8f2e11 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 77a175d..f61152f 100644
--- a/gdb/ui-style.h
+++ b/gdb/ui-style.h
@@ -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 */
diff --git a/gdb/utils.c b/gdb/utils.c
index 3114a4b..10d3d51 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -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 ());
}