[gold commit] Strip .debug_gnu_pubnames/types when building gdb index

[gold commit] Strip .debug_gnu_pubnames/types when building gdb index

[Cary Coutant]

When building a .gdb_index section, gold should strip .debug_gnu_pubnames and
.debug_gnu_pubtypes sections, as it does for .debug_pubnames and
.debug_pubtypes. When not stripping those sections, there was a bug
where gold was incorrectly adjusting section-relative offsets by the offset
of the input section within the output section. That adjustment was both
unnecessary and wrong, causing gold to miss a number of debug entries that
should have been added to .gdb_index. (With stripping, the adjustment was
always 0, so the bug in dwarf_reader.cc would have been hidden by the change
to layout.cc.)

-cary


2014-06-06  Cary Coutant  <ccoutant@google.com>

gold/
	* dwarf_reader.h (Dwarf_pubnames_table): Remove output_section_offset_.
	* dwarf_reader.cc (Dwarf_pubnames_table::read_section): Likewise.
	(Dwarf_pubnames_table::read_header): Likewise.
	* layout.cc (gdb_fast_lookup_sections): Add .debug_gnu_pubnames and
	.debug_gnu_pubtypes.


diff --git a/gold/dwarf_reader.cc b/gold/dwarf_reader.cc
index bc9e85f..df14bd5 100644
--- a/gold/dwarf_reader.cc
+++ b/gold/dwarf_reader.cc
@@ -505,13 +505,11 @@ Dwarf_pubnames_table::read_section(Relobj* object, const unsigned char* symtab,
       if (strcmp(section_name_suffix, name) == 0)
         {
           shndx = i;
-          this->output_section_offset_ = object->output_section_offset(i);
           break;
         }
       else if (strcmp(section_name_suffix, gnu_name) == 0)
         {
           shndx = i;
-          this->output_section_offset_ = object->output_section_offset(i);
           this->is_gnu_style_ = true;
           break;
         }
@@ -560,11 +558,6 @@ Dwarf_pubnames_table::read_header(off_t offset)
   // Make sure we have actually read the section.
   gold_assert(this->buffer_ != NULL);
 
-  // Correct the offset.  For incremental update links, we have a
-  // relocated offset that is relative to the output section, but
-  // here we need an offset relative to the input section.
-  offset -= this->output_section_offset_;
-
   if (offset < 0 || offset + 14 >= this->buffer_end_ - this->buffer_)
     return false;
 
diff --git a/gold/dwarf_reader.h b/gold/dwarf_reader.h
index 8dd62ad..cac413b 100644
--- a/gold/dwarf_reader.h
+++ b/gold/dwarf_reader.h
@@ -401,7 +401,7 @@ class Dwarf_pubnames_table
     : dwinfo_(dwinfo), buffer_(NULL), buffer_end_(NULL), owns_buffer_(false),
       offset_size_(0), pinfo_(NULL), end_of_table_(NULL),
       is_pubtypes_(is_pubtypes), is_gnu_style_(false),
-      output_section_offset_(0), unit_length_(0), cu_offset_(0)
+      unit_length_(0), cu_offset_(0)
   { }
 
   ~Dwarf_pubnames_table()
@@ -455,11 +455,6 @@ class Dwarf_pubnames_table
   // Gnu-style pubnames table. This style has an extra flag byte between the
   // offset and the name, and is used for generating version 7 of gdb-index.
   bool is_gnu_style_;
-  // For incremental update links, this will hold the offset of the
-  // input section within the output section.  Offsets read from
-  // relocated data will be relative to the output section, and need
-  // to be corrected before reading data from the input section.
-  uint64_t output_section_offset_;
   // Fields read from the header.
   uint64_t unit_length_;
   off_t cu_offset_;
diff --git a/gold/layout.cc b/gold/layout.cc
index 147f740..c5c3b57 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -563,7 +563,9 @@ static const char* gdb_fast_lookup_sections[] =
 {
   "aranges",
   "pubnames",
+  "gnu_pubnames",
   "pubtypes",
+  "gnu_pubtypes",
 };
 
 // Returns whether the given debug section is in the list of

Re: [gold commit] Strip .debug_gnu_pubnames/types when building gdb index

[ISHIKAWA,chiaki]

(2014/06/07 8:21), Cary Coutant wrote:
> When building a .gdb_index section, gold should strip .debug_gnu_pubnames and
> .debug_gnu_pubtypes sections, as it does for .debug_pubnames and
> .debug_pubtypes. When not stripping those sections, there was a bug
> where gold was incorrectly adjusting section-relative offsets by the offset
> of the input section within the output section. That adjustment was both
> unnecessary and wrong, causing gold to miss a number of debug entries that
> should have been added to .gdb_index. (With stripping, the adjustment was
> always 0, so the bug in dwarf_reader.cc would have been hidden by the change
> to layout.cc.)
> 
> -cary
> 
> 
> 2014-06-06  Cary Coutant  <ccoutant@google.com>
> 
> gold/
> 	* dwarf_reader.h (Dwarf_pubnames_table): Remove output_section_offset_.
> 	* dwarf_reader.cc (Dwarf_pubnames_table::read_section): Likewise.
> 	(Dwarf_pubnames_table::read_header): Likewise.
> 	* layout.cc (gdb_fast_lookup_sections): Add .debug_gnu_pubnames and
> 	.debug_gnu_pubtypes.
> 
> 
> diff --git a/gold/dwarf_reader.cc b/gold/dwarf_reader.cc
> index bc9e85f..df14bd5 100644
> --- a/gold/dwarf_reader.cc
> +++ b/gold/dwarf_reader.cc
> @@ -505,13 +505,11 @@ Dwarf_pubnames_table::read_section(Relobj* object, const unsigned char* symtab,
>         if (strcmp(section_name_suffix, name) == 0)
>           {
>             shndx = i;
> -          this->output_section_offset_ = object->output_section_offset(i);
>             break;
>           }
>         else if (strcmp(section_name_suffix, gnu_name) == 0)
>           {
>             shndx = i;
> -          this->output_section_offset_ = object->output_section_offset(i);
>             this->is_gnu_style_ = true;
>             break;
>           }
> @@ -560,11 +558,6 @@ Dwarf_pubnames_table::read_header(off_t offset)
>     // Make sure we have actually read the section.
>     gold_assert(this->buffer_ != NULL);
>   
> -  // Correct the offset.  For incremental update links, we have a
> -  // relocated offset that is relative to the output section, but
> -  // here we need an offset relative to the input section.
> -  offset -= this->output_section_offset_;
> -
>     if (offset < 0 || offset + 14 >= this->buffer_end_ - this->buffer_)
>       return false;
>   
> diff --git a/gold/dwarf_reader.h b/gold/dwarf_reader.h
> index 8dd62ad..cac413b 100644
> --- a/gold/dwarf_reader.h
> +++ b/gold/dwarf_reader.h
> @@ -401,7 +401,7 @@ class Dwarf_pubnames_table
>       : dwinfo_(dwinfo), buffer_(NULL), buffer_end_(NULL), owns_buffer_(false),
>         offset_size_(0), pinfo_(NULL), end_of_table_(NULL),
>         is_pubtypes_(is_pubtypes), is_gnu_style_(false),
> -      output_section_offset_(0), unit_length_(0), cu_offset_(0)
> +      unit_length_(0), cu_offset_(0)
>     { }
>   
>     ~Dwarf_pubnames_table()
> @@ -455,11 +455,6 @@ class Dwarf_pubnames_table
>     // Gnu-style pubnames table. This style has an extra flag byte between the
>     // offset and the name, and is used for generating version 7 of gdb-index.
>     bool is_gnu_style_;
> -  // For incremental update links, this will hold the offset of the
> -  // input section within the output section.  Offsets read from
> -  // relocated data will be relative to the output section, and need
> -  // to be corrected before reading data from the input section.
> -  uint64_t output_section_offset_;
>     // Fields read from the header.
>     uint64_t unit_length_;
>     off_t cu_offset_;
> diff --git a/gold/layout.cc b/gold/layout.cc
> index 147f740..c5c3b57 100644
> --- a/gold/layout.cc
> +++ b/gold/layout.cc
> @@ -563,7 +563,9 @@ static const char* gdb_fast_lookup_sections[] =
>   {
>     "aranges",
>     "pubnames",
> +  "gnu_pubnames",
>     "pubtypes",
> +  "gnu_pubtypes",
>   };
>   
>   // Returns whether the given debug section is in the list of
> 
> 

Hi,

Can this change fix the ld segfault issue I have reported?

To verify, I tried to patch my source, then I found that the patch did
not apply cleanly.
So I tried pulling git repository source, but I am afraid that there is
a network issue and I could not pull the source files in a reasonable
time when I tried to access the repository over the weekend.(Or maybe,
just maybe, there are so many patches that accumulate over the years,
that pulling the source from scratch may incur too much overhead on the
git repository server: I mean creating the patch set to send over the
wire, and I notice that the server and my git client said something
about the compression, etc., and I saw very long pause. I tried git: and
http: protocol and both failed...

TIA

Re: [gold commit] Strip .debug_gnu_pubnames/types when building gdb index

[Cary Coutant]

> Can this change fix the ld segfault issue I have reported?

I don't think so. Even with the bad adjustment, the code was careful
to check that the resulting offset was within the bounds of the
current pubnames section.

-cary