Re: [PATCH][gdb/symtab] Fix Dwarf Error: cannot find DIE

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-05-26 5:54 p.m., Tom de Vries wrote:
> Hi,
> 
> When loading the debug info package
> libLLVM.so.10-10.0.1-lp152.30.4.x86_64.debug from openSUSE Leap 15.2, we
> run into a dwarf error:
> ...
> $ gdb -q -batch libLLVM.so.10-10.0.1-lp152.30.4.x86_64.debug
> Dwarf Error: Cannot not find DIE at 0x18a936e7 \
>   [from module libLLVM.so.10-10.0.1-lp152.30.4.x86_64.debug]
> ...
> The DIE @ 0x18a936e7 does in fact exist, and is part of a CU @ 0x18a23e52.
> No error message is printed when using -readnow.
> 
> What happens is the following:
> - a dwarf2_per_cu_data P is created for the CU.
> - a dwarf2_cu A is created for the same CU.
> - another dwarf2_cu B is created for the same CU.
> - the dwarf2_cu B is set in per_objfile->m_dwarf2_cus, such that
>   per_objfile->get_cu (P) returns B.
> - P->load_all_dies is set to 1.
> - all dies are read into the A->partial_dies htab
> - dwarf2_cu A is destroyed.
> - we try to find the partial_die for the DIE @ 0x18a936e7 in B->partial_dies.
>   We can't find it, but do not try to load all dies, because P->load_all_dies
>   is already set to 1.
> - an error message is generated.
> 
> The question is why we're creating dwarf2_cu A and B for the same CU.
> 
> The dwarf2_cu A is created here:
> ...
>  (gdb) bt
>  #0  dwarf2_cu::dwarf2_cu (this=0x79a9660, per_cu=0x23c0b30,
>      per_objfile=0x1ad01b0) at dwarf2/cu.c:38
>  #1  0x0000000000675799 in cutu_reader::cutu_reader (this=0x7fffffffd040,
>      this_cu=0x23c0b30, per_objfile=0x1ad01b0, abbrev_table=0x0,
>      existing_cu=0x0, skip_partial=false) at dwarf2/read.c:6487
>  #2  0x0000000000676eb3 in process_psymtab_comp_unit (this_cu=0x23c0b30,
>       per_objfile=0x1ad01b0, want_partial_unit=false,
>       pretend_language=language_minimal) at dwarf2/read.c:7028
> ...
> 
> And the dwarf2_cu B is created here:
> ...
>  (gdb) bt
>  #0  dwarf2_cu::dwarf2_cu (this=0x885e8c0, per_cu=0x23c0b30,
>      per_objfile=0x1ad01b0) at dwarf2/cu.c:38
>  #1  0x0000000000675799 in cutu_reader::cutu_reader (this=0x7fffffffcc50,
>      this_cu=0x23c0b30, per_objfile=0x1ad01b0, abbrev_table=0x0,
>      existing_cu=0x0, skip_partial=false) at dwarf2/read.c:6487
>  #2  0x0000000000678118 in load_partial_comp_unit (this_cu=0x23c0b30,
>      per_objfile=0x1ad01b0, existing_cu=0x0) at dwarf2/read.c:7436
>  #3  0x000000000069721d in find_partial_die (sect_off=(unknown: 0x18a55054),
>      offset_in_dwz=0, cu=0x0) at dwarf2/read.c:19391
>  #4  0x000000000069755b in partial_die_info::fixup (this=0x9096900,
>      cu=0xa6a85f0) at dwarf2/read.c:19512
>  #5  0x0000000000697586 in partial_die_info::fixup (this=0x8629bb0,
>      cu=0xa6a85f0) at dwarf2/read.c:19516
>  #6  0x00000000006787b1 in scan_partial_symbols (first_die=0x8629b40,
>      lowpc=0x7fffffffcf58, highpc=0x7fffffffcf50, set_addrmap=0, cu=0x79a9660)
>      at dwarf2/read.c:7563
>  #7  0x0000000000678878 in scan_partial_symbols (first_die=0x796ebf0,
>      lowpc=0x7fffffffcf58, highpc=0x7fffffffcf50, set_addrmap=0, cu=0x79a9660)
>      at dwarf2/read.c:7580
>  #8  0x0000000000676b82 in process_psymtab_comp_unit_reader
>      (reader=0x7fffffffd040, info_ptr=0x7fffc1b3f29b, comp_unit_die=0x6ea90f0,
>      pretend_language=language_minimal) at dwarf2/read.c:6954
>  #9  0x0000000000676ffd in process_psymtab_comp_unit (this_cu=0x23c0b30,
>      per_objfile=0x1ad01b0, want_partial_unit=false,
>      pretend_language=language_minimal) at dwarf2/read.c:7057
> ...
> 
> So in frame #9, a cutu_reader is created with dwarf2_cu A.  Then a fixup takes
> us to the following CU @ 0x18aa33d6, in frame #5.  And a similar fixup in
> frame #4 takes us back to CU @ 0x18a23e52.  At that point, there's no
> information available that we're already trying to read that CU, and we end up
> creating another cutu_reader with dwarf2_cu B.
> 
> It seems that there are two related problems:
> - creating two dwarf2_cu's is not optimal
> - the unoptimal case is not handled correctly
> 
> This patch addresses the last problem, by moving the load_all_dies flag from
> dwarf2_per_cu_data to dwarf2_cu, such that it is paired with the partial_dies
> field, which ensures that the two can be kept in sync.
> 
> Tested on x86_64-linux.
> 
> Any comments?
> 
> Thanks,
> - Tom

The fix looks sensible to me.

You know what I'll say: IWBN to have a test case if we don't want this
to regress again, if it's humanly possible to write one using the DWARF
assembler.

> [gdb/symtab] Fix Dwarf Error: cannot find DIE
> 
> gdb/ChangeLog:
> 
> 2021-05-26  Tom de Vries  <tdevries@suse.de>
> 
> 	PR symtab/27898
> 	* dwarf2/cu.c (dwarf2_cu::dwarf2_cu): Add load_all_dies init.
> 	* dwarf2/cu.h (dwarf2_cu): Add load_all_dies field.
> 	* dwarf2/read.c (load_partial_dies): Update.
> 	(find_partial_die):
> 	* dwarf2/read.h (dwarf2_per_cu_data::dwarf2_per_cu_data): Remove
> 	load_all_dies init.
> 	(dwarf2_per_cu_data): Remove load_all_dies field.
> 
> ---
>  gdb/dwarf2/cu.c   | 3 ++-
>  gdb/dwarf2/cu.h   | 6 ++++++
>  gdb/dwarf2/read.c | 6 +++---
>  gdb/dwarf2/read.h | 7 -------
>  4 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/dwarf2/cu.c b/gdb/dwarf2/cu.c
> index 2451df4f5b6..1031ed3aa00 100644
> --- a/gdb/dwarf2/cu.c
> +++ b/gdb/dwarf2/cu.c
> @@ -35,7 +35,8 @@ dwarf2_cu::dwarf2_cu (dwarf2_per_cu_data *per_cu,
>      producer_is_icc (false),
>      producer_is_icc_lt_14 (false),
>      producer_is_codewarrior (false),
> -    processing_has_namespace_info (false)
> +    processing_has_namespace_info (false),
> +    load_all_dies (false)
>  {
>  }
>  
> diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h
> index ff56ec5527b..65a4cf8babf 100644
> --- a/gdb/dwarf2/cu.h
> +++ b/gdb/dwarf2/cu.h
> @@ -283,6 +283,12 @@ struct dwarf2_cu
>  
>      return nullptr;
>    }
> +
> +  /* This flag will be set when reading partial DIEs if we need to load
> +     absolutely all DIEs for this compilation unit, instead of just the ones
> +     we think are interesting.  It gets set if we look for a DIE in the
> +     hash table and don't find it.  */
> +  unsigned int load_all_dies : 1;

If the goal is to save some space by using bitfields, it would be a good
idea to place it next to the other ones above (and change the type to
bool to be like the others).  And maybe check with ptype/o that it has
the desired effect.

Simon