mirror of
https://sourceware.org/git/binutils-gdb.git
synced 2025-03-31 14:11:36 +08:00
gdb: check styled status of source cache entries
Currently GDB's source cache doesn't track whether the entries within the cache are styled or not. This is pretty much fine, the assumption is that any time we are fetching source code, we do so in order to print it to the terminal, so where possible we always want styling applied, and if styling is not applied, then it is because that file cannot be styled for some reason. Changes to 'set style enabled' cause the source cache to be flushed, so future calls to fetch source code will regenerate the cache entries with styling enabled or not as appropriate. But this all assumes that styling is either on or off, and that switching between these two states isn't done very often. However, the Python API allows for individual commands to be executed with styling turned off via gdb.execute(). See commit: commit e5348a7ab3f11f4c096ee4ebcdb9eb2663337357 Date: Thu Feb 13 15:39:31 2025 +0000 gdb/python: new styling argument to gdb.execute Currently the source cache doesn't handle this case. Consider this: (gdb) list main ... snip, styled source code displayed here ... (gdb) python gdb.execute("list main", True, False, False) ... snip, styled source code is still shown here ... In the second case, the final `False` passed to gdb.execute() is asking for unstyled output. The problem is that, `get_source_lines` calls `ensure` to prime the cache for the file in question, then `extract_lines` just pulls the lines of interest from the cached contents. In `ensure`, if there is a cache entry for the desired filename, then that is considered good enough. There is no consideration about whether the cache entry is styled or not. This commit aims to fix this, after this commit, the `ensure` function will make sure that the cache entry used by `get_source_lines` is styled correctly. I think there are two approaches I could take: 1. Allow multiple cache entries for a single file, a styled, and non-styled entry. The `ensure` function would then place the correct cache entry into the last position so that `get_source_lines` would use the correct entry, or 2. Have `ensure` recalculate entries if the required styling mode is different to the styling mode of the current entry. Approach #1 is better if we are rapidly switching between styling modes, while #2 might be better if we want to keep more files in the cache and we only rarely switch styling modes. In the end I chose approach #2, but the good thing is that the changes are all contained within the `ensure` function. If in the future we wanted to change to strategy #1, this could be done transparently to the rest of GDB. So after this commit, the `ensure` function checks if styling is currently possible or not. If it is not, and the current entry is styled, then the current entry only is dropped from the cache, and a new, unstyled entry is created. Likewise, if the current entry is non-styled, but styling is required, we drop one entry and recalculate. With this change in place, I have updated set_style_enabled (in cli/cli-style.c) so the source cache is no longer flushed when the style settings are changed, the source cache will automatically handle changes to the style settings now. This problem was discovered in PR gdb/32676. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32676 Approved-By: Tom Tromey <tom@tromey.com>
This commit is contained in:
parent
daf53ffeaa
commit
05f5f4f274
@ -372,7 +372,9 @@ set_style_enabled (const char *args, int from_tty, struct cmd_list_element *c)
|
||||
warning ("The current terminal doesn't support styling. Styled output "
|
||||
"might not appear as expected.");
|
||||
|
||||
g_source_cache.clear ();
|
||||
/* It is not necessary to flush the source cache here. The source cache
|
||||
tracks whether entries are styled or not. */
|
||||
|
||||
gdb::observers::styling_changed.notify ();
|
||||
}
|
||||
|
||||
|
@ -325,11 +325,26 @@ source_cache::ensure (struct symtab *s)
|
||||
least one caller. */
|
||||
if (i != size - 1)
|
||||
std::swap (m_source_map[i], m_source_map[size - 1]);
|
||||
return true;
|
||||
|
||||
/* If the styling status of the cached entry matches our desired
|
||||
styling status, or we know this file cannot be styled, in
|
||||
which case, this (unstyled) content, is the best we can do. */
|
||||
if (((source_styling && gdb_stdout->can_emit_style_escape ())
|
||||
== m_source_map[size - 1].styled)
|
||||
|| m_no_styling_files.count (fullname) > 0)
|
||||
return true;
|
||||
|
||||
/* We found a match, but styling status doesn't match the desired
|
||||
styling status. We already moved the matching item to the
|
||||
back of M_SOURCE_MAP, so drop the entry now, and then
|
||||
recompute with the desired styling. */
|
||||
m_source_map.pop_back ();
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
std::string contents;
|
||||
bool styled_p = false;
|
||||
try
|
||||
{
|
||||
contents = get_plain_source_lines (s, fullname);
|
||||
@ -343,21 +358,21 @@ source_cache::ensure (struct symtab *s)
|
||||
if (source_styling && gdb_stdout->can_emit_style_escape ()
|
||||
&& m_no_styling_files.count (fullname) == 0)
|
||||
{
|
||||
bool already_styled
|
||||
styled_p
|
||||
= try_source_highlight (contents, s->language (), fullname);
|
||||
|
||||
if (!already_styled)
|
||||
if (!styled_p)
|
||||
{
|
||||
std::optional<std::string> ext_contents;
|
||||
ext_contents = ext_lang_colorize (fullname, contents);
|
||||
if (ext_contents.has_value ())
|
||||
{
|
||||
contents = std::move (*ext_contents);
|
||||
already_styled = true;
|
||||
styled_p = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (!already_styled)
|
||||
if (!styled_p)
|
||||
{
|
||||
/* Styling failed. Styling can fail for instance for these
|
||||
reasons:
|
||||
@ -374,7 +389,8 @@ source_cache::ensure (struct symtab *s)
|
||||
}
|
||||
}
|
||||
|
||||
source_text result = { std::move (fullname), std::move (contents) };
|
||||
source_text result
|
||||
= { std::move (fullname), std::move (contents), styled_p };
|
||||
m_source_map.push_back (std::move (result));
|
||||
|
||||
if (m_source_map.size () > MAX_ENTRIES)
|
||||
|
@ -78,6 +78,8 @@ private:
|
||||
std::string fullname;
|
||||
/* The contents of the file. */
|
||||
std::string contents;
|
||||
/* True if CONTENTS are styled. Otherwise, false. */
|
||||
bool styled;
|
||||
};
|
||||
|
||||
/* A helper function for get_source_lines reads a source file.
|
||||
|
@ -33,12 +33,43 @@ if { [build_executable "failed to build" ${testfile} ${srcfile}] == -1 } {
|
||||
|
||||
set line_number [gdb_get_line_number "List this line."]
|
||||
|
||||
# Helper proc. Run CMD, which should produce a source listing, and
|
||||
# check if the source code is styled or not. EXPECT_STYLED indicates
|
||||
# if we expect the source listing to be styled or not.
|
||||
proc check_source_listing_styling { cmd expect_styled { testname "" } } {
|
||||
if { $testname eq "" } {
|
||||
set testname $cmd
|
||||
}
|
||||
|
||||
set seen_style_escape false
|
||||
gdb_test_multiple $cmd $testname {
|
||||
-re -wrap "Python Exception.*" {
|
||||
fail $gdb_test_name
|
||||
return
|
||||
}
|
||||
-re "\033" {
|
||||
set seen_style_escape true
|
||||
exp_continue
|
||||
}
|
||||
-re "$::gdb_prompt $" {
|
||||
gdb_assert { $seen_style_escape == $expect_styled } \
|
||||
$gdb_test_name
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
# Check that the Python pygments module can be used for source
|
||||
# highlighting when GNU source highlight is not available (or is
|
||||
# disabled, as is done in this test).
|
||||
proc test_pygments_styling {} {
|
||||
clean_restart $::binfile
|
||||
|
||||
# Remote host boards disable styling via GDB's command line. Turn
|
||||
# it back on now.
|
||||
if {[is_remote host]} {
|
||||
gdb_test "set style enabled on"
|
||||
}
|
||||
|
||||
if { ![gdb_py_module_available "pygments"] } {
|
||||
unsupported "pygments module not available"
|
||||
return
|
||||
@ -52,19 +83,7 @@ proc test_pygments_styling {} {
|
||||
|
||||
gdb_test "maint flush source-cache" "Source cache flushed\\."
|
||||
|
||||
set seen_style_escape false
|
||||
gdb_test_multiple "list $::line_number" "" {
|
||||
-re "Python Exception.*" {
|
||||
fail $gdb_test_name
|
||||
}
|
||||
-re "\033" {
|
||||
set seen_style_escape true
|
||||
exp_continue
|
||||
}
|
||||
-re "$::gdb_prompt $" {
|
||||
gdb_assert { $seen_style_escape } $gdb_test_name
|
||||
}
|
||||
}
|
||||
check_source_listing_styling "list $::line_number" true
|
||||
}
|
||||
|
||||
# Use gdb.execute to list source code containing non-utf-8 character.
|
||||
@ -93,6 +112,67 @@ proc test_gdb_execute_non_utf8_source {} {
|
||||
gdb_test "python print(source)" ".*List this line.*"
|
||||
}
|
||||
|
||||
# Use gdb.execute() to list source code. Alternate between asking for
|
||||
# styled, and unstyled source code. In some cases we ask for the
|
||||
# output to be returned via a string, and in other cases we ask for
|
||||
# the output to be sent straight to stdout.
|
||||
proc_with_prefix test_source_cache_style_tracking {} {
|
||||
clean_restart $::binfile
|
||||
|
||||
# Remote host boards disable styling via GDB's command line. Turn
|
||||
# it back on now.
|
||||
if {[is_remote host]} {
|
||||
gdb_test "set style enabled on"
|
||||
}
|
||||
|
||||
gdb_test_no_output "set host-charset ISO-8859-1"
|
||||
|
||||
# Commands which return styled, and non-styled source code mixed
|
||||
# together. This ensures that the source cache will need to keep
|
||||
# discarding the entry with the wrong styling mode. All of these
|
||||
# gdb.execute calls send their output via a string.
|
||||
check_source_listing_styling \
|
||||
"python print(gdb.execute('list $::line_number', to_string=True), end='')" \
|
||||
false
|
||||
check_source_listing_styling \
|
||||
"python print(gdb.execute('list $::line_number', to_string=True, styling=True), end='')" \
|
||||
true
|
||||
foreach from_tty { True False } {
|
||||
check_source_listing_styling \
|
||||
"python print(gdb.execute('list $::line_number', $from_tty, True), end='')" \
|
||||
false
|
||||
check_source_listing_styling \
|
||||
"python print(gdb.execute('list $::line_number', $from_tty, True, True), end='')" \
|
||||
true
|
||||
check_source_listing_styling \
|
||||
"python print(gdb.execute('list $::line_number', $from_tty, True, False), end='')" \
|
||||
false
|
||||
}
|
||||
|
||||
# The same again, but this time the output is sent directly to
|
||||
# stdout.
|
||||
check_source_listing_styling \
|
||||
"python gdb.execute('list $::line_number')" \
|
||||
true
|
||||
check_source_listing_styling \
|
||||
"python gdb.execute('list $::line_number', to_string=False, styling=False)" \
|
||||
false
|
||||
check_source_listing_styling \
|
||||
"python gdb.execute('list $::line_number', to_string=False, styling=True)" \
|
||||
true
|
||||
foreach from_tty { True False } {
|
||||
check_source_listing_styling \
|
||||
"python gdb.execute('list $::line_number', $from_tty, False, False)" \
|
||||
false
|
||||
check_source_listing_styling \
|
||||
"python gdb.execute('list $::line_number', $from_tty, False, True)" \
|
||||
true
|
||||
check_source_listing_styling \
|
||||
"python gdb.execute('list $::line_number', $from_tty, False)" \
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
# We need an ANSI-capable terminal to get the output, additionally we
|
||||
# need to set LC_ALL so GDB knows the terminal is UTF-8 capable,
|
||||
# otherwise we'll get a UnicodeEncodeError trying to encode the
|
||||
@ -100,4 +180,5 @@ proc test_gdb_execute_non_utf8_source {} {
|
||||
with_ansi_styling_terminal {
|
||||
test_pygments_styling
|
||||
test_gdb_execute_non_utf8_source
|
||||
test_source_cache_style_tracking
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user