[v3] gdb: Cache line headers in DWARF per objfile

Message ID 20240430180547.1337554-1-hawkinsw@obs.cr
State New
Headers
Series [v3] gdb: Cache line headers in DWARF per objfile |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Will Hawkins April 30, 2024, 6:05 p.m. UTC
  When the line headers of a DWARF unit are read, make sure that they are
available for reuse by storing them in the line header hash of the per
objfile.  A view of that data structure will be given to each of the
dwarf2_cus as they are instantiated for reading debugging information.
As a result, we can simplify the scoped RAII object (by removing code
for deallocating a line header upon the completion of reading DWARF
debugging information).

Tested on x86_64-redhat-linux and aarch64-linux-gnu.

Signed-off-by: Will Hawkins <hawkinsw@obs.cr>
---
    v2 -> v3:
        - Address helpful feedback.
        - Consolidate line header cache into line_header_hash.
    v1 -> v2:
        - Add back the in-process die RAII object to protect against
          infinite recursion.

 gdb/dwarf2/cu.h          |   6 --
 gdb/dwarf2/line-header.c |   3 +-
 gdb/dwarf2/line-header.h |  10 ++-
 gdb/dwarf2/read.c        | 164 +++++++++++++++++----------------------
 4 files changed, 80 insertions(+), 103 deletions(-)
  

Patch

diff --git a/gdb/dwarf2/cu.h b/gdb/dwarf2/cu.h
index 58e89960aad..c2e02a0db13 100644
--- a/gdb/dwarf2/cu.h
+++ b/gdb/dwarf2/cu.h
@@ -158,12 +158,6 @@  struct dwarf2_cu
 
   /* Header data from the line table, during full symbol processing.  */
   struct line_header *line_header = nullptr;
-  /* Non-NULL if LINE_HEADER is owned by this DWARF_CU.  Otherwise,
-     it's owned by dwarf2_per_bfd::line_header_hash.  If non-NULL,
-     this is the DW_TAG_compile_unit die for this CU.  We'll hold on
-     to the line header as long as this DIE is being processed.  See
-     process_die_scope.  */
-  die_info *line_header_die_owner = nullptr;
 
   /* A list of methods which need to have physnames computed
      after all type information has been read.  */
diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c
index eddb2ef7ae8..7bf4f324ce9 100644
--- a/gdb/dwarf2/line-header.c
+++ b/gdb/dwarf2/line-header.c
@@ -254,7 +254,7 @@  read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd,
 
 line_header_up
 dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
-			   dwarf2_per_objfile *per_objfile,
+			   bool is_partial, dwarf2_per_objfile *per_objfile,
 			   struct dwarf2_section_info *section,
 			   const struct comp_unit_head *cu_header,
 			   const char *comp_dir)
@@ -278,6 +278,7 @@  dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
 
   lh->sect_off = sect_off;
   lh->offset_in_dwz = is_dwz;
+  lh->partial_unit = is_partial;
 
   line_ptr = section->buffer + to_underlying (sect_off);
 
diff --git a/gdb/dwarf2/line-header.h b/gdb/dwarf2/line-header.h
index c068dff70a3..a2e1d92e1f9 100644
--- a/gdb/dwarf2/line-header.h
+++ b/gdb/dwarf2/line-header.h
@@ -74,14 +74,15 @@  struct line_header
      unit in the context of which we are reading this line header, or nullptr
      if unknown or not applicable.  */
   explicit line_header (const char *comp_dir)
-    : offset_in_dwz {}, m_comp_dir (comp_dir)
+    : offset_in_dwz {}, partial_unit {}, m_comp_dir (comp_dir)
   {}
 
   /* This constructor should only be used to create line_header instances to do
      hash table lookups.  */
-  line_header (sect_offset sect_off, bool offset_in_dwz)
+  line_header (sect_offset sect_off, bool offset_in_dwz, bool partial_unit)
     : sect_off (sect_off),
-      offset_in_dwz (offset_in_dwz)
+      offset_in_dwz (offset_in_dwz),
+      partial_unit (partial_unit)
   {}
 
   /* Add an entry to the include directory table.  */
@@ -147,6 +148,7 @@  struct line_header
 
   /* OFFSET is for struct dwz_file associated with dwarf2_per_objfile.  */
   unsigned offset_in_dwz : 1; /* Can't initialize bitfields in-class.  */
+  unsigned partial_unit  : 1; /* Can't initialize bitfields in-class.  */
 
   unsigned short version {};
   unsigned char minimum_instruction_length {};
@@ -213,7 +215,7 @@  file_entry::include_dir (const line_header *lh) const
    and must not be freed.  */
 
 extern line_header_up dwarf_decode_line_header
-  (sect_offset sect_off, bool is_dwz, dwarf2_per_objfile *per_objfile,
+  (sect_offset sect_off, bool is_dwz, bool is_partial, dwarf2_per_objfile *per_objfile,
    struct dwarf2_section_info *section, const struct comp_unit_head *cu_header,
    const char *comp_dir);
 
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 061db8c2a2a..e5bfa4085b4 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1233,7 +1233,8 @@  dwarf2_per_objfile::relocate (unrelocated_addr addr)
 static hashval_t
 line_header_hash (const struct line_header *ofs)
 {
-  return to_underlying (ofs->sect_off) ^ ofs->offset_in_dwz;
+  uint8_t xors = (ofs->offset_in_dwz << 1 | ofs->partial_unit);
+  return to_underlying (ofs->sect_off) ^ xors;
 }
 
 /* Hash function for htab_create_alloc_ex for line_header_hash.  */
@@ -1255,7 +1256,8 @@  line_header_eq_voidp (const void *item_lhs, const void *item_rhs)
   const struct line_header *ofs_rhs = (const struct line_header *) item_rhs;
 
   return (ofs_lhs->sect_off == ofs_rhs->sect_off
-	  && ofs_lhs->offset_in_dwz == ofs_rhs->offset_in_dwz);
+	  && ofs_lhs->offset_in_dwz == ofs_rhs->offset_in_dwz
+	  && ofs_lhs->partial_unit == ofs_rhs->partial_unit);
 }
 
 /* See declaration.  */
@@ -6385,8 +6387,8 @@  process_imported_unit_die (struct die_info *die, struct dwarf2_cu *cu)
 class process_die_scope
 {
 public:
-  process_die_scope (die_info *die, dwarf2_cu *cu)
-    : m_die (die), m_cu (cu)
+  process_die_scope (die_info *die)
+    : m_die (die)
   {
     /* We should only be processing DIEs not already in process.  */
     gdb_assert (!m_die->in_process);
@@ -6396,20 +6398,10 @@  class process_die_scope
   ~process_die_scope ()
   {
     m_die->in_process = false;
-
-    /* If we're done processing the DIE for the CU that owns the line
-       header, we don't need the line header anymore.  */
-    if (m_cu->line_header_die_owner == m_die)
-      {
-	delete m_cu->line_header;
-	m_cu->line_header = NULL;
-	m_cu->line_header_die_owner = NULL;
-      }
   }
 
 private:
   die_info *m_die;
-  dwarf2_cu *m_cu;
 };
 
 /* Process a die and its children.  */
@@ -6417,7 +6409,7 @@  class process_die_scope
 static void
 process_die (struct die_info *die, struct dwarf2_cu *cu)
 {
-  process_die_scope scope (die, cu);
+  process_die_scope scope (die);
 
   switch (die->tag)
     {
@@ -7312,38 +7304,25 @@  find_file_and_directory (struct die_info *die, struct dwarf2_cu *cu)
   return *cu->per_cu->fnd;
 }
 
-/* Handle DW_AT_stmt_list for a compilation unit.
-   DIE is the DW_TAG_compile_unit die for CU.
-   COMP_DIR is the compilation directory.  LOWPC is passed to
-   dwarf_decode_lines.  See dwarf_decode_lines comments about it.  */
-
 static void
-handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
-			const file_and_directory &fnd, unrelocated_addr lowpc,
-			bool have_code) /* ARI: editCase function */
+ensure_line_header_read (dwarf2_cu *cu, die_info *die,
+			 const file_and_directory &fnd,
+			 sect_offset line_offset)
 {
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
-  struct attribute *attr;
   hashval_t line_header_local_hash;
+  bool is_partial_unit = (die->tag == DW_TAG_partial_unit);
   void **slot;
-  int decode_mapping;
 
-  gdb_assert (! cu->per_cu->is_debug_types);
+  /* There is a single place for storing/finding line headers: Line
+     Header Hash.  The decoded line header is owned by the line
+     header hash.  The job of this function is to simply give out a
+     copy of the appropriate entry in the hash.  Of course, the first
+     time that the line header is decoded it must be put in the proper
+     place according to the rules above.  That is the other job of
+     this function.  */
 
-  attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
-  if (attr == NULL || !attr->form_is_unsigned ())
-    return;
-
-  sect_offset line_offset = (sect_offset) attr->as_unsigned ();
-
-  /* The line header hash table is only created if needed (it exists to
-     prevent redundant reading of the line table for partial_units).
-     If we're given a partial_unit, we'll need it.  If we're given a
-     compile_unit, then use the line header hash table if it's already
-     created, but don't create one just yet.  */
-
-  if (per_objfile->line_header_hash == NULL
-      && die->tag == DW_TAG_partial_unit)
+  if (per_objfile->line_header_hash == nullptr)
     {
       per_objfile->line_header_hash
 	.reset (htab_create_alloc (127, line_header_hash_voidp,
@@ -7352,60 +7331,63 @@  handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
 				   xcalloc, xfree));
     }
 
-  line_header line_header_local (line_offset, cu->per_cu->is_dwz);
+  line_header line_header_local (line_offset,
+				 cu->per_cu->is_dwz,
+				 is_partial_unit);
   line_header_local_hash = line_header_hash (&line_header_local);
-  if (per_objfile->line_header_hash != NULL)
-    {
-      slot = htab_find_slot_with_hash (per_objfile->line_header_hash.get (),
-				       &line_header_local,
-				       line_header_local_hash, NO_INSERT);
 
-      /* For DW_TAG_compile_unit we need info like symtab::linetable which
-	 is not present in *SLOT (since if there is something in *SLOT then
-	 it will be for a partial_unit).  */
-      if (die->tag == DW_TAG_partial_unit && slot != NULL)
-	{
-	  gdb_assert (*slot != NULL);
-	  cu->line_header = (struct line_header *) *slot;
-	  return;
-	}
+  slot = htab_find_slot_with_hash (per_objfile->line_header_hash.get (),
+				   &line_header_local,
+				   line_header_local_hash, NO_INSERT);
+
+  /* There is a matching entry. We are done.  */
+  if (slot != nullptr)
+    {
+      gdb_assert (*slot != nullptr);
+      cu->line_header = (struct line_header *) *slot;
+      return;
     }
 
-  /* dwarf_decode_line_header does not yet provide sufficient information.
-     We always have to call also dwarf_decode_lines for it.  */
   line_header_up lh = dwarf_decode_line_header (line_offset, cu,
 						fnd.get_comp_dir ());
-  if (lh == NULL)
+  if (lh == nullptr)
     return;
 
-  cu->line_header = lh.release ();
-  cu->line_header_die_owner = die;
+  slot = htab_find_slot_with_hash (per_objfile->line_header_hash.get (),
+				   &line_header_local,
+				   line_header_local_hash, INSERT);
+  gdb_assert (slot != nullptr);
+
+  /* This newly decoded line number information unit will be owned
+     by line_header_hash hash table.  */
+  *slot = cu->line_header = lh.release ();
+
+}
+
+/* Handle DW_AT_stmt_list for a compilation unit.
+   DIE is the DW_TAG_compile_unit die for CU.
+   COMP_DIR is the compilation directory.  LOWPC is passed to
+   dwarf_decode_lines.  See dwarf_decode_lines comments about it.  */
+
+static void
+handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
+			const file_and_directory &fnd, unrelocated_addr lowpc,
+			bool have_code) /* ARI: editCase function */
+{
+  struct attribute *attr;
+  int decode_mapping;
+
+  gdb_assert (! cu->per_cu->is_debug_types);
+
+  attr = dwarf2_attr (die, DW_AT_stmt_list, cu);
+  if (attr == NULL || !attr->form_is_unsigned ())
+    return;
+
+  sect_offset line_offset = (sect_offset) attr->as_unsigned ();
+
+  ensure_line_header_read (cu, die, fnd, line_offset);
+  ensure_line_header_read (cu, die, fnd, line_offset);
 
-  if (per_objfile->line_header_hash == NULL)
-    slot = NULL;
-  else
-    {
-      slot = htab_find_slot_with_hash (per_objfile->line_header_hash.get (),
-				       &line_header_local,
-				       line_header_local_hash, INSERT);
-      gdb_assert (slot != NULL);
-    }
-  if (slot != NULL && *slot == NULL)
-    {
-      /* This newly decoded line number information unit will be owned
-	 by line_header_hash hash table.  */
-      *slot = cu->line_header;
-      cu->line_header_die_owner = NULL;
-    }
-  else
-    {
-      /* We cannot free any current entry in (*slot) as that struct line_header
-	 may be already used by multiple CUs.  Create only temporary decoded
-	 line_header for this CU - it may happen at most once for each line
-	 number information unit.  And if we're not using line_header_hash
-	 then this is what we want as well.  */
-      gdb_assert (die->tag != DW_TAG_partial_unit);
-    }
   decode_mapping = (die->tag != DW_TAG_partial_unit);
   /* The have_code check is here because, if LOWPC and HIGHPC are both 0x0,
      then there won't be any interesting code in the CU, but a check later on
@@ -7539,13 +7521,13 @@  dwarf2_cu::setup_type_unit_groups (struct die_info *die)
 
   /* We have to handle the case of both a missing DW_AT_stmt_list or bad
      debug info.  */
-  line_header_up lh;
   if (attr != NULL && attr->form_is_unsigned ())
     {
+      file_and_directory &fnd = find_file_and_directory (die, this);
       sect_offset line_offset = (sect_offset) attr->as_unsigned ();
-      lh = dwarf_decode_line_header (line_offset, this, nullptr);
+      ensure_line_header_read (this, die, fnd, line_offset);
     }
-  if (lh == NULL)
+  if (line_header == nullptr)
     {
       if (first_time)
 	start_compunit_symtab ("", NULL, 0);
@@ -7564,9 +7546,6 @@  dwarf2_cu::setup_type_unit_groups (struct die_info *die)
       return;
     }
 
-  line_header = lh.release ();
-  line_header_die_owner = die;
-
   if (first_time)
     {
       struct compunit_symtab *cust = start_compunit_symtab ("", NULL, 0);
@@ -17796,6 +17775,7 @@  dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu,
 {
   struct dwarf2_section_info *section;
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
+  bool is_partial = (cu->per_cu->unit_type () ==  DW_UT_partial);
 
   section = get_debug_line_section (cu);
   section->read (per_objfile->objfile);
@@ -17808,7 +17788,7 @@  dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu,
       return 0;
     }
 
-  return dwarf_decode_line_header (sect_off, cu->per_cu->is_dwz,
+  return dwarf_decode_line_header (sect_off, cu->per_cu->is_dwz, is_partial,
 				   per_objfile, section, &cu->header,
 				   comp_dir);
 }