Re: [PATCH][gdb/symtab] Don't create duplicate psymtab for forward-imported CU

[Tom de Vries <tdevries@suse.de>]

On 08-04-2020 12:19, Tom de Vries wrote:
> Hi,
> 
> Consider the executable generated for test-case gdb.dwarf2/imported-unit.exp.
> 
> When loading the executable using various tracing:
> ...
> $ gdb \
>   outputs/gdb.dwarf2/imported-unit/imported-unit \
>   -batch \
>   -iex "set verbose on" \
>   -iex "set debug symtab-create 1"
>   ...
> Created psymtab 0x213f380 for module <artificial>@0xc7.
> Created psymtab 0x20e7b00 for module imported_unit.c.
> Created psymtab 0x215da20 for module imported_unit.c.
> Created psymtab 0x2133630 for module elf-init.c.
> Created psymtab 0x215b910 for module ../sysdeps/x86_64/crtn.S.
> ...
> we notice that there are two psymtabs generated for imported_unit.c.
> 
> This is due to the following: in dwarf2_build_psymtabs_hard we loop over CUs
> and generate partial symtabs for those, and if we encounter an import of
> another CU, we also generate a partial symtab for that one, unless already
> created.
> 
> This works well with backward import references:
> - the imported CU is read
> - then the importing CU is read
> - the import is encountered, but the imported CU is already read, so
>   we're done.
> 
> But with forward import references, we have instead:
> - the importing CU is read
> - the import is encountered, and the imported CU is read
> - the imported CU is read once more
> 
> Fix this by skipping already created psymtabs in the loop in
> dwarf2_build_psymtabs_hard.
> 
> Build in combination with the tentative fix for PR25718 - "[ada] symbol in
> imported unit not found"
> ( https://sourceware.org/pipermail/gdb-patches/2020-March/167152.html ) to
> prevent this regression:
> ...
> FAIL: gdb.ada/char_enum.exp: ptype pck.Global_Enum_Type
> ...
> 
> Tested on x86_64-linux, with native and target board
> unix/-flto/-O0/-flto-partition=none/-ffat-lto-objects.
> 
> This causes this regression with the target board:
> ...
> FAIL: gdb.ada/dgopt.exp: list x.adb:16, 16
> ...
> which I consider a seperate PR, filed as PR25801 - "Filename of shared psymtab
> is ignored".
> 

[ Forgot to mention ]

Using a 2.8GB libxul.so debug file from opensuse tumbleweed, build with
lto, we have:
...
$ time.sh gdb -batch -iex "maint set dwarf max-cache-age 1000" -iex "set
language c" usr/lib/debug/usr/lib64/firefox/libxul.so-74.0-1.1.x86_64.debug
maxmem: 10417196
real: 116.62
user: 114.85
system: 2.37
...
and with this patch:
...
$ time.sh gdb -batch -iex "maint set dwarf max-cache-age 1000" -iex "set
language c" usr/lib/debug/usr/lib64/firefox/libxul.so-74.0-1.1.x86_64.debug
maxmem: 10436372
real: 77.42
user: 75.54
system: 2.46

OK for trunk?

Thanks,
- Tom

> [gdb/symtab] Don't create duplicate psymtab for forward-imported CU
> 
> gdb/ChangeLog:
> 
> 2020-04-08  Tom de Vries  <tdevries@suse.de>
> 
> 	PR symtab/25700
> 	* dwarf2/read.c (dwarf2_build_psymtabs_hard): Don't create psymtab for
> 	CU if already created.
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-04-08  Tom de Vries  <tdevries@suse.de>
> 
> 	PR symtab/25700
> 	* gdb.dwarf2/imported-unit.exp: Verify that there's only one partial
> 	symtab for imported_unit.c.
> 
> ---
>  gdb/dwarf2/read.c                          |  7 ++++++-
>  gdb/testsuite/gdb.dwarf2/imported-unit.exp | 18 ++++++++++++++++++
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index c0b5e58b17..8b761e891e 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -7781,7 +7781,12 @@ dwarf2_build_psymtabs_hard (struct dwarf2_per_objfile *dwarf2_per_objfile)
>  			   addrmap_create_mutable (&temp_obstack));
>  
>    for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
> -    process_psymtab_comp_unit (per_cu, false, language_minimal);
> +    {
> +      if (per_cu->v.psymtab != NULL)
> +	/* In case a forward DW_TAG_imported_unit has read the CU already.  */
> +	continue;
> +      process_psymtab_comp_unit (per_cu, false, language_minimal);
> +    }
>  
>    /* This has to wait until we read the CUs, we need the list of DWOs.  */
>    process_skeletonless_type_units (dwarf2_per_objfile);
> diff --git a/gdb/testsuite/gdb.dwarf2/imported-unit.exp b/gdb/testsuite/gdb.dwarf2/imported-unit.exp
> index 41a7505459..d7b3d4c539 100644
> --- a/gdb/testsuite/gdb.dwarf2/imported-unit.exp
> +++ b/gdb/testsuite/gdb.dwarf2/imported-unit.exp
> @@ -168,6 +168,24 @@ if { $psymtabs_p } {
>  } else {
>      unsupported $test
>  }
> +
> +# Verify that there's only one partial symtab for imported_unit.c.  Test-case
> +# for PR25700.
> +set test "no duplicate psymtab for imported_unit.c"
> +if { $psymtabs_p } {
> +    set line "Partial symtab for source file imported_unit.c"
> +    gdb_test_multiple "maint print psymbols" $test {
> +	-re -wrap "$line.*$line.*" {
> +	    fail $gdb_test_name
> +	}
> +	-re -wrap "$line.*" {
> +	    pass $gdb_test_name
> +	}
> +    }
> +} else {
> +    unsupported $test
> +}
> +
>  # Sanity check
>  gdb_test "ptype main" "= int \\(void\\)"
>  
>