MatCat.OpenSource/8sa1-binutils-gdb
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

gdb: don't print escape characters when a style is disabled

Browse Source 
While working on another patch I noticed that if I disable a single
style with, for example:

  set style filename background none
  set style filename foreground none
  set style filename intensity normal

Then in some places escape characters are still injected into the
output stream.  This is a bit of an edge case, and I can't think when
this would actually cause problems, but it still felt like a bit of an
annoyance.

One place where this does impact is in testing, where it becomes
harder to write tight test patterns if it is not obvious when GDB will
decide to inject escape sequences.

It's especially annoying because depending on how something is printed
then GDB might, or might not, add escape characters.  So this would
not add escape characters if the filename style was disabled:

  fprintf_filtered (file, "%ps",
                    styled_string (file_name_style.style (),
                                   "This is a test"));

But this would add escape characters:

  fprintf_styled (file, file_name_style.style (),
                  "%s", "This is a test");

I tracked this down to some calls to set_output_style in utils.c.
Currently some calls to set_output_style (in utils.c) are guarded like
this:

  if (!STYLE.is_default ())
    set_output_style (stream, STYLE);

But some calls are not.  It is the calls that are NOT guarded that
cause the extra escape sequences to be emitted.

My initial proposal to resolve this issue was simply to ensure that
all calls to set_output_style were guarded.  The patch I posted for
this can be found here:

  https://sourceware.org/pipermail/gdb-patches/2021-January/175096.html

The feedback on this proposal was that it might be better to guard
against the escape sequences being emitted at a later lever, right
down at emit_style_escape.

So this is what this version does.  In emit_style_escape we already
track the currently applied style, so if the style we are being asked
to switch to is the same as the currently applied style then no escape
sequence needs to be emitted.

Making this change immediately exposed some issues in
fputs_maybe_filtered related to line wrapping.  The best place to start
to understand what's going on with the styling and wrapping is look at
the test:

  gdb.base/style.exp: all styles enabled: frame when width=20

If you run this test and then examine the output in an editor so the
escape sequences can be seen you'll see the duplicate escape sequences
that are emitted before this patch, the compare to after this patch
you'll see the set of escape sequences should be the minimum required.

In order to test these changes I have rewritten the gdb.base/style.exp
test script.  The core of the script is now run multiple times.  The
first time the test is run things are as they were before, all styles
are on.

After that the test is rerun multiple times.  Each time through a
single style is disabled using the 3 explicit set calls listed above.
I then repeat all the tests, however, I arrange so that the patterns
for the disabled style now require no escape sequences.

gdb/ChangeLog:

	* utils.c (emit_style_escape): Only emit an escape sequence if the
	requested style is different than the current applied style.
	(fputs_maybe_filtered): Adjust the juggling of the wrap_style, and
	current applied_style.
	(fputs_styled): Remove is_default check.
	(fputs_styled_unfiltered): Likewise.
	(vfprintf_styled_no_gdbfmt): Likewise.

gdb/testsuite/ChangeLog:

	* gdb.base/style.exp (limited_style): New proc.
	(clean_restart_and_disable): New proc.
	(run_style_tests): New proc.  Most of the old tests from this file
	are now in this proc.
	(test_startup_version_string): New proc.  Reamining test from the
	old file is in this proc.
This commit is contained in:
Andrew Burgess 2021-01-13 20:08:42 +00:00
parent d8c4766d31
commit e7b430724d
4 changed files with 320 additions and 213 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
gdb/ChangeLog
View File

@ -1,3 +1,13 @@
2021-01-22 Andrew Burgess <andrew.burgess@embecosm.com>
* utils.c (emit_style_escape): Only emit an escape sequence if the
requested style is different than the current applied style.
(fputs_maybe_filtered): Adjust the juggling of the wrap_style, and
current applied_style.
(fputs_styled): Remove is_default check.
(fputs_styled_unfiltered): Likewise.
(vfprintf_styled_no_gdbfmt): Likewise.
2021-01-22 Simon Marchi <simon.marchi@polymtl.ca>
* remote.h (remote_debug_printf): New.

9
gdb/testsuite/ChangeLog
View File

@ -1,3 +1,12 @@
2021-01-22 Andrew Burgess <andrew.burgess@embecosm.com>
* gdb.base/style.exp (limited_style): New proc.
(clean_restart_and_disable): New proc.
(run_style_tests): New proc. Most of the old tests from this file
are now in this proc.
(test_startup_version_string): New proc. Reamining test from the
old file is in this proc.
2021-01-22 Simon Marchi <simon.marchi@polymtl.ca>
* lib/range-stepping-support.exp (exec_cmd_expect_vCont_count):

441
gdb/testsuite/gdb.base/style.exp
View File

@ -17,203 +17,290 @@
standard_testfile
save_vars { env(TERM) } {
# We need an ANSI-capable terminal to get the output.
setenv TERM ansi
# Compile the test executable.
set test_macros 0
set options debug
get_compiler_info
if { [test_compiler_info "gcc-*"] || [test_compiler_info "clang-*"] } {
lappend options additional_flags=-g3
set test_macros 1
}
set test_macros 0
set options debug
get_compiler_info
if { [test_compiler_info "gcc-*"] || [test_compiler_info "clang-*"] } {
lappend options additional_flags=-g3
set test_macros 1
if {[build_executable "failed to build" $testfile $srcfile $options]} {
return -1
}
# The tests in this file are run multiple times with GDB's styles
# disabled one at a time. This variable is the style that is
# currently disabled.
set currently_disabled_style ""
# A wrapper around the 'style' function found in gdb-utils.exp,
# filter out requests for the disabled style.
proc limited_style { str style } {
global currently_disabled_style
if { $style != $currently_disabled_style } {
return [style $str $style]
}
if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
$options]} {
return -1
return $str
}
# A wrapper around 'clean_restart' from gdb.exp, this performs the
# normal clean_restart, but then disables the currently disabled
# style.
proc clean_restart_and_disable { args } {
global currently_disabled_style
eval "clean_restart $args"
if { $currently_disabled_style != "" } {
set st $currently_disabled_style
gdb_test_no_output "set style $st background none" ""
gdb_test_no_output "set style $st foreground none" ""
gdb_test_no_output "set style $st intensity normal" ""
}
}
set readnow [readnow]
# The core of this test script. Run some tests of different aspects
# of GDB's styling.
#
# Within this proc always use LIMITED_STYLE instead of STYLE, and
# CLEAN_RESTART_AND_DISABLE instead of CLEAN_RESTART, this ensures
# that the test operates as expected as styles are disabled.
proc run_style_tests { } {
global testfile srcfile hex binfile test_macros
global currently_disabled_style decimal hex
if {![runto_main]} {
fail "style tests failed"
return
}
save_vars { env(TERM) } {
# We need an ANSI-capable terminal to get the output.
setenv TERM ansi
# Check that the source highlighter has not stripped away the leading
# newlines.
set main_line [gdb_get_line_number "break here"]
gdb_test "list $main_line,$main_line" "return.*some_called_function.*"
# Restart GDB with the correct TERM variable setting, this
# means that GDB will enable styling.
clean_restart_and_disable ${binfile}
gdb_test_no_output "set style enabled off"
set readnow [readnow]
set argv ""
gdb_test_multiple "frame" "frame without styling" {
-re -wrap "main \\(argc=.*, (argv=$hex)\\).*style\\.c:\[0-9\].*" {
set argv $expect_out(1,string)
pass $gdb_test_name
}
}
gdb_test_no_output "set style enabled on"
set main_expr [style main function]
set base_file_expr [style ".*style\\.c" file]
set file_expr "$base_file_expr:\[0-9\]"
set arg_expr [style "arg." variable]
gdb_test "frame" \
"$main_expr.*$arg_expr.*$arg_expr.*$file_expr.*"
gdb_test "info breakpoints" "$main_expr at $file_expr.*"
gdb_test_no_output "set style sources off"
gdb_test "frame" \
"\r\n\[^\033\]*break here.*" \
"frame without sources styling"
gdb_test_no_output "set style sources on"
gdb_test "break -q main" "file $base_file_expr.*"
gdb_test "print &main" " = .* [style $hex address] <$main_expr>"
# Regression test for a bug where line-wrapping would occur at the
# wrong spot with styling. There were different bugs at different
# widths, so try two.
foreach width {20 30} {
set argv_len [string length $argv]
if { $argv_len == 0 } {
continue
if {![runto_main]} {
fail "style tests failed"
return
}
# There was also a bug where the styling could be wrong in the
# line listing; this is why the words from the source code are
# spelled out in the final result line of the test.
set re1_styled \
# Check that the source highlighter has not stripped away the
# leading newlines.
set main_line [gdb_get_line_number "break here"]
gdb_test "list $main_line,$main_line" "return.*some_called_function.*"
gdb_test_no_output "set style enabled off"
set argv ""
gdb_test_multiple "frame" "frame without styling" {
-re -wrap "main \\(argc=.*, (argv=$hex)\\).*style\\.c:\[0-9\].*" {
set argv $expect_out(1,string)
pass $gdb_test_name
}
}
gdb_test_no_output "set style enabled on"
set main_expr [limited_style main function]
set base_file_expr [limited_style ".*style\\.c" file]
set file_expr "$base_file_expr:\[0-9\]+"
set arg_expr [limited_style "arg." variable]
gdb_test "frame" \
[multi_line \
"#0 *$main_expr.*$arg_expr.*" \
".*$arg_expr.*" \
".* at .*$file_expr.*" \
"\[0-9\]+.*return.* break here .*"]
set re2_styled \
[multi_line \
"#0 *$main_expr.*$arg_expr.*" \
".*$arg_expr.* at .*$file_expr.*" \
"\[0-9\]+.*return.* break here .*"]
"#0\\s+$main_expr\\s+\\($arg_expr=$decimal,\\s+$arg_expr=$hex\\)\\s+at\\s+$file_expr" \
"\[0-9\]+\\s+.*return.* break here .*"]
gdb_test "info breakpoints" "$main_expr at $file_expr.*"
# The length of the line containing argv containing:
# - 4 leading spaces
# - argv string
# - closing parenthesis
set line_len [expr 4 + $argv_len + 1]
gdb_test_no_output "set style sources off"
gdb_test "frame" \
"\r\n\[^\033\]*break here.*" \
"frame without sources styling"
gdb_test_no_output "set style sources on"
if { $line_len > $width } {
# At on the next line.
set re_styled $re1_styled
} else {
# At on the same line as argv.
set re_styled $re2_styled
gdb_test "break -q main" "file $base_file_expr.*"
gdb_test "print &main" " = .* [limited_style $hex address] <$main_expr>"
# Regression test for a bug where line-wrapping would occur at
# the wrong spot with styling. There were different bugs at
# different widths, so try two.
foreach width {20 30} {
set argv_len [string length $argv]
if { $argv_len == 0 } {
continue
}
# There was also a bug where the styling could be wrong in
# the line listing; this is why the words from the source
# code are spelled out in the final result line of the
# test.
set re1_styled \
[multi_line \
"#0\\s+$main_expr\\s+\\($arg_expr=$decimal,\\s+" \
"\\s+$arg_expr=$hex\\)" \
"\\s+at\\s+$file_expr" \
"\[0-9\]+\\s+.*return.* break here .*"]
set re2_styled \
[multi_line \
"#0\\s+$main_expr\\s+\\($arg_expr=.*" \
"\\s+$arg_expr=$hex\\)\\s+at\\s+$file_expr" \
"\[0-9\]+\\s+.*return.* break here .*"]
# The length of the line containing argv containing:
# - 4 leading spaces
# - argv string
# - closing parenthesis
set line_len [expr 4 + $argv_len + 1]
if { $line_len > $width } {
# At on the next line.
set re_styled $re1_styled
} else {
# At on the same line as argv.
set re_styled $re2_styled
}
gdb_test_no_output "set width $width"
gdb_test "frame" $re_styled "frame when width=$width"
}
gdb_test_no_output "set width $width"
gdb_test "frame" $re_styled "frame when width=$width"
# Reset width back to 0.
gdb_test_no_output "set width 0" ""
if {$test_macros} {
set macro_line [gdb_get_line_number "\#define SOME_MACRO"]
gdb_test "info macro SOME_MACRO" \
"Defined at $base_file_expr:$macro_line\r\n#define SOME_MACRO 23"
}
gdb_test_no_output "set width 0"
set main [limited_style main function]
set func [limited_style some_called_function function]
# Somewhere should see the call to the function.
gdb_test "disassemble main" \
[concat "Dump of assembler code for function $main:.*" \
"[limited_style $hex address].*$func.*"]
set ifield [limited_style int_field variable]
set sfield [limited_style string_field variable]
set efield [limited_style e_field variable]
set evalue [limited_style VALUE_TWO variable]
gdb_test "print struct_value" \
"\{$ifield = 23,.*$sfield = .*,.*$efield = $evalue.*"
set address_style_expr [limited_style ".*\".*address.*\".*style.*" address]
set color "blue"
if { $currently_disabled_style == "address" } {
set color "none"
}
gdb_test "show style address foreground" \
"The ${address_style_expr} foreground color is: ${color}" \
"style name and style word styled using its own style in show style"
set aliases_expr [limited_style ".*aliases.*" title]
set breakpoints_expr [limited_style ".*breakpoints.*" title]
gdb_test "help" \
[multi_line \
"List of classes of commands:" \
"" \
"${aliases_expr} -- User-defined aliases of other commands\." \
"${breakpoints_expr} -- Making program stop at certain points\." \
".*" \
] \
"help classes of commands styled with title"
set taas_expr [limited_style ".*taas.*" title]
set tfaas_expr [limited_style ".*tfaas.*" title]
set cut_for_thre_expr [limited_style "cut for 'thre" highlight]
gdb_test "apropos -v cut for 'thre" \
[multi_line \
"" \
"${taas_expr}" \
"Apply a command to all .*" \
"Usage:.*" \
"short${cut_for_thre_expr}ad apply.*" \
"" \
"${tfaas_expr}" \
"Apply a command to all .*" \
"Usage:.*" \
"short${cut_for_thre_expr}ad apply.*" \
]
clean_restart_and_disable
set quoted [string_to_regexp $binfile]
set pass_re "Reading symbols from [limited_style $quoted file]\.\.\."
if { $readnow } {
set pass_re \
[multi_line \
$pass_re \
"Expanding full symbols from [limited_style $quoted file]\.\.\."]
}
gdb_test "file $binfile" \
$pass_re \
"filename is styled when loading symbol file" \
"Are you sure you want to change the file.*" \
"y"
gdb_test "pwd" "Working directory [limited_style .*? file].*"
gdb_test_no_output "set print repeat 3"
gdb_test "print {0,0,0,0,0,0,0,0}" \
" = \\{0 [limited_style {<repeats.*8.*times>} metadata]\\}"
gdb_test "show logging file" \
"The current logfile is \"[limited_style .*? file]\"\\..*"
# Check warnings are styled by setting a rubbish data
# directory.
gdb_test "set data-directory Makefile" \
"warning: [limited_style .*? file] is not a directory\\..*"
gdb_test "show data-directory" \
"GDB's data directory is \"[limited_style .*? file]\"\\..*"
# Check that deprecation styles command names.
gdb_test_no_output "maintenance deprecate p \"new_p\"" \
"maintenance deprecate p \"new_p\" /1/"
gdb_test "p 5" \
"Warning: '[limited_style p title]', an alias for the command '[limited_style print title]', is deprecated.*Use '[limited_style new_p title]'.*" \
"p deprecated warning, with replacement"
}
}
# Reset width back to 0.
gdb_test_no_output "set width 0"
if {$test_macros} {
set macro_line [gdb_get_line_number "\#define SOME_MACRO"]
gdb_test "info macro SOME_MACRO" \
"Defined at $base_file_expr:$macro_line\r\n#define SOME_MACRO 23"
}
set main [style main function]
set func [style some_called_function function]
# Somewhere should see the call to the function.
gdb_test "disassemble main" \
[concat "Dump of assembler code for function $main:.*" \
"[style $hex address].*$func.*"]
set ifield [style int_field variable]
set sfield [style string_field variable]
set efield [style e_field variable]
set evalue [style VALUE_TWO variable]
gdb_test "print struct_value" \
"\{$ifield = 23,.*$sfield = .*,.*$efield = $evalue.*"
# A separate test from the above as the styled text this checks can't
# currently be disabled (the text is printed too early in GDB's
# startup process).
proc test_startup_version_string { } {
gdb_exit
gdb_spawn
# Deliberate use of base STYLE proc here as the style of the
# startup version string can't (currently) be controlled.
set vers [style "GNU gdb.*" "35;1"]
gdb_test "" "${vers}.*" \
"version is styled"
set address_style_expr [style ".*\".*address.*\".*style.*" address]
gdb_test "show style address foreground" \
"The ${address_style_expr} foreground color is: blue" \
"style name and style word styled using its own style in show style"
set aliases_expr [style ".*aliases.*" title]
set breakpoints_expr [style ".*breakpoints.*" title]
gdb_test "help" \
[multi_line \
"List of classes of commands:" \
"" \
"${aliases_expr} -- User-defined aliases of other commands\." \
"${breakpoints_expr} -- Making program stop at certain points\." \
".*" \
] \
"help classes of commands styled with title"
set taas_expr [style ".*taas.*" title]
set tfaas_expr [style ".*tfaas.*" title]
set cut_for_thre_expr [style "cut for 'thre" highlight]
gdb_test "apropos -v cut for 'thre" \
[multi_line \
"" \
"${taas_expr}" \
"Apply a command to all .*" \
"Usage:.*" \
"short${cut_for_thre_expr}ad apply.*" \
"" \
"${tfaas_expr}" \
"Apply a command to all .*" \
"Usage:.*" \
"short${cut_for_thre_expr}ad apply.*" \
]
set quoted [string_to_regexp $binfile]
set pass_re "Reading symbols from [style $quoted file]\.\.\."
if { $readnow } {
set pass_re \
[multi_line \
$pass_re \
"Expanding full symbols from [style $quoted file]\.\.\."]
}
gdb_test "file $binfile" \
$pass_re \
"filename is styled when loading symbol file"
gdb_test "pwd" "Working directory [style .*? file].*"
gdb_test_no_output "set print repeat 3"
gdb_test "print {0,0,0,0,0,0,0,0}" \
" = \\{0 [style {<repeats.*8.*times>} metadata]\\}"
gdb_test "show logging file" \
"The current logfile is \"[style .*? file]\"\\..*"
# Check warnings are styled by setting a rubbish data directory.
gdb_test "set data-directory Makefile" \
"warning: [style .*? file] is not a directory\\..*"
gdb_test "show data-directory" \
"GDB's data directory is \"[style .*? file]\"\\..*"
# Check that deprecation styles command names.
gdb_test_no_output "maintenance deprecate p \"new_p\"" \
"maintenance deprecate p \"new_p\" /1/"
gdb_test "p 5" \
"Warning: '[style p title]', an alias for the command '[style print title]', is deprecated.*Use '[style new_p title]'.*" \
"p deprecated warning, with replacement"
gdb_test "" "${vers}.*" "version is styled at startup"
}
# Run tests with all styles in their default state.
with_test_prefix "all styles enabled" {
run_style_tests
}
# Now, for each style in turn. Disable that style only and run the
# test again. Things in that style should NOT now be styled.
foreach style { title file function highlight variable \
address metadata } {
set currently_disabled_style $style
with_test_prefix "disable style $style" {
run_style_tests
}
}
# Finally, check the styling of the version string during startup.
test_startup_version_string

73
gdb/utils.c
View File

@ -1426,12 +1426,15 @@ static void
emit_style_escape (const ui_file_style &style,
struct ui_file *stream = nullptr)
{
applied_style = style;
if (applied_style != style)
{
applied_style = style;
if (stream == nullptr)
wrap_buffer.append (style.to_ansi ());
else
stream->puts (style.to_ansi ().c_str ());
if (stream == nullptr)
wrap_buffer.append (style.to_ansi ());
else
stream->puts (style.to_ansi ().c_str ());
}
}
/* Set the current output style. This will affect future uses of the
@ -1800,14 +1803,20 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
prompt is given; and to avoid emitting style
sequences in the middle of a run of text, we track
this as well. */
ui_file_style save_style;
ui_file_style save_style = applied_style;
bool did_paginate = false;
chars_printed = 0;
lines_printed++;
if (wrap_column)
{
save_style = wrap_style;
/* We are about to insert a newline at an historic
location in the WRAP_BUFFER. Before we do we want to
restore the default style. To know if we actually
need to insert an escape sequence we must restore the
current applied style to how it was at the WRAP_COLUMN
location. */
applied_style = wrap_style;
if (stream->can_emit_style_escape ())
emit_style_escape (ui_file_style (), stream);
/* If we aren't actually wrapping, don't output
@ -1822,10 +1831,7 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
stream->puts ("\n");
}
else
{
save_style = applied_style;
flush_wrap_buffer (stream);
}
flush_wrap_buffer (stream);
/* Possible new page. Note that
PAGINATION_DISABLED_FOR_COMMAND might be set during
@ -1841,8 +1847,19 @@ fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
if (wrap_column)
{
stream->puts (wrap_indent);
/* Having finished inserting the wrapping we should
restore the style as it was at the WRAP_COLUMN. */
if (stream->can_emit_style_escape ())
emit_style_escape (save_style, stream);
emit_style_escape (wrap_style, stream);
/* The WRAP_BUFFER will still contain content, and that
content might set some alternative style. Restore
APPLIED_STYLE as it was before we started wrapping,
this reflects the current style for the last character
in WRAP_BUFFER. */
applied_style = save_style;
/* FIXME, this strlen is what prevents wrap_indent from
containing tabs. However, if we recurse to print it
and count its chars, we risk trouble if wrap_indent is
@ -1895,16 +1912,9 @@ void
fputs_styled (const char *linebuffer, const ui_file_style &style,
struct ui_file *stream)
{
/* This just makes it so we emit somewhat fewer escape
sequences. */
if (style.is_default ())
fputs_maybe_filtered (linebuffer, stream, 1);
else
{
set_output_style (stream, style);
fputs_maybe_filtered (linebuffer, stream, 1);
set_output_style (stream, ui_file_style ());
}
set_output_style (stream, style);
fputs_maybe_filtered (linebuffer, stream, 1);
set_output_style (stream, ui_file_style ());
}
/* See utils.h. */
@ -1913,16 +1923,9 @@ void
fputs_styled_unfiltered (const char *linebuffer, const ui_file_style &style,
struct ui_file *stream)
{
/* This just makes it so we emit somewhat fewer escape
sequences. */
if (style.is_default ())
fputs_maybe_filtered (linebuffer, stream, 0);
else
{
set_output_style (stream, style);
fputs_maybe_filtered (linebuffer, stream, 0);
set_output_style (stream, ui_file_style ());
}
set_output_style (stream, style);
fputs_maybe_filtered (linebuffer, stream, 0);
set_output_style (stream, ui_file_style ());
}
/* See utils.h. */
@ -2222,11 +2225,9 @@ vfprintf_styled_no_gdbfmt (struct ui_file *stream, const ui_file_style &style,
std::string str = string_vprintf (format, args);
if (!str.empty ())
{
if (!style.is_default ())
set_output_style (stream, style);
set_output_style (stream, style);
fputs_maybe_filtered (str.c_str (), stream, filter);
if (!style.is_default ())
set_output_style (stream, ui_file_style ());
set_output_style (stream, ui_file_style ());
}
}