| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Remove some includes reported as unused by clangd.
Change-Id: I841938c3c6254e4f0d154a1e172c4968ff326333
|
|
This updates the cooked_index comment with some notes about object
lifetimes, in an attempt to make navigating this code a bit simpler.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
The two readers currently using cooked_index_worker shared some code.
This patch factors this out into a new "done_reading" method.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
cooked_index_worker::result_type is an ad hoc tuple type used for
transferring data between phases of the indexer. It's a bit unwieldy
and another patch I'm working on would be somewhat nicer without it.
This patch removes the type. Now cooked_index_ephemeral objects are
transferred instead, which is handy because they already hold the
needed state.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This updates the "See xyz.h" comments for all the methods that were
moved earlier in this series. Perhaps I should have removed them
instead.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This moves the cooked_index_worker class to cooked-index-worker.[ch].
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes cooked-index-worker.h to include the new header files.
This breaks the circular dependency that would otherwise be introduced
in the next patch.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
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>
|
|
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>
|
|
language_requires_canonicalization is only called from cooked-index.c,
so mark it as static.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This renames cooked_index_storage to cooked_index_worker_result,
making its function more clear. It also updates the class comment to
as well.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
A discussion with Simon made me realize that cooked_index_storage
isn't a very clear name, especially now that it's escaped from read.c.
While it does provide some storage (I guess any object does in a
sense), it is really a helper for cooked_index_worker -- a temporary
object that is destroyed after reading has completed.
This patch renames this file. Later patches will rename the class and
move cooked_index_worker here, something I think is reasonable given
that cooked_index_storage is really something of a helper class for
cooked_index_worker.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
GDB's compile subsystem is deeply tied to GDB's ability to understand
DWARF. A future patch will add the option to disable DWARF at configure
time, but for that to work, the compile subsystem will need to be
entirely disabled as well, so this patch adds that possibility.
I also think there is motive for a security conscious user to disable
compile for it's own sake. Considering that the code is quite
unmaintained, and depends on an equally unmaintained gcc plugin, there
is a case to be made that this is an unnecessary increase in the attack
surface if a user knows they won't use the subsystem. Additionally, this
can make compilation slightly faster and the final binary is around 3Mb
smaller. But these are all secondary to the main goal of being able to
disable dwarf at configure time.
To be able to achieve optional compilation, some of the code that
interfaces with compile had to be changed. All parts that directly
called compile things have been wrapped by ifdefs checking for compile
support. The file compile/compile.c has been setup in a similar way to
how python's and guile's main file has been setup, still being compiled
but only for with placeholder command.
Finally, to avoid several new errors, a new TCL proc was introduced to
gdb.exp, allow_compile_tests, which checks if the "compile" command is
recognized before the inferior is started and otherwise skips the compile
tests. All tests in the gdb.compile subfolder have been updated to use
that, and the test gdb.base/filename-completion also uses this. The proc
skip_compile_feature_tests to recognize when the subsystem has been
disabled at compile time.
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Change some parameters to be references instead of pointers, when the
value must not be nullptr. I'd like to do this more of this kind of
change, but I have to limit the scope of the change, otherwise there's
just no end (and some local variables could also be turned into
references). So for now, just do it the cutu_reader constructors.
Change-Id: I9442c6043726981d58f9b141f516c590c0a71bcc
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The comment on this constructor is really outdated. Update it to better
reflect the reality today.
I'd eventually like to change this cutu_reader constructor not to use
dwarf2_per_cu, because it seems like an abuse of dwarf2_per_cu just to
pass 3 values. But for now, just document the existing behavior.
Change-Id: Id96db020c361e64d9b0d2f25d51950b206658aa2
Approved-By: Tom Tromey <tom@tromey.com>
|
|
lookup_dwo_unit receives the name of the DWO unit to look up, as read
from the DW_AT_dwo_name attribute of the skeleton DIE. But then, it
doesn't use it:
/* Yeah, we look dwo_name up again, but it simplifies the code. */
dwo_name = dwarf2_dwo_name (comp_unit_die, cu);
Perhaps this comment made sense at some point, but with the code we have
today, I don't understand it. It should be fine to use the name passed
as a parameter, which the caller also obtained by calling
dwarf2_dwo_name.
Change-Id: I84723e12726f77e4202d042428ee0eed9962ceb8
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
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>
|
|
We have this pattern of check in multiple places:
/* Skip dummy compilation units. */
if (m_info_ptr >= begin_info_ptr + this_cu->length ()
|| peek_abbrev_code (abfd, m_info_ptr) == 0)
m_dummy_p = true;
In all places except one (read_cutu_die_from_dwo), this is done after
reading the unit header but before potentially reading the first DIE.
The effect is that we consider dummy units that have no DIE at all.
Either the "data" portion of the unit (the portion after the header) has
a size of zero, or the first abbrev code is 0, i.e. "end of list".
According to this old commit I found [1], dummy CUs were used as filler
for incremental LTO linking. A comment reads:
WARNING: If THIS_CU is a "dummy CU" (used as filler by the incremental
linker) then DIE_READER_FUNC will not get called.
In read_cutu_die_from_dwo, however, this check is done after having read
the first DIE. So at the time of the check, m_info_ptr has already been
advanced just past the first DIE. As a result, compilations units with
a single DIE are considered (erroneously, IMO) as dummy.
In commit aab6de1613df ("gdb/dwarf: fix spurious error when encountering
dummy CU") [2], I mentioned a real world case where compilation units
with a single top-level DIE were being considered dummy. I believe that
those units should not actually have been treated as dummy. A CU with
just one DIE may not be very interesting, but I don't see any reason to
consider it dummy.
Move the dummy check above the read_toplevel_die call, and return early
if the CU is dummy.
I am 99% convinced that it's not even possible to encounter an empty
unit here, and considered turning it into an assert (it did pass the
testsuite). This function is passed a dwo_unit, and functions that
create a dwo_unit are:
- create_debug_type_hash_table (creates a dwo_unit for each type unit
found in a dwo file)
- create_cus_hash_table (creates a dwo_unit for each comp unit found in
a dwo file)
- create_dwo_unit_in_dwp_v1
- create_dwo_unit_in_dwp_v2
- create_dwo_unit_in_dwp_v5
In the first two, there are already dummy checks, so we wouldn't even
get to read_cutu_die_from_dwo for such an empty CU. However, in the
last three, there is no such checks, we just trust the dwp file's index
and create dwo_units out of that. So I guess it would be possible to
craft a broken dwp file with a CU that has no DIE. Out of caution, I
didn't switch that to an assert, but I also don't really know what would
be the mode of failure if that were to happen.
Regtested using the various DWARF target boards on Debian 12.
[1] https://gitlab.com/gnutools/binutils-gdb/-/commit/dee91e82ae87f379c90fddff8db7c4b54a116609#dd409f60ba6f9c066432dafbda7093ac5eec76d1_3434_3419
[2] https://gitlab.com/gnutools/binutils-gdb/-/commit/aab6de1613df693059a6a2b505cc8f20d479d109
Change-Id: I90e6fa205cb2d23ebebeae6ae7806461596f9ace
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This parameter is always used to set cutu_reader::m_dwo_abbrev_table.
Remove the parameter, and have read_cutu_die_from_dwo set the field
directly.
Change-Id: I6c0c7d23591fb2c3d28cdea1befa4e6b379fd0d3
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This adds a new die_info::children method. This returns a range that
can be used to iterate over a DIE's children.
Then this goes through and updates all the relevant loops to use
foreach instead. This is a net code reduction.
You'll note that in some places the code was checking the tag as well,
like:
while (child_die && child_die->tag)
I believe this can't happen and is just a copy-paste oddity from the
old days.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
I want to add support for C++ foreach iteration over DIE siblings.
I considered writing a custom iterator for this, but it would be
largely identical to the already-existing next_iterator. I didn't
want to duplicate the code...
Then I tried parameterizing next_iterator by having it take an
optional pointer-to-member template argument. However, this would
involve changes in many places, because currently a next_iterator can
be instantiated before the underlying type is complete.
So in the end I decided to rename die_info::sibling to die_info::next.
This name is slightly worse but (1) IMO it isn't really all that bad,
nobody would have blinked if it was called 'next' in the initial
patch, and (2) with the change to iteration it is barely used.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
A recent patch of mine had a comment with bad grammar; apparently I
didn't finish editing it. This patch cleans it up.
|
|
Replace an htab with gdb::unordered_set. I think we could also use the
dwarf2_per_cu pointer itself as the identity, basically have the
functional equivalent of:
gdb::unordered_map<dwarf2_per_cu *, cutu_reader_up>
But I kept the existing behavior of using dwarf2_per_cu::index as the
identity.
Change-Id: Ief3df9a71ac26ca7c07a7b79ca0c26c9d031c11d
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The type_unit_group is an indirection between a stmt_list_hash (possible
dwo_unit + line table section offset) and a type_unit_group_unshareable
that provides no real value. In dwarf2_per_objfile, we maintain a
stmt_list_hash -> type_unit_group mapping, and in dwarf2_per_objfile, we
maintain a type_unit_group_unshareable mapping. The type_unit_group
type is empty and only exists to have an identity and to be a link
between the two mappings.
This patch changes it so that we have a single stmt_list_hash ->
type_unit_group_unshareable mapping.
Regression tested on Debian 12 amd64 with a bunch of DWARF target
boards.
Change-Id: I9c5778ecb18963f353e9dd058e0f8152f7d8930c
Approved-By: Tom Tromey <tom@tromey.com>
|
|
dwarf2_per_bfd::{quick_file_names_table,type_unit_groups}
Change these two hash tables to use gdb::unordered_map. I changed these
two at the same time because they both use the same key, a
stmt_list_hash. Unlike other previous patches that used a
gdb::unordered_set, use an unordered_map here because the key isn't
found in the element itself (well, it was before, because of how htab
works, but it didn't need to be).
You'll notice that the type_unit_group structure is empty. That
structure isn't really needed. It is removed in the following patch.
Regression tested on Debian 12 amd64 with a bunch of DWARF target
boards.
Change-Id: Iec2289958d0f755cab8198f5b72ecab48358ba11
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This removes attribute::is_nonnegative and attribute::as_nonnegative
in favor of a call to unsigned_constant.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
I noticed that gdb doesn't handle DW_END_default. This patch adds
support for this.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes get_alignment to assume that DW_AT_alignment refers to an
unsigned value.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes read_decl_line and new_symbol to assume that
DW_AT_decl_line should refer to an unsigned value.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This changes dwarf2_record_block_entry_pc to issue a complaint using
the form name rather than a value. This seems more correct to me.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This introduces a new 'unsigned_constant' method on attribute. This
method can be used to get the value as an unsigned number. Unsigned
scalar forms are handled, and signed scalar forms are handled as well
provided that the value is non-negative.
Several spots in the reader that expect small DWARF-defined constants
are updated to use this new method.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32680
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
This renames attribute::form_is_signed to form_is_strictly_signed. I
think this more accurately captures what it does: it says whether a
form will always use signed data -- not whether a form might use
signed data, which DW_FORM_data* do depending on context.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=32680
Approved-By: Simon Marchi <simon.marchi@efficios.com>
|
|
read_cutu_die_from_dwo currently returns the dwo's top-level DIE through
a parameter. Following the previous patch, all code paths end up
setting m_top_level_die. Simplify this by having read_cutu_die_from_dwo
set m_top_level_die directly. I think it's easier to understand,
because there's one less indirection to follow.
Change-Id: Ib659f1d2e38501a8fe2b5dd0ca2add3ef55e8d60
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I built an application with -gsplit-dwarf (i.e. dwo), and some CUs are
considered "dummy" by the DWARF reader. That is, the top-level DIE
(DW_TAG_compile_unit) does not have any children. Here's the skeleton:
0x0000c0cb: Compile Unit: length = 0x0000001d, format = DWARF32, version = 0x0005, unit_type = DW_UT_skeleton, abbr_offset = 0x529b, addr_size = 0x08, DWO_id = 0x0ed2693dd2a756dc (next unit at 0x0000c0ec)
0x0000c0df: DW_TAG_skeleton_unit
DW_AT_stmt_list [DW_FORM_sec_offset] (0x09dee00f)
DW_AT_dwo_name [DW_FORM_strp] ("CMakeFiles/lib_crl.dir/crl/dispatch/crl_dispatch_queue.cpp.dwo")
DW_AT_comp_dir [DW_FORM_strp] ("/home/simark/src/tdesktop/build-relwithdebuginfo-split-nogz/Telegram/lib_crl")
DW_AT_GNU_pubnames [DW_FORM_flag_present] (true)
And here's the entire debug info in the .dwo file:
.debug_info.dwo contents:
0x00000000: Compile Unit: length = 0x0000001a, format = DWARF32, version = 0x0005, unit_type = DW_UT_split_compile, abbr_offset = 0x0000, addr_size = 0x08, DWO_id = 0x0ed2693dd2a756dc (next unit at 0x0000001e)
0x00000014: DW_TAG_compile_unit
DW_AT_producer [DW_FORM_strx] ("GNU C++20 14.2.1 20250207 -mno-direct-extern-access -mtune=generic -march=x86-64 -gsplit-dwarf -g3 -gz=none -O2 -std=gnu++20 -fPIC -fno-strict-aliasing")
DW_AT_language [DW_FORM_data1] (DW_LANG_C_plus_plus_14)
DW_AT_name [DW_FORM_strx] ("/home/simark/src/tdesktop/Telegram/lib_crl/crl/dispatch/crl_dispatch_queue.cpp")
DW_AT_comp_dir [DW_FORM_strx] ("/home/simark/src/tdesktop/build-relwithdebuginfo-split-nogz/Telegram/lib_crl")
When loading the binary in GDB, I see some warnings:
$ ./gdb -q -nx --data-directory=data-directory -ex 'maint set dwarf sync on' -ex "file /home/simark/src/tdesktop/build-relwithdebuginfo-split-nogz/telegram-desktop"
Reading symbols from /home/simark/src/tdesktop/build-relwithdebuginfo-split-nogz/telegram-desktop...
DWARF Error: unexpected tag 'DW_TAG_skeleton_unit' at offset 0xc0cb
DWARF Error: unexpected tag 'DW_TAG_skeleton_unit' at offset 0xc152
DWARF Error: unexpected tag 'DW_TAG_skeleton_unit' at offset 0xc194
DWARF Error: unexpected tag 'DW_TAG_skeleton_unit' at offset 0xc1b5
(gdb)
It turns out that these errors are not really justified. What happens
is:
- cutu_reader::read_cutu_die_from_dwo return 0, indicating that the CU
is "dummy"
- back in cutu_reader::cutu_reader, we omit setting m_top_level_die to
the DIE from the dwo file, meaning that m_top_level_die keeps
pointing to the DIE from the main file (DW_TAG_skeleton_unit)
- later, in cutu_reader::prepare_one_comp_unit, there is a check that
m_top_level_die->tag is one of DW_TAG_{compile,partial,type}_unit,
which triggers
My proposal to fix this is to set m_top_level_die even if the CU is
dummy. Even if the top-level DIE does not have any children, I don't
see any reason to leave cutu_reader::m_top_level_die in a different
state than when the CU is not dummy.
While at it, set m_dummy_p directly in read_cutu_die_from_dwo, instead
of returning a value and having the caller do it. This is all inside
cutu_reader anyway.
Change-Id: I483a68a369bb461a8dfa5bf2106ab1d6a0067198
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This function, as can be seen by its comment, is a remnant of past
design. Inline its content into create_cus_hash_table.
Change-Id: Id900bae2cdce8f33bf01199fb1d366646effc76e
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Following the previous patch, this parameter is now unused. Remove it.
Change-Id: I7e96a3ba61ad9a0d6b64f9129aeeb9a8f3da22a7
Approved-By: Tom Tromey <tom@tromey.com>
|
|
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>
|
|
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>
|
|
After staring at the code, I got convinced that it was not possible for
load_full_comp_unit to be called while a dwarf2_cu object exists in
per_objfile for this_cu. If you follow all callers of
load_full_comp_unit, you can see that all calls to load_full_comp_unit
(except one, see below) are gated one way or another by the fact that:
per_objfile->get_cu (per_cu) == nullptr
Some calls are gated by maybe_queue_comp_unit returning true. If it
returns true, then necessarily the dwarf2_cu is unset for that per_cu.
The spot that didn't seem to check for whether the dwarf2_cu is already
set before calling load_full_comp_unit is dw2_do_instantiate_symtab. It
didn't trigger when running the testsuite, but I could imagine a made up
case where the dwarf2_cu would already be set because we looked up a DIE
reference to it (follow_die_ref) for whatever reason. Then, something
would cause the symtab for that CU to be expanded and
dw2_do_instantiate_symtab to be called.
I added a check in that function, because it seemed prudent to do so.
All other load_cu calls are gated by this check, so it makes this call
look just like the others.
Finally, because all call sites that use cutu_reader::release_cu pass
nullptr for `existing_cu` (and therefore cutu_reader creates a new
dwarf2_cu), we know that cutu_reader::release_cu will always return a
non-nullptr value. Add an assert in it and remove checks in
load_full_comp_unit and read_signatured_type.
Change-Id: I496be34bd4bf7edfa38d5135cf4bc4ccd960abe2
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Following the previous patch, all callers now pass the same thing:
per_objfile->get_cu (this_cu)
Remove that parameter and to the call in the function itself.
Change-Id: Iafd36b058d7b95efae518bb65035c6a03728b018
Approved-By: Tom Tromey <tom@tromey.com>
|
|
After staring at the code for a while, I got convinced that it's not
possible for cu->dies to be nullptr in follow_die_offset. It might be a
leftover from the psymtab days.
In most cases, we see that the dwarf2_cu passedas `*ref_cu` has been
obtained by doing:
per_objfile->get_cu (per_cu);
The only way for a dwarf2_cu to end up in the per_objfile like this is
through load_full_comp_unit or read_signatured_type. Both of these
functions call `reader.read_all_dies ()` (which loads the DIEs in memory
and assigns dwarf2_cu::dies) before transferring the newly created
dwarf2_cu to the per_objfile. So any dwarf2_cu obtained through
per_objfile->get_cu (per_cu)
... will have its DIEs set.
The only case today I'm aware of of a dwarf2_cu without DIEs is in the
cooked indexer. It creates a cutu_reader, but does not call
read_all_dies. Instead, it gets the info_ptr from the cutu_reader and
reads the DIEs from the section buffer directly, on its own. But this
is an entirely different code path that doesn't assign dwarf2_cu
objects to per_objfile.
So, remove the code path in follow_die_offset that tests for
`source_cu->dies == NULL`. I added an assert at the top of the function
to verify that `source_cu->dies` is always non-nullptr, as a way to
test my hypothesis. We could probably get rid of it, but I left it
there because it doesn't cost much to have it.
Change-Id: I97f269f092128800850aa5e64eda7032c2edec60
Approved-By: Tom Tromey <tom@tromey.com>
|
|
Rename some local variables to better make the distinction between the
source and target CUs.
Change-Id: I8b43fac91b8a6f1ca6fd1972846fd6bf28608fe3
Approved-By: Tom Tromey <tom@tromey.com>
|
|
cooked_indexer::ensure_cu_exists
The per_objfile object can be obtained from the cutu_reader. This is
actually how both callers get it in order to pass it as argument.
Change-Id: Iac134ded247d841f80ab5ca55dd9055b556410c3
Approved-By: Tom Tromey <tom@tromey.com>
|
|
These methods don't have (or no longer have) a counterpart without the
_1 suffix, so remove the suffix.
Change-Id: Ifdfe4fb3b6b09c6bb9e30c27acf9f9ecbcb207f2
Approved-By: Tom Tromey <tom@tromey.com>
|
|
This is a bit subjective, but I often struggle to understand what
cutu_reader::keep is meant to do (keep what, where). Perhaps it's just
a question of bad naming, but I think it's a bit confusing for
cutu_reader to transfer the ownership of the dwarf2_cu to the
per_objfile directly.
Add the cutu::release_cu method and make the caller of cutu_reader
transfer the ownership to the per_objfile object.
Right now, it is theoretically possible for release_cu to return
nullptr, so I made callers check the return value. A patch later in
this series will change release_cu to ensure it always return
non-nullptr, so those callers will get simplified.
Change-Id: I3103ff894d1654a95c9d69001073c218501c988a
Approved-By: Tom Tromey <tom@tromey.com>
|
|
cutu_reader::read_all_dies
After construction of a cutu_reader, only the top-level DIE has been
read in memory. If the caller wants to access the full DIE tree, it
does:
reader.top_level_die ()->child
= reader.read_die_and_siblings (reader.top_level_die ());
I don't really like this poking into cutu_reader's data structures from
the outside, I would prefer if that work was done by cutu_reader.
Rename the read_die_and_siblings method to read_all_dies, and do that
work inside cutu_reader.
I also moved these operations inside the read_all_dies method:
gdb_assert (cu->die_hash.empty ());
cu->die_hash.reserve (cu->header.get_length_without_initial () / 12);
...
cu->dies = reader.top_level_die ();
The rationale for this is that read_all_dies (and the functions it
calls) is responsible for filling the die_hash set. So I think it makes
sense for it to do the reserve.
It is also cutu_reader's job, currently, to create and fill the fields
of dwarf2_cu. So I think it makes sense for it to set cu->dies, after
having read the DIEs in memory.
Change-Id: I088c2e0b367db7d1f67e8c9e2d5b0d61165292fc
Approved-By: Tom Tromey <tom@tromey.com>
|
|
The few methods of cutu_reader that read DIEs into memory generally
receive an info_ptr that says where to start reading and return another
one (either by return value or parameter) indicating where the caller
should continue reading.
We can avoid all this passing around by having these methods access
m_info_ptr directly. This allows changing some methods that read DIEs
to return `die_info *`, instead of returning it by parameter, which just
makes the code simpler to read, I think.
The only method that meaningfully reads and writes m_info_ptr (except
the places that initially set it up) is read_full_die_1. It reads and
increments m_info_ptr once to read the abbrev and once again to read
each attribute. Other methods use it for logging.
The methods cutu_reader::read_attribute and
cutu_reader::read_attribute_value do not touch m_info_ptr directly,
because they are used in cooked-indexer.c, which appears to read some
things in a non-linear fashion, unlike cutu_reader's DIE-reading
methods. The cooked indexer calls cutu_reader::info_ptr to get the
m_info_ptr value just after the top-level DIE, and then it does its own
attribute reading after that.
Change-Id: I251f63d13d453a2827b21349760da033171880e2
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I was reading cutu_reader::skip_one_die, and thought that the code to
skip one attribute made it quite difficult to read. Factor this code
out to a new method, to get it out of the way.
As a bonus, it transforms one goto in a recursion call, which is also
easier to follow. Unfortunately, I have no idea how to test
DW_FORM_indirect, as it doesn't seem to appear anywhere in the
testsuite, and I don't think that compilers often emit that.
Change-Id: I2257b3e594aafb7c7da52ddd55baa651cefb802f
Approved-By: Tom Tromey <tom@tromey.com>
|
|
I noticed that load_full_comp_unit and load_full_type_unit didn't use
their pretend_language parameter. Remove them, and then remove more
things that were needed to get the language value to that point,
including the dwarf2_queue_item field.
Change-Id: Ie8cb21c54ae49da065a1b0a20bf18ccb93961d1a
Approved-By: Tom Tromey <tom@tromey.com>
|
