diff options
author | Tom Tromey <tom@tromey.com> | 2025-06-13 12:54:16 -0600 |
---|---|---|
committer | Tom Tromey <tom@tromey.com> | 2025-07-01 15:59:41 -0600 |
commit | 7b18593a9ef94694e2a16ac25133aa8c19dc6a87 (patch) | |
tree | 39ea1de8d6a15125c6def31548cbbff359fd6b3a /gdb | |
parent | b054ff604253af016657d5c93e2f69dab14cc53a (diff) | |
download | binutils-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.exp | 48 | ||||
-rw-r--r-- | gdb/testsuite/gdb.tui/esc-match.py | 26 | ||||
-rw-r--r-- | gdb/tui/tui-file.c | 6 | ||||
-rw-r--r-- | gdb/tui/tui-file.h | 11 | ||||
-rw-r--r-- | gdb/ui-file.c | 64 | ||||
-rw-r--r-- | gdb/ui-file.h | 46 | ||||
-rw-r--r-- | gdb/ui-style.c | 43 | ||||
-rw-r--r-- | gdb/ui-style.h | 26 | ||||
-rw-r--r-- | gdb/utils.c | 2 |
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 ()); } |