Re: [PATCHv7 1/4] gdb: allow gdb.Color to work correctly with pagination

Tom de Vries
2025-08-25T10:22:09Z

On 8/24/25 18:13, Andrew Burgess wrote:
> Andrew Burgess <aburgess@redhat.com> writes:
>
>> This commit allows gdb.Color objects to be used to style output from
>> GDB commands written in Python, and the styled output should work
>> correctly with pagination.
>
> I've gone ahead and pushed this fix.
>

The new test-case fails for me on aarch64-linux. I've filed a PR (
https://sourceware.org/bugzilla/show_bug.cgi?id=33321 ) to track this.

Thanks,
- Tom

> I believe that the change I've made here is inline with how the same
> problem is handled in the TUI, parsing the escape sequence and then
> using the resulting ui_file_style object. And the second part of the
> fix, changing gdb_printf to gdb_puts falls out from making the first
> change.
>
> Given that the gdb.Color API was added since GDB 16, it seems sensible
> to get this bug fixed before GDB 17 branches.
>
> The other patches in this series I've not pushed, pending review.
>
> Thanks,
> Andrew
>
>
>
>>
>> There are two parts to fixing this:
>>
>> First, GDB needs to be able to track the currently applied style
>> within the page_file class. This means that style changes need to be
>> achieved with calls to pager_file::emit_style_escape.
>>
>> Now usually, GDB does this by calling something like fprintf_styled,
>> which takes care to apply the style for us. However, that's not
>> really an option here as a gdb.Color isn't a full style, and as the
>> gdb.Color object is designed to be converted directly into escape
>> sequences that can then be printed, we really need a solution that
>> works with this approach.
>>
>> However pager_file::puts already has code in place to handle escape
>> sequences. Right now all this code does is spot the escape sequence
>> and append it to the m_wrap_buffer. But in this commit I propose that
>> we go one step further, parse the escape sequence back into a
>> ui_file_style object in pager_file::puts, and then we can call
>> pager_file::emit_style_escape.
>>
>> If the parsing doesn't work then we can just add the escape sequence
>> to m_wrap_buffer as we did before.
>>
>> But wait, how can this work if a gdb.Color isn't a full style? Turns
>> out that's not a problem. We only ever emit the escape sequence for
>> those parts of a style that need changing, so a full style that sets
>> the foreground color will emit the same escape sequence as a gdb.Color
>> for the foreground. When we convert the escape sequence back into a
>> ui_file_style, then we get a style with everything set to default,
>> except the foreground color.
>>
>> I had hoped that this would be all that was needed. But unfortunately
>> this doesn't work because of the second problem...
>>
>> ... the implementation of the Python function gdb.write() calls
>> gdb_printf(), which calls gdb_vprintf(), which calls ui_file::vprintf,
>> which calls ui_out::vmessage, which calls ui_out::call_do_message, and
>> finally we reach cli_ui_out::do_message. This final do_message
>> function does this:
>>
>> ui_file *stream = m_streams.back ();
>> stream->emit_style_escape (style);
>> stream->puts (str.c_str ());
>> stream->emit_style_escape (ui_file_style ());
>>
>> If we imagine the case where we are emitting a style, triggered from
>> Python like this:
>>
>> gdb.write(gdb.Color('red').escape_sequence(True))
>>
>> the STYLE in this case will be the default ui_file_style(), and STR
>> will hold the escape sequence we are writing.
>>
>> After the first change, where pager_file::puts now calls
>> pager_file::emit_style_escape, the current style of STREAM will have
>> been updated. But this means that the final emit_style_escape will
>> now restore the default style.
>>
>> The fix for this is to avoid using the high level gdb_printf from
>> gdb.write(), and instead use gdb_puts instead. The gdb_puts function
>> doesn't restore the default style, which means our style modification
>> survives.
>>
>> There's a new test included. This test includes what appears like a
>> pointless extra loop (looping over a single value), but later commits
>> in this series will add more values to this list.
>> ---
>> gdb/python/python.c | 18 +--
>> .../gdb.python/py-color-pagination.exp | 122 ++++++++++++++++++
>> .../gdb.python/py-color-pagination.py | 46 +++++++
>> gdb/utils.c | 21 ++-
>> 4 files changed, 195 insertions(+), 12 deletions(-)
>> create mode 100644 gdb/testsuite/gdb.python/py-color-pagination.exp
>> create mode 100644 gdb/testsuite/gdb.python/py-color-pagination.py
>>
>> diff --git a/gdb/python/python.c b/gdb/python/python.c
>> index cb0d642a67d..1af7896eb08 100644
>> --- a/gdb/python/python.c
>> +++ b/gdb/python/python.c
>> @@ -1570,21 +1570,21 @@ gdbpy_write (PyObject *self, PyObject *args, PyObject *kw)
>>
>> try
>> {
>> + ui_file *stream;
>> switch (stream_type)
>> {
>> case 1:
>> - {
>> - gdb_printf (gdb_stderr, "%s", arg);
>> - break;
>> - }
>> + stream = gdb_stderr;
>> + break;
>> case 2:
>> - {
>> - gdb_printf (gdb_stdlog, "%s", arg);
>> - break;
>> - }
>> + stream = gdb_stdlog;
>> + break;
>> default:
>> - gdb_printf (gdb_stdout, "%s", arg);
>> + stream = gdb_stdout;
>> + break;
>> }
>> +
>> + gdb_puts (arg, stream);
>> }
>> catch (const gdb_exception &except)
>> {
>> diff --git a/gdb/testsuite/gdb.python/py-color-pagination.exp b/gdb/testsuite/gdb.python/py-color-pagination.exp
>> new file mode 100644
>> index 00000000000..3235fffe5cc
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.python/py-color-pagination.exp
>> @@ -0,0 +1,122 @@
>> +# 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/>.
>> +
>> +# This file is part of the GDB testsuite. It tests gdb.Color and how this
>> +# interacts with GDB's pagination system.
>> +
>> +load_lib gdb-python.exp
>> +
>> +require allow_python_tests
>> +
>> +standard_testfile
>> +
>> +set pyfile [gdb_remote_download host ${srcdir}/${subdir}/${testfile}.py]
>> +
>> +set str "<[string repeat - 78]>"
>> +
>> +# These define all the default attributes for a style: background
>> +# color, intensity, italics, and underlined.
>> +set other_attr ";49;22;23;24;27"
>> +
>> +# These colors set the foreground color only. Everything else is the
>> +# default.
>> +set black "(?:\033\\\[30${other_attr}m)"
>> +set red "(?:\033\\\[31${other_attr}m)"
>> +set green "(?:\033\\\[32${other_attr}m)"
>> +set yellow "(?:\033\\\[33${other_attr}m)"
>> +set blue "(?:\033\\\[34${other_attr}m)"
>> +set magenta "(?:\033\\\[35${other_attr}m)"
>> +set cyan "(?:\033\\\[36${other_attr}m)"
>> +set white "(?:\033\\\[37${other_attr}m)"
>> +
>> +set any_color "(?:${black}|${red}|${green}|${yellow}|${blue}|${magenta}|${cyan}|${white})"
>> +
>> +# Run the command 'TYPE-fill MODE' which fills the screen with output and
>> +# triggers the pagination prompt. Check that styling is applied correctly
>> +# to the output.
>> +proc test_pagination { type mode } {
>> +
>> + # Start with a fresh GDB, but enable color support.
>> + with_ansi_styling_terminal {
>> + clean_restart
>> + }
>> +
>> + gdb_test_no_output "source $::pyfile" "source the script"
>> +
>> + gdb_test_no_output "set width 80"
>> + gdb_test_no_output "set height 15"
>> +
>> + set saw_bad_color_handling false
>> + set expected_restore_color ""
>> + set last_color ""
>> + gdb_test_multiple "$type-fill $mode" "" {
>> + -re "^$type-fill $mode\r\n" {
>> + exp_continue
>> + }
>> +
>> + -re "^(${::any_color}?)(${::any_color})$::str" {
>> + # After a continuation prompt GDB will restore the previous
>> + # color, and then we immediately switch to a new color.
>> + set restored_color $expect_out(1,string)
>> + if { $restored_color ne ""
>> + && $restored_color ne $expected_restore_color } {
>> + set saw_bad_color_handling true
>> + }
>> + set last_color $expect_out(2,string)
>> + exp_continue
>> + }
>> +
>> + -re "^\033\\\[${::decimal}m$::str" {
>> + # This catches the case where the color's escape sequence has
>> + # not been converted back into a full style. This indicates
>> + # something went wrong in the pager_file::puts function.
>> + set saw_bad_color_handling true
>> + exp_continue
>> + }
>> +
>> + -re "^((?:\033\\\[m)?)$::pagination_prompt$" {
>> + # After a pagination prompt we expect GDB to restore the last
>> + # color.
>> + set expected_restore_color $last_color
>> +
>> + # If we didn't see a color reset sequence then the pagination
>> + # prompt will have been printed in the wrong color, this is a
>> + # GDB bug.
>> + set color_reset $expect_out(1,string)
>> + if { $color_reset eq "" } {
>> + set saw_bad_color_handling true
>> + }
>> +
>> + # Send '\n' to view more output.
>> + send_gdb "\n"
>> + exp_continue
>> + }
>> +
>> + -re "^\r\n" {
>> + # The matches the newline sent to the continuation prompt.
>> + exp_continue
>> + }
>> +
>> + -re "^\033\\\[m\r\n$::gdb_prompt $" {
>> + gdb_assert { !$saw_bad_color_handling } $gdb_test_name
>> + }
>> + }
>> +}
>> +
>> +foreach_with_prefix type { color } {
>> + foreach_with_prefix mode { write print } {
>> + test_pagination $type $mode
>> + }
>> +}
>> diff --git a/gdb/testsuite/gdb.python/py-color-pagination.py b/gdb/testsuite/gdb.python/py-color-pagination.py
>> new file mode 100644
>> index 00000000000..efd501eedf5
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.python/py-color-pagination.py
>> @@ -0,0 +1,46 @@
>> +# 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 gdb
>> +
>> +basic_colors = ["black", "red", "green", "yellow", "blue", "magenta", "cyan", "white"]
>> +
>> +
>> +def write(mode, text):
>> + if mode == "write":
>> + gdb.write(text)
>> + else:
>> + print(text, end="")
>> +
>> +
>> +class ColorTester(gdb.Command):
>> + def __init__(self):
>> + super().__init__("color-fill", gdb.COMMAND_USER)
>> +
>> + def invoke(self, args, from_tty):
>> + mode = args
>> + str = "<" + "-" * 78 + ">"
>> + for i in range(0, 20):
>> + for color_name in basic_colors:
>> + c = gdb.Color(color_name)
>> + write(mode, c.escape_sequence(True))
>> + write(mode, str)
>> +
>> + default = gdb.Color("none")
>> + write(mode, default.escape_sequence(True))
>> + write(mode, "\n")
>> +
>> +
>> +ColorTester()
>> diff --git a/gdb/utils.c b/gdb/utils.c
>> index 10d3d51e481..92e626a9c75 100644
>> --- a/gdb/utils.c
>> +++ b/gdb/utils.c
>> @@ -1702,10 +1702,25 @@ pager_file::puts (const char *linebuffer)
>> else if (*linebuffer == '\033'
>> && skip_ansi_escape (linebuffer, &skip_bytes))
>> {
>> - m_wrap_buffer.append (linebuffer, skip_bytes);
>> - /* Note that we don't consider this a character, so we
>> + /* We don't consider escape sequences as characters, so we
>> don't increment chars_printed here. */
>> - linebuffer += skip_bytes;
>> +
>> + size_t style_len;
>> + ui_file_style style;
>> + if (style.parse (linebuffer, &style_len)
>> + && style_len <= skip_bytes)
>> + {
>> + this->emit_style_escape (style);
>> +
>> + linebuffer += style_len;
>> + skip_bytes -= style_len;
>> + }
>> +
>> + if (skip_bytes > 0)
>> + {
>> + m_wrap_buffer.append (linebuffer, skip_bytes);
>> + linebuffer += skip_bytes;
>> + }
>> }
>> else if (*linebuffer == '\r')
>> {
>> --
>> 2.47.1
>