[RFC 0/2] [gdb/symtab] Don't expand type units for breakpoints

[RFC 0/2] [gdb/symtab] Don't expand type units for breakpoints

[Tom de Vries]

This RFC patch series contains two patches.

The first patch is a preparation patch.

The second patch contains a naive fix for PR33922.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33922

Tom de Vries (2):
  [gdb/symtab] Propagate search domain to process_queue
  [gdb/symtab] Don't expand type units for breakpoints

 gdb/dwarf2/read.c | 37 +++++++++++++++++++++++++------------
 gdb/dwarf2/read.h |  3 ++-
 2 files changed, 27 insertions(+), 13 deletions(-)


base-commit: 2db79d8000d40451b313ad6c3b6c029bcf82e643
-- 
2.51.0

[RFC 1/2] [gdb/symtab] Propagate search domain to process_queue

[Tom de Vries]

Propagate the active search domain to function process_queue.  No functional
changes.
---
 gdb/dwarf2/read.c | 25 ++++++++++++++-----------
 gdb/dwarf2/read.h |  3 ++-
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4035bba6e45..afd0b60006e 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -935,7 +935,8 @@ static struct type *get_die_type (struct die_info *die, struct dwarf2_cu *cu);
 
 static void queue_comp_unit (dwarf2_cu *cu);
 
-static void process_queue (dwarf2_per_objfile *per_objfile);
+static void process_queue (dwarf2_per_objfile *per_objfile,
+			   domain_search_flags domain = 0);
 
 /* Class, the destructor of which frees all allocated queue entries.  This
    will only have work to do if an error was thrown while processing the
@@ -1481,7 +1482,7 @@ struct readnow_functions : public dwarf2_base_index_functions
 	    || per_objfile->get_compunit_symtab (per_cu.get ()) == nullptr)
 	  continue;
 	if (!search_one (per_cu.get (), per_objfile, cus_to_skip, file_matcher,
-			 listener, lang_matcher))
+			 listener, lang_matcher, domain))
 	  return false;
       }
     return true;
@@ -1532,7 +1533,8 @@ load_cu (dwarf2_per_cu *per_cu, dwarf2_per_objfile *per_objfile,
 
 static void
 dw2_do_instantiate_symtab (dwarf2_per_cu *per_cu,
-			   dwarf2_per_objfile *per_objfile, bool skip_partial)
+			   dwarf2_per_objfile *per_objfile, bool skip_partial,
+			   domain_search_flags domain = 0)
 {
   {
     /* The destructor of dwarf2_queue_guard frees any entries left on
@@ -1560,7 +1562,7 @@ dw2_do_instantiate_symtab (dwarf2_per_cu *per_cu,
 	    && per_objfile->per_bfd->dwp_file == nullptr)
 	  queue_and_load_all_dwo_tus (cu);
 
-	process_queue (per_objfile);
+	process_queue (per_objfile, domain);
       }
   }
 
@@ -1576,7 +1578,7 @@ dw2_do_instantiate_symtab (dwarf2_per_cu *per_cu,
 
 static struct compunit_symtab *
 dw2_instantiate_symtab (dwarf2_per_cu *per_cu, dwarf2_per_objfile *per_objfile,
-			bool skip_partial)
+			bool skip_partial, domain_search_flags domain = 0)
 {
   /* Expand the corresponding canonical outermost CU.  This will
      expand the desired CU as a side effect when following the
@@ -1587,7 +1589,7 @@ dw2_instantiate_symtab (dwarf2_per_cu *per_cu, dwarf2_per_objfile *per_objfile,
     {
       free_cached_comp_units freer (per_objfile);
       scoped_restore decrementer = increment_reading_symtab ();
-      dw2_do_instantiate_symtab (per_cu, per_objfile, skip_partial);
+      dw2_do_instantiate_symtab (per_cu, per_objfile, skip_partial, domain);
     }
 
   return per_objfile->get_compunit_symtab (per_cu);
@@ -1925,7 +1927,8 @@ dwarf2_base_index_functions::search_one
    auto_bool_vector &cus_to_skip,
    search_symtabs_file_matcher file_matcher,
    search_symtabs_expansion_listener listener,
-   search_symtabs_lang_matcher lang_matcher)
+   search_symtabs_lang_matcher lang_matcher,
+   domain_search_flags domain)
 {
   /* Already visited, or intentionally skipped.  */
   if (cus_to_skip.is_set (per_cu->index))
@@ -1941,7 +1944,7 @@ dwarf2_base_index_functions::search_one
     }
 
   compunit_symtab *symtab
-    = dw2_instantiate_symtab (per_cu, per_objfile, false);
+    = dw2_instantiate_symtab (per_cu, per_objfile, false, domain);
   gdb_assert (symtab != nullptr);
 
   if (listener != nullptr)
@@ -3886,7 +3889,7 @@ maybe_queue_comp_unit (dwarf2_cu *cu, dwarf2_cu *dependent_cu)
 /* Process the queue.  */
 
 static void
-process_queue (dwarf2_per_objfile *per_objfile)
+process_queue (dwarf2_per_objfile *per_objfile, domain_search_flags domain)
 {
   DWARF_READ_SCOPED_DEBUG_START_END
     ("Expanding one or more symtabs of objfile %s ...",
@@ -14057,7 +14060,7 @@ cooked_index_functions::search
 	  QUIT;
 
 	  if (!search_one (per_cu, per_objfile, cus_to_skip, file_matcher,
-			   listener, lang_matcher))
+			   listener, lang_matcher, domain))
 	    return false;
 	}
       return true;
@@ -14225,7 +14228,7 @@ cooked_index_functions::search
 	  bool check = entry->visit_defining_cus ([&] (dwarf2_per_cu *per_cu)
 	    {
 	      return search_one (per_cu, per_objfile, cus_to_skip,
-				 file_matcher, listener, nullptr);
+				 file_matcher, listener, nullptr, domain);
 	    });
 	  if (!check)
 	    return false;
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 1150c3cfdca..f21a8bd87aa 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -1324,7 +1324,8 @@ struct dwarf2_base_index_functions : public quick_symbol_functions
 		   auto_bool_vector &cus_to_skip,
 		   search_symtabs_file_matcher file_matcher,
 		   search_symtabs_expansion_listener listener,
-		   search_symtabs_lang_matcher lang_matcher);
+		   search_symtabs_lang_matcher lang_matcher,
+		   domain_search_flags domain);
 };
 
 /* Return pointer to string at .debug_str offset STR_OFFSET.  */
-- 
2.51.0

[RFC 2/2] [gdb/symtab] Don't expand type units for breakpoints

[Tom de Vries]

Consider a cc1plus exec compiled with GCC and LTO [1].

If we try to set a breakpoint on do_rpo_vn:
...
$ gdb -q -batch cc1plus \
    -ex "b do_rpo_vn"
Breakpoint 1 at 0xfd75f0: do_rpo_vn. (2 locations)
...
we expand 435 units:
...
$ gdb -q -batch cc1plus \
    -ex "b do_rpo_vn" \
    -ex "maint print statistics" \
  | grep "Number of read units"
  Number of read units: 435
...

[ When compiling with LTO, GCC generates debug info in two phases:
- in the early phase, it generates a unit for each source file, containing the
  types in the source file.  Let's call these type-only units.
- in the late phase, it generates a so-called artificial unit for each
  optimized code blob.  The artificial unit doesn't contains types, but
  imports them from the type-only units.

The cc1plus exec contains 1150 units, 128 of which are artificials units:
...
$ readelf -wi --dwarf-depth=1 cc1plus | grep -c "Compilation Unit @"
1150
$ readelf -wi --dwarf-depth=1 cc1plus | grep -c "<artificial>"
128
... ]

Most of the expanded units are type-only units, which have no relevant
information when we're looking for function symbols.

They're merely expanded because the artificial units import them.

So the question is: can we skip expanding the type-only units when looking for
function symbols?

This patch contains a naive implementation of that approach, in process_queue.

Using it, we expand just 7 artificial units:
...
$ gdb -q -batch cc1plus \
    -ex "b do_rpo_vn" \
    -ex "maint print statistics" \
  | grep "Number of read units"
  Number of read units: 7
...

But there's a problem with this implementation, which we can illustrate using
DWZ:
...
$ cat test.c
struct s {
  int x0, x1, x2, x3, x4, x5, x6, x7, x8, x9;
};
struct s var1;
int main (void) {
  return 0;
}
$ gcc -g test.c; rm -f ab.dwz; cp a.out b.out; dwz -m ab.dwz a.out b.out
$ gdb -q -batch a.out -ex "b main" -ex "ptype struct s"
Breakpoint 1 at 0x40111a: file test.c, line 6.
No struct type named s.
...

First, we use DWZ to move struct s to a partial unit.

Then in the GDB session the following happens:
- setting a breakpoint on main expands the test.c CU
- the test.c CU has an import of the partial CU, but the partial CU is not
  expanded, because it contains only types
- printing struct s attempts to expand test.c CU, finds that it's already
  expanded, and the partial CU containing struct s remains unexpanded.
- gdb tries to find struct s in the expanded CUs, and struct s is not found.

RFC: what is the best way to fix this problem?

I wondered about expanding function compunit_symtab_set_p et al with a
domain_search_flags parameter, giving us a way to express that a CU is
expanded for SEARCH_FUNCTION_DOMAIN, but not for other domains.

But there may be a better way to achieve this.

[ FWIW, I've tested this on x86_64-linux, and ran into a number of FAILs:
...
      9 gdb.cp/cpexprs-debug-types.exp:
      1 gdb.dlang/circular.exp:
      2 gdb.dwarf2/ada-cold-name.exp:
      4 gdb.dwarf2/ada-linkage-name.exp:
     14 gdb.dwarf2/dw2-bad-abstract-origin.exp:
      1 gdb.dwarf2/dw2-inline-bt.exp:
      1 gdb.dwarf2/dw2-prologue-end-2.exp:
      1 gdb.dwarf2/dw2-ranges-psym.exp:
      7 gdb.dwarf2/dw2-step-between-different-inline-functions.exp:
      6 gdb.dwarf2/dw2-step-between-inline-func-blocks.exp:
     60 gdb.dwarf2/dw2-unexpected-entry-pc.exp:
      1 gdb.dwarf2/inlined_subroutine-inheritance.exp:
      1 gdb.dwarf2/struct-decl.exp:
... ]

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33922
---
 gdb/dwarf2/read.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index afd0b60006e..475dc03f0b0 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1945,7 +1945,8 @@ dwarf2_base_index_functions::search_one
 
   compunit_symtab *symtab
     = dw2_instantiate_symtab (per_cu, per_objfile, false, domain);
-  gdb_assert (symtab != nullptr);
+  if (symtab == nullptr)
+    return true;
 
   if (listener != nullptr)
     {
@@ -3908,6 +3909,15 @@ process_queue (dwarf2_per_objfile *per_objfile, domain_search_flags domain)
 
       gdb_assert (!per_objfile->compunit_symtab_set_p (per_cu));
 
+      bool skip = (domain == SEARCH_FUNCTION_DOMAIN
+		   && !per_cu->addresses_seen);
+      if (skip)
+	{
+	  cu->queued = false;
+	  per_objfile->queue->pop ();
+	  continue;
+	}
+
       namespace chr = std::chrono;
 
       unsigned int debug_print_threshold;
-- 
2.51.0

Re: [RFC 2/2] [gdb/symtab] Don't expand type units for breakpoints

[Tom de Vries]

On 4/8/26 1:08 PM, Tom de Vries wrote:
> Consider a cc1plus exec compiled with GCC and LTO [1].
> 
> If we try to set a breakpoint on do_rpo_vn:
> ...
> $ gdb -q -batch cc1plus \
>      -ex "b do_rpo_vn"
> Breakpoint 1 at 0xfd75f0: do_rpo_vn. (2 locations)
> ...
> we expand 435 units:
> ...
> $ gdb -q -batch cc1plus \
>      -ex "b do_rpo_vn" \
>      -ex "maint print statistics" \
>    | grep "Number of read units"
>    Number of read units: 435
> ...
> 
> [ When compiling with LTO, GCC generates debug info in two phases:
> - in the early phase, it generates a unit for each source file, containing the
>    types in the source file.  Let's call these type-only units.
> - in the late phase, it generates a so-called artificial unit for each
>    optimized code blob.  The artificial unit doesn't contains types, but
>    imports them from the type-only units.
> 
> The cc1plus exec contains 1150 units, 128 of which are artificials units:
> ...
> $ readelf -wi --dwarf-depth=1 cc1plus | grep -c "Compilation Unit @"
> 1150
> $ readelf -wi --dwarf-depth=1 cc1plus | grep -c "<artificial>"
> 128
> ... ]
> 
> Most of the expanded units are type-only units, which have no relevant
> information when we're looking for function symbols.
> 
> They're merely expanded because the artificial units import them.
> 
> So the question is: can we skip expanding the type-only units when looking for
> function symbols?
> 
> This patch contains a naive implementation of that approach, in process_queue.
> 
> Using it, we expand just 7 artificial units:
> ...
> $ gdb -q -batch cc1plus \
>      -ex "b do_rpo_vn" \
>      -ex "maint print statistics" \
>    | grep "Number of read units"
>    Number of read units: 7
> ...
> 
> But there's a problem with this implementation, which we can illustrate using
> DWZ:
> ...
> $ cat test.c
> struct s {
>    int x0, x1, x2, x3, x4, x5, x6, x7, x8, x9;
> };
> struct s var1;
> int main (void) {
>    return 0;
> }
> $ gcc -g test.c; rm -f ab.dwz; cp a.out b.out; dwz -m ab.dwz a.out b.out
> $ gdb -q -batch a.out -ex "b main" -ex "ptype struct s"
> Breakpoint 1 at 0x40111a: file test.c, line 6.
> No struct type named s.
> ...
> 
> First, we use DWZ to move struct s to a partial unit.
> 
> Then in the GDB session the following happens:
> - setting a breakpoint on main expands the test.c CU
> - the test.c CU has an import of the partial CU, but the partial CU is not
>    expanded, because it contains only types
> - printing struct s attempts to expand test.c CU, finds that it's already
>    expanded, and the partial CU containing struct s remains unexpanded.
> - gdb tries to find struct s in the expanded CUs, and struct s is not found.
> 

I tried bypassing this problem for DWZ using:
...
@@ -3910,6 +3910,7 @@ process_queue (dwarf2_per_objfile *per_objfile, 
domain_search_flags domain)
        gdb_assert (!per_objfile->compunit_symtab_set_p (per_cu));

        bool skip = (domain == SEARCH_FUNCTION_DOMAIN
+		   && cu->header.unit_type != DW_UT_partial
  		   && !per_cu->addresses_seen);
        if (skip)
  	{
...
and that does fix the DWZ counter-example, but not the testsuite 
regressions mentioned below.  IWBN though to have this work for partial 
units as well.

> RFC: what is the best way to fix this problem?
> 
> I wondered about expanding function compunit_symtab_set_p et al with a
> domain_search_flags parameter, giving us a way to express that a CU is
> expanded for SEARCH_FUNCTION_DOMAIN, but not for other domains.
> 
> But there may be a better way to achieve this.
> 
> [ FWIW, I've tested this on x86_64-linux, and ran into a number of FAILs:
> ...
>        9 gdb.cp/cpexprs-debug-types.exp:
>        1 gdb.dlang/circular.exp:
>        2 gdb.dwarf2/ada-cold-name.exp:
>        4 gdb.dwarf2/ada-linkage-name.exp:
>       14 gdb.dwarf2/dw2-bad-abstract-origin.exp:
>        1 gdb.dwarf2/dw2-inline-bt.exp:
>        1 gdb.dwarf2/dw2-prologue-end-2.exp:
>        1 gdb.dwarf2/dw2-ranges-psym.exp:
>        7 gdb.dwarf2/dw2-step-between-different-inline-functions.exp:
>        6 gdb.dwarf2/dw2-step-between-inline-func-blocks.exp:
>       60 gdb.dwarf2/dw2-unexpected-entry-pc.exp:
>        1 gdb.dwarf2/inlined_subroutine-inheritance.exp:
>        1 gdb.dwarf2/struct-decl.exp:
> ... ]
>

I investigated the last failure.  The problem is that addresses_seen is 
false in this CU:
...
  <0><2dc>: Abbrev Number: 1 (DW_TAG_compile_unit)
     <2dd>   DW_AT_language    : 4       (C++)
     <2de>   DW_AT_name        : main.c
     <2e5>   DW_AT_comp_dir    : /tmp
  <1><2ea>: Abbrev Number: 2 (DW_TAG_structure_type)
     <2eb>   DW_AT_byte_size   : 8
     <2ec>   DW_AT_encoding    : 5       (signed)
     <2ed>   DW_AT_name        : the_type
     <2f6>   DW_AT_declaration : 1
  <2><2f6>: Abbrev Number: 3 (DW_TAG_subprogram)
     <2f7>   DW_AT_name        : method
     <2fe>   DW_AT_declaration : 1
  <2><2fe>: Abbrev Number: 0
  <1><2ff>: Abbrev Number: 4 (DW_TAG_subprogram)
     <300>   DW_AT_specification: <0x2f6>
     <304>   DW_AT_low_pc      : 0x401116
     <30c>   DW_AT_high_pc     : 0x401121
  <1><314>: Abbrev Number: 0
...
because the top-level DIE does not contain a PC range, despite the CU 
containing a subprogram with PC range.

Thanks,
- Tom

> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33922
> ---
>   gdb/dwarf2/read.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index afd0b60006e..475dc03f0b0 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -1945,7 +1945,8 @@ dwarf2_base_index_functions::search_one
>   
>     compunit_symtab *symtab
>       = dw2_instantiate_symtab (per_cu, per_objfile, false, domain);
> -  gdb_assert (symtab != nullptr);
> +  if (symtab == nullptr)
> +    return true;
>   
>     if (listener != nullptr)
>       {
> @@ -3908,6 +3909,15 @@ process_queue (dwarf2_per_objfile *per_objfile, domain_search_flags domain)
>   
>         gdb_assert (!per_objfile->compunit_symtab_set_p (per_cu));
>   
> +      bool skip = (domain == SEARCH_FUNCTION_DOMAIN
> +		   && !per_cu->addresses_seen);
> +      if (skip)
> +	{
> +	  cu->queued = false;
> +	  per_objfile->queue->pop ();
> +	  continue;
> +	}
> +
>         namespace chr = std::chrono;
>   
>         unsigned int debug_print_threshold;