[PATCH 00/14] Share DWARF partial symtabs between objfiles

[PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

A long-term goal for multi-inferior debugging has been to "split" an
objfile, so that the bulk of the data can be shared across inferiors.
Although a lot of progress has been made on this front, it has turned
out to be surprisingly difficult to fully implement.

"Recently" (a year or two ago) I realized that we could get most of
the benefits of this split by sharing partial symbol tables.  This is
true because reading partial symbols is the slowest operation that
users see -- in most cases, expanding a full symtab is reasonably
quick.

Implementing this also turned out to be tricky; but unlike the
situation with types and symbols, it was possible to do incrementally.

This series implements this idea for DWARF.  It separates
dwarf2_per_objfile into shareable and unshareable parts; then, when
possible, the object is attached to the BFD.

You can see the difference when timing "add-inferior -exec ./gdb",
after "gdb ./gdb":

Before: Command execution time: 1.667661 (cpu), 1.687607 (wall)
After : Command execution time: 0.150011 (cpu), 0.151292 (wall)

In this series I did not rename dwarf2_per_objfile.  I'd prefer to do
this after landing this series; it seemed like a large, mostly
cosmetic, and yet hard-to-rebase patch.  Also, I wanted to consult
with everyone else about what we ought to call it.

I also didn't attempt to implement this sharing for CTF.  I don't
really know much about CTF; but it seems like it ought to be possible.
(Though I wonder if CTF wouldn't benefit more from simply bypassing
partial symbols entirely.)

Regression tested on x86-64 Fedora 28.  I also did various tests by
hand, and tested it by hand, under valgrind, on a gdb built with
-fdebug-types-section.

Like I mentioned earlier, this is a tricky series, so it would benefit
from careful review.  The main danger is that, if I missed a spot, we
could end up in a situation where gdb will crash in a multi-inferior
scenario.

Tom

[PATCH 07/14] Add dwarf2_per_cu_data::index

[Tom Tromey]

Currently a dwarf2_per_cu_data can hold a link to the corresponding
compunit_symtab.  However, once these objects are shared across
objfiles, a simple pointer won't work.

Instead, this link will be stored in the dwarf2_unshareable object.
To enable this, we add an index to each dwarf2_per_cu_data and
signatured_type.

2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.h (struct dwarf2_per_objfile) <allocate_per_cu,
	allocate_signatured_type>: Declare new methods.
	(struct dwarf2_per_cu_data) <index>: New member.
	(struct dwarf2_per_objfile) <num_psymtabs>: New member.
	* dwarf2/read.c (dwarf2_per_objfile::allocate_per_cu)
	(dwarf2_per_objfile::allocate_signatured_type): New methods.
	(create_cu_from_index_list): Use allocate_per_cu.
	(create_signatured_type_table_from_index)
	(create_signatured_type_table_from_debug_names)
	(create_debug_type_hash_table, add_type_unit)
	(read_comp_units_from_section): Use allocate_signatured_type.
---
 gdb/ChangeLog     | 14 +++++++++++++
 gdb/dwarf2/read.c | 52 +++++++++++++++++++++++++++--------------------
 gdb/dwarf2/read.h | 18 ++++++++++++++++
 3 files changed, 62 insertions(+), 22 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index cb04eb19beb..281d39ad271 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2415,18 +2415,36 @@ dwarf2_per_objfile::get_tu (int index)
   return this->all_type_units[index];
 }
 
+/* See read.h.  */
+
+dwarf2_per_cu_data *
+dwarf2_per_objfile::allocate_per_cu ()
+{
+  dwarf2_per_cu_data *result = OBSTACK_ZALLOC (&obstack, dwarf2_per_cu_data);
+  result->index = num_psymtabs++;
+  return result;
+}
+
+/* See read.h.  */
+
+signatured_type *
+dwarf2_per_objfile::allocate_signatured_type ()
+{
+  signatured_type *result = OBSTACK_ZALLOC (&obstack, signatured_type);
+  result->per_cu.index = num_psymtabs++;
+  return result;
+}
+
 /* Return a new dwarf2_per_cu_data allocated on the dwarf2_per_objfile
    obstack, and constructed with the specified field values.  */
 
 static dwarf2_per_cu_data *
 create_cu_from_index_list (struct dwarf2_per_objfile *dwarf2_per_objfile,
-                          struct dwarf2_section_info *section,
-                          int is_dwz,
-                          sect_offset sect_off, ULONGEST length)
+			   struct dwarf2_section_info *section,
+			   int is_dwz,
+			   sect_offset sect_off, ULONGEST length)
 {
-  dwarf2_per_cu_data *the_cu
-    = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
-                     struct dwarf2_per_cu_data);
+  dwarf2_per_cu_data *the_cu = dwarf2_per_objfile->allocate_per_cu ();
   the_cu->sect_off = sect_off;
   the_cu->length = length;
   the_cu->dwarf2_per_objfile = dwarf2_per_objfile;
@@ -2517,8 +2535,7 @@ create_signatured_type_table_from_index
       signature = extract_unsigned_integer (bytes + 16, 8, BFD_ENDIAN_LITTLE);
       bytes += 3 * 8;
 
-      sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
-				 struct signatured_type);
+      sig_type = dwarf2_per_objfile->allocate_signatured_type ();
       sig_type->signature = signature;
       sig_type->type_offset_in_tu = type_offset_in_tu;
       sig_type->per_cu.is_debug_types = 1;
@@ -2574,8 +2591,7 @@ create_signatured_type_table_from_debug_names
 				     section->buffer + to_underlying (sect_off),
 				     rcuh_kind::TYPE);
 
-      sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
-				 struct signatured_type);
+      sig_type = dwarf2_per_objfile->allocate_signatured_type ();
       sig_type->signature = cu_header.signature;
       sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
       sig_type->per_cu.is_debug_types = 1;
@@ -6091,8 +6107,7 @@ create_debug_type_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile,
 	  /* N.B.: type_offset is not usable if this type uses a DWO file.
 	     The real type_offset is in the DWO file.  */
 	  dwo_tu = NULL;
-	  sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
-				     struct signatured_type);
+	  sig_type = dwarf2_per_objfile->allocate_signatured_type ();
 	  sig_type->signature = header.signature;
 	  sig_type->type_offset_in_tu = header.type_cu_offset_in_tu;
 	  sig_type->per_cu.dwarf2_per_objfile = dwarf2_per_objfile;
@@ -6207,8 +6222,7 @@ add_type_unit (struct dwarf2_per_objfile *dwarf2_per_objfile, ULONGEST sig,
       == dwarf2_per_objfile->all_type_units.capacity ())
     ++dwarf2_per_objfile->tu_stats.nr_all_type_units_reallocs;
 
-  signatured_type *sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
-					      struct signatured_type);
+  signatured_type *sig_type = dwarf2_per_objfile->allocate_signatured_type ();
 
   dwarf2_per_objfile->all_type_units.push_back (sig_type);
   sig_type->signature = sig;
@@ -7835,16 +7849,10 @@ read_comp_units_from_section (struct dwarf2_per_objfile *dwarf2_per_objfile,
 
       /* Save the compilation unit for later lookup.  */
       if (cu_header.unit_type != DW_UT_type)
-	{
-	  this_cu = XOBNEW (&dwarf2_per_objfile->obstack,
-			    struct dwarf2_per_cu_data);
-	  memset (this_cu, 0, sizeof (*this_cu));
-	}
+	this_cu = dwarf2_per_objfile->allocate_per_cu ();
       else
 	{
-	  auto sig_type = XOBNEW (&dwarf2_per_objfile->obstack,
-				  struct signatured_type);
-	  memset (sig_type, 0, sizeof (*sig_type));
+	  auto sig_type = dwarf2_per_objfile->allocate_signatured_type ();
 	  sig_type->signature = cu_header.signature;
 	  sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
 	  this_cu = &sig_type->per_cu;
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 06a0fa3a19a..5fc7f7f72e5 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -128,6 +128,17 @@ struct dwarf2_per_objfile
 
   /* Free all cached compilation units.  */
   void free_cached_comp_units ();
+
+  /* A convenience function to allocate a dwarf2_per_cu_data (or
+     object holding one, using C-style derivation).  The returned
+     object has its "index" field set properly.  The object is
+     allocated on the dwarf2_per_objfile obstack.  FIXME the index
+     here is weird since it may not (!?!?) be the same as the other
+     index  */
+  dwarf2_per_cu_data *allocate_per_cu ();
+
+  signatured_type *allocate_signatured_type ();
+
 private:
   /* This function is mapped across the sections and remembers the
      offset and size of each of the debugging sections we are
@@ -255,6 +266,10 @@ public:
   /* CUs that are queued to be read.  */
   std::queue<dwarf2_queue_item> queue;
 
+  /* The total number of per_cu and signatured_type objects that have
+     been created for this reader.  */
+  size_t num_psymtabs = 0;
+
   /* State that cannot be shared across objfiles.  This is normally
      nullptr and is temporarily set to the correct value at the entry
      points of the reader.  */
@@ -336,6 +351,9 @@ struct dwarf2_per_cu_data
      This flag is only valid if is_debug_types is true.  */
   unsigned int tu_read : 1;
 
+  /* Our index in the unshared "all_cutus" vector.  */
+  unsigned index;
+
   /* The section this CU/TU lives in.
      If the DIE refers to a DWO file, this is always the original die,
      not the DWO file.  */
-- 
2.17.2

Re: [PATCH 07/14] Add dwarf2_per_cu_data::index

[Luis Machado]

On 2/15/20 1:54 PM, Tom Tromey wrote:
> Currently a dwarf2_per_cu_data can hold a link to the corresponding
> compunit_symtab.  However, once these objects are shared across
> objfiles, a simple pointer won't work.
> 
> Instead, this link will be stored in the dwarf2_unshareable object.
> To enable this, we add an index to each dwarf2_per_cu_data and
> signatured_type.
> 
> 2020-02-15  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/read.h (struct dwarf2_per_objfile) <allocate_per_cu,
> 	allocate_signatured_type>: Declare new methods.
> 	(struct dwarf2_per_cu_data) <index>: New member.
> 	(struct dwarf2_per_objfile) <num_psymtabs>: New member.
> 	* dwarf2/read.c (dwarf2_per_objfile::allocate_per_cu)
> 	(dwarf2_per_objfile::allocate_signatured_type): New methods.
> 	(create_cu_from_index_list): Use allocate_per_cu.
> 	(create_signatured_type_table_from_index)
> 	(create_signatured_type_table_from_debug_names)
> 	(create_debug_type_hash_table, add_type_unit)
> 	(read_comp_units_from_section): Use allocate_signatured_type.
> ---
>   gdb/ChangeLog     | 14 +++++++++++++
>   gdb/dwarf2/read.c | 52 +++++++++++++++++++++++++++--------------------
>   gdb/dwarf2/read.h | 18 ++++++++++++++++
>   3 files changed, 62 insertions(+), 22 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index cb04eb19beb..281d39ad271 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -2415,18 +2415,36 @@ dwarf2_per_objfile::get_tu (int index)
>     return this->all_type_units[index];
>   }
>   
> +/* See read.h.  */
> +
> +dwarf2_per_cu_data *
> +dwarf2_per_objfile::allocate_per_cu ()
> +{
> +  dwarf2_per_cu_data *result = OBSTACK_ZALLOC (&obstack, dwarf2_per_cu_data);
> +  result->index = num_psymtabs++;
> +  return result;
> +}
> +
> +/* See read.h.  */
> +
> +signatured_type *
> +dwarf2_per_objfile::allocate_signatured_type ()
> +{
> +  signatured_type *result = OBSTACK_ZALLOC (&obstack, signatured_type);
> +  result->per_cu.index = num_psymtabs++;
> +  return result;
> +}
> +
>   /* Return a new dwarf2_per_cu_data allocated on the dwarf2_per_objfile
>      obstack, and constructed with the specified field values.  */
>   
>   static dwarf2_per_cu_data *
>   create_cu_from_index_list (struct dwarf2_per_objfile *dwarf2_per_objfile,
> -                          struct dwarf2_section_info *section,
> -                          int is_dwz,
> -                          sect_offset sect_off, ULONGEST length)
> +			   struct dwarf2_section_info *section,
> +			   int is_dwz,
> +			   sect_offset sect_off, ULONGEST length)

Did only formatting change above?

>   {
> -  dwarf2_per_cu_data *the_cu
> -    = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
> -                     struct dwarf2_per_cu_data);
> +  dwarf2_per_cu_data *the_cu = dwarf2_per_objfile->allocate_per_cu ();
>     the_cu->sect_off = sect_off;
>     the_cu->length = length;
>     the_cu->dwarf2_per_objfile = dwarf2_per_objfile;
> @@ -2517,8 +2535,7 @@ create_signatured_type_table_from_index
>         signature = extract_unsigned_integer (bytes + 16, 8, BFD_ENDIAN_LITTLE);
>         bytes += 3 * 8;
>   
> -      sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
> -				 struct signatured_type);
> +      sig_type = dwarf2_per_objfile->allocate_signatured_type ();
>         sig_type->signature = signature;
>         sig_type->type_offset_in_tu = type_offset_in_tu;
>         sig_type->per_cu.is_debug_types = 1;
> @@ -2574,8 +2591,7 @@ create_signatured_type_table_from_debug_names
>   				     section->buffer + to_underlying (sect_off),
>   				     rcuh_kind::TYPE);
>   
> -      sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
> -				 struct signatured_type);
> +      sig_type = dwarf2_per_objfile->allocate_signatured_type ();
>         sig_type->signature = cu_header.signature;
>         sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
>         sig_type->per_cu.is_debug_types = 1;
> @@ -6091,8 +6107,7 @@ create_debug_type_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile,
>   	  /* N.B.: type_offset is not usable if this type uses a DWO file.
>   	     The real type_offset is in the DWO file.  */
>   	  dwo_tu = NULL;
> -	  sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
> -				     struct signatured_type);
> +	  sig_type = dwarf2_per_objfile->allocate_signatured_type ();
>   	  sig_type->signature = header.signature;
>   	  sig_type->type_offset_in_tu = header.type_cu_offset_in_tu;
>   	  sig_type->per_cu.dwarf2_per_objfile = dwarf2_per_objfile;
> @@ -6207,8 +6222,7 @@ add_type_unit (struct dwarf2_per_objfile *dwarf2_per_objfile, ULONGEST sig,
>         == dwarf2_per_objfile->all_type_units.capacity ())
>       ++dwarf2_per_objfile->tu_stats.nr_all_type_units_reallocs;
>   
> -  signatured_type *sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
> -					      struct signatured_type);
> +  signatured_type *sig_type = dwarf2_per_objfile->allocate_signatured_type ();
>   
>     dwarf2_per_objfile->all_type_units.push_back (sig_type);
>     sig_type->signature = sig;
> @@ -7835,16 +7849,10 @@ read_comp_units_from_section (struct dwarf2_per_objfile *dwarf2_per_objfile,
>   
>         /* Save the compilation unit for later lookup.  */
>         if (cu_header.unit_type != DW_UT_type)
> -	{
> -	  this_cu = XOBNEW (&dwarf2_per_objfile->obstack,
> -			    struct dwarf2_per_cu_data);
> -	  memset (this_cu, 0, sizeof (*this_cu));
> -	}
> +	this_cu = dwarf2_per_objfile->allocate_per_cu ();
>         else
>   	{
> -	  auto sig_type = XOBNEW (&dwarf2_per_objfile->obstack,
> -				  struct signatured_type);
> -	  memset (sig_type, 0, sizeof (*sig_type));
> +	  auto sig_type = dwarf2_per_objfile->allocate_signatured_type ();
>   	  sig_type->signature = cu_header.signature;
>   	  sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
>   	  this_cu = &sig_type->per_cu;
> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index 06a0fa3a19a..5fc7f7f72e5 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -128,6 +128,17 @@ struct dwarf2_per_objfile
>   
>     /* Free all cached compilation units.  */
>     void free_cached_comp_units ();
> +
> +  /* A convenience function to allocate a dwarf2_per_cu_data (or
> +     object holding one, using C-style derivation).  The returned
> +     object has its "index" field set properly.  The object is
> +     allocated on the dwarf2_per_objfile obstack.  FIXME the index
> +     here is weird since it may not (!?!?) be the same as the other
> +     index  */

I'm a bit lost on the above comment. Instead of having a FIXME here, is 
there something we can do better at this point? Or maybe it will be 
addressed later in the series?

I couldn't quite understand the problems with the indexes, but you're 
more familiar with the DWARF reading code than i am.

> +  dwarf2_per_cu_data *allocate_per_cu ();
> +
> +  signatured_type *allocate_signatured_type ();
> +
>   private:
>     /* This function is mapped across the sections and remembers the
>        offset and size of each of the debugging sections we are
> @@ -255,6 +266,10 @@ public:
>     /* CUs that are queued to be read.  */
>     std::queue<dwarf2_queue_item> queue;
>   
> +  /* The total number of per_cu and signatured_type objects that have
> +     been created for this reader.  */
> +  size_t num_psymtabs = 0;
> +
>     /* State that cannot be shared across objfiles.  This is normally
>        nullptr and is temporarily set to the correct value at the entry
>        points of the reader.  */
> @@ -336,6 +351,9 @@ struct dwarf2_per_cu_data
>        This flag is only valid if is_debug_types is true.  */
>     unsigned int tu_read : 1;
>   
> +  /* Our index in the unshared "all_cutus" vector.  */
> +  unsigned index;
> +

What is the all_cutus vector? I don't see it being added later in the 
series. Should we make it more clear? Maybe you meant "all CU's/TU's"?

>     /* The section this CU/TU lives in.
>        If the DIE refers to a DWO file, this is always the original die,
>        not the DWO file.  */
> 

Re: [PATCH 07/14] Add dwarf2_per_cu_data::index

[Tom Tromey]

>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:

>> static dwarf2_per_cu_data *
>> create_cu_from_index_list (struct dwarf2_per_objfile *dwarf2_per_objfile,
>> -                          struct dwarf2_section_info *section,
>> -                          int is_dwz,
>> -                          sect_offset sect_off, ULONGEST length)
>> +			   struct dwarf2_section_info *section,
>> +			   int is_dwz,
>> +			   sect_offset sect_off, ULONGEST length)

Luis> Did only formatting change above?

Yeah.  I backed this bit out.

>> +  /* A convenience function to allocate a dwarf2_per_cu_data (or
>> +     object holding one, using C-style derivation).  The returned
>> +     object has its "index" field set properly.  The object is
>> +     allocated on the dwarf2_per_objfile obstack.  FIXME the index
>> +     here is weird since it may not (!?!?) be the same as the other
>> +     index  */

Luis> I'm a bit lost on the above comment. Instead of having a FIXME here,
Luis> is there something we can do better at this point? Or maybe it will be 
Luis> addressed later in the series?

Luis> I couldn't quite understand the problems with the indexes, but you're
Luis> more familiar with the DWARF reading code than i am.

I updated the comment, and added a comment by allocate_signatured_type.
Part of that comment referred to some earlier code that also tried to
unify signatured_type and type_unit_group into this allocation function.

The FIXME was more of a note to myself about whether this index could
somehow be reused, because the reader sometimes uses an index to look in
all_comp_units and all_type_units.

I think it turns out they are equivalent, but it seems like it doesn't
matter, because there's no need to use this value to index into the
all_type_units / all_comp_units vectors.

>> +  /* Our index in the unshared "all_cutus" vector.  */
>> +  unsigned index;

Luis> What is the all_cutus vector? I don't see it being added later in the
Luis> series. Should we make it more clear? Maybe you meant "all CU's/TU's"?

I changed it to "symtabs" as Simon pointed out down-thread.

Tom

Re: [PATCH 07/14] Add dwarf2_per_cu_data::index

[Simon Marchi]

On 2020-02-15 11:54 a.m., Tom Tromey wrote:
> @@ -255,6 +266,10 @@ public:
>    /* CUs that are queued to be read.  */
>    std::queue<dwarf2_queue_item> queue;
>  
> +  /* The total number of per_cu and signatured_type objects that have
> +     been created for this reader.  */
> +  size_t num_psymtabs = 0;

Make this private, and name it m_num_psymtabs?

> +
>    /* State that cannot be shared across objfiles.  This is normally
>       nullptr and is temporarily set to the correct value at the entry
>       points of the reader.  */
> @@ -336,6 +351,9 @@ struct dwarf2_per_cu_data
>       This flag is only valid if is_debug_types is true.  */
>    unsigned int tu_read : 1;
>  
> +  /* Our index in the unshared "all_cutus" vector.  */
> +  unsigned index;

The "all_cutus" vector does not exist (at least at the moment, maybe it's added later
in the series?).  I presume you are talking about the logical/combined vector made of
all_comp_units and all_type_units, accessible using dwarf2_per_objfile::get_cutu?

This logical/combined vector has all the comp units first, then the type units.  Using
the new interface allocate_per_cu/allocate_signatured_type, the indices are handed out
in any order.  I don't know if it's actually possible to have it in this order, but let's
say we read:

- a CU
- a TU
- a CU

The CUs will have been handed out indices 0 and 2, and the TU will have been handed out
index 1.  But in reality, when accessing them using get_cutu, the two CUs will be at 0 and
1, while the TU will be at 2.  Again, not sure if it matters, but I thought I would point
it out.

Or maybe this is what you meant by "the index here is weird since it may not (!?!?) be the
same as the other index", this is not the same index?  I guess I'll read the rest of the
series and find out.

Simon

Re: [PATCH 07/14] Add dwarf2_per_cu_data::index

[Simon Marchi]

On 2020-02-18 11:36 p.m., Simon Marchi wrote:
> The "all_cutus" vector does not exist (at least at the moment, maybe it's added later
> in the series?).  I presume you are talking about the logical/combined vector made of
> all_comp_units and all_type_units, accessible using dwarf2_per_objfile::get_cutu?
> 
> This logical/combined vector has all the comp units first, then the type units.  Using
> the new interface allocate_per_cu/allocate_signatured_type, the indices are handed out
> in any order.  I don't know if it's actually possible to have it in this order, but let's
> say we read:
> 
> - a CU
> - a TU
> - a CU
> 
> The CUs will have been handed out indices 0 and 2, and the TU will have been handed out
> index 1.  But in reality, when accessing them using get_cutu, the two CUs will be at 0 and
> 1, while the TU will be at 2.  Again, not sure if it matters, but I thought I would point
> it out.
> 
> Or maybe this is what you meant by "the index here is weird since it may not (!?!?) be the
> same as the other index", this is not the same index?  I guess I'll read the rest of the
> series and find out.
> 
> Simon

Ok, after reading the next patch I saw that this is indeed referring to an index into another
vector.  So I suppose it's just that the "all_cutus" reference is stale, it should say "symtabs"?

Re: [PATCH 07/14] Add dwarf2_per_cu_data::index

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Ok, after reading the next patch I saw that this is indeed
Simon> referring to an index into another vector.  So I suppose it's
Simon> just that the "all_cutus" reference is stale, it should say
Simon> "symtabs"?

Yep.  I've updated the comment.

Tom

Re: [PATCH 07/14] Add dwarf2_per_cu_data::index

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> +  size_t num_psymtabs = 0;

Simon> Make this private, and name it m_num_psymtabs?

Good idea, I did this.

Tom

[PATCH 12/14] Fix a memory leak and remove an unused member

[Tom Tromey]

I noticed that setup_type_unit_groups leaks the symtab vector -- it
allocates this with XNEWVEC, but from what I can tell, nothing frees
it.  This patch changes it to use XOBNEWVEC.

Also, the type_unit_unshareable::num_symtabs member is assigned but
never read.  So, this removes it.

2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.h (struct type_unit_unshareable) <num_symtabs>:
	Remove.
	* dwarf2/read.c (dwarf2_cu::setup_type_unit_groups): Use
	XOBNEWVEC.
---
 gdb/ChangeLog     | 7 +++++++
 gdb/dwarf2/read.c | 7 ++++---
 gdb/dwarf2/read.h | 4 ----
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d1c5bee1109..765fc4e8272 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -10973,9 +10973,10 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
 	 process_full_type_unit still needs to know if this is the first
 	 time.  */
 
-      tu_unshare->num_symtabs = line_header->file_names_size ();
-      tu_unshare->symtabs = XNEWVEC (struct symtab *,
-				     line_header->file_names_size ());
+      tu_unshare->symtabs
+	= XOBNEWVEC (&COMPUNIT_OBJFILE (cust)->objfile_obstack,
+		     struct symtab *,
+		     line_header->file_names_size ());
 
       auto &file_names = line_header->file_names ();
       for (i = 0; i < file_names.size (); ++i)
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 8b2e8703aa0..18f3c264a0a 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -76,10 +76,6 @@ struct type_unit_unshareable
      so we create an essentially anonymous symtab as the compunit symtab.  */
   struct compunit_symtab *compunit_symtab = nullptr;
 
-  /* The number of symtabs from the line header.
-     The value here must match line_header.num_file_names.  */
-  unsigned int num_symtabs = 0;
-
   /* The symbol tables for this TU (obtained from the files listed in
      DW_AT_stmt_list).
      WARNING: The order of entries here must match the order of entries
-- 
2.17.2

[PATCH 05/14] Introduce dwarf2_unshareable and move die_type_hash

[Tom Tromey]

The goal of this series is to share dwarf2_per_objfile across
objfiles.  However, this is difficult to do, because it has some
members that are inherently per-objfile.

This patch introduces a new "unshareable" object.  The idea here is
that we'll have one such object per objfile, and we'll temporarily
update the pointer when needed.  This is a bit ugly, but it's
convenient, because it doesn't require a huge rewriting of the DWARF
code.  (I would be in favor of such a rewriting, but ... it's huge.)

This patch further moves one such member, the DIE type hash, to the
new structure.

In this patch, the new object is managed by the dwarf2_per_objfile.
However, by the end of the series, this will change.

gdb/ChangeLog
2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.h (struct dwarf2_unshareable): New type.
	(struct dwarf2_per_objfile) <die_type_hash>: Move to
	dwarf2_unshareable.
	<unshareable>: New member.
	* dwarf2/read.c (dwarf2_per_objfile::dwarf2_per_objfile):
	Initialize "unshareable".
	(dwarf2_per_objfile::~dwarf2_per_objfile): Delete unshareable.
	(set_die_type, get_die_type_at_offset): Update.
---
 gdb/ChangeLog     | 11 +++++++++++
 gdb/dwarf2/read.c | 19 +++++++++++++------
 gdb/dwarf2/read.h | 28 +++++++++++++++++++++++-----
 3 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index a897b11693a..cb04eb19beb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1774,7 +1774,10 @@ dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
 					const dwarf2_debug_sections *names,
 					bool can_copy_)
   : objfile (objfile_),
-    can_copy (can_copy_)
+    can_copy (can_copy_),
+    /* Temporarily just allocate one per objfile -- we don't have
+       sharing yet.  */
+    unshareable (new dwarf2_unshareable)
 {
   if (names == NULL)
     names = &dwarf2_elf_names;
@@ -1796,6 +1799,8 @@ dwarf2_per_objfile::~dwarf2_per_objfile ()
   for (signatured_type *sig_type : all_type_units)
     sig_type->per_cu.imported_symtabs_free ();
 
+  delete unshareable;
+
   /* Everything else should be on the objfile obstack.  */
 }
 
@@ -24434,8 +24439,8 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
 			    cu->per_cu->addr_type ()))
     add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type);
 
-  if (dwarf2_per_objfile->die_type_hash == NULL)
-    dwarf2_per_objfile->die_type_hash
+  if (dwarf2_per_objfile->unshareable->die_type_hash == NULL)
+    dwarf2_per_objfile->unshareable->die_type_hash
       = htab_up (htab_create_alloc (127,
 				    per_cu_offset_and_type_hash,
 				    per_cu_offset_and_type_eq,
@@ -24445,7 +24450,8 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   ofs.sect_off = die->sect_off;
   ofs.type = type;
   slot = (struct dwarf2_per_cu_offset_and_type **)
-    htab_find_slot (dwarf2_per_objfile->die_type_hash.get (), &ofs, INSERT);
+    htab_find_slot (dwarf2_per_objfile->unshareable->die_type_hash.get (),
+		    &ofs, INSERT);
   if (*slot)
     complaint (_("A problem internal to GDB: DIE %s has type already set"),
 	       sect_offset_str (die->sect_off));
@@ -24465,13 +24471,14 @@ get_die_type_at_offset (sect_offset sect_off,
   struct dwarf2_per_cu_offset_and_type *slot, ofs;
   struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;
 
-  if (dwarf2_per_objfile->die_type_hash == NULL)
+  if (dwarf2_per_objfile->unshareable->die_type_hash == NULL)
     return NULL;
 
   ofs.per_cu = per_cu;
   ofs.sect_off = sect_off;
   slot = ((struct dwarf2_per_cu_offset_and_type *)
-	  htab_find (dwarf2_per_objfile->die_type_hash.get (), &ofs));
+	  htab_find (dwarf2_per_objfile->unshareable->die_type_hash.get (),
+		     &ofs));
   if (slot)
     return slot->type;
   else
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 5fa8dec9bfb..06a0fa3a19a 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -66,6 +66,24 @@ struct dwarf2_queue_item
   enum language pretend_language;
 };
 
+/* Some DWARF data cannot (currently) be shared across objfiles.  Such
+   data is stored in this object.
+
+   dwarf2_per_objfile holds a pointer to an instance of this object.
+   This pointer is temporarily set when expanding CUs.  This hackish
+   approach is due to the history of the DWARF reader.  In the past,
+   all objects were stored per-objfile, and this object was introduced
+   in the process of separating out the shareable and per-objfile
+   state.  */
+
+struct dwarf2_unshareable
+{
+  /* Table mapping type DIEs to their struct type *.
+     This is NULL if not allocated yet.
+     The mapping is done via (CU/TU + DIE offset) -> type.  */
+  htab_up die_type_hash;
+};
+
 /* Collection of data recorded per objfile.
    This hangs off of dwarf2_objfile_data_key.  */
 
@@ -214,11 +232,6 @@ public:
      symbols.  */
   bool reading_partial_symbols = false;
 
-  /* Table mapping type DIEs to their struct type *.
-     This is NULL if not allocated yet.
-     The mapping is done via (CU/TU + DIE offset) -> type.  */
-  htab_up die_type_hash;
-
   /* The CUs we recently read.  */
   std::vector<dwarf2_per_cu_data *> just_read_cus;
 
@@ -241,6 +254,11 @@ public:
 
   /* CUs that are queued to be read.  */
   std::queue<dwarf2_queue_item> queue;
+
+  /* State that cannot be shared across objfiles.  This is normally
+     nullptr and is temporarily set to the correct value at the entry
+     points of the reader.  */
+  dwarf2_unshareable *unshareable = nullptr;
 };
 
 /* Get the dwarf2_per_objfile associated to OBJFILE.  */
-- 
2.17.2

Re: [PATCH 05/14] Introduce dwarf2_unshareable and move die_type_hash

[Luis Machado]

Just one nit...

On 2/15/20 1:54 PM, Tom Tromey wrote:
> The goal of this series is to share dwarf2_per_objfile across
> objfiles.  However, this is difficult to do, because it has some
> members that are inherently per-objfile.
> 
> This patch introduces a new "unshareable" object.  The idea here is
> that we'll have one such object per objfile, and we'll temporarily
> update the pointer when needed.  This is a bit ugly, but it's
> convenient, because it doesn't require a huge rewriting of the DWARF
> code.  (I would be in favor of such a rewriting, but ... it's huge.)
> 
> This patch further moves one such member, the DIE type hash, to the
> new structure.
> 
> In this patch, the new object is managed by the dwarf2_per_objfile.
> However, by the end of the series, this will change.
> 
> gdb/ChangeLog
> 2020-02-15  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/read.h (struct dwarf2_unshareable): New type.
> 	(struct dwarf2_per_objfile) <die_type_hash>: Move to
> 	dwarf2_unshareable.
> 	<unshareable>: New member.
> 	* dwarf2/read.c (dwarf2_per_objfile::dwarf2_per_objfile):
> 	Initialize "unshareable".
> 	(dwarf2_per_objfile::~dwarf2_per_objfile): Delete unshareable.
> 	(set_die_type, get_die_type_at_offset): Update.
> ---
>   gdb/ChangeLog     | 11 +++++++++++
>   gdb/dwarf2/read.c | 19 +++++++++++++------
>   gdb/dwarf2/read.h | 28 +++++++++++++++++++++++-----
>   3 files changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index a897b11693a..cb04eb19beb 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -1774,7 +1774,10 @@ dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
>   					const dwarf2_debug_sections *names,
>   					bool can_copy_)
>     : objfile (objfile_),
> -    can_copy (can_copy_)
> +    can_copy (can_copy_),
> +    /* Temporarily just allocate one per objfile -- we don't have
> +       sharing yet.  */
> +    unshareable (new dwarf2_unshareable)
>   {
>     if (names == NULL)
>       names = &dwarf2_elf_names;
> @@ -1796,6 +1799,8 @@ dwarf2_per_objfile::~dwarf2_per_objfile ()
>     for (signatured_type *sig_type : all_type_units)
>       sig_type->per_cu.imported_symtabs_free ();
>   
> +  delete unshareable;
> +
>     /* Everything else should be on the objfile obstack.  */
>   }
>   
> @@ -24434,8 +24439,8 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>   			    cu->per_cu->addr_type ()))
>       add_dyn_prop (DYN_PROP_DATA_LOCATION, prop, type);
>   
> -  if (dwarf2_per_objfile->die_type_hash == NULL)
> -    dwarf2_per_objfile->die_type_hash
> +  if (dwarf2_per_objfile->unshareable->die_type_hash == NULL)
> +    dwarf2_per_objfile->unshareable->die_type_hash
>         = htab_up (htab_create_alloc (127,
>   				    per_cu_offset_and_type_hash,
>   				    per_cu_offset_and_type_eq,
> @@ -24445,7 +24450,8 @@ set_die_type (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
>     ofs.sect_off = die->sect_off;
>     ofs.type = type;
>     slot = (struct dwarf2_per_cu_offset_and_type **)
> -    htab_find_slot (dwarf2_per_objfile->die_type_hash.get (), &ofs, INSERT);
> +    htab_find_slot (dwarf2_per_objfile->unshareable->die_type_hash.get (),
> +		    &ofs, INSERT);
>     if (*slot)
>       complaint (_("A problem internal to GDB: DIE %s has type already set"),
>   	       sect_offset_str (die->sect_off));
> @@ -24465,13 +24471,14 @@ get_die_type_at_offset (sect_offset sect_off,
>     struct dwarf2_per_cu_offset_and_type *slot, ofs;
>     struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;
>   
> -  if (dwarf2_per_objfile->die_type_hash == NULL)
> +  if (dwarf2_per_objfile->unshareable->die_type_hash == NULL)
>       return NULL;
>   
>     ofs.per_cu = per_cu;
>     ofs.sect_off = sect_off;
>     slot = ((struct dwarf2_per_cu_offset_and_type *)
> -	  htab_find (dwarf2_per_objfile->die_type_hash.get (), &ofs));
> +	  htab_find (dwarf2_per_objfile->unshareable->die_type_hash.get (),
> +		     &ofs));
>     if (slot)
>       return slot->type;
>     else
> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index 5fa8dec9bfb..06a0fa3a19a 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -66,6 +66,24 @@ struct dwarf2_queue_item
>     enum language pretend_language;
>   };
>   
> +/* Some DWARF data cannot (currently) be shared across objfiles.  Such
> +   data is stored in this object.
> +
> +   dwarf2_per_objfile holds a pointer to an instance of this object.
> +   This pointer is temporarily set when expanding CUs.  This hackish
> +   approach is due to the history of the DWARF reader.  In the past,
> +   all objects were stored per-objfile, and this object was introduced
> +   in the process of separating out the shareable and per-objfile
> +   state.  */
> +
> +struct dwarf2_unshareable
> +{
> +  /* Table mapping type DIEs to their struct type *.
> +     This is NULL if not allocated yet.

"This is nullptr"

> +     The mapping is done via (CU/TU + DIE offset) -> type.  */
> +  htab_up die_type_hash;
> +};
> +
>   /* Collection of data recorded per objfile.
>      This hangs off of dwarf2_objfile_data_key.  */
>   
> @@ -214,11 +232,6 @@ public:
>        symbols.  */
>     bool reading_partial_symbols = false;
>   
> -  /* Table mapping type DIEs to their struct type *.
> -     This is NULL if not allocated yet.
> -     The mapping is done via (CU/TU + DIE offset) -> type.  */
> -  htab_up die_type_hash;
> -
>     /* The CUs we recently read.  */
>     std::vector<dwarf2_per_cu_data *> just_read_cus;
>   
> @@ -241,6 +254,11 @@ public:
>   
>     /* CUs that are queued to be read.  */
>     std::queue<dwarf2_queue_item> queue;
> +
> +  /* State that cannot be shared across objfiles.  This is normally
> +     nullptr and is temporarily set to the correct value at the entry
> +     points of the reader.  */
> +  dwarf2_unshareable *unshareable = nullptr;
>   };
>   
>   /* Get the dwarf2_per_objfile associated to OBJFILE.  */
> 

Re: [PATCH 05/14] Introduce dwarf2_unshareable and move die_type_hash

[Simon Marchi]

On 2020-02-15 11:54 a.m., Tom Tromey wrote:
> The goal of this series is to share dwarf2_per_objfile across
> objfiles.  However, this is difficult to do, because it has some
> members that are inherently per-objfile.
> 
> This patch introduces a new "unshareable" object.  The idea here is
> that we'll have one such object per objfile, and we'll temporarily
> update the pointer when needed.  This is a bit ugly, but it's
> convenient, because it doesn't require a huge rewriting of the DWARF
> code.  (I would be in favor of such a rewriting, but ... it's huge.)
> 
> This patch further moves one such member, the DIE type hash, to the
> new structure.
> 
> In this patch, the new object is managed by the dwarf2_per_objfile.
> However, by the end of the series, this will change.

So, in the end we'll have dwarf2_per_objfile, of which there will be only
one and will be shared between the objfiles, and dwarf2_unshareable of which
there will be N (for N objfiles), which won't be shared between the objfiles?

I find it a bit confusing, because now the object called "per_objfile" is not
per objfile.

Simon

Re: [PATCH 05/14] Introduce dwarf2_unshareable and move die_type_hash

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

We discussed this online but I thought I'd reply for the record.

>> In this patch, the new object is managed by the dwarf2_per_objfile.
>> However, by the end of the series, this will change.

Simon> So, in the end we'll have dwarf2_per_objfile, of which there will be only
Simon> one and will be shared between the objfiles, and dwarf2_unshareable of which
Simon> there will be N (for N objfiles), which won't be shared between the objfiles?

Yes.

Simon> I find it a bit confusing, because now the object called "per_objfile" is not
Simon> per objfile.

Yeah.  In the cover letter I mentioned perhaps renaming this object.
That could be done with sed I suppose.

Tom

[PATCH 01/14] Fix latent bug in dwarf2_find_containing_comp_unit

[Tom Tromey]

dwarf2_find_containing_comp_unit has this in its binary search:

      if (mid_cu->is_dwz > offset_in_dwz
	  || (mid_cu->is_dwz == offset_in_dwz
	      && mid_cu->sect_off + mid_cu->length >= sect_off))
	high = mid;

The intent here is to determine whether SECT_OFF appears in or before
MID_CU.

I believe this has an off-by-one error, and that the check should use
">" rather than ">=".  If the two side are equal, then SECT_OFF
actually appears at the start of the next CU.

I've had this patch kicking around for ages but I forget how I found
the problem.

gdb/ChangeLog
2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (dwarf2_find_containing_comp_unit): Use ">", not
	">=", in binary search.
---
 gdb/ChangeLog     | 5 +++++
 gdb/dwarf2/read.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index e74383e01dc..e373cc0af2c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -24171,7 +24171,7 @@ dwarf2_find_containing_comp_unit (sect_offset sect_off,
       mid_cu = dwarf2_per_objfile->all_comp_units[mid];
       if (mid_cu->is_dwz > offset_in_dwz
 	  || (mid_cu->is_dwz == offset_in_dwz
-	      && mid_cu->sect_off + mid_cu->length >= sect_off))
+	      && mid_cu->sect_off + mid_cu->length > sect_off))
 	high = mid;
       else
 	low = mid + 1;
-- 
2.17.2

Re: [PATCH 01/14] Fix latent bug in dwarf2_find_containing_comp_unit

[Simon Marchi]

On 2020-02-15 11:54 a.m., Tom Tromey wrote:
> dwarf2_find_containing_comp_unit has this in its binary search:
> 
>       if (mid_cu->is_dwz > offset_in_dwz
> 	  || (mid_cu->is_dwz == offset_in_dwz
> 	      && mid_cu->sect_off + mid_cu->length >= sect_off))
> 	high = mid;
> 
> The intent here is to determine whether SECT_OFF appears in or before
> MID_CU.
> 
> I believe this has an off-by-one error, and that the check should use
> ">" rather than ">=".  If the two side are equal, then SECT_OFF
> actually appears at the start of the next CU.
> 
> I've had this patch kicking around for ages but I forget how I found
> the problem.
> 
> gdb/ChangeLog
> 2020-02-15  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/read.c (dwarf2_find_containing_comp_unit): Use ">", not
> 	">=", in binary search.
> ---
>  gdb/ChangeLog     | 5 +++++
>  gdb/dwarf2/read.c | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index e74383e01dc..e373cc0af2c 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -24171,7 +24171,7 @@ dwarf2_find_containing_comp_unit (sect_offset sect_off,
>        mid_cu = dwarf2_per_objfile->all_comp_units[mid];
>        if (mid_cu->is_dwz > offset_in_dwz
>  	  || (mid_cu->is_dwz == offset_in_dwz
> -	      && mid_cu->sect_off + mid_cu->length >= sect_off))
> +	      && mid_cu->sect_off + mid_cu->length > sect_off))
>  	high = mid;
>        else
>  	low = mid + 1;

I can only convince myself of these things by plugging in real numbers.  So let's say
mid_cu's range is [100,150[, so 150 bytes long, and we are looking for sect_off == 150.
150 is outside (just after) mid_cu's range, so the right answer will be the cu just
after this one.

Without your change, we would have gone in the "if", and therefore excluded the right
answer.  With your change, we would have gone in the "else", and therefore chosen the
range with the right answer.  So that looks correct to me.

I'm going to ask if you thought about a way to test this.  I don't think writing a typical
test case for this is feasible.  By refactoring the code a bit, we could maybe factor out the
meat of this function into one that operates on an std::vector<dwarf2_per_cu_data> instead
of on a dwarf2_per_objfile.  It should then be feasible to create an std::vector with
dwarf2_per_cu_data elements out of thin air to unit-test the function, including edge
cases like this.

Also, do you understand what's the logic with this dwz stuff?  Is it that all the CUs
coming from a dwz file are at the end of the list, or something like that?

Simon

Re: [PATCH 01/14] Fix latent bug in dwarf2_find_containing_comp_unit

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I'm going to ask if you thought about a way to test this.  I
Simon> don't think writing a typical test case for this is feasible.

Yeah.  And like I said, I don't remember how I encountered this.
Maybe only with some other dubious patch in place.

Simon>  By refactoring the code a bit, we could maybe factor out the
Simon> meat of this function into one that operates on an std::vector<dwarf2_per_cu_data> instead
Simon> of on a dwarf2_per_objfile.  It should then be feasible to create an std::vector with
Simon> dwarf2_per_cu_data elements out of thin air to unit-test the function, including edge
Simon> cases like this.

Another idea that occurred to me is that we could just use
std::binary_search here.  Then instead of implementing a binary search,
we'd only need to implement a comparison function -- which seems
simpler.

Anyway I will try to write a unit test, that's a good idea.

Simon> Also, do you understand what's the logic with this dwz stuff?  Is it that all the CUs
Simon> coming from a dwz file are at the end of the list, or something like that?

Yes, exactly.  Maybe this can be cleaned up a bit after this series,
since now we have the index directly in the dwarf2_per_cu_data.  I am
not sure offhand.

Tom

Re: [PATCH 01/14] Fix latent bug in dwarf2_find_containing_comp_unit

[Tom Tromey]

Simon> By refactoring the code a bit, we could maybe factor out the meat
Simon> of this function into one that operates on an
Simon> std::vector<dwarf2_per_cu_data> instead of on a
Simon> dwarf2_per_objfile.  It should then be feasible to create an
Simon> std::vector with dwarf2_per_cu_data elements out of thin air to
Simon> unit-test the function, including edge cases like this.

Here is a new version of this patch.
I propose landing it separately, because it is just a straightforward
latent bug fix.

Simon> Also, do you understand what's the logic with this dwz stuff?  Is it that all the CUs
Simon> coming from a dwz file are at the end of the list, or something like that?

Tom> Yes, exactly.

To expand on that, in multi-file mode, dwz may create a combined, shared
file of partial symtabs.  When reading the DWARF for the main file, gdb
may find it needs the dwz file; and in this case the additional CUs are
put into the same vector, at the end.  It's done this way so that we
can easily find the correct CU (since the section offsets between the
main and shared dwz file will overlap).

It would probably be better to try to share these CUs across objfiles.
That may be somewhat involved given the current code, though.

Tom

Re: [PATCH 01/14] Fix latent bug in dwarf2_find_containing_comp_unit

[Tom Tromey]

Simon> By refactoring the code a bit, we could maybe factor out the
Simon> meat of this function into one that operates on an std::vector<dwarf2_per_cu_data> instead
Simon> of on a dwarf2_per_objfile.  It should then be feasible to create an std::vector with
Simon> dwarf2_per_cu_data elements out of thin air to unit-test the function, including edge
Simon> cases like this.

Oops, I meant to attach the patch in that last email.

Let me know what you think.

Tom

commit 5d02b8ad013451b03e004ef1e71c4636b43252c2
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Feb 15 09:08:09 2020 -0700

    Fix latent bug in dwarf2_find_containing_comp_unit
    
    dwarf2_find_containing_comp_unit has this in its binary search:
    
          if (mid_cu->is_dwz > offset_in_dwz
              || (mid_cu->is_dwz == offset_in_dwz
                  && mid_cu->sect_off + mid_cu->length >= sect_off))
            high = mid;
    
    The intent here is to determine whether SECT_OFF appears in or before
    MID_CU.
    
    I believe this has an off-by-one error, and that the check should use
    ">" rather than ">=".  If the two side are equal, then SECT_OFF
    actually appears at the start of the next CU.
    
    I've had this patch kicking around for ages but I forget how I found
    the problem.
    
    gdb/ChangeLog
    2020-02-19  Tom Tromey  <tom@tromey.com>
    
            * dwarf2/read.c (dwarf2_find_containing_comp_unit): Use ">", not
            ">=", in binary search.
            (dwarf2_find_containing_comp_unit): New overload.
            (run_test): New self-test.
            (_initialize_dwarf2_read): Register new test.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b4a586c333a..b07c9156e28 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-19  Tom Tromey  <tom@tromey.com>
+
+	* dwarf2/read.c (dwarf2_find_containing_comp_unit): Use ">", not
+	">=", in binary search.
+	(dwarf2_find_containing_comp_unit): New overload.
+	(run_test): New self-test.
+	(_initialize_dwarf2_read): Register new test.
+
 2020-02-19  Simon Marchi  <simon.marchi@efficios.com>
 
 	* dwarf2/read.c (allocate_signatured_type_table,
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4d767a59af7..f998fe6b8d0 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -24136,34 +24136,53 @@ dwarf2_per_cu_data::addr_type () const
   return addr_type;
 }
 
-/* Locate the .debug_info compilation unit from CU's objfile which contains
-   the DIE at OFFSET.  Raises an error on failure.  */
+/* A helper function for dwarf2_find_containing_comp_unit that returns
+   the index of the result, and that searches a vector.  It will
+   return a result even if the offset in question does not actually
+   occur in any CU.  This is separate so that it can be unit
+   tested.  */
 
-static struct dwarf2_per_cu_data *
-dwarf2_find_containing_comp_unit (sect_offset sect_off,
-				  unsigned int offset_in_dwz,
-				  struct dwarf2_per_objfile *dwarf2_per_objfile)
+static int
+dwarf2_find_containing_comp_unit
+  (sect_offset sect_off,
+   unsigned int offset_in_dwz,
+   const std::vector<dwarf2_per_cu_data *> &all_comp_units)
 {
-  struct dwarf2_per_cu_data *this_cu;
   int low, high;
 
   low = 0;
-  high = dwarf2_per_objfile->all_comp_units.size () - 1;
+  high = all_comp_units.size () - 1;
   while (high > low)
     {
       struct dwarf2_per_cu_data *mid_cu;
       int mid = low + (high - low) / 2;
 
-      mid_cu = dwarf2_per_objfile->all_comp_units[mid];
+      mid_cu = all_comp_units[mid];
       if (mid_cu->is_dwz > offset_in_dwz
 	  || (mid_cu->is_dwz == offset_in_dwz
-	      && mid_cu->sect_off + mid_cu->length >= sect_off))
+	      && mid_cu->sect_off + mid_cu->length > sect_off))
 	high = mid;
       else
 	low = mid + 1;
     }
   gdb_assert (low == high);
-  this_cu = dwarf2_per_objfile->all_comp_units[low];
+  return low;
+}
+
+/* Locate the .debug_info compilation unit from CU's objfile which contains
+   the DIE at OFFSET.  Raises an error on failure.  */
+
+static struct dwarf2_per_cu_data *
+dwarf2_find_containing_comp_unit (sect_offset sect_off,
+				  unsigned int offset_in_dwz,
+				  struct dwarf2_per_objfile *dwarf2_per_objfile)
+{
+  int low
+    = dwarf2_find_containing_comp_unit (sect_off, offset_in_dwz,
+					dwarf2_per_objfile->all_comp_units);
+  struct dwarf2_per_cu_data *this_cu
+    = dwarf2_per_objfile->all_comp_units[low];
+
   if (this_cu->is_dwz != offset_in_dwz || this_cu->sect_off > sect_off)
     {
       if (low == 0 || this_cu->is_dwz != offset_in_dwz)
@@ -24186,6 +24205,57 @@ dwarf2_find_containing_comp_unit (sect_offset sect_off,
     }
 }
 
+#if GDB_SELF_TEST
+
+namespace selftests {
+namespace find_containing_comp_unit {
+
+static void
+run_test ()
+{
+  struct dwarf2_per_cu_data one {};
+  struct dwarf2_per_cu_data two {};
+  struct dwarf2_per_cu_data three {};
+  struct dwarf2_per_cu_data four {};
+
+  one.length = 5;
+  two.sect_off = sect_offset (one.length);
+  two.length = 7;
+
+  three.length = 5;
+  three.is_dwz = 1;
+  four.sect_off = sect_offset (three.length);
+  four.length = 7;
+  four.is_dwz = 1;
+
+  std::vector<dwarf2_per_cu_data *> units;
+  units.push_back (&one);
+  units.push_back (&two);
+  units.push_back (&three);
+  units.push_back (&four);
+
+  int result;
+
+  result = dwarf2_find_containing_comp_unit (sect_offset (0), 0, units);
+  SELF_CHECK (units[result] == &one);
+  result = dwarf2_find_containing_comp_unit (sect_offset (3), 0, units);
+  SELF_CHECK (units[result] == &one);
+  result = dwarf2_find_containing_comp_unit (sect_offset (5), 0, units);
+  SELF_CHECK (units[result] == &two);
+
+  result = dwarf2_find_containing_comp_unit (sect_offset (0), 1, units);
+  SELF_CHECK (units[result] == &three);
+  result = dwarf2_find_containing_comp_unit (sect_offset (3), 1, units);
+  SELF_CHECK (units[result] == &three);
+  result = dwarf2_find_containing_comp_unit (sect_offset (5), 1, units);
+  SELF_CHECK (units[result] == &four);
+}
+
+}
+}
+
+#endif /* GDB_SELF_TEST */
+
 /* Initialize dwarf2_cu CU, owned by PER_CU.  */
 
 dwarf2_cu::dwarf2_cu (struct dwarf2_per_cu_data *per_cu_)
@@ -24690,5 +24760,7 @@ Warning: This option must be enabled before gdb reads the file."),
 #if GDB_SELF_TEST
   selftests::register_test ("dw2_expand_symtabs_matching",
 			    selftests::dw2_expand_symtabs_matching::run_test);
+  selftests::register_test ("dwarf2_find_containing_comp_unit",
+			    selftests::find_containing_comp_unit::run_test);
 #endif
 }

Re: [PATCH 01/14] Fix latent bug in dwarf2_find_containing_comp_unit

[Simon Marchi]

On 2020-02-19 7:12 p.m., Tom Tromey wrote:
> Simon> By refactoring the code a bit, we could maybe factor out the
> Simon> meat of this function into one that operates on an std::vector<dwarf2_per_cu_data> instead
> Simon> of on a dwarf2_per_objfile.  It should then be feasible to create an std::vector with
> Simon> dwarf2_per_cu_data elements out of thin air to unit-test the function, including edge
> Simon> cases like this.
> 
> Oops, I meant to attach the patch in that last email.
> 
> Let me know what you think.
> 
> Tom

Thanks, that LGTM.  If you want to use std::binary_search, that would be fine
as well.

Simon

Re: [PATCH 01/14] Fix latent bug in dwarf2_find_containing_comp_unit

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Thanks, that LGTM.  If you want to use std::binary_search, that would be fine
Simon> as well.

I tried this but it wasn't notably simpler and it meant changes to the
error handling, so I dropped it.

Tom

Re: [PATCH 01/14] Fix latent bug in dwarf2_find_containing_comp_unit

[Christian Biesinger]

Re std::binary_search, we do have GDB::binary_search which is more
useful/easier to use.

On Thu, Feb 20, 2020, 10:50 Tom Tromey <tom@tromey.com> wrote:

> >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>
> Simon> Thanks, that LGTM.  If you want to use std::binary_search, that
> would be fine
> Simon> as well.
>
> I tried this but it wasn't notably simpler and it meant changes to the
> error handling, so I dropped it.
>
> Tom
>

[PATCH 04/14] Convert IS_TYPE_UNIT_GROUP to method

[Tom Tromey]

This converts the IS_TYPE_UNIT_GROUP to a method on
dwarf2_per_cu_data.

gdb/ChangeLog
2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.h (struct dwarf2_per_cu_data) <type_unit_group_p>:
	New method.
	* dwarf2/read.c (IS_TYPE_UNIT_GROUP): Remove.
	(dw2_do_instantiate_symtab, dw2_get_file_names)
	(build_type_psymtab_dependencies, load_full_type_unit): Update.
---
 gdb/ChangeLog     |  8 ++++++++
 gdb/dwarf2/read.c | 12 +++++-------
 gdb/dwarf2/read.h |  7 +++++++
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d3060be630d..a897b11693a 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -572,9 +572,7 @@ struct type_unit_group
   /* dwarf2read.c's main "handle" on a TU symtab.
      To simplify things we create an artificial CU that "includes" all the
      type units using this stmt_list so that the rest of the code still has
-     a "per_cu" handle on the symtab.
-     This PER_CU is recognized by having no section.  */
-#define IS_TYPE_UNIT_GROUP(per_cu) ((per_cu)->section == NULL)
+     a "per_cu" handle on the symtab.  */
   struct dwarf2_per_cu_data per_cu;
 
   /* The TUs that share this DW_AT_stmt_list entry.
@@ -2321,7 +2319,7 @@ dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
 
   /* Skip type_unit_groups, reading the type units they contain
      is handled elsewhere.  */
-  if (IS_TYPE_UNIT_GROUP (per_cu))
+  if (per_cu->type_unit_group_p ())
     return;
 
   /* The destructor of dwarf2_queue_guard frees any entries left on
@@ -3175,7 +3173,7 @@ dw2_get_file_names (struct dwarf2_per_cu_data *this_cu)
   /* This should never be called for TUs.  */
   gdb_assert (! this_cu->is_debug_types);
   /* Nor type unit groups.  */
-  gdb_assert (! IS_TYPE_UNIT_GROUP (this_cu));
+  gdb_assert (! this_cu->type_unit_group_p ());
 
   if (this_cu->v.quick->file_names != NULL)
     return this_cu->v.quick->file_names;
@@ -7589,7 +7587,7 @@ build_type_psymtab_dependencies (void **slot, void *info)
   int i;
 
   gdb_assert (len > 0);
-  gdb_assert (IS_TYPE_UNIT_GROUP (per_cu));
+  gdb_assert (per_cu->type_unit_group_p ());
 
   pst->number_of_dependencies = len;
   pst->dependencies = objfile->partial_symtabs->allocate_dependencies (len);
@@ -22726,7 +22724,7 @@ load_full_type_unit (struct dwarf2_per_cu_data *per_cu)
   struct signatured_type *sig_type;
 
   /* Caller is responsible for ensuring type_unit_groups don't get here.  */
-  gdb_assert (! IS_TYPE_UNIT_GROUP (per_cu));
+  gdb_assert (! per_cu->type_unit_group_p ());
 
   /* We have the per_cu, but we need the signatured_type.
      Fortunately this is an easy translation.  */
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index a7ac3b6b025..5fa8dec9bfb 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -442,6 +442,13 @@ struct dwarf2_per_cu_data
   {
     return dwarf_version;
   }
+
+  /* A type unit group has a per_cu object that is recognized by
+     having no section.  */
+  bool type_unit_group_p () const
+  {
+    return section == nullptr;
+  }
 };
 
 /* Entry in the signatured_types hash table.  */
-- 
2.17.2

[PATCH 09/14] Add objfile member to DWARF batons

[Tom Tromey]

Various DWARF callbacks expect to be able to fetch the objfile from
the DWARF CU object.  However, this won't be possible once sharing is
implemented.

Because these objects are related to full symbols (e.g., they are used
to implement location expressions), they can simply store the objfile
they need.

This patch adds an objfile member to the various "baton" structures
and arranges to set this value when constructing the baton.

gdb/ChangeLog
2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/loc.c (struct piece_closure) <objfile>: New member.
	(allocate_piece_closure): Set "objfile" member.
	* dwarf2/read.c (read_call_site_scope)
	(mark_common_block_symbol_computed, attr_to_dynamic_prop)
	(dwarf2_const_value_attr, dwarf2_fetch_die_loc_sect_off)
	(fill_in_loclist_baton, dwarf2_symbol_mark_computed): Set objfile
	member.
	* dwarf2/loc.h (struct dwarf2_locexpr_baton) <objfile>: New
	member.
	(struct dwarf2_loclist_baton) <objfile>: New member.
---
 gdb/ChangeLog     | 13 +++++++++++++
 gdb/dwarf2/loc.c  |  5 +++++
 gdb/dwarf2/loc.h  |  6 ++++++
 gdb/dwarf2/read.c | 12 ++++++++++--
 4 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index a9523e9f7ee..4a148c26722 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -1544,6 +1544,9 @@ struct piece_closure
   /* Reference count.  */
   int refc = 0;
 
+  /* The objfile from which this closure's expression came.  */
+  struct objfile *objfile = nullptr;
+
   /* The CU from which this closure's expression came.  */
   struct dwarf2_per_cu_data *per_cu = NULL;
 
@@ -1566,6 +1569,8 @@ allocate_piece_closure (struct dwarf2_per_cu_data *per_cu,
   struct piece_closure *c = new piece_closure;
 
   c->refc = 1;
+  /* We must capture this here due to sharing of DWARF state.  */
+  c->objfile = per_cu->objfile ();
   c->per_cu = per_cu;
   c->pieces = std::move (pieces);
   if (frame == NULL)
diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h
index 8fff663ebf9..51bcf6158a8 100644
--- a/gdb/dwarf2/loc.h
+++ b/gdb/dwarf2/loc.h
@@ -166,6 +166,9 @@ struct dwarf2_locexpr_baton
      directly.  */
   bool is_reference;
 
+  /* The objfile that was used when creating this.  */
+  struct objfile *objfile;
+
   /* The compilation unit containing the symbol whose location
      we're computing.  */
   struct dwarf2_per_cu_data *per_cu;
@@ -183,6 +186,9 @@ struct dwarf2_loclist_baton
   /* Length of the location list.  */
   size_t size;
 
+  /* The objfile that was used when creating this.  */
+  struct objfile *objfile;
+
   /* The compilation unit containing the symbol whose location
      we're computing.  */
   struct dwarf2_per_cu_data *per_cu;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 44fdb070e49..7b493f5a227 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -13298,6 +13298,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
       dlbaton = XOBNEW (&objfile->objfile_obstack, struct dwarf2_locexpr_baton);
       dlbaton->data = DW_BLOCK (attr)->data;
       dlbaton->size = DW_BLOCK (attr)->size;
+      dlbaton->objfile = objfile;
       dlbaton->per_cu = cu->per_cu;
 
       SET_FIELD_DWARF_BLOCK (call_site->target, dlbaton);
@@ -16018,6 +16019,7 @@ mark_common_block_symbol_computed (struct symbol *sym,
 	      || member_loc->form_is_constant ());
 
   baton = XOBNEW (&objfile->objfile_obstack, struct dwarf2_locexpr_baton);
+  baton->objfile = objfile;
   baton->per_cu = cu->per_cu;
   gdb_assert (baton->per_cu);
 
@@ -17119,8 +17121,8 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 		      struct type *default_type)
 {
   struct dwarf2_property_baton *baton;
-  struct obstack *obstack
-    = &cu->per_cu->dwarf2_per_objfile->objfile->objfile_obstack;
+  struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile;
+  struct obstack *obstack = &objfile->objfile_obstack;
 
   gdb_assert (default_type != NULL);
 
@@ -17132,6 +17134,7 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
       baton = XOBNEW (obstack, struct dwarf2_property_baton);
       baton->property_type = default_type;
       baton->locexpr.per_cu = cu->per_cu;
+      baton->locexpr.objfile = objfile;
       baton->locexpr.size = DW_BLOCK (attr)->size;
       baton->locexpr.data = DW_BLOCK (attr)->data;
       switch (attr->name)
@@ -17178,6 +17181,7 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
 		baton = XOBNEW (obstack, struct dwarf2_property_baton);
 		baton->property_type = die_type (target_die, target_cu);
 		baton->locexpr.per_cu = cu->per_cu;
+		baton->locexpr.objfile = objfile;
 		baton->locexpr.size = DW_BLOCK (target_attr)->size;
 		baton->locexpr.data = DW_BLOCK (target_attr)->data;
 		baton->locexpr.is_reference = true;
@@ -21037,6 +21041,7 @@ dwarf2_const_value_attr (const struct attribute *attr, struct type *type,
 	   piggyback on the existing location code rather than writing
 	   a new implementation of symbol_computed_ops.  */
 	*baton = XOBNEW (obstack, struct dwarf2_locexpr_baton);
+	(*baton)->objfile = objfile;
 	(*baton)->per_cu = cu->per_cu;
 	gdb_assert ((*baton)->per_cu);
 
@@ -22411,6 +22416,7 @@ dwarf2_fetch_die_loc_sect_off (sect_offset sect_off,
       retval.data = DW_BLOCK (attr)->data;
       retval.size = DW_BLOCK (attr)->size;
     }
+  retval.objfile = objfile;
   retval.per_cu = cu->per_cu;
 
   age_cached_comp_units (dwarf2_per_objfile);
@@ -24051,6 +24057,7 @@ fill_in_loclist_baton (struct dwarf2_cu *cu,
 
   section->read (dwarf2_per_objfile->objfile);
 
+  baton->objfile = dwarf2_per_objfile->objfile;
   baton->per_cu = cu->per_cu;
   gdb_assert (baton->per_cu);
   /* We don't know how long the location list is, but make sure we
@@ -24096,6 +24103,7 @@ dwarf2_symbol_mark_computed (const struct attribute *attr, struct symbol *sym,
       struct dwarf2_locexpr_baton *baton;
 
       baton = XOBNEW (&objfile->objfile_obstack, struct dwarf2_locexpr_baton);
+      baton->objfile = objfile;
       baton->per_cu = cu->per_cu;
       gdb_assert (baton->per_cu);
 
-- 
2.17.2

[PATCH 14/14] Share DWARF partial symtabs

[Tom Tromey]

This changes the DWARF reader to share partial symtabs (or indices if
they are available) across objfiles.  This has a few parts.

* The dwarf2_unshareable object is created per-objfile and is stored
  on the objfile using a registry key.  dwarf2_enter_objfile is
  modified to save and restore this pointer in the dwarf2_per_objfile.

* objfile::partial_symtabs is changed to be a shared_ptr again.  This
  lets us stash a second reference in dwarf2_per_objfile; if the DWARF
  data is being shared, we can simply copy this value to the new
  objfile.

* The dwarf2_per_objfile itself may be shared via the BFD -- using a
  new per-BFD registry key -- or not.  This depends both on whether
  some other symbol reader has found symbols, and whether any BFD
  sections require relocation.

2020-02-15  Tom Tromey  <tom@tromey.com>

	* objfiles.h (struct objfile) <partial_symtabs>: Now a
	shared_ptr.
	* dwarf2/read.h (struct dwarf2_per_objfile) <partial_symtabs>: New
	member.
	(class dwarf2_enter_objfile): Constructor no longer inline.
	<m_restore_unshared>: New member.
	* dwarf2/read.c (dwarf2_unshared_data_key, dwarf2_bfd_data_key):
	New globals.
	(dwarf2_objfile_data_key): Add comment.
	(get_dwarf2_per_objfile): Rewrite.
	(dwarf2_enter_objfile::dwarf2_enter_objfile): Move from read.h.
	Initialize m_restore_unshared.
	(dwarf2_per_objfile::dwarf2_per_objfile): Add "unshared"
	parameter.  Don't initialize objfile member.
	(dwarf2_per_objfile::~dwarf2_per_objfile): Don't deleted
	"unshareable".
	(dwarf2_has_info): Check dwarf2_bfd_data_key and
	dwarf2_unshared_data_key.
	(dwarf2_get_section_info): Use get_dwarf2_per_objfile.
	(dwarf2_build_psymtabs): Set objfile::partial_symtabs and
	short-circuit when sharing.
	(dwarf2_build_psymtabs): Set dwarf2_per_objfile::partial_symtabs.
	(dwarf2_psymtab::expand_psymtab): Use free_cached_comp_units.
---
 gdb/ChangeLog     | 26 ++++++++++++++++++
 gdb/dwarf2/read.c | 70 +++++++++++++++++++++++++++++++++++++----------
 gdb/dwarf2/read.h | 12 ++++----
 gdb/objfiles.h    |  2 +-
 4 files changed, 89 insertions(+), 21 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 15813b72005..7be57661296 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -100,7 +100,15 @@ static bool check_physname = false;
 /* When true, do not reject deprecated .gdb_index sections.  */
 static bool use_deprecated_index_sections = false;
 
-static const struct objfile_key<dwarf2_per_objfile> dwarf2_objfile_data_key;
+/* This is used to store the data that is always per objfile.  */
+static const objfile_key<dwarf2_unshareable> dwarf2_unshared_data_key;
+
+/* These are used to store the dwarf2_per_objfile; either on the
+   objfile or on the BFD.  */
+static const struct objfile_key<dwarf2_per_objfile>
+  dwarf2_objfile_data_key;
+static const struct bfd_key<dwarf2_per_objfile>
+  dwarf2_bfd_data_key;
 
 /* The "aclass" indices for various kinds of computed DWARF symbols.  */
 
@@ -274,7 +282,18 @@ struct mapped_debug_names final : public mapped_index_base
 dwarf2_per_objfile *
 get_dwarf2_per_objfile (struct objfile *objfile)
 {
-  return dwarf2_objfile_data_key.get (objfile);
+  dwarf2_per_objfile *result = dwarf2_bfd_data_key.get (objfile->obfd);
+  if (result == nullptr)
+    result = dwarf2_objfile_data_key.get (objfile);
+  return result;
+}
+
+dwarf2_enter_objfile::dwarf2_enter_objfile (struct objfile *objfile)
+  : m_per_objfile (get_dwarf2_per_objfile (objfile)),
+    m_restore_objfile (&m_per_objfile->objfile, objfile),
+    m_restore_unshared (&m_per_objfile->unshareable,
+			dwarf2_unshared_data_key.get (objfile))
+{
 }
 
 /* Default names of the debugging sections.  */
@@ -1754,16 +1773,12 @@ line_header_eq_voidp (const void *item_lhs, const void *item_rhs)
 dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
 					const dwarf2_debug_sections *names,
 					bool can_copy_)
-  : objfile (objfile_),
-    can_copy (can_copy_),
-    /* Temporarily just allocate one per objfile -- we don't have
-       sharing yet.  */
-    unshareable (new dwarf2_unshareable)
+  : can_copy (can_copy_)
 {
   if (names == NULL)
     names = &dwarf2_elf_names;
 
-  bfd *obfd = objfile->obfd;
+  bfd *obfd = objfile_->obfd;
 
   for (asection *sec = obfd->sections; sec != NULL; sec = sec->next)
     locate_sections (obfd, sec, *names);
@@ -1780,8 +1795,6 @@ dwarf2_per_objfile::~dwarf2_per_objfile ()
   for (signatured_type *sig_type : all_type_units)
     sig_type->per_cu.imported_symtabs_free ();
 
-  delete unshareable;
-
   /* Everything else should be on the objfile obstack.  */
 }
 
@@ -1841,13 +1854,26 @@ dwarf2_has_info (struct objfile *objfile,
   if (objfile->flags & OBJF_READNEVER)
     return 0;
 
+  struct dwarf2_unshareable *unshared = dwarf2_unshared_data_key.get (objfile);
+  if (unshared == nullptr)
+    unshared = dwarf2_unshared_data_key.emplace (objfile);
+
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
   if (dwarf2_per_objfile == NULL)
-    dwarf2_per_objfile = dwarf2_objfile_data_key.emplace (objfile, objfile,
-							  names,
-							  can_copy);
+    {
+      dwarf2_per_objfile = new ::dwarf2_per_objfile (objfile, names,
+						     can_copy);
+      /* We can share this if the objfile doesn't require relocations
+	 and if there aren't partial symbols from some other
+	 reader.  */
+      if (!objfile_has_partial_symbols (objfile)
+	  && !gdb_bfd_requires_relocations (objfile->obfd))
+	dwarf2_bfd_data_key.set (objfile->obfd, dwarf2_per_objfile);
+      else
+	dwarf2_objfile_data_key.set (objfile, dwarf2_per_objfile);
+    }
 
   return (!dwarf2_per_objfile->info.is_virtual
 	  && dwarf2_per_objfile->info.s.section != NULL
@@ -2006,7 +2032,7 @@ dwarf2_get_section_info (struct objfile *objfile,
                          asection **sectp, const gdb_byte **bufp,
                          bfd_size_type *sizep)
 {
-  struct dwarf2_per_objfile *data = dwarf2_objfile_data_key.get (objfile);
+  struct dwarf2_per_objfile *data = get_dwarf2_per_objfile (objfile);
   struct dwarf2_section_info *info;
 
   /* We may see an objfile without any DWARF, in which case we just
@@ -5862,6 +5888,15 @@ dwarf2_build_psymtabs (struct objfile *objfile)
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
+  if (dwarf2_per_objfile->partial_symtabs != nullptr)
+    {
+      /* Partial symbols were already read, so now we can simply
+	 attach them.  */
+      objfile->partial_symtabs = dwarf2_per_objfile->partial_symtabs;
+      dwarf2_resize_unshareable (dwarf2_per_objfile);
+      return;
+    }
+
   init_psymbol_list (objfile, 1024);
 
   try
@@ -5882,6 +5917,11 @@ dwarf2_build_psymtabs (struct objfile *objfile)
     {
       exception_print (gdb_stderr, except);
     }
+
+  /* Finish by setting the local reference to partial symtabs, so that
+     we don't try to read again.  If we can't in fact share, this
+     won't make a difference anyway.  */
+  dwarf2_per_objfile->partial_symtabs = objfile->partial_symtabs;
 }
 
 /* Find the base address of the compilation unit for range lists and
@@ -8915,8 +8955,8 @@ dwarf2_psymtab::expand_psymtab (struct objfile *objfile)
   if (symtab.has_value ())
     return;
 
+  free_cached_comp_units freer (dwarf2_per_objfile);
   read_dependencies (objfile);
-
   dw2_do_instantiate_symtab (per_cu_data, false);
 }
 
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 241f3c8e71e..891a844a673 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -299,6 +299,11 @@ public:
   /* CUs that are queued to be read.  */
   std::queue<dwarf2_queue_item> queue;
 
+  /* We keep a separate reference to the partial symtabs, in case we
+     are sharing them between objfiles.  This is only set after
+     partial symbols have been read the first time.  */
+  std::shared_ptr<psymtab_storage> partial_symtabs;
+
   /* The total number of per_cu and signatured_type objects that have
      been created for this reader.  */
   size_t num_psymtabs = 0;
@@ -321,11 +326,7 @@ class dwarf2_enter_objfile
 {
 public:
 
-  dwarf2_enter_objfile (struct objfile *objfile)
-    : m_per_objfile (get_dwarf2_per_objfile (objfile)),
-      m_restore_objfile (&m_per_objfile->objfile, objfile)
-  {
-  }
+  dwarf2_enter_objfile (struct objfile *objfile);
 
   ~dwarf2_enter_objfile () = default;
 
@@ -335,6 +336,7 @@ private:
 
   dwarf2_per_objfile *m_per_objfile;
   scoped_restore_tmpl<struct objfile *> m_restore_objfile;
+  scoped_restore_tmpl<dwarf2_unshareable *> m_restore_unshared;
 };
 
 /* A partial symtab specialized for DWARF.  */
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index b71a8a9edb8..12892f80f71 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -558,7 +558,7 @@ public:
 
   /* The partial symbol tables.  */
 
-  std::unique_ptr<psymtab_storage> partial_symtabs;
+  std::shared_ptr<psymtab_storage> partial_symtabs;
 
   /* The object file's BFD.  Can be null if the objfile contains only
      minimal symbols, e.g. the run time common symbols for SunOS4.  */
-- 
2.17.2

Re: [PATCH 14/14] Share DWARF partial symtabs

[Luis Machado]

On 2/15/20 1:54 PM, Tom Tromey wrote:
> This changes the DWARF reader to share partial symtabs (or indices if
> they are available) across objfiles.  This has a few parts.
> 
> * The dwarf2_unshareable object is created per-objfile and is stored
>    on the objfile using a registry key.  dwarf2_enter_objfile is
>    modified to save and restore this pointer in the dwarf2_per_objfile.
> 
> * objfile::partial_symtabs is changed to be a shared_ptr again.  This
>    lets us stash a second reference in dwarf2_per_objfile; if the DWARF
>    data is being shared, we can simply copy this value to the new
>    objfile.
> 
> * The dwarf2_per_objfile itself may be shared via the BFD -- using a
>    new per-BFD registry key -- or not.  This depends both on whether
>    some other symbol reader has found symbols, and whether any BFD
>    sections require relocation.
> 
> 2020-02-15  Tom Tromey  <tom@tromey.com>
> 
> 	* objfiles.h (struct objfile) <partial_symtabs>: Now a
> 	shared_ptr.
> 	* dwarf2/read.h (struct dwarf2_per_objfile) <partial_symtabs>: New
> 	member.
> 	(class dwarf2_enter_objfile): Constructor no longer inline.
> 	<m_restore_unshared>: New member.
> 	* dwarf2/read.c (dwarf2_unshared_data_key, dwarf2_bfd_data_key):
> 	New globals.
> 	(dwarf2_objfile_data_key): Add comment.
> 	(get_dwarf2_per_objfile): Rewrite.
> 	(dwarf2_enter_objfile::dwarf2_enter_objfile): Move from read.h.
> 	Initialize m_restore_unshared.
> 	(dwarf2_per_objfile::dwarf2_per_objfile): Add "unshared"
> 	parameter.  Don't initialize objfile member.
> 	(dwarf2_per_objfile::~dwarf2_per_objfile): Don't deleted
> 	"unshareable".

"Don't delete..."

> 	(dwarf2_has_info): Check dwarf2_bfd_data_key and
> 	dwarf2_unshared_data_key.
> 	(dwarf2_get_section_info): Use get_dwarf2_per_objfile.
> 	(dwarf2_build_psymtabs): Set objfile::partial_symtabs and
> 	short-circuit when sharing.
> 	(dwarf2_build_psymtabs): Set dwarf2_per_objfile::partial_symtabs.
> 	(dwarf2_psymtab::expand_psymtab): Use free_cached_comp_units.
> ---
>   gdb/ChangeLog     | 26 ++++++++++++++++++
>   gdb/dwarf2/read.c | 70 +++++++++++++++++++++++++++++++++++++----------
>   gdb/dwarf2/read.h | 12 ++++----
>   gdb/objfiles.h    |  2 +-
>   4 files changed, 89 insertions(+), 21 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 15813b72005..7be57661296 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -100,7 +100,15 @@ static bool check_physname = false;
>   /* When true, do not reject deprecated .gdb_index sections.  */
>   static bool use_deprecated_index_sections = false;
>   
> -static const struct objfile_key<dwarf2_per_objfile> dwarf2_objfile_data_key;
> +/* This is used to store the data that is always per objfile.  */
> +static const objfile_key<dwarf2_unshareable> dwarf2_unshared_data_key;
> +
> +/* These are used to store the dwarf2_per_objfile; either on the
> +   objfile or on the BFD.  */
> +static const struct objfile_key<dwarf2_per_objfile>
> +  dwarf2_objfile_data_key;
> +static const struct bfd_key<dwarf2_per_objfile>
> +  dwarf2_bfd_data_key;
>   
>   /* The "aclass" indices for various kinds of computed DWARF symbols.  */
>   
> @@ -274,7 +282,18 @@ struct mapped_debug_names final : public mapped_index_base
>   dwarf2_per_objfile *
>   get_dwarf2_per_objfile (struct objfile *objfile)
>   {
> -  return dwarf2_objfile_data_key.get (objfile);
> +  dwarf2_per_objfile *result = dwarf2_bfd_data_key.get (objfile->obfd);
> +  if (result == nullptr)
> +    result = dwarf2_objfile_data_key.get (objfile);
> +  return result;
> +}
> +
> +dwarf2_enter_objfile::dwarf2_enter_objfile (struct objfile *objfile)
> +  : m_per_objfile (get_dwarf2_per_objfile (objfile)),
> +    m_restore_objfile (&m_per_objfile->objfile, objfile),
> +    m_restore_unshared (&m_per_objfile->unshareable,
> +			dwarf2_unshared_data_key.get (objfile))
> +{
>   }
>   
>   /* Default names of the debugging sections.  */
> @@ -1754,16 +1773,12 @@ line_header_eq_voidp (const void *item_lhs, const void *item_rhs)
>   dwarf2_per_objfile::dwarf2_per_objfile (struct objfile *objfile_,
>   					const dwarf2_debug_sections *names,
>   					bool can_copy_)
> -  : objfile (objfile_),
> -    can_copy (can_copy_),
> -    /* Temporarily just allocate one per objfile -- we don't have
> -       sharing yet.  */
> -    unshareable (new dwarf2_unshareable)
> +  : can_copy (can_copy_)
>   {
>     if (names == NULL)
>       names = &dwarf2_elf_names;
>   
> -  bfd *obfd = objfile->obfd;
> +  bfd *obfd = objfile_->obfd;
>   
>     for (asection *sec = obfd->sections; sec != NULL; sec = sec->next)
>       locate_sections (obfd, sec, *names);
> @@ -1780,8 +1795,6 @@ dwarf2_per_objfile::~dwarf2_per_objfile ()
>     for (signatured_type *sig_type : all_type_units)
>       sig_type->per_cu.imported_symtabs_free ();
>   
> -  delete unshareable;
> -
>     /* Everything else should be on the objfile obstack.  */
>   }
>   
> @@ -1841,13 +1854,26 @@ dwarf2_has_info (struct objfile *objfile,
>     if (objfile->flags & OBJF_READNEVER)
>       return 0;
>   
> +  struct dwarf2_unshareable *unshared = dwarf2_unshared_data_key.get (objfile);
> +  if (unshared == nullptr)
> +    unshared = dwarf2_unshared_data_key.emplace (objfile);
> +
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
>     if (dwarf2_per_objfile == NULL)
> -    dwarf2_per_objfile = dwarf2_objfile_data_key.emplace (objfile, objfile,
> -							  names,
> -							  can_copy);
> +    {
> +      dwarf2_per_objfile = new ::dwarf2_per_objfile (objfile, names,
> +						     can_copy);
> +      /* We can share this if the objfile doesn't require relocations
> +	 and if there aren't partial symbols from some other
> +	 reader.  */
> +      if (!objfile_has_partial_symbols (objfile)
> +	  && !gdb_bfd_requires_relocations (objfile->obfd))
> +	dwarf2_bfd_data_key.set (objfile->obfd, dwarf2_per_objfile);
> +      else
> +	dwarf2_objfile_data_key.set (objfile, dwarf2_per_objfile);
> +    }
>   
>     return (!dwarf2_per_objfile->info.is_virtual
>   	  && dwarf2_per_objfile->info.s.section != NULL
> @@ -2006,7 +2032,7 @@ dwarf2_get_section_info (struct objfile *objfile,
>                            asection **sectp, const gdb_byte **bufp,
>                            bfd_size_type *sizep)
>   {
> -  struct dwarf2_per_objfile *data = dwarf2_objfile_data_key.get (objfile);
> +  struct dwarf2_per_objfile *data = get_dwarf2_per_objfile (objfile);
>     struct dwarf2_section_info *info;
>   
>     /* We may see an objfile without any DWARF, in which case we just
> @@ -5862,6 +5888,15 @@ dwarf2_build_psymtabs (struct objfile *objfile)
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> +  if (dwarf2_per_objfile->partial_symtabs != nullptr)
> +    {
> +      /* Partial symbols were already read, so now we can simply
> +	 attach them.  */
> +      objfile->partial_symtabs = dwarf2_per_objfile->partial_symtabs;
> +      dwarf2_resize_unshareable (dwarf2_per_objfile);
> +      return;
> +    }
> +
>     init_psymbol_list (objfile, 1024);
>   
>     try
> @@ -5882,6 +5917,11 @@ dwarf2_build_psymtabs (struct objfile *objfile)
>       {
>         exception_print (gdb_stderr, except);
>       }
> +
> +  /* Finish by setting the local reference to partial symtabs, so that
> +     we don't try to read again.  If we can't in fact share, this
> +     won't make a difference anyway.  */
> +  dwarf2_per_objfile->partial_symtabs = objfile->partial_symtabs;
>   }
>   
>   /* Find the base address of the compilation unit for range lists and
> @@ -8915,8 +8955,8 @@ dwarf2_psymtab::expand_psymtab (struct objfile *objfile)
>     if (symtab.has_value ())
>       return;
>   
> +  free_cached_comp_units freer (dwarf2_per_objfile);
>     read_dependencies (objfile);
> -
>     dw2_do_instantiate_symtab (per_cu_data, false);
>   }
>   

Spurious line removal, but it doesn't matter much.

> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index 241f3c8e71e..891a844a673 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -299,6 +299,11 @@ public:
>     /* CUs that are queued to be read.  */
>     std::queue<dwarf2_queue_item> queue;
>   
> +  /* We keep a separate reference to the partial symtabs, in case we
> +     are sharing them between objfiles.  This is only set after
> +     partial symbols have been read the first time.  */
> +  std::shared_ptr<psymtab_storage> partial_symtabs;
> +
>     /* The total number of per_cu and signatured_type objects that have
>        been created for this reader.  */
>     size_t num_psymtabs = 0;
> @@ -321,11 +326,7 @@ class dwarf2_enter_objfile
>   {
>   public:
>   
> -  dwarf2_enter_objfile (struct objfile *objfile)
> -    : m_per_objfile (get_dwarf2_per_objfile (objfile)),
> -      m_restore_objfile (&m_per_objfile->objfile, objfile)
> -  {
> -  }
> +  dwarf2_enter_objfile (struct objfile *objfile);
>   
>     ~dwarf2_enter_objfile () = default;
>   
> @@ -335,6 +336,7 @@ private:
>   
>     dwarf2_per_objfile *m_per_objfile;
>     scoped_restore_tmpl<struct objfile *> m_restore_objfile;
> +  scoped_restore_tmpl<dwarf2_unshareable *> m_restore_unshared;
>   };
>   
>   /* A partial symtab specialized for DWARF.  */
> diff --git a/gdb/objfiles.h b/gdb/objfiles.h
> index b71a8a9edb8..12892f80f71 100644
> --- a/gdb/objfiles.h
> +++ b/gdb/objfiles.h
> @@ -558,7 +558,7 @@ public:
>   
>     /* The partial symbol tables.  */
>   
> -  std::unique_ptr<psymtab_storage> partial_symtabs;
> +  std::shared_ptr<psymtab_storage> partial_symtabs;
>   
>     /* The object file's BFD.  Can be null if the objfile contains only
>        minimal symbols, e.g. the run time common symbols for SunOS4.  */
> 

I've gone through the series and didn't see anything obviously wrong. I 
suppose this will get more thoroughly exercised when running in 
multi-process/multi-target mode with quite a few objfiles and libraries.

Is it currently possible to exercise such a scenario automatically? I 
suppose manually loading something like a web browser will be a good 
test as well.

Re: [PATCH 14/14] Share DWARF partial symtabs

[Tom Tromey]

>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:

Luis> Is it currently possible to exercise such a scenario automatically? I
Luis> suppose manually loading something like a web browser will be a good 
Luis> test as well.

FWIW just running the gdb.multi tests caught a few bugs.

Tom

[PATCH 13/14] Move signatured_type::type to unshareable object

[Tom Tromey]

signatured_type has a link to the "struct type".  However, types are
inherently objfile-specific, so once sharing is implemented, this will
be incorrect.

This patch moves the type to a new map in the DWARF unshareable
object.

2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.h (struct dwarf2_unshareable) <type_map>: New
	member.
	(struct signatured_type) <type>: Remove.
	* dwarf2/read.c (get_signatured_type): Use type_map.
---
 gdb/ChangeLog     | 7 +++++++
 gdb/dwarf2/read.c | 7 ++++---
 gdb/dwarf2/read.h | 8 +++-----
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 765fc4e8272..15813b72005 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -22767,8 +22767,9 @@ get_signatured_type (struct die_info *die, ULONGEST signature,
     }
 
   /* If we already know the type we're done.  */
-  if (sig_type->type != NULL)
-    return sig_type->type;
+  auto iter = dwarf2_per_objfile->unshareable->type_map.find (sig_type);
+  if (iter != dwarf2_per_objfile->unshareable->type_map.end ())
+    return iter->second;
 
   type_cu = cu;
   type_die = follow_die_sig_1 (die, sig_type, &type_cu);
@@ -22795,7 +22796,7 @@ get_signatured_type (struct die_info *die, ULONGEST signature,
 		 objfile_name (dwarf2_per_objfile->objfile));
       type = build_error_marker_type (cu, die);
     }
-  sig_type->type = type;
+  dwarf2_per_objfile->unshareable->type_map[sig_type] = type;
 
   return type;
 }
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 18f3c264a0a..241f3c8e71e 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -112,6 +112,9 @@ struct dwarf2_unshareable
      structure.  */
   std::unordered_map<type_unit_group *, std::unique_ptr<type_unit_unshareable>>
     type_units;
+
+  /* Map from signatured types to the corresponding struct type.  */
+  std::unordered_map<signatured_type *, struct type *> type_map;
 };
 
 /* Collection of data recorded per objfile.
@@ -574,11 +577,6 @@ struct signatured_type
      can share them.  This points to the containing symtab.  */
   struct type_unit_group *type_unit_group;
 
-  /* The type.
-     The first time we encounter this type we fully read it in and install it
-     in the symbol tables.  Subsequent times we only need the type.  */
-  struct type *type;
-
   /* Containing DWO unit.
      This field is valid iff per_cu.reading_dwo_directly.  */
   struct dwo_unit *dwo_unit;
-- 
2.17.2

[PATCH 08/14] Remove symtab links from dwarf2_psymtab and dwarf2_per_cu_quick_data

[Tom Tromey]

This removes the links from dwarf2_psymtab and
dwarf2_per_cu_quick_data to the compunit_symtab.  Now, the DWARF code
uses the index in these objects to find the corresponding symtab in
the "unshared" object.

2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.h (struct dwarf2_unshareable) <symtabs>: New
	member.
	(struct dwarf2_psymtab): Derive from partial_symtab.
	<readin_p, get_compunit_symtab>: Declare methods.
	* dwarf2/read.c (struct dwarf2_per_cu_quick_data)
	<compunit_symtab>: Remove.
	(dw2_do_instantiate_symtab, dw2_instantiate_symtab)
	(dw2_map_expand_apply, dw2_map_symtabs_matching_filename)
	(dw2_symtab_iter_next, dw2_print_stats)
	(dw2_expand_symtabs_with_fullname)
	(dw2_expand_symtabs_matching_one)
	(dw_expand_symtabs_matching_file_matcher)
	(dw2_find_pc_sect_compunit_symtab, dw2_map_symbol_filenames)
	(dw2_debug_names_iterator::next, does)
	(fill_in_sig_entry_from_dwo_entry, dwarf2_psymtab::read_symtab)
	(process_queue, dwarf2_psymtab::expand_psymtab): Update.
	(dwarf2_psymtab::readin_p, dwarf2_psymtab::get_compunit_symtab):
	New methods.
	(get_compunit_symtab, process_full_comp_unit)
	(process_full_type_unit): Update.
	(dwarf2_build_psymtabs): Resize the symtabs vector.
	(dwarf2_resize_unshareable): New function.
	(dwarf2_initialize_objfile): Call dwarf2_resize_unshareable.
---
 gdb/ChangeLog     |  26 +++++++
 gdb/dwarf2/read.c | 194 +++++++++++++++++++++++++++++++---------------
 gdb/dwarf2/read.h |  14 +++-
 3 files changed, 170 insertions(+), 64 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 281d39ad271..44fdb070e49 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2205,10 +2205,6 @@ struct dwarf2_per_cu_quick_data
      NOTE: This points into dwarf2_per_objfile->quick_file_names_table.  */
   struct quick_file_names *file_names;
 
-  /* The corresponding symbol table.  This is NULL if symbols for this
-     CU have not yet been read.  */
-  struct compunit_symtab *compunit_symtab;
-
   /* A temporary mark bit used when iterating over all CUs in
      expand_symtabs_matching.  */
   unsigned int mark : 1;
@@ -2332,9 +2328,9 @@ dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
      with the dwarf queue empty.  */
   dwarf2_queue_guard q_guard (dwarf2_per_objfile);
 
-  if (dwarf2_per_objfile->using_index
-      ? per_cu->v.quick->compunit_symtab == NULL
-      : (per_cu->v.psymtab == NULL || !per_cu->v.psymtab->readin))
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+  if (!symtab.has_value ())
     {
       queue_comp_unit (per_cu, language_minimal);
       load_cu (per_cu, skip_partial);
@@ -2369,7 +2365,10 @@ dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
   struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;
 
   gdb_assert (dwarf2_per_objfile->using_index);
-  if (!per_cu->v.quick->compunit_symtab)
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+
+  if (!symtab.has_value ())
     {
       free_cached_comp_units freer (dwarf2_per_objfile);
       scoped_restore decrementer = increment_reading_symtab ();
@@ -2377,7 +2376,7 @@ dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
       process_cu_includes (dwarf2_per_objfile);
     }
 
-  return per_cu->v.quick->compunit_symtab;
+  return *symtab;
 }
 
 /* See declaration.  */
@@ -3289,7 +3288,11 @@ dw2_map_expand_apply (struct objfile *objfile,
   struct compunit_symtab *last_made = objfile->compunit_symtabs;
 
   /* Don't visit already-expanded CUs.  */
-  if (per_cu->v.quick->compunit_symtab)
+  struct dwarf2_per_objfile *dwarf2_per_objfile
+    = get_dwarf2_per_objfile (objfile);
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+  if (symtab.has_value ())
     return 0;
 
   /* This may expand more than one symtab, and we want to iterate over
@@ -3317,7 +3320,9 @@ dw2_map_symtabs_matching_filename
   for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
     {
       /* We only need to look at symtabs not already expanded.  */
-      if (per_cu->v.quick->compunit_symtab)
+      gdb::optional<compunit_symtab *> &symtab
+	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+      if (symtab.has_value ())
 	continue;
 
       quick_file_names *file_data = dw2_get_file_names (per_cu);
@@ -3458,7 +3463,9 @@ dw2_symtab_iter_next (struct dw2_symtab_iterator *iter)
       dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (cu_index);
 
       /* Skip if already read in.  */
-      if (per_cu->v.quick->compunit_symtab)
+      gdb::optional<compunit_symtab *> &symtab
+	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+      if (symtab.has_value ())
 	continue;
 
       /* Check static vs global.  */
@@ -3573,7 +3580,9 @@ dw2_print_stats (struct objfile *objfile)
     {
       dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
 
-      if (!per_cu->v.quick->compunit_symtab)
+      gdb::optional<compunit_symtab *> &symtab
+	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+      if (!symtab.has_value ())
 	++count;
     }
   printf_filtered (_("  Number of read CUs: %d\n"), total - count);
@@ -3656,7 +3665,9 @@ dw2_expand_symtabs_with_fullname (struct objfile *objfile,
   for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
     {
       /* We only need to look at symtabs not already expanded.  */
-      if (per_cu->v.quick->compunit_symtab)
+      gdb::optional<compunit_symtab *> &symtab
+	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+      if (symtab.has_value ())
 	continue;
 
       quick_file_names *file_data = dw2_get_file_names (per_cu);
@@ -4455,15 +4466,19 @@ dw2_expand_symtabs_matching_one
 {
   if (file_matcher == NULL || per_cu->v.quick->mark)
     {
-      bool symtab_was_null
-	= (per_cu->v.quick->compunit_symtab == NULL);
+      struct dwarf2_per_objfile *dwarf2_per_objfile
+	= per_cu->dwarf2_per_objfile;
+      gdb::optional<compunit_symtab *> &symtab
+	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+      bool symtab_was_null = !symtab.has_value () || *symtab == nullptr;
 
       dw2_instantiate_symtab (per_cu, false);
 
       if (expansion_notify != NULL
 	  && symtab_was_null
-	  && per_cu->v.quick->compunit_symtab != NULL)
-	expansion_notify (per_cu->v.quick->compunit_symtab);
+	  && symtab.has_value ()
+	  && *symtab != nullptr)
+	expansion_notify (*symtab);
     }
 }
 
@@ -4583,7 +4598,9 @@ dw_expand_symtabs_matching_file_matcher
       per_cu->v.quick->mark = 0;
 
       /* We only need to look at symtabs not already expanded.  */
-      if (per_cu->v.quick->compunit_symtab)
+      gdb::optional<compunit_symtab *> &symtab
+	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+      if (symtab.has_value ())
 	continue;
 
       quick_file_names *file_data = dw2_get_file_names (per_cu);
@@ -4708,7 +4725,11 @@ dw2_find_pc_sect_compunit_symtab (struct objfile *objfile,
   if (!data)
     return NULL;
 
-  if (warn_if_readin && data->v.quick->compunit_symtab)
+  struct dwarf2_per_objfile *dwarf2_per_objfile
+    = get_dwarf2_per_objfile (objfile);
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[data->index];
+  if (warn_if_readin && symtab.has_value ())
     warning (_("(Internal error: pc %s in read in CU, but not in symtab.)"),
 	     paddress (get_objfile_arch (objfile), pc));
 
@@ -4741,7 +4762,9 @@ dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
 
       for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
 	{
-	  if (per_cu->v.quick->compunit_symtab)
+	  gdb::optional<compunit_symtab *> &symtab
+	    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+	  if (symtab.has_value ())
 	    {
 	      void **slot = htab_find_slot (visited.get (),
 					    per_cu->v.quick->file_names,
@@ -4754,7 +4777,9 @@ dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
       for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
 	{
 	  /* We only need to look at symtabs not already expanded.  */
-	  if (per_cu->v.quick->compunit_symtab)
+	  gdb::optional<compunit_symtab *> &symtab
+	    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+	  if (symtab.has_value ())
 	    continue;
 
 	  quick_file_names *file_data = dw2_get_file_names (per_cu);
@@ -5386,7 +5411,9 @@ dw2_debug_names_iterator::next ()
     }
 
   /* Skip if already read in.  */
-  if (per_cu->v.quick->compunit_symtab)
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+  if (symtab.has_value ())
     goto again;
 
   /* Check static vs global.  */
@@ -5627,11 +5654,12 @@ dw2_debug_names_map_matching_symbols
      the psymtab code does.  */
   for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
     {
-      struct compunit_symtab *cust = per_cu->v.quick->compunit_symtab;
-      if (cust != nullptr)
+      gdb::optional<compunit_symtab *> &symtab
+	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+      if (symtab.has_value () && *symtab != nullptr)
 	{
 	  const struct block *block
-	    = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (cust), block_kind);
+	    = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (*symtab), block_kind);
 	  if (!iterate_over_symbols_terminated (block, name,
 						domain, callback))
 	    break;
@@ -5748,6 +5776,15 @@ get_gdb_index_contents_from_cache_dwz (objfile *obj, dwz_file *dwz)
   return global_index_cache.lookup_gdb_index (build_id, &dwz->index_cache_res);
 }
 
+/* Make sure we have space for the compunit_symtabs we may need.  */
+
+static void
+dwarf2_resize_unshareable (dwarf2_per_objfile *dwarf2_per_objfile)
+{
+  dwarf2_per_objfile->unshareable->symtabs.resize
+    (dwarf2_per_objfile->num_psymtabs);
+}
+
 /* See symfile.h.  */
 
 bool
@@ -5768,6 +5805,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
       dwarf2_per_objfile->quick_file_names_table
 	= create_quick_file_names_table
 	    (dwarf2_per_objfile->all_comp_units.size ());
+      dwarf2_resize_unshareable (dwarf2_per_objfile);
 
       for (int i = 0; i < (dwarf2_per_objfile->all_comp_units.size ()
 			   + dwarf2_per_objfile->all_type_units.size ()); ++i)
@@ -5788,6 +5826,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
   if (dwarf2_read_debug_names (dwarf2_per_objfile))
     {
       *index_kind = dw_index_kind::DEBUG_NAMES;
+      dwarf2_resize_unshareable (dwarf2_per_objfile);
       return true;
     }
 
@@ -5796,6 +5835,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
 			     get_gdb_index_contents_from_section<dwz_file>))
     {
       *index_kind = dw_index_kind::GDB_INDEX;
+      dwarf2_resize_unshareable (dwarf2_per_objfile);
       return true;
     }
 
@@ -5806,6 +5846,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
     {
       global_index_cache.hit ();
       *index_kind = dw_index_kind::GDB_INDEX;
+      dwarf2_resize_unshareable (dwarf2_per_objfile);
       return true;
     }
 
@@ -5834,6 +5875,8 @@ dwarf2_build_psymtabs (struct objfile *objfile)
       dwarf2_build_psymtabs_hard (dwarf2_per_objfile);
       psymtabs.keep ();
 
+      dwarf2_resize_unshareable (dwarf2_per_objfile);
+
       /* (maybe) store an index in the cache.  */
       global_index_cache.store (dwarf2_per_objfile);
     }
@@ -6259,7 +6302,7 @@ fill_in_sig_entry_from_dwo_entry (struct dwarf2_per_objfile *dwarf2_per_objfile,
   if (dwarf2_per_objfile->using_index)
     {
       gdb_assert (sig_entry->per_cu.v.quick != NULL);
-      gdb_assert (sig_entry->per_cu.v.quick->compunit_symtab == NULL);
+      gdb_assert (!dwarf2_per_objfile->unshareable->symtabs[sig_entry->per_cu.index].has_value ());
     }
   else
       gdb_assert (sig_entry->per_cu.v.psymtab == NULL);
@@ -8700,7 +8743,10 @@ dwarf2_psymtab::read_symtab (struct objfile *objfile)
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
-  gdb_assert (!readin);
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu_data->index];
+  gdb_assert (!symtab.has_value ());
+
   /* If this psymtab is constructed from a debug-only objfile, the
      has_section_at_zero flag will not necessarily be correct.  We
      can get the correct value for this flag by looking at the data
@@ -8796,9 +8842,9 @@ process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile)
     {
       dwarf2_queue_item &item = dwarf2_per_objfile->queue.front ();
 
-      if ((dwarf2_per_objfile->using_index
-	   ? !item.per_cu->v.quick->compunit_symtab
-	   : (item.per_cu->v.psymtab && !item.per_cu->v.psymtab->readin))
+      gdb::optional<compunit_symtab *> &symtab
+	= dwarf2_per_objfile->unshareable->symtabs[item.per_cu->index];
+      if (!symtab.has_value ()
 	  /* Skip dummy CUs.  */
 	  && item.per_cu->cu != NULL)
 	{
@@ -8853,24 +8899,57 @@ process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile)
 void
 dwarf2_psymtab::expand_psymtab (struct objfile *objfile)
 {
-  struct dwarf2_per_cu_data *per_cu;
-
-  if (readin)
-    return;
-
-  read_dependencies (objfile);
-
-  per_cu = per_cu_data;
+  struct dwarf2_per_objfile *dwarf2_per_objfile
+    = get_dwarf2_per_objfile (objfile);
 
-  if (per_cu == NULL)
+  if (per_cu_data == NULL)
     {
       /* It's an include file, no symbols to read for it.
          Everything is in the parent symtab.  */
-      readin = true;
       return;
     }
 
-  dw2_do_instantiate_symtab (per_cu, false);
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu_data->index];
+
+  if (symtab.has_value ())
+    return;
+
+  read_dependencies (objfile);
+
+  dw2_do_instantiate_symtab (per_cu_data, false);
+}
+
+/* See psympriv.h.  */
+
+bool
+dwarf2_psymtab::readin_p (struct objfile *objfile) const
+{
+  if (per_cu_data == nullptr)
+    return true;
+
+  dwarf2_per_objfile *dwarf2_per_objfile
+    = get_dwarf2_per_objfile (objfile);
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu_data->index];
+  return symtab.has_value ();
+}
+
+/* See psympriv.h.  */
+
+compunit_symtab *
+dwarf2_psymtab::get_compunit_symtab (struct objfile *objfile) const
+{
+  if (per_cu_data == nullptr)
+    return nullptr;
+
+  dwarf2_per_objfile *dwarf2_per_objfile
+    = get_dwarf2_per_objfile (objfile);
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu_data->index];
+  if (!symtab.has_value ())
+    return nullptr;
+  return *symtab;
 }
 
 /* Trivial hash function for die_info: the hash value of a DIE
@@ -9413,9 +9492,12 @@ rust_union_quirks (struct dwarf2_cu *cu)
 static struct compunit_symtab *
 get_compunit_symtab (struct dwarf2_per_cu_data *per_cu)
 {
-  return (per_cu->dwarf2_per_objfile->using_index
-	  ? per_cu->v.quick->compunit_symtab
-	  : per_cu->v.psymtab->compunit_symtab);
+  struct dwarf2_per_objfile *dwarf2_per_objfile
+    = per_cu->dwarf2_per_objfile;
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+  gdb_assert (symtab.has_value ());
+  return *symtab;
 }
 
 /* A helper function for computing the list of all symbol tables
@@ -9621,14 +9703,9 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
       cust->call_site_htab = cu->call_site_htab;
     }
 
-  if (dwarf2_per_objfile->using_index)
-    per_cu->v.quick->compunit_symtab = cust;
-  else
-    {
-      dwarf2_psymtab *pst = per_cu->v.psymtab;
-      pst->compunit_symtab = cust;
-      pst->readin = true;
-    }
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+  symtab = cust;
 
   /* Push it for inclusion processing later.  */
   dwarf2_per_objfile->just_read_cus.push_back (per_cu);
@@ -9701,14 +9778,9 @@ process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
       cust = sig_type->type_unit_group->compunit_symtab;
     }
 
-  if (dwarf2_per_objfile->using_index)
-    per_cu->v.quick->compunit_symtab = cust;
-  else
-    {
-      dwarf2_psymtab *pst = per_cu->v.psymtab;
-      pst->compunit_symtab = cust;
-      pst->readin = true;
-    }
+  gdb::optional<compunit_symtab *> &symtab
+    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
+  symtab = cust;
 
   /* Not needed any more.  */
   cu->reset_builder ();
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 5fc7f7f72e5..98d58fb6880 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -82,6 +82,10 @@ struct dwarf2_unshareable
      This is NULL if not allocated yet.
      The mapping is done via (CU/TU + DIE offset) -> type.  */
   htab_up die_type_hash;
+
+  /* Hold the corresponding compunit_symtab for each CU or TU.  This
+     is indexed by dwarf2_per_cu_data::index.  */
+  std::vector<gdb::optional<compunit_symtab *>> symtabs;
 };
 
 /* Collection of data recorded per objfile.
@@ -281,22 +285,26 @@ public:
 dwarf2_per_objfile *get_dwarf2_per_objfile (struct objfile *objfile);
 
 /* A partial symtab specialized for DWARF.  */
-struct dwarf2_psymtab : public standard_psymtab
+struct dwarf2_psymtab : public partial_symtab
 {
   dwarf2_psymtab (const char *filename, struct objfile *objfile)
-    : standard_psymtab (filename, objfile)
+    : partial_symtab (filename, objfile)
   {
   }
 
   dwarf2_psymtab (const char *filename, struct objfile *objfile,
 		  CORE_ADDR addr)
-    : standard_psymtab (filename, objfile, addr)
+    : partial_symtab (filename, objfile, addr)
   {
   }
 
   void read_symtab (struct objfile *) override;
   void expand_psymtab (struct objfile *) override;
 
+  bool readin_p (struct objfile *) const override;
+  struct compunit_symtab *get_compunit_symtab (struct objfile *) const
+    override;
+
   struct dwarf2_per_cu_data *per_cu_data;
 };
 
-- 
2.17.2

Re: [PATCH 08/14] Remove symtab links from dwarf2_psymtab and dwarf2_per_cu_quick_data

[Luis Machado]

A comment below...

On 2/15/20 1:54 PM, Tom Tromey wrote:
> This removes the links from dwarf2_psymtab and
> dwarf2_per_cu_quick_data to the compunit_symtab.  Now, the DWARF code
> uses the index in these objects to find the corresponding symtab in
> the "unshared" object.
> 
> 2020-02-15  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/read.h (struct dwarf2_unshareable) <symtabs>: New
> 	member.
> 	(struct dwarf2_psymtab): Derive from partial_symtab.
> 	<readin_p, get_compunit_symtab>: Declare methods.
> 	* dwarf2/read.c (struct dwarf2_per_cu_quick_data)
> 	<compunit_symtab>: Remove.
> 	(dw2_do_instantiate_symtab, dw2_instantiate_symtab)
> 	(dw2_map_expand_apply, dw2_map_symtabs_matching_filename)
> 	(dw2_symtab_iter_next, dw2_print_stats)
> 	(dw2_expand_symtabs_with_fullname)
> 	(dw2_expand_symtabs_matching_one)
> 	(dw_expand_symtabs_matching_file_matcher)
> 	(dw2_find_pc_sect_compunit_symtab, dw2_map_symbol_filenames)
> 	(dw2_debug_names_iterator::next, does)
> 	(fill_in_sig_entry_from_dwo_entry, dwarf2_psymtab::read_symtab)
> 	(process_queue, dwarf2_psymtab::expand_psymtab): Update.
> 	(dwarf2_psymtab::readin_p, dwarf2_psymtab::get_compunit_symtab):
> 	New methods.
> 	(get_compunit_symtab, process_full_comp_unit)
> 	(process_full_type_unit): Update.
> 	(dwarf2_build_psymtabs): Resize the symtabs vector.
> 	(dwarf2_resize_unshareable): New function.
> 	(dwarf2_initialize_objfile): Call dwarf2_resize_unshareable.
> ---
>   gdb/ChangeLog     |  26 +++++++
>   gdb/dwarf2/read.c | 194 +++++++++++++++++++++++++++++++---------------
>   gdb/dwarf2/read.h |  14 +++-
>   3 files changed, 170 insertions(+), 64 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 281d39ad271..44fdb070e49 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -2205,10 +2205,6 @@ struct dwarf2_per_cu_quick_data
>        NOTE: This points into dwarf2_per_objfile->quick_file_names_table.  */
>     struct quick_file_names *file_names;
>   
> -  /* The corresponding symbol table.  This is NULL if symbols for this
> -     CU have not yet been read.  */
> -  struct compunit_symtab *compunit_symtab;
> -
>     /* A temporary mark bit used when iterating over all CUs in
>        expand_symtabs_matching.  */
>     unsigned int mark : 1;
> @@ -2332,9 +2328,9 @@ dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
>        with the dwarf queue empty.  */
>     dwarf2_queue_guard q_guard (dwarf2_per_objfile);
>   
> -  if (dwarf2_per_objfile->using_index
> -      ? per_cu->v.quick->compunit_symtab == NULL
> -      : (per_cu->v.psymtab == NULL || !per_cu->v.psymtab->readin))
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +  if (!symtab.has_value ())

I noticed the above sequence gets repeated a lot throughout the code. 
Can we turn it into a function/method and reduce the duplication?

There are other sequences that get repeated, but not as often.

>       {
>         queue_comp_unit (per_cu, language_minimal);
>         load_cu (per_cu, skip_partial);
> @@ -2369,7 +2365,10 @@ dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
>     struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;
>   
>     gdb_assert (dwarf2_per_objfile->using_index);
> -  if (!per_cu->v.quick->compunit_symtab)
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +
> +  if (!symtab.has_value ())
>       {
>         free_cached_comp_units freer (dwarf2_per_objfile);
>         scoped_restore decrementer = increment_reading_symtab ();
> @@ -2377,7 +2376,7 @@ dw2_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
>         process_cu_includes (dwarf2_per_objfile);
>       }
>   
> -  return per_cu->v.quick->compunit_symtab;
> +  return *symtab;
>   }
>   
>   /* See declaration.  */
> @@ -3289,7 +3288,11 @@ dw2_map_expand_apply (struct objfile *objfile,
>     struct compunit_symtab *last_made = objfile->compunit_symtabs;
>   
>     /* Don't visit already-expanded CUs.  */
> -  if (per_cu->v.quick->compunit_symtab)
> +  struct dwarf2_per_objfile *dwarf2_per_objfile
> +    = get_dwarf2_per_objfile (objfile);
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +  if (symtab.has_value ())
>       return 0;
>   
>     /* This may expand more than one symtab, and we want to iterate over
> @@ -3317,7 +3320,9 @@ dw2_map_symtabs_matching_filename
>     for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
>       {
>         /* We only need to look at symtabs not already expanded.  */
> -      if (per_cu->v.quick->compunit_symtab)
> +      gdb::optional<compunit_symtab *> &symtab
> +	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +      if (symtab.has_value ())
>   	continue;
>   
>         quick_file_names *file_data = dw2_get_file_names (per_cu);
> @@ -3458,7 +3463,9 @@ dw2_symtab_iter_next (struct dw2_symtab_iterator *iter)
>         dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (cu_index);
>   
>         /* Skip if already read in.  */
> -      if (per_cu->v.quick->compunit_symtab)
> +      gdb::optional<compunit_symtab *> &symtab
> +	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +      if (symtab.has_value ())
>   	continue;
>   
>         /* Check static vs global.  */
> @@ -3573,7 +3580,9 @@ dw2_print_stats (struct objfile *objfile)
>       {
>         dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
>   
> -      if (!per_cu->v.quick->compunit_symtab)
> +      gdb::optional<compunit_symtab *> &symtab
> +	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +      if (!symtab.has_value ())
>   	++count;
>       }
>     printf_filtered (_("  Number of read CUs: %d\n"), total - count);
> @@ -3656,7 +3665,9 @@ dw2_expand_symtabs_with_fullname (struct objfile *objfile,
>     for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
>       {
>         /* We only need to look at symtabs not already expanded.  */
> -      if (per_cu->v.quick->compunit_symtab)
> +      gdb::optional<compunit_symtab *> &symtab
> +	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +      if (symtab.has_value ())
>   	continue;
>   
>         quick_file_names *file_data = dw2_get_file_names (per_cu);
> @@ -4455,15 +4466,19 @@ dw2_expand_symtabs_matching_one
>   {
>     if (file_matcher == NULL || per_cu->v.quick->mark)
>       {
> -      bool symtab_was_null
> -	= (per_cu->v.quick->compunit_symtab == NULL);
> +      struct dwarf2_per_objfile *dwarf2_per_objfile
> +	= per_cu->dwarf2_per_objfile;
> +      gdb::optional<compunit_symtab *> &symtab
> +	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +      bool symtab_was_null = !symtab.has_value () || *symtab == nullptr;
>   
>         dw2_instantiate_symtab (per_cu, false);
>   
>         if (expansion_notify != NULL
>   	  && symtab_was_null
> -	  && per_cu->v.quick->compunit_symtab != NULL)
> -	expansion_notify (per_cu->v.quick->compunit_symtab);
> +	  && symtab.has_value ()
> +	  && *symtab != nullptr)
> +	expansion_notify (*symtab);
>       }
>   }
>   
> @@ -4583,7 +4598,9 @@ dw_expand_symtabs_matching_file_matcher
>         per_cu->v.quick->mark = 0;
>   
>         /* We only need to look at symtabs not already expanded.  */
> -      if (per_cu->v.quick->compunit_symtab)
> +      gdb::optional<compunit_symtab *> &symtab
> +	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +      if (symtab.has_value ())
>   	continue;
>   
>         quick_file_names *file_data = dw2_get_file_names (per_cu);
> @@ -4708,7 +4725,11 @@ dw2_find_pc_sect_compunit_symtab (struct objfile *objfile,
>     if (!data)
>       return NULL;
>   
> -  if (warn_if_readin && data->v.quick->compunit_symtab)
> +  struct dwarf2_per_objfile *dwarf2_per_objfile
> +    = get_dwarf2_per_objfile (objfile);
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[data->index];
> +  if (warn_if_readin && symtab.has_value ())
>       warning (_("(Internal error: pc %s in read in CU, but not in symtab.)"),
>   	     paddress (get_objfile_arch (objfile), pc));
>   
> @@ -4741,7 +4762,9 @@ dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
>   
>         for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
>   	{
> -	  if (per_cu->v.quick->compunit_symtab)
> +	  gdb::optional<compunit_symtab *> &symtab
> +	    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +	  if (symtab.has_value ())
>   	    {
>   	      void **slot = htab_find_slot (visited.get (),
>   					    per_cu->v.quick->file_names,
> @@ -4754,7 +4777,9 @@ dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
>         for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
>   	{
>   	  /* We only need to look at symtabs not already expanded.  */
> -	  if (per_cu->v.quick->compunit_symtab)
> +	  gdb::optional<compunit_symtab *> &symtab
> +	    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +	  if (symtab.has_value ())
>   	    continue;
>   
>   	  quick_file_names *file_data = dw2_get_file_names (per_cu);
> @@ -5386,7 +5411,9 @@ dw2_debug_names_iterator::next ()
>       }
>   
>     /* Skip if already read in.  */
> -  if (per_cu->v.quick->compunit_symtab)
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +  if (symtab.has_value ())
>       goto again;
>   
>     /* Check static vs global.  */
> @@ -5627,11 +5654,12 @@ dw2_debug_names_map_matching_symbols
>        the psymtab code does.  */
>     for (dwarf2_per_cu_data *per_cu : dwarf2_per_objfile->all_comp_units)
>       {
> -      struct compunit_symtab *cust = per_cu->v.quick->compunit_symtab;
> -      if (cust != nullptr)
> +      gdb::optional<compunit_symtab *> &symtab
> +	= dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +      if (symtab.has_value () && *symtab != nullptr)
>   	{
>   	  const struct block *block
> -	    = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (cust), block_kind);
> +	    = BLOCKVECTOR_BLOCK (COMPUNIT_BLOCKVECTOR (*symtab), block_kind);
>   	  if (!iterate_over_symbols_terminated (block, name,
>   						domain, callback))
>   	    break;
> @@ -5748,6 +5776,15 @@ get_gdb_index_contents_from_cache_dwz (objfile *obj, dwz_file *dwz)
>     return global_index_cache.lookup_gdb_index (build_id, &dwz->index_cache_res);
>   }
>   
> +/* Make sure we have space for the compunit_symtabs we may need.  */
> +
> +static void
> +dwarf2_resize_unshareable (dwarf2_per_objfile *dwarf2_per_objfile)
> +{
> +  dwarf2_per_objfile->unshareable->symtabs.resize
> +    (dwarf2_per_objfile->num_psymtabs);
> +}
> +
>   /* See symfile.h.  */
>   
>   bool
> @@ -5768,6 +5805,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
>         dwarf2_per_objfile->quick_file_names_table
>   	= create_quick_file_names_table
>   	    (dwarf2_per_objfile->all_comp_units.size ());
> +      dwarf2_resize_unshareable (dwarf2_per_objfile);
>   
>         for (int i = 0; i < (dwarf2_per_objfile->all_comp_units.size ()
>   			   + dwarf2_per_objfile->all_type_units.size ()); ++i)
> @@ -5788,6 +5826,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
>     if (dwarf2_read_debug_names (dwarf2_per_objfile))
>       {
>         *index_kind = dw_index_kind::DEBUG_NAMES;
> +      dwarf2_resize_unshareable (dwarf2_per_objfile);
>         return true;
>       }
>   
> @@ -5796,6 +5835,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
>   			     get_gdb_index_contents_from_section<dwz_file>))
>       {
>         *index_kind = dw_index_kind::GDB_INDEX;
> +      dwarf2_resize_unshareable (dwarf2_per_objfile);
>         return true;
>       }
>   
> @@ -5806,6 +5846,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
>       {
>         global_index_cache.hit ();
>         *index_kind = dw_index_kind::GDB_INDEX;
> +      dwarf2_resize_unshareable (dwarf2_per_objfile);
>         return true;
>       }
>   
> @@ -5834,6 +5875,8 @@ dwarf2_build_psymtabs (struct objfile *objfile)
>         dwarf2_build_psymtabs_hard (dwarf2_per_objfile);
>         psymtabs.keep ();
>   
> +      dwarf2_resize_unshareable (dwarf2_per_objfile);
> +
>         /* (maybe) store an index in the cache.  */
>         global_index_cache.store (dwarf2_per_objfile);
>       }
> @@ -6259,7 +6302,7 @@ fill_in_sig_entry_from_dwo_entry (struct dwarf2_per_objfile *dwarf2_per_objfile,
>     if (dwarf2_per_objfile->using_index)
>       {
>         gdb_assert (sig_entry->per_cu.v.quick != NULL);
> -      gdb_assert (sig_entry->per_cu.v.quick->compunit_symtab == NULL);
> +      gdb_assert (!dwarf2_per_objfile->unshareable->symtabs[sig_entry->per_cu.index].has_value ());
>       }
>     else
>         gdb_assert (sig_entry->per_cu.v.psymtab == NULL);
> @@ -8700,7 +8743,10 @@ dwarf2_psymtab::read_symtab (struct objfile *objfile)
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> -  gdb_assert (!readin);
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu_data->index];
> +  gdb_assert (!symtab.has_value ());
> +
>     /* If this psymtab is constructed from a debug-only objfile, the
>        has_section_at_zero flag will not necessarily be correct.  We
>        can get the correct value for this flag by looking at the data
> @@ -8796,9 +8842,9 @@ process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile)
>       {
>         dwarf2_queue_item &item = dwarf2_per_objfile->queue.front ();
>   
> -      if ((dwarf2_per_objfile->using_index
> -	   ? !item.per_cu->v.quick->compunit_symtab
> -	   : (item.per_cu->v.psymtab && !item.per_cu->v.psymtab->readin))
> +      gdb::optional<compunit_symtab *> &symtab
> +	= dwarf2_per_objfile->unshareable->symtabs[item.per_cu->index];
> +      if (!symtab.has_value ()
>   	  /* Skip dummy CUs.  */
>   	  && item.per_cu->cu != NULL)
>   	{
> @@ -8853,24 +8899,57 @@ process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile)
>   void
>   dwarf2_psymtab::expand_psymtab (struct objfile *objfile)
>   {
> -  struct dwarf2_per_cu_data *per_cu;
> -
> -  if (readin)
> -    return;
> -
> -  read_dependencies (objfile);
> -
> -  per_cu = per_cu_data;
> +  struct dwarf2_per_objfile *dwarf2_per_objfile
> +    = get_dwarf2_per_objfile (objfile);
>   
> -  if (per_cu == NULL)
> +  if (per_cu_data == NULL)
>       {
>         /* It's an include file, no symbols to read for it.
>            Everything is in the parent symtab.  */
> -      readin = true;
>         return;
>       }
>   
> -  dw2_do_instantiate_symtab (per_cu, false);
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu_data->index];
> +
> +  if (symtab.has_value ())
> +    return;
> +
> +  read_dependencies (objfile);
> +
> +  dw2_do_instantiate_symtab (per_cu_data, false);
> +}
> +
> +/* See psympriv.h.  */
> +
> +bool
> +dwarf2_psymtab::readin_p (struct objfile *objfile) const
> +{
> +  if (per_cu_data == nullptr)
> +    return true;
> +
> +  dwarf2_per_objfile *dwarf2_per_objfile
> +    = get_dwarf2_per_objfile (objfile);
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu_data->index];
> +  return symtab.has_value ();
> +}
> +
> +/* See psympriv.h.  */
> +
> +compunit_symtab *
> +dwarf2_psymtab::get_compunit_symtab (struct objfile *objfile) const
> +{
> +  if (per_cu_data == nullptr)
> +    return nullptr;
> +
> +  dwarf2_per_objfile *dwarf2_per_objfile
> +    = get_dwarf2_per_objfile (objfile);
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu_data->index];
> +  if (!symtab.has_value ())
> +    return nullptr;
> +  return *symtab;
>   }
>   
>   /* Trivial hash function for die_info: the hash value of a DIE
> @@ -9413,9 +9492,12 @@ rust_union_quirks (struct dwarf2_cu *cu)
>   static struct compunit_symtab *
>   get_compunit_symtab (struct dwarf2_per_cu_data *per_cu)
>   {
> -  return (per_cu->dwarf2_per_objfile->using_index
> -	  ? per_cu->v.quick->compunit_symtab
> -	  : per_cu->v.psymtab->compunit_symtab);
> +  struct dwarf2_per_objfile *dwarf2_per_objfile
> +    = per_cu->dwarf2_per_objfile;
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +  gdb_assert (symtab.has_value ());
> +  return *symtab;
>   }
>   
>   /* A helper function for computing the list of all symbol tables
> @@ -9621,14 +9703,9 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
>         cust->call_site_htab = cu->call_site_htab;
>       }
>   
> -  if (dwarf2_per_objfile->using_index)
> -    per_cu->v.quick->compunit_symtab = cust;
> -  else
> -    {
> -      dwarf2_psymtab *pst = per_cu->v.psymtab;
> -      pst->compunit_symtab = cust;
> -      pst->readin = true;
> -    }
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +  symtab = cust;
>   
>     /* Push it for inclusion processing later.  */
>     dwarf2_per_objfile->just_read_cus.push_back (per_cu);
> @@ -9701,14 +9778,9 @@ process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
>         cust = sig_type->type_unit_group->compunit_symtab;
>       }
>   
> -  if (dwarf2_per_objfile->using_index)
> -    per_cu->v.quick->compunit_symtab = cust;
> -  else
> -    {
> -      dwarf2_psymtab *pst = per_cu->v.psymtab;
> -      pst->compunit_symtab = cust;
> -      pst->readin = true;
> -    }
> +  gdb::optional<compunit_symtab *> &symtab
> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
> +  symtab = cust;
>   
>     /* Not needed any more.  */
>     cu->reset_builder ();
> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index 5fc7f7f72e5..98d58fb6880 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -82,6 +82,10 @@ struct dwarf2_unshareable
>        This is NULL if not allocated yet.
>        The mapping is done via (CU/TU + DIE offset) -> type.  */
>     htab_up die_type_hash;
> +
> +  /* Hold the corresponding compunit_symtab for each CU or TU.  This
> +     is indexed by dwarf2_per_cu_data::index.  */
> +  std::vector<gdb::optional<compunit_symtab *>> symtabs;
>   };
>   
>   /* Collection of data recorded per objfile.
> @@ -281,22 +285,26 @@ public:
>   dwarf2_per_objfile *get_dwarf2_per_objfile (struct objfile *objfile);
>   
>   /* A partial symtab specialized for DWARF.  */
> -struct dwarf2_psymtab : public standard_psymtab
> +struct dwarf2_psymtab : public partial_symtab
>   {
>     dwarf2_psymtab (const char *filename, struct objfile *objfile)
> -    : standard_psymtab (filename, objfile)
> +    : partial_symtab (filename, objfile)
>     {
>     }
>   
>     dwarf2_psymtab (const char *filename, struct objfile *objfile,
>   		  CORE_ADDR addr)
> -    : standard_psymtab (filename, objfile, addr)
> +    : partial_symtab (filename, objfile, addr)
>     {
>     }
>   
>     void read_symtab (struct objfile *) override;
>     void expand_psymtab (struct objfile *) override;
>   
> +  bool readin_p (struct objfile *) const override;
> +  struct compunit_symtab *get_compunit_symtab (struct objfile *) const
> +    override;
> +
>     struct dwarf2_per_cu_data *per_cu_data;
>   };
>   
> 

Re: [PATCH 08/14] Remove symtab links from dwarf2_psymtab and dwarf2_per_cu_quick_data

[Simon Marchi]

>> @@ -2332,9 +2328,9 @@ dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
>>        with the dwarf queue empty.  */
>>     dwarf2_queue_guard q_guard (dwarf2_per_objfile);
>>   
>> -  if (dwarf2_per_objfile->using_index
>> -      ? per_cu->v.quick->compunit_symtab == NULL
>> -      : (per_cu->v.psymtab == NULL || !per_cu->v.psymtab->readin))
>> +  gdb::optional<compunit_symtab *> &symtab
>> +    = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
>> +  if (!symtab.has_value ())
> 
> I noticed the above sequence gets repeated a lot throughout the code. 
> Can we turn it into a function/method and reduce the duplication?

I was going to say the same thing, a getter on dwarf2_unshareable would be nice.  Since this
is a new data type, we might as well design it right from the start.

>> @@ -82,6 +82,10 @@ struct dwarf2_unshareable
>>        This is NULL if not allocated yet.
>>        The mapping is done via (CU/TU + DIE offset) -> type.  */
>>     htab_up die_type_hash;
>> +
>> +  /* Hold the corresponding compunit_symtab for each CU or TU.  This
>> +     is indexed by dwarf2_per_cu_data::index.  */
>> +  std::vector<gdb::optional<compunit_symtab *>> symtabs;

Having pointers in an optional gives us three states:

1- not instantiated
2- instantiated but NULL
3- instantiated and not NULL

Do we actually need the distinction between 1 and 2 here?  If not, I think the optional
doesn't bring much and just makes the code more complicated.

Simon

Re: [PATCH 08/14] Remove symtab links from dwarf2_psymtab and dwarf2_per_cu_quick_data

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I was going to say the same thing, a getter on dwarf2_unshareable
Simon> would be nice.  Since this is a new data type, we might as well
Simon> design it right from the start.

I ended up with a method on dwarf2_per_objfile.

>>> +  std::vector<gdb::optional<compunit_symtab *>> symtabs;

Simon> Having pointers in an optional gives us three states:

Simon> 1- not instantiated
Simon> 2- instantiated but NULL
Simon> 3- instantiated and not NULL

Simon> Do we actually need the distinction between 1 and 2 here?  If not, I think the optional
Simon> doesn't bring much and just makes the code more complicated.

Yes, it's weird; but I did this to preserve the status quo.
It's possible that this is not needed but figuring that out seemed like
a separate effort.

It's possible that this can only happen for include psymtabs.
I plan to make a separate subclass for these, which would clarify this
somewhat.  But I'd rather get this series in first.

Tom

Re: [PATCH 08/14] Remove symtab links from dwarf2_psymtab and dwarf2_per_cu_quick_data

[Tom Tromey]

>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:

Luis> I noticed the above sequence gets repeated a lot throughout the
Luis> code. Can we turn it into a function/method and reduce the
Luis> duplication?

I did this.

Tom

[PATCH 06/14] Add "objfile" parameter to two partial_symtab methods

[Tom Tromey]

This series will cause partial symtabs to be shared across obfiles.
However, full symtabs and symbols will still be objfile-dependent.
So, a debug info reader will need to know which objfile to consult
when expanding a partial symtab.

This patch adds an objfile parameter to the two relevant methods
partial_symtab.  Current implementations simply ignore them.

gdb/ChangeLog
2020-02-15  Tom Tromey  <tom@tromey.com>

	* psymtab.c (partial_map_expand_apply)
	(psym_find_pc_sect_compunit_symtab, psym_lookup_symbol)
	(psymtab_to_symtab, psym_find_last_source_symtab, dump_psymtab)
	(psym_print_stats, psym_expand_symtabs_for_function)
	(psym_map_symbol_filenames, psym_map_matching_symbols)
	(psym_expand_symtabs_matching)
	(partial_symtab::read_dependencies, maintenance_info_psymtabs)
	(maintenance_check_psymtabs): Update.
	* psympriv.h (struct partial_symtab) <readin_p,
	get_compunit_symtab>: Add objfile parameter.
	(struct standard_psymtab) <readin_p, get_compunit_symtab>:
	Likewise.
---
 gdb/ChangeLog  | 15 +++++++++++++++
 gdb/psympriv.h | 10 +++++-----
 gdb/psymtab.c  | 40 ++++++++++++++++++++--------------------
 3 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/gdb/psympriv.h b/gdb/psympriv.h
index 1b1f57cf764..0270f0bb9b2 100644
--- a/gdb/psympriv.h
+++ b/gdb/psympriv.h
@@ -139,12 +139,12 @@ struct partial_symtab
 
   /* Return true if the symtab corresponding to this psymtab has been
      readin.  */
-  virtual bool readin_p () const = 0;
+  virtual bool readin_p (struct objfile *) const = 0;
 
   /* Return a pointer to the compunit allocated for this source file.
      Return nullptr if !readin or if there was no symtab.  */
-  virtual struct compunit_symtab *get_compunit_symtab () const = 0;
-
+  virtual struct compunit_symtab *get_compunit_symtab (struct objfile *) const
+    = 0;
 
   /* Return the raw low text address of this partial_symtab.  */
   CORE_ADDR raw_text_low () const
@@ -310,14 +310,14 @@ struct standard_psymtab : public partial_symtab
   {
   }
 
-  bool readin_p () const override
+  bool readin_p (struct objfile *) const override
   {
     return readin;
   }
 
   /* Return a pointer to the compunit allocated for this source file.
      Return nullptr if !readin or if there was no symtab.  */
-  struct compunit_symtab *get_compunit_symtab () const override
+  struct compunit_symtab *get_compunit_symtab (struct objfile *) const override
   {
     return compunit_symtab;
   }
diff --git a/gdb/psymtab.c b/gdb/psymtab.c
index 56576b3bcec..5c23ebfd94c 100644
--- a/gdb/psymtab.c
+++ b/gdb/psymtab.c
@@ -131,7 +131,7 @@ partial_map_expand_apply (struct objfile *objfile,
   gdb_assert (pst->user == NULL);
 
   /* Don't visit already-expanded psymtabs.  */
-  if (pst->readin_p ())
+  if (pst->readin_p (objfile))
     return 0;
 
   /* This may expand more than one symtab, and we want to iterate over
@@ -385,7 +385,7 @@ psym_find_pc_sect_compunit_symtab (struct objfile *objfile,
 						    msymbol);
   if (ps != NULL)
     {
-      if (warn_if_readin && ps->readin_p ())
+      if (warn_if_readin && ps->readin_p (objfile))
 	/* Might want to error() here (in case symtab is corrupt and
 	   will cause a core dump), but maybe we can successfully
 	   continue, so let's not.  */
@@ -393,7 +393,7 @@ psym_find_pc_sect_compunit_symtab (struct objfile *objfile,
 (Internal error: pc %s in read in psymtab, but not in symtab.)\n"),
 		 paddress (get_objfile_arch (objfile), pc));
       psymtab_to_symtab (objfile, ps);
-      return ps->get_compunit_symtab ();
+      return ps->get_compunit_symtab (objfile);
     }
   return NULL;
 }
@@ -484,8 +484,8 @@ psym_lookup_symbol (struct objfile *objfile,
 
   for (partial_symtab *ps : require_partial_symbols (objfile, true))
     {
-      if (!ps->readin_p () && lookup_partial_symbol (objfile, ps, name,
-						     psymtab_index, domain))
+      if (!ps->readin_p (objfile)
+	  && lookup_partial_symbol (objfile, ps, name, psymtab_index, domain))
 	{
 	  struct symbol *sym, *with_opaque = NULL;
 	  struct compunit_symtab *stab = psymtab_to_symtab (objfile, ps);
@@ -750,11 +750,11 @@ psymtab_to_symtab (struct objfile *objfile, struct partial_symtab *pst)
     pst = pst->user;
 
   /* If it's been looked up before, return it.  */
-  if (pst->get_compunit_symtab ())
-    return pst->get_compunit_symtab ();
+  if (pst->get_compunit_symtab (objfile))
+    return pst->get_compunit_symtab (objfile);
 
   /* If it has not yet been read in, read it.  */
-  if (!pst->readin_p ())
+  if (!pst->readin_p (objfile))
     {
       scoped_restore decrementer = increment_reading_symtab ();
 
@@ -772,7 +772,7 @@ psymtab_to_symtab (struct objfile *objfile, struct partial_symtab *pst)
 	printf_filtered (_("done.\n"));
     }
 
-  return pst->get_compunit_symtab ();
+  return pst->get_compunit_symtab (objfile);
 }
 
 /* Psymtab version of find_last_source_symtab.  See its definition in
@@ -795,7 +795,7 @@ psym_find_last_source_symtab (struct objfile *ofp)
 
   if (cs_pst)
     {
-      if (cs_pst->readin_p ())
+      if (cs_pst->readin_p (ofp))
 	{
 	  internal_error (__FILE__, __LINE__,
 			  _("select_source_symtab: "
@@ -952,11 +952,11 @@ dump_psymtab (struct objfile *objfile, struct partial_symtab *psymtab,
   gdb_print_host_address (objfile, outfile);
   fprintf_filtered (outfile, ")\n");
 
-  if (psymtab->readin_p ())
+  if (psymtab->readin_p (objfile))
     {
       fprintf_filtered (outfile,
 			"  Full symtab was read (at ");
-      gdb_print_host_address (psymtab->get_compunit_symtab (), outfile);
+      gdb_print_host_address (psymtab->get_compunit_symtab (objfile), outfile);
       fprintf_filtered (outfile, ")\n");
     }
 
@@ -1010,7 +1010,7 @@ psym_print_stats (struct objfile *objfile)
   i = 0;
   for (partial_symtab *ps : require_partial_symbols (objfile, true))
     {
-      if (!ps->readin_p ())
+      if (!ps->readin_p (objfile))
 	i++;
     }
   printf_filtered (_("  Number of psym tables (not yet expanded): %d\n"), i);
@@ -1050,7 +1050,7 @@ psym_expand_symtabs_for_function (struct objfile *objfile,
 {
   for (partial_symtab *ps : require_partial_symbols (objfile, true))
     {
-      if (ps->readin_p ())
+      if (ps->readin_p (objfile))
 	continue;
 
       if ((lookup_partial_symbol (objfile, ps, func_name, 1, VAR_DOMAIN)
@@ -1105,7 +1105,7 @@ psym_map_symbol_filenames (struct objfile *objfile,
     {
       const char *fullname;
 
-      if (ps->readin_p ())
+      if (ps->readin_p (objfile))
 	continue;
 
       /* We can skip shared psymtabs here, because any file name will be
@@ -1185,7 +1185,7 @@ psym_map_matching_symbols
   for (partial_symtab *ps : require_partial_symbols (objfile, true))
     {
       QUIT;
-      if (ps->readin_p ()
+      if (ps->readin_p (objfile)
 	  || match_partial_symbol (objfile, ps, global, name, domain,
 				   ordered_compare))
 	{
@@ -1320,7 +1320,7 @@ psym_expand_symtabs_matching
     {
       QUIT;
 
-      if (ps->readin_p ())
+      if (ps->readin_p (objfile))
 	continue;
 
       /* We skip shared psymtabs because file-matching doesn't apply
@@ -1688,7 +1688,7 @@ partial_symtab::read_dependencies (struct objfile *objfile)
 {
   for (int i = 0; i < number_of_dependencies; ++i)
     {
-      if (!dependencies[i]->readin_p ())
+      if (!dependencies[i]->readin_p (objfile))
 	{
 	  /* Inform about additional files to be read in.  */
 	  if (info_verbose)
@@ -2000,7 +2000,7 @@ maintenance_info_psymtabs (const char *regexp, int from_tty)
 				 host_address_to_string (psymtab));
 
 		printf_filtered ("    readin %s\n",
-				 psymtab->readin_p () ? "yes" : "no");
+				 psymtab->readin_p (objfile) ? "yes" : "no");
 		printf_filtered ("    fullname %s\n",
 				 psymtab->fullname
 				 ? psymtab->fullname : "(null)");
@@ -2091,7 +2091,7 @@ maintenance_check_psymtabs (const char *ignore, int from_tty)
 	/* We don't call psymtab_to_symtab here because that may cause symtab
 	   expansion.  When debugging a problem it helps if checkers leave
 	   things unchanged.  */
-	cust = ps->get_compunit_symtab ();
+	cust = ps->get_compunit_symtab (objfile);
 
 	/* First do some checks that don't require the associated symtab.  */
 	if (ps->text_high (objfile) < ps->text_low (objfile))
-- 
2.17.2

Re: [PATCH 06/14] Add "objfile" parameter to two partial_symtab methods

[Luis Machado]

On 2/15/20 1:54 PM, Tom Tromey wrote:
> This series will cause partial symtabs to be shared across obfiles.

"across objfiles."

> However, full symtabs and symbols will still be objfile-dependent.
> So, a debug info reader will need to know which objfile to consult
> when expanding a partial symtab.
> 
> This patch adds an objfile parameter to the two relevant methods
> partial_symtab.  Current implementations simply ignore them.
> 
> gdb/ChangeLog
> 2020-02-15  Tom Tromey  <tom@tromey.com>
> 
> 	* psymtab.c (partial_map_expand_apply)
> 	(psym_find_pc_sect_compunit_symtab, psym_lookup_symbol)
> 	(psymtab_to_symtab, psym_find_last_source_symtab, dump_psymtab)
> 	(psym_print_stats, psym_expand_symtabs_for_function)
> 	(psym_map_symbol_filenames, psym_map_matching_symbols)
> 	(psym_expand_symtabs_matching)
> 	(partial_symtab::read_dependencies, maintenance_info_psymtabs)
> 	(maintenance_check_psymtabs): Update.
> 	* psympriv.h (struct partial_symtab) <readin_p,
> 	get_compunit_symtab>: Add objfile parameter.
> 	(struct standard_psymtab) <readin_p, get_compunit_symtab>:
> 	Likewise.
> ---
>   gdb/ChangeLog  | 15 +++++++++++++++
>   gdb/psympriv.h | 10 +++++-----
>   gdb/psymtab.c  | 40 ++++++++++++++++++++--------------------
>   3 files changed, 40 insertions(+), 25 deletions(-)
> 
> diff --git a/gdb/psympriv.h b/gdb/psympriv.h
> index 1b1f57cf764..0270f0bb9b2 100644
> --- a/gdb/psympriv.h
> +++ b/gdb/psympriv.h
> @@ -139,12 +139,12 @@ struct partial_symtab
>   
>     /* Return true if the symtab corresponding to this psymtab has been
>        readin.  */
> -  virtual bool readin_p () const = 0;
> +  virtual bool readin_p (struct objfile *) const = 0;
>   
>     /* Return a pointer to the compunit allocated for this source file.
>        Return nullptr if !readin or if there was no symtab.  */
> -  virtual struct compunit_symtab *get_compunit_symtab () const = 0;
> -
> +  virtual struct compunit_symtab *get_compunit_symtab (struct objfile *) const
> +    = 0;
>   
>     /* Return the raw low text address of this partial_symtab.  */
>     CORE_ADDR raw_text_low () const
> @@ -310,14 +310,14 @@ struct standard_psymtab : public partial_symtab
>     {
>     }
>   
> -  bool readin_p () const override
> +  bool readin_p (struct objfile *) const override
>     {
>       return readin;
>     }
>   
>     /* Return a pointer to the compunit allocated for this source file.
>        Return nullptr if !readin or if there was no symtab.  */
> -  struct compunit_symtab *get_compunit_symtab () const override
> +  struct compunit_symtab *get_compunit_symtab (struct objfile *) const override
>     {
>       return compunit_symtab;
>     }
> diff --git a/gdb/psymtab.c b/gdb/psymtab.c
> index 56576b3bcec..5c23ebfd94c 100644
> --- a/gdb/psymtab.c
> +++ b/gdb/psymtab.c
> @@ -131,7 +131,7 @@ partial_map_expand_apply (struct objfile *objfile,
>     gdb_assert (pst->user == NULL);
>   
>     /* Don't visit already-expanded psymtabs.  */
> -  if (pst->readin_p ())
> +  if (pst->readin_p (objfile))
>       return 0;
>   
>     /* This may expand more than one symtab, and we want to iterate over
> @@ -385,7 +385,7 @@ psym_find_pc_sect_compunit_symtab (struct objfile *objfile,
>   						    msymbol);
>     if (ps != NULL)
>       {
> -      if (warn_if_readin && ps->readin_p ())
> +      if (warn_if_readin && ps->readin_p (objfile))
>   	/* Might want to error() here (in case symtab is corrupt and
>   	   will cause a core dump), but maybe we can successfully
>   	   continue, so let's not.  */
> @@ -393,7 +393,7 @@ psym_find_pc_sect_compunit_symtab (struct objfile *objfile,
>   (Internal error: pc %s in read in psymtab, but not in symtab.)\n"),
>   		 paddress (get_objfile_arch (objfile), pc));
>         psymtab_to_symtab (objfile, ps);
> -      return ps->get_compunit_symtab ();
> +      return ps->get_compunit_symtab (objfile);
>       }
>     return NULL;
>   }
> @@ -484,8 +484,8 @@ psym_lookup_symbol (struct objfile *objfile,
>   
>     for (partial_symtab *ps : require_partial_symbols (objfile, true))
>       {
> -      if (!ps->readin_p () && lookup_partial_symbol (objfile, ps, name,
> -						     psymtab_index, domain))
> +      if (!ps->readin_p (objfile)
> +	  && lookup_partial_symbol (objfile, ps, name, psymtab_index, domain))
>   	{
>   	  struct symbol *sym, *with_opaque = NULL;
>   	  struct compunit_symtab *stab = psymtab_to_symtab (objfile, ps);
> @@ -750,11 +750,11 @@ psymtab_to_symtab (struct objfile *objfile, struct partial_symtab *pst)
>       pst = pst->user;
>   
>     /* If it's been looked up before, return it.  */
> -  if (pst->get_compunit_symtab ())
> -    return pst->get_compunit_symtab ();
> +  if (pst->get_compunit_symtab (objfile))
> +    return pst->get_compunit_symtab (objfile);
>   
>     /* If it has not yet been read in, read it.  */
> -  if (!pst->readin_p ())
> +  if (!pst->readin_p (objfile))
>       {
>         scoped_restore decrementer = increment_reading_symtab ();
>   
> @@ -772,7 +772,7 @@ psymtab_to_symtab (struct objfile *objfile, struct partial_symtab *pst)
>   	printf_filtered (_("done.\n"));
>       }
>   
> -  return pst->get_compunit_symtab ();
> +  return pst->get_compunit_symtab (objfile);
>   }
>   
>   /* Psymtab version of find_last_source_symtab.  See its definition in
> @@ -795,7 +795,7 @@ psym_find_last_source_symtab (struct objfile *ofp)
>   
>     if (cs_pst)
>       {
> -      if (cs_pst->readin_p ())
> +      if (cs_pst->readin_p (ofp))
>   	{
>   	  internal_error (__FILE__, __LINE__,
>   			  _("select_source_symtab: "
> @@ -952,11 +952,11 @@ dump_psymtab (struct objfile *objfile, struct partial_symtab *psymtab,
>     gdb_print_host_address (objfile, outfile);
>     fprintf_filtered (outfile, ")\n");
>   
> -  if (psymtab->readin_p ())
> +  if (psymtab->readin_p (objfile))
>       {
>         fprintf_filtered (outfile,
>   			"  Full symtab was read (at ");
> -      gdb_print_host_address (psymtab->get_compunit_symtab (), outfile);
> +      gdb_print_host_address (psymtab->get_compunit_symtab (objfile), outfile);
>         fprintf_filtered (outfile, ")\n");
>       }
>   
> @@ -1010,7 +1010,7 @@ psym_print_stats (struct objfile *objfile)
>     i = 0;
>     for (partial_symtab *ps : require_partial_symbols (objfile, true))
>       {
> -      if (!ps->readin_p ())
> +      if (!ps->readin_p (objfile))
>   	i++;
>       }
>     printf_filtered (_("  Number of psym tables (not yet expanded): %d\n"), i);
> @@ -1050,7 +1050,7 @@ psym_expand_symtabs_for_function (struct objfile *objfile,
>   {
>     for (partial_symtab *ps : require_partial_symbols (objfile, true))
>       {
> -      if (ps->readin_p ())
> +      if (ps->readin_p (objfile))
>   	continue;
>   
>         if ((lookup_partial_symbol (objfile, ps, func_name, 1, VAR_DOMAIN)
> @@ -1105,7 +1105,7 @@ psym_map_symbol_filenames (struct objfile *objfile,
>       {
>         const char *fullname;
>   
> -      if (ps->readin_p ())
> +      if (ps->readin_p (objfile))
>   	continue;
>   
>         /* We can skip shared psymtabs here, because any file name will be
> @@ -1185,7 +1185,7 @@ psym_map_matching_symbols
>     for (partial_symtab *ps : require_partial_symbols (objfile, true))
>       {
>         QUIT;
> -      if (ps->readin_p ()
> +      if (ps->readin_p (objfile)
>   	  || match_partial_symbol (objfile, ps, global, name, domain,
>   				   ordered_compare))
>   	{
> @@ -1320,7 +1320,7 @@ psym_expand_symtabs_matching
>       {
>         QUIT;
>   
> -      if (ps->readin_p ())
> +      if (ps->readin_p (objfile))
>   	continue;
>   
>         /* We skip shared psymtabs because file-matching doesn't apply
> @@ -1688,7 +1688,7 @@ partial_symtab::read_dependencies (struct objfile *objfile)
>   {
>     for (int i = 0; i < number_of_dependencies; ++i)
>       {
> -      if (!dependencies[i]->readin_p ())
> +      if (!dependencies[i]->readin_p (objfile))
>   	{
>   	  /* Inform about additional files to be read in.  */
>   	  if (info_verbose)
> @@ -2000,7 +2000,7 @@ maintenance_info_psymtabs (const char *regexp, int from_tty)
>   				 host_address_to_string (psymtab));
>   
>   		printf_filtered ("    readin %s\n",
> -				 psymtab->readin_p () ? "yes" : "no");
> +				 psymtab->readin_p (objfile) ? "yes" : "no");
>   		printf_filtered ("    fullname %s\n",
>   				 psymtab->fullname
>   				 ? psymtab->fullname : "(null)");
> @@ -2091,7 +2091,7 @@ maintenance_check_psymtabs (const char *ignore, int from_tty)
>   	/* We don't call psymtab_to_symtab here because that may cause symtab
>   	   expansion.  When debugging a problem it helps if checkers leave
>   	   things unchanged.  */
> -	cust = ps->get_compunit_symtab ();
> +	cust = ps->get_compunit_symtab (objfile);
>   
>   	/* First do some checks that don't require the associated symtab.  */
>   	if (ps->text_high (objfile) < ps->text_low (objfile))
> 

[PATCH 11/14] Split type_unit_group

[Tom Tromey]

type_unit_group has links to the compunit_symtab and other symtabs.
However, once this object is shared across objfiles, this will no
longer be ok.

This patch introduces a new type_unit_unshareable and arranges to
store a map from type unit groups to type_unit_unshareable objects on
the DWARF unshareable object.

2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.h (struct type_unit_unshareable): New.
	(struct dwarf2_unshareable) <type_units>: New member.
	* dwarf2/read.c (struct type_unit_group) <compunit_symtab,
	num_symtabs, symtabs>: Remove; move to type_unit_unshareable.
	(get_type_unit_group_unshareable): New function.
	(process_full_type_unit, dwarf2_cu::setup_type_unit_groups)
	(dwarf2_cu::setup_type_unit_groups): Use type_unit_unshareable.
---
 gdb/ChangeLog     | 10 ++++++++
 gdb/dwarf2/read.c | 65 +++++++++++++++++++++++++----------------------
 gdb/dwarf2/read.h | 30 ++++++++++++++++++++++
 3 files changed, 74 insertions(+), 31 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f86034f2273..d1c5bee1109 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -580,27 +580,8 @@ struct type_unit_group
      and is deleted afterwards and not used again.  */
   std::vector<signatured_type *> *tus;
 
-  /* The compunit symtab.
-     Type units in a group needn't all be defined in the same source file,
-     so we create an essentially anonymous symtab as the compunit symtab.  */
-  struct compunit_symtab *compunit_symtab;
-
   /* The data used to construct the hash key.  */
   struct stmt_list_hash hash;
-
-  /* The number of symtabs from the line header.
-     The value here must match line_header.num_file_names.  */
-  unsigned int num_symtabs;
-
-  /* The symbol tables for this TU (obtained from the files listed in
-     DW_AT_stmt_list).
-     WARNING: The order of entries here must match the order of entries
-     in the line header.  After the first TU using this type_unit_group, the
-     line header for the subsequent TUs is recreated from this.  This is done
-     because we need to use the same symtabs for each TU using the same
-     DW_AT_stmt_list value.  Also note that symtabs may be repeated here,
-     there's no guarantee the line header doesn't have duplicate entries.  */
-  struct symtab **symtabs;
 };
 
 /* These sections are what may appear in a (real or virtual) DWO file.  */
@@ -9507,6 +9488,22 @@ rust_union_quirks (struct dwarf2_cu *cu)
   cu->rust_unions.clear ();
 }
 
+/* Get the type_unit_unshareable corresponding to TU_GROUP.  If one
+   does not exist, create it.  */
+
+static type_unit_unshareable *
+get_type_unit_group_unshareable (dwarf2_per_objfile *dwarf2_per_objfile,
+				 type_unit_group *tu_group)
+{
+  auto iter = dwarf2_per_objfile->unshareable->type_units.find (tu_group);
+  if (iter != dwarf2_per_objfile->unshareable->type_units.end ())
+    return iter->second.get ();
+  std::unique_ptr<type_unit_unshareable> uniq (new type_unit_unshareable);
+  type_unit_unshareable *result = uniq.get ();
+  dwarf2_per_objfile->unshareable->type_units[tu_group] = std::move (uniq);
+  return result;
+}
+
 /* Return the symtab for PER_CU.  This works properly regardless of
    whether we're using the index or psymtabs.  */
 
@@ -9776,11 +9773,14 @@ process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
      If this is the first TU to use this symtab, complete the construction
      of it with end_expandable_symtab.  Otherwise, complete the addition of
      this TU's symbols to the existing symtab.  */
-  if (sig_type->type_unit_group->compunit_symtab == NULL)
+  type_unit_unshareable *tu_unshare
+    = get_type_unit_group_unshareable (dwarf2_per_objfile,
+				       sig_type->type_unit_group);
+  if (tu_unshare->compunit_symtab == NULL)
     {
       buildsym_compunit *builder = cu->get_builder ();
       cust = builder->end_expandable_symtab (0, SECT_OFF_TEXT (objfile));
-      sig_type->type_unit_group->compunit_symtab = cust;
+      tu_unshare->compunit_symtab = cust;
 
       if (cust != NULL)
 	{
@@ -9796,7 +9796,7 @@ process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
   else
     {
       cu->get_builder ()->augment_type_symtab ();
-      cust = sig_type->type_unit_group->compunit_symtab;
+      cust = tu_unshare->compunit_symtab;
     }
 
   gdb::optional<compunit_symtab *> &symtab
@@ -10930,7 +10930,10 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
      do it again, we could fake it and just recreate the part we need
      (file name,index -> symtab mapping).  If data shows this optimization
      is useful we can do it then.  */
-  first_time = tu_group->compunit_symtab == NULL;
+  struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;
+  type_unit_unshareable *tu_unshare
+    = get_type_unit_group_unshareable (dwarf2_per_objfile, tu_group);
+  first_time = tu_unshare->compunit_symtab == NULL;
 
   /* We have to handle the case of both a missing DW_AT_stmt_list or bad
      debug info.  */
@@ -10946,9 +10949,9 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
 	start_symtab ("", NULL, 0);
       else
 	{
-	  gdb_assert (tu_group->symtabs == NULL);
+	  gdb_assert (tu_unshare->symtabs == NULL);
 	  gdb_assert (m_builder == nullptr);
-	  struct compunit_symtab *cust = tu_group->compunit_symtab;
+	  struct compunit_symtab *cust = tu_unshare->compunit_symtab;
 	  m_builder.reset (new struct buildsym_compunit
 			   (COMPUNIT_OBJFILE (cust), "",
 			    COMPUNIT_DIRNAME (cust),
@@ -10970,9 +10973,9 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
 	 process_full_type_unit still needs to know if this is the first
 	 time.  */
 
-      tu_group->num_symtabs = line_header->file_names_size ();
-      tu_group->symtabs = XNEWVEC (struct symtab *,
-				   line_header->file_names_size ());
+      tu_unshare->num_symtabs = line_header->file_names_size ();
+      tu_unshare->symtabs = XNEWVEC (struct symtab *,
+				     line_header->file_names_size ());
 
       auto &file_names = line_header->file_names ();
       for (i = 0; i < file_names.size (); ++i)
@@ -10993,13 +10996,13 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
 	    }
 
 	  fe.symtab = b->get_current_subfile ()->symtab;
-	  tu_group->symtabs[i] = fe.symtab;
+	  tu_unshare->symtabs[i] = fe.symtab;
 	}
     }
   else
     {
       gdb_assert (m_builder == nullptr);
-      struct compunit_symtab *cust = tu_group->compunit_symtab;
+      struct compunit_symtab *cust = tu_unshare->compunit_symtab;
       m_builder.reset (new struct buildsym_compunit
 		       (COMPUNIT_OBJFILE (cust), "",
 			COMPUNIT_DIRNAME (cust),
@@ -11010,7 +11013,7 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
       for (i = 0; i < file_names.size (); ++i)
 	{
 	  file_entry &fe = file_names[i];
-	  fe.symtab = tu_group->symtabs[i];
+	  fe.symtab = tu_unshare->symtabs[i];
 	}
     }
 
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index bef37e969d3..8b2e8703aa0 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -47,6 +47,7 @@ struct dwarf2_per_cu_data;
 struct mapped_index;
 struct mapped_debug_names;
 struct signatured_type;
+struct type_unit_group;
 
 /* One item on the queue of compilation units to read in full symbols
    for.  */
@@ -66,6 +67,30 @@ struct dwarf2_queue_item
   enum language pretend_language;
 };
 
+/* This is the per-objfile data associated with a type_unit_group.  */
+
+struct type_unit_unshareable
+{
+  /* The compunit symtab.
+     Type units in a group needn't all be defined in the same source file,
+     so we create an essentially anonymous symtab as the compunit symtab.  */
+  struct compunit_symtab *compunit_symtab = nullptr;
+
+  /* The number of symtabs from the line header.
+     The value here must match line_header.num_file_names.  */
+  unsigned int num_symtabs = 0;
+
+  /* The symbol tables for this TU (obtained from the files listed in
+     DW_AT_stmt_list).
+     WARNING: The order of entries here must match the order of entries
+     in the line header.  After the first TU using this type_unit_group, the
+     line header for the subsequent TUs is recreated from this.  This is done
+     because we need to use the same symtabs for each TU using the same
+     DW_AT_stmt_list value.  Also note that symtabs may be repeated here,
+     there's no guarantee the line header doesn't have duplicate entries.  */
+  struct symtab **symtabs = nullptr;
+};
+
 /* Some DWARF data cannot (currently) be shared across objfiles.  Such
    data is stored in this object.
 
@@ -86,6 +111,11 @@ struct dwarf2_unshareable
   /* Hold the corresponding compunit_symtab for each CU or TU.  This
      is indexed by dwarf2_per_cu_data::index.  */
   std::vector<gdb::optional<compunit_symtab *>> symtabs;
+
+  /* Map from a type unit group to the corresponding unshared
+     structure.  */
+  std::unordered_map<type_unit_group *, std::unique_ptr<type_unit_unshareable>>
+    type_units;
 };
 
 /* Collection of data recorded per objfile.
-- 
2.17.2

Re: [PATCH 11/14] Split type_unit_group

[Luis Machado]

On 2/15/20 1:54 PM, Tom Tromey wrote:
> type_unit_group has links to the compunit_symtab and other symtabs.
> However, once this object is shared across objfiles, this will no
> longer be ok.
> 
> This patch introduces a new type_unit_unshareable and arranges to
> store a map from type unit groups to type_unit_unshareable objects on
> the DWARF unshareable object.
> 
> 2020-02-15  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/read.h (struct type_unit_unshareable): New.
> 	(struct dwarf2_unshareable) <type_units>: New member.
> 	* dwarf2/read.c (struct type_unit_group) <compunit_symtab,
> 	num_symtabs, symtabs>: Remove; move to type_unit_unshareable.
> 	(get_type_unit_group_unshareable): New function.
> 	(process_full_type_unit, dwarf2_cu::setup_type_unit_groups)
> 	(dwarf2_cu::setup_type_unit_groups): Use type_unit_unshareable.
> ---
>   gdb/ChangeLog     | 10 ++++++++
>   gdb/dwarf2/read.c | 65 +++++++++++++++++++++++++----------------------
>   gdb/dwarf2/read.h | 30 ++++++++++++++++++++++
>   3 files changed, 74 insertions(+), 31 deletions(-)
> 
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index f86034f2273..d1c5bee1109 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -580,27 +580,8 @@ struct type_unit_group
>        and is deleted afterwards and not used again.  */
>     std::vector<signatured_type *> *tus;
>   
> -  /* The compunit symtab.
> -     Type units in a group needn't all be defined in the same source file,
> -     so we create an essentially anonymous symtab as the compunit symtab.  */
> -  struct compunit_symtab *compunit_symtab;
> -
>     /* The data used to construct the hash key.  */
>     struct stmt_list_hash hash;
> -
> -  /* The number of symtabs from the line header.
> -     The value here must match line_header.num_file_names.  */
> -  unsigned int num_symtabs;
> -
> -  /* The symbol tables for this TU (obtained from the files listed in
> -     DW_AT_stmt_list).
> -     WARNING: The order of entries here must match the order of entries
> -     in the line header.  After the first TU using this type_unit_group, the
> -     line header for the subsequent TUs is recreated from this.  This is done
> -     because we need to use the same symtabs for each TU using the same
> -     DW_AT_stmt_list value.  Also note that symtabs may be repeated here,
> -     there's no guarantee the line header doesn't have duplicate entries.  */
> -  struct symtab **symtabs;
>   };
>   
>   /* These sections are what may appear in a (real or virtual) DWO file.  */
> @@ -9507,6 +9488,22 @@ rust_union_quirks (struct dwarf2_cu *cu)
>     cu->rust_unions.clear ();
>   }
>   
> +/* Get the type_unit_unshareable corresponding to TU_GROUP.  If one
> +   does not exist, create it.  */
> +
> +static type_unit_unshareable *
> +get_type_unit_group_unshareable (dwarf2_per_objfile *dwarf2_per_objfile,
> +				 type_unit_group *tu_group)
> +{
> +  auto iter = dwarf2_per_objfile->unshareable->type_units.find (tu_group);
> +  if (iter != dwarf2_per_objfile->unshareable->type_units.end ())
> +    return iter->second.get ();
> +  std::unique_ptr<type_unit_unshareable> uniq (new type_unit_unshareable);
> +  type_unit_unshareable *result = uniq.get ();
> +  dwarf2_per_objfile->unshareable->type_units[tu_group] = std::move (uniq);
> +  return result;
> +}
> +
>   /* Return the symtab for PER_CU.  This works properly regardless of
>      whether we're using the index or psymtabs.  */
>   
> @@ -9776,11 +9773,14 @@ process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
>        If this is the first TU to use this symtab, complete the construction
>        of it with end_expandable_symtab.  Otherwise, complete the addition of
>        this TU's symbols to the existing symtab.  */
> -  if (sig_type->type_unit_group->compunit_symtab == NULL)
> +  type_unit_unshareable *tu_unshare
> +    = get_type_unit_group_unshareable (dwarf2_per_objfile,
> +				       sig_type->type_unit_group);
> +  if (tu_unshare->compunit_symtab == NULL)
>       {
>         buildsym_compunit *builder = cu->get_builder ();
>         cust = builder->end_expandable_symtab (0, SECT_OFF_TEXT (objfile));
> -      sig_type->type_unit_group->compunit_symtab = cust;
> +      tu_unshare->compunit_symtab = cust;
>   
>         if (cust != NULL)
>   	{
> @@ -9796,7 +9796,7 @@ process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
>     else
>       {
>         cu->get_builder ()->augment_type_symtab ();
> -      cust = sig_type->type_unit_group->compunit_symtab;
> +      cust = tu_unshare->compunit_symtab;
>       }
>   
>     gdb::optional<compunit_symtab *> &symtab
> @@ -10930,7 +10930,10 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
>        do it again, we could fake it and just recreate the part we need
>        (file name,index -> symtab mapping).  If data shows this optimization
>        is useful we can do it then.  */
> -  first_time = tu_group->compunit_symtab == NULL;
> +  struct dwarf2_per_objfile *dwarf2_per_objfile = per_cu->dwarf2_per_objfile;
> +  type_unit_unshareable *tu_unshare
> +    = get_type_unit_group_unshareable (dwarf2_per_objfile, tu_group);
> +  first_time = tu_unshare->compunit_symtab == NULL;
>   
>     /* We have to handle the case of both a missing DW_AT_stmt_list or bad
>        debug info.  */
> @@ -10946,9 +10949,9 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
>   	start_symtab ("", NULL, 0);
>         else
>   	{
> -	  gdb_assert (tu_group->symtabs == NULL);
> +	  gdb_assert (tu_unshare->symtabs == NULL);
>   	  gdb_assert (m_builder == nullptr);
> -	  struct compunit_symtab *cust = tu_group->compunit_symtab;
> +	  struct compunit_symtab *cust = tu_unshare->compunit_symtab;
>   	  m_builder.reset (new struct buildsym_compunit
>   			   (COMPUNIT_OBJFILE (cust), "",
>   			    COMPUNIT_DIRNAME (cust),
> @@ -10970,9 +10973,9 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
>   	 process_full_type_unit still needs to know if this is the first
>   	 time.  */
>   
> -      tu_group->num_symtabs = line_header->file_names_size ();
> -      tu_group->symtabs = XNEWVEC (struct symtab *,
> -				   line_header->file_names_size ());
> +      tu_unshare->num_symtabs = line_header->file_names_size ();
> +      tu_unshare->symtabs = XNEWVEC (struct symtab *,
> +				     line_header->file_names_size ());
>   
>         auto &file_names = line_header->file_names ();
>         for (i = 0; i < file_names.size (); ++i)
> @@ -10993,13 +10996,13 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
>   	    }
>   
>   	  fe.symtab = b->get_current_subfile ()->symtab;
> -	  tu_group->symtabs[i] = fe.symtab;
> +	  tu_unshare->symtabs[i] = fe.symtab;
>   	}
>       }
>     else
>       {
>         gdb_assert (m_builder == nullptr);
> -      struct compunit_symtab *cust = tu_group->compunit_symtab;
> +      struct compunit_symtab *cust = tu_unshare->compunit_symtab;
>         m_builder.reset (new struct buildsym_compunit
>   		       (COMPUNIT_OBJFILE (cust), "",
>   			COMPUNIT_DIRNAME (cust),
> @@ -11010,7 +11013,7 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
>         for (i = 0; i < file_names.size (); ++i)
>   	{
>   	  file_entry &fe = file_names[i];
> -	  fe.symtab = tu_group->symtabs[i];
> +	  fe.symtab = tu_unshare->symtabs[i];
>   	}
>       }
>   
> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index bef37e969d3..8b2e8703aa0 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -47,6 +47,7 @@ struct dwarf2_per_cu_data;
>   struct mapped_index;
>   struct mapped_debug_names;
>   struct signatured_type;
> +struct type_unit_group;
>   
>   /* One item on the queue of compilation units to read in full symbols
>      for.  */
> @@ -66,6 +67,30 @@ struct dwarf2_queue_item
>     enum language pretend_language;
>   };
>   
> +/* This is the per-objfile data associated with a type_unit_group.  */
> +
> +struct type_unit_unshareable
> +{
> +  /* The compunit symtab.
> +     Type units in a group needn't all be defined in the same source file,
> +     so we create an essentially anonymous symtab as the compunit symtab.  */
> +  struct compunit_symtab *compunit_symtab = nullptr;
> +
> +  /* The number of symtabs from the line header.
> +     The value here must match line_header.num_file_names.  */
> +  unsigned int num_symtabs = 0;
> +
> +  /* The symbol tables for this TU (obtained from the files listed in
> +     DW_AT_stmt_list).
> +     WARNING: The order of entries here must match the order of entries
> +     in the line header.  After the first TU using this type_unit_group, the
> +     line header for the subsequent TUs is recreated from this.  This is done
> +     because we need to use the same symtabs for each TU using the same
> +     DW_AT_stmt_list value.  Also note that symtabs may be repeated here,
> +     there's no guarantee the line header doesn't have duplicate entries.  */

Does something need to be adjusted in the comments for "struct 
type_unit_unshareable" now that we've split these fields from the bigger 
type_unit_group struct?

> +  struct symtab **symtabs = nullptr;
> +};
> +
>   /* Some DWARF data cannot (currently) be shared across objfiles.  Such
>      data is stored in this object.
>   
> @@ -86,6 +111,11 @@ struct dwarf2_unshareable
>     /* Hold the corresponding compunit_symtab for each CU or TU.  This
>        is indexed by dwarf2_per_cu_data::index.  */
>     std::vector<gdb::optional<compunit_symtab *>> symtabs;
> +
> +  /* Map from a type unit group to the corresponding unshared
> +     structure.  */
> +  std::unordered_map<type_unit_group *, std::unique_ptr<type_unit_unshareable>>
> +    type_units;
>   };
>   
>   /* Collection of data recorded per objfile.
> 

Re: [PATCH 11/14] Split type_unit_group

[Tom Tromey]

>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:

>> +  /* The symbol tables for this TU (obtained from the files listed in
>> +     DW_AT_stmt_list).
>> +     WARNING: The order of entries here must match the order of entries
>> +     in the line header.  After the first TU using this type_unit_group, the
>> +     line header for the subsequent TUs is recreated from this.  This is done
>> +     because we need to use the same symtabs for each TU using the same
>> +     DW_AT_stmt_list value.  Also note that symtabs may be repeated here,
>> +     there's no guarantee the line header doesn't have duplicate entries.  */

Luis> Does something need to be adjusted in the comments for "struct
Luis> type_unit_unshareable" now that we've split these fields from the
Luis> bigger type_unit_group struct?

I think this all remained the same.

Tom

[PATCH 10/14] Introduce dwarf2_enter_objfile and use it

[Tom Tromey]

dwarf2_per_objfile has a backlink to objfile.  Once the series is
complete, the objfile will only be set temporarily: that is, it will
be set when calling in to some DWARF module, and then cleared when
leaving.

This patch introduces a new RAII class, dwarf2_enter_objfile, which
will be used for this purpose.  Then, it changes all the callbacks to
call this.

2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.h (class dwarf2_enter_objfile): New.
	* dwarf2/read.c (dw2_find_last_source_symtab)
	(dw2_map_symtabs_matching_filename, dw2_lookup_symbol)
	(dw2_print_stats, dw2_expand_symtabs_for_function)
	(dw2_expand_all_symtabs, dw2_expand_symtabs_with_fullname)
	(dw2_expand_symtabs_matching, dw2_find_pc_sect_compunit_symtab)
	(dw2_map_symbol_filenames, dw2_debug_names_lookup_symbol)
	(dw2_debug_names_expand_symtabs_for_function)
	(dw2_debug_names_map_matching_symbols)
	(dw2_debug_names_expand_symtabs_matching)
	(dwarf2_initialize_objfile, dwarf2_build_psymtabs)
	(dwarf2_psymtab::read_symtab, dwarf2_psymtab::expand_psymtab)
	(dwarf2_psymtab::get_compunit_symtab): Use dwarf2_enter_objfile.
	* dwarf2/loc.c (dwarf2_find_location_expression)
	(rw_pieced_value, indirect_pieced_value, coerce_pieced_ref)
	(dwarf2_locexpr_baton_eval, dwarf2_evaluate_property)
	(locexpr_read_variable, locexpr_read_variable_at_entry)
	(locexpr_get_symbol_read_needs, locexpr_describe_location)
	(locexpr_tracepoint_var_ref, locexpr_generate_c_location)
	(loclist_read_variable, loclist_read_variable_at_entry)
	(loclist_describe_location, loclist_tracepoint_var_ref)
	(loclist_generate_c_location): Use dwarf2_enter_objfile.
	* dwarf2/index-write.c (save_gdb_index_command): Use
	dwarf2_enter_objfile.
---
 gdb/ChangeLog            | 27 +++++++++++++++++++++++++++
 gdb/dwarf2/index-write.c |  1 +
 gdb/dwarf2/loc.c         | 34 ++++++++++++++++++++++++++++++----
 gdb/dwarf2/read.c        | 21 +++++++++++++++++++++
 gdb/dwarf2/read.h        | 24 ++++++++++++++++++++++++
 5 files changed, 103 insertions(+), 4 deletions(-)

diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index 2cbf2ebd202..24255f5f76f 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1755,6 +1755,7 @@ save_gdb_index_command (const char *arg, int from_tty)
 
       struct dwarf2_per_objfile *dwarf2_per_objfile
 	= get_dwarf2_per_objfile (objfile);
+      dwarf2_enter_objfile enterer (objfile);
 
       if (dwarf2_per_objfile != NULL)
 	{
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 4a148c26722..b0f650cada8 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -312,7 +312,7 @@ const gdb_byte *
 dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 				 size_t *locexpr_length, CORE_ADDR pc)
 {
-  struct objfile *objfile = baton->per_cu->objfile ();
+  struct objfile *objfile = baton->objfile;
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   unsigned int addr_size = baton->per_cu->addr_size ();
@@ -1612,6 +1612,8 @@ rw_pieced_value (struct value *v, struct value *from)
   gdb::byte_vector buffer;
   bool bits_big_endian = type_byte_order (value_type (v)) == BFD_ENDIAN_BIG;
 
+  dwarf2_enter_objfile enterer (c->objfile);
+
   if (from != NULL)
     {
       from_contents = value_contents (from);
@@ -2015,6 +2017,8 @@ indirect_pieced_value (struct value *value)
 {
   struct piece_closure *c
     = (struct piece_closure *) value_computed_closure (value);
+  dwarf2_enter_objfile enterer (c->objfile);
+
   struct type *type;
   struct frame_info *frame;
   int i, bit_length;
@@ -2105,6 +2109,7 @@ coerce_pieced_ref (const struct value *value)
       gdb_assert (closure != NULL);
       gdb_assert (closure->pieces.size () == 1);
 
+      dwarf2_enter_objfile enterer (closure->objfile);
       return indirect_synthetic_pointer
 	(closure->pieces[0].v.ptr.die_sect_off,
 	 closure->pieces[0].v.ptr.offset,
@@ -2404,7 +2409,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
   ctx.per_cu = dlbaton->per_cu;
   ctx.obj_address = addr;
 
-  objfile = dlbaton->per_cu->objfile ();
+  objfile = dlbaton->objfile;
 
   ctx.gdbarch = get_objfile_arch (objfile);
   ctx.addr_size = dlbaton->per_cu->addr_size ();
@@ -2474,6 +2479,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
 	const struct dwarf2_property_baton *baton
 	  = (const struct dwarf2_property_baton *) prop->data.baton;
 	gdb_assert (baton->property_type != NULL);
+	dwarf2_enter_objfile enterer (baton->locexpr.objfile);
 
 	if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame,
 				       addr_stack ? addr_stack->addr : 0,
@@ -3552,6 +3558,8 @@ locexpr_read_variable (struct symbol *symbol, struct frame_info *frame)
 {
   struct dwarf2_locexpr_baton *dlbaton
     = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
+
   struct value *val;
 
   val = dwarf2_evaluate_loc_desc (SYMBOL_TYPE (symbol), frame, dlbaton->data,
@@ -3569,6 +3577,7 @@ locexpr_read_variable_at_entry (struct symbol *symbol, struct frame_info *frame)
 {
   struct dwarf2_locexpr_baton *dlbaton
     = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
 
   return value_of_dwarf_block_entry (SYMBOL_TYPE (symbol), frame, dlbaton->data,
 				     dlbaton->size);
@@ -3582,6 +3591,7 @@ locexpr_get_symbol_read_needs (struct symbol *symbol)
 {
   struct dwarf2_locexpr_baton *dlbaton
     = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
 
   return dwarf2_loc_desc_get_symbol_read_needs (dlbaton->data, dlbaton->size,
 						dlbaton->per_cu);
@@ -4285,7 +4295,9 @@ locexpr_describe_location (struct symbol *symbol, CORE_ADDR addr,
 {
   struct dwarf2_locexpr_baton *dlbaton
     = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
-  struct objfile *objfile = dlbaton->per_cu->objfile ();
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
+
+  struct objfile *objfile = dlbaton->objfile;
   unsigned int addr_size = dlbaton->per_cu->addr_size ();
   int offset_size = dlbaton->per_cu->offset_size ();
 
@@ -4304,6 +4316,8 @@ locexpr_tracepoint_var_ref (struct symbol *symbol, struct agent_expr *ax,
 {
   struct dwarf2_locexpr_baton *dlbaton
     = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
+
   unsigned int addr_size = dlbaton->per_cu->addr_size ();
 
   if (dlbaton->size == 0)
@@ -4323,6 +4337,8 @@ locexpr_generate_c_location (struct symbol *sym, string_file *stream,
 {
   struct dwarf2_locexpr_baton *dlbaton
     = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (sym);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
+
   unsigned int addr_size = dlbaton->per_cu->addr_size ();
 
   if (dlbaton->size == 0)
@@ -4357,6 +4373,8 @@ loclist_read_variable (struct symbol *symbol, struct frame_info *frame)
 {
   struct dwarf2_loclist_baton *dlbaton
     = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (symbol);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
+
   struct value *val;
   const gdb_byte *data;
   size_t size;
@@ -4382,6 +4400,8 @@ loclist_read_variable_at_entry (struct symbol *symbol, struct frame_info *frame)
 {
   struct dwarf2_loclist_baton *dlbaton
     = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (symbol);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
+
   const gdb_byte *data;
   size_t size;
   CORE_ADDR pc;
@@ -4421,8 +4441,10 @@ loclist_describe_location (struct symbol *symbol, CORE_ADDR addr,
 {
   struct dwarf2_loclist_baton *dlbaton
     = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (symbol);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
+
   const gdb_byte *loc_ptr, *buf_end;
-  struct objfile *objfile = dlbaton->per_cu->objfile ();
+  struct objfile *objfile = dlbaton->objfile;
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   unsigned int addr_size = dlbaton->per_cu->addr_size ();
@@ -4511,6 +4533,8 @@ loclist_tracepoint_var_ref (struct symbol *symbol, struct agent_expr *ax,
 {
   struct dwarf2_loclist_baton *dlbaton
     = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (symbol);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
+
   const gdb_byte *data;
   size_t size;
   unsigned int addr_size = dlbaton->per_cu->addr_size ();
@@ -4533,6 +4557,8 @@ loclist_generate_c_location (struct symbol *sym, string_file *stream,
 {
   struct dwarf2_loclist_baton *dlbaton
     = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (sym);
+  dwarf2_enter_objfile enterer (dlbaton->objfile);
+
   unsigned int addr_size = dlbaton->per_cu->addr_size ();
   const gdb_byte *data;
   size_t size;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 7b493f5a227..f86034f2273 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3234,6 +3234,7 @@ dw2_get_real_path (struct objfile *objfile,
 static struct symtab *
 dw2_find_last_source_symtab (struct objfile *objfile)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
   dwarf2_per_cu_data *dwarf_cu = dwarf2_per_objfile->all_comp_units.back ();
@@ -3310,6 +3311,7 @@ dw2_map_symtabs_matching_filename
   (struct objfile *objfile, const char *name, const char *real_path,
    gdb::function_view<bool (symtab *)> callback)
 {
+  dwarf2_enter_objfile enterer (objfile);
   const char *name_basename = lbasename (name);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
@@ -3528,6 +3530,7 @@ static struct compunit_symtab *
 dw2_lookup_symbol (struct objfile *objfile, block_enum block_index,
 		   const char *name, domain_enum domain)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct compunit_symtab *stab_best = NULL;
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
@@ -3570,6 +3573,7 @@ dw2_lookup_symbol (struct objfile *objfile, block_enum block_index,
 static void
 dw2_print_stats (struct objfile *objfile)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
   int total = (dwarf2_per_objfile->all_comp_units.size ()
@@ -3616,6 +3620,7 @@ static void
 dw2_expand_symtabs_for_function (struct objfile *objfile,
 				 const char *func_name)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -3632,6 +3637,7 @@ dw2_expand_symtabs_for_function (struct objfile *objfile,
 static void
 dw2_expand_all_symtabs (struct objfile *objfile)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
   int total_units = (dwarf2_per_objfile->all_comp_units.size ()
@@ -3654,6 +3660,7 @@ static void
 dw2_expand_symtabs_with_fullname (struct objfile *objfile,
 				  const char *fullname)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -4657,6 +4664,7 @@ dw2_expand_symtabs_matching
    gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
    enum search_domain kind)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -4725,6 +4733,7 @@ dw2_find_pc_sect_compunit_symtab (struct objfile *objfile,
   if (!data)
     return NULL;
 
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
   gdb::optional<compunit_symtab *> &symtab
@@ -4745,6 +4754,7 @@ static void
 dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
 			  void *data, int need_fullname)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -5527,6 +5537,7 @@ static struct compunit_symtab *
 dw2_debug_names_lookup_symbol (struct objfile *objfile, block_enum block_index,
 			       const char *name, domain_enum domain)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -5593,6 +5604,7 @@ static void
 dw2_debug_names_expand_symtabs_for_function (struct objfile *objfile,
 					     const char *func_name)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -5617,6 +5629,7 @@ dw2_debug_names_map_matching_symbols
    gdb::function_view<symbol_found_callback_ftype> callback,
    symbol_compare_ftype *ordered_compare)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -5676,6 +5689,7 @@ dw2_debug_names_expand_symtabs_matching
    gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
    enum search_domain kind)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -5792,6 +5806,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
 {
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
+  dwarf2_enter_objfile enterer (objfile);
 
   /* If we're about to read full symbols, don't bother with the
      indices.  In this case we also don't care if some other debug
@@ -5861,6 +5876,8 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
 void
 dwarf2_build_psymtabs (struct objfile *objfile)
 {
+  dwarf2_enter_objfile enterer (objfile);
+
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -8740,6 +8757,7 @@ locate_pdi_sibling (const struct die_reader_specs *reader,
 void
 dwarf2_psymtab::read_symtab (struct objfile *objfile)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -8899,6 +8917,7 @@ process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile)
 void
 dwarf2_psymtab::expand_psymtab (struct objfile *objfile)
 {
+  dwarf2_enter_objfile enterer (objfile);
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
 
@@ -8928,6 +8947,7 @@ dwarf2_psymtab::readin_p (struct objfile *objfile) const
   if (per_cu_data == nullptr)
     return true;
 
+  dwarf2_enter_objfile enterer (objfile);
   dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
   gdb::optional<compunit_symtab *> &symtab
@@ -8943,6 +8963,7 @@ dwarf2_psymtab::get_compunit_symtab (struct objfile *objfile) const
   if (per_cu_data == nullptr)
     return nullptr;
 
+  dwarf2_enter_objfile enterer (objfile);
   dwarf2_per_objfile *dwarf2_per_objfile
     = get_dwarf2_per_objfile (objfile);
   gdb::optional<compunit_symtab *> &symtab
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 98d58fb6880..bef37e969d3 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -284,6 +284,30 @@ public:
 
 dwarf2_per_objfile *get_dwarf2_per_objfile (struct objfile *objfile);
 
+/* The "objfile" member of a dwarf2_per_objfile is normally nullptr,
+   and temporarily set when calling into the DWARF code.  This class
+   is used "enter" a particular objfile.  */
+
+class dwarf2_enter_objfile
+{
+public:
+
+  dwarf2_enter_objfile (struct objfile *objfile)
+    : m_per_objfile (get_dwarf2_per_objfile (objfile)),
+      m_restore_objfile (&m_per_objfile->objfile, objfile)
+  {
+  }
+
+  ~dwarf2_enter_objfile () = default;
+
+  DISABLE_COPY_AND_ASSIGN (dwarf2_enter_objfile);
+
+private:
+
+  dwarf2_per_objfile *m_per_objfile;
+  scoped_restore_tmpl<struct objfile *> m_restore_objfile;
+};
+
 /* A partial symtab specialized for DWARF.  */
 struct dwarf2_psymtab : public partial_symtab
 {
-- 
2.17.2

Re: [PATCH 10/14] Introduce dwarf2_enter_objfile and use it

[Luis Machado]

On 2/15/20 1:54 PM, Tom Tromey wrote:
> dwarf2_per_objfile has a backlink to objfile.  Once the series is
> complete, the objfile will only be set temporarily: that is, it will
> be set when calling in to some DWARF module, and then cleared when
> leaving.
> 
> This patch introduces a new RAII class, dwarf2_enter_objfile, which
> will be used for this purpose.  Then, it changes all the callbacks to
> call this.
> 
> 2020-02-15  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/read.h (class dwarf2_enter_objfile): New.
> 	* dwarf2/read.c (dw2_find_last_source_symtab)
> 	(dw2_map_symtabs_matching_filename, dw2_lookup_symbol)
> 	(dw2_print_stats, dw2_expand_symtabs_for_function)
> 	(dw2_expand_all_symtabs, dw2_expand_symtabs_with_fullname)
> 	(dw2_expand_symtabs_matching, dw2_find_pc_sect_compunit_symtab)
> 	(dw2_map_symbol_filenames, dw2_debug_names_lookup_symbol)
> 	(dw2_debug_names_expand_symtabs_for_function)
> 	(dw2_debug_names_map_matching_symbols)
> 	(dw2_debug_names_expand_symtabs_matching)
> 	(dwarf2_initialize_objfile, dwarf2_build_psymtabs)
> 	(dwarf2_psymtab::read_symtab, dwarf2_psymtab::expand_psymtab)
> 	(dwarf2_psymtab::get_compunit_symtab): Use dwarf2_enter_objfile.
> 	* dwarf2/loc.c (dwarf2_find_location_expression)
> 	(rw_pieced_value, indirect_pieced_value, coerce_pieced_ref)
> 	(dwarf2_locexpr_baton_eval, dwarf2_evaluate_property)
> 	(locexpr_read_variable, locexpr_read_variable_at_entry)
> 	(locexpr_get_symbol_read_needs, locexpr_describe_location)
> 	(locexpr_tracepoint_var_ref, locexpr_generate_c_location)
> 	(loclist_read_variable, loclist_read_variable_at_entry)
> 	(loclist_describe_location, loclist_tracepoint_var_ref)
> 	(loclist_generate_c_location): Use dwarf2_enter_objfile.
> 	* dwarf2/index-write.c (save_gdb_index_command): Use
> 	dwarf2_enter_objfile.
> ---
>   gdb/ChangeLog            | 27 +++++++++++++++++++++++++++
>   gdb/dwarf2/index-write.c |  1 +
>   gdb/dwarf2/loc.c         | 34 ++++++++++++++++++++++++++++++----
>   gdb/dwarf2/read.c        | 21 +++++++++++++++++++++
>   gdb/dwarf2/read.h        | 24 ++++++++++++++++++++++++
>   5 files changed, 103 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
> index 2cbf2ebd202..24255f5f76f 100644
> --- a/gdb/dwarf2/index-write.c
> +++ b/gdb/dwarf2/index-write.c
> @@ -1755,6 +1755,7 @@ save_gdb_index_command (const char *arg, int from_tty)
>   
>         struct dwarf2_per_objfile *dwarf2_per_objfile
>   	= get_dwarf2_per_objfile (objfile);
> +      dwarf2_enter_objfile enterer (objfile);
>   
>         if (dwarf2_per_objfile != NULL)
>   	{
> diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
> index 4a148c26722..b0f650cada8 100644
> --- a/gdb/dwarf2/loc.c
> +++ b/gdb/dwarf2/loc.c
> @@ -312,7 +312,7 @@ const gdb_byte *
>   dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
>   				 size_t *locexpr_length, CORE_ADDR pc)
>   {
> -  struct objfile *objfile = baton->per_cu->objfile ();
> +  struct objfile *objfile = baton->objfile;

I noticed the above change in a few hunks. Are these spurious changes 
from a different patch in the series? It doesn't seem to be related to 
the dwarf2_enter_objfile change. Am i missing something?

>     struct gdbarch *gdbarch = get_objfile_arch (objfile);
>     enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>     unsigned int addr_size = baton->per_cu->addr_size ();
> @@ -1612,6 +1612,8 @@ rw_pieced_value (struct value *v, struct value *from)
>     gdb::byte_vector buffer;
>     bool bits_big_endian = type_byte_order (value_type (v)) == BFD_ENDIAN_BIG;
>   
> +  dwarf2_enter_objfile enterer (c->objfile);
> +
>     if (from != NULL)
>       {
>         from_contents = value_contents (from);
> @@ -2015,6 +2017,8 @@ indirect_pieced_value (struct value *value)
>   {
>     struct piece_closure *c
>       = (struct piece_closure *) value_computed_closure (value);
> +  dwarf2_enter_objfile enterer (c->objfile);
> +
>     struct type *type;
>     struct frame_info *frame;
>     int i, bit_length;
> @@ -2105,6 +2109,7 @@ coerce_pieced_ref (const struct value *value)
>         gdb_assert (closure != NULL);
>         gdb_assert (closure->pieces.size () == 1);
>   
> +      dwarf2_enter_objfile enterer (closure->objfile);
>         return indirect_synthetic_pointer
>   	(closure->pieces[0].v.ptr.die_sect_off,
>   	 closure->pieces[0].v.ptr.offset,
> @@ -2404,7 +2409,7 @@ dwarf2_locexpr_baton_eval (const struct dwarf2_locexpr_baton *dlbaton,
>     ctx.per_cu = dlbaton->per_cu;
>     ctx.obj_address = addr;
>   
> -  objfile = dlbaton->per_cu->objfile ();
> +  objfile = dlbaton->objfile;
>   
>     ctx.gdbarch = get_objfile_arch (objfile);
>     ctx.addr_size = dlbaton->per_cu->addr_size ();
> @@ -2474,6 +2479,7 @@ dwarf2_evaluate_property (const struct dynamic_prop *prop,
>   	const struct dwarf2_property_baton *baton
>   	  = (const struct dwarf2_property_baton *) prop->data.baton;
>   	gdb_assert (baton->property_type != NULL);
> +	dwarf2_enter_objfile enterer (baton->locexpr.objfile);
>   
>   	if (dwarf2_locexpr_baton_eval (&baton->locexpr, frame,
>   				       addr_stack ? addr_stack->addr : 0,
> @@ -3552,6 +3558,8 @@ locexpr_read_variable (struct symbol *symbol, struct frame_info *frame)
>   {
>     struct dwarf2_locexpr_baton *dlbaton
>       = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
> +
>     struct value *val;
>   
>     val = dwarf2_evaluate_loc_desc (SYMBOL_TYPE (symbol), frame, dlbaton->data,
> @@ -3569,6 +3577,7 @@ locexpr_read_variable_at_entry (struct symbol *symbol, struct frame_info *frame)
>   {
>     struct dwarf2_locexpr_baton *dlbaton
>       = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
>   
>     return value_of_dwarf_block_entry (SYMBOL_TYPE (symbol), frame, dlbaton->data,
>   				     dlbaton->size);
> @@ -3582,6 +3591,7 @@ locexpr_get_symbol_read_needs (struct symbol *symbol)
>   {
>     struct dwarf2_locexpr_baton *dlbaton
>       = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
>   
>     return dwarf2_loc_desc_get_symbol_read_needs (dlbaton->data, dlbaton->size,
>   						dlbaton->per_cu);
> @@ -4285,7 +4295,9 @@ locexpr_describe_location (struct symbol *symbol, CORE_ADDR addr,
>   {
>     struct dwarf2_locexpr_baton *dlbaton
>       = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
> -  struct objfile *objfile = dlbaton->per_cu->objfile ();
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
> +
> +  struct objfile *objfile = dlbaton->objfile;
>     unsigned int addr_size = dlbaton->per_cu->addr_size ();
>     int offset_size = dlbaton->per_cu->offset_size ();
>   
> @@ -4304,6 +4316,8 @@ locexpr_tracepoint_var_ref (struct symbol *symbol, struct agent_expr *ax,
>   {
>     struct dwarf2_locexpr_baton *dlbaton
>       = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (symbol);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
> +
>     unsigned int addr_size = dlbaton->per_cu->addr_size ();
>   
>     if (dlbaton->size == 0)
> @@ -4323,6 +4337,8 @@ locexpr_generate_c_location (struct symbol *sym, string_file *stream,
>   {
>     struct dwarf2_locexpr_baton *dlbaton
>       = (struct dwarf2_locexpr_baton *) SYMBOL_LOCATION_BATON (sym);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
> +
>     unsigned int addr_size = dlbaton->per_cu->addr_size ();
>   
>     if (dlbaton->size == 0)
> @@ -4357,6 +4373,8 @@ loclist_read_variable (struct symbol *symbol, struct frame_info *frame)
>   {
>     struct dwarf2_loclist_baton *dlbaton
>       = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (symbol);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
> +
>     struct value *val;
>     const gdb_byte *data;
>     size_t size;
> @@ -4382,6 +4400,8 @@ loclist_read_variable_at_entry (struct symbol *symbol, struct frame_info *frame)
>   {
>     struct dwarf2_loclist_baton *dlbaton
>       = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (symbol);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
> +
>     const gdb_byte *data;
>     size_t size;
>     CORE_ADDR pc;
> @@ -4421,8 +4441,10 @@ loclist_describe_location (struct symbol *symbol, CORE_ADDR addr,
>   {
>     struct dwarf2_loclist_baton *dlbaton
>       = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (symbol);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
> +
>     const gdb_byte *loc_ptr, *buf_end;
> -  struct objfile *objfile = dlbaton->per_cu->objfile ();
> +  struct objfile *objfile = dlbaton->objfile;
>     struct gdbarch *gdbarch = get_objfile_arch (objfile);
>     enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>     unsigned int addr_size = dlbaton->per_cu->addr_size ();
> @@ -4511,6 +4533,8 @@ loclist_tracepoint_var_ref (struct symbol *symbol, struct agent_expr *ax,
>   {
>     struct dwarf2_loclist_baton *dlbaton
>       = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (symbol);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
> +
>     const gdb_byte *data;
>     size_t size;
>     unsigned int addr_size = dlbaton->per_cu->addr_size ();
> @@ -4533,6 +4557,8 @@ loclist_generate_c_location (struct symbol *sym, string_file *stream,
>   {
>     struct dwarf2_loclist_baton *dlbaton
>       = (struct dwarf2_loclist_baton *) SYMBOL_LOCATION_BATON (sym);
> +  dwarf2_enter_objfile enterer (dlbaton->objfile);
> +
>     unsigned int addr_size = dlbaton->per_cu->addr_size ();
>     const gdb_byte *data;
>     size_t size;
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 7b493f5a227..f86034f2273 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -3234,6 +3234,7 @@ dw2_get_real_path (struct objfile *objfile,
>   static struct symtab *
>   dw2_find_last_source_symtab (struct objfile *objfile)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>     dwarf2_per_cu_data *dwarf_cu = dwarf2_per_objfile->all_comp_units.back ();
> @@ -3310,6 +3311,7 @@ dw2_map_symtabs_matching_filename
>     (struct objfile *objfile, const char *name, const char *real_path,
>      gdb::function_view<bool (symtab *)> callback)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     const char *name_basename = lbasename (name);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
> @@ -3528,6 +3530,7 @@ static struct compunit_symtab *
>   dw2_lookup_symbol (struct objfile *objfile, block_enum block_index,
>   		   const char *name, domain_enum domain)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct compunit_symtab *stab_best = NULL;
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
> @@ -3570,6 +3573,7 @@ dw2_lookup_symbol (struct objfile *objfile, block_enum block_index,
>   static void
>   dw2_print_stats (struct objfile *objfile)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>     int total = (dwarf2_per_objfile->all_comp_units.size ()
> @@ -3616,6 +3620,7 @@ static void
>   dw2_expand_symtabs_for_function (struct objfile *objfile,
>   				 const char *func_name)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -3632,6 +3637,7 @@ dw2_expand_symtabs_for_function (struct objfile *objfile,
>   static void
>   dw2_expand_all_symtabs (struct objfile *objfile)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>     int total_units = (dwarf2_per_objfile->all_comp_units.size ()
> @@ -3654,6 +3660,7 @@ static void
>   dw2_expand_symtabs_with_fullname (struct objfile *objfile,
>   				  const char *fullname)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -4657,6 +4664,7 @@ dw2_expand_symtabs_matching
>      gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
>      enum search_domain kind)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -4725,6 +4733,7 @@ dw2_find_pc_sect_compunit_symtab (struct objfile *objfile,
>     if (!data)
>       return NULL;
>   
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>     gdb::optional<compunit_symtab *> &symtab
> @@ -4745,6 +4754,7 @@ static void
>   dw2_map_symbol_filenames (struct objfile *objfile, symbol_filename_ftype *fun,
>   			  void *data, int need_fullname)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -5527,6 +5537,7 @@ static struct compunit_symtab *
>   dw2_debug_names_lookup_symbol (struct objfile *objfile, block_enum block_index,
>   			       const char *name, domain_enum domain)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -5593,6 +5604,7 @@ static void
>   dw2_debug_names_expand_symtabs_for_function (struct objfile *objfile,
>   					     const char *func_name)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -5617,6 +5629,7 @@ dw2_debug_names_map_matching_symbols
>      gdb::function_view<symbol_found_callback_ftype> callback,
>      symbol_compare_ftype *ordered_compare)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -5676,6 +5689,7 @@ dw2_debug_names_expand_symtabs_matching
>      gdb::function_view<expand_symtabs_exp_notify_ftype> expansion_notify,
>      enum search_domain kind)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -5792,6 +5806,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
>   {
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
> +  dwarf2_enter_objfile enterer (objfile);
>   
>     /* If we're about to read full symbols, don't bother with the
>        indices.  In this case we also don't care if some other debug
> @@ -5861,6 +5876,8 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
>   void
>   dwarf2_build_psymtabs (struct objfile *objfile)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
> +
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -8740,6 +8757,7 @@ locate_pdi_sibling (const struct die_reader_specs *reader,
>   void
>   dwarf2_psymtab::read_symtab (struct objfile *objfile)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -8899,6 +8917,7 @@ process_queue (struct dwarf2_per_objfile *dwarf2_per_objfile)
>   void
>   dwarf2_psymtab::expand_psymtab (struct objfile *objfile)
>   {
> +  dwarf2_enter_objfile enterer (objfile);
>     struct dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>   
> @@ -8928,6 +8947,7 @@ dwarf2_psymtab::readin_p (struct objfile *objfile) const
>     if (per_cu_data == nullptr)
>       return true;
>   
> +  dwarf2_enter_objfile enterer (objfile);
>     dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>     gdb::optional<compunit_symtab *> &symtab
> @@ -8943,6 +8963,7 @@ dwarf2_psymtab::get_compunit_symtab (struct objfile *objfile) const
>     if (per_cu_data == nullptr)
>       return nullptr;
>   
> +  dwarf2_enter_objfile enterer (objfile);
>     dwarf2_per_objfile *dwarf2_per_objfile
>       = get_dwarf2_per_objfile (objfile);
>     gdb::optional<compunit_symtab *> &symtab
> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index 98d58fb6880..bef37e969d3 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -284,6 +284,30 @@ public:
>   
>   dwarf2_per_objfile *get_dwarf2_per_objfile (struct objfile *objfile);
>   
> +/* The "objfile" member of a dwarf2_per_objfile is normally nullptr,
> +   and temporarily set when calling into the DWARF code.  This class
> +   is used "enter" a particular objfile.  */

"is used to "enter" a ..." ?

> +
> +class dwarf2_enter_objfile
> +{
> +public:
> +
> +  dwarf2_enter_objfile (struct objfile *objfile)
> +    : m_per_objfile (get_dwarf2_per_objfile (objfile)),
> +      m_restore_objfile (&m_per_objfile->objfile, objfile)
> +  {
> +  }
> +
> +  ~dwarf2_enter_objfile () = default;
> +
> +  DISABLE_COPY_AND_ASSIGN (dwarf2_enter_objfile);
> +
> +private:
> +
> +  dwarf2_per_objfile *m_per_objfile;
> +  scoped_restore_tmpl<struct objfile *> m_restore_objfile;
> +};
> +
>   /* A partial symtab specialized for DWARF.  */
>   struct dwarf2_psymtab : public partial_symtab
>   {
> 

Re: [PATCH 10/14] Introduce dwarf2_enter_objfile and use it

[Tom Tromey]

>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:

>> -  struct objfile *objfile = baton->per_cu->objfile ();
>> +  struct objfile *objfile = baton->objfile;

Luis> I noticed the above change in a few hunks. Are these spurious changes
Luis> from a different patch in the series? It doesn't seem to be related to 
Luis> the dwarf2_enter_objfile change. Am i missing something?

I've moved these hunks into the preceding patch, which adds the objfile
member to these batons.

Tom

[PATCH 03/14] Introduce dwarf2_per_objfile::obstack

[Tom Tromey]

Currently much of the DWARF-related data is stored on the obfile
obstack.  This prevents sharing this data across objfiles, so this
patch adds a new obstack to dwarf2_per_objfile.

One way to check whether this is correct is to look at the remaining
uses of objfile_obstack in the DWARF code and note that they all
appear in the "full CU" code paths.

The converse -- storing per-objfile data on the shared obstack -- is
not good, but it is just a memory leak, not a potential
use-after-free.  Double-checking this would also be useful, though.

gdb/ChangeLog
2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.h (struct dwarf2_per_objfile) <obstack>: New
	member.
	* dwarf2/read.c (delete_file_name_entry): Fix comment.
	(create_cu_from_index_list)
	(create_signatured_type_table_from_index)
	(create_signatured_type_table_from_debug_names)
	(dw2_get_file_names_reader, dw2_get_real_path)
	(dwarf2_initialize_objfile, dwarf2_create_include_psymtab)
	(create_debug_type_hash_table, add_type_unit)
	(create_type_unit_group, read_comp_units_from_section)
	(dwarf2_compute_name, create_cus_hash_table)
	(create_dwp_hash_table, create_dwo_unit_in_dwp_v1)
	(create_dwo_unit_in_dwp_v2, open_and_init_dwp_file): Use new
	obstack.
---
 gdb/ChangeLog     | 17 +++++++++
 gdb/dwarf2/read.c | 88 ++++++++++++++++++++++-------------------------
 gdb/dwarf2/read.h |  5 +++
 3 files changed, 64 insertions(+), 46 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 81d8cbf0b13..d3060be630d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -2280,8 +2280,8 @@ delete_file_name_entry (void *e)
 	xfree ((void*) file_data->real_names[i]);
     }
 
-  /* The space for the struct itself lives on objfile_obstack,
-     so we don't free it here.  */
+  /* The space for the struct itself lives on the obstack, so we don't
+     free it here.  */
 }
 
 /* Create a quick_file_names hash table.  */
@@ -2412,9 +2412,8 @@ dwarf2_per_objfile::get_tu (int index)
   return this->all_type_units[index];
 }
 
-/* Return a new dwarf2_per_cu_data allocated on OBJFILE's
-   objfile_obstack, and constructed with the specified field
-   values.  */
+/* Return a new dwarf2_per_cu_data allocated on the dwarf2_per_objfile
+   obstack, and constructed with the specified field values.  */
 
 static dwarf2_per_cu_data *
 create_cu_from_index_list (struct dwarf2_per_objfile *dwarf2_per_objfile,
@@ -2422,16 +2421,15 @@ create_cu_from_index_list (struct dwarf2_per_objfile *dwarf2_per_objfile,
                           int is_dwz,
                           sect_offset sect_off, ULONGEST length)
 {
-  struct objfile *objfile = dwarf2_per_objfile->objfile;
   dwarf2_per_cu_data *the_cu
-    = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+    = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
                      struct dwarf2_per_cu_data);
   the_cu->sect_off = sect_off;
   the_cu->length = length;
   the_cu->dwarf2_per_objfile = dwarf2_per_objfile;
   the_cu->section = section;
-  the_cu->v.quick = OBSTACK_ZALLOC (&objfile->objfile_obstack,
-                                   struct dwarf2_per_cu_quick_data);
+  the_cu->v.quick = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
+				    struct dwarf2_per_cu_quick_data);
   the_cu->is_dwz = is_dwz;
   return the_cu;
 }
@@ -2516,7 +2514,7 @@ create_signatured_type_table_from_index
       signature = extract_unsigned_integer (bytes + 16, 8, BFD_ENDIAN_LITTLE);
       bytes += 3 * 8;
 
-      sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+      sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 				 struct signatured_type);
       sig_type->signature = signature;
       sig_type->type_offset_in_tu = type_offset_in_tu;
@@ -2525,7 +2523,7 @@ create_signatured_type_table_from_index
       sig_type->per_cu.sect_off = sect_off;
       sig_type->per_cu.dwarf2_per_objfile = dwarf2_per_objfile;
       sig_type->per_cu.v.quick
-	= OBSTACK_ZALLOC (&objfile->objfile_obstack,
+	= OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 			  struct dwarf2_per_cu_quick_data);
 
       slot = htab_find_slot (sig_types_hash.get (), sig_type, INSERT);
@@ -2573,7 +2571,7 @@ create_signatured_type_table_from_debug_names
 				     section->buffer + to_underlying (sect_off),
 				     rcuh_kind::TYPE);
 
-      sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+      sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 				 struct signatured_type);
       sig_type->signature = cu_header.signature;
       sig_type->type_offset_in_tu = cu_header.type_cu_offset_in_tu;
@@ -2582,7 +2580,7 @@ create_signatured_type_table_from_debug_names
       sig_type->per_cu.sect_off = sect_off;
       sig_type->per_cu.dwarf2_per_objfile = dwarf2_per_objfile;
       sig_type->per_cu.v.quick
-	= OBSTACK_ZALLOC (&objfile->objfile_obstack,
+	= OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 			  struct dwarf2_per_cu_quick_data);
 
       slot = htab_find_slot (sig_types_hash.get (), sig_type, INSERT);
@@ -3094,7 +3092,6 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   struct dwarf2_per_cu_data *this_cu = cu->per_cu;
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = cu->per_cu->dwarf2_per_objfile;
-  struct objfile *objfile = dwarf2_per_objfile->objfile;
   struct dwarf2_per_cu_data *lh_cu;
   struct attribute *attr;
   void **slot;
@@ -3143,7 +3140,7 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
       return;
     }
 
-  qfn = XOBNEW (&objfile->objfile_obstack, struct quick_file_names);
+  qfn = XOBNEW (&dwarf2_per_objfile->obstack, struct quick_file_names);
   qfn->hash.dwo_unit = cu->dwo_unit;
   qfn->hash.line_sect_off = line_offset;
   gdb_assert (slot != NULL);
@@ -3157,7 +3154,8 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
 
   qfn->num_file_names = offset + lh->file_names_size ();
   qfn->file_names =
-    XOBNEWVEC (&objfile->objfile_obstack, const char *, qfn->num_file_names);
+    XOBNEWVEC (&dwarf2_per_objfile->obstack, const char *,
+	       qfn->num_file_names);
   if (offset != 0)
     qfn->file_names[0] = xstrdup (fnd.name);
   for (int i = 0; i < lh->file_names_size (); ++i)
@@ -3202,8 +3200,12 @@ dw2_get_real_path (struct objfile *objfile,
 		   struct quick_file_names *qfn, int index)
 {
   if (qfn->real_names == NULL)
-    qfn->real_names = OBSTACK_CALLOC (&objfile->objfile_obstack,
-				      qfn->num_file_names, const char *);
+    {
+      struct dwarf2_per_objfile *dwarf2_per_objfile
+	= get_dwarf2_per_objfile (objfile);
+      qfn->real_names = OBSTACK_CALLOC (&dwarf2_per_objfile->obstack,
+					qfn->num_file_names, const char *);
+    }
 
   if (qfn->real_names[index] == NULL)
     qfn->real_names[index] = gdb_realpath (qfn->file_names[index]).release ();
@@ -5753,7 +5755,7 @@ dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
 	{
 	  dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->get_cutu (i);
 
-	  per_cu->v.quick = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+	  per_cu->v.quick = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 					    struct dwarf2_per_cu_quick_data);
 	}
 
@@ -5909,10 +5911,7 @@ dwarf2_create_include_psymtab (const char *name, dwarf2_psymtab *pst,
   dwarf2_psymtab *subpst = new dwarf2_psymtab (name, objfile);
 
   if (!IS_ABSOLUTE_PATH (subpst->filename))
-    {
-      /* It shares objfile->objfile_obstack.  */
-      subpst->dirname = pst->dirname;
-    }
+    subpst->dirname = pst->dirname;
 
   subpst->dependencies = objfile->partial_symtabs->allocate_dependencies (1);
   subpst->dependencies[0] = pst;
@@ -6075,7 +6074,7 @@ create_debug_type_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile,
       if (dwo_file)
 	{
 	  sig_type = NULL;
-	  dwo_tu = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+	  dwo_tu = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 				   struct dwo_unit);
 	  dwo_tu->dwo_file = dwo_file;
 	  dwo_tu->signature = header.signature;
@@ -6089,7 +6088,7 @@ create_debug_type_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile,
 	  /* N.B.: type_offset is not usable if this type uses a DWO file.
 	     The real type_offset is in the DWO file.  */
 	  dwo_tu = NULL;
-	  sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+	  sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 				     struct signatured_type);
 	  sig_type->signature = header.signature;
 	  sig_type->type_offset_in_tu = header.type_cu_offset_in_tu;
@@ -6201,13 +6200,11 @@ static struct signatured_type *
 add_type_unit (struct dwarf2_per_objfile *dwarf2_per_objfile, ULONGEST sig,
 	       void **slot)
 {
-  struct objfile *objfile = dwarf2_per_objfile->objfile;
-
   if (dwarf2_per_objfile->all_type_units.size ()
       == dwarf2_per_objfile->all_type_units.capacity ())
     ++dwarf2_per_objfile->tu_stats.nr_all_type_units_reallocs;
 
-  signatured_type *sig_type = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+  signatured_type *sig_type = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 					      struct signatured_type);
 
   dwarf2_per_objfile->all_type_units.push_back (sig_type);
@@ -6216,7 +6213,7 @@ add_type_unit (struct dwarf2_per_objfile *dwarf2_per_objfile, ULONGEST sig,
   if (dwarf2_per_objfile->using_index)
     {
       sig_type->per_cu.v.quick =
-	OBSTACK_ZALLOC (&objfile->objfile_obstack,
+	OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 			struct dwarf2_per_cu_quick_data);
     }
 
@@ -7108,18 +7105,17 @@ create_type_unit_group (struct dwarf2_cu *cu, sect_offset line_offset_struct)
 {
   struct dwarf2_per_objfile *dwarf2_per_objfile
     = cu->per_cu->dwarf2_per_objfile;
-  struct objfile *objfile = dwarf2_per_objfile->objfile;
   struct dwarf2_per_cu_data *per_cu;
   struct type_unit_group *tu_group;
 
-  tu_group = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+  tu_group = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 			     struct type_unit_group);
   per_cu = &tu_group->per_cu;
   per_cu->dwarf2_per_objfile = dwarf2_per_objfile;
 
   if (dwarf2_per_objfile->using_index)
     {
-      per_cu->v.quick = OBSTACK_ZALLOC (&objfile->objfile_obstack,
+      per_cu->v.quick = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
 					struct dwarf2_per_cu_quick_data);
     }
   else
@@ -7837,13 +7833,13 @@ read_comp_units_from_section (struct dwarf2_per_objfile *dwarf2_per_objfile,
       /* Save the compilation unit for later lookup.  */
       if (cu_header.unit_type != DW_UT_type)
 	{
-	  this_cu = XOBNEW (&objfile->objfile_obstack,
+	  this_cu = XOBNEW (&dwarf2_per_objfile->obstack,
 			    struct dwarf2_per_cu_data);
 	  memset (this_cu, 0, sizeof (*this_cu));
 	}
       else
 	{
-	  auto sig_type = XOBNEW (&objfile->objfile_obstack,
+	  auto sig_type = XOBNEW (&dwarf2_per_objfile->obstack,
 				  struct signatured_type);
 	  memset (sig_type, 0, sizeof (*sig_type));
 	  sig_type->signature = cu_header.signature;
@@ -9985,7 +9981,8 @@ dw2_linkage_name (struct die_info *die, struct dwarf2_cu *cu)
    For Ada, return the DIE's linkage name rather than the fully qualified
    name.  PHYSNAME is ignored..
 
-   The result is allocated on the objfile_obstack and canonicalized.  */
+   The result is allocated on the dwarf2_per_objfile obstack and
+   canonicalized.  */
 
 static const char *
 dwarf2_compute_name (const char *name,
@@ -11145,7 +11142,8 @@ create_cus_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile,
       if (cus_htab == NULL)
 	cus_htab = allocate_dwo_unit_table (objfile);
 
-      dwo_unit = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct dwo_unit);
+      dwo_unit = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack,
+				 struct dwo_unit);
       *dwo_unit = read_unit;
       slot = htab_find_slot (cus_htab.get (), dwo_unit, INSERT);
       gdb_assert (slot != NULL);
@@ -11348,7 +11346,7 @@ create_dwp_hash_table (struct dwarf2_per_objfile *dwarf2_per_objfile,
 	     pulongest (nr_slots), dwp_file->name);
     }
 
-  htab = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct dwp_hash_table);
+  htab = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack, struct dwp_hash_table);
   htab->version = version;
   htab->nr_columns = nr_columns;
   htab->nr_units = nr_units;
@@ -11545,7 +11543,6 @@ create_dwo_unit_in_dwp_v1 (struct dwarf2_per_objfile *dwarf2_per_objfile,
 			   const char *comp_dir,
 			   ULONGEST signature, int is_debug_types)
 {
-  struct objfile *objfile = dwarf2_per_objfile->objfile;
   const struct dwp_hash_table *dwp_htab =
     is_debug_types ? dwp_file->tus : dwp_file->cus;
   bfd *dbfd = dwp_file->dbfd.get ();
@@ -11650,7 +11647,7 @@ create_dwo_unit_in_dwp_v1 (struct dwarf2_per_objfile *dwarf2_per_objfile,
 			      virtual_dwo_name.c_str ());
 	}
       dwo_file = new struct dwo_file;
-      dwo_file->dwo_name = obstack_strdup (&objfile->objfile_obstack,
+      dwo_file->dwo_name = obstack_strdup (&dwarf2_per_objfile->obstack,
 					   virtual_dwo_name);
       dwo_file->comp_dir = comp_dir;
       dwo_file->sections.abbrev = sections.abbrev;
@@ -11680,11 +11677,11 @@ create_dwo_unit_in_dwp_v1 (struct dwarf2_per_objfile *dwarf2_per_objfile,
       dwo_file = (struct dwo_file *) *dwo_file_slot;
     }
 
-  dwo_unit = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct dwo_unit);
+  dwo_unit = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack, struct dwo_unit);
   dwo_unit->dwo_file = dwo_file;
   dwo_unit->signature = signature;
   dwo_unit->section =
-    XOBNEW (&objfile->objfile_obstack, struct dwarf2_section_info);
+    XOBNEW (&dwarf2_per_objfile->obstack, struct dwarf2_section_info);
   *dwo_unit->section = sections.info_or_types;
   /* dwo_unit->{offset,length,type_offset_in_tu} are set later.  */
 
@@ -11745,7 +11742,6 @@ create_dwo_unit_in_dwp_v2 (struct dwarf2_per_objfile *dwarf2_per_objfile,
 			   const char *comp_dir,
 			   ULONGEST signature, int is_debug_types)
 {
-  struct objfile *objfile = dwarf2_per_objfile->objfile;
   const struct dwp_hash_table *dwp_htab =
     is_debug_types ? dwp_file->tus : dwp_file->cus;
   bfd *dbfd = dwp_file->dbfd.get ();
@@ -11846,7 +11842,7 @@ create_dwo_unit_in_dwp_v2 (struct dwarf2_per_objfile *dwarf2_per_objfile,
 			      virtual_dwo_name.c_str ());
 	}
       dwo_file = new struct dwo_file;
-      dwo_file->dwo_name = obstack_strdup (&objfile->objfile_obstack,
+      dwo_file->dwo_name = obstack_strdup (&dwarf2_per_objfile->obstack,
 					   virtual_dwo_name);
       dwo_file->comp_dir = comp_dir;
       dwo_file->sections.abbrev =
@@ -11890,11 +11886,11 @@ create_dwo_unit_in_dwp_v2 (struct dwarf2_per_objfile *dwarf2_per_objfile,
       dwo_file = (struct dwo_file *) *dwo_file_slot;
     }
 
-  dwo_unit = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct dwo_unit);
+  dwo_unit = OBSTACK_ZALLOC (&dwarf2_per_objfile->obstack, struct dwo_unit);
   dwo_unit->dwo_file = dwo_file;
   dwo_unit->signature = signature;
   dwo_unit->section =
-    XOBNEW (&objfile->objfile_obstack, struct dwarf2_section_info);
+    XOBNEW (&dwarf2_per_objfile->obstack, struct dwarf2_section_info);
   *dwo_unit->section = create_dwp_v2_section (dwarf2_per_objfile,
 					      is_debug_types
 					      ? &dwp_file->sections.types
@@ -12395,7 +12391,7 @@ open_and_init_dwp_file (struct dwarf2_per_objfile *dwarf2_per_objfile)
 
   dwp_file->num_sections = elf_numsections (dwp_file->dbfd);
   dwp_file->elf_sections =
-    OBSTACK_CALLOC (&objfile->objfile_obstack,
+    OBSTACK_CALLOC (&dwarf2_per_objfile->obstack,
 		    dwp_file->num_sections, asection *);
 
   bfd_map_over_sections (dwp_file->dbfd.get (),
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index b06c2e218c1..a7ac3b6b025 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -118,6 +118,11 @@ private:
 			const dwarf2_debug_sections &names);
 
 public:
+  /* Objects that can be shared across objfiles are stored in this
+     obstack or on the psymtab obstack, while objects that are
+     objfile-specific are stored on the objfile obstack.  */
+  auto_obstack obstack;
+
   dwarf2_section_info info {};
   dwarf2_section_info abbrev {};
   dwarf2_section_info line {};
-- 
2.17.2

Re: [PATCH 03/14] Introduce dwarf2_per_objfile::obstack

[Simon Marchi]

On 2020-02-15 11:54 a.m., Tom Tromey wrote:
> Currently much of the DWARF-related data is stored on the obfile
> obstack.  This prevents sharing this data across objfiles, so this
> patch adds a new obstack to dwarf2_per_objfile.
> 
> One way to check whether this is correct is to look at the remaining
> uses of objfile_obstack in the DWARF code and note that they all
> appear in the "full CU" code paths.
> 
> The converse -- storing per-objfile data on the shared obstack -- is
> not good, but it is just a memory leak, not a potential
> use-after-free.  Double-checking this would also be useful, though.

I looked and nothing wrong stood out, but there are a lot of uses and they
are not all trivial, so I might have very well missed some mistakes.

Kind of unrelated, but while reviewing this, I noticed:

- allocate_dwo_file_hash_table
- allocate_type_unit_groups_table
- allocate_signatured_type_table
- allocate_dwp_loaded_cutus_table
- allocate_dwo_unit_table

All have objfile parameters which are unused and could probably be removed.  In at least
one instance, it would allow removing ann objfile local variable.

> @@ -3202,8 +3200,12 @@ dw2_get_real_path (struct objfile *objfile,
>  		   struct quick_file_names *qfn, int index)
>  {
>    if (qfn->real_names == NULL)
> -    qfn->real_names = OBSTACK_CALLOC (&objfile->objfile_obstack,
> -				      qfn->num_file_names, const char *);
> +    {
> +      struct dwarf2_per_objfile *dwarf2_per_objfile
> +	= get_dwarf2_per_objfile (objfile);
> +      qfn->real_names = OBSTACK_CALLOC (&dwarf2_per_objfile->obstack,
> +					qfn->num_file_names, const char *);
> +    }

The callers of dw2_get_real_path should be able to pass the dwarf2_per_objfile directly,
instead of the objfile, simplifying this.

Simon

Re: [PATCH 03/14] Introduce dwarf2_per_objfile::obstack

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> +      struct dwarf2_per_objfile *dwarf2_per_objfile
>> +	= get_dwarf2_per_objfile (objfile);
>> +      qfn->real_names = OBSTACK_CALLOC (&dwarf2_per_objfile->obstack,
>> +					qfn->num_file_names, const char *);
>> +    }

Simon> The callers of dw2_get_real_path should be able to pass the dwarf2_per_objfile directly,
Simon> instead of the objfile, simplifying this.

Indeed.  I made this change.

Tom

[PATCH 02/14] Simplify setting of reading_partial_symbols

[Tom Tromey]

This simplifies the setting and clearing of reading_partial_symbols,
by using scoped_restore in the function that reads partial symbols.

gdb/ChangeLog
2020-02-15  Tom Tromey  <tom@tromey.com>

	* dwarf2read.c (dwarf2_build_psymtabs_hard): Use
	make_scoped_restore.
	(dwarf2_psymtab::read_symtab): Don't clear
	reading_partial_symbols.
---
 gdb/ChangeLog     | 7 +++++++
 gdb/dwarf2/read.c | 6 +++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index e373cc0af2c..81d8cbf0b13 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -7731,7 +7731,9 @@ dwarf2_build_psymtabs_hard (struct dwarf2_per_objfile *dwarf2_per_objfile)
 			  objfile_name (objfile));
     }
 
-  dwarf2_per_objfile->reading_partial_symbols = 1;
+  scoped_restore restore_reading_psyms
+    = make_scoped_restore (&dwarf2_per_objfile->reading_partial_symbols,
+			   true);
 
   dwarf2_per_objfile->info.read (objfile);
 
@@ -8705,8 +8707,6 @@ dwarf2_psymtab::read_symtab (struct objfile *objfile)
 	= dpo_backlink->has_section_at_zero;
     }
 
-  dwarf2_per_objfile->reading_partial_symbols = 0;
-
   expand_psymtab (objfile);
 
   process_cu_includes (dwarf2_per_objfile);
-- 
2.17.2

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Luis Machado]

On 2/15/20 1:54 PM, Tom Tromey wrote:
> A long-term goal for multi-inferior debugging has been to "split" an
> objfile, so that the bulk of the data can be shared across inferiors.
> Although a lot of progress has been made on this front, it has turned
> out to be surprisingly difficult to fully implement.
> 
> "Recently" (a year or two ago) I realized that we could get most of
> the benefits of this split by sharing partial symbol tables.  This is
> true because reading partial symbols is the slowest operation that
> users see -- in most cases, expanding a full symtab is reasonably
> quick.
> 
> Implementing this also turned out to be tricky; but unlike the
> situation with types and symbols, it was possible to do incrementally.
> 
> This series implements this idea for DWARF.  It separates
> dwarf2_per_objfile into shareable and unshareable parts; then, when
> possible, the object is attached to the BFD.
> 
> You can see the difference when timing "add-inferior -exec ./gdb",
> after "gdb ./gdb":
> 

Out of curiosity, what use cases would this cover? I imagine symbol data 
could be shared, with a few exceptions, only with binaries that are 
equal to the one we've already loaded/we're already debugging.

Is this a common scenario? Would type-sharing also be a common scenario?

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

>>>>> "Luis" == Luis Machado <luis.machado@linaro.org> writes:

>> "Recently" (a year or two ago) I realized that we could get most of
>> the benefits of this split by sharing partial symbol tables.  This is
>> true because reading partial symbols is the slowest operation that
>> users see -- in most cases, expanding a full symtab is reasonably
>> quick.

Luis> Out of curiosity, what use cases would this cover? I imagine symbol
Luis> data could be shared, with a few exceptions, only with binaries that
Luis> are equal to the one we've already loaded/we're already debugging.

Luis> Is this a common scenario?

I think it's uncommon right now, because multi-inferior debugging is
uncommon in general.

For multi-inferior debugging, I guess it would be more common for this
reuse to happen for shared libraries than for executables.

For instance, when debugging Firefox, most of the symbols are in one
giant shared library, libxul.  It is loaded once by the launcher, which
then execs and the new inferior loads it again.  (This case, I think,
still isn't handled -- there's no mechanism for preserving the BFD
across an exec yet.)  And then, Firefox is now multi-process, and each
such process also loads libxul.  So here at least, it would be a big
improvement.

> Would type-sharing also be a common scenario?

IMO the ideal would be for all symbols and types to be
objfile-independent.  The data does not inherently need to be
objfile-dependent, it is just an accident of history, where some earlier
programmer decided to bake the objfile offsets into symbol addresses.

We've made this transformation (to remove the offset from the
representation) for minimal and partial symbols, but for full symbols it
is just a lot harder to untangle everything.

Types, maybe surprisingly, fall into this because types can link to
symbols (see cplus_struct_type::template_arguments).

Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom de Vries]

On 15-02-2020 17:54, Tom Tromey wrote:
> A long-term goal for multi-inferior debugging has been to "split" an
> objfile, so that the bulk of the data can be shared across inferiors.
> Although a lot of progress has been made on this front, it has turned
> out to be surprisingly difficult to fully implement.
> 
> "Recently" (a year or two ago) I realized that we could get most of
> the benefits of this split by sharing partial symbol tables.  This is
> true because reading partial symbols is the slowest operation that
> users see -- in most cases, expanding a full symtab is reasonably
> quick.
> 
> Implementing this also turned out to be tricky; but unlike the
> situation with types and symbols, it was possible to do incrementally.
> 
> This series implements this idea for DWARF.  It separates
> dwarf2_per_objfile into shareable and unshareable parts; then, when
> possible, the object is attached to the BFD.
> 
> You can see the difference when timing "add-inferior -exec ./gdb",
> after "gdb ./gdb":
> 
> Before: Command execution time: 1.667661 (cpu), 1.687607 (wall)
> After : Command execution time: 0.150011 (cpu), 0.151292 (wall)
> 
> In this series I did not rename dwarf2_per_objfile.  I'd prefer to do
> this after landing this series; it seemed like a large, mostly
> cosmetic, and yet hard-to-rebase patch.  Also, I wanted to consult
> with everyone else about what we ought to call it.
> 
> I also didn't attempt to implement this sharing for CTF.  I don't
> really know much about CTF; but it seems like it ought to be possible.
> (Though I wonder if CTF wouldn't benefit more from simply bypassing
> partial symbols entirely.)
> 
> Regression tested on x86-64 Fedora 28.  I also did various tests by
> hand, and tested it by hand, under valgrind, on a gdb built with
> -fdebug-types-section.
> 
> Like I mentioned earlier, this is a tricky series, so it would benefit
> from careful review.  The main danger is that, if I missed a spot, we
> could end up in a situation where gdb will crash in a multi-inferior
> scenario.

I would like to take a look at this.

Is this tromey/t/reorganize-dwarf-code-sharing on your github?

Thanks,
- Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> Is this tromey/t/reorganize-dwarf-code-sharing on your github?

Yes.  Thanks for looking.  Simon also mentioned that he wanted to try
some changes.  I don't plan to check this in until I hear back from him.
If you have concerns / changes, I'm happy to wait for your investigation
as well.

Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Simon Marchi]

On 2020-02-15 11:54 a.m., Tom Tromey wrote:
> A long-term goal for multi-inferior debugging has been to "split" an
> objfile, so that the bulk of the data can be shared across inferiors.
> Although a lot of progress has been made on this front, it has turned
> out to be surprisingly difficult to fully implement.
> 
> "Recently" (a year or two ago) I realized that we could get most of
> the benefits of this split by sharing partial symbol tables.  This is
> true because reading partial symbols is the slowest operation that
> users see -- in most cases, expanding a full symtab is reasonably
> quick.
> 
> Implementing this also turned out to be tricky; but unlike the
> situation with types and symbols, it was possible to do incrementally.
> 
> This series implements this idea for DWARF.  It separates
> dwarf2_per_objfile into shareable and unshareable parts; then, when
> possible, the object is attached to the BFD.
> 
> You can see the difference when timing "add-inferior -exec ./gdb",
> after "gdb ./gdb":
> 
> Before: Command execution time: 1.667661 (cpu), 1.687607 (wall)
> After : Command execution time: 0.150011 (cpu), 0.151292 (wall)
> 
> In this series I did not rename dwarf2_per_objfile.  I'd prefer to do
> this after landing this series; it seemed like a large, mostly
> cosmetic, and yet hard-to-rebase patch.  Also, I wanted to consult
> with everyone else about what we ought to call it.
> 
> I also didn't attempt to implement this sharing for CTF.  I don't
> really know much about CTF; but it seems like it ought to be possible.
> (Though I wonder if CTF wouldn't benefit more from simply bypassing
> partial symbols entirely.)
> 
> Regression tested on x86-64 Fedora 28.  I also did various tests by
> hand, and tested it by hand, under valgrind, on a gdb built with
> -fdebug-types-section.
> 
> Like I mentioned earlier, this is a tricky series, so it would benefit
> from careful review.  The main danger is that, if I missed a spot, we
> could end up in a situation where gdb will crash in a multi-inferior
> scenario.
> 
> Tom

I've started to look at this, and this is my initial feeling (as discussed
with Tom on IRC).

This patch series implements:

  objfile -> dwarf2 object shared between objfiles -> dwarf2 objfile-specific object

With the last pointer getting updated at each entry point of the DWARF code to point
to the right "dwarf2 objfile-specific object" according to the context.

I think it would be more natural and easier to understand to have :

  objfile -> dwarf2 objfile-specific object -> dwarf2 object shared between objfiles

Many "dwarf2 objfile-specific" objects would point to one "dwarf2 object shared between
objfiles", without special tricks.  Of course, it's much easier said than done.

I'd like to know: did you consider this way of doing it and chose not to do it
because of the complexity / size of the change?  Is the current patchset a stepping
stone towards it?

Simon

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I think it would be more natural and easier to understand to have :

Simon>   objfile -> dwarf2 objfile-specific object -> dwarf2 object shared between objfiles

Simon> Many "dwarf2 objfile-specific" objects would point to one "dwarf2 object shared between
Simon> objfiles", without special tricks.  Of course, it's much easier said than done.

Simon> I'd like to know: did you consider this way of doing it and chose not to do it
Simon> because of the complexity / size of the change?  Is the current patchset a stepping
Simon> stone towards it?

I was afraid of the size of the resulting patch.  However, I think
you're right that this would be better.  And, I feel pretty strongly
that we should be making the DWARF reader less awful, not more.

Probably the thing to do is reorder the series a little (some patches
don't depend on this), then redo the "unsharing" patch, and finally
update the subsequent patches.

A few of these patches (1-3, 5, and 11) could probably go in sooner.
What do you think of that?

Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Simon Marchi]

On 2020-02-23 6:58 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> I think it would be more natural and easier to understand to have :
> 
> Simon>   objfile -> dwarf2 objfile-specific object -> dwarf2 object shared between objfiles
> 
> Simon> Many "dwarf2 objfile-specific" objects would point to one "dwarf2 object shared between
> Simon> objfiles", without special tricks.  Of course, it's much easier said than done.
> 
> Simon> I'd like to know: did you consider this way of doing it and chose not to do it
> Simon> because of the complexity / size of the change?  Is the current patchset a stepping
> Simon> stone towards it?
> 
> I was afraid of the size of the resulting patch.  However, I think
> you're right that this would be better.  And, I feel pretty strongly
> that we should be making the DWARF reader less awful, not more.

Yeah, as I already showed you, I started making such a patch (more details
below).  I think it's worth doing this work, because in the end it's easier
to follow the code.  I find that the dwarf2_enter_objfile approach is
analogous to having to set a "current context" (e.g. call switch_to_thread)
before calling a certain function, which goes against what we are trying
to do in general (for example, trying to change functions to accept a
thread / ptid rather than rely on inferior_ptid).

Since dwarf2_per_cu_data objects become objfile-independent, functions that
need to work on a dwarf2_per_cu_data in an objfile-dependent way then need
to accept an objfile or dwarf2_per_objfile object along with the
dwarf2_per_cu_data.  It's a bit tedious, but in the end it forces us to pass
the objfile context all the way from the callers, and it's less likely that
we forget a spot, like we could with dwarf2_enter_objfile, since it would just
not compile.

> Probably the thing to do is reorder the series a little (some patches
> don't depend on this), then redo the "unsharing" patch, and finally
> update the subsequent patches.
> 
> A few of these patches (1-3, 5, and 11) could probably go in sooner.
> What do you think of that?

#1 is already in, right?
#2: Yes, sounds like a good cleanup.
#4: Yes, good cleanup.

#3: It's only useful if we end up doing psymtabs sharing, so I would not push
    it yet.
#5 and #11: I'm not sure how much of that I end up un-doing in my prototype patch,
    swapping the shareable and unshareable parts.  And it is also only useful in
    the context of psymtab sharing, so I wouldn't push it yet.

I'm currently experimenting with what I suggested above, this is what I have for
now (this will get force-pushed as I progress):

  https://github.com/simark/binutils-gdb/commits/share-psymtabs

The status is: I'm based on your patch 9/14 ("Add objfile member to DWARF batons"),
as it aims mainly to replace patch patch 10/14 ("Introduce dwarf2_enter_objfile and
use it") with the architecture I proposed.  I see no regression in a simple
"make check" between it and the previous commit.  I have yet to apply and adapt the
rest of your series on top of my commit.  I would also like to test with the variety
of testsuite boards that Tom de Vries added, since they will test various DWARF
configurations (and I would feel bad if he had to clean up after us again).

Simon

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> A few of these patches (1-3, 5, and 11) could probably go in sooner.
>> What do you think of that?

Simon> #1 is already in, right?
Simon> #2: Yes, sounds like a good cleanup.
Simon> #4: Yes, good cleanup.

Simon> #3: It's only useful if we end up doing psymtabs sharing, so I would not push
Simon>     it yet.
Simon> #5 and #11: I'm not sure how much of that I end up un-doing in my prototype patch,
Simon>     swapping the shareable and unshareable parts.  And it is also only useful in
Simon>     the context of psymtab sharing, so I wouldn't push it yet.

Ugh, I used the wrong numbers.

#1 is already in, so really I mean #2-#4, #6, #12.

#2 (simplify setting of reading_partial_symbols)
#3 dwarf2_per_objfile::obstack
#4 TYPE_UNIT_GROUP method
#6 Add "objfile" parameter to two partial_symtab methods
#12 Fix a memory leak

Maybe the baton change could be done as well.

Simon> I'm currently experimenting with what I suggested above, this is what I have for
Simon> now (this will get force-pushed as I progress):

Simon>   https://github.com/simark/binutils-gdb/commits/share-psymtabs

Simon> The status is: I'm based on your patch 9/14 ("Add objfile member to DWARF batons"),
Simon> as it aims mainly to replace patch patch 10/14 ("Introduce dwarf2_enter_objfile and
Simon> use it") with the architecture I proposed.  I see no regression in a simple
Simon> "make check" between it and the previous commit.  I have yet to apply and adapt the
Simon> rest of your series on top of my commit.  I would also like to test with the variety
Simon> of testsuite boards that Tom de Vries added, since they will test various DWARF
Simon> configurations (and I would feel bad if he had to clean up after us again).

Yeah.

Take a look at my branch, t/reorganize-dwarf-code-sharing-3.
I rearranged it to move all the "unshared" stuff to the end, so there
should be less to fix up (though I see now I neglected to move the baton
patch earlier).

Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

Tom> Take a look at my branch, t/reorganize-dwarf-code-sharing-3.
Tom> I rearranged it to move all the "unshared" stuff to the end, so there
Tom> should be less to fix up (though I see now I neglected to move the baton
Tom> patch earlier).

I made this change, so now the branch looks like (order is reversed from
how git send-email would send it):

 0 eeeb07c1120 Share DWARF partial symtabs
 1 597fab7df48 Move signatured_type::type to unshareable object
 2 9ec85b37d88 Split type_unit_group
 3 f4d2c85a250 Introduce dwarf2_enter_objfile and use it
 4 afcc2a6d028 Remove symtab links from dwarf2_psymtab and dwarf2_per_cu_quick_data
 5 11fad00951b Introduce dwarf2_unshareable and move die_type_hash
 6 49fe37ffefb Add objfile member to DWARF batons
 7 842794fd4d9 Add dwarf2_per_cu_data::index
 8 cbf63c32179 Fix a memory leak and remove an unused member
 9 fe38cf467e1 Add "objfile" parameter to two partial_symtab methods
10 e0e096e6956 Convert IS_TYPE_UNIT_GROUP to method
11 ad71f2eb272 Introduce dwarf2_per_objfile::obstack
12 4a127c55a71 Simplify setting of reading_partial_symbols


Basically I think switching the sharing would start at patch #5, and
then parts of patch #0 would be salvaged, but not most of the rest of
patches #1-#5.

Also I have another patch here that adds a subclass of partial_symtab
for DWARF includes.  I *think* this solves the need to wrap the
compunit_symtab pointer in an optional<>, though it remains hard to be
completely certain.  I'll send that sometime soon, I think it could go
in separately.

Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom de Vries]

On 24-02-2020 04:22, Tom Tromey wrote:
> Tom> Take a look at my branch, t/reorganize-dwarf-code-sharing-3.

FWIW, I tried to build this earlier today and ran into a build error. [
Sorry, don't have the specifics available right now, my laptop is tied
up building a series of commits. ]

Thanks,
- Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom de Vries]

On 24-02-2020 14:42, Tom de Vries wrote:
> On 24-02-2020 04:22, Tom Tromey wrote:
>> Tom> Take a look at my branch, t/reorganize-dwarf-code-sharing-3.
> 
> FWIW, I tried to build this earlier today and ran into a build error. [
> Sorry, don't have the specifics available right now, my laptop is tied
> up building a series of commits. ]
> 

Here it is:
...
src/gdb/dwarf2/read.c:10967:41: error: macro "XOBNEWVEC" requires 3
arguments, but only 2 given
          line_header->file_names_size ());
                                         ^
src/gdb/dwarf2/read.c: In member function 'void
dwarf2_cu::setup_type_unit_groups(die_info*)':
/data/gdb_versions/devel/src/gdb/dwarf2/read.c:10966:17: error: 'struct
type_unit_group' has no member named 'symtabs'
       tu_group->symtabs = XOBNEWVEC (struct symtab *,
                 ^~~~~~~
src/gdb/dwarf2/read.c:10966:27: error: 'XOBNEWVEC' was not declared in
this scope
       tu_group->symtabs = XOBNEWVEC (struct symtab *,
                           ^~~~~~~~~
make[1]: *** [Makefile:1600: dwarf2/read.o] Error 1
...

Thanks,
- Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

>> FWIW, I tried to build this earlier today and ran into a build error. [
>> Sorry, don't have the specifics available right now, my laptop is tied
>> up building a series of commits. ]

Sorry about that.  I'll fix it up later.

Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

>>> FWIW, I tried to build this earlier today and ran into a build error. [
>>> Sorry, don't have the specifics available right now, my laptop is tied
>>> up building a series of commits. ]

Tom> Sorry about that.  I'll fix it up later.

I pushed the 3 simpler fixes to master, rebased my branch again, fixed
the build bugs, and force-pushed it to my github again.

Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Simon Marchi]

On 2020-02-23 10:22 p.m., Tom Tromey wrote:
> Tom> Take a look at my branch, t/reorganize-dwarf-code-sharing-3.
> Tom> I rearranged it to move all the "unshared" stuff to the end, so there
> Tom> should be less to fix up (though I see now I neglected to move the baton
> Tom> patch earlier).
> 
> I made this change, so now the branch looks like (order is reversed from
> how git send-email would send it):
> 
>  0 eeeb07c1120 Share DWARF partial symtabs
>  1 597fab7df48 Move signatured_type::type to unshareable object
>  2 9ec85b37d88 Split type_unit_group
>  3 f4d2c85a250 Introduce dwarf2_enter_objfile and use it
>  4 afcc2a6d028 Remove symtab links from dwarf2_psymtab and dwarf2_per_cu_quick_data
>  5 11fad00951b Introduce dwarf2_unshareable and move die_type_hash
>  6 49fe37ffefb Add objfile member to DWARF batons
>  7 842794fd4d9 Add dwarf2_per_cu_data::index
>  8 cbf63c32179 Fix a memory leak and remove an unused member
>  9 fe38cf467e1 Add "objfile" parameter to two partial_symtab methods
> 10 e0e096e6956 Convert IS_TYPE_UNIT_GROUP to method
> 11 ad71f2eb272 Introduce dwarf2_per_objfile::obstack
> 12 4a127c55a71 Simplify setting of reading_partial_symbols
> 
> 
> Basically I think switching the sharing would start at patch #5, and
> then parts of patch #0 would be salvaged, but not most of the rest of
> patches #1-#5.

Ok, thanks for doing this.

I don't undersatnd why you say that patches #1-#5 won't be salvaged, I think
they will still be necessary.  For example, #1 and #2 are still necessary to
take out objfile-specific parts from dwarf2_per_cu_data, aren't they?

I would just replace patch #4 (with probably a few consecutive patches), and
probably alter patch #5 to maybe change the naming.

Naming-wise, I'm planning to use:

  objfile -> dwarf2_per_objfile -> dwarf2_shared_between_objfiles

I find the last one a bit long, although it's clear.  I'm open to any suggestions.

I am also thinking of renaming dwarf2_per_cu_data to just dwarf2_per_cu.  I think it
would be coherent with dwarf2_per_objfile.

> Also I have another patch here that adds a subclass of partial_symtab
> for DWARF includes.  I *think* this solves the need to wrap the
> compunit_symtab pointer in an optional<>, though it remains hard to be
> completely certain.  I'll send that sometime soon, I think it could go
> in separately.

I don't really know about that and the current work uses enough of my brain
cycles, so I could take a look, but only once we are done with the current
series (unless you think it impacts the current series).

Simon

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> 0 eeeb07c1120 Share DWARF partial symtabs
>> 1 597fab7df48 Move signatured_type::type to unshareable object
>> 2 9ec85b37d88 Split type_unit_group
>> 3 f4d2c85a250 Introduce dwarf2_enter_objfile and use it
>> 4 afcc2a6d028 Remove symtab links from dwarf2_psymtab and dwarf2_per_cu_quick_data
>> 5 11fad00951b Introduce dwarf2_unshareable and move die_type_hash
>> 6 49fe37ffefb Add objfile member to DWARF batons
>> 7 842794fd4d9 Add dwarf2_per_cu_data::index
>> 8 cbf63c32179 Fix a memory leak and remove an unused member
>> 9 fe38cf467e1 Add "objfile" parameter to two partial_symtab methods
>> 10 e0e096e6956 Convert IS_TYPE_UNIT_GROUP to method
>> 11 ad71f2eb272 Introduce dwarf2_per_objfile::obstack
>> 12 4a127c55a71 Simplify setting of reading_partial_symbols

Simon> I don't undersatnd why you say that patches #1-#5 won't be salvaged, I think
Simon> they will still be necessary.  For example, #1 and #2 are still necessary to
Simon> take out objfile-specific parts from dwarf2_per_cu_data, aren't they?

Yeah, I suppose so.  I think the goal should be to eliminate the
dwarf2_enter_objfile stuff, so I suppose really only one patch would be
wholly gone; and dwarf2_unshareable changed to sort of the "opposite"
patch.

Simon>   objfile -> dwarf2_per_objfile -> dwarf2_shared_between_objfiles

Simon> I find the last one a bit long, although it's clear.  I'm open to
Simon> any suggestions.

We talked on irc and thought dwarf2_per_bfd sounded ok, given that this
naming is already used for objfile data.

>> Also I have another patch here that adds a subclass of partial_symtab
>> for DWARF includes.  I *think* this solves the need to wrap the
>> compunit_symtab pointer in an optional<>, though it remains hard to be
>> completely certain.  I'll send that sometime soon, I think it could go
>> in separately.

Simon> I don't really know about that and the current work uses enough of my brain
Simon> cycles, so I could take a look, but only once we are done with the current
Simon> series (unless you think it impacts the current series).

If it could eliminate the optional, it would simplify some of the code.
I'll look at that a bit.

Tom

Re: [PATCH 00/14] Share DWARF partial symtabs between objfiles

[Tom Tromey]

Simon> #1 is already in, right?
Simon> #2: Yes, sounds like a good cleanup.
Simon> #4: Yes, good cleanup.

I'm going to push these soon.

Tom