[PATCH][gdb/symtab] Fix CU list in .debug_names for dummy CUs

[PATCH][gdb/symtab] Fix CU list in .debug_names for dummy CUs

[Tom de Vries]

Hi,

With current trunk and target board cc-with-debug-names we have:
...
(gdb) file dw2-ranges-psym^M
Reading symbols from dw2-ranges-psym...^M
warning: Section .debug_names in dw2-ranges-psym has abbreviation_table of \
  size 1 vs. written as 28, ignoring .debug_names.^M
(gdb) set complaints 0^M
(gdb) FAIL: gdb.dwarf2/dw2-ranges-psym.exp: No complaints
...

The executable has 8 compilation units:
...
$ readelf -wi dw2-ranges-psym | grep @
  Compilation Unit @ offset 0x0:
  Compilation Unit @ offset 0x2e:
  Compilation Unit @ offset 0xa5:
  Compilation Unit @ offset 0xc7:
  Compilation Unit @ offset 0xd2:
  Compilation Unit @ offset 0x145:
  Compilation Unit @ offset 0x150:
  Compilation Unit @ offset 0x308:
...
of which the ones at 0xc7 and 0x145 are dummy CUs (that is, they do not
contain a single DIE), which were added by recent commit 5ef670d81fd
"[gdb/testsuite] Add dummy start and end CUs in dwarf assembly".

The .debug_names section contains this CU table:
...
[  0] 0x0
[  1] 0x2e
[  2] 0xa5
[  3] 0xd2
[  4] 0x150
[  5] 0x308
[  6] 0x1
[  7] 0x0
...
The last two entries are incorrect, and the entries for the dummy CUs are
missing.

The last two entries are incorrect because here in write_debug_names we write
the dimension of the CU list as 8:
...
  /* comp_unit_count - The number of CUs in the CU list.  */
  header.append_uint (4, dwarf5_byte_order,
                     per_objfile->per_bfd->all_comp_units.size ()
                     - per_objfile->per_bfd->tu_stats.nr_tus);
...
while the actual dimension of the CU list is 6.

The discrepancy is caused by this code which skips the dummy CUs:
...
  for (int i = 0; i < per_objfile->per_bfd->all_comp_units.size (); ++i)
    {
      ...
      /* CU of a shared file from 'dwz -m' may be unused by this main
        file.  It may be referenced from a local scope but in such
        case it does not need to be present in .debug_names.  */
      if (psymtab == NULL)
       continue;
...
because they have a null partial symtab.

We can fix this by writing the actual dimension of the CU list, but that still
leaves the dummy CUs out of the CU list.  The purpose of having these is to
delimit the end of preceding CUs.

So, fix this by:
- removing the code that skips the dummy CUs (note that the same change
  was done for .gdb_index in commit efba5c2319d '[gdb/symtab] Handle PU
  without import in "save gdb-index"'.
- verifying that all units are represented in the CU/TU lists
- using the actual CU list size when writing the dimension of the CU list
  (and likewise for the TU list).

Tested on x86_64-linux with native and target board cc-with-debug-names.

Any comments?

Thanks,
- Tom

[gdb/symtab] Fix CU list in .debug_names for dummy CUs

---
 gdb/dwarf2/index-write.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 4e00c716d91..0fe2bc4dd05 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1428,16 +1428,10 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
 	= per_objfile->per_bfd->all_comp_units[i].get ();
       partial_symtab *psymtab = per_cu->v.psymtab;
 
-      /* CU of a shared file from 'dwz -m' may be unused by this main
-	 file.  It may be referenced from a local scope but in such
-	 case it does not need to be present in .debug_names.  */
-      if (psymtab == NULL)
-	continue;
-
       int &this_counter = per_cu->is_debug_types ? types_counter : counter;
       data_buf &this_list = per_cu->is_debug_types ? types_cu_list : cu_list;
 
-      if (psymtab->user == NULL)
+      if (psymtab != nullptr && psymtab->user == nullptr)
 	nametable.recursively_write_psymbols (objfile, psymtab, psyms_seen,
 					      this_counter);
 
@@ -1447,6 +1441,11 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
       ++this_counter;
     }
 
+   /* Verify that all units are represented.  */
+  gdb_assert (counter == (per_objfile->per_bfd->all_comp_units.size ()
+			  - per_objfile->per_bfd->tu_stats.nr_tus));
+  gdb_assert (types_counter == per_objfile->per_bfd->tu_stats.nr_tus);
+
   nametable.build ();
 
   /* No addr_vec - DWARF-5 uses .debug_aranges generated by GCC.  */
@@ -1481,14 +1480,11 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
   header.append_uint (2, dwarf5_byte_order, 0);
 
   /* comp_unit_count - The number of CUs in the CU list.  */
-  header.append_uint (4, dwarf5_byte_order,
-		      per_objfile->per_bfd->all_comp_units.size ()
-		      - per_objfile->per_bfd->tu_stats.nr_tus);
+  header.append_uint (4, dwarf5_byte_order, counter);
 
   /* local_type_unit_count - The number of TUs in the local TU
      list.  */
-  header.append_uint (4, dwarf5_byte_order,
-		      per_objfile->per_bfd->tu_stats.nr_tus);
+  header.append_uint (4, dwarf5_byte_order, types_counter);
 
   /* foreign_type_unit_count - The number of TUs in the foreign TU
      list.  */

[committed][gdb/symtab] Fix CU list in .debug_names for dummy CUs

[Tom de Vries]

On 8/26/21 1:59 PM, Tom de Vries wrote:
> Hi,
> 
> With current trunk and target board cc-with-debug-names we have:
> ...
> (gdb) file dw2-ranges-psym^M
> Reading symbols from dw2-ranges-psym...^M
> warning: Section .debug_names in dw2-ranges-psym has abbreviation_table of \
>   size 1 vs. written as 28, ignoring .debug_names.^M
> (gdb) set complaints 0^M
> (gdb) FAIL: gdb.dwarf2/dw2-ranges-psym.exp: No complaints
> ...
> 
> The executable has 8 compilation units:
> ...
> $ readelf -wi dw2-ranges-psym | grep @
>   Compilation Unit @ offset 0x0:
>   Compilation Unit @ offset 0x2e:
>   Compilation Unit @ offset 0xa5:
>   Compilation Unit @ offset 0xc7:
>   Compilation Unit @ offset 0xd2:
>   Compilation Unit @ offset 0x145:
>   Compilation Unit @ offset 0x150:
>   Compilation Unit @ offset 0x308:
> ...
> of which the ones at 0xc7 and 0x145 are dummy CUs (that is, they do not
> contain a single DIE), which were added by recent commit 5ef670d81fd
> "[gdb/testsuite] Add dummy start and end CUs in dwarf assembly".
> 
> The .debug_names section contains this CU table:
> ...
> [  0] 0x0
> [  1] 0x2e
> [  2] 0xa5
> [  3] 0xd2
> [  4] 0x150
> [  5] 0x308
> [  6] 0x1
> [  7] 0x0
> ...
> The last two entries are incorrect, and the entries for the dummy CUs are
> missing.
> 
> The last two entries are incorrect because here in write_debug_names we write
> the dimension of the CU list as 8:
> ...
>   /* comp_unit_count - The number of CUs in the CU list.  */
>   header.append_uint (4, dwarf5_byte_order,
>                      per_objfile->per_bfd->all_comp_units.size ()
>                      - per_objfile->per_bfd->tu_stats.nr_tus);
> ...
> while the actual dimension of the CU list is 6.
> 
> The discrepancy is caused by this code which skips the dummy CUs:
> ...
>   for (int i = 0; i < per_objfile->per_bfd->all_comp_units.size (); ++i)
>     {
>       ...
>       /* CU of a shared file from 'dwz -m' may be unused by this main
>         file.  It may be referenced from a local scope but in such
>         case it does not need to be present in .debug_names.  */
>       if (psymtab == NULL)
>        continue;
> ...
> because they have a null partial symtab.
> 
> We can fix this by writing the actual dimension of the CU list, but that still
> leaves the dummy CUs out of the CU list.  The purpose of having these is to
> delimit the end of preceding CUs.
> 
> So, fix this by:
> - removing the code that skips the dummy CUs (note that the same change
>   was done for .gdb_index in commit efba5c2319d '[gdb/symtab] Handle PU
>   without import in "save gdb-index"'.
> - verifying that all units are represented in the CU/TU lists
> - using the actual CU list size when writing the dimension of the CU list
>   (and likewise for the TU list).
> 
> Tested on x86_64-linux with native and target board cc-with-debug-names.
> 
> Any comments?
> 

I've committed this.

Thanks,
- Tom

> [gdb/symtab] Fix CU list in .debug_names for dummy CUs
> 
> ---
>  gdb/dwarf2/index-write.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
> index 4e00c716d91..0fe2bc4dd05 100644
> --- a/gdb/dwarf2/index-write.c
> +++ b/gdb/dwarf2/index-write.c
> @@ -1428,16 +1428,10 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
>  	= per_objfile->per_bfd->all_comp_units[i].get ();
>        partial_symtab *psymtab = per_cu->v.psymtab;
>  
> -      /* CU of a shared file from 'dwz -m' may be unused by this main
> -	 file.  It may be referenced from a local scope but in such
> -	 case it does not need to be present in .debug_names.  */
> -      if (psymtab == NULL)
> -	continue;
> -
>        int &this_counter = per_cu->is_debug_types ? types_counter : counter;
>        data_buf &this_list = per_cu->is_debug_types ? types_cu_list : cu_list;
>  
> -      if (psymtab->user == NULL)
> +      if (psymtab != nullptr && psymtab->user == nullptr)
>  	nametable.recursively_write_psymbols (objfile, psymtab, psyms_seen,
>  					      this_counter);
>  
> @@ -1447,6 +1441,11 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
>        ++this_counter;
>      }
>  
> +   /* Verify that all units are represented.  */
> +  gdb_assert (counter == (per_objfile->per_bfd->all_comp_units.size ()
> +			  - per_objfile->per_bfd->tu_stats.nr_tus));
> +  gdb_assert (types_counter == per_objfile->per_bfd->tu_stats.nr_tus);
> +
>    nametable.build ();
>  
>    /* No addr_vec - DWARF-5 uses .debug_aranges generated by GCC.  */
> @@ -1481,14 +1480,11 @@ write_debug_names (dwarf2_per_objfile *per_objfile,
>    header.append_uint (2, dwarf5_byte_order, 0);
>  
>    /* comp_unit_count - The number of CUs in the CU list.  */
> -  header.append_uint (4, dwarf5_byte_order,
> -		      per_objfile->per_bfd->all_comp_units.size ()
> -		      - per_objfile->per_bfd->tu_stats.nr_tus);
> +  header.append_uint (4, dwarf5_byte_order, counter);
>  
>    /* local_type_unit_count - The number of TUs in the local TU
>       list.  */
> -  header.append_uint (4, dwarf5_byte_order,
> -		      per_objfile->per_bfd->tu_stats.nr_tus);
> +  header.append_uint (4, dwarf5_byte_order, types_counter);
>  
>    /* foreign_type_unit_count - The number of TUs in the foreign TU
>       list.  */
>