From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id gJ7LGmsVAGDjHgAAWB0awg (envelope-from ) for ; Thu, 14 Jan 2021 04:56:59 -0500 Received: by simark.ca (Postfix, from userid 112) id 67D381EF80; Thu, 14 Jan 2021 04:56:59 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED, MAILING_LIST_MULTI,T_DKIM_INVALID,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 375401E590 for ; Thu, 14 Jan 2021 04:56:55 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id D6B343857C61; Thu, 14 Jan 2021 09:56:54 +0000 (GMT) Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by sourceware.org (Postfix) with ESMTPS id 55B993857C61 for ; Thu, 14 Jan 2021 09:56:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 55B993857C61 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wm1-x32f.google.com with SMTP id g10so4115288wmh.2 for ; Thu, 14 Jan 2021 01:56:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ib8GCbUznoKKWLDgPDRiorQ+VCUDQI/6L1sn/jKBA5E=; b=FLgXvwQhD52PfRyWdTiZOxh8bcpaCyw7DxV8VBLTK5QrZywMzhYExAZq73FV48vn9C HN7cbW3erNMAH2l0+M3ihqCBoUqmSQA0iMiqRGu2fmr4s7Wxfi8zAEVFbw5PJY9iyRgq wKhICMpkwWugZ3iPMSzKjKjMzWPLiW4NOK9hUUqY+TrUb2QyR+OD3TiFFWHkzZpTFeqF 7ZN+XqMLSI1gcmWneaqUVymXSJ3ujd8RVxJ39MS315dLD53nH0BXLtPmKkwTXQeegqqZ Rdk4QnsB/ESJ5dNmdjpErK/GQRbjRvo5JzIumjSCD/rpEX9p0AeoiM7QEu8CAsDw2YKP bM8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ib8GCbUznoKKWLDgPDRiorQ+VCUDQI/6L1sn/jKBA5E=; b=NseViuPd9WZFbD+2FvSL7mn8tG1JBE7ATrkd0ezoxOWYfkepdGaRpPAYbzxNUAOcx3 6bnzagW8ul5bATX+4Uvg1MMCMxYd3UZ0Jv2V+00x3p6iXNJIQMPZQOaGPc9JLQ5wir3b B+nfWcL/dXUwy2KQVgtWKhBLIk3RqVgSFISWxkZs5UHyOiIciXXKKSVceqHMwA9/68ac 9p2tU6QXqkNSWM3DlhIrxT3Dwcx9RkrhPlKwjlkdUvDhr4ii2tc1o9ijPPtSrP/FcPRy sIlcm99WhVF1A2s3ejFTY6lglxEhV4Q9lPmX9gvrHtj6ucoWPtZTfRAY3znH0n8YWNEq 4/bg== X-Gm-Message-State: AOAM5333COtp1p5FvM/bL35Bo6Gr9aT3xA0aACau9E6jc6m41u8jb1Z5 aFvq8B8khSOeksW8RMLbpFiI9MlAmQ7npQ== X-Google-Smtp-Source: ABdhPJwynNNNunrHZONN3B5TNK6XXYseDsCxNGT2hVf+GxkWeQSzTP0KH7jn0DphG7zNiA0t85gwfw== X-Received: by 2002:a1c:a145:: with SMTP id k66mr3155697wme.18.1610618208823; Thu, 14 Jan 2021 01:56:48 -0800 (PST) Received: from localhost (host86-166-129-230.range86-166.btcentralplus.com. [86.166.129.230]) by smtp.gmail.com with ESMTPSA id s4sm5219328wme.38.2021.01.14.01.56.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Jan 2021 01:56:48 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Subject: [PATCH 1/2] gdb: don't print escape characters when a style is disabled Date: Thu, 14 Jan 2021 09:56:42 +0000 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" 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. 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 unguarded calls to set_output_style in utils.c. In order to test this 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 though 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 that expected the now disabled styling expect no styling. gdb/ChangeLog: * utils.c (fputs_highlighted): Only call set_output_style when the requested style is not the default. (fprintf_styled): Likewise. (vfprintf_styled): 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. (run_all_tests): New proc. --- gdb/ChangeLog | 7 + gdb/testsuite/ChangeLog | 10 + gdb/testsuite/gdb.base/style.exp | 443 +++++++++++++++++++------------ gdb/utils.c | 18 +- 4 files changed, 302 insertions(+), 176 deletions(-) diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp index aec1d0f42ed..58c8da0a10e 100644 --- a/gdb/testsuite/gdb.base/style.exp +++ b/gdb/testsuite/gdb.base/style.exp @@ -17,203 +17,306 @@ standard_testfile -save_vars { env(TERM) } { - # We need an ANSI-capable terminal to get the output. - setenv TERM ansi - - 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 - } +# 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 +} + +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 "" - if {[prepare_for_testing "failed to prepare" $testfile $srcfile \ - $options]} { - return -1 +# A replacement for 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 [global_style_func $str $style] } - set readnow [readnow] + 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 "global_clean_restart $args" - if {![runto_main]} { - fail "style tests failed" - return + 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" "" } +} + +# The core of this test script. Run some tests of different aspects +# of GDB's styling. +proc run_style_tests { } { + global testfile srcfile hex binfile test_macros + global currently_disabled_style + + 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 ${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 + if {![runto_main]} { + fail "style tests failed" + return } - } - gdb_test_no_output "set style enabled on" + # 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.*" - 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_no_output "set style enabled off" - gdb_test "frame" \ - "$main_expr.*$arg_expr.*$arg_expr.*$file_expr.*" - gdb_test "info breakpoints" "$main_expr at $file_expr.*" + 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 sources off" - gdb_test "frame" \ - "\r\n\[^\033\]*break here.*" \ - "frame without sources styling" - gdb_test_no_output "set style sources on" + 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 + } + + # 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 *$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 .*"] + + # 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 "break -q main" "file $base_file_expr.*" + # Reset width back to 0. + gdb_test_no_output "set width 0" "" - gdb_test "print &main" " = .* [style $hex address] <$main_expr>" + 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" + } - # 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 + gdb_test_no_output "set width 0" + + 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.*" + + set address_style_expr [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" - # 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 \ + set aliases_expr [style ".*aliases.*" title] + set breakpoints_expr [style ".*breakpoints.*" title] + gdb_test "help" \ [multi_line \ - "#0 *$main_expr.*$arg_expr.*" \ - ".*$arg_expr.*" \ - ".* at .*$file_expr.*" \ - "\[0-9\]+.*return.* break here .*"] - set re2_styled \ + "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 \ - "#0 *$main_expr.*$arg_expr.*" \ - ".*$arg_expr.* at .*$file_expr.*" \ - "\[0-9\]+.*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 + "" \ + "${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 + + 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_no_output "set width $width" - gdb_test "frame" $re_styled "frame when width=$width" + 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 [style .*? file].*" + + gdb_test_no_output "set print repeat 3" + gdb_test "print {0,0,0,0,0,0,0,0}" \ + " = \\{0 [style {} 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" } +} - # Reset width back to 0. - gdb_test_no_output "set width 0" +# 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 - 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 vers [style "GNU gdb.*" "35;1"] + gdb_test "" "${vers}.*" "version is styled at startup" +} - 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.*"] +# A test driver. Calls RUN_STYLE_TESTS multiple times with each GDB +# style disabled one at a time. +proc run_all_tests { } { + global currently_disabled_style - 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.*" + # Run tests with all styles in their default state. + with_test_prefix "all styles enabled" { + run_style_tests + } - gdb_exit - gdb_spawn + # 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 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]\.\.\."] + set currently_disabled_style $style + with_test_prefix "disable style $style" { + run_style_tests + } } - 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 {} 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" } + +# Override the 'style' function. +rename style global_style_func +rename limited_style style + +# Override the 'clean_restart' function. +rename clean_restart global_clean_restart +rename clean_restart_and_disable clean_restart + +run_all_tests + +# Restore the 'clean_restart' function. +rename clean_restart clean_restart_and_disable +rename global_clean_restart clean_restart + +# Restore the 'style' function. +rename style limited_style +rename global_style_func style + +# Finally, check the styling of the version string during startup. +test_startup_version_string diff --git a/gdb/utils.c b/gdb/utils.c index 414e7b153a0..3aad9c7af74 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -1946,14 +1946,16 @@ fputs_highlighted (const char *str, const compiled_regex &highlight, } /* Output pmatch with the highlight style. */ - set_output_style (stream, highlight_style.style ()); + if (!highlight_style.style ().is_default ()) + set_output_style (stream, highlight_style.style ()); while (n_highlight > 0) { fputc_filtered (*str, stream); n_highlight--; str++; } - set_output_style (stream, ui_file_style ()); + if (!highlight_style.style ().is_default ()) + set_output_style (stream, ui_file_style ()); } /* Output the trailing part of STR not matching HIGHLIGHT. */ @@ -2195,11 +2197,13 @@ fprintf_styled (struct ui_file *stream, const ui_file_style &style, { va_list args; - set_output_style (stream, style); + if (!style.is_default ()) + set_output_style (stream, style); va_start (args, format); vfprintf_filtered (stream, format, args); va_end (args); - set_output_style (stream, ui_file_style ()); + if (!style.is_default ()) + set_output_style (stream, ui_file_style ()); } /* See utils.h. */ @@ -2208,9 +2212,11 @@ void vfprintf_styled (struct ui_file *stream, const ui_file_style &style, const char *format, va_list args) { - set_output_style (stream, style); + if (!style.is_default ()) + set_output_style (stream, style); vfprintf_filtered (stream, format, args); - set_output_style (stream, ui_file_style ()); + if (!style.is_default ()) + set_output_style (stream, ui_file_style ()); } /* See utils.h. */ -- 2.25.4