[PATCH] DWARF 5 support: Handle line table and file indexes

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] DWARF 5 support: Handle line table and file indexes

Sourceware - gdb-patches mailing list
*  Fix handling of file and directory indexes in line tables; in DWARF 5 the
indexes are zero-based. Make file_names field private to abstract this detail
from the clients. Introduce a new file_names_size method. Reflect these changes
in clients.
*  Pass whether the file index is zero or one based to a few methods.
*  Handle DW_FORM_data16 in read_formatted_entries; it is used to record MD5
of the file entries in DWARF 5.
*  Fix a bug in line header parsing that calculates the length of the header
incorrectly. (Seemingly this manifests itself only in DWARF 5).
*  Introduce a new method, is_valid_file_index that takes into account whether
it is DWARF 5. Use it in related clients.

Tested with CC=/usr/bin/gcc (version 8.3.0) against master branch (also with
-gsplit-dwarf and -gdwarf-4 flags) and there was no increase in the set of
tests that fails. (gdb still cannot debug a 'hello world' program with DWARF 5,
so for the time being, this is all we care about).

This is part of an effort to support DWARF 5 in gdb.
---
 gdb/dwarf2read.c | 161 +++++++++++++++++++++++++++--------------------
 1 file changed, 94 insertions(+), 67 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 53e7393a7c..c474bdcd8b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -922,13 +922,13 @@ typedef void (die_reader_func_ftype) (const struct die_reader_specs *reader,
       int has_children,
       void *data);
 
-/* A 1-based directory index.  This is a strong typedef to prevent
-   accidentally using a directory index as a 0-based index into an
-   array/vector.  */
-enum class dir_index : unsigned int {};
+/* dir_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5 and
+   later.  */
+typedef int dir_index;
 
-/* Likewise, a 1-based file name index.  */
-enum class file_name_index : unsigned int {};
+/* file_name_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5
+   and later.  */
+typedef int file_name_index;
 
 struct file_entry
 {
@@ -980,26 +980,30 @@ struct line_header
   void add_file_name (const char *name, dir_index d_index,
       unsigned int mod_time, unsigned int length);
 
-  /* Return the include dir at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
+  /* Return the include dir 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
   {
-    /* Convert directory index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
+    size_t vec_index;
+    if (version <= 4)
+      vec_index = index - 1;
+    else
+      vec_index = index;
 
     if (vec_index >= include_dirs.size ())
       return NULL;
     return include_dirs[vec_index];
   }
 
-  /* Return the file name at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
-  file_entry *file_name_at (file_name_index index)
+  /* Return the file name 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, bool is_zero_indexed)
   {
-    /* Convert file name index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
+    size_t vec_index;
+    if (is_zero_indexed || version >= 5)
+        vec_index = index;
+    else
+      vec_index = index - 1;
 
     if (vec_index >= file_names.size ())
       return NULL;
@@ -1032,12 +1036,20 @@ struct line_header
      pointers.  The memory is owned by debug_line_buffer.  */
   std::vector<const char *> include_dirs;
 
-  /* The file_names table.  */
-  std::vector<file_entry> file_names;
+  int file_names_size () {
+    return 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 {};
+
+ private:
+  /* The file_names table. This is private because the meaning of indexes differ
+     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> file_names;
 };
 
 typedef std::unique_ptr<line_header> line_header_up;
@@ -1962,7 +1974,7 @@ static file_and_directory find_file_and_directory (struct die_info *die,
    struct dwarf2_cu *cu);
 
 static char *file_full_name (int file, struct line_header *lh,
-     const char *comp_dir);
+     const char *comp_dir, bool is_zero_indexed);
 
 /* Expected enum dwarf_unit_type for read_comp_unit_head.  */
 enum class rcuh_kind { COMPILE, TYPE };
@@ -3638,7 +3650,7 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   struct objfile *objfile = dwarf2_per_objfile->objfile;
   struct dwarf2_per_cu_data *lh_cu;
   struct attribute *attr;
-  int i;
+  int i = 0;
   void **slot;
   struct quick_file_names *qfn;
 
@@ -3697,13 +3709,13 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   if (strcmp (fnd.name, "<unknown>") != 0)
     ++offset;
 
-  qfn->num_file_names = offset + lh->file_names.size ();
+  qfn->num_file_names = offset + lh->file_names_size ();
   qfn->file_names =
     XOBNEWVEC (&objfile->objfile_obstack, const char *, qfn->num_file_names);
   if (offset != 0)
     qfn->file_names[0] = xstrdup (fnd.name);
-  for (i = 0; i < lh->file_names.size (); ++i)
-    qfn->file_names[i + offset] = file_full_name (i + 1, lh.get (), fnd.comp_dir);
+  for (i = 0; i < lh->file_names_size (); ++i)
+    qfn->file_names[i + offset] = file_full_name (i + 1, lh.get (), fnd.comp_dir, true);
   qfn->real_names = NULL;
 
   lh_cu->v.quick->file_names = qfn;
@@ -11696,16 +11708,16 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  process_full_type_unit still needs to know if this is the first
  time.  */
 
-      tu_group->num_symtabs = line_header->file_names.size ();
+      tu_group->num_symtabs = line_header->file_names_size ();
       tu_group->symtabs = XNEWVEC (struct symtab *,
-   line_header->file_names.size ());
+   line_header->file_names_size ());
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      for (i = 0; i < line_header->file_names_size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
+  file_entry *fe = line_header->file_name_at (i, true);
 
-  dwarf2_start_subfile (this, fe.name,
- fe.include_dir (line_header));
+  dwarf2_start_subfile (this, fe->name,
+ fe->include_dir (line_header));
   buildsym_compunit *b = get_builder ();
   if (b->get_current_subfile ()->symtab == NULL)
     {
@@ -11718,8 +11730,8 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  = allocate_symtab (cust, b->get_current_subfile ()->name);
     }
 
-  fe.symtab = b->get_current_subfile ()->symtab;
-  tu_group->symtabs[i] = fe.symtab;
+  fe->symtab = b->get_current_subfile ()->symtab;
+  tu_group->symtabs[i] = fe->symtab;
  }
     }
   else
@@ -11732,11 +11744,10 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  compunit_language (cust),
  0, cust));
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      for (i = 0; i < line_header->file_names_size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
-
-  fe.symtab = tu_group->symtabs[i];
+  file_entry *fe = line_header->file_name_at (i, true);
+  fe->symtab = tu_group->symtabs[i];
  }
     }
 
@@ -16209,7 +16220,7 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
     {
       /* Any related symtab will do.  */
       symtab
- = cu->line_header->file_name_at (file_name_index (1))->symtab;
+ = cu->line_header->file_name_at (0, true)->symtab;
     }
   else
     {
@@ -20277,7 +20288,7 @@ line_header::add_file_name (const char *name,
 {
   if (dwarf_line_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "Adding file %u: %s\n",
- (unsigned) file_names.size () + 1, name);
+ (unsigned) file_names_size () + 1, name);
 
   file_names.emplace_back (name, d_index, mod_time, length);
 }
@@ -20393,6 +20404,11 @@ read_formatted_entries (struct dwarf2_per_objfile *dwarf2_per_objfile,
       buf += 8;
       break;
 
+    case DW_FORM_data16:
+      /*  This is used for MD5, but file_entry does not record MD5s. */
+      buf += 16;
+      break;
+
     case DW_FORM_udata:
       uint.emplace (read_unsigned_leb128 (abfd, buf, &bytes_read));
       buf += bytes_read;
@@ -20493,12 +20509,15 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     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;
+
   if (line_ptr + lh->total_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
       return 0;
     }
-  lh->statement_program_end = line_ptr + lh->total_length;
+  lh->statement_program_end = start_here + lh->total_length;
   lh->version = read_2_bytes (abfd, line_ptr);
   line_ptr += 2;
   if (lh->version > 5)
@@ -20528,6 +20547,7 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     }
   lh->header_length = read_offset_1 (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)
@@ -20612,7 +20632,6 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
  }
       line_ptr += bytes_read;
     }
-  lh->statement_program_start = line_ptr;
 
   if (line_ptr > (section->buffer + section->size))
     complaint (_("line number info header doesn't "
@@ -20629,18 +20648,18 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
    Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename.  */
 
 static const char *
-psymtab_include_file_name (const struct line_header *lh, int file_index,
+psymtab_include_file_name (struct line_header *lh, int file_index,
    const struct partial_symtab *pst,
    const char *comp_dir,
    gdb::unique_xmalloc_ptr<char> *name_holder)
 {
-  const file_entry &fe = lh->file_names[file_index];
-  const char *include_name = fe.name;
+  file_entry *fe = lh->file_name_at (file_index, true);
+  const char *include_name = fe->name;
   const char *include_name_to_compare = include_name;
   const char *pst_filename;
   int file_is_pst;
 
-  const char *dir_name = fe.include_dir (lh);
+  const char *dir_name = fe->include_dir (lh);
 
   gdb::unique_xmalloc_ptr<char> hold_compare;
   if (!IS_ABSOLUTE_PATH (include_name)
@@ -20712,7 +20731,7 @@ public:
   {
     /* 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
@@ -20809,8 +20828,8 @@ private:
      and initialized according to the DWARF spec.  */
 
   unsigned char m_op_index = 0;
-  /* The line table index (1-based) of the current file.  */
-  file_name_index m_file = (file_name_index) 1;
+  /* The line table index of the current file.  */
+  file_name_index m_file = 1;
   unsigned int m_line = 1;
 
   /* These are initialized in the constructor.  */
@@ -21002,7 +21021,7 @@ lnp_state_machine::record_line (bool end_sequence)
       fprintf_unfiltered (gdb_stdlog,
   "Processing actual line %u: file %u,"
   " address %s, is_stmt %u, discrim %u\n",
-  m_line, to_underlying (m_file),
+  m_line, m_file,
   paddress (m_gdbarch, m_address),
   m_is_stmt, m_discriminator);
     }
@@ -21345,8 +21364,8 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
 
       /* Now that we're done scanning the Line Header Program, we can
          create the psymtab of each included file.  */
-      for (file_index = 0; file_index < lh->file_names.size (); file_index++)
-        if (lh->file_names[file_index].included_p == 1)
+      for (file_index = 0; file_index < lh->file_names_size (); file_index++)
+        if (lh->file_name_at (file_index, true)->included_p == 1)
           {
     gdb::unique_xmalloc_ptr<char> name_holder;
     const char *include_name =
@@ -21365,11 +21384,11 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
       struct compunit_symtab *cust = builder->get_compunit_symtab ();
       int i;
 
-      for (i = 0; i < lh->file_names.size (); i++)
+      for (i = 0; i < lh->file_names_size (); i++)
  {
-  file_entry &fe = lh->file_names[i];
+  file_entry *fe = lh->file_name_at (i, true);
 
-  dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh));
+  dwarf2_start_subfile (cu, fe->name, fe->include_dir (lh));
 
   if (builder->get_current_subfile ()->symtab == NULL)
     {
@@ -21377,7 +21396,7 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
  = allocate_symtab (cust,
    builder->get_current_subfile ()->name);
     }
-  fe.symtab = builder->get_current_subfile ()->symtab;
+  fe->symtab = builder->get_current_subfile ()->symtab;
  }
     }
 }
@@ -21596,7 +21615,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, false);
   else
     fe = NULL;
 
@@ -24154,22 +24173,29 @@ dwarf_alloc_die (struct dwarf2_cu *cu, int num_attrs)
    *LH's file name table.  The result is allocated using xmalloc; the caller is
    responsible for freeing it.  */
 
+bool is_valid_file_index (int file_index, struct line_header *lh,
+  bool is_zero_indexed) {
+  if (is_zero_indexed || lh->version >= 5)
+    return 0 <= file_index && file_index < lh->file_names_size ();
+  return 1 <= file_index && file_index <= lh->file_names_size ();
+}
+
 static char *
-file_file_name (int file, struct line_header *lh)
+file_file_name (int file, struct line_header *lh, bool is_zero_indexed)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (is_valid_file_index (file, lh, is_zero_indexed))
     {
-      const file_entry &fe = lh->file_names[file - 1];
+      const file_entry *fe = lh->file_name_at (file, is_zero_indexed);
 
-      if (!IS_ABSOLUTE_PATH (fe.name))
+      if (!IS_ABSOLUTE_PATH (fe->name))
  {
-  const char *dir = fe.include_dir (lh);
+  const char *dir = fe->include_dir (lh);
   if (dir != NULL)
-    return concat (dir, SLASH_STRING, fe.name, (char *) NULL);
+    return concat (dir, SLASH_STRING, fe->name, (char *) NULL);
  }
-      return xstrdup (fe.name);
+      return xstrdup (fe->name);
     }
   else
     {
@@ -24193,13 +24219,14 @@ file_file_name (int file, struct line_header *lh)
    compilation.  The result is allocated using xmalloc; the caller is
    responsible for freeing it.  */
 static char *
-file_full_name (int file, struct line_header *lh, const char *comp_dir)
+file_full_name (int file, struct line_header *lh, const char *comp_dir,
+ bool is_zero_indexed)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (is_valid_file_index (file, lh, is_zero_indexed))
     {
-      char *relative = file_file_name (file, lh);
+      char *relative = file_file_name (file, lh, is_zero_indexed);
 
       if (IS_ABSOLUTE_PATH (relative) || comp_dir == NULL)
  return relative;
@@ -24207,7 +24234,7 @@ file_full_name (int file, struct line_header *lh, const char *comp_dir)
        relative, (char *) NULL);
     }
   else
-    return file_file_name (file, lh);
+    return file_file_name (file, lh, is_zero_indexed);
 }
 
 
@@ -24218,7 +24245,7 @@ macro_start_file (struct dwarf2_cu *cu,
                   struct line_header *lh)
 {
   /* File name relative to the compilation directory of this source file.  */
-  char *file_name = file_file_name (file, lh);
+  char *file_name = file_file_name (file, lh, false);
 
   if (! current_file)
     {
--
2.23.0.444.g18eeb5a265-goog

Reply | Threaded
Open this post in threaded view
|

[PATCH] DWARF 5 support: Handle line table and file indexes

Sourceware - gdb-patches mailing list
[Sorry, I forgot to add gdb/ChangeLog, resending]

*  Fix handling of file and directory indexes in line tables; in DWARF 5 the
indexes are zero-based. Make file_names field private to abstract this detail
from the clients. Introduce a new file_names_size method. Reflect these changes
in clients.
*  Pass whether the file index is zero or one based to a few methods.
*  Handle DW_FORM_data16 in read_formatted_entries; it is used to record MD5
of the file entries in DWARF 5.
*  Fix a bug in line header parsing that calculates the length of the header
incorrectly. (Seemingly this manifests itself only in DWARF 5).
*  Introduce a new method, is_valid_file_index that takes into account whether
it is DWARF 5. Use it in related clients.

Tested with CC=/usr/bin/gcc (version 8.3.0) against master branch (also with
-gsplit-dwarf and -gdwarf-4 flags) and there was no increase in the set of
tests that fails. (gdb still cannot debug a 'hello world' program with DWARF 5,
so for the time being, this is all we care about).

This is part of an effort to support DWARF 5 in gdb.

gdb/ChangeLog:

        * dwarf2read.c (dir_index): Change type.
        (file_name_index): Likewise.
        (line_header::include_dir_at): Change comment and implementation on
        whether it is DWARF 5.
        (line_header::file_name_at): Change comment, API and implementation.
        (line_header::file_names_size): New method.
        (line_header::file_names): Change to private field.
        (file_full_name): Change API.
        (dw2_get_file_names_reader): Initialize loval variable. Use new methods.
        (dwarf2_cu::setup_type_unit_groups): Change implementation and use new
        methods.
        (process_structure_scope): Reflect API change.
        (line_header::add_file_name): Likewise.
        (read_formatted_entries): Handle DW_FORM_data16.
        (dwarf_decode_line_header): Fix line header length calculation.
        (psymtab_include_file_name): Reflect API change.
        (lnp_state_machine::current_file): Likewise.
        (lnp_state_machine::m_file): Update comment.
        (lnp_state_machine::record_line): Reflect type change.
        (dwarf_decode_lines): Reflect API change.
        (new_symbol): Likewise.
        (is_valid_file_index): New function.
        (file_file_name): Change API and implementation to take care of DWARF 5.
        (file_full_name): Likewise.
        (macro_start_file): Reflect API change.
---
 gdb/dwarf2read.c | 161 +++++++++++++++++++++++++++--------------------
 1 file changed, 94 insertions(+), 67 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 53e7393a7c..c474bdcd8b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -922,13 +922,13 @@ typedef void (die_reader_func_ftype) (const struct die_reader_specs *reader,
       int has_children,
       void *data);
 
-/* A 1-based directory index.  This is a strong typedef to prevent
-   accidentally using a directory index as a 0-based index into an
-   array/vector.  */
-enum class dir_index : unsigned int {};
+/* dir_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5 and
+   later.  */
+typedef int dir_index;
 
-/* Likewise, a 1-based file name index.  */
-enum class file_name_index : unsigned int {};
+/* file_name_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5
+   and later.  */
+typedef int file_name_index;
 
 struct file_entry
 {
@@ -980,26 +980,30 @@ struct line_header
   void add_file_name (const char *name, dir_index d_index,
       unsigned int mod_time, unsigned int length);
 
-  /* Return the include dir at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
+  /* Return the include dir 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
   {
-    /* Convert directory index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
+    size_t vec_index;
+    if (version <= 4)
+      vec_index = index - 1;
+    else
+      vec_index = index;
 
     if (vec_index >= include_dirs.size ())
       return NULL;
     return include_dirs[vec_index];
   }
 
-  /* Return the file name at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
-  file_entry *file_name_at (file_name_index index)
+  /* Return the file name 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, bool is_zero_indexed)
   {
-    /* Convert file name index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
+    size_t vec_index;
+    if (is_zero_indexed || version >= 5)
+        vec_index = index;
+    else
+      vec_index = index - 1;
 
     if (vec_index >= file_names.size ())
       return NULL;
@@ -1032,12 +1036,20 @@ struct line_header
      pointers.  The memory is owned by debug_line_buffer.  */
   std::vector<const char *> include_dirs;
 
-  /* The file_names table.  */
-  std::vector<file_entry> file_names;
+  int file_names_size () {
+    return 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 {};
+
+ private:
+  /* The file_names table. This is private because the meaning of indexes differ
+     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> file_names;
 };
 
 typedef std::unique_ptr<line_header> line_header_up;
@@ -1962,7 +1974,7 @@ static file_and_directory find_file_and_directory (struct die_info *die,
    struct dwarf2_cu *cu);
 
 static char *file_full_name (int file, struct line_header *lh,
-     const char *comp_dir);
+     const char *comp_dir, bool is_zero_indexed);
 
 /* Expected enum dwarf_unit_type for read_comp_unit_head.  */
 enum class rcuh_kind { COMPILE, TYPE };
@@ -3638,7 +3650,7 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   struct objfile *objfile = dwarf2_per_objfile->objfile;
   struct dwarf2_per_cu_data *lh_cu;
   struct attribute *attr;
-  int i;
+  int i = 0;
   void **slot;
   struct quick_file_names *qfn;
 
@@ -3697,13 +3709,13 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   if (strcmp (fnd.name, "<unknown>") != 0)
     ++offset;
 
-  qfn->num_file_names = offset + lh->file_names.size ();
+  qfn->num_file_names = offset + lh->file_names_size ();
   qfn->file_names =
     XOBNEWVEC (&objfile->objfile_obstack, const char *, qfn->num_file_names);
   if (offset != 0)
     qfn->file_names[0] = xstrdup (fnd.name);
-  for (i = 0; i < lh->file_names.size (); ++i)
-    qfn->file_names[i + offset] = file_full_name (i + 1, lh.get (), fnd.comp_dir);
+  for (i = 0; i < lh->file_names_size (); ++i)
+    qfn->file_names[i + offset] = file_full_name (i + 1, lh.get (), fnd.comp_dir, true);
   qfn->real_names = NULL;
 
   lh_cu->v.quick->file_names = qfn;
@@ -11696,16 +11708,16 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  process_full_type_unit still needs to know if this is the first
  time.  */
 
-      tu_group->num_symtabs = line_header->file_names.size ();
+      tu_group->num_symtabs = line_header->file_names_size ();
       tu_group->symtabs = XNEWVEC (struct symtab *,
-   line_header->file_names.size ());
+   line_header->file_names_size ());
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      for (i = 0; i < line_header->file_names_size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
+  file_entry *fe = line_header->file_name_at (i, true);
 
-  dwarf2_start_subfile (this, fe.name,
- fe.include_dir (line_header));
+  dwarf2_start_subfile (this, fe->name,
+ fe->include_dir (line_header));
   buildsym_compunit *b = get_builder ();
   if (b->get_current_subfile ()->symtab == NULL)
     {
@@ -11718,8 +11730,8 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  = allocate_symtab (cust, b->get_current_subfile ()->name);
     }
 
-  fe.symtab = b->get_current_subfile ()->symtab;
-  tu_group->symtabs[i] = fe.symtab;
+  fe->symtab = b->get_current_subfile ()->symtab;
+  tu_group->symtabs[i] = fe->symtab;
  }
     }
   else
@@ -11732,11 +11744,10 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  compunit_language (cust),
  0, cust));
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      for (i = 0; i < line_header->file_names_size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
-
-  fe.symtab = tu_group->symtabs[i];
+  file_entry *fe = line_header->file_name_at (i, true);
+  fe->symtab = tu_group->symtabs[i];
  }
     }
 
@@ -16209,7 +16220,7 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
     {
       /* Any related symtab will do.  */
       symtab
- = cu->line_header->file_name_at (file_name_index (1))->symtab;
+ = cu->line_header->file_name_at (0, true)->symtab;
     }
   else
     {
@@ -20277,7 +20288,7 @@ line_header::add_file_name (const char *name,
 {
   if (dwarf_line_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "Adding file %u: %s\n",
- (unsigned) file_names.size () + 1, name);
+ (unsigned) file_names_size () + 1, name);
 
   file_names.emplace_back (name, d_index, mod_time, length);
 }
@@ -20393,6 +20404,11 @@ read_formatted_entries (struct dwarf2_per_objfile *dwarf2_per_objfile,
       buf += 8;
       break;
 
+    case DW_FORM_data16:
+      /*  This is used for MD5, but file_entry does not record MD5s. */
+      buf += 16;
+      break;
+
     case DW_FORM_udata:
       uint.emplace (read_unsigned_leb128 (abfd, buf, &bytes_read));
       buf += bytes_read;
@@ -20493,12 +20509,15 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     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;
+
   if (line_ptr + lh->total_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
       return 0;
     }
-  lh->statement_program_end = line_ptr + lh->total_length;
+  lh->statement_program_end = start_here + lh->total_length;
   lh->version = read_2_bytes (abfd, line_ptr);
   line_ptr += 2;
   if (lh->version > 5)
@@ -20528,6 +20547,7 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     }
   lh->header_length = read_offset_1 (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)
@@ -20612,7 +20632,6 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
  }
       line_ptr += bytes_read;
     }
-  lh->statement_program_start = line_ptr;
 
   if (line_ptr > (section->buffer + section->size))
     complaint (_("line number info header doesn't "
@@ -20629,18 +20648,18 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
    Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename.  */
 
 static const char *
-psymtab_include_file_name (const struct line_header *lh, int file_index,
+psymtab_include_file_name (struct line_header *lh, int file_index,
    const struct partial_symtab *pst,
    const char *comp_dir,
    gdb::unique_xmalloc_ptr<char> *name_holder)
 {
-  const file_entry &fe = lh->file_names[file_index];
-  const char *include_name = fe.name;
+  file_entry *fe = lh->file_name_at (file_index, true);
+  const char *include_name = fe->name;
   const char *include_name_to_compare = include_name;
   const char *pst_filename;
   int file_is_pst;
 
-  const char *dir_name = fe.include_dir (lh);
+  const char *dir_name = fe->include_dir (lh);
 
   gdb::unique_xmalloc_ptr<char> hold_compare;
   if (!IS_ABSOLUTE_PATH (include_name)
@@ -20712,7 +20731,7 @@ public:
   {
     /* 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
@@ -20809,8 +20828,8 @@ private:
      and initialized according to the DWARF spec.  */
 
   unsigned char m_op_index = 0;
-  /* The line table index (1-based) of the current file.  */
-  file_name_index m_file = (file_name_index) 1;
+  /* The line table index of the current file.  */
+  file_name_index m_file = 1;
   unsigned int m_line = 1;
 
   /* These are initialized in the constructor.  */
@@ -21002,7 +21021,7 @@ lnp_state_machine::record_line (bool end_sequence)
       fprintf_unfiltered (gdb_stdlog,
   "Processing actual line %u: file %u,"
   " address %s, is_stmt %u, discrim %u\n",
-  m_line, to_underlying (m_file),
+  m_line, m_file,
   paddress (m_gdbarch, m_address),
   m_is_stmt, m_discriminator);
     }
@@ -21345,8 +21364,8 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
 
       /* Now that we're done scanning the Line Header Program, we can
          create the psymtab of each included file.  */
-      for (file_index = 0; file_index < lh->file_names.size (); file_index++)
-        if (lh->file_names[file_index].included_p == 1)
+      for (file_index = 0; file_index < lh->file_names_size (); file_index++)
+        if (lh->file_name_at (file_index, true)->included_p == 1)
           {
     gdb::unique_xmalloc_ptr<char> name_holder;
     const char *include_name =
@@ -21365,11 +21384,11 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
       struct compunit_symtab *cust = builder->get_compunit_symtab ();
       int i;
 
-      for (i = 0; i < lh->file_names.size (); i++)
+      for (i = 0; i < lh->file_names_size (); i++)
  {
-  file_entry &fe = lh->file_names[i];
+  file_entry *fe = lh->file_name_at (i, true);
 
-  dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh));
+  dwarf2_start_subfile (cu, fe->name, fe->include_dir (lh));
 
   if (builder->get_current_subfile ()->symtab == NULL)
     {
@@ -21377,7 +21396,7 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
  = allocate_symtab (cust,
    builder->get_current_subfile ()->name);
     }
-  fe.symtab = builder->get_current_subfile ()->symtab;
+  fe->symtab = builder->get_current_subfile ()->symtab;
  }
     }
 }
@@ -21596,7 +21615,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, false);
   else
     fe = NULL;
 
@@ -24154,22 +24173,29 @@ dwarf_alloc_die (struct dwarf2_cu *cu, int num_attrs)
    *LH's file name table.  The result is allocated using xmalloc; the caller is
    responsible for freeing it.  */
 
+bool is_valid_file_index (int file_index, struct line_header *lh,
+  bool is_zero_indexed) {
+  if (is_zero_indexed || lh->version >= 5)
+    return 0 <= file_index && file_index < lh->file_names_size ();
+  return 1 <= file_index && file_index <= lh->file_names_size ();
+}
+
 static char *
-file_file_name (int file, struct line_header *lh)
+file_file_name (int file, struct line_header *lh, bool is_zero_indexed)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (is_valid_file_index (file, lh, is_zero_indexed))
     {
-      const file_entry &fe = lh->file_names[file - 1];
+      const file_entry *fe = lh->file_name_at (file, is_zero_indexed);
 
-      if (!IS_ABSOLUTE_PATH (fe.name))
+      if (!IS_ABSOLUTE_PATH (fe->name))
  {
-  const char *dir = fe.include_dir (lh);
+  const char *dir = fe->include_dir (lh);
   if (dir != NULL)
-    return concat (dir, SLASH_STRING, fe.name, (char *) NULL);
+    return concat (dir, SLASH_STRING, fe->name, (char *) NULL);
  }
-      return xstrdup (fe.name);
+      return xstrdup (fe->name);
     }
   else
     {
@@ -24193,13 +24219,14 @@ file_file_name (int file, struct line_header *lh)
    compilation.  The result is allocated using xmalloc; the caller is
    responsible for freeing it.  */
 static char *
-file_full_name (int file, struct line_header *lh, const char *comp_dir)
+file_full_name (int file, struct line_header *lh, const char *comp_dir,
+ bool is_zero_indexed)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (is_valid_file_index (file, lh, is_zero_indexed))
     {
-      char *relative = file_file_name (file, lh);
+      char *relative = file_file_name (file, lh, is_zero_indexed);
 
       if (IS_ABSOLUTE_PATH (relative) || comp_dir == NULL)
  return relative;
@@ -24207,7 +24234,7 @@ file_full_name (int file, struct line_header *lh, const char *comp_dir)
        relative, (char *) NULL);
     }
   else
-    return file_file_name (file, lh);
+    return file_file_name (file, lh, is_zero_indexed);
 }
 
 
@@ -24218,7 +24245,7 @@ macro_start_file (struct dwarf2_cu *cu,
                   struct line_header *lh)
 {
   /* File name relative to the compilation directory of this source file.  */
-  char *file_name = file_file_name (file, lh);
+  char *file_name = file_file_name (file, lh, false);
 
   if (! current_file)
     {
--
2.23.0.444.g18eeb5a265-goog

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] DWARF 5 support: Handle line table and file indexes

Sourceware - gdb-patches mailing list
Hi. Friendly ping?

On Tue, Oct 1, 2019 at 6:52 PM Ali Tamur <[hidden email]> wrote:

>
> [Sorry, I forgot to add gdb/ChangeLog, resending]
>
> *  Fix handling of file and directory indexes in line tables; in DWARF 5 the
> indexes are zero-based. Make file_names field private to abstract this detail
> from the clients. Introduce a new file_names_size method. Reflect these changes
> in clients.
> *  Pass whether the file index is zero or one based to a few methods.
> *  Handle DW_FORM_data16 in read_formatted_entries; it is used to record MD5
> of the file entries in DWARF 5.
> *  Fix a bug in line header parsing that calculates the length of the header
> incorrectly. (Seemingly this manifests itself only in DWARF 5).
> *  Introduce a new method, is_valid_file_index that takes into account whether
> it is DWARF 5. Use it in related clients.
>
> Tested with CC=/usr/bin/gcc (version 8.3.0) against master branch (also with
> -gsplit-dwarf and -gdwarf-4 flags) and there was no increase in the set of
> tests that fails. (gdb still cannot debug a 'hello world' program with DWARF 5,
> so for the time being, this is all we care about).
>
> This is part of an effort to support DWARF 5 in gdb.
>
> gdb/ChangeLog:
>
>         * dwarf2read.c (dir_index): Change type.
>         (file_name_index): Likewise.
>         (line_header::include_dir_at): Change comment and implementation on
>         whether it is DWARF 5.
>         (line_header::file_name_at): Change comment, API and implementation.
>         (line_header::file_names_size): New method.
>         (line_header::file_names): Change to private field.
>         (file_full_name): Change API.
>         (dw2_get_file_names_reader): Initialize loval variable. Use new methods.
>         (dwarf2_cu::setup_type_unit_groups): Change implementation and use new
>         methods.
>         (process_structure_scope): Reflect API change.
>         (line_header::add_file_name): Likewise.
>         (read_formatted_entries): Handle DW_FORM_data16.
>         (dwarf_decode_line_header): Fix line header length calculation.
>         (psymtab_include_file_name): Reflect API change.
>         (lnp_state_machine::current_file): Likewise.
>         (lnp_state_machine::m_file): Update comment.
>         (lnp_state_machine::record_line): Reflect type change.
>         (dwarf_decode_lines): Reflect API change.
>         (new_symbol): Likewise.
>         (is_valid_file_index): New function.
>         (file_file_name): Change API and implementation to take care of DWARF 5.
>         (file_full_name): Likewise.
>         (macro_start_file): Reflect API change.
> ---
>  gdb/dwarf2read.c | 161 +++++++++++++++++++++++++++--------------------
>  1 file changed, 94 insertions(+), 67 deletions(-)
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 53e7393a7c..c474bdcd8b 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -922,13 +922,13 @@ typedef void (die_reader_func_ftype) (const struct die_reader_specs *reader,
>                                       int has_children,
>                                       void *data);
>
> -/* A 1-based directory index.  This is a strong typedef to prevent
> -   accidentally using a directory index as a 0-based index into an
> -   array/vector.  */
> -enum class dir_index : unsigned int {};
> +/* dir_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5 and
> +   later.  */
> +typedef int dir_index;
>
> -/* Likewise, a 1-based file name index.  */
> -enum class file_name_index : unsigned int {};
> +/* file_name_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5
> +   and later.  */
> +typedef int file_name_index;
>
>  struct file_entry
>  {
> @@ -980,26 +980,30 @@ struct line_header
>    void add_file_name (const char *name, dir_index d_index,
>                       unsigned int mod_time, unsigned int length);
>
> -  /* Return the include dir at INDEX (1-based).  Returns NULL if INDEX
> -     is out of bounds.  */
> +  /* Return the include dir 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
>    {
> -    /* Convert directory index number (1-based) to vector index
> -       (0-based).  */
> -    size_t vec_index = to_underlying (index) - 1;
> +    size_t vec_index;
> +    if (version <= 4)
> +      vec_index = index - 1;
> +    else
> +      vec_index = index;
>
>      if (vec_index >= include_dirs.size ())
>        return NULL;
>      return include_dirs[vec_index];
>    }
>
> -  /* Return the file name at INDEX (1-based).  Returns NULL if INDEX
> -     is out of bounds.  */
> -  file_entry *file_name_at (file_name_index index)
> +  /* Return the file name 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, bool is_zero_indexed)
>    {
> -    /* Convert file name index number (1-based) to vector index
> -       (0-based).  */
> -    size_t vec_index = to_underlying (index) - 1;
> +    size_t vec_index;
> +    if (is_zero_indexed || version >= 5)
> +        vec_index = index;
> +    else
> +      vec_index = index - 1;
>
>      if (vec_index >= file_names.size ())
>        return NULL;
> @@ -1032,12 +1036,20 @@ struct line_header
>       pointers.  The memory is owned by debug_line_buffer.  */
>    std::vector<const char *> include_dirs;
>
> -  /* The file_names table.  */
> -  std::vector<file_entry> file_names;
> +  int file_names_size () {
> +    return 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 {};
> +
> + private:
> +  /* The file_names table. This is private because the meaning of indexes differ
> +     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> file_names;
>  };
>
>  typedef std::unique_ptr<line_header> line_header_up;
> @@ -1962,7 +1974,7 @@ static file_and_directory find_file_and_directory (struct die_info *die,
>                                                    struct dwarf2_cu *cu);
>
>  static char *file_full_name (int file, struct line_header *lh,
> -                            const char *comp_dir);
> +                            const char *comp_dir, bool is_zero_indexed);
>
>  /* Expected enum dwarf_unit_type for read_comp_unit_head.  */
>  enum class rcuh_kind { COMPILE, TYPE };
> @@ -3638,7 +3650,7 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
>    struct objfile *objfile = dwarf2_per_objfile->objfile;
>    struct dwarf2_per_cu_data *lh_cu;
>    struct attribute *attr;
> -  int i;
> +  int i = 0;
>    void **slot;
>    struct quick_file_names *qfn;
>
> @@ -3697,13 +3709,13 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
>    if (strcmp (fnd.name, "<unknown>") != 0)
>      ++offset;
>
> -  qfn->num_file_names = offset + lh->file_names.size ();
> +  qfn->num_file_names = offset + lh->file_names_size ();
>    qfn->file_names =
>      XOBNEWVEC (&objfile->objfile_obstack, const char *, qfn->num_file_names);
>    if (offset != 0)
>      qfn->file_names[0] = xstrdup (fnd.name);
> -  for (i = 0; i < lh->file_names.size (); ++i)
> -    qfn->file_names[i + offset] = file_full_name (i + 1, lh.get (), fnd.comp_dir);
> +  for (i = 0; i < lh->file_names_size (); ++i)
> +    qfn->file_names[i + offset] = file_full_name (i + 1, lh.get (), fnd.comp_dir, true);
>    qfn->real_names = NULL;
>
>    lh_cu->v.quick->file_names = qfn;
> @@ -11696,16 +11708,16 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
>          process_full_type_unit still needs to know if this is the first
>          time.  */
>
> -      tu_group->num_symtabs = line_header->file_names.size ();
> +      tu_group->num_symtabs = line_header->file_names_size ();
>        tu_group->symtabs = XNEWVEC (struct symtab *,
> -                                  line_header->file_names.size ());
> +                                  line_header->file_names_size ());
>
> -      for (i = 0; i < line_header->file_names.size (); ++i)
> +      for (i = 0; i < line_header->file_names_size (); ++i)
>         {
> -         file_entry &fe = line_header->file_names[i];
> +         file_entry *fe = line_header->file_name_at (i, true);
>
> -         dwarf2_start_subfile (this, fe.name,
> -                               fe.include_dir (line_header));
> +         dwarf2_start_subfile (this, fe->name,
> +                               fe->include_dir (line_header));
>           buildsym_compunit *b = get_builder ();
>           if (b->get_current_subfile ()->symtab == NULL)
>             {
> @@ -11718,8 +11730,8 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
>                 = allocate_symtab (cust, b->get_current_subfile ()->name);
>             }
>
> -         fe.symtab = b->get_current_subfile ()->symtab;
> -         tu_group->symtabs[i] = fe.symtab;
> +         fe->symtab = b->get_current_subfile ()->symtab;
> +         tu_group->symtabs[i] = fe->symtab;
>         }
>      }
>    else
> @@ -11732,11 +11744,10 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
>                         compunit_language (cust),
>                         0, cust));
>
> -      for (i = 0; i < line_header->file_names.size (); ++i)
> +      for (i = 0; i < line_header->file_names_size (); ++i)
>         {
> -         file_entry &fe = line_header->file_names[i];
> -
> -         fe.symtab = tu_group->symtabs[i];
> +         file_entry *fe = line_header->file_name_at (i, true);
> +         fe->symtab = tu_group->symtabs[i];
>         }
>      }
>
> @@ -16209,7 +16220,7 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
>             {
>               /* Any related symtab will do.  */
>               symtab
> -               = cu->line_header->file_name_at (file_name_index (1))->symtab;
> +               = cu->line_header->file_name_at (0, true)->symtab;
>             }
>           else
>             {
> @@ -20277,7 +20288,7 @@ line_header::add_file_name (const char *name,
>  {
>    if (dwarf_line_debug >= 2)
>      fprintf_unfiltered (gdb_stdlog, "Adding file %u: %s\n",
> -                       (unsigned) file_names.size () + 1, name);
> +                       (unsigned) file_names_size () + 1, name);
>
>    file_names.emplace_back (name, d_index, mod_time, length);
>  }
> @@ -20393,6 +20404,11 @@ read_formatted_entries (struct dwarf2_per_objfile *dwarf2_per_objfile,
>               buf += 8;
>               break;
>
> +           case DW_FORM_data16:
> +             /*  This is used for MD5, but file_entry does not record MD5s. */
> +             buf += 16;
> +             break;
> +
>             case DW_FORM_udata:
>               uint.emplace (read_unsigned_leb128 (abfd, buf, &bytes_read));
>               buf += bytes_read;
> @@ -20493,12 +20509,15 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
>      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;
> +
>    if (line_ptr + lh->total_length > (section->buffer + section->size))
>      {
>        dwarf2_statement_list_fits_in_line_number_section_complaint ();
>        return 0;
>      }
> -  lh->statement_program_end = line_ptr + lh->total_length;
> +  lh->statement_program_end = start_here + lh->total_length;
>    lh->version = read_2_bytes (abfd, line_ptr);
>    line_ptr += 2;
>    if (lh->version > 5)
> @@ -20528,6 +20547,7 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
>      }
>    lh->header_length = read_offset_1 (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)
> @@ -20612,7 +20632,6 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
>         }
>        line_ptr += bytes_read;
>      }
> -  lh->statement_program_start = line_ptr;
>
>    if (line_ptr > (section->buffer + section->size))
>      complaint (_("line number info header doesn't "
> @@ -20629,18 +20648,18 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
>     Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename.  */
>
>  static const char *
> -psymtab_include_file_name (const struct line_header *lh, int file_index,
> +psymtab_include_file_name (struct line_header *lh, int file_index,
>                            const struct partial_symtab *pst,
>                            const char *comp_dir,
>                            gdb::unique_xmalloc_ptr<char> *name_holder)
>  {
> -  const file_entry &fe = lh->file_names[file_index];
> -  const char *include_name = fe.name;
> +  file_entry *fe = lh->file_name_at (file_index, true);
> +  const char *include_name = fe->name;
>    const char *include_name_to_compare = include_name;
>    const char *pst_filename;
>    int file_is_pst;
>
> -  const char *dir_name = fe.include_dir (lh);
> +  const char *dir_name = fe->include_dir (lh);
>
>    gdb::unique_xmalloc_ptr<char> hold_compare;
>    if (!IS_ABSOLUTE_PATH (include_name)
> @@ -20712,7 +20731,7 @@ public:
>    {
>      /* 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
> @@ -20809,8 +20828,8 @@ private:
>       and initialized according to the DWARF spec.  */
>
>    unsigned char m_op_index = 0;
> -  /* The line table index (1-based) of the current file.  */
> -  file_name_index m_file = (file_name_index) 1;
> +  /* The line table index of the current file.  */
> +  file_name_index m_file = 1;
>    unsigned int m_line = 1;
>
>    /* These are initialized in the constructor.  */
> @@ -21002,7 +21021,7 @@ lnp_state_machine::record_line (bool end_sequence)
>        fprintf_unfiltered (gdb_stdlog,
>                           "Processing actual line %u: file %u,"
>                           " address %s, is_stmt %u, discrim %u\n",
> -                         m_line, to_underlying (m_file),
> +                         m_line, m_file,
>                           paddress (m_gdbarch, m_address),
>                           m_is_stmt, m_discriminator);
>      }
> @@ -21345,8 +21364,8 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
>
>        /* Now that we're done scanning the Line Header Program, we can
>           create the psymtab of each included file.  */
> -      for (file_index = 0; file_index < lh->file_names.size (); file_index++)
> -        if (lh->file_names[file_index].included_p == 1)
> +      for (file_index = 0; file_index < lh->file_names_size (); file_index++)
> +        if (lh->file_name_at (file_index, true)->included_p == 1)
>            {
>             gdb::unique_xmalloc_ptr<char> name_holder;
>             const char *include_name =
> @@ -21365,11 +21384,11 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
>        struct compunit_symtab *cust = builder->get_compunit_symtab ();
>        int i;
>
> -      for (i = 0; i < lh->file_names.size (); i++)
> +      for (i = 0; i < lh->file_names_size (); i++)
>         {
> -         file_entry &fe = lh->file_names[i];
> +         file_entry *fe = lh->file_name_at (i, true);
>
> -         dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh));
> +         dwarf2_start_subfile (cu, fe->name, fe->include_dir (lh));
>
>           if (builder->get_current_subfile ()->symtab == NULL)
>             {
> @@ -21377,7 +21396,7 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
>                 = allocate_symtab (cust,
>                                    builder->get_current_subfile ()->name);
>             }
> -         fe.symtab = builder->get_current_subfile ()->symtab;
> +         fe->symtab = builder->get_current_subfile ()->symtab;
>         }
>      }
>  }
> @@ -21596,7 +21615,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, false);
>           else
>             fe = NULL;
>
> @@ -24154,22 +24173,29 @@ dwarf_alloc_die (struct dwarf2_cu *cu, int num_attrs)
>     *LH's file name table.  The result is allocated using xmalloc; the caller is
>     responsible for freeing it.  */
>
> +bool is_valid_file_index (int file_index, struct line_header *lh,
> +                         bool is_zero_indexed) {
> +  if (is_zero_indexed || lh->version >= 5)
> +    return 0 <= file_index && file_index < lh->file_names_size ();
> +  return 1 <= file_index && file_index <= lh->file_names_size ();
> +}
> +
>  static char *
> -file_file_name (int file, struct line_header *lh)
> +file_file_name (int file, struct line_header *lh, bool is_zero_indexed)
>  {
>    /* 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 (1 <= file && file <= lh->file_names.size ())
> +  if (is_valid_file_index (file, lh, is_zero_indexed))
>      {
> -      const file_entry &fe = lh->file_names[file - 1];
> +      const file_entry *fe = lh->file_name_at (file, is_zero_indexed);
>
> -      if (!IS_ABSOLUTE_PATH (fe.name))
> +      if (!IS_ABSOLUTE_PATH (fe->name))
>         {
> -         const char *dir = fe.include_dir (lh);
> +         const char *dir = fe->include_dir (lh);
>           if (dir != NULL)
> -           return concat (dir, SLASH_STRING, fe.name, (char *) NULL);
> +           return concat (dir, SLASH_STRING, fe->name, (char *) NULL);
>         }
> -      return xstrdup (fe.name);
> +      return xstrdup (fe->name);
>      }
>    else
>      {
> @@ -24193,13 +24219,14 @@ file_file_name (int file, struct line_header *lh)
>     compilation.  The result is allocated using xmalloc; the caller is
>     responsible for freeing it.  */
>  static char *
> -file_full_name (int file, struct line_header *lh, const char *comp_dir)
> +file_full_name (int file, struct line_header *lh, const char *comp_dir,
> +               bool is_zero_indexed)
>  {
>    /* 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 (1 <= file && file <= lh->file_names.size ())
> +  if (is_valid_file_index (file, lh, is_zero_indexed))
>      {
> -      char *relative = file_file_name (file, lh);
> +      char *relative = file_file_name (file, lh, is_zero_indexed);
>
>        if (IS_ABSOLUTE_PATH (relative) || comp_dir == NULL)
>         return relative;
> @@ -24207,7 +24234,7 @@ file_full_name (int file, struct line_header *lh, const char *comp_dir)
>                        relative, (char *) NULL);
>      }
>    else
> -    return file_file_name (file, lh);
> +    return file_file_name (file, lh, is_zero_indexed);
>  }
>
>
> @@ -24218,7 +24245,7 @@ macro_start_file (struct dwarf2_cu *cu,
>                    struct line_header *lh)
>  {
>    /* File name relative to the compilation directory of this source file.  */
> -  char *file_name = file_file_name (file, lh);
> +  char *file_name = file_file_name (file, lh, false);
>
>    if (! current_file)
>      {
> --
> 2.23.0.444.g18eeb5a265-goog
>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] DWARF 5 support: Handle line table and file indexes

Simon Marchi
In reply to this post by Sourceware - gdb-patches mailing list
Hi Ali,

Thanks for the patch.  I noted a few minor comments below.

On 2019-10-01 21:52, Ali Tamur via gdb-patches wrote:
> [Sorry, I forgot to add gdb/ChangeLog, resending]
>
> *  Fix handling of file and directory indexes in line tables; in DWARF
> 5 the
> indexes are zero-based. Make file_names field private to abstract this
> detail
> from the clients. Introduce a new file_names_size method. Reflect these
> changes
> in clients.

I think it's a good idea to encapsulate this.  Should include_dirs be
private as well, since it is in the same situation?

> *  Pass whether the file index is zero or one based to a few methods.

Hmm, I am not a big fan of that, I had a hard time understanding why
that was necessary. From what I understand now, it's only necessary so
that some code that is agnostic to DWARF 4 vs 5 can iterate on all file
entries.

Instead, maybe we could have the line_header object provide a way for
clients to iterate on file entries, without exposing the vector.  Maybe
a range adapter that would let clients do:

for (const file_entry &fe : lh->file_names ())
   {
     ...
   }

Hopefully, that is enough for all cases that pass is_zero_based = true
in your patch.

> *  Handle DW_FORM_data16 in read_formatted_entries; it is used to
> record MD5
> of the file entries in DWARF 5.
> *  Fix a bug in line header parsing that calculates the length of the
> header
> incorrectly. (Seemingly this manifests itself only in DWARF 5).
> *  Introduce a new method, is_valid_file_index that takes into account
> whether
> it is DWARF 5. Use it in related clients.
>
> Tested with CC=/usr/bin/gcc (version 8.3.0) against master branch (also
> with
> -gsplit-dwarf and -gdwarf-4 flags) and there was no increase in the set
> of
> tests that fails. (gdb still cannot debug a 'hello world' program with
> DWARF 5,
> so for the time being, this is all we care about).
>
> This is part of an effort to support DWARF 5 in gdb.
>
> gdb/ChangeLog:
>
> * dwarf2read.c (dir_index): Change type.
> (file_name_index): Likewise.
> (line_header::include_dir_at): Change comment and implementation on
> whether it is DWARF 5.
> (line_header::file_name_at): Change comment, API and implementation.
> (line_header::file_names_size): New method.
> (line_header::file_names): Change to private field.
> (file_full_name): Change API.
> (dw2_get_file_names_reader): Initialize loval variable. Use new
> methods.
> (dwarf2_cu::setup_type_unit_groups): Change implementation and use new
> methods.
> (process_structure_scope): Reflect API change.
> (line_header::add_file_name): Likewise.
> (read_formatted_entries): Handle DW_FORM_data16.
> (dwarf_decode_line_header): Fix line header length calculation.
> (psymtab_include_file_name): Reflect API change.
> (lnp_state_machine::current_file): Likewise.
> (lnp_state_machine::m_file): Update comment.
> (lnp_state_machine::record_line): Reflect type change.
> (dwarf_decode_lines): Reflect API change.
> (new_symbol): Likewise.
> (is_valid_file_index): New function.
> (file_file_name): Change API and implementation to take care of DWARF
> 5.
> (file_full_name): Likewise.
> (macro_start_file): Reflect API change.
> ---
>  gdb/dwarf2read.c | 161 +++++++++++++++++++++++++++--------------------
>  1 file changed, 94 insertions(+), 67 deletions(-)
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 53e7393a7c..c474bdcd8b 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -922,13 +922,13 @@ typedef void (die_reader_func_ftype) (const
> struct die_reader_specs *reader,
>        int has_children,
>        void *data);
>
> -/* A 1-based directory index.  This is a strong typedef to prevent
> -   accidentally using a directory index as a 0-based index into an
> -   array/vector.  */
> -enum class dir_index : unsigned int {};
> +/* dir_index is 1-based in DWARF 4 and before, and is 0-based in DWARF
> 5 and
> +   later.  */
> +typedef int dir_index;
>
> -/* Likewise, a 1-based file name index.  */
> -enum class file_name_index : unsigned int {};
> +/* file_name_index is 1-based in DWARF 4 and before, and is 0-based in
> DWARF 5
> +   and later.  */
> +typedef int file_name_index;

Is there a reason you changed these?  The reason why these were of enum
class type (or "strong typedef") still seems valid to me.  We don't want
some code accessing the vectors blindy using these indices, so it
encourages people to use the right accessor method instead.

>
>  struct file_entry
>  {
> @@ -980,26 +980,30 @@ struct line_header
>    void add_file_name (const char *name, dir_index d_index,
>        unsigned int mod_time, unsigned int length);
>
> -  /* Return the include dir at INDEX (1-based).  Returns NULL if INDEX
> -     is out of bounds.  */
> +  /* Return the include dir 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
>    {
> -    /* Convert directory index number (1-based) to vector index
> -       (0-based).  */
> -    size_t vec_index = to_underlying (index) - 1;
> +    size_t vec_index;
> +    if (version <= 4)
> +      vec_index = index - 1;
> +    else
> +      vec_index = index;
>
>      if (vec_index >= include_dirs.size ())
>        return NULL;
>      return include_dirs[vec_index];
>    }
>
> -  /* Return the file name at INDEX (1-based).  Returns NULL if INDEX
> -     is out of bounds.  */
> -  file_entry *file_name_at (file_name_index index)
> +  /* Return the file name 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, bool
> is_zero_indexed)
>    {
> -    /* Convert file name index number (1-based) to vector index
> -       (0-based).  */
> -    size_t vec_index = to_underlying (index) - 1;
> +    size_t vec_index;
> +    if (is_zero_indexed || version >= 5)
> +        vec_index = index;

Remove one indentation level.

> +    else
> +      vec_index = index - 1;
>
>      if (vec_index >= file_names.size ())
>        return NULL;
> @@ -1032,12 +1036,20 @@ struct line_header
>       pointers.  The memory is owned by debug_line_buffer.  */
>    std::vector<const char *> include_dirs;
>
> -  /* The file_names table.  */
> -  std::vector<file_entry> file_names;
> +  int file_names_size () {
> +    return file_names.size();
> +  }

Format it like this:

   int file_names_size ()
   {
     return file_names.size();
   }

or we also accept this for short/trivial methods:

   int file_names_size ()
   { return 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
> {};
> +
> + private:
> +  /* The file_names table. This is private because the meaning of
> indexes differ

differ -> 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

Two spaces after period.

> +     method for access.  */
> +  std::vector<file_entry> file_names;

You can rename this to m_file_names (that's the style we use for private
members).

> @@ -3638,7 +3650,7 @@ dw2_get_file_names_reader (const struct
> die_reader_specs *reader,
>    struct objfile *objfile = dwarf2_per_objfile->objfile;
>    struct dwarf2_per_cu_data *lh_cu;
>    struct attribute *attr;
> -  int i;
> +  int i = 0;

Unnecessary change?  In fact, I'd take the opportunity to move that
declaration in the for loop that uses it.

> @@ -20629,18 +20648,18 @@ dwarf_decode_line_header (sect_offset
> sect_off, struct dwarf2_cu *cu)
>     Returns NULL if FILE_INDEX should be ignored, i.e., it is
> pst->filename.  */
>
>  static const char *
> -psymtab_include_file_name (const struct line_header *lh, int
> file_index,
> +psymtab_include_file_name (struct line_header *lh, int file_index,
>     const struct partial_symtab *pst,
>     const char *comp_dir,
>     gdb::unique_xmalloc_ptr<char> *name_holder)
>  {
> -  const file_entry &fe = lh->file_names[file_index];
> -  const char *include_name = fe.name;
> +  file_entry *fe = lh->file_name_at (file_index, true);

Hmm it's not great to have to lose the const here.  Can you try adding a
const version of file_name_at, that returns a const file_entry *?

> @@ -24154,22 +24173,29 @@ dwarf_alloc_die (struct dwarf2_cu *cu, int
> num_attrs)
>     *LH's file name table.  The result is allocated using xmalloc; the
> caller is
>     responsible for freeing it.  */

The comment above applies to the `file_file_name` function, so it should
stay right on top of it.  Also, can you please document the new
IS_ZERO_INDEXED parameter?

>
> +bool is_valid_file_index (int file_index, struct line_header *lh,

This function should be static.  Also, place the return type on its own
line:

static bool
is_valid_file_index (...

Simon
Reply | Threaded
Open this post in threaded view
|

[PATCH] DWARF 5 support: Handle line table and file indexes

Sourceware - gdb-patches mailing list
Hi Simon,

Thank you for the review. I did all the changes you asked except for one,
which I explain below. Please take another look.

Ali

> > Make file_names field private to abstract this detail from the clients.
>
> I think it's a good idea to encapsulate this.  Should include_dirs be
> private as well, since it is in the same situation?
Done.

> > *  Pass whether the file index is zero or one based to a few methods.
>
> Hmm, I am not a big fan of that, ..
I got rid of is_zero_indexed parameter and introduced a file_names method.

> > -enum class file_name_index : unsigned int {};
> > +typedef int file_name_index;
>
> Is there a reason you changed these?  The reason why these were of enum
> class type (or "strong typedef") still seems valid to me.  We don't want
> some code accessing the vectors blindy using these indices, so it
> encourages people to use the right accessor method instead.
I respectfully disagree. We do "index - 1" calculation in file_name_at and
include_dir_at methods. If we are going to cast back and forth to integers
anyways, having an enum loses its appeal.

> Remove one indentation level.
> Format it like this:
> differ -> differs
> Two spaces after period.
> Unnecessary change?
> Hmm it's not great to have to lose the const here.
> This function should be static.
Done.
---

*  Fix handling of file and directory indexes in line tables; in DWARF 5 the
indexes are zero-based. Make file_names field private to abstract this detail
from the clients. Introduce file_names and file_names_size methods. Reflect
these changes in clients.
*  Handle DW_FORM_data16 in read_formatted_entries; it is used to record MD5
of the file entries in DWARF 5.
*  Fix a bug in line header parsing that calculates the length of the header
incorrectly. (Seemingly this manifests itself only in DWARF 5).
*  Introduce a new method, is_valid_file_index that takes into account whether
it is DWARF 5. Use it in related clients.

Tested with CC=/usr/bin/gcc (version 8.3.0) against master branch (also with
-gsplit-dwarf and -gdwarf-4 flags) and there was no increase in the set of
tests that fails. (gdb still cannot debug a 'hello world' program with DWARF 5,
so for the time being, this is all we care about).

This is part of an effort to support DWARF 5 in gdb.
---
 gdb/dwarf2read.c | 174 ++++++++++++++++++++++++++++-------------------
 1 file changed, 104 insertions(+), 70 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index ee9df34ed2..8e272b1da3 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -922,13 +922,13 @@ typedef void (die_reader_func_ftype) (const struct die_reader_specs *reader,
       int has_children,
       void *data);
 
-/* A 1-based directory index.  This is a strong typedef to prevent
-   accidentally using a directory index as a 0-based index into an
-   array/vector.  */
-enum class dir_index : unsigned int {};
+/* dir_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5 and
+   later.  */
+typedef int dir_index;
 
-/* Likewise, a 1-based file name index.  */
-enum class file_name_index : unsigned int {};
+/* file_name_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5
+   and later.  */
+typedef int file_name_index;
 
 struct file_entry
 {
@@ -980,32 +980,40 @@ struct line_header
   void add_file_name (const char *name, dir_index d_index,
       unsigned int mod_time, unsigned int length);
 
-  /* Return the include dir at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
+  /* Return the include dir 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
   {
-    /* Convert directory index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
-
-    if (vec_index >= include_dirs.size ())
+    size_t vec_index;
+    if (version >= 5)
+      vec_index = index;
+    else
+      vec_index = index - 1;
+    if (vec_index >= m_include_dirs.size ())
       return NULL;
-    return include_dirs[vec_index];
+    return m_include_dirs[vec_index];
   }
 
-  /* Return the file name at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
+  /* Return the file name 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)
   {
-    /* Convert file name index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
-
-    if (vec_index >= file_names.size ())
+    size_t vec_index;
+    if (version >= 5)
+      vec_index = index;
+    else
+      vec_index = index - 1;
+    if (vec_index >= m_file_names.size ())
       return NULL;
-    return &file_names[vec_index];
+    return &m_file_names[vec_index];
   }
 
+  /* The indexes are 0-based in DWARF 5 and 1-based in DWARF 4. Therefore,
+     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; }
+
   /* Offset of line number information in .debug_line section.  */
   sect_offset sect_off {};
 
@@ -1032,12 +1040,23 @@ struct line_header
      pointers.  The memory is owned by debug_line_buffer.  */
   std::vector<const char *> include_dirs;
 
-  /* The file_names table.  */
-  std::vector<file_entry> file_names;
+  int file_names_size ()
+  { return m_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 {};
+
+ 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 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;
 };
 
 typedef std::unique_ptr<line_header> line_header_up;
@@ -3644,7 +3663,6 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   struct objfile *objfile = dwarf2_per_objfile->objfile;
   struct dwarf2_per_cu_data *lh_cu;
   struct attribute *attr;
-  int i;
   void **slot;
   struct quick_file_names *qfn;
 
@@ -3703,12 +3721,12 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   if (strcmp (fnd.name, "<unknown>") != 0)
     ++offset;
 
-  qfn->num_file_names = offset + lh->file_names.size ();
+  qfn->num_file_names = offset + lh->file_names_size ();
   qfn->file_names =
     XOBNEWVEC (&objfile->objfile_obstack, const char *, qfn->num_file_names);
   if (offset != 0)
     qfn->file_names[0] = xstrdup (fnd.name);
-  for (i = 0; i < lh->file_names.size (); ++i)
+  for (int i = 0; i < lh->file_names_size (); ++i)
     qfn->file_names[i + offset] = file_full_name (i + 1, lh.get (), fnd.comp_dir);
   qfn->real_names = NULL;
 
@@ -11717,16 +11735,16 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  process_full_type_unit still needs to know if this is the first
  time.  */
 
-      tu_group->num_symtabs = line_header->file_names.size ();
+      tu_group->num_symtabs = line_header->file_names_size ();
       tu_group->symtabs = XNEWVEC (struct symtab *,
-   line_header->file_names.size ());
+   line_header->file_names_size ());
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      auto &file_names = line_header->file_names ();
+      for (i = 0; i < file_names.size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
-
-  dwarf2_start_subfile (this, fe.name,
- fe.include_dir (line_header));
+  file_entry *fe = &file_names[i];
+  dwarf2_start_subfile (this, fe->name,
+ fe->include_dir (line_header));
   buildsym_compunit *b = get_builder ();
   if (b->get_current_subfile ()->symtab == NULL)
     {
@@ -11739,8 +11757,8 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  = allocate_symtab (cust, b->get_current_subfile ()->name);
     }
 
-  fe.symtab = b->get_current_subfile ()->symtab;
-  tu_group->symtabs[i] = fe.symtab;
+  fe->symtab = b->get_current_subfile ()->symtab;
+  tu_group->symtabs[i] = fe->symtab;
  }
     }
   else
@@ -11753,11 +11771,11 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  compunit_language (cust),
  0, cust));
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      auto &file_names = line_header->file_names ();
+      for (i = 0; i < file_names.size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
-
-  fe.symtab = tu_group->symtabs[i];
+  file_entry *fe = &file_names[i];
+  fe->symtab = tu_group->symtabs[i];
  }
     }
 
@@ -16230,7 +16248,7 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
     {
       /* Any related symtab will do.  */
       symtab
- = cu->line_header->file_name_at (file_name_index (1))->symtab;
+ = cu->line_header->file_names ()[0].symtab;
     }
   else
     {
@@ -20286,9 +20304,9 @@ line_header::add_include_dir (const char *include_dir)
 {
   if (dwarf_line_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "Adding dir %zu: %s\n",
- include_dirs.size () + 1, include_dir);
+ m_include_dirs.size () + 1, include_dir);
 
-  include_dirs.push_back (include_dir);
+  m_include_dirs.push_back (include_dir);
 }
 
 void
@@ -20299,9 +20317,9 @@ line_header::add_file_name (const char *name,
 {
   if (dwarf_line_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "Adding file %u: %s\n",
- (unsigned) file_names.size () + 1, name);
+ (unsigned) file_names_size () + 1, name);
 
-  file_names.emplace_back (name, d_index, mod_time, length);
+  m_file_names.emplace_back (name, d_index, mod_time, length);
 }
 
 /* A convenience function to find the proper .debug_line section for a CU.  */
@@ -20415,6 +20433,11 @@ read_formatted_entries (struct dwarf2_per_objfile *dwarf2_per_objfile,
       buf += 8;
       break;
 
+    case DW_FORM_data16:
+      /*  This is used for MD5, but file_entry does not record MD5s. */
+      buf += 16;
+      break;
+
     case DW_FORM_udata:
       uint.emplace (read_unsigned_leb128 (abfd, buf, &bytes_read));
       buf += bytes_read;
@@ -20515,12 +20538,15 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     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;
+
   if (line_ptr + lh->total_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
       return 0;
     }
-  lh->statement_program_end = line_ptr + lh->total_length;
+  lh->statement_program_end = start_here + lh->total_length;
   lh->version = read_2_bytes (abfd, line_ptr);
   line_ptr += 2;
   if (lh->version > 5)
@@ -20550,6 +20576,7 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     }
   lh->header_length = read_offset_1 (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)
@@ -20634,7 +20661,6 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
  }
       line_ptr += bytes_read;
     }
-  lh->statement_program_start = line_ptr;
 
   if (line_ptr > (section->buffer + section->size))
     complaint (_("line number info header doesn't "
@@ -20651,18 +20677,17 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
    Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename.  */
 
 static const char *
-psymtab_include_file_name (const struct line_header *lh, int file_index,
+psymtab_include_file_name (const struct line_header *lh, const file_entry *fe,
    const struct partial_symtab *pst,
    const char *comp_dir,
    gdb::unique_xmalloc_ptr<char> *name_holder)
 {
-  const file_entry &fe = lh->file_names[file_index];
-  const char *include_name = fe.name;
+  const char *include_name = fe->name;
   const char *include_name_to_compare = include_name;
   const char *pst_filename;
   int file_is_pst;
 
-  const char *dir_name = fe.include_dir (lh);
+  const char *dir_name = fe->include_dir (lh);
 
   gdb::unique_xmalloc_ptr<char> hold_compare;
   if (!IS_ABSOLUTE_PATH (include_name)
@@ -20831,8 +20856,8 @@ private:
      and initialized according to the DWARF spec.  */
 
   unsigned char m_op_index = 0;
-  /* The line table index (1-based) of the current file.  */
-  file_name_index m_file = (file_name_index) 1;
+  /* The line table index of the current file.  */
+  file_name_index m_file = 1;
   unsigned int m_line = 1;
 
   /* These are initialized in the constructor.  */
@@ -21024,7 +21049,7 @@ lnp_state_machine::record_line (bool end_sequence)
       fprintf_unfiltered (gdb_stdlog,
   "Processing actual line %u: file %u,"
   " address %s, is_stmt %u, discrim %u\n",
-  m_line, to_underlying (m_file),
+  m_line, m_file,
   paddress (m_gdbarch, m_address),
   m_is_stmt, m_discriminator);
     }
@@ -21367,13 +21392,14 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
 
       /* Now that we're done scanning the Line Header Program, we can
          create the psymtab of each included file.  */
-      for (file_index = 0; file_index < lh->file_names.size (); file_index++)
-        if (lh->file_names[file_index].included_p == 1)
+      auto &file_names = lh->file_names ();
+      for (file_index = 0; file_index < file_names.size (); file_index++)
+        if (file_names[file_index].included_p == 1)
           {
     gdb::unique_xmalloc_ptr<char> name_holder;
     const char *include_name =
-      psymtab_include_file_name (lh, file_index, pst, comp_dir,
- &name_holder);
+      psymtab_include_file_name (lh, &file_names[file_index], pst,
+ comp_dir, &name_holder);
     if (include_name != NULL)
               dwarf2_create_include_psymtab (include_name, pst, objfile);
           }
@@ -21385,13 +21411,13 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
  line numbers).  */
       buildsym_compunit *builder = cu->get_builder ();
       struct compunit_symtab *cust = builder->get_compunit_symtab ();
-      int i;
 
-      for (i = 0; i < lh->file_names.size (); i++)
+      auto &file_names = lh->file_names ();
+      for (int i = 0; i < file_names.size (); i++)
  {
-  file_entry &fe = lh->file_names[i];
+  file_entry *fe = &file_names[i];
 
-  dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh));
+  dwarf2_start_subfile (cu, fe->name, fe->include_dir (lh));
 
   if (builder->get_current_subfile ()->symtab == NULL)
     {
@@ -21399,7 +21425,7 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
  = allocate_symtab (cust,
    builder->get_current_subfile ()->name);
     }
-  fe.symtab = builder->get_current_subfile ()->symtab;
+  fe->symtab = builder->get_current_subfile ()->symtab;
  }
     }
 }
@@ -24190,6 +24216,14 @@ dwarf_alloc_die (struct dwarf2_cu *cu, int num_attrs)
 
 /* Macro support.  */
 
+static bool
+is_valid_file_index (int file_index, struct line_header *lh)
+{
+  if (lh->version >= 5)
+    return 0 <= file_index && file_index < lh->file_names_size ();
+  return 1 <= file_index && file_index <= lh->file_names_size ();
+}
+
 /* Return file name relative to the compilation directory of file number I in
    *LH's file name table.  The result is allocated using xmalloc; the caller is
    responsible for freeing it.  */
@@ -24199,17 +24233,17 @@ file_file_name (int file, struct line_header *lh)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (is_valid_file_index (file, lh))
     {
-      const file_entry &fe = lh->file_names[file - 1];
+      const file_entry *fe = lh->file_name_at (file);
 
-      if (!IS_ABSOLUTE_PATH (fe.name))
+      if (!IS_ABSOLUTE_PATH (fe->name))
  {
-  const char *dir = fe.include_dir (lh);
+  const char *dir = fe->include_dir (lh);
   if (dir != NULL)
-    return concat (dir, SLASH_STRING, fe.name, (char *) NULL);
+    return concat (dir, SLASH_STRING, fe->name, (char *) NULL);
  }
-      return xstrdup (fe.name);
+      return xstrdup (fe->name);
     }
   else
     {
@@ -24237,7 +24271,7 @@ file_full_name (int file, struct line_header *lh, const char *comp_dir)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (is_valid_file_index (file, lh))
     {
       char *relative = file_file_name (file, lh);
 
--
2.23.0.700.g56cf767bdb-goog

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] DWARF 5 support: Handle line table and file indexes

Simon Marchi
On 2019-10-15 16:19, Ali Tamur wrote:

>> > -enum class file_name_index : unsigned int {};
>> > +typedef int file_name_index;
>>
>> Is there a reason you changed these?  The reason why these were of
>> enum
>> class type (or "strong typedef") still seems valid to me.  We don't
>> want
>> some code accessing the vectors blindy using these indices, so it
>> encourages people to use the right accessor method instead.
> I respectfully disagree. We do "index - 1" calculation in file_name_at
> and
> include_dir_at methods. If we are going to cast back and forth to
> integers
> anyways, having an enum loses its appeal.

Ok, I guess that if we access the vector through these accessor
functions, it should be safe.

I'd still suggest adding a gdb_assert to verify that in the case of
DWARF <= 4, index should be > 0.

> @@ -11717,16 +11735,16 @@ dwarf2_cu::setup_type_unit_groups (struct
> die_info *die)
>   process_full_type_unit still needs to know if this is the first
>   time.  */
>
> -      tu_group->num_symtabs = line_header->file_names.size ();
> +      tu_group->num_symtabs = line_header->file_names_size ();
>        tu_group->symtabs = XNEWVEC (struct symtab *,
> -   line_header->file_names.size ());
> +   line_header->file_names_size ());
>
> -      for (i = 0; i < line_header->file_names.size (); ++i)
> +      auto &file_names = line_header->file_names ();
> +      for (i = 0; i < file_names.size (); ++i)
>   {
> -  file_entry &fe = line_header->file_names[i];
> -
> -  dwarf2_start_subfile (this, fe.name,
> - fe.include_dir (line_header));
> +  file_entry *fe = &file_names[i];

In these various loops, the `fe` variable can stay a reference, unless
there's a specific reason why you changed them to pointers.

> @@ -20286,9 +20304,9 @@ line_header::add_include_dir (const char
> *include_dir)
>  {
>    if (dwarf_line_debug >= 2)
>      fprintf_unfiltered (gdb_stdlog, "Adding dir %zu: %s\n",
> - include_dirs.size () + 1, include_dir);
> + m_include_dirs.size () + 1, include_dir);

I think this debug message showing the index should be 0 based in
DWARF5, and 1 based before.

>
> -  include_dirs.push_back (include_dir);
> +  m_include_dirs.push_back (include_dir);
>  }
>
>  void
> @@ -20299,9 +20317,9 @@ line_header::add_file_name (const char *name,
>  {
>    if (dwarf_line_debug >= 2)
>      fprintf_unfiltered (gdb_stdlog, "Adding file %u: %s\n",
> - (unsigned) file_names.size () + 1, name);
> + (unsigned) file_names_size () + 1, name);

Likewise.

> @@ -20651,18 +20677,17 @@ dwarf_decode_line_header (sect_offset
> sect_off, struct dwarf2_cu *cu)
>     Returns NULL if FILE_INDEX should be ignored, i.e., it is
> pst->filename.  */
>
>  static const char *
> -psymtab_include_file_name (const struct line_header *lh, int
> file_index,
> +psymtab_include_file_name (const struct line_header *lh, const
> file_entry *fe,

Pass `fe` by const reference instead?

> @@ -21367,13 +21392,14 @@ dwarf_decode_lines (struct line_header *lh,
> const char *comp_dir,
>
>        /* Now that we're done scanning the Line Header Program, we can
>           create the psymtab of each included file.  */
> -      for (file_index = 0; file_index < lh->file_names.size ();
> file_index++)
> -        if (lh->file_names[file_index].included_p == 1)
> +      auto &file_names = lh->file_names ();
> +      for (file_index = 0; file_index < file_names.size ();
> file_index++)
> +        if (file_names[file_index].included_p == 1)

Whenever you can iterate using a range-for (when you don't need the
iteration index), please do so:

       for (const file_entry &fe : lh->file_names ())
         {
           ...
         }

Simon
Reply | Threaded
Open this post in threaded view
|

[PATCH] DWARF 5 support: Handle line table and file indexes

Sourceware - gdb-patches mailing list
I've done everything asked, except this bit:
> I'd still suggest adding a gdb_assert to verify that in the case of
DWARF <= 4, index should be > 0.

This caused failure in multiple tests, I think the correct behaviour is to
return nullptr when DWARF <= 4 and index = 0 (done).
---
*  Fix handling of file and directory indexes in line tables; in DWARF 5 the
indexes are zero-based. Make file_names field private to abstract this detail
from the clients. Introduce file_names and file_names_size methods. Reflect
these changes in clients.
*  Handle DW_FORM_data16 in read_formatted_entries; it is used to record MD5
of the file entries in DWARF 5.
*  Fix a bug in line header parsing that calculates the length of the header
incorrectly. (Seemingly this manifests itself only in DWARF 5).
*  Introduce a new method, is_valid_file_index that takes into account whether
it is DWARF 5. Use it in related clients.

Tested with CC=/usr/bin/gcc (version 8.3.0) against master branch (also with
-gsplit-dwarf and -gdwarf-4 flags) and there was no increase in the set of
tests that fails. (gdb still cannot debug a 'hello world' program with DWARF 5,
so for the time being, this is all we care about).

This is part of an effort to support DWARF 5 in gdb.
---
 gdb/dwarf2read.c | 177 +++++++++++++++++++++++++++++------------------
 1 file changed, 108 insertions(+), 69 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index ee9df34ed2..fd241bf780 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -922,13 +922,13 @@ typedef void (die_reader_func_ftype) (const struct die_reader_specs *reader,
       int has_children,
       void *data);
 
-/* A 1-based directory index.  This is a strong typedef to prevent
-   accidentally using a directory index as a 0-based index into an
-   array/vector.  */
-enum class dir_index : unsigned int {};
+/* dir_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5 and
+   later.  */
+typedef int dir_index;
 
-/* Likewise, a 1-based file name index.  */
-enum class file_name_index : unsigned int {};
+/* file_name_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5
+   and later.  */
+typedef int file_name_index;
 
 struct file_entry
 {
@@ -980,32 +980,40 @@ struct line_header
   void add_file_name (const char *name, dir_index d_index,
       unsigned int mod_time, unsigned int length);
 
-  /* Return the include dir at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
+  /* Return the include dir 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
   {
-    /* Convert directory index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
-
-    if (vec_index >= include_dirs.size ())
+    int vec_index;
+    if (version >= 5)
+      vec_index = index;
+    else
+      vec_index = index - 1;
+    if (vec_index < 0 || vec_index >= m_include_dirs.size ())
       return NULL;
-    return include_dirs[vec_index];
+    return m_include_dirs[vec_index];
   }
 
-  /* Return the file name at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
+  /* Return the file name 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)
   {
-    /* Convert file name index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
-
-    if (vec_index >= file_names.size ())
+    int vec_index;
+    if (version >= 5)
+      vec_index = index;
+    else
+      vec_index = index - 1;
+    if (vec_index < 0 || vec_index >= m_file_names.size ())
       return NULL;
-    return &file_names[vec_index];
+    return &m_file_names[vec_index];
   }
 
+  /* The indexes are 0-based in DWARF 5 and 1-based in DWARF 4. Therefore,
+     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; }
+
   /* Offset of line number information in .debug_line section.  */
   sect_offset sect_off {};
 
@@ -1032,12 +1040,23 @@ struct line_header
      pointers.  The memory is owned by debug_line_buffer.  */
   std::vector<const char *> include_dirs;
 
-  /* The file_names table.  */
-  std::vector<file_entry> file_names;
+  int file_names_size ()
+  { return m_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 {};
+
+ 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 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;
 };
 
 typedef std::unique_ptr<line_header> line_header_up;
@@ -3644,7 +3663,6 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   struct objfile *objfile = dwarf2_per_objfile->objfile;
   struct dwarf2_per_cu_data *lh_cu;
   struct attribute *attr;
-  int i;
   void **slot;
   struct quick_file_names *qfn;
 
@@ -3703,12 +3721,12 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   if (strcmp (fnd.name, "<unknown>") != 0)
     ++offset;
 
-  qfn->num_file_names = offset + lh->file_names.size ();
+  qfn->num_file_names = offset + lh->file_names_size ();
   qfn->file_names =
     XOBNEWVEC (&objfile->objfile_obstack, const char *, qfn->num_file_names);
   if (offset != 0)
     qfn->file_names[0] = xstrdup (fnd.name);
-  for (i = 0; i < lh->file_names.size (); ++i)
+  for (int i = 0; i < lh->file_names_size (); ++i)
     qfn->file_names[i + offset] = file_full_name (i + 1, lh.get (), fnd.comp_dir);
   qfn->real_names = NULL;
 
@@ -11717,14 +11735,14 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  process_full_type_unit still needs to know if this is the first
  time.  */
 
-      tu_group->num_symtabs = line_header->file_names.size ();
+      tu_group->num_symtabs = line_header->file_names_size ();
       tu_group->symtabs = XNEWVEC (struct symtab *,
-   line_header->file_names.size ());
+   line_header->file_names_size ());
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      auto &file_names = line_header->file_names ();
+      for (i = 0; i < file_names.size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
-
+  file_entry &fe = file_names[i];
   dwarf2_start_subfile (this, fe.name,
  fe.include_dir (line_header));
   buildsym_compunit *b = get_builder ();
@@ -11753,10 +11771,10 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  compunit_language (cust),
  0, cust));
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      auto &file_names = line_header->file_names ();
+      for (i = 0; i < file_names.size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
-
+  file_entry &fe = file_names[i];
   fe.symtab = tu_group->symtabs[i];
  }
     }
@@ -16230,7 +16248,7 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
     {
       /* Any related symtab will do.  */
       symtab
- = cu->line_header->file_name_at (file_name_index (1))->symtab;
+ = cu->line_header->file_names ()[0].symtab;
     }
   else
     {
@@ -20285,10 +20303,16 @@ void
 line_header::add_include_dir (const char *include_dir)
 {
   if (dwarf_line_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "Adding dir %zu: %s\n",
- include_dirs.size () + 1, include_dir);
-
-  include_dirs.push_back (include_dir);
+    {
+      size_t new_size;
+      if (version >= 5)
+        new_size = m_include_dirs.size ();
+      else
+        new_size = m_include_dirs.size () + 1;
+      fprintf_unfiltered (gdb_stdlog, "Adding dir %zu: %s\n",
+  new_size, include_dir);
+    }
+  m_include_dirs.push_back (include_dir);
 }
 
 void
@@ -20298,10 +20322,16 @@ line_header::add_file_name (const char *name,
     unsigned int length)
 {
   if (dwarf_line_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "Adding file %u: %s\n",
- (unsigned) file_names.size () + 1, name);
-
-  file_names.emplace_back (name, d_index, mod_time, length);
+    {
+      size_t new_size;
+      if (version >= 5)
+        new_size = file_names_size ();
+      else
+        new_size = file_names_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);
 }
 
 /* A convenience function to find the proper .debug_line section for a CU.  */
@@ -20415,6 +20445,11 @@ read_formatted_entries (struct dwarf2_per_objfile *dwarf2_per_objfile,
       buf += 8;
       break;
 
+    case DW_FORM_data16:
+      /*  This is used for MD5, but file_entry does not record MD5s. */
+      buf += 16;
+      break;
+
     case DW_FORM_udata:
       uint.emplace (read_unsigned_leb128 (abfd, buf, &bytes_read));
       buf += bytes_read;
@@ -20515,12 +20550,15 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     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;
+
   if (line_ptr + lh->total_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
       return 0;
     }
-  lh->statement_program_end = line_ptr + lh->total_length;
+  lh->statement_program_end = start_here + lh->total_length;
   lh->version = read_2_bytes (abfd, line_ptr);
   line_ptr += 2;
   if (lh->version > 5)
@@ -20550,6 +20588,7 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     }
   lh->header_length = read_offset_1 (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)
@@ -20634,7 +20673,6 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
  }
       line_ptr += bytes_read;
     }
-  lh->statement_program_start = line_ptr;
 
   if (line_ptr > (section->buffer + section->size))
     complaint (_("line number info header doesn't "
@@ -20651,12 +20689,11 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
    Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename.  */
 
 static const char *
-psymtab_include_file_name (const struct line_header *lh, int file_index,
+psymtab_include_file_name (const struct line_header *lh, const file_entry &fe,
    const struct partial_symtab *pst,
    const char *comp_dir,
    gdb::unique_xmalloc_ptr<char> *name_holder)
 {
-  const file_entry &fe = lh->file_names[file_index];
   const char *include_name = fe.name;
   const char *include_name_to_compare = include_name;
   const char *pst_filename;
@@ -20831,8 +20868,8 @@ private:
      and initialized according to the DWARF spec.  */
 
   unsigned char m_op_index = 0;
-  /* The line table index (1-based) of the current file.  */
-  file_name_index m_file = (file_name_index) 1;
+  /* The line table index of the current file.  */
+  file_name_index m_file = 1;
   unsigned int m_line = 1;
 
   /* These are initialized in the constructor.  */
@@ -21024,7 +21061,7 @@ lnp_state_machine::record_line (bool end_sequence)
       fprintf_unfiltered (gdb_stdlog,
   "Processing actual line %u: file %u,"
   " address %s, is_stmt %u, discrim %u\n",
-  m_line, to_underlying (m_file),
+  m_line, m_file,
   paddress (m_gdbarch, m_address),
   m_is_stmt, m_discriminator);
     }
@@ -21363,17 +21400,15 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
 
   if (decode_for_pst_p)
     {
-      int file_index;
-
       /* Now that we're done scanning the Line Header Program, we can
          create the psymtab of each included file.  */
-      for (file_index = 0; file_index < lh->file_names.size (); file_index++)
-        if (lh->file_names[file_index].included_p == 1)
+      for (auto &file_entry : lh->file_names ())
+        if (file_entry.included_p == 1)
           {
     gdb::unique_xmalloc_ptr<char> name_holder;
     const char *include_name =
-      psymtab_include_file_name (lh, file_index, pst, comp_dir,
- &name_holder);
+      psymtab_include_file_name (lh, file_entry, pst,
+ comp_dir, &name_holder);
     if (include_name != NULL)
               dwarf2_create_include_psymtab (include_name, pst, objfile);
           }
@@ -21385,14 +21420,10 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
  line numbers).  */
       buildsym_compunit *builder = cu->get_builder ();
       struct compunit_symtab *cust = builder->get_compunit_symtab ();
-      int i;
 
-      for (i = 0; i < lh->file_names.size (); i++)
+      for (auto &fe : lh->file_names ())
  {
-  file_entry &fe = lh->file_names[i];
-
   dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh));
-
   if (builder->get_current_subfile ()->symtab == NULL)
     {
       builder->get_current_subfile ()->symtab
@@ -24190,6 +24221,14 @@ dwarf_alloc_die (struct dwarf2_cu *cu, int num_attrs)
 
 /* Macro support.  */
 
+static bool
+is_valid_file_index (int file_index, struct line_header *lh)
+{
+  if (lh->version >= 5)
+    return 0 <= file_index && file_index < lh->file_names_size ();
+  return 1 <= file_index && file_index <= lh->file_names_size ();
+}
+
 /* Return file name relative to the compilation directory of file number I in
    *LH's file name table.  The result is allocated using xmalloc; the caller is
    responsible for freeing it.  */
@@ -24199,17 +24238,17 @@ file_file_name (int file, struct line_header *lh)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (is_valid_file_index (file, lh))
     {
-      const file_entry &fe = lh->file_names[file - 1];
+      const file_entry *fe = lh->file_name_at (file);
 
-      if (!IS_ABSOLUTE_PATH (fe.name))
+      if (!IS_ABSOLUTE_PATH (fe->name))
  {
-  const char *dir = fe.include_dir (lh);
+  const char *dir = fe->include_dir (lh);
   if (dir != NULL)
-    return concat (dir, SLASH_STRING, fe.name, (char *) NULL);
+    return concat (dir, SLASH_STRING, fe->name, (char *) NULL);
  }
-      return xstrdup (fe.name);
+      return xstrdup (fe->name);
     }
   else
     {
@@ -24237,7 +24276,7 @@ file_full_name (int file, struct line_header *lh, const char *comp_dir)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (is_valid_file_index (file, lh))
     {
       char *relative = file_file_name (file, lh);
 
--
2.23.0.866.gb869b98d4c-goog

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] DWARF 5 support: Handle line table and file indexes

Simon Marchi
On 2019-10-18 3:31 p.m., Ali Tamur wrote:
> I've done everything asked, except this bit:
>> I'd still suggest adding a gdb_assert to verify that in the case of
> DWARF <= 4, index should be > 0.
>
> This caused failure in multiple tests, I think the correct behaviour is to
> return nullptr when DWARF <= 4 and index = 0 (done).

Oh, right, requesting the entry 0 isn't invalid, it's just that it represents the
compilation directory, which is not explicitly represented.  We indeed return
NULL at the moment when requesting index 0 (it underflows the unsigned variable),
so you are right that we should continue doing that.

> @@ -20651,12 +20689,11 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
>     Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename.  */
>  
>  static const char *
> -psymtab_include_file_name (const struct line_header *lh, int file_index,
> +psymtab_include_file_name (const struct line_header *lh, const file_entry &fe,
>     const struct partial_symtab *pst,
>     const char *comp_dir,
>     gdb::unique_xmalloc_ptr<char> *name_holder)

The comment above this function says:

  Return the file name of the psymtab for included file FILE_INDEX

It would need to be updated.

> @@ -24190,6 +24221,14 @@ dwarf_alloc_die (struct dwarf2_cu *cu, int num_attrs)
>  
>  /* Macro support.  */
>  
> +static bool
> +is_valid_file_index (int file_index, struct line_header *lh)
> +{
> +  if (lh->version >= 5)
> +    return 0 <= file_index && file_index < lh->file_names_size ();
> +  return 1 <= file_index && file_index <= lh->file_names_size ();
> +}

I think it would make sense if this was a method of line_header, since it's related to
the line_header::file_name_at method.

Simon
Reply | Threaded
Open this post in threaded view
|

[PATCH] DWARF 5 support: Handle line table and file indexes

Sourceware - gdb-patches mailing list
> Update the comment for psymtab_include_file_name
Done.

> is_valid_file_index should be a method of line_header
Done.
---

*  Fix handling of file and directory indexes in line tables; in DWARF 5 the
indexes are zero-based. Make file_names field private to abstract this detail
from the clients. Introduce file_names, is_valid_file_index and
file_names_size methods. Reflect these changes in clients.
*  Handle DW_FORM_data16 in read_formatted_entries; it is used to record MD5
of the file entries in DWARF 5.
*  Fix a bug in line header parsing that calculates the length of the header
incorrectly. (Seemingly this manifests itself only in DWARF 5).

Tested with CC=/usr/bin/gcc (version 8.3.0) against master branch (also with
-gsplit-dwarf and -gdwarf-4 flags) and there was no increase in the set of
tests that fails. (gdb still cannot debug a 'hello world' program with DWARF 5,
so for the time being, this is all we care about).

This is part of an effort to support DWARF 5 in gdb.
---
 gdb/dwarf2read.c | 179 ++++++++++++++++++++++++++++-------------------
 1 file changed, 108 insertions(+), 71 deletions(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index ee9df34ed2..fa83384fd9 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -922,13 +922,13 @@ typedef void (die_reader_func_ftype) (const struct die_reader_specs *reader,
       int has_children,
       void *data);
 
-/* A 1-based directory index.  This is a strong typedef to prevent
-   accidentally using a directory index as a 0-based index into an
-   array/vector.  */
-enum class dir_index : unsigned int {};
+/* dir_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5 and
+   later.  */
+typedef int dir_index;
 
-/* Likewise, a 1-based file name index.  */
-enum class file_name_index : unsigned int {};
+/* file_name_index is 1-based in DWARF 4 and before, and is 0-based in DWARF 5
+   and later.  */
+typedef int file_name_index;
 
 struct file_entry
 {
@@ -980,32 +980,47 @@ struct line_header
   void add_file_name (const char *name, dir_index d_index,
       unsigned int mod_time, unsigned int length);
 
-  /* Return the include dir at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
+  /* Return the include dir 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
   {
-    /* Convert directory index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
-
-    if (vec_index >= include_dirs.size ())
+    int vec_index;
+    if (version >= 5)
+      vec_index = index;
+    else
+      vec_index = index - 1;
+    if (vec_index < 0 || vec_index >= m_include_dirs.size ())
       return NULL;
-    return include_dirs[vec_index];
+    return m_include_dirs[vec_index];
   }
 
-  /* Return the file name at INDEX (1-based).  Returns NULL if INDEX
-     is out of bounds.  */
-  file_entry *file_name_at (file_name_index index)
+  bool is_valid_file_index (int file_index)
   {
-    /* Convert file name index number (1-based) to vector index
-       (0-based).  */
-    size_t vec_index = to_underlying (index) - 1;
+    if (version >= 5)
+      return 0 <= file_index && file_index < file_names_size ();
+    return 1 <= file_index && file_index <= file_names_size ();
+  }
 
-    if (vec_index >= file_names.size ())
+  /* Return the file name 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)
+      vec_index = index;
+    else
+      vec_index = index - 1;
+    if (vec_index < 0 || vec_index >= m_file_names.size ())
       return NULL;
-    return &file_names[vec_index];
+    return &m_file_names[vec_index];
   }
 
+  /* The indexes are 0-based in DWARF 5 and 1-based in DWARF 4. Therefore,
+     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; }
+
   /* Offset of line number information in .debug_line section.  */
   sect_offset sect_off {};
 
@@ -1032,12 +1047,23 @@ struct line_header
      pointers.  The memory is owned by debug_line_buffer.  */
   std::vector<const char *> include_dirs;
 
-  /* The file_names table.  */
-  std::vector<file_entry> file_names;
+  int file_names_size ()
+  { return m_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 {};
+
+ 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 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;
 };
 
 typedef std::unique_ptr<line_header> line_header_up;
@@ -3644,7 +3670,6 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   struct objfile *objfile = dwarf2_per_objfile->objfile;
   struct dwarf2_per_cu_data *lh_cu;
   struct attribute *attr;
-  int i;
   void **slot;
   struct quick_file_names *qfn;
 
@@ -3703,12 +3728,12 @@ dw2_get_file_names_reader (const struct die_reader_specs *reader,
   if (strcmp (fnd.name, "<unknown>") != 0)
     ++offset;
 
-  qfn->num_file_names = offset + lh->file_names.size ();
+  qfn->num_file_names = offset + lh->file_names_size ();
   qfn->file_names =
     XOBNEWVEC (&objfile->objfile_obstack, const char *, qfn->num_file_names);
   if (offset != 0)
     qfn->file_names[0] = xstrdup (fnd.name);
-  for (i = 0; i < lh->file_names.size (); ++i)
+  for (int i = 0; i < lh->file_names_size (); ++i)
     qfn->file_names[i + offset] = file_full_name (i + 1, lh.get (), fnd.comp_dir);
   qfn->real_names = NULL;
 
@@ -11717,14 +11742,14 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  process_full_type_unit still needs to know if this is the first
  time.  */
 
-      tu_group->num_symtabs = line_header->file_names.size ();
+      tu_group->num_symtabs = line_header->file_names_size ();
       tu_group->symtabs = XNEWVEC (struct symtab *,
-   line_header->file_names.size ());
+   line_header->file_names_size ());
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      auto &file_names = line_header->file_names ();
+      for (i = 0; i < file_names.size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
-
+  file_entry &fe = file_names[i];
   dwarf2_start_subfile (this, fe.name,
  fe.include_dir (line_header));
   buildsym_compunit *b = get_builder ();
@@ -11753,10 +11778,10 @@ dwarf2_cu::setup_type_unit_groups (struct die_info *die)
  compunit_language (cust),
  0, cust));
 
-      for (i = 0; i < line_header->file_names.size (); ++i)
+      auto &file_names = line_header->file_names ();
+      for (i = 0; i < file_names.size (); ++i)
  {
-  file_entry &fe = line_header->file_names[i];
-
+  file_entry &fe = file_names[i];
   fe.symtab = tu_group->symtabs[i];
  }
     }
@@ -16230,7 +16255,7 @@ process_structure_scope (struct die_info *die, struct dwarf2_cu *cu)
     {
       /* Any related symtab will do.  */
       symtab
- = cu->line_header->file_name_at (file_name_index (1))->symtab;
+ = cu->line_header->file_names ()[0].symtab;
     }
   else
     {
@@ -20285,10 +20310,16 @@ void
 line_header::add_include_dir (const char *include_dir)
 {
   if (dwarf_line_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "Adding dir %zu: %s\n",
- include_dirs.size () + 1, include_dir);
-
-  include_dirs.push_back (include_dir);
+    {
+      size_t new_size;
+      if (version >= 5)
+        new_size = m_include_dirs.size ();
+      else
+        new_size = m_include_dirs.size () + 1;
+      fprintf_unfiltered (gdb_stdlog, "Adding dir %zu: %s\n",
+  new_size, include_dir);
+    }
+  m_include_dirs.push_back (include_dir);
 }
 
 void
@@ -20298,10 +20329,16 @@ line_header::add_file_name (const char *name,
     unsigned int length)
 {
   if (dwarf_line_debug >= 2)
-    fprintf_unfiltered (gdb_stdlog, "Adding file %u: %s\n",
- (unsigned) file_names.size () + 1, name);
-
-  file_names.emplace_back (name, d_index, mod_time, length);
+    {
+      size_t new_size;
+      if (version >= 5)
+        new_size = file_names_size ();
+      else
+        new_size = file_names_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);
 }
 
 /* A convenience function to find the proper .debug_line section for a CU.  */
@@ -20415,6 +20452,11 @@ read_formatted_entries (struct dwarf2_per_objfile *dwarf2_per_objfile,
       buf += 8;
       break;
 
+    case DW_FORM_data16:
+      /*  This is used for MD5, but file_entry does not record MD5s. */
+      buf += 16;
+      break;
+
     case DW_FORM_udata:
       uint.emplace (read_unsigned_leb128 (abfd, buf, &bytes_read));
       buf += bytes_read;
@@ -20515,12 +20557,15 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     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;
+
   if (line_ptr + lh->total_length > (section->buffer + section->size))
     {
       dwarf2_statement_list_fits_in_line_number_section_complaint ();
       return 0;
     }
-  lh->statement_program_end = line_ptr + lh->total_length;
+  lh->statement_program_end = start_here + lh->total_length;
   lh->version = read_2_bytes (abfd, line_ptr);
   line_ptr += 2;
   if (lh->version > 5)
@@ -20550,6 +20595,7 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
     }
   lh->header_length = read_offset_1 (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)
@@ -20634,7 +20680,6 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
  }
       line_ptr += bytes_read;
     }
-  lh->statement_program_start = line_ptr;
 
   if (line_ptr > (section->buffer + section->size))
     complaint (_("line number info header doesn't "
@@ -20644,19 +20689,17 @@ dwarf_decode_line_header (sect_offset sect_off, struct dwarf2_cu *cu)
 }
 
 /* Subroutine of dwarf_decode_lines to simplify it.
-   Return the file name of the psymtab for included file FILE_INDEX
-   in line header LH of PST.
+   Return the file name of the psymtab for the given file_entry.
    COMP_DIR is the compilation directory (DW_AT_comp_dir) or NULL if unknown.
    If space for the result is malloc'd, *NAME_HOLDER will be set.
    Returns NULL if FILE_INDEX should be ignored, i.e., it is pst->filename.  */
 
 static const char *
-psymtab_include_file_name (const struct line_header *lh, int file_index,
+psymtab_include_file_name (const struct line_header *lh, const file_entry &fe,
    const struct partial_symtab *pst,
    const char *comp_dir,
    gdb::unique_xmalloc_ptr<char> *name_holder)
 {
-  const file_entry &fe = lh->file_names[file_index];
   const char *include_name = fe.name;
   const char *include_name_to_compare = include_name;
   const char *pst_filename;
@@ -20831,8 +20874,8 @@ private:
      and initialized according to the DWARF spec.  */
 
   unsigned char m_op_index = 0;
-  /* The line table index (1-based) of the current file.  */
-  file_name_index m_file = (file_name_index) 1;
+  /* The line table index of the current file.  */
+  file_name_index m_file = 1;
   unsigned int m_line = 1;
 
   /* These are initialized in the constructor.  */
@@ -21024,7 +21067,7 @@ lnp_state_machine::record_line (bool end_sequence)
       fprintf_unfiltered (gdb_stdlog,
   "Processing actual line %u: file %u,"
   " address %s, is_stmt %u, discrim %u\n",
-  m_line, to_underlying (m_file),
+  m_line, m_file,
   paddress (m_gdbarch, m_address),
   m_is_stmt, m_discriminator);
     }
@@ -21363,17 +21406,15 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
 
   if (decode_for_pst_p)
     {
-      int file_index;
-
       /* Now that we're done scanning the Line Header Program, we can
          create the psymtab of each included file.  */
-      for (file_index = 0; file_index < lh->file_names.size (); file_index++)
-        if (lh->file_names[file_index].included_p == 1)
+      for (auto &file_entry : lh->file_names ())
+        if (file_entry.included_p == 1)
           {
     gdb::unique_xmalloc_ptr<char> name_holder;
     const char *include_name =
-      psymtab_include_file_name (lh, file_index, pst, comp_dir,
- &name_holder);
+      psymtab_include_file_name (lh, file_entry, pst,
+ comp_dir, &name_holder);
     if (include_name != NULL)
               dwarf2_create_include_psymtab (include_name, pst, objfile);
           }
@@ -21385,14 +21426,10 @@ dwarf_decode_lines (struct line_header *lh, const char *comp_dir,
  line numbers).  */
       buildsym_compunit *builder = cu->get_builder ();
       struct compunit_symtab *cust = builder->get_compunit_symtab ();
-      int i;
 
-      for (i = 0; i < lh->file_names.size (); i++)
+      for (auto &fe : lh->file_names ())
  {
-  file_entry &fe = lh->file_names[i];
-
   dwarf2_start_subfile (cu, fe.name, fe.include_dir (lh));
-
   if (builder->get_current_subfile ()->symtab == NULL)
     {
       builder->get_current_subfile ()->symtab
@@ -24199,17 +24236,17 @@ file_file_name (int file, struct line_header *lh)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (lh->is_valid_file_index (file))
     {
-      const file_entry &fe = lh->file_names[file - 1];
+      const file_entry *fe = lh->file_name_at (file);
 
-      if (!IS_ABSOLUTE_PATH (fe.name))
+      if (!IS_ABSOLUTE_PATH (fe->name))
  {
-  const char *dir = fe.include_dir (lh);
+  const char *dir = fe->include_dir (lh);
   if (dir != NULL)
-    return concat (dir, SLASH_STRING, fe.name, (char *) NULL);
+    return concat (dir, SLASH_STRING, fe->name, (char *) NULL);
  }
-      return xstrdup (fe.name);
+      return xstrdup (fe->name);
     }
   else
     {
@@ -24237,7 +24274,7 @@ file_full_name (int file, struct line_header *lh, const char *comp_dir)
 {
   /* 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 (1 <= file && file <= lh->file_names.size ())
+  if (lh->is_valid_file_index (file))
     {
       char *relative = file_file_name (file, lh);
 
--
2.23.0.866.gb869b98d4c-goog

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] DWARF 5 support: Handle line table and file indexes

Simon Marchi
On 2019-10-21 3:47 p.m., Ali Tamur wrote:

>> Update the comment for psymtab_include_file_name
> Done.
>
>> is_valid_file_index should be a method of line_header
> Done.
> ---
>
> *  Fix handling of file and directory indexes in line tables; in DWARF 5 the
> indexes are zero-based. Make file_names field private to abstract this detail
> from the clients. Introduce file_names, is_valid_file_index and
> file_names_size methods. Reflect these changes in clients.
> *  Handle DW_FORM_data16 in read_formatted_entries; it is used to record MD5
> of the file entries in DWARF 5.
> *  Fix a bug in line header parsing that calculates the length of the header
> incorrectly. (Seemingly this manifests itself only in DWARF 5).
>
> Tested with CC=/usr/bin/gcc (version 8.3.0) against master branch (also with
> -gsplit-dwarf and -gdwarf-4 flags) and there was no increase in the set of
> tests that fails. (gdb still cannot debug a 'hello world' program with DWARF 5,
> so for the time being, this is all we care about).
>
> This is part of an effort to support DWARF 5 in gdb.

Thanks, this version LGTM, please push.

Simon