public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf.
@ 2021-11-19  6:56 Achra, Nitika
  2021-11-19 18:55 ` Keith Seitz
  0 siblings, 1 reply; 7+ messages in thread
From: Achra, Nitika @ 2021-11-19  6:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: George, Jini Susan

[-- Attachment #1: Type: text/plain, Size: 668 bytes --]

[AMD Official Use Only]

Requesting to please review the attached patch.

Command used: make check RUNTESTFLAGS='CC_FOR_TARGET="clang -gdwarf-5 -gsplit-dwarf -fdebug-macro"'
                                         TESTS="gdb.base/macscp.exp"

Before the patch:
Number of expected passes            234
Number of unexpected failures        65
Number of known failures             40

After the patch:
Number of expected passes            315
Number of unexpected failures        1
Number of known failures             23

Compiler used: clang 12.0.0

Regards,
Nitika

Sent from Mail<https://go.microsoft.com/fwlink/?LinkId=550986> for Windows


[-- Attachment #2: 0001-Fix-for-the-gdb.base-macscp.exp-testcase-failure-wit.patch --]
[-- Type: application/octet-stream, Size: 30206 bytes --]

From d8460a4f4a8924a427fb8ec193ab5d932135060c Mon Sep 17 00:00:00 2001
From: nitachra <Nitika.Achra@amd.com>
Date: Fri, 1 Oct 2021 01:11:24 +0530
Subject: [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split
 dwarf.

Command used: make check RUNTESTFLAGS='CC_FOR_TARGET="clang -gdwarf-5 -gsplit-dwarf -fdebug-macro"'
			 TESTS="gdb.base/macscp.exp"

Before the patch:
Number of expected passes            234
Number of unexpected failures        65
Number of known failures             40

After the patch:
Number of expected passes            315
Number of unexpected failures        1
Number of known failures             23

Compiler used: clang 12.0.0

gdb/ChangeLog:

	* dwarf2/line-header.c (add_include_dir): Handle .debug_line.dwo
	  include directories table entries.
	* (add_file_name): Handle .debug_line.dwo file names entries.
	* (file_file_name): Read the file name from .debug_line.dwo or
	  .debug_line sections.
	* (read_formatted_entries): Handle the .debug_line.dwo section.
	* (dwarf_decode_line_header): Read both .debug_line and
	   .debug_line.dwo headers.
	* dwarf2/line-header.h (include_dir): Handle the .debug_line.dwo
	  section entries.
	* (add_include_dir): Add an entry to the m_include_dirs vector or
	  to m_dwo_include_dirs vector if reading .debug_line.dwo
	  section.
	* (add_file_name): Add an entry to the m_file_names vector or to
	  the m_dwo_file_names vector if reading dwo section.
	* (include_dir_at): Return the include dir from m_include_dirs or
	  from the m_dwo_include_dirs.
	* (is_valid_dwo_file_index): Added this new function which will
	  check if the index is valid index in file_names table in
	  .debug_line.dwo.
	* (file_name_at): Return the file name from m_file_names or
	  from m_dwo_file_names vector.
	* (m_dwo_file_names): Return the file names vector containg the
	   .debug_line.dwo section file names entries.
	* (struct line_header): Added .debug_line.dwo header fields.
	* (m_dwo_include_dirs): New vector to contain include directories
	  of .debug_line.dwo.
	* (m_dwo_file_names): New vector to contain file names entries of
	  .debug_line.dwo section.
	* dwarf2/read.c (compute_include_file_name): Added a parameter
	  to check if the dwo is true.
	* (dw2_get_file_names_reader): Handle the file names entries from
	  .debug_line.dwo section.
	* (dwarf_decode_line_header): Read both .debug_line and
	  .debug_line.dwo section. If the CU type is debug_types, read
	  only the .debug_line.dwo section.
	* (compute_include_file_name). Handle the dwo section.
	* (class lnp_state_machine): For state machine, always read the
	  .debug_line section because line table is present only in the primary
	  file.
	* (handle_set_file): Likewise.
	* (dwarf_decode_lines_1): Likewise.
	* (new_symbol): Add a parameter in file_name_at function to
	   handle the dwo unit.
	* gdb/macrotab.c (compare_locations): Compare the file names
	  instaed of macro source file pointers because there are duplicate file
	  names in .debug_line and .debug_line.dwo sections. So, the pointers may
	  not match even if the files are same.
---
 gdb/dwarf2/line-header.c | 159 +++++++++++++++++++++++++--------------
 gdb/dwarf2/line-header.h |  89 ++++++++++++++++------
 gdb/dwarf2/read.c        | 106 +++++++++++++++++++++-----
 gdb/macrotab.c           |   4 +-
 4 files changed, 260 insertions(+), 98 deletions(-)

diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c
index 15195764c89..deeb28f361e 100644
--- a/gdb/dwarf2/line-header.c
+++ b/gdb/dwarf2/line-header.c
@@ -26,58 +26,69 @@
 #include "filenames.h"
 
 void
-line_header::add_include_dir (const char *include_dir)
+line_header::add_include_dir (const char *include_dir, bool is_dwo)
 {
   if (dwarf_line_debug >= 2)
     {
       size_t new_size;
+      int include_dir_size = is_dwo ? m_dwo_include_dirs.size ():
+			     m_include_dirs.size ();
       if (version >= 5)
-	new_size = m_include_dirs.size ();
+	new_size = include_dir_size;
       else
-	new_size = m_include_dirs.size () + 1;
+	new_size = include_dir_size + 1;
       fprintf_unfiltered (gdb_stdlog, "Adding dir %zu: %s\n",
 			  new_size, include_dir);
     }
-  m_include_dirs.push_back (include_dir);
+
+    if (is_dwo)
+      m_dwo_include_dirs.push_back (include_dir);
+    else
+      m_include_dirs.push_back (include_dir);
 }
 
 void
 line_header::add_file_name (const char *name,
 			    dir_index d_index,
 			    unsigned int mod_time,
-			    unsigned int length)
+			    unsigned int length,
+			    bool is_dwo)
 {
   if (dwarf_line_debug >= 2)
     {
       size_t new_size;
+      bool file_name_size = is_dwo ? dwo_file_names_size ():
+			     file_names_size ();
       if (version >= 5)
-	new_size = file_names_size ();
+	new_size = file_name_size;
       else
-	new_size = file_names_size () + 1;
+	new_size = file_name_size + 1;
       fprintf_unfiltered (gdb_stdlog, "Adding file %zu: %s\n",
 			  new_size, name);
     }
-  m_file_names.emplace_back (name, d_index, mod_time, length);
+
+    if (is_dwo)
+      m_dwo_file_names.emplace_back (name, d_index, mod_time, length);
+    else
+      m_file_names.emplace_back (name, d_index, mod_time, length);
 }
 
 gdb::unique_xmalloc_ptr<char>
 line_header::file_file_name (int file) const
 {
   /* Is the file number a valid index into the line header's file name
-     table?  Remember that file numbers start with one, not zero.  */
-  if (is_valid_file_index (file))
+     table?  First check in the .debug_line.dwo file table and then in
+     .debug_line table. Remember that file numbers start with one in
+     DWARFv4 and with zero in DWARFv5.  */
+  if (is_valid_dwo_file_index (file))
     {
-      const file_entry *fe = file_name_at (file);
-
-      if (!IS_ABSOLUTE_PATH (fe->name))
-	{
-	  const char *dir = fe->include_dir (this);
-	  if (dir != NULL)
-	    return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
-							  fe->name,
-							  (char *) NULL));
-	}
-      return make_unique_xstrdup (fe->name);
+      const file_entry *fe = file_name_at (file, true);
+      return make_unique_xstrdup (fe->symtab->filename);
+    }
+  else if (is_valid_file_index (file))
+    {
+      const file_entry *fe = file_name_at (file, false);
+      return make_unique_xstrdup (fe->symtab->filename);
     }
   else
     {
@@ -137,12 +148,13 @@ read_checked_initial_length_and_offset (bfd *abfd, const gdb_byte *buf,
 static void
 read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd,
 			const gdb_byte **bufp, struct line_header *lh,
-			const struct comp_unit_head *cu_header,
+			const struct comp_unit_head *cu_header, bool is_dwo,
 			void (*callback) (struct line_header *lh,
 					  const char *name,
 					  dir_index d_index,
 					  unsigned int mod_time,
-					  unsigned int length))
+					  unsigned int length,
+					  bool is_dwo_file))
 {
   gdb_byte format_count, formati;
   ULONGEST data_count, datai;
@@ -254,7 +266,7 @@ read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd,
 	    }
 	}
 
-      callback (lh, fe.name, fe.d_index, fe.mod_time, fe.length);
+      callback (lh, fe.name, fe.d_index, fe.mod_time, fe.length, is_dwo);
     }
 
   *bufp = buf;
@@ -262,11 +274,12 @@ read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd,
 
 /* See line-header.h.  */
 
-line_header_up
+void
 dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
 			   dwarf2_per_objfile *per_objfile,
 			   struct dwarf2_section_info *section,
-			   const struct comp_unit_head *cu_header)
+			   const struct comp_unit_head *cu_header,
+			   line_header_up& lh, bool is_dwo)
 {
   const gdb_byte *line_ptr;
   unsigned int bytes_read, offset_size;
@@ -280,38 +293,47 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
   if (to_underlying (sect_off) + 4 >= section->size)
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
-      return 0;
+      return;
     }
 
-  line_header_up lh (new line_header ());
-
   lh->sect_off = sect_off;
   lh->offset_in_dwz = is_dwz;
 
   line_ptr = section->buffer + to_underlying (sect_off);
 
   /* Read in the header.  */
-  lh->total_length =
+  unsigned int total_length = 0;
+  total_length =
     read_checked_initial_length_and_offset (abfd, line_ptr, cu_header,
-					    &bytes_read, &offset_size);
-  line_ptr += bytes_read;
+                                            &bytes_read, &offset_size);
+  if (is_dwo)
+    lh->dwo_total_length = total_length;
+  else
+    lh->total_length = total_length;
 
-  const gdb_byte *start_here = line_ptr;
+  line_ptr += bytes_read;
 
-  if (line_ptr + lh->total_length > (section->buffer + section->size))
+  if (line_ptr + total_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
-      return 0;
+      return;
     }
-  lh->statement_program_end = start_here + lh->total_length;
+
+   if (!is_dwo)
+   {
+     const gdb_byte *start_here = line_ptr;
+     lh->statement_program_end = start_here + lh->total_length;
+   }
+
   lh->version = read_2_bytes (abfd, line_ptr);
   line_ptr += 2;
+
   if (lh->version > 5)
     {
       /* This is a version we don't understand.  The format could have
 	 changed in ways we don't handle properly so just punt.  */
       complaint (_("unsupported version in .debug_line section"));
-      return NULL;
+      return;
     }
   if (lh->version >= 5)
     {
@@ -328,12 +350,22 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
 	  complaint (_("unsupported segment selector size %u "
 		       "in .debug_line section"),
 		     segment_selector_size);
-	  return NULL;
+	  return;
 	}
     }
-  lh->header_length = read_offset (abfd, line_ptr, offset_size);
+
+  unsigned int header_length = 0;
+  header_length = read_offset (abfd, line_ptr, offset_size);
   line_ptr += offset_size;
-  lh->statement_program_start = line_ptr + lh->header_length;
+
+  if (is_dwo)
+    lh->dwo_header_length = header_length;
+  else
+    lh->header_length = header_length;
+
+   if (!is_dwo)
+     lh->statement_program_start = line_ptr + lh->header_length;
+
   lh->minimum_instruction_length = read_1_byte (abfd, line_ptr);
   line_ptr += 1;
   if (lh->version >= 4)
@@ -357,46 +389,65 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
   line_ptr += 1;
   lh->line_range = read_1_byte (abfd, line_ptr);
   line_ptr += 1;
-  lh->opcode_base = read_1_byte (abfd, line_ptr);
+
+  unsigned char opcode_base;
+  opcode_base = read_1_byte (abfd, line_ptr);
   line_ptr += 1;
-  lh->standard_opcode_lengths.reset (new unsigned char[lh->opcode_base]);
 
-  lh->standard_opcode_lengths[0] = 1;  /* This should never be used anyway.  */
-  for (i = 1; i < lh->opcode_base; ++i)
+  if (is_dwo)
+  {
+    lh->dwo_opcode_base = opcode_base;
+    lh->dwo_standard_opcode_lengths.reset (new unsigned char[lh->dwo_opcode_base]);
+
+    lh->dwo_standard_opcode_lengths[0] = 1;  /* This should never be used anyway.  */
+    for (i = 1; i < lh->dwo_opcode_base; ++i)
+    {
+      lh->dwo_standard_opcode_lengths[i] = read_1_byte (abfd, line_ptr);
+      line_ptr += 1;
+    }
+  }
+  else
+  {
+    lh->opcode_base = opcode_base;
+    lh->standard_opcode_lengths.reset (new unsigned char[lh->opcode_base]);
+
+    lh->standard_opcode_lengths[0] = 1;  /* This should never be used anyway.  */
+    for (i = 1; i < lh->opcode_base; ++i)
     {
       lh->standard_opcode_lengths[i] = read_1_byte (abfd, line_ptr);
       line_ptr += 1;
     }
+   }
 
   if (lh->version >= 5)
     {
       /* Read directory table.  */
       read_formatted_entries (per_objfile, abfd, &line_ptr, lh.get (),
-			      cu_header,
+			      cu_header, is_dwo,
 			      [] (struct line_header *header, const char *name,
 				  dir_index d_index, unsigned int mod_time,
-				  unsigned int length)
+				  unsigned int length, bool is_dwo_section)
 	{
-	  header->add_include_dir (name);
+	  header->add_include_dir (name, is_dwo_section);
 	});
 
       /* Read file name table.  */
       read_formatted_entries (per_objfile, abfd, &line_ptr, lh.get (),
-			      cu_header,
+			      cu_header, is_dwo,
 			      [] (struct line_header *header, const char *name,
 				  dir_index d_index, unsigned int mod_time,
-				  unsigned int length)
+				  unsigned int length, bool is_dwo_section)
 	{
-	  header->add_file_name (name, d_index, mod_time, length);
+	  header->add_file_name (name, d_index, mod_time, length, is_dwo_section);
 	});
     }
   else
     {
       /* Read directory table.  */
       while ((cur_dir = read_direct_string (abfd, line_ptr, &bytes_read)) != NULL)
-	{
+        {
 	  line_ptr += bytes_read;
-	  lh->add_include_dir (cur_dir);
+	  lh->add_include_dir (cur_dir, is_dwo);
 	}
       line_ptr += bytes_read;
 
@@ -414,7 +465,7 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
 	  length = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
 	  line_ptr += bytes_read;
 
-	  lh->add_file_name (cur_file, d_index, mod_time, length);
+	  lh->add_file_name (cur_file, d_index, mod_time, length, is_dwo);
 	}
       line_ptr += bytes_read;
     }
@@ -422,6 +473,4 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
   if (line_ptr > (section->buffer + section->size))
     complaint (_("line number info header doesn't "
 		 "fit in `.debug_line' section"));
-
-  return lh;
 }
diff --git a/gdb/dwarf2/line-header.h b/gdb/dwarf2/line-header.h
index 4cacb8ca344..ee586e8b65d 100644
--- a/gdb/dwarf2/line-header.h
+++ b/gdb/dwarf2/line-header.h
@@ -44,9 +44,12 @@ struct file_entry
       length (length_)
   {}
 
-  /* Return the include directory at D_INDEX stored in LH.  Returns
-     NULL if D_INDEX is out of bounds.  */
-  const char *include_dir (const line_header *lh) const;
+  /* Return the include directory at D_INDEX stored in m_include_dirs vector
+     in LH or in m_dwo_include_dirs vector in LH if IS_DWO is true. m_dwo_
+     include_dirs contains the include_directories entries of .debug_line.dwo
+     section whereas m_include_dirs contains the include_directories array
+     entries of .debug_line section. Returns NULL if D_INDEX is out of bounds.  */
+  const char *include_dir (const line_header *lh, bool is_dwo) const;
 
   /* The file name.  Note this is an observing pointer.  The memory is
      owned by debug_line_buffer.  */
@@ -75,25 +78,33 @@ struct line_header
     : offset_in_dwz {}
   {}
 
-  /* Add an entry to the include directory table.  */
-  void add_include_dir (const char *include_dir);
+  /* Add an entry to the m_include_dirs vector or to m_dwo_include_dirs vector
+     if IS_DWO is true.  */
+  void add_include_dir (const char *include_dir, bool is_dwo);
 
-  /* Add an entry to the file name table.  */
+  /* Add an entry to the m_file_names vector or to the m_dwo_file_names vector
+     if IS_DWO is true. m_dwo_file_names vector contains the file_names entries of
+     .debug_line.dwo section whereas m_file_names contains the file_names array
+     entries of .debug_line section.  */
   void add_file_name (const char *name, dir_index d_index,
-		      unsigned int mod_time, unsigned int length);
+		      unsigned int mod_time, unsigned int length, bool is_dwo);
 
-  /* Return the include dir at INDEX (0-based in DWARF 5 and 1-based before).
+  /* Return the include dir from m_include_dirs or from the m_dwo_include_dirs
+     if IS_DWO is true at INDEX (0-based in DWARF 5 and 1-based before).
      Returns NULL if INDEX is out of bounds.  */
-  const char *include_dir_at (dir_index index) const
+  const char *include_dir_at (dir_index index, bool is_dwo) const
   {
     int vec_index;
     if (version >= 5)
       vec_index = index;
     else
       vec_index = index - 1;
-    if (vec_index < 0 || vec_index >= m_include_dirs.size ())
+    int include_dirs_size = is_dwo ? m_dwo_include_dirs.size () :
+			    m_include_dirs.size ();
+    if (vec_index < 0 || vec_index >= include_dirs_size)
       return NULL;
-    return m_include_dirs[vec_index];
+    return is_dwo ? m_dwo_include_dirs[vec_index] :
+		    m_include_dirs[vec_index];
   }
 
   bool is_valid_file_index (int file_index) const
@@ -103,25 +114,36 @@ struct line_header
     return 1 <= file_index && file_index <= file_names_size ();
   }
 
-  /* Return the file name at INDEX (0-based in DWARF 5 and 1-based before).
+  bool is_valid_dwo_file_index (int file_index) const
+  {
+    if (version >= 5)
+      return 0 <= file_index && file_index < dwo_file_names_size ();
+    return 1 <= file_index && file_index <= dwo_file_names_size ();
+
+  }
+
+  /* Return the file name from m_file_names or from m_dwo_file_names vector
+     if IS_DWO is true at INDEX (0-based in DWARF 5 and 1-based before).
      Returns NULL if INDEX is out of bounds.  */
-  file_entry *file_name_at (file_name_index index)
+  file_entry *file_name_at (file_name_index index, bool is_dwo)
   {
     int vec_index;
     if (version >= 5)
       vec_index = index;
     else
       vec_index = index - 1;
-    if (vec_index < 0 || vec_index >= m_file_names.size ())
+    int file_names_size = is_dwo ? m_dwo_file_names.size ():
+			  m_file_names.size ();
+    if (vec_index < 0 || vec_index >= file_names_size)
       return NULL;
-    return &m_file_names[vec_index];
+    return is_dwo ? &m_dwo_file_names[vec_index] : &m_file_names[vec_index];
   }
 
   /* A const overload of the same.  */
-  const file_entry *file_name_at (file_name_index index) const
+  const file_entry *file_name_at (file_name_index index, bool is_dwo) const
   {
     line_header *lh = const_cast<line_header *> (this);
-    return lh->file_name_at (index);
+    return lh->file_name_at (index, is_dwo);
   }
 
   /* The indexes are 0-based in DWARF 5 and 1-based in DWARF 4. Therefore,
@@ -133,6 +155,12 @@ struct line_header
   const std::vector<file_entry> &file_names () const
   { return m_file_names; }
 
+  std::vector<file_entry> &dwo_file_names ()
+  { return m_dwo_file_names; }
+  /* A const overload of the same.  */
+  const std::vector<file_entry> &dwo_file_names () const
+  { return m_dwo_file_names; }
+
   /* Offset of line number information in .debug_line section.  */
   sect_offset sect_off {};
 
@@ -140,14 +168,17 @@ struct line_header
   unsigned offset_in_dwz : 1; /* Can't initialize bitfields in-class.  */
 
   unsigned int total_length {};
+  unsigned int dwo_total_length {};
   unsigned short version {};
   unsigned int header_length {};
+  unsigned int dwo_header_length {};
   unsigned char minimum_instruction_length {};
   unsigned char maximum_ops_per_instruction {};
   unsigned char default_is_stmt {};
   int line_base {};
   unsigned char line_range {};
   unsigned char opcode_base {};
+  unsigned char dwo_opcode_base {};
 
   /* standard_opcode_lengths[i] is the number of operands for the
      standard opcode whose value is i.  This means that
@@ -155,9 +186,18 @@ struct line_header
      element is standard_opcode_lengths[opcode_base - 1].  */
   std::unique_ptr<unsigned char[]> standard_opcode_lengths;
 
+  /* dwo_standard_opcode_lengths[i] is the number of operands for the
+     standard opcode whose value is i.  This means that
+     dwo_standard_opcode_lengths[0] is unused, and the last meaningful
+     element is dwo_standard_opcode_lengths[dwo_opcode_base - 1].  */
+  std::unique_ptr<unsigned char[]> dwo_standard_opcode_lengths;
+
   int file_names_size () const
   { return m_file_names.size(); }
 
+  int dwo_file_names_size () const
+  { return m_dwo_file_names.size (); }
+
   /* The start and end of the statement program following this
      header.  These point into dwarf2_per_objfile->line_buffer.  */
   const gdb_byte *statement_program_start {}, *statement_program_end {};
@@ -173,19 +213,25 @@ struct line_header
      pointers.  The memory is owned by debug_line_buffer.  */
   std::vector<const char *> m_include_dirs;
 
+  /* The include_directories table from .debug_line.dwo section.  */
+  std::vector<const char *> m_dwo_include_dirs;
+
   /* The file_names table. This is private because the meaning of indexes
      differs among DWARF versions (The first valid index is 1 in DWARF 4 and
      before, and is 0 in DWARF 5 and later).  So the client should use
      file_name_at method for access.  */
   std::vector<file_entry> m_file_names;
+
+  /* The file_names table from .debug_line.dwo section.  */
+  std::vector<file_entry> m_dwo_file_names;
 };
 
 typedef std::unique_ptr<line_header> line_header_up;
 
 inline const char *
-file_entry::include_dir (const line_header *lh) const
+file_entry::include_dir (const line_header *lh, bool is_dwo) const
 {
-  return lh->include_dir_at (d_index);
+  return lh->include_dir_at (d_index, is_dwo);
 }
 
 /* Read the statement program header starting at SECT_OFF in SECTION.
@@ -196,8 +242,9 @@ file_entry::include_dir (const line_header *lh) const
    the returned object point into the dwarf line section buffer,
    and must not be freed.  */
 
-extern line_header_up dwarf_decode_line_header
+extern void dwarf_decode_line_header
   (sect_offset sect_off, bool is_dwz, dwarf2_per_objfile *per_objfile,
-   struct dwarf2_section_info *section, const struct comp_unit_head *cu_header);
+   struct dwarf2_section_info *section, const struct comp_unit_head *cu_header,
+   line_header_up& lh, bool is_dwo);
 
 #endif /* DWARF2_LINE_HEADER_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4a963e4a236..6ee64615eab 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1560,7 +1560,7 @@ static const char *compute_include_file_name
      (const struct line_header *lh,
       const file_entry &fe,
       const file_and_directory &cu_info,
-      gdb::unique_xmalloc_ptr<char> *name_holder);
+      gdb::unique_xmalloc_ptr<char> *name_holder, bool is_dwo);
 
 static htab_up allocate_signatured_type_table ();
 
@@ -3043,7 +3043,19 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
 	{
 	  gdb::unique_xmalloc_ptr<char> name_holder;
 	  const char *include_name =
-	    compute_include_file_name (lh.get (), entry, fnd, &name_holder);
+	    compute_include_file_name (lh.get (), entry, fnd, &name_holder, false);
+	  if (include_name != nullptr)
+	    {
+	      include_name = per_objfile->objfile->intern (include_name);
+	      include_names.push_back (include_name);
+	    }
+	}
+
+      for (const auto &entry : lh->dwo_file_names ())
+	{
+	  gdb::unique_xmalloc_ptr<char> name_holder;
+	  const char *include_name =
+	    compute_include_file_name (lh.get (), entry, fnd, &name_holder, false);
 	  if (include_name != nullptr)
 	    {
 	      include_name = per_objfile->objfile->intern (include_name);
@@ -10774,7 +10786,7 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
 	{
 	  file_entry &fe = file_names[i];
 	  dwarf2_start_subfile (this, fe.name,
-				fe.include_dir (line_header));
+				fe.include_dir (line_header, false));
 	  buildsym_compunit *b = get_builder ();
 	  if (b->get_current_subfile ()->symtab == NULL)
 	    {
@@ -20704,7 +20716,7 @@ get_debug_line_section (struct dwarf2_cu *cu)
 }
 
 /* Read the statement program header starting at OFFSET in
-   .debug_line, or .debug_line.dwo.  Return a pointer
+   .debug_line and .debug_line.dwo.  Return a pointer
    to a struct line_header, allocated using xmalloc.
    Returns NULL if there is a problem reading the header, e.g., if it
    has a version we don't understand.
@@ -20717,21 +20729,42 @@ static line_header_up
 dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
 {
   struct dwarf2_section_info *section;
+  struct dwarf2_section_info *dwo_section = NULL;
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
 
   section = get_debug_line_section (cu);
   section->read (per_objfile->objfile);
+
   if (section->buffer == NULL)
+  {
+    if (cu->dwo_unit && cu->per_cu->is_debug_types)
+      complaint (_("missing .debug_line.dwo section"));
+    else
+      complaint (_("missing .debug_line section"));
+    return 0;
+  }
+
+  if (cu->dwo_unit && !cu->per_cu->is_debug_types)
     {
-      if (cu->dwo_unit && cu->per_cu->is_debug_types)
-	complaint (_("missing .debug_line.dwo section"));
-      else
-	complaint (_("missing .debug_line section"));
-      return 0;
+      dwo_section = &cu->dwo_unit->dwo_file->sections.line;
+      dwo_section->read (per_objfile->objfile);
+
+      if (dwo_section->buffer == NULL)
+	{
+	  complaint (_("missing .debug_line.dwo section"));
+	}
     }
 
-  return dwarf_decode_line_header (sect_off, cu->per_cu->is_dwz,
-				   per_objfile, section, &cu->header);
+  line_header_up lh (new line_header ());
+
+  dwarf_decode_line_header (sect_off, cu->per_cu->is_dwz,
+			    per_objfile, section,
+			    &cu->header, lh, false);
+  if (cu->dwo_unit)
+    dwarf_decode_line_header (sect_off, cu->per_cu->is_dwz,
+			      per_objfile, dwo_section,
+			      &cu->header, lh, true);
+  return lh;
 }
 
 /* Subroutine of dwarf_decode_lines to simplify it.
@@ -20744,12 +20777,20 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
 static const char *
 compute_include_file_name (const struct line_header *lh, const file_entry &fe,
 			   const file_and_directory &cu_info,
-			   gdb::unique_xmalloc_ptr<char> *name_holder)
+			   gdb::unique_xmalloc_ptr<char> *name_holder, bool is_dwo)
 {
   const char *include_name = fe.name;
   const char *include_name_to_compare = include_name;
+  const char *dir_name;
 
-  const char *dir_name = fe.include_dir (lh);
+  if (is_dwo)
+    {
+      dir_name = fe.include_dir (lh, true);
+    }
+  else
+    {
+      dir_name = fe.include_dir (lh, false);
+    }
 
   gdb::unique_xmalloc_ptr<char> hold_compare;
   if (!IS_ABSOLUTE_PATH (include_name)
@@ -20819,7 +20860,7 @@ class lnp_state_machine
   {
     /* lh->file_names is 0-based, but the file name numbers in the
        statement program are 1-based.  */
-    return m_line_header->file_name_at (m_file);
+    return m_line_header->file_name_at (m_file, false);
   }
 
   /* Record the line in the state machine.  END_SEQUENCE is true if
@@ -20993,7 +21034,7 @@ lnp_state_machine::handle_set_file (file_name_index file)
     dwarf2_debug_line_missing_file_complaint ();
   else if (m_record_lines_p)
     {
-      const char *dir = fe->include_dir (m_line_header);
+      const char *dir = fe->include_dir (m_line_header, false);
 
       m_last_subfile = m_cu->get_builder ()->get_current_subfile ();
       m_line_has_non_zero_discriminator = m_discriminator != 0;
@@ -21285,7 +21326,7 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
 	  const file_entry *fe = state_machine.current_file ();
 
 	  if (fe != NULL)
-	    dwarf2_start_subfile (cu, fe->name, fe->include_dir (lh));
+	    dwarf2_start_subfile (cu, fe->name, fe->include_dir (lh, false));
 	}
 
       /* Decode the table.  */
@@ -21350,7 +21391,7 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
 		    length =
 		      read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
 		    line_ptr += bytes_read;
-		    lh->add_file_name (cur_file, dindex, mod_time, length);
+		    lh->add_file_name (cur_file, dindex, mod_time, length, false);
 		  }
 		  break;
 		case DW_LNE_set_discriminator:
@@ -21507,7 +21548,20 @@ dwarf_decode_lines (struct line_header *lh, const file_and_directory &fnd,
 	  {
 	    gdb::unique_xmalloc_ptr<char> name_holder;
 	    const char *include_name =
-	      compute_include_file_name (lh, file_entry, fnd, &name_holder);
+	      compute_include_file_name (lh, file_entry, fnd, &name_holder, false);
+	    if (include_name != NULL)
+	      dwarf2_create_include_psymtab
+		(cu->per_objfile->per_bfd, include_name, pst,
+		 cu->per_objfile->per_bfd->partial_symtabs.get (),
+		 objfile->per_bfd);
+	  }
+
+      for (auto &file_entry : lh->dwo_file_names ())
+	if (file_entry.included_p)
+	  {
+	    gdb::unique_xmalloc_ptr<char> name_holder;
+	    const char *include_name =
+	      compute_include_file_name (lh, file_entry, fnd, &name_holder, true);
 	    if (include_name != NULL)
 	      dwarf2_create_include_psymtab
 		(cu->per_objfile->per_bfd, include_name, pst,
@@ -21525,7 +21579,19 @@ dwarf_decode_lines (struct line_header *lh, const file_and_directory &fnd,
 
       for (auto &fe : lh->file_names ())
 	{
-	  dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh));
+	  dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh, false));
+	  if (builder->get_current_subfile ()->symtab == NULL)
+	    {
+	      builder->get_current_subfile ()->symtab
+		= allocate_symtab (cust,
+				   builder->get_current_subfile ()->name);
+	    }
+	  fe.symtab = builder->get_current_subfile ()->symtab;
+	}
+
+      for (auto &fe : lh->dwo_file_names ())
+	{
+	  dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh, true));
 	  if (builder->get_current_subfile ()->symtab == NULL)
 	    {
 	      builder->get_current_subfile ()->symtab
@@ -21741,7 +21807,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	  struct file_entry *fe;
 
 	  if (cu->line_header != NULL)
-	    fe = cu->line_header->file_name_at (file_index);
+	    fe = cu->line_header->file_name_at (file_index, (cu->dwo_unit != nullptr));
 	  else
 	    fe = NULL;
 
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index bf2d79d219f..0c2899d274f 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -238,7 +238,7 @@ compare_locations (struct macro_source_file *file1, int line1,
 
   /* If the two files are not the same, find their common ancestor in
      the #inclusion tree.  */
-  if (file1 != file2)
+  if (filename_cmp(file1->filename, file2->filename) != 0)
     {
       /* If one file is deeper than the other, walk up the #inclusion
 	 chain until the two files are at least at the same *depth*.
@@ -266,7 +266,7 @@ compare_locations (struct macro_source_file *file1, int line1,
 
       /* Now both file1 and file2 are at the same depth.  Walk toward
 	 the root of the tree until we find where the branches meet.  */
-      while (file1 != file2)
+      while (filename_cmp(file1->filename, file2->filename) != 0)
 	{
 	  line1 = file1->included_at_line;
 	  file1 = file1->included_by;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf.
  2021-11-19  6:56 [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf Achra, Nitika
@ 2021-11-19 18:55 ` Keith Seitz
  2022-03-14 14:32   ` Achra, Nitika
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Seitz @ 2021-11-19 18:55 UTC (permalink / raw)
  To: Achra, Nitika, gdb-patches; +Cc: George, Jini Susan

On 11/18/21 22:56, Achra, Nitika via Gdb-patches wrote:
> [AMD Official Use Only]
> 
> Requesting to please review the attached patch.

Hi, thank you for submitting your patch for review!

I've given the patch a very quick look, and in general,
I think this is on the right path. Based on a preliminary
review, however, I have several questions (and a request or two).

> Command used: make check RUNTESTFLAGS='CC_FOR_TARGET="clang -gdwarf-5 -gsplit-dwarf -fdebug-macro"'
>                                           TESTS="gdb.base/macscp.exp"> 
> Before the patch:
> Number of expected passes            234
> Number of unexpected failures        65
> Number of known failures             40
> 
> After the patch:
> Number of expected passes            315
> Number of unexpected failures        1
> Number of known failures             23
> 
> Compiler used: clang 12.0.0

While you've described the ultimate outcome of this patch, I am
missing a description of how you fixed it. Yes, I could study the patch,
but a written preface would help set up what we are reviewing.

[See advice in the official Contribution Checklist,
https://sourceware.org/gdb/wiki/ContributionChecklist .]

Have you tested this patch against any other compilers? This patch includes
a patch I posted an RFC on several months ago:

   https://sourceware.org/pipermail/gdb-patches/2021-June/179698.html

In that thread, Simon discusses testing this line_header::file_file_name
change against several compilers, including gcc and clang. More on this
below.

While the goal of this patch is to improve the -gsplit-dwarf experience
(which I gather is mainly used by clang users), I would certainly like to know
how this patch may impact users not using this compiler configuration, including
gcc users.

To understand the impact here, I ran this patch using gcc-11 on
gdb.base/macscp.exp. [Results are same -gdwarf-4 or -gdwarf-5]

unpatched; default flags
# of expected passes		320
# of known failures		19

w/this patch; default flags
# of expected passes		317
# of known failures		22

unpatched; with -gsplit-dwarf
# of expected passes		1
# of unsupported tests		1

w/this patch; with -gsplit-dwarf
# of expected passes		179
# of unexpected failures	121
# of known failures		40

These discrepancies need at the least an explanation.

I also ran the entirety of the test suite against gcc-11 with/without this
patch applied (no -gsplit-dwarf). Other than the above gdb.base/macscp.exp,
this patch would introduce regressions in gdb.dwarf2/fission-base.exp:

  PASS: gdb.dwarf2/fission-base.exp: ptype main
  PASS: gdb.dwarf2/fission-base.exp: ptype func
-PASS: gdb.dwarf2/fission-base.exp: frame in main
-PASS: gdb.dwarf2/fission-base.exp: break func
-PASS: gdb.dwarf2/fission-base.exp: continue to func
-PASS: gdb.dwarf2/fission-base.exp: frame in func
+FAIL: gdb.dwarf2/fission-base.exp: frame in main
+FAIL: gdb.dwarf2/fission-base.exp: break func
+FAIL: gdb.dwarf2/fission-base.exp: continue to func
+FAIL: gdb.dwarf2/fission-base.exp: frame in func

I also ran the full testsuite using clang-12. That shows the
above regressions plus a few more related to fission.
[gdb.dwarf2/fission-reread.exp and gdb.dwarf2/fission-multi-cu.exp]


> gdb/ChangeLog:
> 
> 	* dwarf2/line-header.c (add_include_dir): Handle .debug_line.dwo
> 	  include directories table entries.
> 	* (add_file_name): Handle .debug_line.dwo file names entries.
> 	* (file_file_name): Read the file name from .debug_line.dwo or
> 	  .debug_line sections.

We no longer use/require ChangeLog entries. :-)

Until the above issues are documented and/or resolved, I will forgo a
more in-depth look at the patch, but I do want to mention this change:

>  gdb::unique_xmalloc_ptr<char>
>  line_header::file_file_name (int file) const
>  {
>    /* Is the file number a valid index into the line header's file name
> -     table?  Remember that file numbers start with one, not zero.  */
> -  if (is_valid_file_index (file))
> +     table?  First check in the .debug_line.dwo file table and then in
> +     .debug_line table. Remember that file numbers start with one in
> +     DWARFv4 and with zero in DWARFv5.  */
> +  if (is_valid_dwo_file_index (file))
>      {
> -      const file_entry *fe = file_name_at (file);
> -
> -      if (!IS_ABSOLUTE_PATH (fe->name))
> -	{
> -	  const char *dir = fe->include_dir (this);
> -	  if (dir != NULL)
> -	    return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
> -							  fe->name,
> -							  (char *) NULL));
> -	}
> -      return make_unique_xstrdup (fe->name);
> +      const file_entry *fe = file_name_at (file, true);
> +      return make_unique_xstrdup (fe->symtab->filename);
> +    }
> +  else if (is_valid_file_index (file))
> +    {
> +      const file_entry *fe = file_name_at (file, false);
> +      return make_unique_xstrdup (fe->symtab->filename);
>      }
>    else
>      {

This is the change that Simon and I were discussing several months ago.
Coincidentally, I have been revisiting this patch and testing it this
week, and I believe this bit (using the symtab's filename) is a significant
enough change that it should be submitted separately to discuss.

Keith


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf.
  2021-11-19 18:55 ` Keith Seitz
@ 2022-03-14 14:32   ` Achra, Nitika
  2022-03-18 20:15     ` Keith Seitz
  0 siblings, 1 reply; 7+ messages in thread
From: Achra, Nitika @ 2022-03-14 14:32 UTC (permalink / raw)
  To: Keith Seitz, gdb-patches; +Cc: George, Jini Susan, Natarajan, Kavitha

[-- Attachment #1: Type: text/plain, Size: 8353 bytes --]

[AMD Official Use Only]

Hi Keith,

Thanks for the review and sorry for the delay in the reply. I have fixed the regressions mentioned by you. Also, I have removed the change you have posted for the RFC. I have attached the updated patch. So, this patch along with your fix will result in the following for gdb.base/macscp.exp testcase with -gsplit-dwarf:

Command Used:
make check RUNTESTFLAGS='CC_FOR_TARGET="clang -gdwarf-5 -gsplit-dwarf -fdebug-macro"'
                         TESTS="gdb.base/macscp.exp"
Compiler used: clang 12.0.0

Before the patch:
Number of expected passes            234
Number of unexpected failures        65
Number of known failures             40

After applying the attached patch and including your change also:
Number of expected passes            319
Number of unexpected failures        1
Number of known failures             19

Compiler used: clang 12.0.0

Root cause and the fix for these failures:
Clang is emitting both .debug_line and .debug_line.dwo sections and .debug_macro.dwo is pointing to the .debug_line.dwo section(file index in the .debug_macro.dwo is the index in the file_names array of
.debug_line.dwo). However, GDB is reading only .debug_line section. In this patch, I have added the support
for reading .debug_line.dwo section and added m_dwo_include_dirs and m_dwo_file_names vectors in struct line_header to store the directories and files respectively and reading the file and directory names from
these vectors whenever required.

Could you please take another look? Thanks in advance.

Regards,
Nitika


Sent from Mail<https://go.microsoft.com/fwlink/?LinkId=550986> for Windows

From: Keith Seitz<mailto:keiths@redhat.com>
Sent: Saturday, November 20, 2021 12:25 AM
To: Achra, Nitika<mailto:Nitika.Achra@amd.com>; gdb-patches@sourceware.org<mailto:gdb-patches@sourceware.org>
Cc: George, Jini Susan<mailto:JiniSusan.George@amd.com>
Subject: Re: [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf.

[CAUTION: External Email]

On 11/18/21 22:56, Achra, Nitika via Gdb-patches wrote:
> [AMD Official Use Only]
>
> Requesting to please review the attached patch.

Hi, thank you for submitting your patch for review!

I've given the patch a very quick look, and in general,
I think this is on the right path. Based on a preliminary
review, however, I have several questions (and a request or two).

> Command used: make check RUNTESTFLAGS='CC_FOR_TARGET="clang -gdwarf-5 -gsplit-dwarf -fdebug-macro"'
>                                           TESTS="gdb.base/macscp.exp">
> Before the patch:
> Number of expected passes            234
> Number of unexpected failures        65
> Number of known failures             40
>
> After the patch:
> Number of expected passes            315
> Number of unexpected failures        1
> Number of known failures             23
>
> Compiler used: clang 12.0.0

While you've described the ultimate outcome of this patch, I am
missing a description of how you fixed it. Yes, I could study the patch,
but a written preface would help set up what we are reviewing.

[See advice in the official Contribution Checklist,
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsourceware.org%2Fgdb%2Fwiki%2FContributionChecklist&amp;data=04%7C01%7Cnitika.achra%40amd.com%7Ca3fd2143112f429956c508d9ab8e36cb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637729449561032249%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=AFlGKgdVVamKqpddCxazSqqty7b5uvgcCHcvHEPwR1M%3D&amp;reserved=0 .]

Have you tested this patch against any other compilers? This patch includes
a patch I posted an RFC on several months ago:

   https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsourceware.org%2Fpipermail%2Fgdb-patches%2F2021-June%2F179698.html&amp;data=04%7C01%7Cnitika.achra%40amd.com%7Ca3fd2143112f429956c508d9ab8e36cb%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637729449561032249%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=zQ5bO%2FwGGpRJYB4VXslE%2BDwCDTxCDxhnh04feoMobCA%3D&amp;reserved=0

In that thread, Simon discusses testing this line_header::file_file_name
change against several compilers, including gcc and clang. More on this
below.

While the goal of this patch is to improve the -gsplit-dwarf experience
(which I gather is mainly used by clang users), I would certainly like to know
how this patch may impact users not using this compiler configuration, including
gcc users.

To understand the impact here, I ran this patch using gcc-11 on
gdb.base/macscp.exp. [Results are same -gdwarf-4 or -gdwarf-5]

unpatched; default flags
# of expected passes            320
# of known failures             19

w/this patch; default flags
# of expected passes            317
# of known failures             22

unpatched; with -gsplit-dwarf
# of expected passes            1
# of unsupported tests          1

w/this patch; with -gsplit-dwarf
# of expected passes            179
# of unexpected failures        121
# of known failures             40

These discrepancies need at the least an explanation.

I also ran the entirety of the test suite against gcc-11 with/without this
patch applied (no -gsplit-dwarf). Other than the above gdb.base/macscp.exp,
this patch would introduce regressions in gdb.dwarf2/fission-base.exp:

  PASS: gdb.dwarf2/fission-base.exp: ptype main
  PASS: gdb.dwarf2/fission-base.exp: ptype func
-PASS: gdb.dwarf2/fission-base.exp: frame in main
-PASS: gdb.dwarf2/fission-base.exp: break func
-PASS: gdb.dwarf2/fission-base.exp: continue to func
-PASS: gdb.dwarf2/fission-base.exp: frame in func
+FAIL: gdb.dwarf2/fission-base.exp: frame in main
+FAIL: gdb.dwarf2/fission-base.exp: break func
+FAIL: gdb.dwarf2/fission-base.exp: continue to func
+FAIL: gdb.dwarf2/fission-base.exp: frame in func

I also ran the full testsuite using clang-12. That shows the
above regressions plus a few more related to fission.
[gdb.dwarf2/fission-reread.exp and gdb.dwarf2/fission-multi-cu.exp]


> gdb/ChangeLog:
>
>       * dwarf2/line-header.c (add_include_dir): Handle .debug_line.dwo
>         include directories table entries.
>       * (add_file_name): Handle .debug_line.dwo file names entries.
>       * (file_file_name): Read the file name from .debug_line.dwo or
>         .debug_line sections.

We no longer use/require ChangeLog entries. :-)

Until the above issues are documented and/or resolved, I will forgo a
more in-depth look at the patch, but I do want to mention this change:

>  gdb::unique_xmalloc_ptr<char>
>  line_header::file_file_name (int file) const
>  {
>    /* Is the file number a valid index into the line header's file name
> -     table?  Remember that file numbers start with one, not zero.  */
> -  if (is_valid_file_index (file))
> +     table?  First check in the .debug_line.dwo file table and then in
> +     .debug_line table. Remember that file numbers start with one in
> +     DWARFv4 and with zero in DWARFv5.  */
> +  if (is_valid_dwo_file_index (file))
>      {
> -      const file_entry *fe = file_name_at (file);
> -
> -      if (!IS_ABSOLUTE_PATH (fe->name))
> -     {
> -       const char *dir = fe->include_dir (this);
> -       if (dir != NULL)
> -         return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
> -                                                       fe->name,
> -                                                       (char *) NULL));
> -     }
> -      return make_unique_xstrdup (fe->name);
> +      const file_entry *fe = file_name_at (file, true);
> +      return make_unique_xstrdup (fe->symtab->filename);
> +    }
> +  else if (is_valid_file_index (file))
> +    {
> +      const file_entry *fe = file_name_at (file, false);
> +      return make_unique_xstrdup (fe->symtab->filename);
>      }
>    else
>      {

This is the change that Simon and I were discussing several months ago.
Coincidentally, I have been revisiting this patch and testing it this
week, and I believe this bit (using the symtab's filename) is a significant
enough change that it should be submitted separately to discuss.

Keith


[-- Attachment #2: 0001-Fix-for-the-gdb.base-macscp.exp-testcase-failure-wit.patch --]
[-- Type: application/octet-stream, Size: 31563 bytes --]

From 1bfdfae2ff97e3dc4868864f523d799d2b029d37 Mon Sep 17 00:00:00 2001
From: nitachra <Nitika.Achra@amd.com>
Date: Fri, 1 Oct 2021 01:11:24 +0530
Subject: [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split
 dwarf.

Command used: make check RUNTESTFLAGS='CC_FOR_TARGET="clang -gdwarf-5 -gsplit-dwarf -fdebug-macro"'
			 TESTS="gdb.base/macscp.exp"

Before the patch:
Number of expected passes            234
Number of unexpected failures        65
Number of known failures             40

After the patch:
Number of expected passes            319
Number of unexpected failures        1
Number of known failures             19

Compiler used: clang 12.0.0

Clang is emitting both .debug_line and .debug_line.dwo sections and .debug
_macro.dwo is pointing to the .debug_line.dwo section(file index in the
.debug_macro.dwo is the index in the file_names array of .debug_line.dwo).
However, GDB is reading only .debug_line section. In this patch, I have added
the support for reading .debug_line.dwo section and added m_dwo_include
_dirs and m_dwo_file_names vectors in struct line_header to store the
directories and files respectively.
---
 gdb/dwarf2/line-header.c | 246 ++++++++++++++++++++++++++-------------
 gdb/dwarf2/line-header.h | 105 +++++++++++++----
 gdb/dwarf2/read.c        | 110 ++++++++++++++---
 gdb/macrotab.c           |   4 +-
 4 files changed, 341 insertions(+), 124 deletions(-)

diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c
index 938e9fd55d4..621c1c69ae5 100644
--- a/gdb/dwarf2/line-header.c
+++ b/gdb/dwarf2/line-header.c
@@ -26,57 +26,88 @@
 #include "filenames.h"
 
 void
-line_header::add_include_dir (const char *include_dir)
+line_header::add_include_dir (const char *include_dir, bool is_dwo)
 {
   if (dwarf_line_debug >= 2)
     {
       size_t new_size;
-      if (version >= 5)
-	new_size = m_include_dirs.size ();
+      int include_dir_size = is_dwo ? m_dwo_include_dirs.size ():
+			     m_include_dirs.size ();
+      if ((is_dwo && dwo_version >= 5) || (!is_dwo && version >= 5))
+	new_size = include_dir_size;
       else
-	new_size = m_include_dirs.size () + 1;
+	new_size = include_dir_size + 1;
       fprintf_unfiltered (gdb_stdlog, "Adding dir %zu: %s\n",
 			  new_size, include_dir);
     }
-  m_include_dirs.push_back (include_dir);
+
+    if (is_dwo)
+      m_dwo_include_dirs.push_back (include_dir);
+    else
+      m_include_dirs.push_back (include_dir);
 }
 
 void
 line_header::add_file_name (const char *name,
 			    dir_index d_index,
 			    unsigned int mod_time,
-			    unsigned int length)
+			    unsigned int length,
+			    bool is_dwo)
 {
   if (dwarf_line_debug >= 2)
     {
       size_t new_size;
-      if (version >= 5)
-	new_size = file_names_size ();
+      bool file_name_size = is_dwo ? dwo_file_names_size ():
+			     file_names_size ();
+      if ((is_dwo && dwo_version >= 5) || (!is_dwo && version >= 5))
+	new_size = file_name_size;
       else
-	new_size = file_names_size () + 1;
+	new_size = file_name_size + 1;
       fprintf_unfiltered (gdb_stdlog, "Adding file %zu: %s\n",
 			  new_size, name);
     }
-  m_file_names.emplace_back (name, d_index, mod_time, length);
+
+    if (is_dwo)
+      m_dwo_file_names.emplace_back (name, d_index, mod_time, length);
+    else
+      m_file_names.emplace_back (name, d_index, mod_time, length);
 }
 
 gdb::unique_xmalloc_ptr<char>
 line_header::file_file_name (int file) const
 {
   /* Is the file number a valid index into the line header's file name
-     table?  Remember that file numbers start with one, not zero.  */
-  if (is_valid_file_index (file))
+     table?  First check in the .debug_line.dwo file table and then in
+     .debug_line table. Remember that file numbers start with one in
+     DWARFv4 and with zero in DWARFv5.  */
+  if (is_valid_dwo_file_index (file))
     {
-      const file_entry *fe = file_name_at (file);
+      const file_entry *fe = file_name_at (file, true);
 
       if (!IS_ABSOLUTE_PATH (fe->name))
-	{
-	  const char *dir = fe->include_dir (this);
-	  if (dir != NULL)
-	    return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
-							  fe->name,
-							  (char *) NULL));
-	}
+       {
+	 const char *dir = fe->include_dir (this, true);
+	 if (dir != NULL)
+	   return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
+							 fe->name,
+							 (char *) NULL));
+       }
+
+      return make_unique_xstrdup (fe->name);
+    }
+  else if (is_valid_file_index (file))
+    {
+      const file_entry *fe = file_name_at (file, false);
+
+      if (!IS_ABSOLUTE_PATH (fe->name))
+       {
+	 const char *dir = fe->include_dir (this, false);
+	 if (dir != NULL)
+	   return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
+							 fe->name,
+							 (char *) NULL));
+       }
+
       return make_unique_xstrdup (fe->name);
     }
   else
@@ -137,12 +168,13 @@ read_checked_initial_length_and_offset (bfd *abfd, const gdb_byte *buf,
 static void
 read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd,
 			const gdb_byte **bufp, struct line_header *lh,
-			unsigned int offset_size,
+			unsigned int offset_size, bool is_dwo,
 			void (*callback) (struct line_header *lh,
 					  const char *name,
 					  dir_index d_index,
 					  unsigned int mod_time,
-					  unsigned int length))
+					  unsigned int length,
+					  bool is_dwo_section))
 {
   gdb_byte format_count, formati;
   ULONGEST data_count, datai;
@@ -257,7 +289,7 @@ read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd,
 	    }
 	}
 
-      callback (lh, fe.name, fe.d_index, fe.mod_time, fe.length);
+      callback (lh, fe.name, fe.d_index, fe.mod_time, fe.length, is_dwo);
     }
 
   *bufp = buf;
@@ -265,11 +297,12 @@ read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd,
 
 /* See line-header.h.  */
 
-line_header_up
+bool
 dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
 			   dwarf2_per_objfile *per_objfile,
 			   struct dwarf2_section_info *section,
-			   const struct comp_unit_head *cu_header)
+			   const struct comp_unit_head *cu_header,
+			   line_header_up& lh, bool is_dwo)
 {
   const gdb_byte *line_ptr;
   unsigned int bytes_read, offset_size;
@@ -283,40 +316,61 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
   if (to_underlying (sect_off) + 4 >= section->size)
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
-      return 0;
+      return false;
     }
 
-  line_header_up lh (new line_header ());
-
   lh->sect_off = sect_off;
   lh->offset_in_dwz = is_dwz;
 
   line_ptr = section->buffer + to_underlying (sect_off);
 
   /* Read in the header.  */
-  lh->total_length =
+  LONGEST total_length =
     read_checked_initial_length_and_offset (abfd, line_ptr, cu_header,
 					    &bytes_read, &offset_size);
-  line_ptr += bytes_read;
 
-  const gdb_byte *start_here = line_ptr;
+  line_ptr += bytes_read;
 
-  if (line_ptr + lh->total_length > (section->buffer + section->size))
+  if (line_ptr + total_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
-      return 0;
+      return false;
     }
-  lh->statement_program_end = start_here + lh->total_length;
-  lh->version = read_2_bytes (abfd, line_ptr);
-  line_ptr += 2;
-  if (lh->version > 5)
+
+  if (is_dwo)
     {
-      /* This is a version we don't understand.  The format could have
-	 changed in ways we don't handle properly so just punt.  */
-      complaint (_("unsupported version in .debug_line section"));
-      return NULL;
+      lh->dwo_total_length = total_length;
+      lh->dwo_version = read_2_bytes (abfd, line_ptr);
+
+      if (lh->dwo_version > 5)
+	{
+	  /* This is a version we don't understand.  The format could have
+	     changed in ways we don't handle properly so just punt.  */
+	  complaint (_("unsupported version in .debug_line.dwo section"));
+	  return false;
+	}
+
+      line_ptr += 2;
     }
-  if (lh->version >= 5)
+  else
+    {
+      lh->total_length = total_length;
+      const gdb_byte *start_here = line_ptr;
+      lh->statement_program_end = start_here + lh->total_length;
+      lh->version = read_2_bytes (abfd, line_ptr);
+
+      if (lh->version > 5)
+	{
+	  /* This is a version we don't understand.  The format could have
+	     changed in ways we don't handle properly so just punt.  */
+	  complaint (_("unsupported version in .debug_line section"));
+	  return false;
+	}
+
+      line_ptr += 2;
+    }
+
+  if ((is_dwo && lh->dwo_version >= 5) || (!is_dwo && lh->version >= 5))
     {
       gdb_byte segment_selector_size;
 
@@ -326,71 +380,103 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
 
       segment_selector_size = read_1_byte (abfd, line_ptr);
       line_ptr += 1;
+
       if (segment_selector_size != 0)
 	{
 	  complaint (_("unsupported segment selector size %u "
 		       "in .debug_line section"),
 		     segment_selector_size);
-	  return NULL;
+	  return false;
 	}
     }
-  lh->header_length = read_offset (abfd, line_ptr, offset_size);
+
+  unsigned int header_length = read_offset (abfd, line_ptr, offset_size);
   line_ptr += offset_size;
-  lh->statement_program_start = line_ptr + lh->header_length;
-  lh->minimum_instruction_length = read_1_byte (abfd, line_ptr);
-  line_ptr += 1;
-  if (lh->version >= 4)
+
+  if (is_dwo)
     {
-      lh->maximum_ops_per_instruction = read_1_byte (abfd, line_ptr);
+      lh->dwo_header_length = header_length;
+      line_ptr += 1; /* Skip the minimum instruction length.  */
+      if (lh->dwo_version >= 4)
+	line_ptr += 1; /* Skip maximum ops per instruction.  */
+      line_ptr += 1; /* Skip default is stmt.  */
+      line_ptr += 2; /* Skip line base and line range.  */
+
+      /* Read the opcode base.  */
+      lh->dwo_opcode_base = read_1_byte (abfd, line_ptr);
       line_ptr += 1;
+      lh->dwo_standard_opcode_lengths.reset (new unsigned char[lh->dwo_opcode_base]);
+      lh->dwo_standard_opcode_lengths[0] = 1;  /* This should never be used anyway.  */
+
+      for (i = 1; i < lh->dwo_opcode_base; ++i)
+	{
+	  lh->dwo_standard_opcode_lengths[i] = read_1_byte (abfd, line_ptr);
+	  line_ptr += 1;
+	}
     }
   else
-    lh->maximum_ops_per_instruction = 1;
-
-  if (lh->maximum_ops_per_instruction == 0)
     {
-      lh->maximum_ops_per_instruction = 1;
-      complaint (_("invalid maximum_ops_per_instruction "
-		   "in `.debug_line' section"));
-    }
+      lh->header_length = header_length;
+      lh->statement_program_start = line_ptr + lh->header_length;
+      lh->minimum_instruction_length = read_1_byte (abfd, line_ptr);
+      line_ptr += 1;
 
-  lh->default_is_stmt = read_1_byte (abfd, line_ptr);
-  line_ptr += 1;
-  lh->line_base = read_1_signed_byte (abfd, line_ptr);
-  line_ptr += 1;
-  lh->line_range = read_1_byte (abfd, line_ptr);
-  line_ptr += 1;
-  lh->opcode_base = read_1_byte (abfd, line_ptr);
-  line_ptr += 1;
-  lh->standard_opcode_lengths.reset (new unsigned char[lh->opcode_base]);
-
-  lh->standard_opcode_lengths[0] = 1;  /* This should never be used anyway.  */
-  for (i = 1; i < lh->opcode_base; ++i)
-    {
-      lh->standard_opcode_lengths[i] = read_1_byte (abfd, line_ptr);
+      if (lh->version >= 4)
+	{
+	  lh->maximum_ops_per_instruction = read_1_byte (abfd, line_ptr);
+	  line_ptr += 1;
+	}
+      else
+	{
+	  lh->maximum_ops_per_instruction = 1;
+	}
+
+      if (lh->maximum_ops_per_instruction == 0)
+	{
+	  lh->maximum_ops_per_instruction = 1;
+	  complaint (_("invalid maximum_ops_per_instruction "
+		       "in `.debug_line' section"));
+	}
+
+      lh->default_is_stmt = read_1_byte (abfd, line_ptr);
+      line_ptr += 1;
+      lh->line_base = read_1_signed_byte (abfd, line_ptr);
       line_ptr += 1;
+      lh->line_range = read_1_byte (abfd, line_ptr);
+      line_ptr += 1;
+
+      lh->opcode_base = read_1_byte (abfd, line_ptr);
+      line_ptr += 1;
+      lh->standard_opcode_lengths.reset (new unsigned char[lh->opcode_base]);
+      lh->standard_opcode_lengths[0] = 1;  /* This should never be used anyway.  */
+
+      for (i = 1; i < lh->opcode_base; ++i)
+	{
+	  lh->standard_opcode_lengths[i] = read_1_byte (abfd, line_ptr);
+	  line_ptr += 1;
+	}
     }
 
-  if (lh->version >= 5)
+  if ((is_dwo && lh->dwo_version >= 5) || (!is_dwo && lh->version >=5))
     {
       /* Read directory table.  */
       read_formatted_entries (per_objfile, abfd, &line_ptr, lh.get (),
-			      offset_size,
+			      offset_size, is_dwo,
 			      [] (struct line_header *header, const char *name,
 				  dir_index d_index, unsigned int mod_time,
-				  unsigned int length)
+				  unsigned int length, bool is_dwo_section)
 	{
-	  header->add_include_dir (name);
+	  header->add_include_dir (name, is_dwo_section);
 	});
 
       /* Read file name table.  */
       read_formatted_entries (per_objfile, abfd, &line_ptr, lh.get (),
-			      offset_size,
+			      offset_size, is_dwo,
 			      [] (struct line_header *header, const char *name,
 				  dir_index d_index, unsigned int mod_time,
-				  unsigned int length)
+				  unsigned int length, bool is_dwo_section)
 	{
-	  header->add_file_name (name, d_index, mod_time, length);
+	  header->add_file_name (name, d_index, mod_time, length, is_dwo_section);
 	});
     }
   else
@@ -399,7 +485,7 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
       while ((cur_dir = read_direct_string (abfd, line_ptr, &bytes_read)) != NULL)
 	{
 	  line_ptr += bytes_read;
-	  lh->add_include_dir (cur_dir);
+	  lh->add_include_dir (cur_dir, is_dwo);
 	}
       line_ptr += bytes_read;
 
@@ -417,7 +503,7 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
 	  length = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
 	  line_ptr += bytes_read;
 
-	  lh->add_file_name (cur_file, d_index, mod_time, length);
+	  lh->add_file_name (cur_file, d_index, mod_time, length, is_dwo);
 	}
       line_ptr += bytes_read;
     }
@@ -426,5 +512,5 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
     complaint (_("line number info header doesn't "
 		 "fit in `.debug_line' section"));
 
-  return lh;
+  return true;
 }
diff --git a/gdb/dwarf2/line-header.h b/gdb/dwarf2/line-header.h
index 8fb44be56b2..48cf9a5b742 100644
--- a/gdb/dwarf2/line-header.h
+++ b/gdb/dwarf2/line-header.h
@@ -44,9 +44,12 @@ struct file_entry
       length (length_)
   {}
 
-  /* Return the include directory at D_INDEX stored in LH.  Returns
-     NULL if D_INDEX is out of bounds.  */
-  const char *include_dir (const line_header *lh) const;
+  /* Return the include directory at D_INDEX stored in m_include_dirs vector
+     in LH or in m_dwo_include_dirs vector in LH if IS_DWO is true. m_dwo_
+     include_dirs contains the include_directories entries of .debug_line.dwo
+     section whereas m_include_dirs contains the include_directories array
+     entries of .debug_line section. Returns NULL if D_INDEX is out of bounds.  */
+  const char *include_dir (const line_header *lh, bool is_dwo) const;
 
   /* The file name.  Note this is an observing pointer.  The memory is
      owned by debug_line_buffer.  */
@@ -75,25 +78,37 @@ struct line_header
     : offset_in_dwz {}
   {}
 
-  /* Add an entry to the include directory table.  */
-  void add_include_dir (const char *include_dir);
+  /* Add an entry to the m_include_dirs vector or to m_dwo_include_dirs vector
+     if IS_DWO is true.  */
+  void add_include_dir (const char *include_dir, bool is_dwo);
 
-  /* Add an entry to the file name table.  */
+  /* Add an entry to the m_file_names vector or to the m_dwo_file_names vector
+     if IS_DWO is true. m_dwo_file_names vector contains the file_names entries of
+     .debug_line.dwo section whereas m_file_names contains the file_names array
+     entries of .debug_line section.  */
   void add_file_name (const char *name, dir_index d_index,
-		      unsigned int mod_time, unsigned int length);
+		      unsigned int mod_time, unsigned int length, bool is_dwo);
 
-  /* Return the include dir at INDEX (0-based in DWARF 5 and 1-based before).
+  /* Return the include dir from m_include_dirs or from the m_dwo_include_dirs
+     if IS_DWO is true at INDEX (0-based in DWARF 5 and 1-based before).
      Returns NULL if INDEX is out of bounds.  */
-  const char *include_dir_at (dir_index index) const
+  const char *include_dir_at (dir_index index, bool is_dwo) const
   {
     int vec_index;
-    if (version >= 5)
+
+    if (is_dwo && m_dwo_include_dirs.empty())
+      is_dwo = false;
+
+    if ((is_dwo && dwo_version >= 5) || (!is_dwo && version >= 5))
       vec_index = index;
     else
       vec_index = index - 1;
-    if (vec_index < 0 || vec_index >= m_include_dirs.size ())
+    int include_dirs_size = is_dwo ? m_dwo_include_dirs.size () :
+			    m_include_dirs.size ();
+    if (vec_index < 0 || vec_index >= include_dirs_size)
       return NULL;
-    return m_include_dirs[vec_index];
+    return is_dwo ? m_dwo_include_dirs[vec_index] :
+		    m_include_dirs[vec_index];
   }
 
   bool is_valid_file_index (int file_index) const
@@ -103,25 +118,43 @@ struct line_header
     return 1 <= file_index && file_index <= file_names_size ();
   }
 
-  /* Return the file name at INDEX (0-based in DWARF 5 and 1-based before).
+  /* Returns true if the FILE_INDEX is a valid index in the file_names vector
+     of .debug_line.dwo section. */
+  bool is_valid_dwo_file_index (int file_index) const
+  {
+    if (dwo_version >= 5)
+      return 0 <= file_index && file_index < dwo_file_names_size ();
+    return 1 <= file_index && file_index <= dwo_file_names_size ();
+
+  }
+
+  /* Return the file name from m_file_names or from m_dwo_file_names vector
+     if IS_DWO is true at INDEX (0-based in DWARF 5 and 1-based before).
      Returns NULL if INDEX is out of bounds.  */
-  file_entry *file_name_at (file_name_index index)
+  file_entry *file_name_at (file_name_index index, bool is_dwo)
   {
     int vec_index;
-    if (version >= 5)
+
+    if (is_dwo && m_dwo_file_names.empty ())
+      is_dwo = false;
+
+    if ((is_dwo && dwo_version >= 5) || (!is_dwo && version >= 5))
       vec_index = index;
     else
       vec_index = index - 1;
-    if (vec_index < 0 || vec_index >= m_file_names.size ())
+    int file_names_size = is_dwo ? m_dwo_file_names.size ():
+			  m_file_names.size ();
+
+    if (vec_index < 0 || vec_index >= file_names_size)
       return NULL;
-    return &m_file_names[vec_index];
+    return is_dwo ? &m_dwo_file_names[vec_index] : &m_file_names[vec_index];
   }
 
   /* A const overload of the same.  */
-  const file_entry *file_name_at (file_name_index index) const
+  const file_entry *file_name_at (file_name_index index, bool is_dwo) const
   {
     line_header *lh = const_cast<line_header *> (this);
-    return lh->file_name_at (index);
+    return lh->file_name_at (index, is_dwo);
   }
 
   /* The indexes are 0-based in DWARF 5 and 1-based in DWARF 4. Therefore,
@@ -133,6 +166,12 @@ struct line_header
   const std::vector<file_entry> &file_names () const
   { return m_file_names; }
 
+  std::vector<file_entry> &dwo_file_names ()
+  { return m_dwo_file_names; }
+  /* A const overload of the same.  */
+  const std::vector<file_entry> &dwo_file_names () const
+  { return m_dwo_file_names; }
+
   /* Offset of line number information in .debug_line section.  */
   sect_offset sect_off {};
 
@@ -140,14 +179,18 @@ struct line_header
   unsigned offset_in_dwz : 1; /* Can't initialize bitfields in-class.  */
 
   unsigned int total_length {};
+  unsigned int dwo_total_length {};
   unsigned short version {};
+  unsigned short dwo_version {};
   unsigned int header_length {};
+  unsigned int dwo_header_length {};
   unsigned char minimum_instruction_length {};
   unsigned char maximum_ops_per_instruction {};
   unsigned char default_is_stmt {};
   int line_base {};
   unsigned char line_range {};
   unsigned char opcode_base {};
+  unsigned char dwo_opcode_base {};
 
   /* standard_opcode_lengths[i] is the number of operands for the
      standard opcode whose value is i.  This means that
@@ -155,9 +198,18 @@ struct line_header
      element is standard_opcode_lengths[opcode_base - 1].  */
   std::unique_ptr<unsigned char[]> standard_opcode_lengths;
 
+  /* dwo_standard_opcode_lengths[i] is the number of operands for the
+     standard opcode whose value is i.  This means that
+     dwo_standard_opcode_lengths[0] is unused, and the last meaningful
+     element is dwo_standard_opcode_lengths[dwo_opcode_base - 1].  */
+  std::unique_ptr<unsigned char[]> dwo_standard_opcode_lengths;
+
   int file_names_size () const
   { return m_file_names.size(); }
 
+  int dwo_file_names_size () const
+  { return m_dwo_file_names.size (); }
+
   /* The start and end of the statement program following this
      header.  These point into dwarf2_per_objfile->line_buffer.  */
   const gdb_byte *statement_program_start {}, *statement_program_end {};
@@ -173,19 +225,25 @@ struct line_header
      pointers.  The memory is owned by debug_line_buffer.  */
   std::vector<const char *> m_include_dirs;
 
+  /* The include_directories table from .debug_line.dwo section.  */
+  std::vector<const char *> m_dwo_include_dirs;
+
   /* The file_names table. This is private because the meaning of indexes
      differs among DWARF versions (The first valid index is 1 in DWARF 4 and
      before, and is 0 in DWARF 5 and later).  So the client should use
      file_name_at method for access.  */
   std::vector<file_entry> m_file_names;
+
+  /* The file_names table from .debug_line.dwo section.  */
+  std::vector<file_entry> m_dwo_file_names;
 };
 
 typedef std::unique_ptr<line_header> line_header_up;
 
 inline const char *
-file_entry::include_dir (const line_header *lh) const
+file_entry::include_dir (const line_header *lh, bool is_dwo) const
 {
-  return lh->include_dir_at (d_index);
+  return lh->include_dir_at (d_index, is_dwo);
 }
 
 /* Read the statement program header starting at SECT_OFF in SECTION.
@@ -196,8 +254,9 @@ file_entry::include_dir (const line_header *lh) const
    the returned object point into the dwarf line section buffer,
    and must not be freed.  */
 
-extern line_header_up dwarf_decode_line_header
+extern bool dwarf_decode_line_header
   (sect_offset sect_off, bool is_dwz, dwarf2_per_objfile *per_objfile,
-   struct dwarf2_section_info *section, const struct comp_unit_head *cu_header);
+   struct dwarf2_section_info *section, const struct comp_unit_head *cu_header,
+   line_header_up& lh, bool is_dwo);
 
 #endif /* DWARF2_LINE_HEADER_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 10550336063..b9700f70e9c 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1544,7 +1544,7 @@ static const char *compute_include_file_name
      (const struct line_header *lh,
       const file_entry &fe,
       const file_and_directory &cu_info,
-      gdb::unique_xmalloc_ptr<char> *name_holder);
+      gdb::unique_xmalloc_ptr<char> *name_holder, bool is_dwo);
 
 static htab_up allocate_signatured_type_table ();
 
@@ -3027,7 +3027,19 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
 	{
 	  gdb::unique_xmalloc_ptr<char> name_holder;
 	  const char *include_name =
-	    compute_include_file_name (lh.get (), entry, fnd, &name_holder);
+	    compute_include_file_name (lh.get (), entry, fnd, &name_holder, false);
+	  if (include_name != nullptr)
+	    {
+	      include_name = per_objfile->objfile->intern (include_name);
+	      include_names.push_back (include_name);
+	    }
+	}
+
+      for (const auto &entry : lh->dwo_file_names ())
+	{
+	  gdb::unique_xmalloc_ptr<char> name_holder;
+	  const char *include_name =
+	    compute_include_file_name (lh.get (), entry, fnd, &name_holder, true);
 	  if (include_name != nullptr)
 	    {
 	      include_name = per_objfile->objfile->intern (include_name);
@@ -10753,7 +10765,7 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
 	{
 	  file_entry &fe = file_names[i];
 	  dwarf2_start_subfile (this, fe.name,
-				fe.include_dir (line_header));
+				fe.include_dir (line_header, false));
 	  buildsym_compunit *b = get_builder ();
 	  if (b->get_current_subfile ()->symtab == NULL)
 	    {
@@ -20722,7 +20734,7 @@ get_debug_line_section (struct dwarf2_cu *cu)
 }
 
 /* Read the statement program header starting at OFFSET in
-   .debug_line, or .debug_line.dwo.  Return a pointer
+   .debug_line and .debug_line.dwo.  Return a pointer
    to a struct line_header, allocated using xmalloc.
    Returns NULL if there is a problem reading the header, e.g., if it
    has a version we don't understand.
@@ -20735,21 +20747,48 @@ static line_header_up
 dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
 {
   struct dwarf2_section_info *section;
+  struct dwarf2_section_info *dwo_section = NULL;
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
 
   section = get_debug_line_section (cu);
   section->read (per_objfile->objfile);
+
   if (section->buffer == NULL)
+  {
+    if (cu->dwo_unit && cu->per_cu->is_debug_types)
+      complaint (_("missing .debug_line.dwo section"));
+    else
+      complaint (_("missing .debug_line section"));
+    return 0;
+  }
+
+  line_header_up lh (new line_header ());
+
+  if (!dwarf_decode_line_header (sect_off, cu->per_cu->is_dwz,
+				 per_objfile, section,
+				 &cu->header, lh, false))
     {
-      if (cu->dwo_unit && cu->per_cu->is_debug_types)
-	complaint (_("missing .debug_line.dwo section"));
-      else
-	complaint (_("missing .debug_line section"));
       return 0;
     }
 
-  return dwarf_decode_line_header (sect_off, cu->per_cu->is_dwz,
-				   per_objfile, section, &cu->header);
+  if (cu->dwo_unit && !cu->per_cu->is_debug_types)
+    {
+      dwo_section = &cu->dwo_unit->dwo_file->sections.line;
+      dwo_section->read (per_objfile->objfile);
+    }
+
+  if (cu->dwo_unit && dwo_section && (dwo_section->buffer != NULL))
+    {
+      sect_offset dwo_sect_off = (sect_offset) 0;
+      if (!dwarf_decode_line_header (dwo_sect_off, cu->per_cu->is_dwz,
+				     per_objfile, dwo_section,
+				     &cu->header, lh, true))
+      {
+	return 0;
+      }
+    }
+
+  return lh;
 }
 
 /* Subroutine of dwarf_decode_lines to simplify it.
@@ -20762,12 +20801,20 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
 static const char *
 compute_include_file_name (const struct line_header *lh, const file_entry &fe,
 			   const file_and_directory &cu_info,
-			   gdb::unique_xmalloc_ptr<char> *name_holder)
+			   gdb::unique_xmalloc_ptr<char> *name_holder, bool is_dwo)
 {
   const char *include_name = fe.name;
   const char *include_name_to_compare = include_name;
+  const char *dir_name;
 
-  const char *dir_name = fe.include_dir (lh);
+  if (is_dwo)
+    {
+      dir_name = fe.include_dir (lh, true);
+    }
+  else
+    {
+      dir_name = fe.include_dir (lh, false);
+    }
 
   gdb::unique_xmalloc_ptr<char> hold_compare;
   if (!IS_ABSOLUTE_PATH (include_name)
@@ -20838,7 +20885,7 @@ class lnp_state_machine
   {
     /* lh->file_names is 0-based, but the file name numbers in the
        statement program are 1-based.  */
-    return m_line_header->file_name_at (m_file);
+    return m_line_header->file_name_at (m_file, false);
   }
 
   /* Record the line in the state machine.  END_SEQUENCE is true if
@@ -21012,7 +21059,7 @@ lnp_state_machine::handle_set_file (file_name_index file)
     dwarf2_debug_line_missing_file_complaint ();
   else if (m_record_lines_p)
     {
-      const char *dir = fe->include_dir (m_line_header);
+      const char *dir = fe->include_dir (m_line_header, false);
 
       m_last_subfile = m_cu->get_builder ()->get_current_subfile ();
       m_line_has_non_zero_discriminator = m_discriminator != 0;
@@ -21304,7 +21351,7 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
 	  const file_entry *fe = state_machine.current_file ();
 
 	  if (fe != NULL)
-	    dwarf2_start_subfile (cu, fe->name, fe->include_dir (lh));
+	    dwarf2_start_subfile (cu, fe->name, fe->include_dir (lh, false));
 	}
 
       /* Decode the table.  */
@@ -21369,7 +21416,7 @@ dwarf_decode_lines_1 (struct line_header *lh, struct dwarf2_cu *cu,
 		    length =
 		      read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
 		    line_ptr += bytes_read;
-		    lh->add_file_name (cur_file, dindex, mod_time, length);
+		    lh->add_file_name (cur_file, dindex, mod_time, length, false);
 		  }
 		  break;
 		case DW_LNE_set_discriminator:
@@ -21526,7 +21573,20 @@ dwarf_decode_lines (struct line_header *lh, const file_and_directory &fnd,
 	  {
 	    gdb::unique_xmalloc_ptr<char> name_holder;
 	    const char *include_name =
-	      compute_include_file_name (lh, file_entry, fnd, &name_holder);
+	      compute_include_file_name (lh, file_entry, fnd, &name_holder, false);
+	    if (include_name != NULL)
+	      dwarf2_create_include_psymtab
+		(cu->per_objfile->per_bfd, include_name, pst,
+		 cu->per_objfile->per_bfd->partial_symtabs.get (),
+		 objfile->per_bfd);
+	  }
+
+      for (auto &file_entry : lh->dwo_file_names ())
+	if (file_entry.included_p)
+	  {
+	    gdb::unique_xmalloc_ptr<char> name_holder;
+	    const char *include_name =
+	      compute_include_file_name (lh, file_entry, fnd, &name_holder, true);
 	    if (include_name != NULL)
 	      dwarf2_create_include_psymtab
 		(cu->per_objfile->per_bfd, include_name, pst,
@@ -21544,7 +21604,19 @@ dwarf_decode_lines (struct line_header *lh, const file_and_directory &fnd,
 
       for (auto &fe : lh->file_names ())
 	{
-	  dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh));
+	  dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh, false));
+	  if (builder->get_current_subfile ()->symtab == NULL)
+	    {
+	      builder->get_current_subfile ()->symtab
+		= allocate_symtab (cust,
+				   builder->get_current_subfile ()->name);
+	    }
+	  fe.symtab = builder->get_current_subfile ()->symtab;
+	}
+
+      for (auto &fe : lh->dwo_file_names ())
+	{
+	  dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh, true));
 	  if (builder->get_current_subfile ()->symtab == NULL)
 	    {
 	      builder->get_current_subfile ()->symtab
@@ -21760,7 +21832,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	  struct file_entry *fe;
 
 	  if (cu->line_header != NULL)
-	    fe = cu->line_header->file_name_at (file_index);
+	    fe = cu->line_header->file_name_at (file_index, (cu->dwo_unit != nullptr));
 	  else
 	    fe = NULL;
 
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index 7ca3f312d33..73a917541b6 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -238,7 +238,7 @@ compare_locations (struct macro_source_file *file1, int line1,
 
   /* If the two files are not the same, find their common ancestor in
      the #inclusion tree.  */
-  if (file1 != file2)
+  if (file1 != file2 || filename_cmp(file1->filename, file2->filename) != 0)
     {
       /* If one file is deeper than the other, walk up the #inclusion
 	 chain until the two files are at least at the same *depth*.
@@ -266,7 +266,7 @@ compare_locations (struct macro_source_file *file1, int line1,
 
       /* Now both file1 and file2 are at the same depth.  Walk toward
 	 the root of the tree until we find where the branches meet.  */
-      while (file1 != file2)
+      while (file1 != file2 || filename_cmp(file1->filename, file2->filename) != 0)
 	{
 	  line1 = file1->included_at_line;
 	  file1 = file1->included_by;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf.
  2022-03-14 14:32   ` Achra, Nitika
@ 2022-03-18 20:15     ` Keith Seitz
  2022-03-18 20:23       ` Keith Seitz
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Seitz @ 2022-03-18 20:15 UTC (permalink / raw)
  To: Achra, Nitika, gdb-patches; +Cc: George, Jini Susan, Natarajan, Kavitha

On 3/14/22 07:32, Achra, Nitika wrote:
> Thanks for the review and sorry for the delay in the reply.

No problem; it happens. [At least to me!]

I'm happy to see you return to the problem/patch.

> I have fixed the regressions mentioned by you. Also, I have removed
> the change you have posted for the RFC. I have attached the updated
> patch. So, this patch along with your fix will result in the
> following for gdb.base/macscp.exp testcase with -gsplit-dwarf:
>
> 
> Command Used:
> 
> make check RUNTESTFLAGS='CC_FOR_TARGET="clang -gdwarf-5 -gsplit-dwarf -fdebug-macro"'
> 
>                           TESTS="gdb.base/macscp.exp"

Just a cautionary note: When running C++ tests (gdb.cp), don't forget to
set CXX_FOR_TARGET, too.

> Compiler used: clang 12.0.0

I tested w/GCC and Clang 12 (Fedora 12.0.1-1.fc34)
GCC results show no regressions.

> *Before the patch:*
> 
> Number of expected passes            234
> Number of unexpected failures        65
> Number of known failures             40

I also see those results with origin/master.

> *After applying the attached patch and including your change also:*
> 
> Number of expected passes            319
> Number of unexpected failures        1
> Number of known failures             19

After applying both our patches, I have an equal number of PASSes,
but 4 unexpected failures. [which could mean just about anything]

> Clang is emitting both .debug_line and .debug_line.dwo sections and
> .debug_macro.dwo is pointing to the .debug_line.dwo section(file
> index in the .debug_macro.dwo is the index in the file_names array
> of .debug_line.dwo). However, GDB is reading only .debug_line section.
>
> In this patch, I have added the support for reading .debug_line.dwo 
> section and added m_dwo_include_dirs and m_dwo_file_names vectors in 
> struct line_header to store the directories and files respectively 
> and reading the file and directory names from these vectors whenever
> required.
I think there is still something missing. Regression testing the entire
testsuite (with both our patches), I see the following three issues
(w/clang; gcc is fine):

1. gdb.base/included.exp
(gdb) list integer
-18     int integer; [PASS]
+18     #include "included.h" [FAIL]

2. gdb.cp/m-static.exp
(gdb) info variable everywhere
-File ../../../src/gdb/testsuite/gdb.cp/m-static.h: [PASS]
+File ../../../src/gdb/testsuite/gdb.cp/m-static.cc: [FAIL]

3. gdb.threads/siginfo-threads.exp
(gdb) p $_siginfo.si_signo
-$4 = 3182372 [PASS]
+There is no member named _sifields. [FAIL]
(gdb) p $_siginfo._sifields._kill.si_pid
-$7 = 3182372 [PASS]
+There is no member named _sifields. [FAIL]

I haven't pursued these further to understand the problem.

See other discussion below.

Keith

> diff --git a/gdb/dwarf2/line-header.h b/gdb/dwarf2/line-header.h
> index 8fb44be56b2..48cf9a5b742 100644
> --- a/gdb/dwarf2/line-header.h
> +++ b/gdb/dwarf2/line-header.h
> @@ -44,9 +44,12 @@ struct file_entry
>        length (length_)
>    {}
>  
> -  /* Return the include directory at D_INDEX stored in LH.  Returns
> -     NULL if D_INDEX is out of bounds.  */
> -  const char *include_dir (const line_header *lh) const;
> +  /* Return the include directory at D_INDEX stored in m_include_dirs vector
> +     in LH or in m_dwo_include_dirs vector in LH if IS_DWO is true. m_dwo_
> +     include_dirs contains the include_directories entries of .debug_line.dwo
> +     section whereas m_include_dirs contains the include_directories array
> +     entries of .debug_line section. Returns NULL if D_INDEX is out of bounds.  */
> +  const char *include_dir (const line_header *lh, bool is_dwo) const;

Regarding passing a boolean everywhere (like above) and special-casing on this,
like:

> -  /* Return the include dir at INDEX (0-based in DWARF 5 and 1-based before).
> +  /* Return the include dir from m_include_dirs or from the m_dwo_include_dirs
> +     if IS_DWO is true at INDEX (0-based in DWARF 5 and 1-based before).
>       Returns NULL if INDEX is out of bounds.  */
> -  const char *include_dir_at (dir_index index) const
> +  const char *include_dir_at (dir_index index, bool is_dwo) const
>    {
>      int vec_index;
> -    if (version >= 5)
> +
> +    if (is_dwo && m_dwo_include_dirs.empty())
> +      is_dwo = false;
> +
> +    if ((is_dwo && dwo_version >= 5) || (!is_dwo && version >= 5))
>        vec_index = index;
>      else
>        vec_index = index - 1;
> -    if (vec_index < 0 || vec_index >= m_include_dirs.size ())
> +    int include_dirs_size = is_dwo ? m_dwo_include_dirs.size () :
> +			    m_include_dirs.size ();
> +    if (vec_index < 0 || vec_index >= include_dirs_size)
>        return NULL;
> -    return m_include_dirs[vec_index];
> +    return is_dwo ? m_dwo_include_dirs[vec_index] :
> +		    m_include_dirs[vec_index];
>    }

I've seen this implementation pattern elsewhere (mostly older code), and
I just can't help but ask... It seems to unnecessarily clutter the code compared
to creating a new struct/class, inheriting from the existing line_header. [In other
words, it seems C-ish to me.]

Was that approach considered?

Keith


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf.
  2022-03-18 20:15     ` Keith Seitz
@ 2022-03-18 20:23       ` Keith Seitz
  2022-05-17  5:40         ` Natarajan, Kavitha
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Seitz @ 2022-03-18 20:23 UTC (permalink / raw)
  To: Achra, Nitika, gdb-patches; +Cc: George, Jini Susan, Natarajan, Kavitha

On 3/18/22 13:15, Keith Seitz via Gdb-patches wrote:
> I've seen this implementation pattern elsewhere (mostly older code), and
> I just can't help but ask... It seems to unnecessarily clutter the code compared
> to creating a new struct/class, inheriting from the existing line_header. [In other
> words, it seems C-ish to me.]
> 
> Was that approach considered?

For the record (my bad), this might actually mean putting all the things that is_dwo
changes into a class inside line_header.

Pardon my hasty reply.

Keith


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf.
  2022-03-18 20:23       ` Keith Seitz
@ 2022-05-17  5:40         ` Natarajan, Kavitha
  2022-05-17  8:47           ` Natarajan, Kavitha
  0 siblings, 1 reply; 7+ messages in thread
From: Natarajan, Kavitha @ 2022-05-17  5:40 UTC (permalink / raw)
  To: Keith Seitz, Achra, Nitika, gdb-patches; +Cc: George, Jini Susan

[Public]

> On 3/18/22 13:15, Keith Seitz via Gdb-patches wrote:
> > I've seen this implementation pattern elsewhere (mostly older code),
> > and I just can't help but ask... It seems to unnecessarily clutter the
> > code compared to creating a new struct/class, inheriting from the
> > existing line_header. [In other words, it seems C-ish to me.]

Hi Keith,

I have moved the is_dwo related changes within line_header class which avoids
passing the flag across functions. All gdb.base/macscp.exp failures are passing
now and also the regressions which you mentioned in your previous email is
not appearing with the latest patch. I have also tested this patch with gcc with all
combination of flags (-gsplit-dwarf -gdwarf-[45]). I am attaching the latest patch for you review.

Please note that your patch in line_header::file_file_name() ( https://sourceware.org/pipermail/gdb-patches/2021-June/179698.html )
is also included in this patch which is needed for its completion. The change is little
different than your original patch due to the latest changes in the function.

Regards,
Kavitha
==================================================================
Clang emits both .debug_line and .debug_line.dwo sections.
The section .debug__macro.dwo points to the .debug_line.dwo section
(file index in the .debug_macro.dwo is the index in the file_names
array of .debug_line.dwo). However, GDB is reading only .debug_line
section. Added support for reading .debug_line.dwo section and added
m_dwo_include_dirs and m_dwo_file_names vectors in struct line_header
to store the directories and files respectively.

This fix removes a number failures in gdb.base/macscp.exp testcase.

---
 gdb/dwarf2/line-header.c | 156 +++++++++++++++++++++++----------------
 gdb/dwarf2/line-header.h | 103 +++++++++++++++++++++-----
 gdb/dwarf2/read.c        |  38 +++++++++-
 gdb/macrotab.c           |   6 +-
 4 files changed, 216 insertions(+), 87 deletions(-)

diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c
index 63230847568..02ef3652efb 100644
--- a/gdb/dwarf2/line-header.c
+++ b/gdb/dwarf2/line-header.c
@@ -32,14 +32,19 @@ line_header::add_include_dir (const char *include_dir)
   if (dwarf_line_debug >= 2)
     {
       size_t new_size;
-      if (version >= 5)
-       new_size = m_include_dirs.size ();
+      int include_dir_size = is_dwo ? m_dwo_include_dirs.size ():
+                                     m_include_dirs.size ();
+      if (get_version () >= 5)
+       new_size = include_dir_size;
       else
-       new_size = m_include_dirs.size () + 1;
+       new_size = include_dir_size + 1;
       gdb_printf (gdb_stdlog, "Adding dir %zu: %s\n",
                  new_size, include_dir);
     }
-  m_include_dirs.push_back (include_dir);
+  if (is_dwo)
+    m_dwo_include_dirs.push_back (include_dir);
+  else
+    m_include_dirs.push_back (include_dir);
 }

 void
@@ -51,33 +56,32 @@ line_header::add_file_name (const char *name,
   if (dwarf_line_debug >= 2)
     {
       size_t new_size;
-      if (version >= 5)
-       new_size = file_names_size ();
+      int file_name_size = is_dwo ? dwo_file_names_size ():
+                                    file_names_size ();
+      if (get_version () >= 5)
+       new_size = file_name_size;
       else
-       new_size = file_names_size () + 1;
+       new_size = file_name_size + 1;
       gdb_printf (gdb_stdlog, "Adding file %zu: %s\n",
                  new_size, name);
     }
-  m_file_names.emplace_back (name, d_index, mod_time, length);
+  if (is_dwo)
+    m_dwo_file_names.emplace_back (name, d_index, mod_time, length);
+  else
+    m_file_names.emplace_back (name, d_index, mod_time, length);
 }

 std::string
 line_header::file_file_name (int file) const
 {
   /* Is the file number a valid index into the line header's file name
-     table?  Remember that file numbers start with one, not zero.  */
+     table?  First check in the .debug_line.dwo file table if present,
+     otherwise, in .debug_line table. Remember that file numbers start
+     with one in DWARFv4 and with zero in DWARFv5.  */
   if (is_valid_file_index (file))
     {
       const file_entry *fe = file_name_at (file);
-
-      if (!IS_ABSOLUTE_PATH (fe->name))
-       {
-         const char *dir = fe->include_dir (this);
-         if (dir != NULL)
-           return path_join (dir, fe->name);
-       }
-
-      return fe->name;
+      return fe->symtab->filename;
     }
   else
     {
@@ -260,11 +264,12 @@ read_formatted_entries (dwarf2_per_objfile *per_objfile, bfd *abfd,

 /* See line-header.h.  */

-line_header_up
+bool
 dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
                           dwarf2_per_objfile *per_objfile,
                           struct dwarf2_section_info *section,
-                          const struct comp_unit_head *cu_header)
+                          const struct comp_unit_head *cu_header,
+                          line_header_up& lh)
 {
   const gdb_byte *line_ptr;
   unsigned int bytes_read, offset_size;
@@ -278,11 +283,9 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
   if (to_underlying (sect_off) + 4 >= section->size)
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
-      return 0;
+      return false;
     }

-  line_header_up lh (new line_header ());
-
   lh->sect_off = sect_off;
   lh->offset_in_dwz = is_dwz;

@@ -294,24 +297,28 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
                                              &bytes_read, &offset_size);
   line_ptr += bytes_read;

-  const gdb_byte *start_here = line_ptr;
-
   if (line_ptr + unit_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
-      return 0;
+      return false;
+    }
+  if (lh->is_dwo)
+    lh->dwo_version = read_2_bytes (abfd, line_ptr);
+  else
+    {
+      lh->statement_program_end = line_ptr + unit_length;
+      lh->version = read_2_bytes (abfd, line_ptr);
     }
-  lh->statement_program_end = start_here + unit_length;
-  lh->version = read_2_bytes (abfd, line_ptr);
   line_ptr += 2;
-  if (lh->version > 5)
+  if (lh->get_version () > 5)
     {
       /* This is a version we don't understand.  The format could have
         changed in ways we don't handle properly so just punt.  */
-      complaint (_("unsupported version in .debug_line section"));
-      return NULL;
+      complaint (_("unsupported version in .debug_line and "
+                  "in .debug_line.dwo section"));
+      return false;
     }
-  if (lh->version >= 5)
+  if (lh->get_version () >= 5)
     {
       gdb_byte segment_selector_size;

@@ -324,51 +331,74 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
       if (segment_selector_size != 0)
        {
          complaint (_("unsupported segment selector size %u "
-                      "in .debug_line section"),
+                      "in .debug_line and .debug_line.dwo section"),
                     segment_selector_size);
-         return NULL;
+         return false;
        }
     }

   LONGEST header_length = read_offset (abfd, line_ptr, offset_size);
   line_ptr += offset_size;
-  lh->statement_program_start = line_ptr + header_length;
-  lh->minimum_instruction_length = read_1_byte (abfd, line_ptr);
-  line_ptr += 1;
-
-  if (lh->version >= 4)
+  if (lh->is_dwo)
     {
-      lh->maximum_ops_per_instruction = read_1_byte (abfd, line_ptr);
+      line_ptr += 1; /* Skip the minimum instruction length.  */
+      if (lh->dwo_version >= 4)
+       line_ptr += 1; /* Skip maximum ops per instruction.  */
+      line_ptr += 1; /* Skip default is stmt.  */
+      line_ptr += 2; /* Skip line base and line range.  */
+
+      /* Read the opcode base.  */
+      lh->dwo_opcode_base = read_1_byte (abfd, line_ptr);
       line_ptr += 1;
+      lh->dwo_standard_opcode_lengths.reset (new unsigned char[lh->dwo_opcode_base]);
+      lh->dwo_standard_opcode_lengths[0] = 1;  /* This should never be used anyway.  */
+
+      for (i = 1; i < lh->dwo_opcode_base; ++i)
+       {
+         lh->dwo_standard_opcode_lengths[i] = read_1_byte (abfd, line_ptr);
+         line_ptr += 1;
+       }
     }
   else
-    lh->maximum_ops_per_instruction = 1;
-
-  if (lh->maximum_ops_per_instruction == 0)
     {
-      lh->maximum_ops_per_instruction = 1;
-      complaint (_("invalid maximum_ops_per_instruction "
-                  "in `.debug_line' section"));
-    }
+      lh->statement_program_start = line_ptr + header_length;
+      lh->minimum_instruction_length = read_1_byte (abfd, line_ptr);
+      line_ptr += 1;

-  lh->default_is_stmt = read_1_byte (abfd, line_ptr);
-  line_ptr += 1;
-  lh->line_base = read_1_signed_byte (abfd, line_ptr);
-  line_ptr += 1;
-  lh->line_range = read_1_byte (abfd, line_ptr);
-  line_ptr += 1;
-  lh->opcode_base = read_1_byte (abfd, line_ptr);
-  line_ptr += 1;
-  lh->standard_opcode_lengths.reset (new unsigned char[lh->opcode_base]);
-
-  lh->standard_opcode_lengths[0] = 1;  /* This should never be used anyway.  */
-  for (i = 1; i < lh->opcode_base; ++i)
-    {
-      lh->standard_opcode_lengths[i] = read_1_byte (abfd, line_ptr);
+      if (lh->get_version () >= 4)
+       {
+         lh->maximum_ops_per_instruction = read_1_byte (abfd, line_ptr);
+         line_ptr += 1;
+       }
+      else
+       lh->maximum_ops_per_instruction = 1;
+
+      if (lh->maximum_ops_per_instruction == 0)
+       {
+         lh->maximum_ops_per_instruction = 1;
+         complaint (_("invalid maximum_ops_per_instruction "
+                      "in `.debug_line' section"));
+       }
+
+      lh->default_is_stmt = read_1_byte (abfd, line_ptr);
+      line_ptr += 1;
+      lh->line_base = read_1_signed_byte (abfd, line_ptr);
+      line_ptr += 1;
+      lh->line_range = read_1_byte (abfd, line_ptr);
       line_ptr += 1;
+      lh->opcode_base = read_1_byte (abfd, line_ptr);
+      line_ptr += 1;
+      lh->standard_opcode_lengths.reset (new unsigned char[lh->opcode_base]);
+
+      lh->standard_opcode_lengths[0] = 1;  /* This should never be used anyway.  */
+      for (i = 1; i < lh->opcode_base; ++i)
+       {
+         lh->standard_opcode_lengths[i] = read_1_byte (abfd, line_ptr);
+         line_ptr += 1;
+       }
     }

-  if (lh->version >= 5)
+  if (lh->get_version () >= 5)
     {
       /* Read directory table.  */
       read_formatted_entries (per_objfile, abfd, &line_ptr, lh.get (),
@@ -423,5 +453,5 @@ dwarf_decode_line_header  (sect_offset sect_off, bool is_dwz,
     complaint (_("line number info header doesn't "
                 "fit in `.debug_line' section"));

-  return lh;
+  return true;
 }
diff --git a/gdb/dwarf2/line-header.h b/gdb/dwarf2/line-header.h
index 59b317d0d72..014ad831c13 100644
--- a/gdb/dwarf2/line-header.h
+++ b/gdb/dwarf2/line-header.h
@@ -44,7 +44,11 @@ struct file_entry
       length (length_)
   {}

-  /* Return the include directory at D_INDEX stored in LH.  Returns
+  /* Return the include directory at D_INDEX stored in m_include_dirs
+     vector in LH or in m_dwo_include_dirs vector in LH if is_dwo is
+     true. m_dwo_include_dirs contains the include_directories entries
+     of .debug_line.dwo section whereas m_include_dirs contains the
+     include_directories array entries of .debug_line section. Returns
      NULL if D_INDEX is out of bounds.  */
   const char *include_dir (const line_header *lh) const;

@@ -72,46 +76,71 @@ struct line_header
     : offset_in_dwz {}
   {}

-  /* Add an entry to the include directory table.  */
+  /* Add an entry to the m_include_dirs vector or to m_dwo_include_dirs
+     vector if is_dwo is true.  */
   void add_include_dir (const char *include_dir);

-  /* Add an entry to the file name table.  */
+  /* Add an entry to the m_file_names vector or to the m_dwo_file_names vector
+     if is_dwo is true. m_dwo_file_names vector contains the file_names entries
+     of .debug_line.dwo section whereas m_file_names contains the file_names
+     array entries of .debug_line section.  */
   void add_file_name (const char *name, dir_index d_index,
                      unsigned int mod_time, unsigned int length);

-  /* Return the include dir at INDEX (0-based in DWARF 5 and 1-based before).
+  /* Return the include dir from m_include_dirs or from the m_dwo_include_dirs
+     if is_dwo is true at INDEX (0-based in DWARF 5 and 1-based before).
      Returns NULL if INDEX is out of bounds.  */
   const char *include_dir_at (dir_index index) const
   {
     int vec_index;
-    if (version >= 5)
+    bool is_dwo_dir = is_dwo;
+
+    if (is_dwo && m_dwo_include_dirs.empty())
+      is_dwo_dir = false;
+
+    if (get_version () >= 5)
       vec_index = index;
     else
       vec_index = index - 1;
-    if (vec_index < 0 || vec_index >= m_include_dirs.size ())
+
+    int include_dirs_size = is_dwo_dir ? m_dwo_include_dirs.size () :
+                                    m_include_dirs.size ();
+    if (vec_index < 0 || vec_index >= include_dirs_size)
       return NULL;
-    return m_include_dirs[vec_index];
+    return is_dwo_dir ? m_dwo_include_dirs[vec_index] :
+                   m_include_dirs[vec_index];
   }

+  /* Return the file name at INDEX (0-based in DWARF 5 and 1-based before).  */
   bool is_valid_file_index (int file_index) const
   {
-    if (version >= 5)
-      return 0 <= file_index && file_index < file_names_size ();
-    return 1 <= file_index && file_index <= file_names_size ();
+    int file_names_size = is_dwo ? m_dwo_file_names.size () :
+                                  m_file_names.size ();
+    if (get_version () >= 5)
+      return 0 <= file_index && file_index < file_names_size;
+    return 1 <= file_index && file_index <= file_names_size;
   }

-  /* Return the file name at INDEX (0-based in DWARF 5 and 1-based before).
+  /* Return the file name from m_file_names or from m_dwo_file_names vector
+     if is_dwo is true at INDEX (0-based in DWARF 5 and 1-based before).
      Returns NULL if INDEX is out of bounds.  */
   file_entry *file_name_at (file_name_index index)
   {
     int vec_index;
-    if (version >= 5)
+    bool is_dwo_file = is_dwo;
+
+    if (is_dwo && m_dwo_file_names.empty ())
+      is_dwo_file = false;
+
+    if (get_version () >= 5)
       vec_index = index;
     else
       vec_index = index - 1;
-    if (vec_index < 0 || vec_index >= m_file_names.size ())
+    int file_names_size = is_dwo_file ? m_dwo_file_names.size ():
+                                  m_file_names.size ();
+    if (vec_index < 0 || vec_index >= file_names_size)
       return NULL;
-    return &m_file_names[vec_index];
+    return is_dwo_file ? &m_dwo_file_names[vec_index] : &m_file_names[vec_index];
   }

   /* A const overload of the same.  */
@@ -125,10 +154,20 @@ struct line_header
      this method should only be used to iterate through all file entries in an
      index-agnostic manner.  */
   std::vector<file_entry> &file_names ()
-  { return m_file_names; }
+  {
+    if (is_dwo)
+      return m_dwo_file_names;
+    else
+      return m_file_names;
+  }
   /* A const overload of the same.  */
   const std::vector<file_entry> &file_names () const
-  { return m_file_names; }
+  {
+    if (is_dwo)
+      return m_dwo_file_names;
+    else
+      return m_file_names;
+  }

   /* Offset of line number information in .debug_line section.  */
   sect_offset sect_off {};
@@ -136,13 +175,16 @@ 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.  */

+  bool is_dwo {};
   unsigned short version {};
+  unsigned short dwo_version {};
   unsigned char minimum_instruction_length {};
   unsigned char maximum_ops_per_instruction {};
   unsigned char default_is_stmt {};
   int line_base {};
   unsigned char line_range {};
   unsigned char opcode_base {};
+  unsigned char dwo_opcode_base {};

   /* standard_opcode_lengths[i] is the number of operands for the
      standard opcode whose value is i.  This means that
@@ -150,9 +192,18 @@ struct line_header
      element is standard_opcode_lengths[opcode_base - 1].  */
   std::unique_ptr<unsigned char[]> standard_opcode_lengths;

+  /* dwo_standard_opcode_lengths[i] is the number of operands for the
+     standard opcode whose value is i.  This means that
+     dwo_standard_opcode_lengths[0] is unused, and the last meaningful
+     element is dwo_standard_opcode_lengths[dwo_opcode_base - 1].  */
+  std::unique_ptr<unsigned char[]> dwo_standard_opcode_lengths;
+
   int file_names_size () const
   { return m_file_names.size(); }

+  int dwo_file_names_size () const
+  { return m_dwo_file_names.size (); }
+
   /* The start and end of the statement program following this
      header.  These point into dwarf2_per_objfile->line_buffer.  */
   const gdb_byte *statement_program_start {}, *statement_program_end {};
@@ -161,16 +212,31 @@ struct line_header
      number FILE in this object's file name table.  */
   std::string file_file_name (int file) const;

+  /* Get version of line header.  */
+  unsigned short get_version () const
+  {
+    if (is_dwo)
+      return dwo_version;
+    else
+      return version;
+  }
+
  private:
   /* The include_directories table.  Note these are observing
      pointers.  The memory is owned by debug_line_buffer.  */
   std::vector<const char *> m_include_dirs;

+  /* The include_directories table from .debug_line.dwo section.  */
+  std::vector<const char *> m_dwo_include_dirs;
+
   /* The file_names table. This is private because the meaning of indexes
      differs among DWARF versions (The first valid index is 1 in DWARF 4 and
      before, and is 0 in DWARF 5 and later).  So the client should use
      file_name_at method for access.  */
   std::vector<file_entry> m_file_names;
+
+  /* The file_names table from .debug_line.dwo section.  */
+  std::vector<file_entry> m_dwo_file_names;
 };

 typedef std::unique_ptr<line_header> line_header_up;
@@ -189,8 +255,9 @@ file_entry::include_dir (const line_header *lh) const
    the returned object point into the dwarf line section buffer,
    and must not be freed.  */

-extern line_header_up dwarf_decode_line_header
+extern bool dwarf_decode_line_header
   (sect_offset sect_off, bool is_dwz, dwarf2_per_objfile *per_objfile,
-   struct dwarf2_section_info *section, const struct comp_unit_head *cu_header);
+   struct dwarf2_section_info *section, const struct comp_unit_head *cu_header,
+   line_header_up& lh);

 #endif /* DWARF2_LINE_HEADER_H */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d146d525066..b7c2758b0f1 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -19654,7 +19654,7 @@ get_debug_line_section (struct dwarf2_cu *cu)
 }

 /* Read the statement program header starting at OFFSET in
-   .debug_line, or .debug_line.dwo.  Return a pointer
+   .debug_line and .debug_line.dwo.  Return a pointer
    to a struct line_header, allocated using xmalloc.
    Returns NULL if there is a problem reading the header, e.g., if it
    has a version we don't understand.
@@ -19667,6 +19667,7 @@ static line_header_up
 dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
 {
   struct dwarf2_section_info *section;
+  struct dwarf2_section_info *dwo_section = NULL;
   dwarf2_per_objfile *per_objfile = cu->per_objfile;

   section = get_debug_line_section (cu);
@@ -19680,8 +19681,31 @@ 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,
-                                  per_objfile, section, &cu->header);
+  line_header_up lh (new line_header ());
+  lh->is_dwo = false;
+  if (!dwarf_decode_line_header (sect_off, cu->per_cu->is_dwz,
+                                per_objfile, section,
+                                &cu->header, lh))
+    return 0;
+
+  if (cu->dwo_unit)
+    {
+      if (!cu->per_cu->is_debug_types)
+       {
+         dwo_section = &cu->dwo_unit->dwo_file->sections.line;
+         dwo_section->read (per_objfile->objfile);
+       }
+      if (dwo_section && (dwo_section->buffer != NULL))
+       {
+         sect_offset dwo_sect_off = (sect_offset) 0;
+         lh->is_dwo = true;
+         if (!dwarf_decode_line_header (dwo_sect_off, cu->per_cu->is_dwz,
+                                        per_objfile, dwo_section,
+                                        &cu->header, lh))
+           return 0;
+       }
+    }
+  return lh;
 }

 /* Subroutine of dwarf_decode_lines to simplify it.
@@ -20432,7 +20456,13 @@ dwarf_decode_lines (struct line_header *lh, struct dwarf2_cu *cu,
                    CORE_ADDR lowpc, int decode_mapping)
 {
   if (decode_mapping)
-    dwarf_decode_lines_1 (lh, cu, lowpc);
+    {
+      /* No need to look at .debug_macro.dwo section here.  */
+      bool save_is_dwo = lh->is_dwo;
+      lh->is_dwo = false;
+      dwarf_decode_lines_1 (lh, cu, lowpc);
+      lh->is_dwo = save_is_dwo;
+    }

   /* Make sure a symtab is created for every file, even files
      which contain only variables (i.e. no code with associated
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index 108e6f1bbae..7c0571e0dab 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -239,7 +239,8 @@ compare_locations (struct macro_source_file *file1, int line1,

   /* If the two files are not the same, find their common ancestor in
      the #inclusion tree.  */
-  if (file1 != file2)
+  if ((file1 != file2)
+      || filename_cmp(file1->filename, file2->filename) != 0)
     {
       /* If one file is deeper than the other, walk up the #inclusion
         chain until the two files are at least at the same *depth*.
@@ -267,7 +268,8 @@ compare_locations (struct macro_source_file *file1, int line1,

       /* Now both file1 and file2 are at the same depth.  Walk toward
         the root of the tree until we find where the branches meet.  */
-      while (file1 != file2)
+      while ((file1 != file2)
+            || filename_cmp(file1->filename, file2->filename) != 0)
        {
          line1 = file1->included_at_line;
          file1 = file1->included_by;
--


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf.
  2022-05-17  5:40         ` Natarajan, Kavitha
@ 2022-05-17  8:47           ` Natarajan, Kavitha
  0 siblings, 0 replies; 7+ messages in thread
From: Natarajan, Kavitha @ 2022-05-17  8:47 UTC (permalink / raw)
  To: Keith Seitz, Achra, Nitika, gdb-patches; +Cc: George, Jini Susan

[Public]

Here are the references to the previous patch threads:
https://sourceware.org/pipermail/gdb-patches/2022-March/186621.html
https://sourceware.org/pipermail/gdb-patches/2021-November/183578.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-05-17  8:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-19  6:56 [PATCH] Fix for the gdb.base/macscp.exp testcase failure with split dwarf Achra, Nitika
2021-11-19 18:55 ` Keith Seitz
2022-03-14 14:32   ` Achra, Nitika
2022-03-18 20:15     ` Keith Seitz
2022-03-18 20:23       ` Keith Seitz
2022-05-17  5:40         ` Natarajan, Kavitha
2022-05-17  8:47           ` Natarajan, Kavitha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).