Patchwork [RFC,3/5] Make index reading functions more modular

login
register
mail settings
Submitter Simon Marchi
Date May 9, 2018, 9:26 p.m.
Message ID <1525901216-15031-4-git-send-email-simon.marchi@ericsson.com>
Download mbox | patch
Permalink /patch/27188/
State New
Headers show

Comments

Simon Marchi - May 9, 2018, 9:26 p.m.
The read_gdb_index_from_section and read_debug_names_from_section
functions read the index content, as their names state, from sections of
object files.  A following patch will make it possible to read index
content from standalone files.

This patch therefore decouples the code that reads the index content
from object file sections from the code that processes that content.
Functions dwarf2_read_gdb_index and dwarf2_read_debug_names receive
callbacks that are responsible for providing the index contents (for
both the main file and the potential dwz file).

gdb/ChangeLog:

	* dwarf2read.c (read_gdb_index_from_section): Rename to...
	(read_gdb_index_from_buffer): ... this.  Remove section
	parameter, add buffer parameter.
	(get_gdb_index_contents_ftype,
	get_gdb_index_contents_dwz_ftype): New typedefs.
	(dwarf2_read_gdb_index): Add callback parameters to get the
	index contents.
	(read_debug_names_from_section): Rename to...
	(read_debug_names_from_buffer): ... this.  Remove section
	parameter, add buffer and abfd parameters.
	(get_debug_names_contents_ftype,
	get_debug_names_contents_dwz_ftype): New typedefs.
	(dwarf2_read_debug_names): Add callbacks parameters to get index
	contents.
	(get_gdb_index_contents_from_section): New.
	(get_debug_names_contents_from_section): New.
	(dwarf2_initialize_objfile): Update calls to
	dwarf2_read_debug_names and dwarf2_read_gdb_index.
---
 gdb/dwarf2read.c | 207 ++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 136 insertions(+), 71 deletions(-)
Tom Tromey - May 10, 2018, 8:54 p.m.
>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> The read_gdb_index_from_section and read_debug_names_from_section
Simon> functions read the index content, as their names state, from sections of
Simon> object files.  A following patch will make it possible to read index
Simon> content from standalone files.

Simon> This patch therefore decouples the code that reads the index content
Simon> from object file sections from the code that processes that content.
Simon> Functions dwarf2_read_gdb_index and dwarf2_read_debug_names receive
Simon> callbacks that are responsible for providing the index contents (for
Simon> both the main file and the potential dwz file).

This callback style makes the code harder to read and understand; but
other than that this seems fine.

Tom
Simon Marchi - May 10, 2018, 9:04 p.m.
On 2018-05-10 16:54, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> The read_gdb_index_from_section and 
> read_debug_names_from_section
> Simon> functions read the index content, as their names state, from 
> sections of
> Simon> object files.  A following patch will make it possible to read 
> index
> Simon> content from standalone files.
> 
> Simon> This patch therefore decouples the code that reads the index 
> content
> Simon> from object file sections from the code that processes that 
> content.
> Simon> Functions dwarf2_read_gdb_index and dwarf2_read_debug_names 
> receive
> Simon> callbacks that are responsible for providing the index contents 
> (for
> Simon> both the main file and the potential dwz file).
> 
> This callback style makes the code harder to read and understand; but
> other than that this seems fine.

Indeed, I'm open to suggestions :).

Simon

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 1c72818..bc92756 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3406,8 +3406,8 @@  find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
     }
 }
 
-/* A helper function that reads the .gdb_index from SECTION and fills
-   in MAP.  FILENAME is the name of the file containing the section;
+/* A helper function that reads the .gdb_index from BUFFER and fills
+   in MAP.  FILENAME is the name of the file containing the data;
    it is used for error reporting.  DEPRECATED_OK is true if it is
    ok to use deprecated sections.
 
@@ -3415,37 +3415,23 @@  find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
    out parameters that are filled in with information about the CU and
    TU lists in the section.
 
-   Returns 1 if all went well, 0 otherwise.  */
+   Returns true if all went well, false otherwise.  */
 
 static bool
-read_gdb_index_from_section (struct objfile *objfile,
-			     const char *filename,
-			     bool deprecated_ok,
-			     struct dwarf2_section_info *section,
-			     struct mapped_index *map,
-			     const gdb_byte **cu_list,
-			     offset_type *cu_list_elements,
-			     const gdb_byte **types_list,
-			     offset_type *types_list_elements)
-{
-  const gdb_byte *addr;
-  offset_type version;
-  offset_type *metadata;
-  int i;
-
-  if (dwarf2_section_empty_p (section))
-    return 0;
+read_gdb_index_from_buffer (struct objfile *objfile,
+			    const char *filename,
+			    bool deprecated_ok,
+			    gdb::array_view<const gdb_byte> buffer,
+			    struct mapped_index *map,
+			    const gdb_byte **cu_list,
+			    offset_type *cu_list_elements,
+			    const gdb_byte **types_list,
+			    offset_type *types_list_elements)
+{
+  const gdb_byte *addr = &buffer[0];
 
-  /* Older elfutils strip versions could keep the section in the main
-     executable while splitting it for the separate debug info file.  */
-  if ((get_section_flags (section) & SEC_HAS_CONTENTS) == 0)
-    return 0;
-
-  dwarf2_read_section (objfile, section);
-
-  addr = section->buffer;
   /* Version check.  */
-  version = MAYBE_SWAP (*(offset_type *) addr);
+  offset_type version = MAYBE_SWAP (*(offset_type *) addr);
   /* Versions earlier than 3 emitted every copy of a psymbol.  This
      causes the index to behave very poorly for certain requests.  Version 3
      contained incomplete addrmap.  So, it seems better to just ignore such
@@ -3498,9 +3484,9 @@  to use the section anyway."),
 
   map->version = version;
 
-  metadata = (offset_type *) (addr + sizeof (offset_type));
+  offset_type *metadata = (offset_type *) (addr + sizeof (offset_type));
 
-  i = 0;
+  int i = 0;
   *cu_list = addr + MAYBE_SWAP (metadata[i]);
   *cu_list_elements = ((MAYBE_SWAP (metadata[i + 1]) - MAYBE_SWAP (metadata[i]))
 		       / 8);
@@ -3531,11 +3517,23 @@  to use the section anyway."),
   return 1;
 }
 
+/* Callback types for dwarf2_read_gdb_index.  */
+
+typedef gdb::function_view
+    <gdb::array_view<const gdb_byte>(objfile *, dwarf2_per_objfile *)>
+    get_gdb_index_contents_ftype;
+typedef gdb::function_view
+    <gdb::array_view<const gdb_byte>(objfile *, dwz_file *)>
+    get_gdb_index_contents_dwz_ftype;
+
 /* Read .gdb_index.  If everything went ok, initialize the "quick"
    elements of all the CUs and return 1.  Otherwise, return 0.  */
 
 static int
-dwarf2_read_gdb_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
+dwarf2_read_gdb_index
+  (struct dwarf2_per_objfile *dwarf2_per_objfile,
+   get_gdb_index_contents_ftype get_gdb_index_contents,
+   get_gdb_index_contents_dwz_ftype get_gdb_index_contents_dwz)
 {
   struct mapped_index local_map, *map;
   const gdb_byte *cu_list, *types_list, *dwz_list = NULL;
@@ -3543,11 +3541,17 @@  dwarf2_read_gdb_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
   struct dwz_file *dwz;
   struct objfile *objfile = dwarf2_per_objfile->objfile;
 
-  if (!read_gdb_index_from_section (objfile, objfile_name (objfile),
-				    use_deprecated_index_sections,
-				    &dwarf2_per_objfile->gdb_index, &local_map,
-				    &cu_list, &cu_list_elements, &types_list,
-				    &types_list_elements))
+  gdb::array_view<const gdb_byte> main_index_contents
+    = get_gdb_index_contents (objfile, dwarf2_per_objfile);
+
+  if (main_index_contents.empty ())
+    return 0;
+
+  if (!read_gdb_index_from_buffer (objfile, objfile_name (objfile),
+				   use_deprecated_index_sections,
+				   main_index_contents, &local_map, &cu_list,
+				   &cu_list_elements, &types_list,
+				   &types_list_elements))
     return 0;
 
   /* Don't use the index if it's empty.  */
@@ -3563,12 +3567,18 @@  dwarf2_read_gdb_index (struct dwarf2_per_objfile *dwarf2_per_objfile)
       const gdb_byte *dwz_types_ignore;
       offset_type dwz_types_elements_ignore;
 
-      if (!read_gdb_index_from_section (objfile,
-					bfd_get_filename (dwz->dwz_bfd), 1,
-					&dwz->gdb_index, &dwz_map,
-					&dwz_list, &dwz_list_elements,
-					&dwz_types_ignore,
-					&dwz_types_elements_ignore))
+      gdb::array_view<const gdb_byte> dwz_index_content
+	= get_gdb_index_contents_dwz (objfile, dwz);
+
+      if (dwz_index_content.empty ())
+	return 0;
+
+      if (!read_gdb_index_from_buffer (objfile,
+				       bfd_get_filename (dwz->dwz_bfd), 1,
+				       dwz_index_content, &dwz_map,
+				       &dwz_list, &dwz_list_elements,
+				       &dwz_types_ignore,
+				       &dwz_types_elements_ignore))
 	{
 	  warning (_("could not read '.gdb_index' section from %s; skipping"),
 		   bfd_get_filename (dwz->dwz_bfd));
@@ -5357,40 +5367,28 @@  static const gdb_byte dwarf5_augmentation[] = { 'G', 'D', 'B', 0 };
    Returns true if all went well, false otherwise.  */
 
 static bool
-read_debug_names_from_section (struct objfile *objfile,
-			       const char *filename,
-			       struct dwarf2_section_info *section,
-			       mapped_debug_names &map)
+read_debug_names_from_buffer (struct objfile *objfile,
+			      const char *filename,
+			      gdb::array_view<const gdb_byte> buffer,
+			      bfd *abfd, mapped_debug_names &map)
 {
-  if (dwarf2_section_empty_p (section))
-    return false;
-
-  /* Older elfutils strip versions could keep the section in the main
-     executable while splitting it for the separate debug info file.  */
-  if ((get_section_flags (section) & SEC_HAS_CONTENTS) == 0)
-    return false;
-
-  dwarf2_read_section (objfile, section);
+  const gdb_byte *addr = &buffer[0];
 
   map.dwarf5_byte_order = gdbarch_byte_order (get_objfile_arch (objfile));
 
-  const gdb_byte *addr = section->buffer;
-
-  bfd *const abfd = get_section_bfd_owner (section);
-
   unsigned int bytes_read;
   LONGEST length = read_initial_length (abfd, addr, &bytes_read);
   addr += bytes_read;
 
   map.dwarf5_is_dwarf64 = bytes_read != 4;
   map.offset_size = map.dwarf5_is_dwarf64 ? 8 : 4;
-  if (bytes_read + length != section->size)
+  if (bytes_read + length != buffer.size ())
     {
       /* There may be multiple per-CU indices.  */
       warning (_("Section .debug_names in %s length %s does not match "
 		 "section length %s, ignoring .debug_names."),
 	       filename, plongest (bytes_read + length),
-	       pulongest (section->size));
+	       pulongest (buffer.size ()));
       return false;
     }
 
@@ -5592,19 +5590,37 @@  create_cus_from_debug_names (struct dwarf2_per_objfile *dwarf2_per_objfile,
 				    true /* is_dwz */);
 }
 
+/* Callback types for dwarf2_read_debug_names.  */
+
+typedef gdb::function_view
+    <gdb::array_view<const gdb_byte>(objfile *, dwarf2_per_objfile *)>
+    get_debug_names_contents_ftype;
+typedef gdb::function_view
+    <gdb::array_view<const gdb_byte>(objfile *, dwz_file *)>
+    get_debug_names_contents_dwz_ftype;
+
 /* Read .debug_names.  If everything went ok, initialize the "quick"
    elements of all the CUs and return true.  Otherwise, return false.  */
 
 static bool
-dwarf2_read_debug_names (struct dwarf2_per_objfile *dwarf2_per_objfile)
+dwarf2_read_debug_names
+  (struct dwarf2_per_objfile *dwarf2_per_objfile,
+   get_debug_names_contents_ftype get_debug_names_contents,
+   get_debug_names_contents_dwz_ftype get_debug_names_contents_dwz)
 {
   mapped_debug_names local_map (dwarf2_per_objfile);
   mapped_debug_names dwz_map (dwarf2_per_objfile);
   struct objfile *objfile = dwarf2_per_objfile->objfile;
 
-  if (!read_debug_names_from_section (objfile, objfile_name (objfile),
-				      &dwarf2_per_objfile->debug_names,
-				      local_map))
+  gdb::array_view<const gdb_byte> main_debug_names_content
+    = get_debug_names_contents (objfile, dwarf2_per_objfile);
+
+  if (main_debug_names_content.empty ())
+    return false;
+
+  if (!read_debug_names_from_buffer (objfile, objfile_name (objfile),
+				     main_debug_names_content, objfile->obfd,
+				     local_map))
     return false;
 
   /* Don't use the index if it's empty.  */
@@ -5616,9 +5632,16 @@  dwarf2_read_debug_names (struct dwarf2_per_objfile *dwarf2_per_objfile)
   dwz_file *dwz = dwarf2_get_dwz_file (dwarf2_per_objfile);
   if (dwz != NULL)
     {
-      if (!read_debug_names_from_section (objfile,
-					  bfd_get_filename (dwz->dwz_bfd),
-					  &dwz->debug_names, dwz_map))
+      gdb::array_view<const gdb_byte> dwz_debug_names_content
+        = get_debug_names_contents_dwz (objfile, dwz);
+
+      if (dwz_debug_names_content.empty ())
+	return false;
+
+      if (!read_debug_names_from_buffer (objfile,
+					 bfd_get_filename (dwz->dwz_bfd),
+					 dwz_debug_names_content, dwz->dwz_bfd,
+					 dwz_map))
 	{
 	  warning (_("could not read '.debug_names' section from %s; skipping"),
 		   bfd_get_filename (dwz->dwz_bfd));
@@ -6166,6 +6189,44 @@  const struct quick_symbol_functions dwarf2_debug_names_functions =
   dw2_map_symbol_filenames
 };
 
+template <typename T>
+static gdb::array_view<const gdb_byte>
+get_gdb_index_contents_from_section (objfile *obj, T *section_owner)
+{
+  dwarf2_section_info *section = &section_owner->gdb_index;
+
+  if (dwarf2_section_empty_p (section))
+    return {};
+
+  /* Older elfutils strip versions could keep the section in the main
+     executable while splitting it for the separate debug info file.  */
+  if ((get_section_flags (section) & SEC_HAS_CONTENTS) == 0)
+    return {};
+
+  dwarf2_read_section (obj, section);
+
+  return {section->buffer, section->size};
+}
+
+template<typename T>
+static gdb::array_view<const gdb_byte>
+get_debug_names_contents_from_section (objfile *obj, T *section_owner)
+{
+  dwarf2_section_info *section = &section_owner->debug_names;
+
+  if (dwarf2_section_empty_p (section))
+    return {};
+
+  /* Older elfutils strip versions could keep the section in the main
+     executable while splitting it for the separate debug info file.  */
+  if ((get_section_flags (section) & SEC_HAS_CONTENTS) == 0)
+    return {};
+
+  dwarf2_read_section (obj, section);
+
+  return {section->buffer, section->size};
+}
+
 /* See symfile.h.  */
 
 bool
@@ -6203,13 +6264,17 @@  dwarf2_initialize_objfile (struct objfile *objfile, dw_index_kind *index_kind)
       return true;
     }
 
-  if (dwarf2_read_debug_names (dwarf2_per_objfile))
+  if (dwarf2_read_debug_names (dwarf2_per_objfile,
+			       get_debug_names_contents_from_section<struct dwarf2_per_objfile>,
+			       get_debug_names_contents_from_section<dwz_file>))
     {
       *index_kind = dw_index_kind::DEBUG_NAMES;
       return true;
     }
 
-  if (dwarf2_read_gdb_index (dwarf2_per_objfile))
+  if (dwarf2_read_gdb_index (dwarf2_per_objfile,
+			     get_gdb_index_contents_from_section<struct dwarf2_per_objfile>,
+			     get_gdb_index_contents_from_section<dwz_file>))
     {
       *index_kind = dw_index_kind::GDB_INDEX;
       return true;