Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] gdb: add gdb_bfd_all_sections for range-based iteration
@ 2020-08-24 19:43 Simon Marchi
  2020-08-24 20:31 ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2020-08-24 19:43 UTC (permalink / raw)
  To: gdb-patches

I wanted to make a nicer / type-safe interface for
bfd_map_over_sections, avoiding the `void *` data parameter.

My first shot was to make a wrapper for bfd_map_over_sections,
gdb_bfd_map_over_sections that took a gdb::function_view.

However, I think that a range adapter gives nicer and simpler code, as a
simple for loop is easier to read than a callback / lambda function.  So
here it is, it uses next_iterator and next_adapter, so it's not much
code.

As an example, I ported maintenance_info_sections and friends to use it.
The maint_print_section_data type could probably be removed now, but I
didn't want to do too much in one patch.

gdb/ChangeLog:

	* gdb_bfd.h (gdb_bfd_section_iterator, gdb_bfd_section_range,
	gdb_bfd_all_sections): New.
	* maint.c (print_bfd_section_info): Change param type to
	maint_print_section_data.
	(print_objfile_section_info): Likewise.
	(print_bfd_section_info_maybe_relocated): Likewise.
	(maintenance_info_sections): Use gdb_bfd_all_sections.

Change-Id: Ib496f6b0a0eb7aadb10da1dd381304014d934ea0
---
 gdb/gdb_bfd.h | 18 ++++++++++++++++++
 gdb/maint.c   | 35 ++++++++++++++++-------------------
 2 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index a941b79fe734..3cb50e56e8ac 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -23,6 +23,7 @@
 #include "registry.h"
 #include "gdbsupport/byte-vector.h"
 #include "gdbsupport/gdb_ref_ptr.h"
+#include "gdbsupport/next-iterator.h"
 
 DECLARE_REGISTRY (bfd);
 
@@ -193,4 +194,21 @@ int gdb_bfd_requires_relocations (bfd *abfd);
 bool gdb_bfd_get_full_section_contents (bfd *abfd, asection *section,
 					gdb::byte_vector *contents);
 
+/* Range adapter for a BFD's sections.
+
+   To be used as:
+
+     for (asection *sect : gdb_bfd_all_sections (bfd))
+       ... use SECT ...
+ */
+
+using gdb_bfd_section_iterator = next_iterator<asection>;
+using gdb_bfd_section_range = next_adapter<asection, gdb_bfd_section_iterator>;
+
+static inline
+gdb_bfd_section_range gdb_bfd_all_sections (bfd *abfd)
+{
+  return gdb_bfd_section_range (abfd->sections);
+}
+
 #endif /* GDB_BFD_H */
diff --git a/gdb/maint.c b/gdb/maint.c
index 3368769ad96f..8adac24efcb0 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -266,7 +266,7 @@ struct maint_print_section_data
   maint_print_section_data (struct objfile *objfile, const char *arg,
 			    bfd *abfd)
     : objfile (objfile),
-      arg(arg)
+      arg (arg)
   {
     int section_count = gdb_bfd_count_sections (abfd);
     index_digits = ((int) log10 ((float) section_count)) + 1;
@@ -292,20 +292,18 @@ print_section_index (bfd *abfd,
   printf_filtered ("%-*s", (index_digits + 4), result.c_str ());
 }
 
-/* Print information about ASECT from ABFD.  DATUM holds a pointer to a
-   maint_print_section_data object.  The section will be printed using the
-   VMA's from the bfd, which will not be the relocated addresses for bfds
-   that should be relocated.  The information must be printed with the
-   same layout as PRINT_OBJFILE_SECTION_INFO below.  */
+/* Print information about ASECT from ABFD.  The section will be printed using
+   the VMA's from the bfd, which will not be the relocated addresses for bfds
+   that should be relocated.  The information must be printed with the same
+   layout as PRINT_OBJFILE_SECTION_INFO below.  */
 
 static void
 print_bfd_section_info (bfd *abfd,
 			asection *asect,
-			void *datum)
+			const maint_print_section_data *print_data)
 {
   flagword flags = bfd_section_flags (asect);
   const char *name = bfd_section_name (asect);
-  maint_print_section_data *print_data = (maint_print_section_data *) datum;
   const char *arg = print_data->arg;
 
   if (arg == NULL || *arg == '\0'
@@ -332,7 +330,7 @@ print_bfd_section_info (bfd *abfd,
 static void
 print_objfile_section_info (bfd *abfd,
 			    struct obj_section *asect,
-			    maint_print_section_data *print_data)
+			    const maint_print_section_data *print_data)
 {
   flagword flags = bfd_section_flags (asect->the_bfd_section);
   const char *name = bfd_section_name (asect->the_bfd_section);
@@ -376,16 +374,14 @@ maint_obj_section_from_bfd_section (bfd *abfd,
   return osect;
 }
 
-/* Print information about ASECT from ABFD.  DATUM holds a pointer to a
-   maint_print_section_data object.  Where possible the information for
+/* Print information about ASECT from ABFD.  Where possible the information for
    ASECT will print the relocated addresses of the section.  */
 
 static void
 print_bfd_section_info_maybe_relocated (bfd *abfd,
 					asection *asect,
-					void *datum)
+					const maint_print_section_data *print_data)
 {
-  maint_print_section_data *print_data = (maint_print_section_data *) datum;
   objfile *objfile = print_data->objfile;
 
   gdb_assert (objfile->sections != NULL);
@@ -393,7 +389,7 @@ print_bfd_section_info_maybe_relocated (bfd *abfd,
     = maint_obj_section_from_bfd_section (abfd, asect, objfile);
 
   if (osect->the_bfd_section == NULL)
-    print_bfd_section_info (abfd, asect, datum);
+    print_bfd_section_info (abfd, asect, print_data);
   else
     print_objfile_section_info (abfd, osect, print_data);
 }
@@ -432,9 +428,9 @@ maintenance_info_sections (const char *arg, int from_tty)
 
 	  maint_print_section_data print_data (ofile, arg, ofile->obfd);
 
-	  bfd_map_over_sections (ofile->obfd,
-				 print_bfd_section_info_maybe_relocated,
-				 (void *) &print_data);
+	  for (asection *sect : gdb_bfd_all_sections (ofile->obfd))
+	    print_bfd_section_info_maybe_relocated (ofile->obfd, sect,
+						    &print_data);
 	}
     }
 
@@ -446,8 +442,9 @@ maintenance_info_sections (const char *arg, int from_tty)
       printf_filtered ("    `%s', ", bfd_get_filename (core_bfd));
       wrap_here ("        ");
       printf_filtered (_("file type %s.\n"), bfd_get_target (core_bfd));
-      bfd_map_over_sections (core_bfd, print_bfd_section_info,
-			     (void *) &print_data);
+
+      for (asection *sect : gdb_bfd_all_sections (core_bfd))
+	print_bfd_section_info (core_bfd, sect, &print_data);
     }
 }
 
-- 
2.28.0



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

* Re: [PATCH] gdb: add gdb_bfd_all_sections for range-based iteration
  2020-08-24 19:43 [PATCH] gdb: add gdb_bfd_all_sections for range-based iteration Simon Marchi
@ 2020-08-24 20:31 ` Tom Tromey
  2020-08-24 20:34   ` Simon Marchi
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2020-08-24 20:31 UTC (permalink / raw)
  To: Simon Marchi via Gdb-patches

>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> I wanted to make a nicer / type-safe interface for
Simon> bfd_map_over_sections, avoiding the `void *` data parameter.

Simon> My first shot was to make a wrapper for bfd_map_over_sections,
Simon> gdb_bfd_map_over_sections that took a gdb::function_view.

Simon> However, I think that a range adapter gives nicer and simpler code, as a
Simon> simple for loop is easier to read than a callback / lambda function.  So
Simon> here it is, it uses next_iterator and next_adapter, so it's not much
Simon> code.

Simon> As an example, I ported maintenance_info_sections and friends to use it.
Simon> The maint_print_section_data type could probably be removed now, but I
Simon> didn't want to do too much in one patch.

I also did this kind of thing -- I have some patches to remove many uses
of bfd_map_over_sections, though not all of them, since some seem hard
to test.

Simon> 	* gdb_bfd.h (gdb_bfd_section_iterator, gdb_bfd_section_range,
Simon> 	gdb_bfd_all_sections): New.
Simon> 	* maint.c (print_bfd_section_info): Change param type to
Simon> 	maint_print_section_data.
Simon> 	(print_objfile_section_info): Likewise.
Simon> 	(print_bfd_section_info_maybe_relocated): Likewise.
Simon> 	(maintenance_info_sections): Use gdb_bfd_all_sections.

Looks good.  Thanks.

Tom


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

* Re: [PATCH] gdb: add gdb_bfd_all_sections for range-based iteration
  2020-08-24 20:31 ` Tom Tromey
@ 2020-08-24 20:34   ` Simon Marchi
  2020-08-24 20:57     ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2020-08-24 20:34 UTC (permalink / raw)
  To: Tom Tromey, Simon Marchi via Gdb-patches

On 2020-08-24 4:31 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Simon> I wanted to make a nicer / type-safe interface for
> Simon> bfd_map_over_sections, avoiding the `void *` data parameter.
> 
> Simon> My first shot was to make a wrapper for bfd_map_over_sections,
> Simon> gdb_bfd_map_over_sections that took a gdb::function_view.
> 
> Simon> However, I think that a range adapter gives nicer and simpler code, as a
> Simon> simple for loop is easier to read than a callback / lambda function.  So
> Simon> here it is, it uses next_iterator and next_adapter, so it's not much
> Simon> code.
> 
> Simon> As an example, I ported maintenance_info_sections and friends to use it.
> Simon> The maint_print_section_data type could probably be removed now, but I
> Simon> didn't want to do too much in one patch.
> 
> I also did this kind of thing -- I have some patches to remove many uses
> of bfd_map_over_sections, though not all of them, since some seem hard
> to test.

What approach did you take?

Simon


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

* Re: [PATCH] gdb: add gdb_bfd_all_sections for range-based iteration
  2020-08-24 20:34   ` Simon Marchi
@ 2020-08-24 20:57     ` Tom Tromey
  2020-08-24 21:30       ` Simon Marchi
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Tromey @ 2020-08-24 20:57 UTC (permalink / raw)
  To: Simon Marchi; +Cc: Tom Tromey, Simon Marchi via Gdb-patches

>> I also did this kind of thing -- I have some patches to remove many uses
>> of bfd_map_over_sections, though not all of them, since some seem hard
>> to test.

Simon> What approach did you take?

Almost exactly identical to yours -- I named the adapter
"gdb_bfd_sections".  I can resurrect some of my patches after yours
lands.

Tom


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

* Re: [PATCH] gdb: add gdb_bfd_all_sections for range-based iteration
  2020-08-24 20:57     ` Tom Tromey
@ 2020-08-24 21:30       ` Simon Marchi
  2020-08-27 12:59         ` Simon Marchi
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Marchi @ 2020-08-24 21:30 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Simon Marchi via Gdb-patches

On 2020-08-24 4:57 p.m., Tom Tromey wrote:
>>> I also did this kind of thing -- I have some patches to remove many uses
>>> of bfd_map_over_sections, though not all of them, since some seem hard
>>> to test.
> 
> Simon> What approach did you take?
> 
> Almost exactly identical to yours -- I named the adapter
> "gdb_bfd_sections".  I can resurrect some of my patches after yours
> lands.
> 
> Tom
> 

I modeled the "gdb_bfd_all_sections" on "all_threads" and "all_inferiors".  But honestly
the "all" sounds superfluous now that I re-read it.

I'll push the patch with "all" removed from the name.

Thanks!

Simon


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

* Re: [PATCH] gdb: add gdb_bfd_all_sections for range-based iteration
  2020-08-24 21:30       ` Simon Marchi
@ 2020-08-27 12:59         ` Simon Marchi
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Marchi @ 2020-08-27 12:59 UTC (permalink / raw)
  To: Simon Marchi, Tom Tromey; +Cc: Simon Marchi via Gdb-patches

On 2020-08-24 5:30 p.m., Simon Marchi via Gdb-patches wrote:
> On 2020-08-24 4:57 p.m., Tom Tromey wrote:
>>>> I also did this kind of thing -- I have some patches to remove many uses
>>>> of bfd_map_over_sections, though not all of them, since some seem hard
>>>> to test.
>>
>> Simon> What approach did you take?
>>
>> Almost exactly identical to yours -- I named the adapter
>> "gdb_bfd_sections".  I can resurrect some of my patches after yours
>> lands.
>>
>> Tom
>>
> 
> I modeled the "gdb_bfd_all_sections" on "all_threads" and "all_inferiors".  But honestly
> the "all" sounds superfluous now that I re-read it.
> 
> I'll push the patch with "all" removed from the name.
> 
> Thanks!
> 
> Simon
> 

I finally did this.  I fixed some formatting issues at the same time and made the
print_data parameters references.  Here's what I pushed:

From b886559f31fabb8bf234c1818dd01f8987e3c190 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Thu, 27 Aug 2020 08:58:43 -0400
Subject: [PATCH] gdb: add gdb_bfd_sections for range-based iteration

I wanted to make a nicer / type-safe interface for
bfd_map_over_sections, avoiding the `void *` data parameter.

My first shot was to make a wrapper for bfd_map_over_sections,
gdb_bfd_map_over_sections that took a gdb::function_view.

However, I think that a range adapter gives nicer and simpler code, as a
simple for loop is easier to read than a callback / lambda function.  So
here it is, it uses next_iterator and next_adapter, so it's not much
code.

As an example, I ported maintenance_info_sections and friends to use it.
The maint_print_section_data type could probably be removed now, but I
didn't want to do too much in one patch.

gdb/ChangeLog:

	* gdb_bfd.h (gdb_bfd_section_iterator, gdb_bfd_section_range,
	gdb_bfd_sections): New.
	* maint.c (print_bfd_section_info): Change param type to
	maint_print_section_data.
	(print_objfile_section_info): Likewise.
	(print_bfd_section_info_maybe_relocated): Likewise.
	(maintenance_info_sections): Use gdb_bfd_sections.

Change-Id: Ib496f6b0a0eb7aadb10da1dd381304014d934ea0
---
 gdb/ChangeLog | 10 ++++++++++
 gdb/gdb_bfd.h | 18 ++++++++++++++++++
 gdb/maint.c   | 48 ++++++++++++++++++++++--------------------------
 3 files changed, 50 insertions(+), 26 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ceff808d8296..4fd89745713d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2020-08-27  Simon Marchi  <simon.marchi@polymtl.ca>
+
+	* gdb_bfd.h (gdb_bfd_section_iterator, gdb_bfd_section_range,
+	gdb_bfd_sections): New.
+	* maint.c (print_bfd_section_info): Change param type to
+	maint_print_section_data.
+	(print_objfile_section_info): Likewise.
+	(print_bfd_section_info_maybe_relocated): Likewise.
+	(maintenance_info_sections): Use gdb_bfd_sections.
+
 2020-08-25  Shahab Vahedi  <shahab@synopsys.com>

 	* MAINTAINERS: Add ARC target and maintainer.
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index a941b79fe734..0f450567d50e 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -23,6 +23,7 @@
 #include "registry.h"
 #include "gdbsupport/byte-vector.h"
 #include "gdbsupport/gdb_ref_ptr.h"
+#include "gdbsupport/next-iterator.h"

 DECLARE_REGISTRY (bfd);

@@ -193,4 +194,21 @@ int gdb_bfd_requires_relocations (bfd *abfd);
 bool gdb_bfd_get_full_section_contents (bfd *abfd, asection *section,
 					gdb::byte_vector *contents);

+/* Range adapter for a BFD's sections.
+
+   To be used as:
+
+     for (asection *sect : gdb_bfd_all_sections (bfd))
+       ... use SECT ...
+ */
+
+using gdb_bfd_section_iterator = next_iterator<asection>;
+using gdb_bfd_section_range = next_adapter<asection, gdb_bfd_section_iterator>;
+
+static inline
+gdb_bfd_section_range gdb_bfd_sections (bfd *abfd)
+{
+  return gdb_bfd_section_range (abfd->sections);
+}
+
 #endif /* GDB_BFD_H */
diff --git a/gdb/maint.c b/gdb/maint.c
index 3368769ad96f..be0b1605159a 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -266,7 +266,7 @@ struct maint_print_section_data
   maint_print_section_data (struct objfile *objfile, const char *arg,
 			    bfd *abfd)
     : objfile (objfile),
-      arg(arg)
+      arg (arg)
   {
     int section_count = gdb_bfd_count_sections (abfd);
     index_digits = ((int) log10 ((float) section_count)) + 1;
@@ -292,21 +292,19 @@ print_section_index (bfd *abfd,
   printf_filtered ("%-*s", (index_digits + 4), result.c_str ());
 }

-/* Print information about ASECT from ABFD.  DATUM holds a pointer to a
-   maint_print_section_data object.  The section will be printed using the
-   VMA's from the bfd, which will not be the relocated addresses for bfds
-   that should be relocated.  The information must be printed with the
-   same layout as PRINT_OBJFILE_SECTION_INFO below.  */
+/* Print information about ASECT from ABFD.  The section will be printed using
+   the VMA's from the bfd, which will not be the relocated addresses for bfds
+   that should be relocated.  The information must be printed with the same
+   layout as PRINT_OBJFILE_SECTION_INFO below.  */

 static void
 print_bfd_section_info (bfd *abfd,
 			asection *asect,
-			void *datum)
+			const maint_print_section_data &print_data)
 {
   flagword flags = bfd_section_flags (asect);
   const char *name = bfd_section_name (asect);
-  maint_print_section_data *print_data = (maint_print_section_data *) datum;
-  const char *arg = print_data->arg;
+  const char *arg = print_data.arg;

   if (arg == NULL || *arg == '\0'
       || match_substring (arg, name)
@@ -318,7 +316,7 @@ print_bfd_section_info (bfd *abfd,

       addr = bfd_section_vma (asect);
       endaddr = addr + bfd_section_size (asect);
-      print_section_index (abfd, asect, print_data->index_digits);
+      print_section_index (abfd, asect, print_data.index_digits);
       maint_print_section_info (name, flags, addr, endaddr,
 				asect->filepos, addr_size);
     }
@@ -332,11 +330,11 @@ print_bfd_section_info (bfd *abfd,
 static void
 print_objfile_section_info (bfd *abfd,
 			    struct obj_section *asect,
-			    maint_print_section_data *print_data)
+			    const maint_print_section_data &print_data)
 {
   flagword flags = bfd_section_flags (asect->the_bfd_section);
   const char *name = bfd_section_name (asect->the_bfd_section);
-  const char *string = print_data->arg;
+  const char *string = print_data.arg;

   if (string == NULL || *string == '\0'
       || match_substring (string, name)
@@ -346,7 +344,7 @@ print_objfile_section_info (bfd *abfd,
       int addr_size = gdbarch_addr_bit (gdbarch) / 8;

       print_section_index (abfd, asect->the_bfd_section,
-			   print_data->index_digits);
+			   print_data.index_digits);
       maint_print_section_info (name, flags,
 				obj_section_addr (asect),
 				obj_section_endaddr (asect),
@@ -376,24 +374,21 @@ maint_obj_section_from_bfd_section (bfd *abfd,
   return osect;
 }

-/* Print information about ASECT from ABFD.  DATUM holds a pointer to a
-   maint_print_section_data object.  Where possible the information for
+/* Print information about ASECT from ABFD.  Where possible the information for
    ASECT will print the relocated addresses of the section.  */

 static void
-print_bfd_section_info_maybe_relocated (bfd *abfd,
-					asection *asect,
-					void *datum)
+print_bfd_section_info_maybe_relocated
+  (bfd *abfd, asection *asect, const maint_print_section_data &print_data)
 {
-  maint_print_section_data *print_data = (maint_print_section_data *) datum;
-  objfile *objfile = print_data->objfile;
+  objfile *objfile = print_data.objfile;

   gdb_assert (objfile->sections != NULL);
   obj_section *osect
     = maint_obj_section_from_bfd_section (abfd, asect, objfile);

   if (osect->the_bfd_section == NULL)
-    print_bfd_section_info (abfd, asect, datum);
+    print_bfd_section_info (abfd, asect, print_data);
   else
     print_objfile_section_info (abfd, osect, print_data);
 }
@@ -432,9 +427,9 @@ maintenance_info_sections (const char *arg, int from_tty)

 	  maint_print_section_data print_data (ofile, arg, ofile->obfd);

-	  bfd_map_over_sections (ofile->obfd,
-				 print_bfd_section_info_maybe_relocated,
-				 (void *) &print_data);
+	  for (asection *sect : gdb_bfd_sections (ofile->obfd))
+	    print_bfd_section_info_maybe_relocated (ofile->obfd, sect,
+						    print_data);
 	}
     }

@@ -446,8 +441,9 @@ maintenance_info_sections (const char *arg, int from_tty)
       printf_filtered ("    `%s', ", bfd_get_filename (core_bfd));
       wrap_here ("        ");
       printf_filtered (_("file type %s.\n"), bfd_get_target (core_bfd));
-      bfd_map_over_sections (core_bfd, print_bfd_section_info,
-			     (void *) &print_data);
+
+      for (asection *sect : gdb_bfd_sections (core_bfd))
+	print_bfd_section_info (core_bfd, sect, print_data);
     }
 }

-- 
2.28.0



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

end of thread, other threads:[~2020-08-27 12:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-24 19:43 [PATCH] gdb: add gdb_bfd_all_sections for range-based iteration Simon Marchi
2020-08-24 20:31 ` Tom Tromey
2020-08-24 20:34   ` Simon Marchi
2020-08-24 20:57     ` Tom Tromey
2020-08-24 21:30       ` Simon Marchi
2020-08-27 12:59         ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox