[PATCH 0/2] Two minor abbrev improvements

[PATCH 0/2] Two minor abbrev improvements

[Tom Tromey]

Here are a couple of minor abbrev-related improvements I've had
sitting around.

One removes an old workaround.
The other applies "const" in some spots.

Let me know what you think.

Tom

[PATCH 1/2] Remove Irix 6 workaround from DWARF abbrev reader

[Tom Tromey]

abbrev_table::read has a workaround for Irix 6.  The last release of
Irix was in 2006, and (according to Wikipedia) hardware produced after
2007 cannot run Irix.  I think this workaround can safely be retired.

gdb/ChangeLog
2021-03-06  Tom Tromey  <tom@tromey.com>

	* dwarf2/abbrev.c (abbrev_table::read): Remove Irix 6 workaround.
---
 gdb/ChangeLog       |  4 ++++
 gdb/dwarf2/abbrev.c | 28 +++++++++-------------------
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/gdb/dwarf2/abbrev.c b/gdb/dwarf2/abbrev.c
index 9ece708a1ac..a8bdf7182de 100644
--- a/gdb/dwarf2/abbrev.c
+++ b/gdb/dwarf2/abbrev.c
@@ -84,7 +84,6 @@ abbrev_table::read (struct dwarf2_section_info *section,
   bfd *abfd = section->get_bfd_owner ();
   const gdb_byte *abbrev_ptr;
   struct abbrev_info *cur_abbrev;
-  unsigned int abbrev_number, bytes_read;
 
   abbrev_table_up abbrev_table (new struct abbrev_table (sect_off));
   struct obstack *obstack = &abbrev_table->m_abbrev_obstack;
@@ -92,12 +91,17 @@ abbrev_table::read (struct dwarf2_section_info *section,
   /* Caller must ensure this.  */
   gdb_assert (section->readin);
   abbrev_ptr = section->buffer + to_underlying (sect_off);
-  abbrev_number = read_unsigned_leb128 (abfd, abbrev_ptr, &bytes_read);
-  abbrev_ptr += bytes_read;
 
-  /* Loop until we reach an abbrev number of 0.  */
-  while (abbrev_number)
+  while (true)
     {
+      unsigned int bytes_read;
+      /* Loop until we reach an abbrev number of 0.  */
+      unsigned int abbrev_number = read_unsigned_leb128 (abfd, abbrev_ptr,
+							 &bytes_read);
+      if (abbrev_number == 0)
+	break;
+      abbrev_ptr += bytes_read;
+
       /* Start without any attrs.  */
       obstack_blank (obstack, offsetof (abbrev_info, attrs));
       cur_abbrev = (struct abbrev_info *) obstack_base (obstack);
@@ -144,20 +148,6 @@ abbrev_table::read (struct dwarf2_section_info *section,
       cur_abbrev = (struct abbrev_info *) obstack_finish (obstack);
       cur_abbrev->num_attrs = num_attrs;
       abbrev_table->add_abbrev (cur_abbrev);
-
-      /* Get next abbreviation.
-	 Under Irix6 the abbreviations for a compilation unit are not
-	 always properly terminated with an abbrev number of 0.
-	 Exit loop if we encounter an abbreviation which we have
-	 already read (which means we are about to read the abbreviations
-	 for the next compile unit) or if the end of the abbreviation
-	 table is reached.  */
-      if ((unsigned int) (abbrev_ptr - section->buffer) >= section->size)
-	break;
-      abbrev_number = read_unsigned_leb128 (abfd, abbrev_ptr, &bytes_read);
-      abbrev_ptr += bytes_read;
-      if (abbrev_table->lookup_abbrev (abbrev_number) != NULL)
-	break;
     }
 
   return abbrev_table;
-- 
2.26.2

Re: [PATCH 1/2] Remove Irix 6 workaround from DWARF abbrev reader

[Maciej W. Rozycki]

On Sat, 6 Mar 2021, Tom Tromey wrote:

> abbrev_table::read has a workaround for Irix 6.  The last release of
> Irix was in 2006, and (according to Wikipedia) hardware produced after
> 2007 cannot run Irix.  I think this workaround can safely be retired.

 I missed this submission previously.

 For the record, we dropped IRIX target support in GDB years ago, with 
commit 3831839c089c ("Delete IRIX support").  It could have been useful to 
have a reference to said commit included with the change description, alas 
it didn't happen.

 We continue having IRIX support in binutils, because the underlying ELF 
format peculiarities continue being used by some embedded targets and just 
removing the IRIX target triplets in these circumstances would be rather 
pointless and merely an potential unnecessary complication for someone who 
may still be using GNU binutils for IRIX.  I suspect the OS may still have 
a niche use, perhaps among the vintage computing enthusiasts.

  Maciej

Re: [PATCH 1/2] Remove Irix 6 workaround from DWARF abbrev reader

[Tom de Vries]

On 3/6/21 6:01 PM, Tom Tromey wrote:
> abbrev_table::read has a workaround for Irix 6.  The last release of
> Irix was in 2006, and (according to Wikipedia) hardware produced after
> 2007 cannot run Irix.  I think this workaround can safely be retired.
> 

This caused the following regression:
...
FAIL: gdb.dwarf2/dw2-cu-size.exp: ptype noloc
...

Filed at https://sourceware.org/bugzilla/show_bug.cgi?id=27604 .

Thanks,
- Tom

> gdb/ChangeLog
> 2021-03-06  Tom Tromey  <tom@tromey.com>
> 
> 	* dwarf2/abbrev.c (abbrev_table::read): Remove Irix 6 workaround.
> ---
>  gdb/ChangeLog       |  4 ++++
>  gdb/dwarf2/abbrev.c | 28 +++++++++-------------------
>  2 files changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/gdb/dwarf2/abbrev.c b/gdb/dwarf2/abbrev.c
> index 9ece708a1ac..a8bdf7182de 100644
> --- a/gdb/dwarf2/abbrev.c
> +++ b/gdb/dwarf2/abbrev.c
> @@ -84,7 +84,6 @@ abbrev_table::read (struct dwarf2_section_info *section,
>    bfd *abfd = section->get_bfd_owner ();
>    const gdb_byte *abbrev_ptr;
>    struct abbrev_info *cur_abbrev;
> -  unsigned int abbrev_number, bytes_read;
>  
>    abbrev_table_up abbrev_table (new struct abbrev_table (sect_off));
>    struct obstack *obstack = &abbrev_table->m_abbrev_obstack;
> @@ -92,12 +91,17 @@ abbrev_table::read (struct dwarf2_section_info *section,
>    /* Caller must ensure this.  */
>    gdb_assert (section->readin);
>    abbrev_ptr = section->buffer + to_underlying (sect_off);
> -  abbrev_number = read_unsigned_leb128 (abfd, abbrev_ptr, &bytes_read);
> -  abbrev_ptr += bytes_read;
>  
> -  /* Loop until we reach an abbrev number of 0.  */
> -  while (abbrev_number)
> +  while (true)
>      {
> +      unsigned int bytes_read;
> +      /* Loop until we reach an abbrev number of 0.  */
> +      unsigned int abbrev_number = read_unsigned_leb128 (abfd, abbrev_ptr,
> +							 &bytes_read);
> +      if (abbrev_number == 0)
> +	break;
> +      abbrev_ptr += bytes_read;
> +
>        /* Start without any attrs.  */
>        obstack_blank (obstack, offsetof (abbrev_info, attrs));
>        cur_abbrev = (struct abbrev_info *) obstack_base (obstack);
> @@ -144,20 +148,6 @@ abbrev_table::read (struct dwarf2_section_info *section,
>        cur_abbrev = (struct abbrev_info *) obstack_finish (obstack);
>        cur_abbrev->num_attrs = num_attrs;
>        abbrev_table->add_abbrev (cur_abbrev);
> -
> -      /* Get next abbreviation.
> -	 Under Irix6 the abbreviations for a compilation unit are not
> -	 always properly terminated with an abbrev number of 0.
> -	 Exit loop if we encounter an abbreviation which we have
> -	 already read (which means we are about to read the abbreviations
> -	 for the next compile unit) or if the end of the abbreviation
> -	 table is reached.  */
> -      if ((unsigned int) (abbrev_ptr - section->buffer) >= section->size)
> -	break;
> -      abbrev_number = read_unsigned_leb128 (abfd, abbrev_ptr, &bytes_read);
> -      abbrev_ptr += bytes_read;
> -      if (abbrev_table->lookup_abbrev (abbrev_number) != NULL)
> -	break;
>      }
>  
>    return abbrev_table;
> 

[PATCH 2/2] Constify abbrev_table::lookup_abbrev

[Tom Tromey]

This changes abbrev_table::lookup_abbrev to return a pointer to const,
then fixes up the affected code.

gdb/ChangeLog
2021-03-06  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (struct partial_die_info): Update.
	(peek_die_abbrev, skip_children, skip_one_die, read_full_die_1)
	(load_partial_dies, partial_die_info::partial_die_info): Update.
	* dwarf2/abbrev.h (lookup_abbrev): Constify.
---
 gdb/ChangeLog       |  7 +++++++
 gdb/dwarf2/abbrev.h |  2 +-
 gdb/dwarf2/read.c   | 21 ++++++++++++---------
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/gdb/dwarf2/abbrev.h b/gdb/dwarf2/abbrev.h
index e1d8b80619b..50f2ed46083 100644
--- a/gdb/dwarf2/abbrev.h
+++ b/gdb/dwarf2/abbrev.h
@@ -71,7 +71,7 @@ struct abbrev_table
   /* Look up an abbrev in the table.
      Returns NULL if the abbrev is not found.  */
 
-  struct abbrev_info *lookup_abbrev (unsigned int abbrev_number)
+  const struct abbrev_info *lookup_abbrev (unsigned int abbrev_number) const
   {
     struct abbrev_info search;
     search.number = abbrev_number;
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index f550a4e5008..3b72a96affe 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1024,7 +1024,7 @@ class cutu_reader : public die_reader_specs
    need this much information.  */
 struct partial_die_info : public allocate_on_obstack
   {
-    partial_die_info (sect_offset sect_off, struct abbrev_info *abbrev);
+    partial_die_info (sect_offset sect_off, const struct abbrev_info *abbrev);
 
     /* Disable assign but still keep copy ctor, which is needed
        load_partial_dies.   */
@@ -1624,7 +1624,7 @@ static void dwarf2_symbol_mark_computed (const struct attribute *attr,
 
 static const gdb_byte *skip_one_die (const struct die_reader_specs *reader,
 				     const gdb_byte *info_ptr,
-				     struct abbrev_info *abbrev);
+				     const struct abbrev_info *abbrev);
 
 static hashval_t partial_die_hash (const void *item);
 
@@ -8697,7 +8697,7 @@ peek_abbrev_code (bfd *abfd, const gdb_byte *info_ptr)
    an empty DIE).  In either case *BYTES_READ will be set to the length of
    the initial number.  */
 
-static struct abbrev_info *
+static const struct abbrev_info *
 peek_die_abbrev (const die_reader_specs &reader,
 		 const gdb_byte *info_ptr, unsigned int *bytes_read)
 {
@@ -8709,7 +8709,8 @@ peek_die_abbrev (const die_reader_specs &reader,
   if (abbrev_number == 0)
     return NULL;
 
-  abbrev_info *abbrev = reader.abbrev_table->lookup_abbrev (abbrev_number);
+  const abbrev_info *abbrev
+    = reader.abbrev_table->lookup_abbrev (abbrev_number);
   if (!abbrev)
     {
       error (_("Dwarf Error: Could not find abbrev number %d in %s"
@@ -8731,7 +8732,8 @@ skip_children (const struct die_reader_specs *reader, const gdb_byte *info_ptr)
   while (1)
     {
       unsigned int bytes_read;
-      abbrev_info *abbrev = peek_die_abbrev (*reader, info_ptr, &bytes_read);
+      const abbrev_info *abbrev = peek_die_abbrev (*reader, info_ptr,
+						   &bytes_read);
 
       if (abbrev == NULL)
 	return info_ptr + bytes_read;
@@ -8748,7 +8750,7 @@ skip_children (const struct die_reader_specs *reader, const gdb_byte *info_ptr)
 
 static const gdb_byte *
 skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
-	      struct abbrev_info *abbrev)
+	      const struct abbrev_info *abbrev)
 {
   unsigned int bytes_read;
   struct attribute attr;
@@ -19169,7 +19171,7 @@ read_full_die_1 (const struct die_reader_specs *reader,
 		 int num_extra_attrs)
 {
   unsigned int abbrev_number, bytes_read, i;
-  struct abbrev_info *abbrev;
+  const struct abbrev_info *abbrev;
   struct die_info *die;
   struct dwarf2_cu *cu = reader->cu;
   bfd *abfd = reader->abfd;
@@ -19333,7 +19335,8 @@ load_partial_dies (const struct die_reader_specs *reader,
 
   while (1)
     {
-      abbrev_info *abbrev = peek_die_abbrev (*reader, info_ptr, &bytes_read);
+      const abbrev_info *abbrev = peek_die_abbrev (*reader, info_ptr,
+						   &bytes_read);
 
       /* A NULL abbrev means the end of a series of children.  */
       if (abbrev == NULL)
@@ -19565,7 +19568,7 @@ load_partial_dies (const struct die_reader_specs *reader,
 }
 
 partial_die_info::partial_die_info (sect_offset sect_off_,
-				    struct abbrev_info *abbrev)
+				    const struct abbrev_info *abbrev)
   : partial_die_info (sect_off_, abbrev->tag, abbrev->has_children)
 {
 }
-- 
2.26.2

Re: [PATCH 0/2] Two minor abbrev improvements

[Simon Marchi]

On 2021-03-06 12:01 p.m., Tom Tromey wrote:
> Here are a couple of minor abbrev-related improvements I've had
> sitting around.
> 
> One removes an old workaround.
> The other applies "const" in some spots.
> 
> Let me know what you think.
> 
> Tom
> 
> 

This LGTM, thanks.

Simon

Re: [PATCH 0/2] Two minor abbrev improvements

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 2021-03-06 12:01 p.m., Tom Tromey wrote:
>> Here are a couple of minor abbrev-related improvements I've had
>> sitting around.
>> One removes an old workaround.
>> The other applies "const" in some spots.
>> Let me know what you think.
>> Tom
>> 

Simon> This LGTM, thanks.

I'm checking these in now.

Tom