This fixes a couple of comments in dwarf2/cooked-index.h.
The comment by cooked_index_entry::canonical mentions C++, but this
field can also be different from 'name' in other situations. Rather
than enumerate the cases here (which doesn't seem important), make the
text a little less specific.
Also, cooked_index_entry::write_scope doesn't document its "for_main"
parameter -- and it is misnamed in the prototype as well.
Reviewed-By: Tom de Vries <tdevries@suse.de>
This changes cooked_index_shard::handle_gnat_encoded_entry to modify
the incoming entry itself, and to return void rather than a new name.
this simplifies the caller a little, which is convenient for a
different series I am working on.
Approved-By: Tom de Vries <tdevries@suse.de>
Cleanup includes in dwarf2/*.
1. Add the necessary includes so that clangd reports no errors when
opening header files. This ensures that header files include what
they use.
2. Remove all includes reported as unused by clangd (except
gdb-safe-ctype.h, which I think does some magic that affects what
follows).
Built-tested --enable-threading at "yes" and "no", since there are some
portions of code gated by `#ifdef CXX_STD_THREAD`.
Change-Id: I21debffcd7c2caf90f08e1e0fbba3ce30422d042
Approved-By: Tom Tromey <tom@tromey.com>
This is a simple find / replace from "struct bound_minimal_symbol" to
"bound_minimal_symbol", to make things shorter and more consisten
througout. In some cases, move variable declarations where first used.
Change-Id: Ica4af11c4ac528aa842bfa49a7afe8fe77a66849
Reviewed-by: Keith Seitz <keiths@redhat.com>
Approved-By: Andrew Burgess <aburgess@redhat.com>
dwarf2_per_bfd::index_addrmap is only used by the .gdb_index reader,
so this field can be moved to mapped_gdb_index instead. Then,
cooked_index_functions::find_per_cu can be removed in favor of a
method on the index object.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31821
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Tom de Vries pointed out that the combination of sharding,
multi-threading, and per-CU "racing" means that sometimes a cross-CU
DIE reference might not be correctly resolved. However, it's
important to handle this correctly, due to some unfortunate aspects of
DWARF.
This patch implements this by arranging to preserve each worker's DIE
map through the end of index finalization. The extra data is
discarded when finalization is done. This approach also allows the
parent name resolution to be sharded, by integrating it into the
existing entry finalization loop.
In an earlier review, I remarked that addrmap couldn't be used here.
However, I was mistaken. A *mutable* addrmap cannot be used, as those
are based on splay trees and restructure the tree even during lookups
(and thus aren't thread-safe). A fixed addrmap, on the other hand, is
just a vector and is thread-safe.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30846
This changes the DIE range map from a raw addrmap to a custom class.
A new type is used to represent the ranges, in an attempt to gain a
little type safety as well.
Note that the new code includes a map-of-maps type. This is not used
yet, but will be used in the next patch.
Co-Authored-By: Tom de Vries <tdevries@suse.de>
This patch makes allocate_on_obstack a little bit safer, by enforcing
the rule that objects allocated on an obstack must have a trivial
destructor.
The static assert is done in a method -- doing it inside the class
itself won't work because the class is incomplete at that point.
There are a few spots in the tree that use 'addrmap' where only an
addrmap_fixed will ever really be seen. This patch changes this code
to use the more specific type.
The background DWARF reader changes introduced a race when writing to
the index cache. The problem here is that constructing the
index_cache_store_context object should only happen on the main
thread, to ensure that the various value captures do not race.
This patch adds an assert to the construct to that effect, and then
arranges for this object to be constructed by the cooked_index_worker
constructor -- which is only invoked on the main thread.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31262
This changes index_cache_store_context to also capture the per-BFD
object when it is constructed. This is used when storing to the
cache, and this approach makes the code a little simpler.
This changes quick_symbol_functions::lookup_global_symbol_language to
accept domain_search_flags rather than just a domain_enum, and fixes
up the fallout.
To avoid introducing any regressions, any code passing VAR_DOMAIN now
uses SEARCH_VFT.
That is, no visible changes should result from this patch. However,
it sets the stage to refine some searches later on.
I noticed that cooked_index_worker::start_reading isn't really needed.
This patch removes it, and also removes the SCOPED_EXIT, in favor of a
direct call.
This changes cooked_index_worker to be an abstract base class. The
base class implementation is moved to cooked-index.c, and a concrete
subclass is added to read.c.
This change is preparation for the new .debug_names reader, which will
supply its own concrete implementation of the worker.
This moves the declaration of cooked_index_functions to
cooked-index.h. This makes it visible for use by the rewritten
.debug_names reader, and it also lets us move the implementation of
make_quick_functions into cooked-index.c, where it really belongs.
This adds a new 'lang' member to cooked_index_entry. This holds the
language of the symbol. This is primarily useful for the new
.debug_names reader, which will not scan the CUs for languages up
front.
This also changes cooked_index_shard::add to return a non-const
pointer. This doesn't impact the current code, but is needed for the
new reader.
This commit is the result of the following actions:
- Running gdb/copyright.py to update all of the copyright headers to
include 2024,
- Manually updating a few files the copyright.py script told me to
update, these files had copyright headers embedded within the
file,
- Regenerating gdbsupport/Makefile.in to refresh it's copyright
date,
- Using grep to find other files that still mentioned 2023. If
these files were updated last year from 2022 to 2023 then I've
updated them this year to 2024.
I'm sure I've probably missed some dates. Feel free to fix them up as
you spot them.
Currently cooked_index entry creation is either:
- done immediately if the parent_entry is known, or
- deferred if the parent_entry is not yet known, and done later while
resolving the deferred entries.
Instead, create all cooked_index entries immediately, and keep track of which
entries have a parent_entry that needs resolving later using the new
IS_PARENT_DEFERRED flag.
Tested on x86_64-linux.
Approved-By: Tom Tromey <tom@tromey.com>
Make cooked_index_entry::parent_entry private, and add member functions to
access it.
Tested on x86_64-linux and ppc64le-linux.
Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
Approved-By: Tom Tromey <tom@tromey.com>
Make cooked_index_storage::add and cooked_index_entry::add return a
"cooked_index_entry *" instead of a "const cooked_index_entry *".
Tested on x86_64-linux and ppc64le-linux.
Tested-By: Alexandra Petlanova Hajkova <ahajkova@redhat.com>
Approved-By: Tom Tromey <tom@tromey.com>
A user pointed out that the recent background DWARF reader series
broke the build when --disable-threading is in use. This patch fixes
the problem. I am checking it in.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31223
dwarf2_base_index_functions::find_per_cu is documented as using an
unrelocated address. This patch changes the interface to use the
unrelocated_addr type, just to be a bit more type-safe.
Regression tested on x86-64 Fedora 38.
This patch rearranges the DWARF reader so that more work is done in
the background. This is PR symtab/29942.
The idea here is that there is only a small amount of work that must
be done on the main thread when scanning DWARF -- before the main
scan, the only part is mapping the section data.
Currently, the DWARF reader uses the quick_symbol_functions "lazy"
functionality to defer even starting to read. This patch instead
changes the reader to start reading immediately, but doing more in
worker tasks.
Before this patch, "file" on my machine:
(gdb) file /tmp/gdb
2023-10-23 12:29:56.885 - command started
Reading symbols from /tmp/gdb...
2023-10-23 12:29:58.047 - command finished
Command execution time: 5.867228 (cpu), 1.162444 (wall)
After the patch, more work is done in the background and so this takes
a bit less time:
(gdb) file /tmp/gdb
2023-10-23 13:25:51.391 - command started
Reading symbols from /tmp/gdb...
2023-10-23 13:25:51.712 - command finished
Command execution time: 1.894500 (cpu), 0.320306 (wall)
I think this could be further sped up by using the shared library load
map to avoid objfile loops like the one in expand_symtab_containing_pc
-- it seems like the correct objfile could be chosen more directly.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29942
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30174
This changes the cooked index code to wait for threads in its
public-facing API. That is, the waits are done in cooked_index now,
and never in the cooked_index_shard. Centralizing this decision makes
it easier to wait for other events here as well.
Fortran provides additional entry points for subroutines and functions.
These entry points may use only a subset (or a different set) of the
parameters of the original subroutine. The entry points may be described
via the DWARF tag DW_TAG_entry_point.
This commit adds support for parsing the DW_TAG_entry_point DWARF tag.
Currently, between ifx/ifort/gfortran, only ifort is actually emitting
this tag. Both, ifx and gfortran use the DW_TAG_subprogram tag as
workaround/alternative. Thus, this patch really only adds more ifort
support. Even so, some of the attached tests still fail for ifort, due
to some wrong line info generated for the entry points in ifort.
After this patch it is possible to set a breakpoint in gdb with the
ifort compiled example at the entry points 'foo' and 'foobar', which was not
possible before.
As gcc and ifx do not emit the tag I also added a test to gdb.dwarf2
which uses some underlying c compiled code and adds some Fortran style DWARF
to it emitting the DW_TAG_entry_point. Before this patch it was not
possible to actually define breakpoint at the entry point tags.
For gfortran there actually exists a bug on bugzilla, asking for the use
of DW_TAG_entry_point over DW_TAG_subprogram:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37134
This patch was originally posted here
https://sourceware.org/legacy-ml/gdb-patches/2017-07/msg00317.html
but its review/pinging got lost after a while. I reworked it to fit the
current GDB.
Co-authored-by: Bernhard Heckel <bernhard.heckel@intel.com>
Co-authored-by: Tim Wiederhake <tim.wiederhake@intel.com>
Approved-by: Tom Tromey <tom@tromey.com>
Given that GDB now requires a C++17, replace all uses of
gdb::string_view with std::string_view.
This change has mostly been done automatically:
- gdb::string_view -> std::string_view
- #include "gdbsupport/gdb_string_view.h" -> #include <string_view>
One things which got brought up during review is that gdb::stging_view
does support being built from "nullptr" while std::sting_view does not.
Two places are manually adjusted to account for this difference:
gdb/tui/tui-io.c:tui_getc_1 and
gdbsupport/format.h:format_piece::format_piece.
The above automatic change transformed
"gdb::to_string (const gdb::string_view &)" into
"gdb::to_string (const std::string_view &)". The various direct users
of this function are now explicitly including
"gdbsupport/gdb_string_view.h". A later patch will remove the users of
gdb::to_string.
The implementation and tests of gdb::string_view are unchanged, they will
be removed in a following patch.
Change-Id: Ibb806a7e9c79eb16a55c87c6e41ad396fecf0207
Approved-By: Tom Tromey <tom@tromey.com>
Approved-By: Pedro Alves <pedro@palves.net>
Unfortunately MinGW doesn't support std::future yet, so this causes the
build to fail. Use GDB's version which provides a fallback for this case.
Tested for regressions on native aarch64-linux.
Approved-By: Tom Tromey <tromey@adacore.com>
Tom de Vries pointed out a bug in the index-cache background writer --
sometimes it will fail. He also noted that it fails when the number
of worker threads is set to zero. These turn out to be the same
problem -- the cache can't be written to until the per-BFD's
"index_table" member is set.
This patch avoids the race by rearranging the code slightly, to ensure
the cache cannot possibly be written before the member is set.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30261
The new DWARF cooked indexer interacts poorly with the DWARF index
cache. In particular, the cache will require gdb to wait for the
cooked index to be finalized. As this happens in the foreground, it
means that users with this setting enabled will see a slowdown.
This patch changes gdb to write the cache entry a worker thread. (As
usual, in the absence of threads, this work is simply done immediately
in the main thread.)
Some care is taken to ensure that this can't crash, and that gdb will
not exit before the task is complete.
To avoid use-after-free problems, the DWARF per-BFD object explicitly
waits for the index cache task to complete.
To avoid gdb exiting early, an exit observer is used to wait for all
such pending tasks.
In normal use, neither of these waits will be very visible. For users
using "-batch" to pre-generate the index, though, it would be.
However I don't think there is much to be done about this, as it was
the status quo ante.
The new DWARF indexer broke "start" for some languages.
For D, it is broken because, while the code in cooked_index_shard::add
specifically excludes Ada, it fails to exclude D. This means that the
C "main" will be detected as "main" here -- whereas what is intended
is for the code in find_main_name to use d_main_name to find the name.
The Rust compiler, on the other hand, uses DW_AT_main_subprogram.
However, the code in dwarf2_build_psymtabs_hard fails to create a
fully-qualified name, so the name always ends up as plain "main".
For D and Ada, a very simple approach suffices: remove the check
against "main" from cooked_index_shard::add. This also has the
benefit of slightly speeding up DWARF indexing. I assume this
approach will work for Pascal and Modula-2 as well, but I don't have a
way to test those at present.
For Rust, though, this is not sufficient. And, computing the
fully-qualified name in dwarf2_build_psymtabs_hard will crash, because
cooked_index_entry::full_name uses the canonical name -- and that is
not computed until after canonicalization.
However, we don't want to wait for canonicalization to be done before
computing the main name. That would remove any benefit from doing
canonicalization is the background.
This patch solves this dilemma by noticing that languages using
DW_AT_main_subprogram are, currently, disjoint from languages
requiring canonicalization. Because of this, we can add a parameter
to full_name to let us avoid crashes, slowdowns, and races here.
This is kind of tricky and ugly, so I've tried to comment it
sufficiently.
While doing this, I had to change gdb.dwarf2/main-subprogram.exp. A
different possibility here would be to ignore the canonicalization
needs of C in this situation, because those only affect certain types.
However, I chose this approach because the test case is artificial
anyhow.
A long time ago, in an earlier threading attempt, I changed the global
current_language to be a function (hidden behind a macro) to let us
attempt lazily computing the current language. Perhaps this approach
could still be made to work. However, that also seemed rather tricky,
more so than this patch.
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30116
In PR gdb/29854, Simon pointed out that it would be good to be able to
use C-c when the DWARF cooked index is waiting for finalization. The
idea here is to be able to interrupt a command like "break" -- not to
stop the finalization process itself, which runs in a worker thread.
This patch implements this idea, by changing the index wait functions
to, by default, allow a quit. Polling is done, because there doesn't
seem to be a better way to interrupt a wait on a std::future.
For v2, I realized that the thread compatibility code in thread-pool.h
also needed an update.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29854
I propose to rename cooked_index_vector and cooked_index such that the
"main" object, that is the entry point to the index, is called
cooked_index. The fact that the cooked index is implemented as a vector
of smaller indexes is an implementation detail.
This patch renames cooked_index to cooked_index_shard. The following
patch renames cooked_index_vector to cooked_index.
Change-Id: Id650f97dcb23c48f8409fa0974cd093ca0b75177
Approved-By: Tom Tromey <tom@tromey.com>
As I am investigating a crash I see with the cooked index, I thought it
would be useful to have a way to dump the index contents. For those not
too familiar with it (that includes me), it can help get a feel of what
it contains and how it is structured.
The cooked_index_functions::dump function is called as part of the
"maintenance print objfiles" command. I tried to make the output
well structured and indented to help readability, as this prints a lot
of text.
The dump function first dumps all cooked index entries, like this:
[25] ((cooked_index_entry *) 0x621000121220)
name: __ioinit
canonical: __ioinit
DWARF tag: DW_TAG_variable
flags: 0x2 [IS_STATIC]
DIE offset: 0x21a4
parent: ((cooked_index_entry *) 0x6210000f9610) [std]
Then the information about the main symbol:
main: ((cooked_index_entry *) 0x621000123b40) [main]
And finally the address map contents:
[1] ((addrmap *) 0x6210000f7910)
[0x0] ((dwarf2_per_cu_data *) 0)
[0x118a] ((dwarf2_per_cu_data *) 0x60c000007f00)
[0x1cc7] ((dwarf2_per_cu_data *) 0)
[0x1cc8] ((dwarf2_per_cu_data *) 0x60c000007f00)
[0x1cdf] ((dwarf2_per_cu_data *) 0)
[0x1ce0] ((dwarf2_per_cu_data *) 0x60c000007f00)
The display of address maps above could probably be improved, to show it
more as ranges, but I think this is a reasonable start.
Note that this patch depends on Pedro Alves' patch "enum_flags
to_string" [1]. If my patch is to be merged before Pedro's series, I
will cherry-pick this patch from his series and merge it before mine.
[1] https://inbox.sourceware.org/gdb-patches/20221212203101.1034916-8-pedro@palves.net/
Change-Id: Ida13e479fd4c8d21102ddd732241778bc3b6904a
Simon pointed out that the cooked index template-matching patch
introduced a failure in libstdc++ debug mode. In particular, the new
code violates the assumption of std::lower_bound and std::upper_bound
that the range is sorted with respect to the comparison.
When I first debugged this, I thought the problem was unfixable as-is
and that a second layer of filtering would have to be done. However,
on irc, Simon pointed out that it could perhaps be solved if the
comparison function were assured that one operand always came from the
index, with the other always being the search string.
This patch implements this idea.
First, a new mode is introduced: a sorting mode for
cooked_index_entry::compare. In this mode, strings are compared
case-insensitively, but we're careful to always sort '<' before any
other printable character. This way, two names like "func" and
"func<param>" will be sorted next to each other -- i.e., "func1" will
not be seen between them. This is important when searching.
Second, the compare function is changed to work in a strcmp-like way.
This makes it easier to test and (IMO) understand.
Third, the compare function is modified so that in non-sorting modes,
the index entry is always the first argument. This allows consistency
in compares.
I regression tested this in libstdc++ debug mode on x86-64 Fedora 36.
It fixes the crash that Simon saw.
This is v2. I believe it addresses the review comments, except for
the 'enum class' change, as I mentioned in email on the list.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Users of addrmap::find and addrmap::foreach that have a const addrmap
should ideally receive const pointers to objects, to indicate they
should not be modified. However, users that have a non-const addrmap
should still receive a non-const pointer.
To achieve this, without adding more virtual methods, make the existing
find and foreach virtual methods private and prefix them with "do_".
Add small non-const and const wrappers for find and foreach.
Obviously, the const can be cast away, but if using static_cast
instead of C-style casts, then the compiler won't let you cast
the const away. I changed all the callers of addrmap::find and
addrmap::foreach I could find to make them use static_cast.
Change-Id: Ia8e69d022564f80d961413658fe6068edc71a094
I noticed that iterating over the index yields non-const
cooked_index_entry objects. However, after finalization, they should
not be modified. This patch enforces this by adding const where
needed.
v2 makes the find, all_entries, and wait methods const as well.
I hesitated between putting the file in the dwarf2 directory (as
gdb/dwarf2/call-site.h) or in the common directory (as gdb/call-site.h).
The concept of call site is not DWARF-specific, another debug info
reader could provide this information. But as it is, the implementation
is a bit DWARF-specific, as one form it can take is a DWARF expression
and parameters can be defined using a DWARF register number. So I ended up
choosing to put it under dwarf2/. If another debug info reader ever
wants to provide call site information, we can introduce a layer of
abstraction between the "common" call site and the "dwarf2" call site.
The copyright start year comes from the date `struct call_site` was
introduced.
Change-Id: I1cd84aa581fbbf729edc91b20f7d7a6e0377014d
Reviewed-By: Bruno Larsen <blarsen@redhat.com>
PR c++/29896 points out a regression in the new DWARF reader. It does
not properly handle a case like "break fn", where "fn" is a template
function.
This happens because the new index uses strncasecmp to compare.
However, to make this work correctly, we need a custom function that
ignores template parameters.
This patch adds a custom comparison function and fixes the bug. A new
test case is included.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29896
This commit is the result of running the gdb/copyright.py script,
which automated the update of the copyright year range for all
source files managed by the GDB project to be updated to include
year 2023.
PR symtab/29694 points out a regression caused by the new DWARF
scanner when the cc-with-gdb-index target board is used.
What happens here is that an older version of gdb will make an index
describing the "A" type as:
[737] A: 1 [global, type]
whereas the new gdb says:
[1008] A: 0 [global, type]
Here the old one is correct because the A in CU 0 is just a
declaration without a size:
<1><45>: Abbrev Number: 10 (DW_TAG_structure_type)
<46> DW_AT_name : A
<48> DW_AT_declaration : 1
<48> DW_AT_sibling : <0x6d>
This patch fixes the problem by introducing the idea of a "type
declaration". I think gdb still needs to recurse into these types,
searching for methods, but by marking the type itself as a
declaration, gdb can skip this type during lookups and when writing
the index.
Regression tested on x86-64 using the cc-with-gdb-index board.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29694
When building gdb with -fsanitize-threads and running test-case
gdb.ada/char_enum_unicode.exp, I run into:
...
WARNING: ThreadSanitizer: data race (pid=21301)^M
Write of size 8 at 0x7b2000008080 by main thread:^M
#0 free <null> (libtsan.so.2+0x4c5e2)^M
#1 _dl_close_worker <null> (ld-linux-x86-64.so.2+0x4b7b)^M
#2 convert_between_encodings() charset.c:584^M
...
#21 cooked_index_functions::expand_symtabs_matching() read.c:18606
...
This is fixed by making cooked_index_functions::expand_symtabs_matching wait
for the cooked index finalization to be done.
Tested on x86_64-linux.
https://sourceware.org/bugzilla/show_bug.cgi?id=29311https://sourceware.org/bugzilla/show_bug.cgi?id=29286
