Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: Simon Marchi <simon.marchi@polymtl.ca>,
	Simon Marchi <simon.marchi@efficios.com>,
	gdb-patches@sourceware.org
Subject: Re: [PATCH 1/6] gdb/testsuite/dwarf: use single abbrev table in .dwo files
Date: Tue, 18 Nov 2025 13:32:04 +0000	[thread overview]
Message-ID: <878qg3v54b.fsf@redhat.com> (raw)
In-Reply-To: <ebb39a80-0ace-4721-a60a-94a73131b7a4@polymtl.ca>

Simon Marchi <simon.marchi@polymtl.ca> writes:

> On 11/7/25 4:10 PM, Simon Marchi wrote:
>> From: Simon Marchi <simon.marchi@polymtl.ca>
>> 
>> When I wrote test gdb.dwarf2/fission-with-type-unit.exp, I did not use
>> build_executable_and_dwo_files, because it wouldn't work to have
>> multiple units in the .dwo file, each referring to their own abbrev
>> table using labels.  build_executable_and_dwo_files extracts the .dwo
>> file content from the .o using objcopy (just like gcc does, I learned),
>> meaning that the .dwo file never runs through a linker.  Anything
>> needing relocation (like labels pointing to abbrev tables) doesn't work.
>> 
>> I instead opted to use gdb_compile_shlib to build the .dwo file on its
>> own, so that those labels would get resolved.  That causes problems now
>> that I'm trying to write a test with multiple type units in a .dwo file,
>> where each type unit should be in its own .debug_types section.  Running
>> the .dwo file through the linker causes all the .debug_types section to
>> be collapsed into one.  And generally, I think it was a bad idea to
>> generate a .dwo file using the linker, since the idea behind .dwo files
>> is that they do not need to be linked (therefore improving link
>> times).  We want to produce files as close to what an actual compiler
>> would produce.
>> 
>> This patch fixes this by doing what compilers do in the same situation:
>> use a single abbrev table shared by all units in the .dwo file.  This
>> requires the following changes in lib/dwarf.exp:
>> 
>>  - Declare a new variable _dwo_abbrev_num, which holds the next
>>    abbrev number to use in the .dwo file's abbrev section
>>    (.debug_abbrev.dwo).  Initialize this variable to 1.
>>  - When producing a CU or TU in a .dwo file, use 0 as the abbrev table
>>    offset.
>>  - When generating a DIE, return $_dwo_abbrev_num or $_abbrev_num,
>>    depending on whether the current CU is in a .dwo file.
>>  - After producing a CU or TU in a .dwo file, don't append the
>>    terminator byte.
>>  - After finishing producing the CUs and TUs, append the terminator byte
>>    in .debug_abbrev.dwo if we did output anything there.
>> 
>> Update gdb.dwarf2/fission-with-type-units.exp to use
>> build_executable_and_dwo_files, as it should.
>
> I spotted that running this test with the cc-with-gdb-index board now
> shows a new internal error.  I just wanted to note that this is not a
> regression, it's actually the test going further and catching an actual
> bug in GDB.  Please consider that the following text is appended to the
> commit message.
>
> ---
>     With this change, running with the cc-with-gdb-index board, I see:
>
>         (gdb) maint expand-symtabs
>         /home/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:3056: internal-error: cutu_reader: Assertion `sig_type->signature == cu->header.signature' failed.
>         A problem internal to GDB has been detected,
>         further debugging may prove unreliable.
>         ----- Backtrace -----
>         FAIL: gdb.dwarf2/fission-with-type-unit.exp: maint expand-symtabs (GDB internal error)
>
>     This is actually an improvement, as the test case didn't run properly
>     before.  The compilation failed with:
>
>         gdb compile failed, During symbol reading: Could not find DWO CU fission-with-type-unit.dwo(0xf00d) referenced by CU at offset 0xc [in module /home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.dwarf2/fission-with-type-unit/.tmp/fission-with-type-unit]
>
>     The reason was that the old code would try to generate the GDB index
>     during this step:
>
>         # Build main file.
>         if { [build_executable "${testfile}.exp" $binfile \
>              [list ${srcfile} ${main_asm_file}] {nodebug}] } {
>             return
>         }
>
>     ... which is even the DWO file is even generated.  With this patch
>     things are done in the correct order:

I cannot parse the "which is even the DWO file is even generated" part
here.  Can you reword this?

>
>      - The -dw.S file is generated
>      - The -dw.o file is compiled from the -dw.S
>      - The .dwo sections are extracted to the .dwo file, and stripped from
>        the -dw.o file
>      - The executable is linked from the .o and -dw.o
>      - gdb-add-index is ran on the executable

Not critical, but I think s/ran/run/ would be better.

Thanks,
Andrew


>
>     When gdb-add-index runs, the .dwo file exists, so GDB is able to produce
>     an index.  That index is bogus though, because the .gdb_index format is
>     unable to describe skeletonless type units.  And then GDB gets confused
>     trying to use that index, leading to the internal error.
> ---
>
> Simon


  reply	other threads:[~2025-11-18 13:33 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-07 21:10 [PATCH 0/6] Type unit + split DWARF fixes (PR 33307) Simon Marchi
2025-11-07 21:10 ` [PATCH 1/6] gdb/testsuite/dwarf: use single abbrev table in .dwo files Simon Marchi
2025-11-10 20:14   ` Simon Marchi
2025-11-18 13:32     ` Andrew Burgess [this message]
2025-11-18 16:50       ` Simon Marchi
2025-11-18 16:55   ` Andrew Burgess
2025-11-18 17:20     ` Simon Marchi
2025-11-19 16:05       ` Tom Tromey
2025-11-19 20:21         ` Simon Marchi
2025-11-07 21:10 ` [PATCH 2/6] gdb/testsuite/dwarf: convert _section proc to use parse_options Simon Marchi
2025-11-19 10:59   ` Andrew Burgess
2025-11-19 15:53   ` Tom Tromey
2025-11-19 20:40     ` Simon Marchi
2025-11-19 21:03       ` Tom Tromey
2025-11-07 21:10 ` [PATCH 3/6] gdb/testsuite/dwarf: emit type unit sections as COMDAT Simon Marchi
2025-11-19 11:43   ` Andrew Burgess
2025-11-07 21:10 ` [PATCH 4/6] gdb/dwarf: when in dwarf2_cu, read addr_size from dwarf2_cu::header Simon Marchi
2025-11-19 14:30   ` Andrew Burgess
2025-11-19 20:46     ` Simon Marchi
2025-11-19 16:11   ` Tom Tromey
2025-11-19 20:51     ` Simon Marchi
2025-11-07 21:10 ` [PATCH 5/6] gdb/dwarf: store addr/offset/ref_addr sizes in dwarf2_per_cu Simon Marchi
2025-11-19 14:42   ` Andrew Burgess
2025-11-19 16:14   ` Tom Tromey
2025-11-21 19:54     ` Simon Marchi
2025-11-21 21:25       ` Tom Tromey
2025-11-07 21:10 ` [PATCH 6/6] gdb/dwarf: use dwarf2_per_cu::ref_addr_size in one spot Simon Marchi
2025-11-19 14:44   ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878qg3v54b.fsf@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@efficios.com \
    --cc=simon.marchi@polymtl.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox