| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
When loading the debug-names-duplicate-cu executable included in this
test-case, we run into:
...
(gdb) file debug-names-duplicate-cu^M
Reading symbols from debug-names-duplicate-cu...^M
src/gdb/dwarf2/read.c:2353: internal-error: read_addrmap_from_aranges: \
Assertion `insertpair.second' failed.^M
...
This assert was added in recent commit 75337cbc147 ("[gdb/symtab] Fix
.debug_aranges duplicate offset warning").
The assert triggers because the CU table in the .debug_names section contains
a duplicate:
...
Version 5
Augmentation string: 47 44 42 00 ("GDB")
CU table:
[ 0] 0x0
[ 1] 0x0
...
Fix this by rejecting the .debug_names index:
...
(gdb) file debug-names-duplicate-cu^M
Reading symbols from debug-names-duplicate-cu...^M
warning: Section .debug_names has duplicate entry in CU table, \
ignoring .debug_names.^M
...
Likewise for the case where the CU table is not sorted by increasing offset.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29436
|
|
In dwarf2_build_psymtabs_hard, we use a parallel_for_each to distribute CUs
over threads.
Ensuring a fair distribution over the worker threads and main thread in terms
of number of CUs might not be the most efficient way, given that CUs can vary
in size.
Fix this by using per_cu->get_length () as the task size.
I've used this experiment to verify the performance impact:
...
$ for n in $(seq 1 10); do \
time gdb -q -batch ~/firefox/libxul.so-93.0-1.1.x86_64.debug \
2>&1 \
| grep "real:"; \
done
...
and without the patch got:
...
real: 4.71
real: 4.88
real: 4.29
real: 4.30
real: 4.65
real: 4.27
real: 4.27
real: 4.27
real: 4.75
real: 4.41
...
and with the patch:
...
real: 3.68
real: 3.81
real: 3.80
real: 3.68
real: 3.75
real: 3.69
real: 3.69
real: 3.74
real: 3.67
real: 3.74
...
so that seems a reasonable improvement.
With parallel_for_each_debug set to true, we get some more detail about
the difference in behaviour. Without the patch we have:
...
Parallel for: n_elements: 2818
Parallel for: minimum elements per thread: 1
Parallel for: elts_per_thread: 704
Parallel for: elements on worker thread 0 : 705
Parallel for: elements on worker thread 1 : 705
Parallel for: elements on worker thread 2 : 704
Parallel for: elements on worker thread 3 : 0
Parallel for: elements on main thread : 704
...
and with the patch:
...
Parallel for: n_elements: 2818
Parallel for: total_size: 1483674865
Parallel for: size_per_thread: 370918716
Parallel for: elements on worker thread 0 : 752 (size: 371811790)
Parallel for: elements on worker thread 1 : 360 (size: 371509370)
Parallel for: elements on worker thread 2 : 1130 (size: 372681710)
Parallel for: elements on worker thread 3 : 0 (size: 0)
Parallel for: elements on main thread : 576 (size: 367671995)
...
Tested on x86_64-linux.
|
|
gdbarch implements its own registry-like approach. This patch changes
it to instead use registry.h. It's a rather large patch but largely
uninteresting -- it's mostly a straightforward conversion from the old
approach to the new one.
The main benefit of this change is that it introduces type safety to
the gdbarch registry. It also removes a bunch of code.
One possible drawback is that, previously, the gdbarch registry
differentiated between pre- and post-initialization setup. This
doesn't seem very important to me, though.
|
|
This changes struct objfile to use a gdb_bfd_ref_ptr. In addition to
removing some manual memory management, this fixes a use-after-free
that was introduced by the registry rewrite series. The issue there
was that, in some cases, registry shutdown could refer to memory that
had already been freed. This help fix the bug by delaying the
destruction of the BFD reference (and thus the per-bfd object) until
after the registry has been shut down.
|
|
The function read_addrmap_from_aranges contains code to issue a warning:
...
if (!insertpair.second)
{
warning (_("Section .debug_aranges in %s has duplicate "
"debug_info_offset %s, ignoring .debug_aranges."),
objfile_name (objfile), sect_offset_str (per_cu->sect_off));
return false;
}
...
but the warning is in fact activated when all_comp_units has duplicate
entries, which is very misleading.
Fix this by:
- adding a test-case that should trigger the warning,
- replacing the current implementation of the warning with an
assert that all_comp_units should not contain duplicates, and
- properly re-implementing the warning, such that it is triggered
by the test-case.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29381
|
|
Printing macros defined in the main source file doesn't work reliably
using various toolchains, especially when DWARF 5 is used. For example,
using the binaries produced by either of these commands:
$ gcc --version
gcc (GCC) 11.2.0
$ ld --version
GNU ld (GNU Binutils) 2.38
$ gcc test.c -g3 -gdwarf-5
$ clang --version
clang version 13.0.1
$ clang test.c -gdwarf-5 -fdebug-macro
I get:
$ ./gdb -nx -q --data-directory=data-directory a.out
(gdb) start
Temporary breakpoint 1 at 0x111d: file test.c, line 6.
Starting program: /home/simark/build/binutils-gdb-one-target/gdb/a.out
Temporary breakpoint 1, main () at test.c:6
6 return ZERO;
(gdb) p ZERO
No symbol "ZERO" in current context.
When starting to investigate this (taking the gcc-compiled binary as an
example), we see that GDB fails to look up the appropriate macro scope
when evaluating the expression. While stopped in
macro_lookup_inclusion:
(top-gdb) p name
$1 = 0x62100011a980 "test.c"
(top-gdb) p source.filename
$2 = 0x62100011a9a0 "/home/simark/build/binutils-gdb-one-target/gdb/test.c"
`source` is the macro_source_file that we would expect GDB to find.
`name` comes from the symtab::filename field of the symtab we are
stopped in. GDB doesn't find the appropriate macro_source_file because
the name of the macro_source_file doesn't match exactly the name of the
symtab.
The name of the main symtab comes from the compilation unit's
DW_AT_name, passed to the buildsym_compunit's constructor:
https://gitlab.com/gnutools/binutils-gdb/-/blob/4815d6125ec580cc02a1094d61b8c9d1cc83c0a1/gdb/dwarf2/read.c#L10627-10630
The contents of DW_AT_name, in this case, is "test.c". It is typically
(what I witnessed all compilers do) the same string that was passed to
the compiler on the command-line.
The name of the macro_source_file comes from the line number program
header's file table, from the call to the line_header::file_file_name
method:
https://gitlab.com/gnutools/binutils-gdb/-/blob/4815d6125ec580cc02a1094d61b8c9d1cc83c0a1/gdb/dwarf2/macro.c#L54-65
line_header::file_file_name prepends the directory path that the file
entry refers to, in the file table (if the file name is not already
absolute). In this case, the file name is "test.c", appended to the
directory "/home/simark/build/binutils-gdb-one-target/gdb".
Because the symtab's name is not created the same way as the
macro_source_file's name is created, we get this mismatch. GDB fails to
find the appropriate macro scope for the symtab, and we can't print
macros when stopped in that symtab.
To make this work, we must ensure that paths produced in these two ways
end up identical. This can be tricky because of the different ways a
path can be passed to the compiler by the user.
Another thing to consider is that while the main symtab's name (or
subfile, before it becomes a symtab) is created using DW_AT_name, the
main symtab is also referred to using its entry in the line table
header's file table, when processing the line table. We must therefore
ensure that the same name is produced in both cases, so that a call to
"start_subfile" for the main subfile will correctly find the
already-created subfile, created by buildsym_compunit's constructor. If
we fail to do that, things still often work, because of a fallback: the
watch_main_source_file_lossage method. This method determines that if
the main subfile has no symbols but there exists another subfile with
the same basename (e.g. "test.c") that does have symbols, it's probably
because there was some filename mismatch. So it replaces the main
subfile with that other subfile. I think that heuristic is useful as a
last effort to work around any bug or bad debug info, but I don't think
we should design things such as to rely on it. It's a heuristic, it can
get things wrong. So in my search for a fix, it is important that given
some good debug info, we don't end up relying on that for things to
work.
A first attempt at fixing this was to try to prepend the compilation
directory here or not prepend it there. In practice, because of all the
possible combinations of debug info the compilers produce, it was not
possible to get something that would produce reliable, consistent paths.
Another attempt at fixing this was to make both macro_source_file
objects and symtab objects use the most complete form of path possible.
That means to prepend directories at least until we get an absolute
path. In theory, we should end up with the same path in all cases.
This generally worked, but because it changed the symtab names, it
resulted in user-visible changes (for example, paths to source files in
Breakpoint hit messages becoming always absolute). I didn't find this
very good, first because there is a "set filename-display" setting that
lets the user control how they want the paths to be displayed, and that
would suddenly make this setting completely ineffective (although even
today, it is a bit dependent on the debug info). Second, it would
require a good amount of testsuite tweaks to make tests accept these
suddenly absolute paths.
This new patch is a slight variation of that: it adds a new field called
"filename_for_id" in struct symtab and struct subfile, next to the
existing filename field. The goal is to separate the internal ids used
for finding objects from the names used for presentation. This field is
used for identifying subfiles, symtabs and macro_source_files
internally. For DWARF symtabs, this new field is meant to contain the
"most complete possible" path, as discussed above. So for a given file,
it must always be in the same form, everywhere. The existing
symtab::filename field remains the one used for printing to the user, so
there shouldn't be any change in how paths are printed.
Changes in the core symtab files are:
- Add "name_for_id" and "filename_for_id" fields to "struct subfile"
and "struct symtab", next to existing "name" and "filename" fields.
- Make buildsym_compunit::buildsym_compunit and
buildsym_compunit::start_subfile accept a "name_for_id" parameter
next to the existing "name" ones.
- Make buildsym_compunit::start_subfile use "name_for_id" for looking
up existing subfiles. This is the key thing for making calls
to start_subfile for the main source file look up the existing
subfile successfully, and avoid relying on
watch_main_source_file_lossage.
- Make sal_macro_scope pass "filename_for_id", rather than "filename",
to macro_lookup_inclusion. This is the key thing to making the
lookup work and macro printing work.
Changes in the DWARF files are:
- Make line_header::file_file_name return the "most complete possible"
name. The only pre-existing user of this method is the macro code,
to give the macro_source_file objects their name. And we now want
them to have this "most complete possible" name, which will match the
corresponding symtab's "filename_for_id".
- Make dwarf2_cu::start_compunit_symtab pass the "most complete
possible" name for the main symtab's "filename_for_id". In this
context, where the info comes from the compilation unit's DW_AT_name
/ DW_AT_comp_dir, it means prepending DW_AT_comp_dir to DW_AT_name if
DW_AT_name is not already absolute.
- Change dwarf2_start_subfile to build a name_for_id for the subfile
being started. The simplest way is to re-use
line_header::file_file_name, since the callers always have a
file_entry handy. This ensures that it will get the exact same path
representation as the macro code does, for the same file (since it
also uses line_header::file_file_name).
- Update calls to allocate_symtab to pass the "name_for_id" from the
subfile.
Tests exercising all this are added by the following patch.
Of all the cases I tried, the only one I found that ends up relying on
watch_main_source_file_lossage is the following one:
$ clang --version
clang version 13.0.1
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$ clang ./test.c -g3 -O0 -gdwarf-4
$ ./gdb -nx --data-directory=data-directory -q -readnow -iex "set debug symtab-create 1" a.out
...
[symtab-create] start_subfile: name = test.c, name_for_id = /home/simark/build/binutils-gdb-one-target/gdb/test.c
[symtab-create] start_subfile: name = ./test.c, name_for_id = /home/simark/build/binutils-gdb-one-target/gdb/./test.c
[symtab-create] start_subfile: name = ./test.c, name_for_id = /home/simark/build/binutils-gdb-one-target/gdb/./test.c
[symtab-create] start_subfile: found existing symtab with name_for_id /home/simark/build/binutils-gdb-one-target/gdb/./test.c (/home/simark/build/binutils-gdb-one-target/gdb/./test.c)
[symtab-create] watch_main_source_file_lossage: using subfile ./test.c as the main subfile
As we can see, there are two forms used for "test.c", one with a "." and
one without. This comes from the fact that the compilation unit DIE
contains:
DW_AT_name ("test.c")
DW_AT_comp_dir ("/home/simark/build/binutils-gdb-one-target/gdb")
without a ".", and the line table for that file contains:
include_directories[ 1] = "."
file_names[ 1]:
name: "test.c"
dir_index: 1
When assembling the filename from that entry, we get a ".".
It is a bit unexpected that the main filename resulting from the line
table header does not match exactly the name in the compilation unit.
For instance, gcc uses "./test.c" for the DW_AT_name, which gives
identical paths in the compilation unit and in the line table header.
Similarly, with DWARF 5:
$ clang ./test.c -g3 -O0 -gdwarf-5
clang create two entries that refer to the same file but are of in a different
form.
include_directories[ 0] = "/home/simark/build/binutils-gdb-one-target/gdb"
include_directories[ 1] = "."
file_names[ 0]:
name: "test.c"
dir_index: 0
file_names[ 1]:
name: "test.c"
dir_index: 1
The first file name produces a path without a "." while the second does.
This is not caught by watch_main_source_file_lossage, because of
dwarf_decode_lines that creates a symtab for each file entry in the line
table. It therefore appears as "non-empty" to
watch_main_source_file_lossage. This results in two symtabs:
(gdb) maintenance info symtabs
{ objfile /home/simark/build/binutils-gdb-one-target/gdb/a.out ((struct objfile *) 0x613000005d00)
{ ((struct compunit_symtab *) 0x62100011aca0)
debugformat DWARF 5
producer clang version 13.0.1
name test.c
dirname /home/simark/build/binutils-gdb-one-target/gdb
blockvector ((struct blockvector *) 0x621000129ec0)
user ((struct compunit_symtab *) (null))
{ symtab test.c ((struct symtab *) 0x62100011ad20)
fullname (null)
linetable ((struct linetable *) 0x0)
}
{ symtab ./test.c ((struct symtab *) 0x62100011ad60)
fullname (null)
linetable ((struct linetable *) 0x621000129ef0)
}
}
}
I am not sure what is the consequence of this, but this is also what
happens before my patch, so I think its acceptable to leave it as-is.
To handle these two cases nicely, I think we will need a function that
removes the unnecessary "." from path names, something that can be done
later.
Finally, I made a change in find_file_and_directory is necessary to
avoid breaking test
gdb.dwarf2/dw2-compdir-oldgcc.exp: info source gcc42
Without that change, we would get:
(gdb) info source
Current source file is /dir/d/dw2-compdir-oldgcc42.S
Compilation directory is /dir/d
whereas the expected result is:
(gdb) info source
Current source file is dw2-compdir-oldgcc42.S
Compilation directory is /dir/d
This test was added here:
https://sourceware.org/pipermail/gdb-patches/2012-November/098144.html
Long story short, GCC <= 4.2 apparently had a bug where it would
generate a DW_AT_name with a full path ("/dir/d/dw2-compdir-oldgcc42.S")
and no DW_AT_comp_dir. The line table has one entry with filename
"dw2-compdir-oldgcc42.S", which refers to directory 0. Directory 0
normally refers to the compilation unit's comp dir, but it is
non-existent in this case.
This caused some symtab lookup problems, and to work around them, some
workaround was added, which today reads as:
if (res.get_comp_dir () == nullptr
&& producer_is_gcc_lt_4_3 (cu)
&& res.get_name () != nullptr
&& IS_ABSOLUTE_PATH (res.get_name ()))
res.set_comp_dir (ldirname (res.get_name ()));
Source: https://gitlab.com/gnutools/binutils-gdb/-/blob/6577f365ebdee7dda71cb996efa29d3714cbccd0/gdb/dwarf2/read.c#L9428-9432
It extracts an artificial DW_AT_comp_dir from DW_AT_name, if there is no
DW_AT_comp_dir and DW_AT_name is absolute.
Prior to my patch, a subfile would get created with filename
"/dir/d/dw2-compdir-oldgcc42.S", from DW_AT_name, and another would get
created with filename "dw2-compdir-oldgcc42.S" from the line table's
file table. Then watch_main_source_file_lossage would kick in and merge
them, keeping only the "dw2-compdir-oldgcc42.S" one:
[symtab-create] start_subfile: name = /dir/d/dw2-compdir-oldgcc42.S
[symtab-create] start_subfile: name = dw2-compdir-oldgcc42.S
[symtab-create] start_subfile: name = dw2-compdir-oldgcc42.S
[symtab-create] start_subfile: found existing symtab with name dw2-compdir-oldgcc42.S (dw2-compdir-oldgcc42.S)
[symtab-create] watch_main_source_file_lossage: using subfile dw2-compdir-oldgcc42.S as the main subfile
And so "info source" would show "dw2-compdir-oldgcc42.S" as the
filename.
With my patch applied, but without the change in
find_file_and_directory, both DW_AT_name and the line table would try to
start a subfile with the same filename_for_id, and there was no need for
watch_main_source_file_lossage - which is what we want:
[symtab-create] start_subfile: name = /dir/d/dw2-compdir-oldgcc42.S, name_for_id = /dir/d/dw2-compdir-oldgcc42.S
[symtab-create] start_subfile: name = dw2-compdir-oldgcc42.S, name_for_id = /dir/d/dw2-compdir-oldgcc42.S
[symtab-create] start_subfile: found existing symtab with name_for_id /dir/d/dw2-compdir-oldgcc42.S (/dir/d/dw2-compdir-oldgcc42.S)
[symtab-create] start_subfile: name = dw2-compdir-oldgcc42.S, name_for_id = /dir/d/dw2-compdir-oldgcc42.S
[symtab-create] start_subfile: found existing symtab with name_for_id /dir/d/dw2-compdir-oldgcc42.S (/dir/d/dw2-compdir-oldgcc42.S)
But since the one with name == "/dir/d/dw2-compdir-oldgcc42.S", coming
from DW_AT_name, gets created first, it wins, and the symtab ends up
with "/dir/d/dw2-compdir-oldgcc42.S" as the name, "info source" shows
"/dir/d/dw2-compdir-oldgcc42.S" and the test breaks.
This is not wrong per-se, after all DW_AT_name is
"/dir/d/dw2-compdir-oldgcc42.S", so it wouldn't be wrong to report the
current source file as "/dir/d/dw2-compdir-oldgcc42.S". If you compile
a file passing "/an/absolute/path.c", DW_AT_name typically contains (at
least with GCC) "/an/absolute/path.c" and GDB tells you that the source
file is "/an/absolute/path.c". But we can also keep the existing
behavior fairly easily with a little change in find_file_and_directory.
When extracting an artificial DW_AT_comp_dir from DW_AT_name, we now
modify the name to just keep the file part. The result is coherent with
what compilers do when you compile a file by just passing its filename
("gcc path.c -g"):
DW_AT_name ("path.c")
DW_AT_comp_dir ("/home/simark/build/binutils-gdb-one-target/gdb")
With this change, filename_for_id is still the full name,
"/dir/d/dw2-compdir-oldgcc42.S", but the filename of the subfile /
symtab (what ends up shown by "info source") is just
"dw2-compdir-oldgcc42.S", and that makes the test happy.
Change-Id: I8b5cc4bb3052afdb172ee815c051187290566307
|
|
In the following patch, there will be some callers of file_file_name
that will already have access to the file_entry object for which they
want the file name. It would be inefficient to have them pass an index,
only for line_header::file_file_name to re-lookup the same file_entry
object. Change line_header::file_file_name to accept a file_entry
object reference, instead of an index to look up.
I think this change makes sense in any case. Callers that have an index
can first obtain a file_entry using line_header::file_name_at or
line_header::file_names.
When passing a file_entry object, we can assume that the file_entry's
index is valid, unlike when passing an index. So, push the special case
about an invalid index to the sole current caller of file_file_name,
macro_start_file. I think that error belongs there anyway, since it
specifically talks about "bad file number in macro information".
This requires recording the file index in the file_entry structure, so
add that.
Change-Id: Ic6e44c407539d92b7863d7ba82405ade17f384ad
|
|
The following patch changes line_header::file_file_name to prepend the
compilation directory to the file name, if needed. For that, the line
header needs to know about the compilation directory. Prepare for that
by adding a constructor that takes it as a parameter, and passing the
value down everywhere needed. Add a second constructor for the special
case of building a line_header for doing a hash table lookup, since that
case doesn't require a compilation directory value.
Change-Id: Iba3ba0293e4e2d13a64b257cf9a3094684d54330
|
|
This rewrites registry.h, removing all the macros and replacing it
with relatively ordinary template classes. The result is less code
than the previous setup. It replaces large macros with a relatively
straightforward C++ class, and now manages its own cleanup.
The existing type-safe "key" class is replaced with the equivalent
template class. This approach ended up requiring relatively few
changes to the users of the registry code in gdb -- code using the key
system just required a small change to the key's declaration.
All existing users of the old C-like API are now converted to use the
type-safe API. This mostly involved changing explicit deletion
functions to be an operator() in a deleter class.
The old "save/free" two-phase process is removed, and replaced with a
single "free" phase. No existing code used both phases.
The old "free" callbacks took a parameter for the enclosing container
object. However, this wasn't truly needed and is removed here as
well.
|
|
When running test-case gdb.cp/cpexprs-debug-types.exp with target board
cc-with-debug-names on a system with gcc 12.1.1 (defaulting to dwarf 5), I
run into:
...
(gdb) file cpexprs-debug-types^M
Reading symbols from cpexprs-debug-types...^M
warning: Section .debug_aranges in cpexprs-debug-types has duplicate \
debug_info_offset 0x0, ignoring .debug_aranges.^M
gdb/dwarf2/read.h:309: internal-error: set_length: \
Assertion `m_length == length' failed.^M
...
The exec contains a .debug_names section, which gdb rejects due to
.debug_names containing a list of TUs, while the exec doesn't contain a
.debug_types section (which is what you'd expect for dwarf 4).
Gdb then falls back onto the cooked index, which calls create_all_comp_units
to create all_comp_units. However, the failed index reading left some
elements in all_comp_units, so we end up with duplicates in all_comp_units,
which causes the misleading complaint and the assert.
Fix this by:
- asserting at the start of create_all_comp_units that all_comp_units is empty,
as we do in create_cus_from_index and create_cus_from_debug_names, and
- cleaning up all_comp_units when failing in dwarf2_read_debug_names.
Add a similar cleanup in dwarf2_read_gdb_index.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29381
|
|
I noticed this code in dw2_debug_names_iterator::next:
...
case DW_IDX_compile_unit:
/* Don't crash on bad data. */
if (ull >= per_bfd->all_comp_units.size ())
{
complaint (_(".debug_names entry has bad CU index %s"
" [in module %s]"),
pulongest (ull),
objfile_name (objfile));
continue;
}
per_cu = per_bfd->get_cu (ull);
break;
...
This code used to DTRT, before we started keeping both CUs and TUs in
all_comp_units.
Fix by using "per_bfd->all_comp_units.size () - per_bfd->tu_stats.nr_tus"
instead.
It's hard to produce a test-case for this, but let's try at least to trigger
the complaint somehow. We start out by creating an exec with .debug_types and
.debug_names:
...
$ gcc -g ~/hello.c -fdebug-types-section
$ gdb-add-index -dwarf-5 a.out
...
and verify that we don't see any complaints:
...
$ gdb -q -batch -iex "set complaints 100" ./a.out
...
We look at the CU and TU table using readelf -w and conclude that we have
nr_cus == 6 and nr_tus == 1.
Now override ull in dw2_debug_names_iterator::next for the DW_IDX_compile_unit
case to 6, and we have:
...
$ gdb -q -batch -iex "set complaints 100" ./a.out
During symbol reading: .debug_names entry has bad CU index 6 [in module a.out]
...
After this, it still crashes because this code in
dw2_debug_names_iterator::next:
...
/* Skip if already read in. */
if (m_per_objfile->symtab_set_p (per_cu))
goto again;
...
is called with per_cu == nullptr.
Fix this by skipping the entry if per_cu == nullptr.
Now revert the fix and observe that the complaint disappears, so we've
confirmed that the fix is required.
A somewhat similar issue for .gdb_index in dw2_symtab_iter_next has been filed
as PR29367.
Tested on x86_64-linux, with native and target board cc-with-debug-names.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29336
|
|
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=29311
https://sourceware.org/bugzilla/show_bug.cgi?id=29286
|
|
When building gdb with -fsanitize=thread and running test-case
gdb.dwarf2/inlined_subroutine-inheritance.exp, we run into a data race
between:
...
Read of size 1 at 0x7b2000003010 by thread T4:
#0 packed<language, 1ul>::operator language() const packed.h:54
#1 dwarf2_per_cu_data::set_lang(language) read.h:363
...
and:
...
Previous write of size 1 at 0x7b2000003010 by main thread:
#0 dwarf2_per_cu_data::set_lang(language) read.h:365
...
Fix this by making per_cu->m_lang atomic.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29286
|
|
With gdb with -fsanitize=thread and test-case gdb.ada/array_bounds.exp, I run
into a data race between:
...
Read of size 1 at 0x7b2000025f0f by main thread:
#0 packed<dwarf_unit_type, 1ul>::operator dwarf_unit_type() const packed.h:54
#1 dwarf2_per_cu_data::set_unit_type(dwarf_unit_type) read.h:339
...
and:
...
Previous write of size 1 at 0x7b2000025f0f by thread T3:
#0 dwarf2_per_cu_data::set_unit_type(dwarf_unit_type) read.h:341
...
Fix this by making per_cu->unit_type atomic.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29286
|
|
We have in per_cu->set_lang this comment:
...
void set_lang (enum language lang)
{
/* We'd like to be more strict here, similar to what is done in
set_unit_type, but currently a partial unit can go from unknown to
minimal to ada to c. */
...
Fix this by not setting m_lang for partial units.
This requires us to move the m_unit_type initialization to ensure that
m_unit_type is initialized before per_cu->m_lang.
Tested on x86_64-linux, with native and target board cc-with-dwz-m.
|
|
The cu->per_cu->lang field was added to carry information from the initial
partial symtabs phase to the symtab expansion phase, for the benefit of a
particular optimization in process_imported_unit_die.
Other uses have been added, but since the first phase now has been
parallelized, those have become problematic and sources of race conditions.
Fix this by adding dwarf2_cu::lang () and using it where we can to replace
cu->per_cu->lang () with cu->lang ().
Also assert in dwarf2_cu::lang () that we're not returning language_unknown.
Tested on x86_64-linux.
|
|
When building gdb with -fsanitize=thread and gcc 12, and running test-case
gdb.dwarf2/dwz.exp, we run into a data race between:
...
Read of size 1 at 0x7b200000300d by thread T2:^M
#0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6164 \
(gdb+0x82ec95)^M
...
and:
...
Previous write of size 1 at 0x7b200000300d by main thread:^M
#0 prepare_one_comp_unit gdb/dwarf2/read.c:23588 (gdb+0x86f973)^M
...
In other words, between:
...
if (this_cu->reading_dwo_directly)
...
and:
...
cu->per_cu->lang = pretend_language;
...
Likewise, we run into a data race between:
...
Write of size 1 at 0x7b200000300e by thread T4:
#0 process_psymtab_comp_unit gdb/dwarf2/read.c:6789 (gdb+0x830720)
...
and:
...
Previous read of size 1 at 0x7b200000300e by main thread:
#0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6164 \
(gdb+0x82edab)
...
In other words, between:
...
this_cu->unit_type = DW_UT_partial;
...
and:
...
if (this_cu->reading_dwo_directly)
...
Likewise for the write to addresses_seen in cooked_indexer::check_bounds and a
read from is_dwz in dwarf2_find_containing_comp_unit for test-case
gdb.dwarf2/dw2-dir-file-name.exp and target board cc-with-dwz-m.
The problem is that the written fields are part of the same memory location as
the read fields, so executing a read and write in different threads is
undefined behavour.
Making the written fields separate memory locations, using the new
struct packed template fixes this.
The set of fields has been established experimentally to be the
minimal set to get rid of this type of -fsanitize=thread errors, but
more fields might require the same treatment.
Looking at the properties of the lang field, unlike dwarf_version it's
not available in the unit header, so it will be set the first time
during the parallel cooked index reading. The same holds for
unit_type, and likewise for addresses_seen.
dwarf2_per_cu_data::addresses_seen is moved so that the bitfields that
currently follow it can be merged in the same memory location as the
bitfields that currently precede it, for better packing.
Tested on x86_64-linux.
Co-Authored-By: Pedro Alves <pedro@palves.net>
Change-Id: Ifa94f0a2cebfae5e8f6ddc73265f05e7fd9e1532
|
|
With gdb build with -fsanitize=thread and test-case
gdb.dwarf2/dw4-sig-types.exp and target board cc-with-dwz-m we run into a data
race between:
...
Write of size 4 at 0x7b2800002268 by thread T4:^M
#0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6236 \
(gdb+0x82f525)^M
...
and this read:
...
Previous read of size 4 at 0x7b2800002268 by thread T1:^M
#0 dwarf2_find_containing_comp_unit gdb/dwarf2/read.c:23444 \
(gdb+0x86e22e)^M
...
In other words, between this write:
...
this_cu->length = cu->header.get_length ();
...
and this read:
...
&& mid_cu->sect_off + mid_cu->length > sect_off))
...
of per_cu->length.
Fix this similar to the per_cu->dwarf_version case, by only setting it if
needed, and otherwise verifying that the same value is used. [ Note that the
same code is already present in the other cutu_reader constructor. ]
Move this logic into into a member function set_length to make sure it's used
consistenly, and make the field private in order to enforce access through the
member functions, and rename it to m_length.
This exposes (running test-case gdb.dwarf2/fission-reread.exp) that in
fill_in_sig_entry_from_dwo_entry, the m_length field is overwritten.
For now, allow for that exception.
While we're at it, make sure that the length is set before read.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29344
|
|
There's a spot in read_comp_units_from_section where we explictly use
initial_length_size to get the total length:
...
this_cu->length = cu_header.length + cu_header.initial_length_size;
...
Instead, just use cu_header.get_length ().
Tested on x86_64-linux.
|
|
When running test-case gdb.dwarf2/dw2-symtab-includes.exp with target board
cc-with-debug-names, I run into:
...
(gdb) maint expand-symtab dw2-symtab-includes.h^M
src/gdb/dwarf2/read.h:311: internal-error: unit_type: \
Assertion `m_unit_type != 0' failed.^M
A problem internal to GDB has been detected,^M
further debugging may prove unreliable.^M
----- Backtrace -----^M
FAIL: gdb.dwarf2/dw2-symtab-includes.exp: maint expand-symtab \
dw2-symtab-includes.h (GDB internal error)
...
The assert was recently added in commit 2c474c46943 ("[gdb/symtab] Add get/set
functions for per_cu->lang/unit_type").
The assert is triggered here:
...
/* We're importing a C++ compilation unit with tag DW_TAG_compile_unit
into another compilation unit, at root level. Regard this as a hint,
and ignore it. */
if (die->parent && die->parent->parent == NULL
&& per_cu->unit_type () == DW_UT_compile
&& per_cu->lang () == language_cplus)
return;
...
We're trying to access unit_type / lang which hasn't been set yet.
Normally, these are set during cooked index creation, but that's not the case
when using an index (or using -readnow).
In other words, this lto binary reading speed optimization only works in the
normal use case.
IWBN to have this working in all use cases, but for now, allow lang () and
unit_type () to return language_unknown and 0 here.
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29321
|
|
When running test-case gdb.cp/cpexprs-debug-types.exp with target board
cc-with-debug-names, I run into:
...
(gdb) print base::operator new^M
^M
^M
Fatal signal: Segmentation fault^M
----- Backtrace -----^M
0x57ea46 gdb_internal_backtrace_1^M
/home/vries/gdb_versions/devel/src/gdb/bt-utils.c:122^M
0x57eae9 _Z22gdb_internal_backtracev^M
/home/vries/gdb_versions/devel/src/gdb/bt-utils.c:168^M
0x75b8ad handle_fatal_signal^M
/home/vries/gdb_versions/devel/src/gdb/event-top.c:946^M
0x75ba19 handle_sigsegv^M
/home/vries/gdb_versions/devel/src/gdb/event-top.c:1019^M
0x7f795f46a8bf ???^M
0x6d3cb1 _ZNK18dwarf2_per_objfile12symtab_set_pEPK18dwarf2_per_cu_data^M
/home/vries/gdb_versions/devel/src/gdb/dwarf2/read.c:1515^M
...
The problem is in this piece of code in dw2_debug_names_iterator::next:
...
case DW_IDX_type_unit:
/* Don't crash on bad data. */
if (ull >= per_bfd->tu_stats.nr_tus)
{
complaint (_(".debug_names entry has bad TU index %s"
" [in module %s]"),
pulongest (ull),
objfile_name (objfile));
continue;
}
per_cu = per_bfd->get_cu (ull + per_bfd->tu_stats.nr_tus);
break;
...
The all_comp_units vector (which get_cu accesses) contains both CUs and TUs,
with CUs first.
So to get the nth TU we need the element at "nr_cus + n", but
the code uses "nr_tus + n" instead.
Fix this by using "nr_cus + n".
Tested on x86_64-linux.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29334
|
|
The dwarf2_per_cu_data fields lang and unit_type both have a dont-know
initial value (respectively language_unknown and (dwarf_unit_type)0), which
allows us to add certain checks, f.i. checking that that a field is not read
before written.
Add get/set member functions for the two fields as a convenient location to
add such checks, make the fields private to enforce using the member
functions, and add the m_ prefix.
Tested on x86_64-linux.
|
|
When building gdb with -fsanitize=thread and gcc 12, and running test-case
gdb.dwarf2/dwz.exp, we run into a data race between thread T2 and the main
thread in the same write:
...
Write of size 1 at 0x7b200000300c:^M
#0 cutu_reader::cutu_reader(dwarf2_per_cu_data*, dwarf2_per_objfile*, \
abbrev_table*, dwarf2_cu*, bool, abbrev_cache*) gdb/dwarf2/read.c:6252 \
(gdb+0x82f3b3)^M
...
which is here:
...
this_cu->dwarf_version = cu->header.version;
...
Both writes are called from the parallel for in dwarf2_build_psymtabs_hard,
this one directly:
...
#1 process_psymtab_comp_unit gdb/dwarf2/read.c:6774 (gdb+0x8304d7)^M
#2 operator() gdb/dwarf2/read.c:7098 (gdb+0x8317be)^M
#3 operator() gdbsupport/parallel-for.h:163 (gdb+0x872380)^M
...
and this via the PU import:
...
#1 cooked_indexer::ensure_cu_exists(cutu_reader*, dwarf2_per_objfile*, \
sect_offset, bool, bool) gdb/dwarf2/read.c:17964 (gdb+0x85c43b)^M
#2 cooked_indexer::index_imported_unit(cutu_reader*, unsigned char const*, \
abbrev_info const*) gdb/dwarf2/read.c:18248 (gdb+0x85d8ff)^M
#3 cooked_indexer::index_dies(cutu_reader*, unsigned char const*, \
cooked_index_entry const*, bool) gdb/dwarf2/read.c:18302 (gdb+0x85dcdb)^M
#4 cooked_indexer::make_index(cutu_reader*) gdb/dwarf2/read.c:18443 \
(gdb+0x85e68a)^M
#5 process_psymtab_comp_unit gdb/dwarf2/read.c:6812 (gdb+0x830879)^M
#6 operator() gdb/dwarf2/read.c:7098 (gdb+0x8317be)^M
#7 operator() gdbsupport/parallel-for.h:171 (gdb+0x8723e2)^M
...
Fix this by setting the field earlier, in read_comp_units_from_section.
The write in cutu_reader::cutu_reader() is still needed, in case
read_comp_units_from_section is not used (run the test-case with say, target
board cc-with-gdb-index).
Make the write conditional, such that it doesn't trigger if the field is
already set by read_comp_units_from_section. Instead, verify that the
field already has the value that we're trying to set it to.
Move this logic into into a member function set_version (in analogy to the
already present member function version) to make sure it's used consistenly,
and make the field private in order to enforce access through the member
functions, and rename it to m_dwarf_version.
While we're at it, make sure that the version is set before read, to avoid
say returning true for "per_cu.version () < 5" if "per_cu.version () == 0".
Tested on x86_64-linux.
|
|
When running test-case gdb.dwarf2/fission-mix.exp with target board dwarf64
and gcc-12 (defaulting to DWARF5), I run into:
...
(gdb) break func2^M
Offset from DW_FORM_GNU_str_index or DW_FORM_strx pointing outside of \
.debug_str.dwo section in CU at offset 0x0 [in module fission-mix]^M
(gdb) FAIL: gdb.dwarf2/fission-mix.exp: break func2
...
The .debug_str_offsets section has version 5, so as per the standard it has
it's own header, with initial length and version:
...
Contents of the .debug_str_offsets.dwo section (loaded from fission-mix2.dwo):
Length: 0x1c
Version: 0x5
Index Offset [String]
0 0 build/gdb/testsuite
1 33 GNU C17
2 8f src/gdb/testsuite/gdb.dwarf2/fission-mix-2.c
...
But when trying to read the string offset at index 0 in the table (which
is 0), we start reading at offset 8, which points in the header, at the last
4 bytes of the initial length (it's 12 bytes because of 64-bit dwarf), as well
at the 2-byte version field and 2 bytes of padding, so we get:
...
(gdb) p /x str_offset
$1 = 0x500000000
...
which indeed is an offset that doesn't fit in the .debug_str section.
The offset 8 is based on reader->cu->header.addr_size:
...
static const char *
read_dwo_str_index (const struct die_reader_specs *reader, ULONGEST str_index)
{
ULONGEST str_offsets_base = reader->cu->header.version >= 5
? reader->cu->header.addr_size : 0;
...
which doesn't in look in agreement with the standard.
Note that this happens to give the right answer for 32-bit dwarf and
addr_size == 8, because then we have header size ==
(initial length (4) + version (2) + padding (2)) == 8.
Conversely, for 32-bit dwarf and addr_size == 4 (target board unix/-m32)
we run into a similar problem. It just happens to not trigger the warning,
instead we get the wrong strings, like "func2" for DW_AT_producer and
"build/gdb/testsuite" for DW_AT_name of the DW_TAG_compile_unit DIE.
Fix this by parsing the .debug_str_offsets header in read_dwo_str_index.
Add a FIXME that we should not parse this for every call.
Tested on x86_64-linux.
|
|
cooked_indexer::index_dies incorrect computes the end of the current
CU in the .debug_info. This isn't readily testable without writing
intentionally corrupt DWARF, but it's apparent through observation: it
is currently based on 'info_ptr', which does not always point to the
start of the CU. This patch fixes the expression. Tested on x86-64
Fedora 34.
|
|
Starting with (future) Clang 15 (since
https://reviews.llvm.org/D120989), Clang emits the DWARF information
of global alias variables as DW_TAG_imported_declaration. However,
GDB does not handle it. It incorrectly always reads this tag as
C++/Fortran imported declaration (type alias, namespace alias and
Fortran module). This commit adds support to handle this tag as an
alias variable.
This change fixes the failures in the gdb.base/symbol-alias.exp
testcase with current git Clang. This testcase is also updated to
test nested (recursive) aliases.
|
|
While working on addrmaps, I noticed that psymtab_addrmap is no longer
needed now. It was introduced in ancient times as an optimization for
DWARF, but no other symbol reader was ever updated to use it. Now
that DWARF does not use psymtabs, it can be deleted.
|
|
Mutable addrmaps currently require an obstack. This was probably done
to avoid having to call splay_tree_delete, but examination of the code
shows that all mutable obstacks have a limited lifetime -- now it's
simple to treat them as ordinary C++ objects, in some cases
stack-allocating them, and have a destructor to make the needed call.
This patch implements this change.
|
|
addrmap::create_fixed is just a simple wrapper for 'new', so remove it
in favor of uses of 'new'.
|
|
This removes addrmap_create_mutable in favor of using 'new' at the
spots where the addrmap is created.
|
|
This removes the various addrmap wrapper functions in favor of simple
method calls on the objects themselves.
|
|
The CU queue is a member of dwarf2_per_bfd, but it is only used when
expanding CUs. Also, the dwarf2_per_objfile destructor checks the
queue -- however, if the per-BFD object is destroyed first, this will
not work. This was pointed out Lancelot as fallout from the patch to
rewrite the registry system.
This patch avoids this problem by moving the queue to the per-objfile
object.
|
|
m_dwarf2_cus manually manages the 'dwarf2_cu' pointers it owns. This
patch simplifies the code by changing it to use unique_ptr.
|
|
In the name matching unit tests in gdb/dwarf2/read.c, explain better
why we test symbols with \377 / 0xff characters (Latin1 'ΓΏ').
Change-Id: I517f13adfff2e4d3cd783fec1d744e2b26e18b8e
|
|
PR gdb/29128 points out a crash in the new DWARF index code. This
happens if the aranges for a CU claims a PC, but the symtab that is
created during CU expansion does not actually contain the PC. This
can only occur due to bad debuginfo, but at the same time, gdb should
not crash.
This patch fixes the bug and further merges some code into
dwarf2_base_index_functions. This merger helps prevent the same issue
from arising from the other index implementations.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29128
|
|
After DWARF has been scanned, the cooked index code does a
"finalization" step in a worker thread. This step combines all the
index entries into a single master list, canonicalizes C++ names, and
splits Ada names to synthesize package names.
While this step is run in the background, gdb will wait for the
results in some situations, and it turns out that this step can be
slow. This is PR symtab/29105.
This can be sped up by parallelizing, at a small memory cost. Now
each index is finalized on its own, in a worker thread. The cost
comes from name canonicalization: if a given non-canonical name is
referred to by multiple indices, there will be N canonical copies (one
per index) rather than just one.
This requires changing the users of the index to iterate over multiple
results. However, this is easily done by introducing a new "chained
range" class.
When run on gdb itself, the memory cost seems rather low -- on my
current machine, "maint space 1" reports no change due to the patch.
For performance testing, using "maint time 1" and "file" will not show
correct results. That approach measures "time to next prompt", but
because the patch only affects background work, this shouldn't (and
doesn't) change. Instead, a simple way to make gdb wait for the
results is to set a breakpoint.
Before:
$ /bin/time -f%e ~/gdb/install/bin/gdb -nx -q -batch \
-ex 'break main' /tmp/gdb
Breakpoint 1 at 0x43ec30: file ../../binutils-gdb/gdb/gdb.c, line 28.
2.00
After:
$ /bin/time -f%e ./gdb/gdb -nx -q -batch \
-ex 'break main' /tmp/gdb
Breakpoint 1 at 0x43ec30: file ../../binutils-gdb/gdb/gdb.c, line 28.
0.65
Regression tested on x86-64 Fedora 34.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29105
|
|
Currently GDB is not able to debug (Binary generated with Clang) variables
present in shared/private clause of OpenMP Task construct. Please note that
LLVM debugger LLDB is able to debug.
In case of OpenMP, compilers generate artificial functions which are not
present in actual program. This is done to apply parallelism to block of
code.
For non-artifical functions, DW_AT_name attribute should contains the name
exactly as present in actual program.
(Ref# http://wiki.dwarfstd.org/index.php?title=Best_Practices)
Since artificial functions are not present in actual program they not having
DW_AT_name and having DW_AT_linkage_name instead should be fine.
Currently GDB is invalidating any function not havnig DW_AT_name which is why
it is not able to debug OpenMP (Clang).
It should be fair to fallback to check DW_AT_linkage_name in case DW_AT_name
is absent.
|
|
cooked_index::m_start is unused and can be removed. I think this was
a leftover from a previous approach in the index finalization code,
and then when rewriting it I forgot to remove it.
Tested by rebuilding.
|
|
PR build/29110 points out that GDB fails to build on mingw when the
"win32" thread model is in use. It turns out that the Fedora cross
tools using the "posix" thread model, which somehow manages to support
std::future, whereas the win32 model does not.
While looking into this, I found that the configuring with
--disable-threading will also cause a build failure.
This patch fixes this build by introducing a compatibility wrapper for
std::future.
I am not able to test the win32 thread model build, but I'm going to
ask the reporter to try this patch.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29110
|
|
My patches yesterday to unify the DWARF index base classes had a bug
-- namely, I did the wholesale dynamic_cast-to-static_cast too hastily
and introduced a crash. This can be seen by trying to add an index to
a file that has an index, or by running a test like gdb-index-cxx.exp
using the cc-with-debug-names.exp target board.
This patch fixes the crash by introducing a new virtual method and
removing some of the static casts.
|
|
This de-duplicates variables and types in .gdb_index, making the new
index closer to what gdb generated before the new DWARF scanner
series. Spot-checking the resulting index for gdb itself, it seems
that the new scanner picks up some extra symbols not detected by the
old one. I tested both the new and old versions of gdb on both new
and old versions of the index, and startup time in all cases is
roughly the same (it's worth noting that, for gdb itself, the index no
longer provides any benefit over the DWARF scanner). So, I think this
fixes the size issue with the new index writer.
Regression tested on x86-64 Fedora 34.
|
|
At AdaCore, we run the internal gdb test suite in several modes,
including one using the .debug_names index. This caught a regression
caused by the new DWARF indexer.
First, the psymtabs-based .debug_names generator was completely wrong.
However, to avoid making the rewrite series even bigger (fixing the
writer will also require rewriting the .debug_names reader), it
attempted to preserve the weirdness.
However, this was not done properly. For example the old writer did
this:
- case STRUCT_DOMAIN:
- return DW_TAG_structure_type;
The new code, instead, simply preserves the actual DWARF tag -- but
this makes future lookups fail, because the .debug_names reader only
looks for DW_TAG_structure_type.
This patch attempts to revert to the old behavior in the writer.
|
|
The DWARF index code currently uses 'stat' to see if an objfile
represents a real file. However, I think it's more correct to check
OBJF_NOT_FILENAME instead.
Regression tested on x86-64 Fedora 34.
|
|
Replace with equivalent method.
Change-Id: I0e033095e7358799930775e61028b48246971a7d
|
|
Replace with an equivalent method on struct block.
Change-Id: I6dcf13e9464ba8a08ade85c89e7329c300fd6c2a
|
|
This changes the .gdb_index writer to skip linkage names. This was
always done historically (though somewhat implicitly).
|
|
Pedro pointed out that gdb-add-index is much slower with the new DWARF
indexer. He also noticed that, in some cases, the generated
.gdb_index would have the wrong fully-qualified name for a method.
I tracked this down to a bug in the indexer. If a type could have
methods but was marked as a declaration, the indexer was ignoring it.
However, this meant that the internal map to find the qualified name
was not updated for this container.
|
|
This can be a local in dwarf_decode_line_header.
Change-Id: I2ecf4616d1a3197bd1e81ded9f999a2da9a685af
|
|
This doesn' have to be a field, it can simply be a local variable in
dwarf_decode_line_header. Name the local variable "unit_length", since
that's what the field in called in DWARF 4 and 5. It's always easier to
follow the code with the standard on the side when we use the same
terminology.
Change-Id: I3ad1022afd9410b193ea11b9b5437686c1e4e633
|
|
Internally we noticed that some tests would fail like so on Windows:
warning: Section .debug_aranges in [...] has duplicate debug_info_offset 0x0, ignoring .debug_aranges.
Debugging showed that, in fact, a second CU was being created at this
offset. We tracked this down to the fact that, while the ELF reader
is careful to re-use the per-BFD data, other readers are not, and
could re-read the DWARF data multiple times.
However, since the change to allow an objfile to have multiple "quick
symbol" implementations, there's no reason for this approach -- it's
safe and easy for all symbol readers to reuse the per-BFD data when
reading DWARF.
This patch implements this idea, simplifying dwarf2_build_psymtabs and
making it private, and then switching to dwarf2_initialize_objfile as
the sole way to start the DWARF reader.
Note that, while I think the call to dwarf2_build_frame_info in
machoread.c is also obsolete, I haven't attempted to remove it here.
|
