Commit Graph

100 Commits

Author SHA1 Message Date
Simon Marchi
a5cbe67512 gdb, gdbserver, gdbsupport: trim trailing whitespaces
I noticed my IDE (VSCode) starting to automatically trim trailing
whitespaces on save, despite the setting for it being disabled.  I
realized that this is because the .editorconfig file now has

    trim_trailing_whitespace = true

for many file types.  If we have this EditorConfig setting forcing
editors to trim trailing whitespaces, I think it would make sense to
clean up trailing whitespaces from our files.  Otherwise, people will
always get spurious whitespace changes when editing these files.

I did a mass cleanup using this command:

$ find gdb gdbserver gdbsupport -type f \( \
    -name "*.c" -o \
    -name "*.h" -o \
    -name "*.cc" -o \
    -name "*.texi" -o \
    -name "*.exp" -o \
    -name "*.tcl" -o \
    -name "*.py" -o \
    -name "*.s" -o \
    -name "*.S" -o \
    -name "*.asm" -o \
    -name "*.awk" -o \
    -name "*.ac" -o \
    -name "Makefile*" -o \
    -name "*.sh" -o \
    -name "*.adb" -o \
    -name "*.ads" -o \
    -name "*.d" -o \
    -name "*.go" -o \
    -name "*.F90" -o \
    -name "*.f90" \
\) -exec sed -ri 's/[ \t]+$//' {} +

I then did an autotools regen, because we don't actually want to change
the Makefile and Makefile.in files that are generated.

Change-Id: I6f91b83e3b8c4dc7d5d51a2ebf60706120efe691
2025-10-20 15:44:08 -04:00
Tom Tromey
f283e80fed Fix use of "main" marker in gdb index
Tom de Vries noticed that with .gdb_index, the "main" marker would
sometimes seemingly be ignored.

I tracked this down to an interaction between the rewritten reader and
the "main"-finding code in cooked_index.  With the ordinary DWARF
scanner, a C "main" won't be marked as IS_MAIN; whereas with
.gdb_index this can happen.  In this case, the code thinks that C
requires canonicalization (which is only true for types), and skips
using the symbol.

This patch fixes the problem and adds some comments explaining what is
going on.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33441
2025-10-14 10:18:09 -06:00
Tom Tromey
5fe70629ce Change file initialization to use INIT_GDB_FILE macro
This patch introduces a new macro, INIT_GDB_FILE.  This is used to
replace the current "_initialize_" idiom when introducing a per-file
initialization function.  That is, rather than write:

    void _initialize_something ();
    void
    _initialize_something ()
    {
       ...
    }

... now you would write:

    INIT_GDB_FILE (something)
    {
       ...
    }

The macro handles both the declaration and definition of the function.

The point of this approach is that it makes it harder to accidentally
cause an initializer to be omitted; see commit 2711e475 ("Ensure
cooked_index_entry self-tests are run").  Specifically, the regexp now
used by make-init-c seems harder to trick.

New in v2: un-did some erroneous changes made by the script.

The bulk of this patch was written by script.
Regression tested on x86-64 Fedora 41.
2025-06-26 06:15:59 -06:00
Tom de Vries
c6115b5eac [gdb/cli] Use captured per_command_time in worker threads
With test-case gdb.base/maint.exp, I ran into:
...
(gdb) file maint^M
Reading symbols from maint...^M
(gdb) mt set per-command on^M
(gdb) Time for "DWARF indexing worker": ...^M
Time for "DWARF indexing worker": ...^M
Time for "DWARF indexing worker": ...^M
Time for "DWARF indexing worker": ...^M
Time for "DWARF skeletonless type units": ...^M
Time for "DWARF add parent map": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
Time for "DWARF finalize worker": ...^M
FAIL: $exp: warnings: per-command: mt set per-command on (timeout)
mt set per-command off^M
2025-05-31 09:33:44.711 - command started^M
(gdb) PASS: $exp: warnings: per-command: mt set per-command off
...

I didn't manage to reproduce this by rerunning the test-case, but it's fairly
easy to reproduce using a file with more debug info, for instance gdb:
...
$ gdb -q -batch -ex "file build/gdb/gdb" -ex "mt set per-command on"
...

Due to the default "mt dwarf synchronous" == off, the file command starts
building the cooked index in the background, and returns immediately without
waiting for the result.

The subsequent "mt set per-command on" implies "mt set per-command time on",
which switches on displaying of per-command execution time.

The "Time for" lines are the result of those two commands, but these lines
shouldn't be there because "mt per-command time" == off at the point of
issuing the file command.

Fix this by capturing the per_command_time variable, and using the captured
value instead.

Tested on x86_64-linux.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

PR cli/33039
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33039
2025-06-03 08:59:58 +02:00
Simon Marchi
cc55260231 gdb: add some scoped_time_its to profile startup time
I'm investigating some issues where GDB takes a lot of time to start
up (read: for the DWARF index to be ready to do anything useful).
Adding those scoped_time_it instances helped me gain some insights about
where GDB spends time.  I think they would be useful to have upstream,
to make investigating future problems easier.  It would also be useful
to be able to give some numbers in the commit messages.

Here's an example of what GDB outputs:

    Time for "minsyms install worker": wall 0.045, user 0.040, sys 0.004, user+sys 0.044, 97.8 % CPU
    Time for "minsyms install worker": wall 0.511, user 0.457, sys 0.048, user+sys 0.505, 98.8 % CPU
    Time for "minsyms install worker": wall 1.513, user 1.389, sys 0.111, user+sys 1.500, 99.1 % CPU
    Time for "minsyms install worker": wall 1.688, user 1.451, sys 0.102, user+sys 1.553, 92.0 % CPU
    Time for "minsyms install worker": wall 1.897, user 1.518, sys 0.089, user+sys 1.607, 84.7 % CPU
    Time for "minsyms install worker": wall 2.811, user 2.558, sys 0.231, user+sys 2.789, 99.2 % CPU
    Time for "minsyms install worker": wall 3.257, user 3.049, sys 0.188, user+sys 3.237, 99.4 % CPU
    Time for "minsyms install worker": wall 3.617, user 3.089, sys 0.211, user+sys 3.300, 91.2 % CPU
    Time for "DWARF indexing worker": wall 19.517, user 0.894, sys 0.075, user+sys 0.969, 5.0 % CPU
    Time for "DWARF indexing worker": wall 19.807, user 0.891, sys 0.086, user+sys 0.977, 4.9 % CPU
    Time for "DWARF indexing worker": wall 20.270, user 1.559, sys 0.119, user+sys 1.678, 8.3 % CPU
    Time for "DWARF indexing worker": wall 20.329, user 1.878, sys 0.147, user+sys 2.025, 10.0 % CPU
    Time for "DWARF indexing worker": wall 20.848, user 3.483, sys 0.224, user+sys 3.707, 17.8 % CPU
    Time for "DWARF indexing worker": wall 21.088, user 4.285, sys 0.295, user+sys 4.580, 21.7 % CPU
    Time for "DWARF indexing worker": wall 21.109, user 4.501, sys 0.274, user+sys 4.775, 22.6 % CPU
    Time for "DWARF indexing worker": wall 21.198, user 5.087, sys 0.319, user+sys 5.406, 25.5 % CPU
    Time for "DWARF skeletonless type units": wall 4.024, user 3.858, sys 0.115, user+sys 3.973, 98.7 % CPU
    Time for "DWARF add parent map": wall 0.092, user 0.086, sys 0.004, user+sys 0.090, 97.8 % CPU
    Time for "DWARF finalize worker": wall 0.278, user 0.241, sys 0.009, user+sys 0.250, 89.9 % CPU
    Time for "DWARF finalize worker": wall 0.307, user 0.304, sys 0.000, user+sys 0.304, 99.0 % CPU
    Time for "DWARF finalize worker": wall 0.727, user 0.719, sys 0.000, user+sys 0.719, 98.9 % CPU
    Time for "DWARF finalize worker": wall 0.913, user 0.901, sys 0.003, user+sys 0.904, 99.0 % CPU
    Time for "DWARF finalize worker": wall 0.776, user 0.767, sys 0.004, user+sys 0.771, 99.4 % CPU
    Time for "DWARF finalize worker": wall 1.897, user 1.869, sys 0.006, user+sys 1.875, 98.8 % CPU
    Time for "DWARF finalize worker": wall 2.534, user 2.512, sys 0.005, user+sys 2.517, 99.3 % CPU
    Time for "DWARF finalize worker": wall 2.607, user 2.583, sys 0.006, user+sys 2.589, 99.3 % CPU
    Time for "DWARF finalize worker": wall 3.142, user 3.094, sys 0.022, user+sys 3.116, 99.2 % CPU

Change-Id: I9453589b9005c3226499428ae9cab9f4a8c22904
Approved-By: Tom Tromey <tom@tromey.com>
2025-04-29 15:10:17 -04:00
Tom Tromey
d01e823438 Update copyright dates to include 2025
This updates the copyright headers to include 2025.  I did this by
running gdb/copyright.py and then manually modifying a few files as
noted by the script.

Approved-By: Eli Zaretskii <eliz@gnu.org>
2025-04-08 10:54:39 -06:00
Tom Tromey
60ac9c60fe Clean up cooked_index::done_reading
The cooked index worker maintains the state for the various state
transition in the scanner.  It is held by the cooked_index while
scanning is in progress, then deleted once this has completed.

I noticed that none of the arguments to cooked_index::done_reading
were really needed -- the cooked_index already has access to the
worker should it need it.  Removing these parameters makes the code a
bit simpler and also cleans up some confusing code around the use of
the deferred warnings object.

Regression tested on x86-64 Fedora 40.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2025-04-02 15:50:04 -06:00
Simon Marchi
27c130fbdc gdb/dwarf2: remove unused includes
Remove some includes reported as unused by clangd.

Change-Id: I841938c3c6254e4f0d154a1e172c4968ff326333
2025-04-02 12:42:35 -04:00
Tom Tromey
51b1421594 Move cooked_index_worker to cooked-index-worker.[ch]
This moves the cooked_index_worker class to cooked-index-worker.[ch].

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2025-04-01 07:30:10 -06:00
Tom Tromey
3351f741c8 Move cooked_index_shard to new files
This moves cooked_index_shard to a couple of new files,
dwarf2/cooked-index-shard.[ch].  The rationale is the same as the
previous patch: cooked-index.h had to be split to enable other
cleanups.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2025-04-01 07:30:10 -06:00
Tom Tromey
5bc2273db4 Move cooked_index_entry to new files
This moves cooked_index_entry and some related helper code to a couple
of new files, dwarf2/cooked-index-entry.[ch].

The main rationale for this is that in order to finish this series and
remove "cooked_index_worker::result_type", I had to split
cooked-index.h into multiple parts to avoid circular includes.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2025-04-01 07:30:10 -06:00
Tom Tromey
5021c5bbf8 Make language_requires_canonicalization 'static'
language_requires_canonicalization is only called from cooked-index.c,
so mark it as static.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2025-04-01 07:30:10 -06:00
Simon Marchi
ebfdb1089b gdb/dwarf: use std::equal_range in cooked_index_shard::find
Looking at `cooked_index_shard::find`, I thought that we could make a
small optimization: when finding the upper bound, we already know the
lower bound.  And we know that the upper bound is >= the lower bound.
So we could pass `lower` as the first argument of the `std::upper_bound`
call to cut the part of the search space that is below `lower`.

It then occured to me that what we do is basically what
`std::equal_range` is for, so why not use it.  Implementations of
`std::equal_range` are likely do to things as efficiently as possible.

Unfortunately, because `cooked_index_entry::compare` is sensitive to the
order of its parameters, we need to provide two different comparison
functions (just like we do know, to the lower_bound and upper_bound
calls).  But I think that the use of equal_range makes it clear what the
intent of the code is.

Regression tested using the various DWARF target boards on Debian 12.

Change-Id: Idfad812fb9abae1b942d81ad9976aeed7c2cf762
Approved-By: Tom Tromey <tom@tromey.com>
2025-03-25 11:44:37 -04:00
Simon Marchi
8cbf7d2a47 gdb/dwarf: remove unnecessary comparison in cooked_index_entry::compare
I believe that the `(mode == MATCH && a == munge ('<'))` part of the
condition is unnecesary.  Or perhaps I don't understand the algorithm.

The use of "munge" above effectively makes it so that the template
portion of names is completely ignored for the sake of the comparison.

Then, in the condition, this:

    a == munge ('<')

is functionally equivalent to

    a == '\0'

If `a` is indeed '\0', and `b` is also '\0', then we would have taken
the earlier branch:

    if (a == b)
      return 0;

If `b` is not '\0', then we won't take this branch and we'll go into the
final comparison:

    return a < b ? -1 : 1;

So, as far as I can see, there is no case where `mode == MATCH`, where
we're going to use this special `return 0`.

Regression tested using the various DWARF target boards on Debian 12.

Change-Id: I5ea0463c1fdbbc1b003de2f0a423fd0073cc9dec
Approved-By: Tom Tromey <tom@tromey.com>
2025-03-25 11:32:35 -04:00
Simon Marchi
aca980917b gdb/dwarf: remove unused cooked_index::cooked_index parameter
Following the previous patch, this parameter is now unused.  Remove it.

Change-Id: I7e96a3ba61ad9a0d6b64f9129aeeb9a8f3da22a7
Approved-By: Tom Tromey <tom@tromey.com>
2025-03-17 16:14:08 -04:00
Simon Marchi
6fca4d9694 gdbsupport: add some -Wunused-* warning flags
Add a few -Wunused-* diagnostic flags that look useful.  Some are known
to gcc, some to clang, some to both.  Fix the fallouts.

-Wunused-const-variable=1 is understood by gcc, but not clang.
-Wunused-const-variable would be undertsood by both, but for gcc at
least it would flag the unused const variables in headers.  This doesn't
make sense to me, because as soon as one source file includes a header
but doesn't use a const variable defined in that header, it's an error.
With `=1`, gcc only warns about unused const variable in the main source
file.  It's not a big deal that clang doesn't understand it though: any
instance of that problem will be flagged by any gcc build.

Change-Id: Ie20d99524b3054693f1ac5b53115bb46c89a5156
Approved-By: Tom Tromey <tom@tromey.com>
2025-03-17 16:14:08 -04:00
Simon Marchi
cc2f268d14 gdb/dwarf: use gdb::unordered_set for seen_names
Direct replacement of an htab with a gdb::unordered_set.

Using a large test program, I see a small but consistent performance
improvement.  The "file" command time goes on average from 7.88 to 7.73
seconds (~2%).  To give a rough estimate of the scale of the test
program, the 8 seen_names hash tables (one for each worker thread) had
between 173846 and 866961 entries.

Change-Id: I0157cbd04bb55338bb1fcefd2690aeef52fe3afe
Approved-By: Tom Tromey <tom@tromey.com>
2025-03-17 12:30:18 -04:00
Tom Tromey
2609ee6f19 Add string cache and use it in cooked index
The cooked index needs to allocate names in some cases -- when
canonicalizing or when synthesizing Ada package names.  This process
currently uses a vector of unique_ptrs to manage the memory.

Another series I'm writing adds another spot where this allocation
must be done, and examining the result showed that certain names were
allocated multiple times.

To clean this up, this patch introduces a string cache object and
changes the cooked indexer to use it.  I considered using bcache here,
but bcache doesn't work as nicely with string_view -- because bcache
is fundamentally memory-based, a temporary copy of the contents must
be made to ensure that bcache can see the trailing \0.  Furthermore,
writing a custom class lets us avoid another copy when canonicalizing
C++ names.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2025-03-10 15:05:34 -06:00
Tom Tromey
c05c9914b1 Use flags enum for cooked_index_entry::full_name
I found a small bug coming from a couple of  recent patches of mine for
cooked_index_entry::full_name.

First, commit aab26529b3 (Add "Ada linkage" mode to
cooked_index_entry::full_name) added a small hack to optionally
compute the Ada linkage name.

Then, commit aab2ac34d7 (Avoid excessive CU expansion on failed
matches) changed the relevant expand_symtabs_matching implementation
to use this feature.

However, the feature was used unconditionally, causing a bad side
effect: the non-canonical name is now used for all languages, not just
Ada.  But, for C++ this is wrong.

Furthermore, consider the declaration of full_name:

   const char *full_name (struct obstack *storage,
			 bool for_main = false,
			 bool for_ada_linkage = false,
 			 const char *default_sep = nullptr) const;

... and then consider this call in cooked_index::dump:

       gdb_printf ("    qualified:  %s\n",
		  entry->full_name (&temp_storage, false, "::"));

Oops!  The "::" is silently converted to 'true' here.

To fix both of these problems, this patch changes full_name to accept
a flags enum rather than booleans.  This avoids the type-safety
problem.

Then, full_name is changed to remove the "Ada" flag when the entry is
not in fact an Ada symbol.

Regression tested on x86-64 Fedora 40.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2025-03-10 13:40:25 -06:00
Tom Tromey
aa24bf2c05 Add support for hierarchical Ada names
In the near future, GNAT will start emitting DWARF names in a more
standard way -- specifically, the package structure will be indicated
by nested DW_TAG_module DIEs and a given entity will be nested in its
package and only have a simple name.

This patch changes gdb to understand this style of naming, while still
supporting the existing GNAT output.

A few special cases are needed.  I've commented them.

The name-computing code for the full DWARF reader is very complicated
-- much too complicated, in my opinion.  There are already several
bugs in bugzilla about this (search for "physname"... but there are
others as well), so I haven't filed any new ones.

When I started this project, I thought it would solve some memory
overuse issues we sometimes see from how the index-sharding code
interacts with the GNAT-specific post-pass.  However, to my surprise,
the Ada code in gdb relies on some details of symbol naming, and so
I've had to add code here to synthesize "linkage" names in some cases.
This is unfortunate, but I think can eventually be fixed; I will file
a bug to track this issue.
2025-03-06 14:17:18 -07:00
Tom Tromey
aab26529b3 Add "Ada linkage" mode to cooked_index_entry::full_name
Unfortunately, due to some details of how the Ada support in gdb
currently works, the DWARF reader will still have to synthesize some
"full name" entries after the cooked index has been constructed.

You can see one particular finding related to this in:

    https://sourceware.org/bugzilla/show_bug.cgi?id=32142

This patch adds a new flag to cooked_index_entry::full_name to enable
the construction of these names.

I hope to redo this part of the Ada support eventually, so that this
code can be removed and the full-name entries simply not created.
2025-03-06 14:17:18 -07:00
Tom Tromey
c70ac07a79 Store new Ada entries in cooked_index_shard::m_entries
handle_gnat_encoded_entry might create synthetic cooked index entries
for Ada packages.  These aren't currently kept in m_entries, but it
seems to me that they should be, particularly because a forthcoming
GNAT will emit explicit DW_TAG_module for these names -- with this
change, the indexes will be roughly equivalent regardless of which
compiler was used.
2025-03-06 14:17:18 -07:00
Tom Tromey
4a4a50517b Add "synthetic" marker for index entries
Currently, gdb will synthesize DW_TAG_module entries for Ada names.
These entries are treated specially by the index writer,

When GNAT starts emitting DW_TAG_module, the special case will be
incorrect, because there will be non-synthetic DW_TAG_module entries
in the index.

This patch arranges to mark the synthetic entries and changes the
index writer to follow.
2025-03-06 14:17:17 -07:00
Tom Tromey
e382ede5ea Use DW_TAG_module for Ada
In GCC we decided to use DW_TAG_module to represent Ada packages, so
make this same decision in gdb.  This also updates tag_matches_domain
to handle this case.
2025-03-06 14:17:17 -07:00
Tom Tromey
9d3fbbd4c4 Use "::" as separator for Fortran in cooked index
This teaches cooked_index_entry::full_name that "::" is the separator
for Fortran.  I don't know enough Fortran to write a test case for
this.  However, a different series I am working on has a regression if
this patch is not applied.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2025-03-06 07:33:28 -07:00
Tom Tromey
268c8bda25 Add language to type unit in debug-names-tu.exp.tcl
I think debug-names-tu.exp.tcl only passes by accident -- the type
unit does not have a language, which gdb essentially requires.

This isn't noticeable right now because the type unit in question is
expanded in one phase and then the symbol found in another.  However,
I'm working on a series that would regress this.

This patch partially fixes the problem by correcting the test case,
adding the language to the TU.

Hoewver, it then goes a bit further and arranges for this information
not to be written to .debug_names.  Whether or not a type should be
considered "static" seems like something that is purely internal to
gdb, so this patch has the entry-creation function apply the
appropriate transform.

It also may make sense to change the "debug_names" proc in the test
suite to process attributes more like the ordinary "cu" proc does.
2025-03-03 14:16:44 -07:00
Simon Marchi
b55c841c51 gdb/dwarf: rename dwarf2_per_cu_data -> dwarf2_per_cu
This scratches an itch I had for a while.  I don't know why this struct
type has "data" in its name.  Others like "dwarf2_per_objfile" and
"dwarf2_per_bfd" don't.  The primary job of a structure is to hold data,
there's no need to specify it.  It also makes the name a bit shorter,
which is always nice.

Rename related types too.

Change-Id: Ifb63195ff105809fc15b502f639c0bb4d18a675e
Approved-By: Tom Tromey <tom@tromey.com>
Reviewed-By: Guinevere Larsen <guinevere@redhat.com>
2025-03-03 15:57:03 -05:00
Simon Marchi
890d1d385f gdb/dwarf: std::unordered_{set,map} -> gdb::unordered_{set,map} throughout
No behavior changes expected.

Change-Id: I16ff6c67058362c65cc8edb05d1948e48be6b2e1
Approved-By: Tom Tromey <tom@tromey.com>
2025-02-19 11:14:41 -05:00
Simon Marchi
f24d402bf8 gdb/dwarf: unique_ptr cleanup
Throughout gdb/dwarf2, use `*_up` typedefs.  Add a few missing typedefs,
and move some so they are, ideally, just after the corresponding class.

Change-Id: Iab5cd8fc2e9989d4bd8d4868586703c2312f254f
Approved-By: Tom Tromey <tom@tromey.com>
2025-02-14 13:17:35 -05:00
Simon Marchi
8e745eac7d gdb/dwarf: rename cooked_index::m_vector to m_shards
I think that is clearer and helps readability.

Rename a few iteration variables from "index" or "idx" to "shard".  In
my mental model, the "index" is the whole thing, so it's confusing to
use that word when referring to shards.

Change-Id: I208cb839e873c514d1f8eae250d4a16f31016148
Approved-By: Tom Tromey <tom@tromey.com>
2025-02-11 10:10:31 -05:00
Simon Marchi
6c6492e7d3 gdb/dwarf: remove cooked_index::vec_type
I find this typedef to be confusing.  The name is a bit too generic, so
it's not clear what it represents.  When using the typedef for a
cooked_index_shard unique pointer, I think that spelling out the vector
type is not overly long.

Change-Id: I99fdab5cd925c37c3835b466ce40ec9c1ec7209d
Approved-By: Tom Tromey <tom@tromey.com>
2025-02-11 10:10:31 -05:00
Simon Marchi
de33cf88da gdb/dwarf: allow for cooked_index_shard::m_addrmap to be nullptr
The following patch makes the .debug_names reader create multiple cooked
index shards, only one of them having an address map.  The others will
have a nullptr address map.

Change the code using cooked_index_shard::m_addrmap to account for the
fact that it can be nullptr.

Change-Id: Id05b974e661d901dd43bb5ecb3a8fcfc15abc7ed
Approved-By: Tom Tromey <tom@tromey.com>
2025-02-10 11:28:56 -05:00
Simon Marchi
bbd252584f gdb: remove includes from dwarf2/mapped-index.h
They are unused, according to clangd.

Add some includes to other files, which were relying on transitive
includes.

Change-Id: I3bcb4be93b3a18bf44a4068f4067e567f83e1d4f
2025-01-30 12:46:54 -05:00
Tom de Vries
163d991175 [gdb/build] Fix build with gcc 7.5.0
When building gdb with gcc 7.5.0, I run into:
...
gdb/dwarf2/cooked-index.c: In lambda function:
gdb/dwarf2/cooked-index.c:104:5: error: \
  the value of ‘_sch_tolower’ is not usable in a constant expression
     };
     ^
In file included from gdbsupport/gdb-safe-ctype.h:47:0,
                 from gdb/dwarf2/cooked-index.c:34:
include/safe-ctype.h:111:29: note: ‘_sch_tolower’ was not declared ‘constexpr’
 extern const unsigned char  _sch_tolower[256];
                             ^~~~~~~~~~~~
...

This does not happen with gcc 8.2.1.

Fix this by dropping the constexpr on lambda function munge in
cooked_index_entry::compare for gcc 7.

Tested by completing the build on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>
2025-01-27 16:38:32 +01:00
Tom Tromey
2e72448926 Fix C++ template function matching in cooked index
In commit 64a97606 ("Support template lookups in
strncmp_iw_with_mode"), gdb was changed so that a command like "break
func<templ>" would match instantiations like "func<templ<int>>".

The new indexer does not support this and so this is a regression.
This went unnoticed because gdb.linespec.cpcompletion.exp puts all
these functions into the main file, and this CU is expanded early.

This patch fixes the bug by changing the cooked index entry comparison
function.  It also updates the test to fail without this fix.

Regression tested on x86-64 Fedora 40.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32482
2025-01-24 13:53:11 -07:00
Tom de Vries
ced885161b [gdb/build, c++20] Fix more deprecated implicit capture of this
When building gdb with -std=c++20 I run into:
...
gdb/dwarf2/cooked-index.c: In lambda function:
gdb/dwarf2/cooked-index.c:471:47: error: implicit capture of ‘this’ via \
  ‘[=]’ is deprecated in C++20 [-Werror=deprecated]
  471 |   gdb::thread_pool::g_thread_pool->post_task ([=] ()
      |                                               ^
gdb/dwarf2/cooked-index.c:471:47: note: add explicit ‘this’ or ‘*this’ capture
...

Fix this and two more spots by removing the capture default, and explicitly
listing all captures.

Tested on x86_64-linux.
2024-10-21 08:04:07 +02:00
Tom de Vries
3e0c29b24a [gdb/symtab] Fix qualified name for cooked index dump
While looking at the cooked index entry for local variable l4 of function test
in test-case gdb.fortran/logical.exp:
...
$ gdb -q -batch outputs/gdb.fortran/logical/logical \
  -ex "maint print objfiles"
  ...
    [9] ((cooked_index_entry *) 0x7fc6e0003010)
    name:       l4
    canonical:  l4
    qualified:  l4
    DWARF tag:  DW_TAG_variable
    flags:      0x2 [IS_STATIC]
    DIE offset: 0x17c
    parent:     ((cooked_index_entry *) 0x7fc6e0002f20) [test]
...
I noticed that while the entry does have a parent, that's not reflected in the
qualified name.

This makes it harder to write test-cases that check the parent of a cooked
index entry.

This is due to the implementation of full_name, which skips printing
parents if the language does not specify an appropriate separator.

Fix this by using "::" as default separator, getting us instead:
...
    [9] ((cooked_index_entry *) 0x7f94ec0040c0)
    name:       l4
    canonical:  l4
    qualified:  test::l4
    DWARF tag:  DW_TAG_variable
    flags:      0x2 [IS_STATIC]
    DIE offset: 0x17c
    parent:     ((cooked_index_entry *) 0x7f94ec003fd0) [test]
...

Tested on x86_64-linux.

Approved-By: Tom Tromey <tom@tromey.com>
2024-10-18 00:15:57 +02:00
Tom Tromey
052736020a Refactor cooked_index_shard::handle_gnat_encoded_entry
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>
2024-09-09 11:37:35 -06:00
Simon Marchi
e5afccc7e4 gdb/dwarf2: cleanup includes
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>
2024-08-30 13:57:11 -04:00
Simon Marchi
05d9d66d92 gdb: remove unused includes in utils.h
Remove some includes reported as unused by clangd.  Add some includes in
other files that were previously relying on the transitive include.

Change-Id: Ibdd0a998b04d21362a20d0ca8e5267e21e2e133e
2024-05-30 22:43:52 -04:00
Hannes Domani
5140d8e013 Fix heap-use-after-free in index-cached with --disable-threading
If threads are disabled, either by --disable-threading explicitely, or by
missing std::thread support, you get the following ASAN error when
loading symbols:

==7310==ERROR: AddressSanitizer: heap-use-after-free on address 0x614000002128 at pc 0x00000098794a bp 0x7ffe37e6af70 sp 0x7ffe37e6af68
READ of size 1 at 0x614000002128 thread T0
    #0 0x987949 in index_cache_store_context::store() const ../../gdb/dwarf2/index-cache.c:163
    #1 0x943467 in cooked_index_worker::write_to_cache(cooked_index const*, deferred_warnings*) const ../../gdb/dwarf2/cooked-index.c:601
    #2 0x1705e39 in std::function<void ()>::operator()() const /gcc/9/include/c++/9.2.0/bits/std_function.h:690
    #3 0x1705e39 in gdb::task_group::impl::~impl() ../../gdbsupport/task-group.cc:38

0x614000002128 is located 232 bytes inside of 408-byte region [0x614000002040,0x6140000021d8)
freed by thread T0 here:
    #0 0x7fd75ccf8ea5 in operator delete(void*, unsigned long) ../../.././libsanitizer/asan/asan_new_delete.cc:177
    #1 0x9462e5 in cooked_index::index_for_writing() ../../gdb/dwarf2/cooked-index.h:689
    #2 0x9462e5 in operator() ../../gdb/dwarf2/cooked-index.c:657
    #3 0x9462e5 in _M_invoke /gcc/9/include/c++/9.2.0/bits/std_function.h:300

It's happening because cooked_index_worker::wait always returns true in
this case, which tells cooked_index::wait it can delete the m_state
cooked_index_worker member, but cooked_index_worker::write_to_cache tries
to access it immediately afterwards.

Fixed by making cooked_index_worker::wait only return true if desired_state
is CACHE_DONE, same as if threading was enabled, so m_state will not be
prematurely deleted.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31694
Approved-By: Tom Tromey <tom@tromey.com>
2024-05-04 18:55:20 +02:00
Simon Marchi
e5dc0d5d04 gdb: move a bunch of quit-related things to event-top.{c,h}
Move some declarations related to the "quit" machinery from defs.h to
event-top.h.  Most of the definitions associated to these declarations
are in event-top.c.  The exceptions are `quit()` and `maybe_quit()`,
that are defined in utils.c.  For consistency, move these two
definitions to event-top.c.

Include "event-top.h" in many files that use these things.

Change-Id: I6594f6df9047a9a480e7b9934275d186afb14378
Approved-By: Tom Tromey <tom@tromey.com>
2024-04-23 11:26:14 -04:00
Tom Tromey
4320a9c921 Correctly handle DIE parent computations
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
2024-04-16 11:54:46 -06:00
Tom Tromey
75670e0075 Avoid complaint warning on mingw
The mingw build currently issues a warning:

./../../src/gdb/utils.h:378:56: warning: ignoring attributes on template argument 'void(const char*, va_list)' {aka 'void(const char*, char*)'} [-Wignored-attributes]

This patch fixes the problem as suggested by Simon:

    https://sourceware.org/pipermail/gdb-patches/2024-April/207908.html

...that is, by changing the warning interceptor to a class with a
single 'warn' method.

Approved-By: Simon Marchi <simon.marchi@efficios.com>
2024-04-15 09:21:40 -06:00
Simon Marchi
18d2988e5d gdb, gdbserver, gdbsupport: remove includes of early headers
Now that defs.h, server.h and common-defs.h are included via the
`-include` option, it is no longer necessary for source files to include
them.  Remove all the inclusions of these files I could find.  Update
the generation scripts where relevant.

Change-Id: Ia026cff269c1b7ae7386dd3619bc9bb6a5332837
Approved-By: Pedro Alves <pedro@palves.net>
2024-03-26 21:13:22 -04:00
Tom Tromey
818ef5f413 Capture warnings when writing to the index cache
PR symtab/30837 points out a race that can occur when writing to the
index cache: a call to ada_encode can cause a warning, which is
forbidden on a worker thread.

This patch fixes the problem by arranging to capture any such
warnings.

This is v2 of the patch.  It is rebased on top of some other changes
in the same area.  v1 was here:

    https://sourceware.org/pipermail/gdb-patches/2024-February/206595.html

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30837
2024-03-26 09:49:43 -06:00
Tom Tromey
ed29a346be Avoid race when writing to index cache
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
2024-03-08 17:25:50 -07:00
Tom Tromey
ba9583c7d5 Move the 'store' method to index_cache_store_context
I think it is cleaner for 'store' to be a method on
index_cache_store_context rather than on the global index cache
itself.  This patch makes this change.
2024-03-08 17:25:50 -07:00
Tom Tromey
b183313dfa Capture the per-BFD object in index_cache_store_context
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.
2024-03-08 17:25:49 -07:00
Tom Tromey
974b36c2ae Use the new symbol domains
This patch changes the DWARF reader to use the new symbol domains.  It
also adjusts many bits of associated code to adapt to this change.

The non-DWARF readers are updated on a best-effort basis.  This is
somewhat simpler since most of them only support C and C++.  I have no
way to test a few of these.

I went back and forth a few times on how to handle the "tag"
situation.  The basic problem is that C has a special namespace for
tags, which is separate from the type namespace.  Other languages
don't do this.  So, the question is, should a DW_TAG_structure_type
end up in the tag domain, or the type domain, or should it be
language-dependent?

I settled on making it language-dependent using a thought experiment.
Suppose there was a Rust compiler that only emitted nameless
DW_TAG_structure_type objects, and specified all structure type names
using DW_TAG_typedef.  This DWARF would be correct, in that it
faithfully represents the source language -- but would not work with a
purely struct-domain implementation in gdb.  Therefore gdb would be
wrong.

Now, this approach is a little tricky for C++, which uses tags but
also enters a typedef for them.  I notice that some other readers --
like stabsread -- actually emit a typedef symbol as well.  And, I
think this is a reasonable approach.  It uses more memory, but it
makes the internals simpler.  However, DWARF never did this for
whatever reason, and so in the interest of keeping the series slightly
shorter, I've left some C++-specific hacks in place here.

Note that this patch includes language_minimal as a language that uses
tags.  I did this to avoid regressing gdb.dwarf2/debug-names-tu.exp,
which doesn't specify the language for a type unit.  Arguably this
test case is wrong.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30164
2024-01-28 10:58:16 -07:00