[PATCH v2] gdb: Cache line headers in DWARF per CU

[Will Hawkins <hawkinsw@obs.cr>]

When the line headers of a DWARF unit are read, make sure that they are
available for reuse by storing them in the per CU. 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>
---
    v1 -> v2:
      - Add back the in-process die RAII object to protect against
        infinite recursion.

 gdb/dwarf2/cu.h   |   6 ---
 gdb/dwarf2/read.c | 108 +++++++++++++++++++++++++++-------------------
 gdb/dwarf2/read.h |   9 ++++
 3 files changed, 72 insertions(+), 51 deletions(-)

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/read.c b/gdb/dwarf2/read.c
index 061db8c2a2a..71d19510491 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -6385,8 +6385,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 +6396,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 +6407,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,29 +7302,38 @@ 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 (struct dwarf2_cu *cu, struct die_info *die,
+			 const file_and_directory &fnd,
+			 sect_offset line_offset)
 {
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
-  struct attribute *attr;
+  dwarf2_per_cu_data *per_cu = cu->per_cu;
   hashval_t line_header_local_hash;
   void **slot;
-  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 ();
+  /* There are two places for storing/finding line headers:
+     1. Per CU: When DIE is not a partial unit, the decoded
+	line header is owned by the per CU. In this case, the
+	job of this function is to simply give out a copy of
+	that pointer to CU -- the per cu outlives CU so this
+	lending is safe.
+     2. Line Header Hash: When the DIE is a partial unit, the
+	decoded line header is owned by the line header hash.
+	In this case, the job of this function is to simply
+	give out a copy of the appropriate entry in the hash.
+	Caveat: If a decoded line header of a partial unit
+	does not fit in the line header hash, it will be
+	stored in/owned by the per cu.
+    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.  */
+
+  if (per_cu->line_headers != NULL)
+    {
+      cu->line_header = per_cu->line_headers.get ();
+      return;
+    }
 
   /* The line header hash table is only created if needed (it exists to
      prevent redundant reading of the line table for partial_units).
@@ -7378,9 +7377,6 @@ handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
   if (lh == NULL)
     return;
 
-  cu->line_header = lh.release ();
-  cu->line_header_die_owner = die;
-
   if (per_objfile->line_header_hash == NULL)
     slot = NULL;
   else
@@ -7394,18 +7390,43 @@ handle_DW_AT_stmt_list (struct die_info *die, struct dwarf2_cu *cu,
     {
       /* 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;
+      *slot = cu->line_header = lh.release ();
     }
   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.  */
+	 may be already used by multiple CUs.  Therefore, this newly read line
+	 header will be owned by the per_cu.  */
       gdb_assert (die->tag != DW_TAG_partial_unit);
+
+      per_cu->line_headers = std::move (lh);
+      cu->line_header = per_cu->line_headers.get ();
     }
+}
+
+/* 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);
+
   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 +7560,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 == NULL)
     {
       if (first_time)
 	start_compunit_symtab ("", NULL, 0);
@@ -7564,9 +7585,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);
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 73def88c4c0..ed659fbd4ca 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -25,6 +25,7 @@
 #include "dwarf2/comp-unit-head.h"
 #include "dwarf2/file-and-dir.h"
 #include "dwarf2/index-cache.h"
+#include "dwarf2/line-header.h"
 #include "dwarf2/mapped-index.h"
 #include "dwarf2/section.h"
 #include "dwarf2/cu.h"
@@ -222,6 +223,14 @@ struct dwarf2_per_cu_data
      have one.  */
   std::unique_ptr<file_and_directory> fnd;
 
+  /* The decoded line headers for this CU.  This is cached so that
+     there is no need to refetch it repeatedly. ensure_line_headers_read
+     in read.c is responsible for transferring a view of this structure
+     to dwarf2_cus as they are instantiated. This may be nullptr when
+     the decoded line header is owned by the line header hash associated
+     with the per objfile in the presence of a partial unit.  */
+  std::unique_ptr<struct line_header> line_headers;
+
   /* The file table.  This can be NULL if there was no file table
      or it's currently not read in.
      NOTE: This points into dwarf2_per_objfile->per_bfd->quick_file_names_table.  */
-- 
2.44.0