Bug 30841 - [gdb, symtab] support .gnu_debugaltlink in gdb-add-index
Summary: [gdb, symtab] support .gnu_debugaltlink in gdb-add-index
Status: NEW
Alias: None
Product: gdb
Classification: Unclassified
Component: symtab (show other bugs)
Version: HEAD
: P2 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-09-12 09:41 UTC by Tom de Vries
Modified: 2023-09-13 06:53 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom de Vries 2023-09-12 09:41:54 UTC
Say we have an executable a.out, with a .gnu_debugaltlink to a dwz file a.dwz.

Adding a .gdb_index section to a.out also requires adding a minimal .gdb_index
section to a.dwz.

Our current implementation of gdb-add-index, based on the "save gdb-index"
command (which currently only generates a symbol-file.gdb-index) doesn't support this.

Supporting this would involve:
- modifying "save gdb-index" to also generate a symbol-file.dwz-gdb-index,
  and
- modifying gdb-add-index to add that as a .gdb_section to the file pointed to
  by the .gnu_debugaltlink.
Comment 1 Tom Tromey 2023-09-12 15:46:34 UTC
My recollection is that in 'dwz -m' mode, all symbols from
PUs should be attributed to one of their containing CUs,
and that the separate file should not require an index.
Comment 2 Tom de Vries 2023-09-12 16:25:58 UTC
(In reply to Tom Tromey from comment #1)
> My recollection is that in 'dwz -m' mode, all symbols from
> PUs should be attributed to one of their containing CUs,

Yes.  That doesn't cover the case of a CU in the dwz file though.

> and that the separate file should not require an index.

That was added in this commit:
...
commit c4973306879b6079bdfc997474a2cbbd89f49bd2
Author: Simon Marchi <simon.marchi@polymtl.ca>
Date:   Sun Jun 16 10:13:56 2019 -0400

    Write index for dwz -m file
    
    PR 24445 ("dwz multifile index not written to index cache") exposed the
    fact that we are not doing things right when we generate an index for an
    object file that has is linked to a dwz file.  The same happens whether
    the index is generated with the intent of populating the index cache or
    using the save gdb-index command.
    
    The problem can be observed when running these tests with the
    cc-with-dwz-m board:
    
        FAIL: gdb.base/index-cache.exp: test_cache_enabled_hit: check index-cache stats
        FAIL: gdb.dwarf2/gdb-index.exp: index used
        FAIL: gdb.dwarf2/gdb-index.exp: index used after symbol reloading
    
    When generating the index for such file and inspecting the CU list of the
    resulting index (with readelf --debug-dump=gdb_index), we can see something
    like:
    
        CU table:
        [  0] 0x0 - 0xb9
        [  1] 0x0 - 0x44
    
    This is supposed to be a sorted list of the ranges of all CUs in the
    file this index represents, so already having some overlap is a red
    flag.  It turns out that we save the ranges of CUs coming from both the
    main file and the dwz file in the same index.
    
    After digging a little bit, it became quite obvious that the index in
    the main file should only list the CUs present in the main file, and a
    separate index should be generated for the dwz file, listing the CUs
    present in that file.
    
    First, that's what happens if you run dwz on a file that already has a
    GDB index embedded.  Second, dwarf2read.c has code to read an index from
    a dwz file.  The index in the dwz file is actually required to be
    present, if the main file has an index.
    
    So this patch changes write_psymtabs_to_index to generate an index for
    the dwz file, if present.  That index only contains a CU list, just like
    what the dwz tool does when processing a file that already contains an
    index.
    
    Some notes about the implementation:
    
    - The file management (creating a temp file, make sure it's
      close/removed on error - in the right order) is a bit heavy in
      write_psymtabs_to_index, and I needed to add a third file.  I factored
      this pattern in a separate class, index_wip_file.
    - It became a bit tedious to keep the call to assert_file_size in
      write_psymtabs_to_index, write_gdbindex would have had to return two
      sizes.  Instead, I moved the calls to assert_file_size where the file
      is written.  The downside is that we lose the filename at this point,
      but it was only used for the very improbable case of ftell failing, so
      I think it's not a problem.
    - The actual writing of the index file is factored out to
      write_gdbindex_1, so it can be re-used for both index files.
    - While the "save gdb-index" command will now write two .gdb-index
      files, this patch does not update the gdb-add-index.sh script, this
      will come in a later patch.
...
Comment 3 Tom de Vries 2023-09-13 06:53:26 UTC
(In reply to Tom de Vries from comment #2)
> (In reply to Tom Tromey from comment #1)
> > My recollection is that in 'dwz -m' mode, all symbols from
> > PUs should be attributed to one of their containing CUs,
> 
> Yes.  That doesn't cover the case of a CU in the dwz file though.
> 

See also https://sourceware.org/git/?p=dwz.git;a=commit;h=b7111689a2ccec2f57343f1051ec8f1df5e89e5c