diff options
author | Andrew Burgess <andrew.burgess@embecosm.com> | 2021-04-19 13:14:41 +0100 |
---|---|---|
committer | Andrew Burgess <andrew.burgess@embecosm.com> | 2021-04-23 09:40:42 +0100 |
commit | 9fc299558896b4ff19c45a3e62459851e4d96cb9 (patch) | |
tree | 0af699b88a9261fbec735e25b2e59038f7fa36a9 /gdb/testsuite | |
parent | c21346c5e224ace15a3ba8b7f56cd375e30e29a8 (diff) | |
download | binutils-9fc299558896b4ff19c45a3e62459851e4d96cb9.zip binutils-9fc299558896b4ff19c45a3e62459851e4d96cb9.tar.gz binutils-9fc299558896b4ff19c45a3e62459851e4d96cb9.tar.bz2 |
gdb: remove some caching from the dwarf reader
While working on some changes to 'info sources' I ran into a situation
where I was seeing the same source files reported twice in the output
of the 'info sources' command when using either .gdb_index or the
.debug_name index.
I traced the problem back to some caching in
dwarf2_base_index_functions::map_symbol_filenames; when called GDB
caches the set of filenames, but, filesnames are not removed as the
index entries are expanded into full symtabs. As a result we can end
up seeing filenames reported both from a full symtab _and_ from
a (stale) previously cached index entry.
Now, obviously, when seeing a problem like this the "correct" fix is
to remove the stale entries from the cache, however, I ran a few
experiments to see why this wasn't really hitting us anywhere, and, as
far as I can tell, ::map_symbol_filenames is only called from three
places:
1. The mi command -file-list-exec-source-files,
2. The 'info sources' command, and
3. Filename completion
However, the result of this "bug" is that we will see duplicate
filenames, and readline's completion mechanism already removes
duplicates, so for case #3 we will never see any problems.
Cases #1 and #2 are basically the same, and in each case, to see a
problem we need to ensure we craft the test in a particular way, start
up ensuring we have some unexpected symtabs, then run one of the
commands to populate the cache, then expand one of the symtabs, and
list the sources again. At this point you'll see duplicate entries in
the results. Hardly surprising we haven't randomly hit this situation
in testing.
So, considering that use cases #1 and #2 are certainly not "high
performance" code (i.e. I don't think these justify the need for
caching) this leaves use case #3. Does this use justify the need for
caching? Well the psymbol_functions::map_symbol_filenames function
doesn't seem to do any extra caching, and within
dwarf2_base_index_functions::map_symbol_filenames, the only expensive
bit appears to be the call to dw2_get_file_names, and this already
does its own caching via this_cu->v.quick->file_names.
The upshot of all this analysis was that I'm not convinced the need
for the additional caching is justified, and so, I propose that to fix
the bug in GDB, I just remove the extra caching (for now).
If we later find that the caching _was_ useful, then we can
reintroduce it, but add it back such that it doesn't reintroduce this
bug.
As I was changing dwarf2_base_index_functions::map_symbol_filenames I
replaced the use of htab_up with std::unordered_set.
Tested using target_boards cc-with-debug-names and dwarf4-gdb-index.
gdb/ChangeLog:
* dwarf2/read.c: Add 'unordered_set' include.
(dwarf2_base_index_functions::map_symbol_filenames): Replace
'visited' hash table with 'qfn_cache' unordered_set. Remove use
of per_Bfd->filenames_cache cache, and use function local
filenames_cache instead. Reindent.
* dwarf2/read.h (struct dwarf2_per_bfd) <filenames_cache>: Delete.
gdb/testsuite/ChangeLog:
* gdb.base/info_sources.exp: Add new tests.
Diffstat (limited to 'gdb/testsuite')
-rw-r--r-- | gdb/testsuite/ChangeLog | 4 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/info_sources.exp | 17 |
2 files changed, 15 insertions, 6 deletions
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 1a181f3..73f95b9 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2021-04-23 Andrew Burgess <andrew.burgess@embecosm.com> + + * gdb.base/info_sources.exp: Add new tests. + 2021-04-22 Tom Tromey <tom@tromey.com> * gdb.base/ptype-offsets.cc (struct empty_member): New. diff --git a/gdb/testsuite/gdb.base/info_sources.exp b/gdb/testsuite/gdb.base/info_sources.exp index 81edb3d..a4f7d19 100644 --- a/gdb/testsuite/gdb.base/info_sources.exp +++ b/gdb/testsuite/gdb.base/info_sources.exp @@ -28,18 +28,18 @@ if {[prepare_for_testing $testfile.exp $testfile \ # in the output. Similarly, EXPECT_SEEN_INFO_SOURCES_BASE indicates that the source file # info_sources_base.c must be seen in the output. proc test_info_sources {args expect_seen_info_sources expect_seen_info_sources_base} { - global gdb_prompt + global gdb_prompt srcfile srcfile2 set seen_info_sources 0 set seen_info_sources_base 0 set cmd [concat "info sources " $args] gdb_test_multiple $cmd $cmd { - -re "^\[^,\]*info_sources.c(, |\[\r\n\]+)" { - set seen_info_sources 1 + -re "^\[^,\]*${srcfile}(, |\[\r\n\]+)" { + incr seen_info_sources exp_continue } - -re "^\[^,\]*info_sources_base.c(, |\[\r\n\]+)" { - set seen_info_sources_base 1 + -re "^\[^,\]*${srcfile2}(, |\[\r\n\]+)" { + incr seen_info_sources_base 1 exp_continue } -re ", " { @@ -60,8 +60,13 @@ if ![runto_main] { untested $testfile.exp return -1 } -gdb_test "break some_other_func" "" +# List both files with no regexp: +with_test_prefix "in main" { + test_info_sources "" 1 1 +} + +gdb_test "break some_other_func" "" gdb_test "continue" # List both files with no regexp: |