]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commit
gdb/dwarf: write offset to parent entry for DW_IDX_parent
authorSimon Marchi <simon.marchi@polymtl.ca>
Sun, 9 Feb 2025 05:51:01 +0000 (00:51 -0500)
committerSimon Marchi <simon.marchi@efficios.com>
Mon, 10 Feb 2025 16:26:44 +0000 (11:26 -0500)
commitad6dde5aaae846176fcc984e438ee7c187f27044
tree70c8ae10b575073c33b745fa20a1e69b4ffe6530
parent9044044c27c3619a18f58c8c235250264cd95a7e
gdb/dwarf: write offset to parent entry for DW_IDX_parent

New in v2:

 - add doc
 - fix computation of offset in entry pool

Due to a mistake in the DWARF 5 spec, the way that GDB interprets
DW_IDX_parent when generating and reading .debug_names is not correct.

In Section 6.1.1.2, the parent index entry attribute is described as:

  Parent debugging information entry, a reference to the index entry for
  the parent. This is represented as the offset of the entry relative to
  the start of the entry pool.

But in Table 6.1, DW_IDX_parent is described as:

  Index of name table entry for parent

These two contradict each other.  The former is the correct one and the
latter is an unfortunate leftover from an earlier version of the
proposal, according to [1].  It does make sense, because pointing to a
name table entry is ambiguous, while poiting to an index entry directly
is not.  Unfortunately, GDB implemented pointing to a name table entry.

Changes on the writer side:

 - For each written pool entry, remember the offset within the pool.

 - Change the DW_IDX_parent form to DW_FORM_data4.

   Using DW_FORM_udata isn't an option, because we don't know the actual
   value when doing the first pass of writing the pool (see next point),
   so we wouldn't know how many bytes to reserve, if we used a
   variable-size encoding.

   Using a fixed 4 bytes encoding would be an issue if the entry pool
   was larger than 4 GiB, but that seems unlikely.

   Note that clang uses DW_FORM_ref4 for this, but I'm not sure it is
   appropriate, since forms of the reference class are specified as
   referring "to one of the debugging information entries that describe
   the program".  Since we're not referring to a DIE, I decided to stay
   with a form of the "constant" class.  I think that readers will be
   able to understand either way.

 - Write a dummy 4 byte number when writing the pool, then patch those
   values later.  This is needed because parents can appear before their
   children in the pool (there's no way to ensure that parents always
   appear before their children), so we might now know at first what
   value to put in.

 - Add a `write_uint` method to `class data_buf` to support that use
   case of patching a value in the middle of the data buffer.

 - Simplify the type of `m_name_to_value_set`, we no longer need to
   track the index at which a name will be written at.

 - Produce a new augmentation string, "GDB3", to be able to distinguish
   "old" and "new" indexes.  It would be possible for a reader to
   distinguish the two semantics of DW_IDX_parent using the form.
   However, current versions of GDB don't do that, so they would be
   confused trying to read a new index.  I think it is preferable to use
   a new augmentation string so that they will reject a new index
   instead.

Changes on the reader side:

 - Track the GDB augmentation version, in addition to whether the
   augmentation string indicates the index was produced by GDB.

 - When reading index entries, maintain a "pool offset" -> "cooked index
   entry" mapping, to be able to find parents by pool offset.

 - When resolving parents, keep the existing behavior of finding parents
   by name table index if the augmentation string is "GDB2.  Otherwise,
   look up parents by pool offset.  This assumes that .debug_names from
   other producers (if/when we add support for reading them) use pool
   offsets for DW_IDX_parent.  This at least what clang does.

 - Simplify augmentation string comparison a bit by using array views.

Update the "Extensions to ‘.debug_names’" section of the documentation
to reflect the new augmentation string version.

Tested by:

 - manually producing executables with "GDB2" and "GDB3" .debug_names
   sections and reading them.
 - running the testsuite with the cc-with-debug-names board

[1] https://lists.dwarfstd.org/pipermail/dwarf-discuss/2025-January/002618.html

Change-Id: I265fa38070b86ef320e0a972c300d1d755735d8d
Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>
gdb/doc/gdb.texinfo
gdb/dwarf2/index-write.c
gdb/dwarf2/read-debug-names.c
gdb/dwarf2/read-debug-names.h