Re: [PATCH v2 8/8] gdb/dwarf: read foreign type units

[Simon Marchi <simark@simark.ca>]

On 4/28/26 3:33 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> However, I know that the current GDB DWARF reader is not able to load
> Simon> multiple type units with the same signature but different content.  Once
> Simon> it loads one type unit with a given signature, all subsequent references
> Simon> to that signature will use that loaded type unit.
> 
> FWIW this seems completely sensible to me, since the whole idea of
> signatured types is to exploit ODR and linker features to reduce the
> size of the DWARF.

I had some meeting with Greg Clayton (from LLDB), and he mentioned that
in practice, compilers will only include in a given type unit the
methods actually used when compiling the compile unit.  But the
different versions of the type unit for the same type will still have
the same signature. For example, if you don't use std::vector::size() in
one compile unit, the type unit for std::vector in that .dwo will not
include the description of std::vector::size().  If that version of the
type unit gets loaded first, and you later try to do "print
myVector.size()", then too bad.  Even if another instance of the type
unit does have the information.

> Simon> Setting a dwarf2_per_cu's section a posteriori breaks the assumed
> Simon> ordering of the dwarf2_per_bfd::all_units vector.  After setting the
> Simon> section, re-sort the vector.
> 
> I'm not a huge fan of this but I guess we can live with it.

I tried to keep foreign units in a vector on the side (they are not
needed for DW_FORM_ref_addr resolution), but it made the code much
uglier than just having this re-sort call.

> Simon> There is one known failure that I am unable to get to the bottom of.  It
> Simon> seems orthogonal to my change though, more like an indexer or symbol
> Simon> reader issue.  There are maybe more of this kind, but this is one
> Simon> example:
> 
> Simon>   FAIL: gdb.ada/tick_length_array_enum_idx.exp: ptype variable_table'length (GDB internal error)
> Simon>   /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:1839: internal-error: search_one: Assertion `symtab != nullptr' failed.
> 
> See https://sourceware.org/bugzilla/show_bug.cgi?id=31648
> 
> Not really the same, but the same test case, so I kind of suspect that
> older compilers had some issue here.  Though:
> 
> Simon> The issue seems sensitive to some aspects of the environment (gnat
> Simon> version?).  I am able to reproduce the issue on Arch Linux (gnat 15)
> Simon> with:
> 
> Simon>   $ make check TESTS="gdb.ada/tick_length_array_enum_idx.exp" RUNTESTFLAGS="--target_board=dwarf5-fission-debug-types-debug-names"
> 
> Simon> But it doesn't reproduce on Debian 13 (gnat 14), Ubuntu 24.04 (gnat
> Simon> 13) or Fedora Rawhide (gnat 16).
> 
> ... this goes against my thinking here.
> 
> One question is whether this compiler defaults to "minimal" encodings or
> GNAT encodings.

This Arch machine now has gcc (and gnat) 16.1, and now the test passes
with the command above :/.  And it's not easy to go back to a previous
release so... I guess we'll never know (unless I build myself a gcc 15).

The default encodings for the 16.1 one are "gdb".

And when forcing minimal encodings, like this, the test still passes:

$  make check TESTS="gdb.ada/tick_length_array_enum_idx.exp" RUNTESTFLAGS="--target_board=dwarf5-fission-debug-types-debug-names GNATMAKE_FOR_TARGET='gnatmake -fgnat-encodings=minimal'"

> Simon> +  /* A convenience function to allocate a signatured_type.  The
> Simon> +     returned object has its "index" field set properly.
> Simon> +
> 
> I don't really understand how the index field even really makes sense
> any more, because now the vector is sorted after the fact.
> 
> Not that this is new with this change, just pointing out that it seems
> incoherent with the description of the field:
> 
>   /* Our index in the unshared "symtabs" vector.  */
> 
> Like is this still true?

Yes, it is still the index in the dwarf2_per_objfile::m_compunit_symtabs
vector.  It's really just an arbitrary 0-based unique ID that we assign
to each unit.  I attempted to remove it once, using an unordered_map for
m_compunit_symtabs, but at the end of the day it just switched a O(1)
lookup for a O(log(N)) one, so not a net improvement.

To be clear, it's not the index in the dwarf2_per_bfd::all_units vector,
if that was your worry.

Other spots use that index as the key for a map
(cooked_index_worker_result::m_reader_hash), I wonder if those should
just use the pointer directly as the key.

Simon