public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] MIPS support for --hash-style=gnu
@ 2015-10-05 23:11 Neil Schellenberger (neschell)
  2015-10-30 11:09 ` Nick Clifton
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Neil Schellenberger (neschell) @ 2015-10-05 23:11 UTC (permalink / raw)
  To: binutils

[-- Attachment #1: Type: text/plain, Size: 10929 bytes --]

I am tasked with reducing the time spent in the interpreter/loader at program start time for a very large, multi-platform, multi-architecture, legacy system (25+Mloc, 2000+ DSOs, 1+M symbols).  On MIPS this loader overhead is several much larger than for other supported architectures, which is not unexpected given the lack of .gnu.hash support. [1]  Measurement shows that a principal factor for the programs most affected is the very large number of DSOs which are directly or indirectly needed -- the chief expense being the cost of successive table misses during symbol resolution.  A secondary factor is that some of the executables and DSOs have a very large number of symbols with relocations -- on MIPS these tend to run afoul of the multi-GOT mechanism which places many into secondary GOTs, resulting in unconditionally forcing their resolution at load time. [2][3][4]  At this stage in the lifecycle of this particular product, large scale architectural changes tend not to be feasible (e.g. appreciably decreasing either the number of DSOs or the number of relocations) so a more transparent solution is preferable.  (c.f. [5][6])

In order to tackle the main problem -- large numbers of needed DSOs -- some means of avoiding unprofitable (i.e. certain miss) hash table probes would help significantly.  Since code to support Bloom filtering already exists for GNU-style hash tables (DT_GNU_HASH), it seemed attractive to enable that feature for MIPS (and then also benefit from as much of the hash chain optimization as possible). [6]  As I understand it, the fundamental problem which has historically prevented this support is, briefly, that the MIPS ABI requires that the .dynsym table be sorted in a particular order, while .gnu.hash mandates a different order; this appears to have stymied at least one earlier attempt. [8]  As I am expert neither with MIPS and its many ABI oddities, nor with the implementation of the BFD linker, I have opted to take a very, very simple -- if perhaps non-optimal -- approach.  Inspired by the comment made by Richard Sandison to Hiroki Kaminaga concerning external symbol blocks [9], I chose to reuse GNU-style hash tables, only with the simple addition of a translation table to map the GNU symbol order to the MIPS symbol order.  The MIPS .dynsym table proper continues to be completely identical: its sort order and content are entirely unchanged.  The proposed changes also leave the output of all other targets/backends entirely unchanged (including MIPS using traditional SysV .hash).

In an attempt to avoid forward compatibility issues, a new section and related dynamic tag are proposed: .gnu.xhash and DT_GNU_XHASH.  (The "X" standing either for "extended" or as a mnemonic for "translated", as the reader prefers.)  This ensures that old binutils, glibc, and other third party code do not attempt to behave as if a traditional .gnu.hash/DT_GNU_HASH is present.  Likewise, the name of the section was chosen so that it is not a textual prefix of .gnu.hash in an attempt to preclude any insufficiently discriminating pattern from matching inadvertently (e.g. in a script parsing readelf output).

In practice, though, the .gnu.xhash section is virtually identical to .gnu.hash. [9]

	// Part 0: .gnu.xhash header
	Elf32_Word  ngnusyms;  // number of entries in chains (and xlat); dynsymcount=symndx+ngnusyms
	// Part 1: .gnu.hash header
	Elf32_Word  nbuckets;  // number of hash table buckets
	Elf32_Word  symndx;  // number of initial .dynsym entires skipped in chains[] (and xlat[])
	Elf32_Word  maskwords; // number of ElfW(Addr) words in bitmask
	Elf32_Word  shift2;  // bit shift of hashval for second Bloom filter bit
	// Part 2: .gnu.hash Bloom filter
	ElfW(Addr)  bitmask[maskwords];  // 2 bit Bloom filter on hashval
	// Part 3: .gnu.hash buckets
	Elf32_Word  buckets[nbuckets];  // indices into chains[]
	// Part 4: .gnu.hash chains
	Elf32_Word  chains[ngnusyms];  // consecutive hashvals in a given bucket; last entry in chain has LSB set
	// Part 5: .gnu.xhash translation table
	Elf32_Word  xlat[ngnusyms];  // parallel to chains[]; index into .dynsym

Parts 1 through 4 correspond exactly in layout to the traditional .gnu.hash; part 4 has slightly different semantics since the correct MIPS .dynsym index has to be first looked up in the parallel xlat table i.e. the symbol corresponding to the hashval in chain[N] is .dynsym[xlat[N]].  It is intentional that the .gnu.xhash layout contains a .gnu.hash layout as a subcomponent: it represents an attempt to reuse unchanged as much BFD and readelf code as possible.  The space cost of .gnu.xhash relative to .gnu.hash is ngnusyms+1 32 bit words.  (For time cost, the L2 cache hit from the xlat table lookup is probably atrocious, but at that point we're already about to perform a string compare which is probably going to be even more atrocious....  In any case, it's a whole lot better than SysV .hash.)

In order to reuse the maximum amount of existing code with the minimum amount of copying while also maintaining 100% backward compatibility, I chose to implement the support as a BFD ELF backend hook: record_hash_symbol().  For all platforms other than MIPS, this hook is NULL and the behaviour is to write .gnu.hash (or not) exactly as they always have done.  For MIPS, this hook is non-NULL and (when --hash-style=gnu) will output a .gnu.xhash section which the MIPS specific ELF backend then updates with a translation table to allow .gnu.xhash symbol indices to be translated to MIPS .dynsym indices at the right time during linking.  The principal changes to the common support (in bfd/elflink.c) are in bfd_elf_size_dynsym_hash_dynstr() which computes the correct size for the .gnu.[x]hash section; and in elf_renumber_gnu_hash_syms() which did the sorting for .gnu.hash.  On the ELF MIPS specific side, the main changes are to mips_elf_sort_hash_table_f(); and in the addition of the backend function _bfd_mips_elf_record_hash_symbol() with an associated new field in struct mips_elf_link_hash_entry.

In bfd_elf_size_dynsym_hash_dynstr() the code was modified as little as possible in order to keep the diff small and simple to review; the unfortunate corollary to that is that the changes are a little ugly and somewhat brittle (conditionally shifting the contents pointers along etc.)  This also to an extent dictated the layout of the .gnu.xhash section: it is essentially a .gnu.hash section with a leading ngnusyms word and a following translation table.  (The basic form of the .gnu.hash section was retained so as also to keep the readelf changes to a minimum.)

The elf_renumber_gnu_hash_syms() function no longer unconditionally renumbers the symbols.  Instead, if the backend supplies record_hash_symbol(), then that is called instead to allow it to record the offset of the translation table entry for that symbol.  The MIPS backend will then fill this in later once it has finished fiddling with the GOT(s).  I chose to pass an offset here rather than a pointer only because I wasn't entirely certain if it was architecturally acceptable to assume that the content of the section would never be replaced sometime in between (although this is not currently the case).

On the ELF MIPS side, mips_elf_sort_hash_table_f() now also outputs the final index of each symbol into the .gnu.xhash translation table as required.  This is also a bit brittle since it assumes that nothing else is going to come along and change the order yet again afterwards, but that is also true of all of the already existing MIPS backend code.

The readelf support essentially speaks for itself.  Again, it's a bit ugly since I tried to stick only to insertions and not changing existing lines, so as to make the code inspect simpler.

No changes to gold are proposed here because, if I understand correctly, there is as yet no general MIPS support in any case.

I have included the glibc changes here only as a convenience to reviewers; I will be posting to libc-alpha as well.  (It is perhaps interesting to note in passing that although --hash-style=gnu was disabled in the linker, DT_GNU_HASH support was never removed from the glibc MIPS sysdep dl-lookup.c.  This means that on MIPS the new and old hashvals are currently always both computed at runtime for every symbol.  Fortunately in practice this cost appears to be entirely negligible.  Laterally, I suspect that Jakub Jelinek had independently confirmed this insignificance and is why .hashvals never made it into .gnu.hash. [11]  I experimented with adding .hashvals as well as .gnu.xhash, but it made no appreciable difference.)

The patch is relative to binutils-2_24 (which is where I'll ultimately need it) but is equally applicable to trunk.  (The glibc patch is relative to a lightly customized 2.16 but again is equally applicable to trunk.)  As this is my first attempt at a patch for the linker, I've omitted the ChangeLog and test cases for the moment, pending feedback.  Technical and procedural criticism is gratefully welcomed.  I should very much like to thank the many who have taken the time to post on these and related subjects over the years -- I would have found even this modest attempt very difficult without the historical context.  Particular thanks are due to Michael Meeks, Jakub Jelinek, Richard Sandiford, and Hiroki Kaminaga.  Errors in comprehension and judgement are entirely my own.

Regards,
Neil

P.S.  It is not entirely clear to me how xgot support does or should interact with multi-GOT. [12]  With xgot supporting about 2**32 entries, shouldn't it be the case that multiple GOTs are almost never needed?


[1]  Disable .gnu.hash on MIPS targets  https://www.sourceware.org/ml/binutils/2006-07/msg00341.html
[2]  MIPS Multi GOT  https://dmz-portal.mips.com/wiki/MIPS_Multi_GOT
[3]  MIPS multi-got link support  https://www.sourceware.org/ml/binutils/2003-01/msg00165.html
[4]  Description of MIPS/IRIX multigot  https://sourceware.org/ml/binutils/2014-03/msg00174.html
[5]  Speeding up the dynamic linker with 100s of DSOs?  https://www.sourceware.org/ml/libc-alpha/2006-01/msg00018.html
[6]  Re: OO.o / ld / -Bsymbolic patch   https://sourceware.org/ml/binutils/2005-07/msg00109.html
[7]  [PATCH] DT_GNU_HASH: ~ 50% dynamic linking improvement  https://www.sourceware.org/ml/binutils/2006-06/msg00418.html
[8]  [RFC][PATCH][EXPERIMENTAL] enable MIPS gnu hash (was: Re: use of gnu hash for MIPS)  https://sourceware.org/ml/binutils/2007-08/msg00387.html
[9]  Re: use of gnu hash for MIPS  https://sourceware.org/ml/binutils/2007-03/msg00126.html
[10]  Re: GNU_HASH section format  https://sourceware.org/ml/binutils/2006-10/msg00377.html
[11]  binutils/glibc .hashvals section ...  https://sourceware.org/ml/binutils/2006-01/msg00171.html
[12]  Add -mxgot option to mips backend  https://gcc.gnu.org/ml/gcc-patches/2003-08/msg01856.html


[-- Attachment #2: binutils-gnuxhash.patch --]
[-- Type: application/octet-stream, Size: 21107 bytes --]

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index add80b3..5418e1d 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1216,6 +1216,13 @@ struct elf_backend_data
   /* Return TRUE if symbol should be hashed in the `.gnu.hash' section.  */
   bfd_boolean (*elf_hash_symbol) (struct elf_link_hash_entry *);
 
+  /* If non-NULL, called to register the location XLAT_LOC within
+     .gnu.xhash at which real final dynindx for H should be written.
+     If XLAT_LOC is zero, the symbol is not included in
+     .gnu.xhash and no dynindx should be written.  */
+  void (*record_hash_symbol)
+    (struct elf_link_hash_entry *h, bfd_vma xlat_loc);
+
   /* Return TRUE if type is a function symbol type.  */
   bfd_boolean (*is_function_type) (unsigned int type);
 
diff --git a/bfd/elf.c b/bfd/elf.c
index 8df38ee..0526183 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -1309,6 +1309,7 @@ _bfd_elf_print_private_bfd_data (bfd *abfd, void *farg)
 	    case DT_USED: name = "USED"; break;
 	    case DT_FILTER: name = "FILTER"; stringp = TRUE; break;
 	    case DT_GNU_HASH: name = "GNU_HASH"; break;
+	    case DT_GNU_XHASH: name = "GNU_XHASH"; break;
 	    }
 
 	  fprintf (f, "  %-20s ", name);
@@ -1575,6 +1576,7 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex)
     case SHT_PREINIT_ARRAY:	/* .preinit_array section.  */
     case SHT_GNU_LIBLIST:	/* .gnu.liblist section.  */
     case SHT_GNU_HASH:		/* .gnu.hash section.  */
+    case SHT_GNU_XHASH:		/* .gnu.xhash section.  */
       return _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex);
 
     case SHT_DYNAMIC:	/* Dynamic linking information.  */
@@ -2102,6 +2104,7 @@ static const struct bfd_elf_special_section special_sections_g[] =
   { STRING_COMMA_LEN (".gnu.liblist"),     0, SHT_GNU_LIBLIST, SHF_ALLOC },
   { STRING_COMMA_LEN (".gnu.conflict"),    0, SHT_RELA,        SHF_ALLOC },
   { STRING_COMMA_LEN (".gnu.hash"),        0, SHT_GNU_HASH,    SHF_ALLOC },
+  { STRING_COMMA_LEN (".gnu.xhash"),       0, SHT_GNU_XHASH,   SHF_ALLOC },
   { NULL,                        0,        0, 0,               0 }
 };
 
@@ -2714,6 +2717,7 @@ elf_fake_sections (bfd *abfd, asection *asect, void *fsarg)
       break;
 
     case SHT_GNU_HASH:
+    case SHT_GNU_XHASH:
       this_hdr->sh_entsize = bed->s->arch_size == 64 ? 0 : 4;
       break;
     }
@@ -3220,6 +3224,7 @@ assign_section_numbers (bfd *abfd, struct bfd_link_info *link_info)
 
 	case SHT_HASH:
 	case SHT_GNU_HASH:
+	case SHT_GNU_XHASH:
 	case SHT_GNU_versym:
 	  /* sh_link is the section header index of the symbol table
 	     this hash table or version table is for.  */
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 99b7ca1..1e3e2db 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -271,8 +271,12 @@ _bfd_elf_link_create_dynamic_sections (bfd *abfd, struct bfd_link_info *info)
 
   if (info->emit_gnu_hash)
     {
-      s = bfd_make_section_anyway_with_flags (abfd, ".gnu.hash",
-					      flags | SEC_READONLY);
+      if (bed->record_hash_symbol == NULL)
+	s = bfd_make_section_anyway_with_flags (abfd, ".gnu.hash",
+						flags | SEC_READONLY);
+      else
+	s = bfd_make_section_anyway_with_flags (abfd, ".gnu.xhash",
+						flags | SEC_READONLY);
       if (s == NULL
 	  || ! bfd_set_section_alignment (abfd, s, bed->s->log_file_align))
 	return FALSE;
@@ -5199,6 +5203,7 @@ struct collect_gnu_hash_codes
   unsigned long int *counts;
   bfd_vma *bitmask;
   bfd_byte *contents;
+  bfd_vma xlat;
   long int min_dynindx;
   unsigned long int bucketcount;
   unsigned long int symindx;
@@ -5278,7 +5283,15 @@ elf_renumber_gnu_hash_syms (struct elf_link_hash_entry *h, void *data)
   if (! (*s->bed->elf_hash_symbol) (h))
     {
       if (h->dynindx >= s->min_dynindx)
-	h->dynindx = s->local_indx++;
+	{
+	  if (s->bed->record_hash_symbol != NULL)
+	    {
+	      (*s->bed->record_hash_symbol) (h, 0);
+	      ++s->local_indx;
+	    }
+	  else
+	    h->dynindx = s->local_indx++;
+	}
       return TRUE;
     }
 
@@ -5295,7 +5308,14 @@ elf_renumber_gnu_hash_syms (struct elf_link_hash_entry *h, void *data)
   bfd_put_32 (s->output_bfd, val,
 	      s->contents + (s->indx[bucket] - s->symindx) * 4);
   --s->counts[bucket];
-  h->dynindx = s->indx[bucket]++;
+  if (s->bed->record_hash_symbol != NULL)
+    {
+      bfd_vma xlat_loc = s->xlat + (s->indx[bucket]++ - s->symindx) * 4;
+      BFD_ASSERT (xlat_loc != 0);
+      (*s->bed->record_hash_symbol) (h, xlat_loc);
+    }
+  else
+    h->dynindx = s->indx[bucket]++;
   return TRUE;
 }
 
@@ -5917,7 +5937,9 @@ bfd_elf_size_dynamic_sections (bfd *output_bfd,
 	  if ((info->emit_hash
 	       && !_bfd_elf_add_dynamic_entry (info, DT_HASH, 0))
 	      || (info->emit_gnu_hash
-		  && !_bfd_elf_add_dynamic_entry (info, DT_GNU_HASH, 0))
+		  && (bed->record_hash_symbol == NULL
+		      ? !_bfd_elf_add_dynamic_entry (info, DT_GNU_HASH, 0)
+		      : !_bfd_elf_add_dynamic_entry (info, DT_GNU_XHASH, 0)))
 	      || !_bfd_elf_add_dynamic_entry (info, DT_STRTAB, 0)
 	      || !_bfd_elf_add_dynamic_entry (info, DT_SYMTAB, 0)
 	      || !_bfd_elf_add_dynamic_entry (info, DT_STRSZ, strsize)
@@ -6535,19 +6557,30 @@ bfd_elf_size_dynsym_hash_dynstr (bfd *output_bfd, struct bfd_link_info *info)
 	      return FALSE;
 	    }
 
-	  s = bfd_get_linker_section (dynobj, ".gnu.hash");
+	  if (bed->record_hash_symbol == NULL)
+	    s = bfd_get_linker_section (dynobj, ".gnu.hash");
+	  else
+	    s = bfd_get_linker_section (dynobj, ".gnu.xhash");
 	  BFD_ASSERT (s != NULL);
 
 	  if (cinfo.nsyms == 0)
 	    {
-	      /* Empty .gnu.hash section is special.  */
+	      /* Empty .gnu.hash or .gnu.xhash section is special.  */
 	      BFD_ASSERT (cinfo.min_dynindx == -1);
 	      free (cinfo.hashcodes);
 	      s->size = 5 * 4 + bed->s->arch_size / 8;
+	      if (bed->record_hash_symbol != NULL)
+		s->size += 4;
 	      contents = (unsigned char *) bfd_zalloc (output_bfd, s->size);
 	      if (contents == NULL)
 		return FALSE;
 	      s->contents = contents;
+	      if (bed->record_hash_symbol != NULL)
+		{
+		  /* Chain and xlat symbol count is zero.  */
+		  bfd_put_32 (output_bfd, 0, contents);
+		  contents += 4;
+		}
 	      /* 1 empty bucket.  */
 	      bfd_put_32 (output_bfd, 1, contents);
 	      /* SYMIDX above the special symbol 0.  */
@@ -6620,6 +6653,8 @@ bfd_elf_size_dynsym_hash_dynstr (bfd *output_bfd, struct bfd_link_info *info)
 
 	      s->size = (4 + bucketcount + cinfo.nsyms) * 4;
 	      s->size += cinfo.maskbits / 8;
+	      if (bed->record_hash_symbol != NULL)
+		s->size += (1 + cinfo.nsyms) * 4;
 	      contents = (unsigned char *) bfd_zalloc (output_bfd, s->size);
 	      if (contents == NULL)
 		{
@@ -6629,6 +6664,11 @@ bfd_elf_size_dynsym_hash_dynstr (bfd *output_bfd, struct bfd_link_info *info)
 		}
 
 	      s->contents = contents;
+	      if (bed->record_hash_symbol != NULL)
+		{
+		  bfd_put_32 (output_bfd, cinfo.nsyms, contents);
+		  contents += 4;
+		}
 	      bfd_put_32 (output_bfd, bucketcount, contents);
 	      bfd_put_32 (output_bfd, cinfo.symindx, contents + 4);
 	      bfd_put_32 (output_bfd, maskwords, contents + 8);
@@ -6645,12 +6685,15 @@ bfd_elf_size_dynsym_hash_dynstr (bfd *output_bfd, struct bfd_link_info *info)
 		}
 
 	      cinfo.contents = contents;
-
+	      if (bed->record_hash_symbol != NULL)
+		cinfo.xlat = contents + cinfo.nsyms * 4 - s->contents;
 	      /* Renumber dynamic symbols, populate .gnu.hash section.  */
 	      elf_link_hash_traverse (elf_hash_table (info),
 				      elf_renumber_gnu_hash_syms, &cinfo);
 
 	      contents = s->contents + 16;
+	      if (bed->record_hash_symbol != NULL)
+		contents += 4;
 	      for (i = 0; i < maskwords; ++i)
 		{
 		  bfd_put (bed->s->arch_size, output_bfd, cinfo.bitmask[i],
@@ -11301,6 +11344,9 @@ bfd_elf_final_link (bfd *abfd, struct bfd_link_info *info)
 	    case DT_GNU_HASH:
 	      name = ".gnu.hash";
 	      goto get_vma;
+	    case DT_GNU_XHASH:
+	      name = ".gnu.xhash";
+	      goto get_vma;
 	    case DT_STRTAB:
 	      name = ".dynstr";
 	      goto get_vma;
diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
index d7498e1..d286a62 100644
--- a/bfd/elfxx-mips.c
+++ b/bfd/elfxx-mips.c
@@ -315,6 +315,11 @@ struct mips_elf_hash_sort_data
   /* The greatest dynamic symbol table index not corresponding to a
      symbol without a GOT entry.  */
   long max_non_got_dynindx;
+  /* If non-NULL, output BFD for .gnu.xhash finalization.  */
+  bfd *output_bfd;
+  /* If non-NULL, pointer to contents of .gnu.xhash for filling in
+     real final dynindx.  */
+  bfd_byte *gnuxhash;
 };
 
 /* We make up to two PLT entries if needed, one for standard MIPS code
@@ -372,6 +377,9 @@ struct mips_elf_link_hash_entry
      being called returns a floating point value.  */
   asection *call_fp_stub;
 
+  /* If non-zero, location in .gnu.xhash to write real final dynindx.  */
+  bfd_vma gnuxhash_loc;
+
   /* The highest GGA_* value that satisfies all references to this symbol.  */
   unsigned int global_got_area : 2;
 
@@ -1246,6 +1254,7 @@ mips_elf_link_hash_newfunc (struct bfd_hash_entry *entry,
       ret->fn_stub = NULL;
       ret->call_stub = NULL;
       ret->call_fp_stub = NULL;
+      ret->gnuxhash_loc = 0;
       ret->global_got_area = GGA_NONE;
       ret->got_only_for_calls = TRUE;
       ret->readonly_reloc = FALSE;
@@ -3725,6 +3734,18 @@ mips_elf_sort_hash_table (bfd *abfd, struct bfd_link_info *info)
     = hsd.min_got_dynindx
     = (elf_hash_table (info)->dynsymcount - g->reloc_only_gotno);
   hsd.max_non_got_dynindx = count_section_dynsyms (abfd, info) + 1;
+  hsd.output_bfd = abfd;
+  if (htab->root.dynobj != NULL
+      && htab->root.dynamic_sections_created
+      && info->emit_gnu_hash)
+    {
+      asection *s = bfd_get_linker_section (htab->root.dynobj, ".gnu.xhash");
+      BFD_ASSERT (s != NULL);
+      hsd.gnuxhash = s->contents;
+      BFD_ASSERT (hsd.gnuxhash != NULL);
+    }
+  else
+    hsd.gnuxhash = NULL;
   mips_elf_link_hash_traverse (((struct mips_elf_link_hash_table *)
 				elf_hash_table (info)),
 			       mips_elf_sort_hash_table_f,
@@ -3777,6 +3798,12 @@ mips_elf_sort_hash_table_f (struct mips_elf_link_hash_entry *h, void *data)
       break;
     }
 
+  if (h->gnuxhash_loc != 0 && hsd->gnuxhash != NULL)
+    {
+      bfd_put_32 (hsd->output_bfd, h->root.dynindx,
+		  hsd->gnuxhash + h->gnuxhash_loc);
+    }
+
   return TRUE;
 }
 
@@ -15285,3 +15312,10 @@ _bfd_mips_post_process_headers (bfd *abfd, struct bfd_link_info *link_info)
 	i_ehdrp->e_ident[EI_ABIVERSION] = 1;
     }
 }
+
+void
+_bfd_mips_elf_record_hash_symbol (struct elf_link_hash_entry *h, bfd_vma xlat_loc)
+{
+  struct mips_elf_link_hash_entry *hmips = (struct mips_elf_link_hash_entry *) h;
+  hmips->gnuxhash_loc = xlat_loc;
+}
diff --git a/bfd/elfxx-mips.h b/bfd/elfxx-mips.h
index f27dc15..f474f81 100644
--- a/bfd/elfxx-mips.h
+++ b/bfd/elfxx-mips.h
@@ -158,6 +158,8 @@ extern long _bfd_mips_elf_get_synthetic_symtab
   (bfd *, long, asymbol **, long, asymbol **, asymbol **);
 extern void _bfd_mips_post_process_headers
   (bfd *abfd, struct bfd_link_info *link_info);
+extern void _bfd_mips_elf_record_hash_symbol
+  (struct elf_link_hash_entry *h, bfd_vma xlat_loc);
 
 extern const struct bfd_elf_special_section _bfd_mips_elf_special_sections [];
 
@@ -186,3 +188,4 @@ literal_reloc_p (int r_type)
 #define elf_backend_merge_symbol_attribute  _bfd_mips_elf_merge_symbol_attribute
 #define elf_backend_ignore_undef_symbol _bfd_mips_elf_ignore_undef_symbol
 #define elf_backend_post_process_headers _bfd_mips_post_process_headers
+#define elf_backend_record_hash_symbol _bfd_mips_elf_record_hash_symbol
diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h
index d42ce26..fd20943 100644
--- a/bfd/elfxx-target.h
+++ b/bfd/elfxx-target.h
@@ -656,6 +656,10 @@
 #define elf_backend_hash_symbol _bfd_elf_hash_symbol
 #endif
 
+#ifndef elf_backend_record_hash_symbol
+#define elf_backend_record_hash_symbol NULL
+#endif
+
 #ifndef elf_backend_is_function_type
 #define elf_backend_is_function_type _bfd_elf_is_function_type
 #endif
@@ -758,6 +762,7 @@ static struct elf_backend_data elfNN_bed =
   elf_backend_common_section,
   elf_backend_merge_symbol,
   elf_backend_hash_symbol,
+  elf_backend_record_hash_symbol,
   elf_backend_is_function_type,
   elf_backend_maybe_function_sym,
   elf_backend_link_order_error_handler,
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 61ea0ad..e87b111 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -181,6 +181,7 @@ static unsigned int dynamic_syminfo_nent;
 static char program_interpreter[PATH_MAX];
 static bfd_vma dynamic_info[DT_ENCODING];
 static bfd_vma dynamic_info_DT_GNU_HASH;
+static bfd_vma dynamic_info_DT_GNU_XHASH;
 static bfd_vma version_info[16];
 static Elf_Internal_Ehdr elf_header;
 static Elf_Internal_Shdr * section_headers;
@@ -1805,6 +1806,7 @@ get_dynamic_type (unsigned long type)
     case DT_GNU_LIBLIST: return "GNU_LIBLIST";
     case DT_GNU_LIBLISTSZ: return "GNU_LIBLISTSZ";
     case DT_GNU_HASH:	return "GNU_HASH";
+    case DT_GNU_XHASH:	return "GNU_XHASH";
 
     default:
       if ((type >= DT_LOPROC) && (type <= DT_HIPROC))
@@ -3302,6 +3304,7 @@ get_section_type_name (unsigned int sh_type)
     case SHT_FINI_ARRAY:	return "FINI_ARRAY";
     case SHT_PREINIT_ARRAY:	return "PREINIT_ARRAY";
     case SHT_GNU_HASH:		return "GNU_HASH";
+    case SHT_GNU_XHASH:		return "GNU_XHASH";
     case SHT_GROUP:		return "GROUP";
     case SHT_SYMTAB_SHNDX:	return "SYMTAB SECTION INDICIES";
     case SHT_GNU_verdef:	return "VERDEF";
@@ -8454,6 +8457,16 @@ process_dynamic_section (FILE * file)
 	    }
 	  break;
 
+	case DT_GNU_XHASH:
+	  dynamic_info_DT_GNU_XHASH = entry->d_un.d_val;
+	  dynamic_info_DT_GNU_HASH = dynamic_info_DT_GNU_XHASH + 4;
+	  if (do_dynamic)
+	    {
+	      print_vma (entry->d_un.d_val, PREFIX_HEX);
+	      putchar ('\n');
+	    }
+	  break;
+
 	default:
 	  if ((entry->d_tag >= DT_VERSYM) && (entry->d_tag <= DT_VERNEEDNUM))
 	    version_info[DT_VERSIONTAGIDX (entry->d_tag)] =
@@ -9380,6 +9393,7 @@ process_symbol_table (FILE * file)
   bfd_vma ngnubuckets = 0;
   bfd_vma * gnubuckets = NULL;
   bfd_vma * gnuchains = NULL;
+  bfd_vma * gnuxlat = NULL;
   bfd_vma gnusymidx = 0;
 
   if (!do_syms && !do_dyn_syms && !do_histogram)
@@ -9510,7 +9524,7 @@ process_symbol_table (FILE * file)
       if (fseek (file,
 		 (archive_file_offset
 		  + offset_from_vma (file, buckets_vma
-					   + 4 * (ngnubuckets + maxchain), 4)),
+				     + 4 * (ngnubuckets + maxchain), 4)),
 		 SEEK_SET))
 	{
 	  error (_("Unable to seek to start of dynamic information\n"));
@@ -9532,6 +9546,30 @@ process_symbol_table (FILE * file)
 	}
       while ((byte_get (nb, 4) & 1) == 0);
 
+      if (dynamic_info_DT_GNU_XHASH)
+	{
+	  if (fseek (file,
+		     (archive_file_offset
+		      + offset_from_vma (file, dynamic_info_DT_GNU_XHASH, 4)),
+		     SEEK_SET))
+	    {
+	      error (_("Unable to seek to start of dynamic information\n"));
+	      goto no_gnu_hash;
+	    }
+
+	  if (fread (nb, 4, 1, file) != 1)
+	    {
+	      error (_("Failed to determine last chain length\n"));
+	      goto no_gnu_hash;
+	    }
+
+	  if (maxchain != byte_get (nb, 4))
+	    {
+	      error (_("Failed to determine last chain length\n"));
+	      goto no_gnu_hash;
+	    }
+	}
+
       if (fseek (file,
 		 (archive_file_offset
 		  + offset_from_vma (file, buckets_vma + 4 * ngnubuckets, 4)),
@@ -9543,7 +9581,45 @@ process_symbol_table (FILE * file)
 
       gnuchains = get_dynamic_data (file, maxchain, 4);
 
+      if (gnuchains == NULL)
+	goto no_gnu_hash;
+
+      if (dynamic_info_DT_GNU_XHASH)
+	{
+	  if (fseek (file,
+		     (archive_file_offset
+		      + offset_from_vma (file, dynamic_info_DT_GNU_XHASH, 4)),
+		     SEEK_SET))
+	    {
+	      error (_("Unable to seek to start of dynamic information\n"));
+	      goto no_gnu_hash;
+	    }
+
+	  if (fread (nb, 4, 1, file) != 1)
+	    {
+	      error (_("Failed to read in number of buckets\n"));
+	      goto no_gnu_hash;
+	    }
+	  if (fseek (file,
+		     (archive_file_offset
+		      + offset_from_vma (file, buckets_vma
+					       + 4 * (ngnubuckets
+						      + maxchain), 4)),
+		     SEEK_SET))
+	    {
+	      error (_("Unable to seek to start of dynamic information\n"));
+	      goto no_gnu_hash;
+	    }
+
+	  gnuxlat = get_dynamic_data (file, maxchain, 4);
+	}
+
     no_gnu_hash:
+      if (dynamic_info_DT_GNU_XHASH && gnuxlat == NULL)
+	{
+	  free (gnuchains);
+	  gnuchains = NULL;
+	}
       if (gnuchains == NULL)
 	{
 	  free (gnubuckets);
@@ -9583,7 +9659,8 @@ process_symbol_table (FILE * file)
 
       if (dynamic_info_DT_GNU_HASH)
 	{
-	  printf (_("\nSymbol table of `.gnu.hash' for image:\n"));
+	  printf (_("\nSymbol table of `%s' for image:\n"),
+		  !dynamic_info_DT_GNU_XHASH ? ".gnu.hash" : ".gnu.xhash");
 	  if (is_32bit_elf)
 	    printf (_("  Num Buc:    Value  Size   Type   Bind Vis      Ndx Name\n"));
 	  else
@@ -9597,7 +9674,10 @@ process_symbol_table (FILE * file)
 
 		do
 		  {
-		    print_dynamic_symbol (si, hn);
+		    if (!dynamic_info_DT_GNU_XHASH)
+		      print_dynamic_symbol (si, hn);
+		    else
+		      print_dynamic_symbol (gnuxlat[off], hn);
 		    si++;
 		  }
 		while ((gnuchains[off++] & 1) == 0);
@@ -9931,7 +10011,8 @@ process_symbol_table (FILE * file)
 	  return 0;
 	}
 
-      printf (_("\nHistogram for `.gnu.hash' bucket list length (total of %lu buckets):\n"),
+      printf (_("\nHistogram for `%s' bucket list length (total of %lu buckets):\n"),
+	      !dynamic_info_DT_GNU_XHASH ? ".gnu.hash" : ".gnu.xhash",
 	      (unsigned long) ngnubuckets);
       printf (_(" Length  Number     %% of total  Coverage\n"));
 
@@ -9977,6 +10058,8 @@ process_symbol_table (FILE * file)
       free (lengths);
       free (gnubuckets);
       free (gnuchains);
+      free (gnuxlat);
+
     }
 
   return 1;
@@ -13913,6 +13996,7 @@ process_object (char * file_name, FILE * file)
   for (i = ARRAY_SIZE (dynamic_info); i--;)
     dynamic_info[i] = 0;
   dynamic_info_DT_GNU_HASH = 0;
+  dynamic_info_DT_GNU_XHASH = 0;
 
   /* Process the file.  */
   if (show_name)
diff --git a/include/elf/common.h b/include/elf/common.h
index cd3bcdd..7cdbdeb 100644
--- a/include/elf/common.h
+++ b/include/elf/common.h
@@ -472,6 +472,7 @@
 #define SHT_HIOS	0x6fffffff	/* Last of OS specific semantics */
 
 #define SHT_GNU_INCREMENTAL_INPUTS 0x6fff4700   /* incremental build data */
+#define SHT_GNU_XHASH	0x6ffffff4	/* GNU style symbol hash table with xlat */
 #define SHT_GNU_ATTRIBUTES 0x6ffffff5	/* Object attributes */
 #define SHT_GNU_HASH	0x6ffffff6	/* GNU style symbol hash table */
 #define SHT_GNU_LIBLIST	0x6ffffff7	/* List of prelink dependencies */
@@ -775,6 +776,7 @@
 #define DT_VALRNGHI	0x6ffffdff
 
 #define DT_ADDRRNGLO	0x6ffffe00
+#define DT_GNU_XHASH	0x6ffffef4
 #define DT_GNU_HASH	0x6ffffef5
 #define DT_TLSDESC_PLT	0x6ffffef6
 #define DT_TLSDESC_GOT	0x6ffffef7
diff --git a/ld/emultempl/mipself.em b/ld/emultempl/mipself.em
index 3c6ec9f..e7d4a16 100644
--- a/ld/emultempl/mipself.em
+++ b/ld/emultempl/mipself.em
@@ -35,21 +35,6 @@ static bfd *stub_bfd;
 
 static bfd_boolean insn32;
 
-static void
-mips_after_parse (void)
-{
-  /* .gnu.hash and the MIPS ABI require .dynsym to be sorted in different
-     ways.  .gnu.hash needs symbols to be grouped by hash code whereas the
-     MIPS ABI requires a mapping between the GOT and the symbol table.  */
-  if (link_info.emit_gnu_hash)
-    {
-      einfo ("%X%P: .gnu.hash is incompatible with the MIPS ABI\n");
-      link_info.emit_hash = TRUE;
-      link_info.emit_gnu_hash = FALSE;
-    }
-  after_parse_default ();
-}
-
 struct hook_stub_info
 {
   lang_statement_list_type add;
@@ -281,6 +266,5 @@ PARSE_AND_LIST_ARGS_CASES='
       break;
 '
 
-LDEMUL_AFTER_PARSE=mips_after_parse
 LDEMUL_BEFORE_ALLOCATION=mips_before_allocation
 LDEMUL_CREATE_OUTPUT_SECTION_STATEMENTS=mips_create_output_section_statements
diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
index e8126cb..2d89ab1 100644
--- a/ld/scripttempl/elf.sc
+++ b/ld/scripttempl/elf.sc
@@ -353,6 +353,7 @@ cat > ldscripts/dyntmp.$$ <<EOF
   ${TEXT_DYNAMIC+${DYNAMIC}}
   .hash         ${RELOCATING-0} : { *(.hash) }
   .gnu.hash     ${RELOCATING-0} : { *(.gnu.hash) }
+  .gnu.xhash    ${RELOCATING-0} : { *(.gnu.xhash) }
   .dynsym       ${RELOCATING-0} : { *(.dynsym) }
   .dynstr       ${RELOCATING-0} : { *(.dynstr) }
   .gnu.version  ${RELOCATING-0} : { *(.gnu.version) }
diff --git a/ld/testsuite/ld-mips-elf/hash1b.d b/ld/testsuite/ld-mips-elf/hash1b.d
index 5af9037..6836ba9 100644
--- a/ld/testsuite/ld-mips-elf/hash1b.d
+++ b/ld/testsuite/ld-mips-elf/hash1b.d
@@ -1,3 +1,4 @@
 #source: hash1.s
 #ld: -shared --hash-style=both
-#error: .gnu.hash is incompatible with the MIPS ABI
+#objdump: -dr
+#pass
diff --git a/ld/testsuite/ld-mips-elf/hash1c.d b/ld/testsuite/ld-mips-elf/hash1c.d
index 09bff3c..72bdc18 100644
--- a/ld/testsuite/ld-mips-elf/hash1c.d
+++ b/ld/testsuite/ld-mips-elf/hash1c.d
@@ -1,3 +1,4 @@
 #source: hash1.s
 #ld: -shared --hash-style=gnu
-#error: .gnu.hash is incompatible with the MIPS ABI
+#objdump: -dr
+#pass

[-- Attachment #3: glibc-gnuxhash.patch --]
[-- Type: application/octet-stream, Size: 9544 bytes --]

Index: glibc-2.16/elf/dynamic-link.h
===================================================================
--- glibc-2.16.orig/elf/dynamic-link.h
+++ glibc-2.16/elf/dynamic-link.h
@@ -172,6 +172,8 @@ elf_get_dynamic_info (struct link_map *l
       ADJUST_DYN_INFO (VERSYMIDX (DT_VERSYM));
       ADJUST_DYN_INFO (DT_ADDRTAGIDX (DT_GNU_HASH) + DT_NUM + DT_THISPROCNUM
 		       + DT_VERSIONTAGNUM + DT_EXTRANUM + DT_VALNUM);
+      ADJUST_DYN_INFO (DT_ADDRTAGIDX (DT_GNU_XHASH) + DT_NUM + DT_THISPROCNUM
+		       + DT_VERSIONTAGNUM + DT_EXTRANUM + DT_VALNUM);
 # undef ADJUST_DYN_INFO
       assert (cnt <= DL_RO_DYN_TEMP_CNT);
     }
Index: glibc-2.16/elf/elf.h
===================================================================
--- glibc-2.16.orig/elf/elf.h
+++ glibc-2.16/elf/elf.h
@@ -334,6 +334,7 @@ typedef struct
 #define SHT_SYMTAB_SHNDX  18		/* Extended section indeces */
 #define	SHT_NUM		  19		/* Number of defined types.  */
 #define SHT_LOOS	  0x60000000	/* Start OS-specific.  */
+#define SHT_GNU_XHASH	  0x6ffffff4	/* GNU-style hash table with xlat.  */
 #define SHT_GNU_ATTRIBUTES 0x6ffffff5	/* Object attributes.  */
 #define SHT_GNU_HASH	  0x6ffffff6	/* GNU-style hash table.  */
 #define SHT_GNU_LIBLIST	  0x6ffffff7	/* Prelink library list */
@@ -720,6 +721,7 @@ typedef struct
    If any adjustment is made to the ELF object after it has been
    built these entries will need to be adjusted.  */
 #define DT_ADDRRNGLO	0x6ffffe00
+#define DT_GNU_XHASH	0x6ffffef4	/* GNU-style hash table with xlat.  */
 #define DT_GNU_HASH	0x6ffffef5	/* GNU-style hash table.  */
 #define DT_TLSDESC_PLT	0x6ffffef6
 #define DT_TLSDESC_GOT	0x6ffffef7
@@ -733,7 +735,7 @@ typedef struct
 #define DT_SYMINFO	0x6ffffeff	/* Syminfo table.  */
 #define DT_ADDRRNGHI	0x6ffffeff
 #define DT_ADDRTAGIDX(tag)	(DT_ADDRRNGHI - (tag))	/* Reverse order! */
-#define DT_ADDRNUM 11
+#define DT_ADDRNUM 12
 
 /* The versioning entry types.  The next are defined as part of the
    GNU extension.  */
Index: glibc-2.16/ports/sysdeps/mips/dl-addr.c
===================================================================
--- /dev/null
+++ glibc-2.16/ports/sysdeps/mips/dl-addr.c
@@ -0,0 +1,157 @@
+/* Locate the shared object symbol nearest a given address.
+   Copyright (C) 1996-2007, 2009, 2011 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <dlfcn.h>
+#include <stddef.h>
+#include <ldsodefs.h>
+
+
+static inline void
+__attribute ((always_inline))
+determine_info (const ElfW(Addr) addr, struct link_map *match, Dl_info *info,
+		struct link_map **mapp, const ElfW(Sym) **symbolp)
+{
+  /* Now we know what object the address lies in.  */
+  info->dli_fname = match->l_name;
+  info->dli_fbase = (void *) match->l_map_start;
+
+  /* If this is the main program the information is incomplete.  */
+  if (__builtin_expect (match->l_name[0], 'a') == '\0'
+      && match->l_type == lt_executable)
+    info->dli_fname = _dl_argv[0];
+
+  const ElfW(Sym) *symtab
+    = (const ElfW(Sym) *) D_PTR (match, l_info[DT_SYMTAB]);
+  const char *strtab = (const char *) D_PTR (match, l_info[DT_STRTAB]);
+
+  ElfW(Word) strtabsize = match->l_info[DT_STRSZ]->d_un.d_val;
+
+  const ElfW(Sym) *matchsym = NULL;
+  if (match->l_info[DT_ADDRTAGIDX (DT_GNU_XHASH) + DT_NUM + DT_THISPROCNUM
+		    + DT_VERSIONTAGNUM + DT_EXTRANUM + DT_VALNUM] != NULL)
+    {
+      const Elf32_Word * const xhash32
+	= (void *) D_PTR (match, l_info[DT_ADDRTAGIDX (DT_GNU_XHASH) + DT_NUM
+					+ DT_THISPROCNUM + DT_VERSIONTAGNUM
+					+ DT_EXTRANUM + DT_VALNUM]);
+      const Elf32_Word ngnusyms = xhash32[0];
+      const Elf32_Word symbias = xhash32[2];
+      for (Elf32_Word symndx = symbias; symndx < symbias + ngnusyms; ++symndx)
+	{
+	  /* The hash table never references local symbols so
+	     we can omit that test here.  */
+	  if ((symtab[symndx].st_shndx != SHN_UNDEF
+	       || symtab[symndx].st_value != 0)
+	      && ELFW(ST_TYPE) (symtab[symndx].st_info) != STT_TLS
+	      && DL_ADDR_SYM_MATCH (match, &symtab[symndx],
+				    matchsym, addr)
+	      && symtab[symndx].st_name < strtabsize)
+	    matchsym = (ElfW(Sym) *) &symtab[symndx];
+	}
+    }
+  else
+    {
+      const ElfW(Sym) *symtabend;
+      if (match->l_info[DT_HASH] != NULL)
+	symtabend = (symtab
+		     + ((Elf_Symndx *) D_PTR (match, l_info[DT_HASH]))[1]);
+      else
+	/* There is no direct way to determine the number of symbols in the
+	   dynamic symbol table and no hash table is present.  The ELF
+	   binary is ill-formed but what shall we do?  Use the beginning of
+	   the string table which generally follows the symbol table.  */
+	symtabend = (const ElfW(Sym) *) strtab;
+
+      for (; (void *) symtab < (void *) symtabend; ++symtab)
+	if ((ELFW(ST_BIND) (symtab->st_info) == STB_GLOBAL
+	     || ELFW(ST_BIND) (symtab->st_info) == STB_WEAK)
+	    && ELFW(ST_TYPE) (symtab->st_info) != STT_TLS
+	    && (symtab->st_shndx != SHN_UNDEF
+		|| symtab->st_value != 0)
+	    && DL_ADDR_SYM_MATCH (match, symtab, matchsym, addr)
+	    && symtab->st_name < strtabsize)
+	  matchsym = (ElfW(Sym) *) symtab;
+    }
+
+  if (mapp)
+    *mapp = match;
+  if (symbolp)
+    *symbolp = matchsym;
+
+  if (matchsym)
+    {
+      /* We found a symbol close by.  Fill in its name and exact
+	 address.  */
+      lookup_t matchl = LOOKUP_VALUE (match);
+
+      info->dli_sname = strtab + matchsym->st_name;
+      info->dli_saddr = DL_SYMBOL_ADDRESS (matchl, matchsym);
+    }
+  else
+    {
+      /* No symbol matches.  We return only the containing object.  */
+      info->dli_sname = NULL;
+      info->dli_saddr = NULL;
+    }
+}
+
+
+int
+internal_function
+_dl_addr (const void *address, Dl_info *info,
+	  struct link_map **mapp, const ElfW(Sym) **symbolp)
+{
+  const ElfW(Addr) addr = DL_LOOKUP_ADDRESS (address);
+  int result = 0;
+
+  /* Protect against concurrent loads and unloads.  */
+  __rtld_lock_lock_recursive (GL(dl_load_lock));
+
+  /* Find the highest-addressed object that ADDRESS is not below.  */
+  for (Lmid_t ns = 0; ns < GL(dl_nns); ++ns)
+    for (struct link_map *l = GL(dl_ns)[ns]._ns_loaded; l; l = l->l_next)
+      if (addr >= l->l_map_start && addr < l->l_map_end
+	  && (l->l_contiguous || _dl_addr_inside_object (l, addr)))
+	{
+	  determine_info (addr, l, info, mapp, symbolp);
+	  result = 1;
+	  goto out;
+	}
+
+ out:
+  __rtld_lock_unlock_recursive (GL(dl_load_lock));
+
+  return result;
+}
+libc_hidden_def (_dl_addr)
+
+/* Return non-zero if ADDR lies within one of L's segments.  */
+int
+internal_function
+_dl_addr_inside_object (struct link_map *l, const ElfW(Addr) addr)
+{
+  int n = l->l_phnum;
+  const ElfW(Addr) reladdr = addr - l->l_addr;
+
+  while (--n >= 0)
+    if (l->l_phdr[n].p_type == PT_LOAD
+	&& reladdr - l->l_phdr[n].p_vaddr >= 0
+	&& reladdr - l->l_phdr[n].p_vaddr < l->l_phdr[n].p_memsz)
+      return 1;
+  return 0;
+}
Index: glibc-2.16/ports/sysdeps/mips/dl-lookup.c
===================================================================
--- glibc-2.16.orig/ports/sysdeps/mips/dl-lookup.c
+++ glibc-2.16/ports/sysdeps/mips/dl-lookup.c
@@ -262,7 +262,9 @@ do_lookup_x (const char *undef_name, uin
 		  do
 		    if (((*hasharr ^ new_hash) >> 1) == 0)
 		      {
-			symidx = hasharr - map->l_gnu_chain_zero;
+			symidx =
+			  map->l_mach.gnu_xlat_zero[hasharr -
+						    map->l_gnu_chain_zero];
 			sym = check_match (&symtab[symidx]);
 			if (sym != NULL)
 			  goto found_it;
@@ -878,14 +880,15 @@ _dl_setup_hash (struct link_map *map)
 {
   Elf_Symndx *hash;
 
-  if (__builtin_expect (map->l_info[DT_ADDRTAGIDX (DT_GNU_HASH) + DT_NUM
+  if (__builtin_expect (map->l_info[DT_ADDRTAGIDX (DT_GNU_XHASH) + DT_NUM
 				    + DT_THISPROCNUM + DT_VERSIONTAGNUM
 				    + DT_EXTRANUM + DT_VALNUM] != NULL, 1))
     {
       Elf32_Word *hash32
-	= (void *) D_PTR (map, l_info[DT_ADDRTAGIDX (DT_GNU_HASH) + DT_NUM
+	= (void *) D_PTR (map, l_info[DT_ADDRTAGIDX (DT_GNU_XHASH) + DT_NUM
 				      + DT_THISPROCNUM + DT_VERSIONTAGNUM
 				      + DT_EXTRANUM + DT_VALNUM]);
+      Elf32_Word ngnusyms = *hash32++;
       map->l_nbuckets = *hash32++;
       Elf32_Word symbias = *hash32++;
       Elf32_Word bitmask_nwords = *hash32++;
@@ -900,6 +903,8 @@ _dl_setup_hash (struct link_map *map)
       map->l_gnu_buckets = hash32;
       hash32 += map->l_nbuckets;
       map->l_gnu_chain_zero = hash32 - symbias;
+      hash32 += ngnusyms;
+      map->l_mach.gnu_xlat_zero = hash32 - symbias;
       return;
     }
 
Index: glibc-2.16/ports/sysdeps/mips/bits/linkmap.h
===================================================================
--- glibc-2.16.orig/ports/sysdeps/mips/bits/linkmap.h
+++ glibc-2.16/ports/sysdeps/mips/bits/linkmap.h
@@ -1,4 +1,5 @@
 struct link_map_machine
   {
     ElfW(Addr) plt; /* Address of .plt */
+    const Elf32_Word *gnu_xlat_zero; /* .gnu.xhash */
   };

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

* Re: [PATCH] MIPS support for --hash-style=gnu
  2015-10-05 23:11 [PATCH] MIPS support for --hash-style=gnu Neil Schellenberger (neschell)
@ 2015-10-30 11:09 ` Nick Clifton
  2015-11-05 17:14 ` Rafael Espíndola
  2016-02-05 22:38 ` Maciej W. Rozycki
  2 siblings, 0 replies; 17+ messages in thread
From: Nick Clifton @ 2015-10-30 11:09 UTC (permalink / raw)
  To: Neil Schellenberger (neschell), binutils

Hi Neil,

> The patch is relative to binutils-2_24 (which is where I'll ultimately need it) but is equally applicable to trunk.  (The glibc patch is relative to a lightly customized 2.16 but again is equally applicable to trunk.)  As this is my first attempt at a patch for the linker, I've omitted the ChangeLog and test cases for the moment, pending feedback.  Technical and procedural criticism is gratefully welcomed.  I should very much like to thank the many who have taken the time to post on these and related subjects over the years -- I would have found even this modest attempt very difficult without the historical context.  Particular thanks are due to Michael Meeks, Jakub Jelinek, Richard Sandiford, and Hiroki Kaminaga.  Errors in comprehension and judgement are entirely my own.

The patch is fine, and I would be happy to apply it.  But I need to 
check one thing first - do you have an FSF binutils copyright assignment 
on file ?  Copyright assignments for various other employees at Cisco 
are currently on file, but I could not find your name.

Cheers
   Nick

PS.  Sorry for taking so long to review the patch.  I was hoping that 
one of the MIPS maintainers would like it over.


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

* Re: [PATCH] MIPS support for --hash-style=gnu
  2015-10-05 23:11 [PATCH] MIPS support for --hash-style=gnu Neil Schellenberger (neschell)
  2015-10-30 11:09 ` Nick Clifton
@ 2015-11-05 17:14 ` Rafael Espíndola
  2015-11-05 18:27   ` Neil Schellenberger (neschell)
  2015-11-08 13:52   ` Richard Sandiford
  2016-02-05 22:38 ` Maciej W. Rozycki
  2 siblings, 2 replies; 17+ messages in thread
From: Rafael Espíndola @ 2015-11-05 17:14 UTC (permalink / raw)
  To: Neil Schellenberger (neschell); +Cc: binutils

Wouldn't it be possible to sort the got entries so that the symbols
end up in the same order as the gnu hash table?

Cheers,
Rafael


On 5 October 2015 at 19:11, Neil Schellenberger (neschell)
<neschell@cisco.com> wrote:
> I am tasked with reducing the time spent in the interpreter/loader at program start time for a very large, multi-platform, multi-architecture, legacy system (25+Mloc, 2000+ DSOs, 1+M symbols).  On MIPS this loader overhead is several much larger than for other supported architectures, which is not unexpected given the lack of .gnu.hash support. [1]  Measurement shows that a principal factor for the programs most affected is the very large number of DSOs which are directly or indirectly needed -- the chief expense being the cost of successive table misses during symbol resolution.  A secondary factor is that some of the executables and DSOs have a very large number of symbols with relocations -- on MIPS these tend to run afoul of the multi-GOT mechanism which places many into secondary GOTs, resulting in unconditionally forcing their resolution at load time. [2][3][4]  At this stage in the lifecycle of this particular product, large scale architectural changes tend not to be feasible (e.g. appreciably decreasing either the number of DSOs or the number of relocations) so a more transparent solution is preferable.  (c.f. [5][6])
>
> In order to tackle the main problem -- large numbers of needed DSOs -- some means of avoiding unprofitable (i.e. certain miss) hash table probes would help significantly.  Since code to support Bloom filtering already exists for GNU-style hash tables (DT_GNU_HASH), it seemed attractive to enable that feature for MIPS (and then also benefit from as much of the hash chain optimization as possible). [6]  As I understand it, the fundamental problem which has historically prevented this support is, briefly, that the MIPS ABI requires that the .dynsym table be sorted in a particular order, while .gnu.hash mandates a different order; this appears to have stymied at least one earlier attempt. [8]  As I am expert neither with MIPS and its many ABI oddities, nor with the implementation of the BFD linker, I have opted to take a very, very simple -- if perhaps non-optimal -- approach.  Inspired by the comment made by Richard Sandison to Hiroki Kaminaga concerning external symbol blocks [9], I chose to reuse GNU-style hash tables, only with the simple addition of a translation table to map the GNU symbol order to the MIPS symbol order.  The MIPS .dynsym table proper continues to be completely identical: its sort order and content are entirely unchanged.  The proposed changes also leave the output of all other targets/backends entirely unchanged (including MIPS using traditional SysV .hash).
>
> In an attempt to avoid forward compatibility issues, a new section and related dynamic tag are proposed: .gnu.xhash and DT_GNU_XHASH.  (The "X" standing either for "extended" or as a mnemonic for "translated", as the reader prefers.)  This ensures that old binutils, glibc, and other third party code do not attempt to behave as if a traditional .gnu.hash/DT_GNU_HASH is present.  Likewise, the name of the section was chosen so that it is not a textual prefix of .gnu.hash in an attempt to preclude any insufficiently discriminating pattern from matching inadvertently (e.g. in a script parsing readelf output).
>
> In practice, though, the .gnu.xhash section is virtually identical to .gnu.hash. [9]
>
>         // Part 0: .gnu.xhash header
>         Elf32_Word  ngnusyms;  // number of entries in chains (and xlat); dynsymcount=symndx+ngnusyms
>         // Part 1: .gnu.hash header
>         Elf32_Word  nbuckets;  // number of hash table buckets
>         Elf32_Word  symndx;  // number of initial .dynsym entires skipped in chains[] (and xlat[])
>         Elf32_Word  maskwords; // number of ElfW(Addr) words in bitmask
>         Elf32_Word  shift2;  // bit shift of hashval for second Bloom filter bit
>         // Part 2: .gnu.hash Bloom filter
>         ElfW(Addr)  bitmask[maskwords];  // 2 bit Bloom filter on hashval
>         // Part 3: .gnu.hash buckets
>         Elf32_Word  buckets[nbuckets];  // indices into chains[]
>         // Part 4: .gnu.hash chains
>         Elf32_Word  chains[ngnusyms];  // consecutive hashvals in a given bucket; last entry in chain has LSB set
>         // Part 5: .gnu.xhash translation table
>         Elf32_Word  xlat[ngnusyms];  // parallel to chains[]; index into .dynsym
>
> Parts 1 through 4 correspond exactly in layout to the traditional .gnu.hash; part 4 has slightly different semantics since the correct MIPS .dynsym index has to be first looked up in the parallel xlat table i.e. the symbol corresponding to the hashval in chain[N] is .dynsym[xlat[N]].  It is intentional that the .gnu.xhash layout contains a .gnu.hash layout as a subcomponent: it represents an attempt to reuse unchanged as much BFD and readelf code as possible.  The space cost of .gnu.xhash relative to .gnu.hash is ngnusyms+1 32 bit words.  (For time cost, the L2 cache hit from the xlat table lookup is probably atrocious, but at that point we're already about to perform a string compare which is probably going to be even more atrocious....  In any case, it's a whole lot better than SysV .hash.)
>
> In order to reuse the maximum amount of existing code with the minimum amount of copying while also maintaining 100% backward compatibility, I chose to implement the support as a BFD ELF backend hook: record_hash_symbol().  For all platforms other than MIPS, this hook is NULL and the behaviour is to write .gnu.hash (or not) exactly as they always have done.  For MIPS, this hook is non-NULL and (when --hash-style=gnu) will output a .gnu.xhash section which the MIPS specific ELF backend then updates with a translation table to allow .gnu.xhash symbol indices to be translated to MIPS .dynsym indices at the right time during linking.  The principal changes to the common support (in bfd/elflink.c) are in bfd_elf_size_dynsym_hash_dynstr() which computes the correct size for the .gnu.[x]hash section; and in elf_renumber_gnu_hash_syms() which did the sorting for .gnu.hash.  On the ELF MIPS specific side, the main changes are to mips_elf_sort_hash_table_f(); and in the addition of the backend function _bfd_mips_elf_record_hash_symbol() with an associated new field in struct mips_elf_link_hash_entry.
>
> In bfd_elf_size_dynsym_hash_dynstr() the code was modified as little as possible in order to keep the diff small and simple to review; the unfortunate corollary to that is that the changes are a little ugly and somewhat brittle (conditionally shifting the contents pointers along etc.)  This also to an extent dictated the layout of the .gnu.xhash section: it is essentially a .gnu.hash section with a leading ngnusyms word and a following translation table.  (The basic form of the .gnu.hash section was retained so as also to keep the readelf changes to a minimum.)
>
> The elf_renumber_gnu_hash_syms() function no longer unconditionally renumbers the symbols.  Instead, if the backend supplies record_hash_symbol(), then that is called instead to allow it to record the offset of the translation table entry for that symbol.  The MIPS backend will then fill this in later once it has finished fiddling with the GOT(s).  I chose to pass an offset here rather than a pointer only because I wasn't entirely certain if it was architecturally acceptable to assume that the content of the section would never be replaced sometime in between (although this is not currently the case).
>
> On the ELF MIPS side, mips_elf_sort_hash_table_f() now also outputs the final index of each symbol into the .gnu.xhash translation table as required.  This is also a bit brittle since it assumes that nothing else is going to come along and change the order yet again afterwards, but that is also true of all of the already existing MIPS backend code.
>
> The readelf support essentially speaks for itself.  Again, it's a bit ugly since I tried to stick only to insertions and not changing existing lines, so as to make the code inspect simpler.
>
> No changes to gold are proposed here because, if I understand correctly, there is as yet no general MIPS support in any case.
>
> I have included the glibc changes here only as a convenience to reviewers; I will be posting to libc-alpha as well.  (It is perhaps interesting to note in passing that although --hash-style=gnu was disabled in the linker, DT_GNU_HASH support was never removed from the glibc MIPS sysdep dl-lookup.c.  This means that on MIPS the new and old hashvals are currently always both computed at runtime for every symbol.  Fortunately in practice this cost appears to be entirely negligible.  Laterally, I suspect that Jakub Jelinek had independently confirmed this insignificance and is why .hashvals never made it into .gnu.hash. [11]  I experimented with adding .hashvals as well as .gnu.xhash, but it made no appreciable difference.)
>
> The patch is relative to binutils-2_24 (which is where I'll ultimately need it) but is equally applicable to trunk.  (The glibc patch is relative to a lightly customized 2.16 but again is equally applicable to trunk.)  As this is my first attempt at a patch for the linker, I've omitted the ChangeLog and test cases for the moment, pending feedback.  Technical and procedural criticism is gratefully welcomed.  I should very much like to thank the many who have taken the time to post on these and related subjects over the years -- I would have found even this modest attempt very difficult without the historical context.  Particular thanks are due to Michael Meeks, Jakub Jelinek, Richard Sandiford, and Hiroki Kaminaga.  Errors in comprehension and judgement are entirely my own.
>
> Regards,
> Neil
>
> P.S.  It is not entirely clear to me how xgot support does or should interact with multi-GOT. [12]  With xgot supporting about 2**32 entries, shouldn't it be the case that multiple GOTs are almost never needed?
>
>
> [1]  Disable .gnu.hash on MIPS targets  https://www.sourceware.org/ml/binutils/2006-07/msg00341.html
> [2]  MIPS Multi GOT  https://dmz-portal.mips.com/wiki/MIPS_Multi_GOT
> [3]  MIPS multi-got link support  https://www.sourceware.org/ml/binutils/2003-01/msg00165.html
> [4]  Description of MIPS/IRIX multigot  https://sourceware.org/ml/binutils/2014-03/msg00174.html
> [5]  Speeding up the dynamic linker with 100s of DSOs?  https://www.sourceware.org/ml/libc-alpha/2006-01/msg00018.html
> [6]  Re: OO.o / ld / -Bsymbolic patch   https://sourceware.org/ml/binutils/2005-07/msg00109.html
> [7]  [PATCH] DT_GNU_HASH: ~ 50% dynamic linking improvement  https://www.sourceware.org/ml/binutils/2006-06/msg00418.html
> [8]  [RFC][PATCH][EXPERIMENTAL] enable MIPS gnu hash (was: Re: use of gnu hash for MIPS)  https://sourceware.org/ml/binutils/2007-08/msg00387.html
> [9]  Re: use of gnu hash for MIPS  https://sourceware.org/ml/binutils/2007-03/msg00126.html
> [10]  Re: GNU_HASH section format  https://sourceware.org/ml/binutils/2006-10/msg00377.html
> [11]  binutils/glibc .hashvals section ...  https://sourceware.org/ml/binutils/2006-01/msg00171.html
> [12]  Add -mxgot option to mips backend  https://gcc.gnu.org/ml/gcc-patches/2003-08/msg01856.html
>

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

* RE: [PATCH] MIPS support for --hash-style=gnu
  2015-11-05 17:14 ` Rafael Espíndola
@ 2015-11-05 18:27   ` Neil Schellenberger (neschell)
  2015-11-05 18:45     ` Rafael Espíndola
  2015-11-08 13:52   ` Richard Sandiford
  1 sibling, 1 reply; 17+ messages in thread
From: Neil Schellenberger (neschell) @ 2015-11-05 18:27 UTC (permalink / raw)
  To: Rafael Espíndola; +Cc: binutils

I did make a couple of abortive attempts to think of ways to do that,
but in the end I failed.  The chief difficulty seems to me (perhaps
entirely incorrectly) that any change in the order of the GOT entries
would require a related change to all references to them from the 
.text and/or relocations.  (I was really trying to avoid fiddling with
that since I was already in over my head trying to grok BFD, 
multi-got, mxgot, etc.... ) Is there some provably correct way to go
about back-patching the new GOT entry numbers into the .text?
(For obvious reasons, I /really/ want to avoid also needing any
changes in all possible upstream code generators e.g. gcc, gas,
clang etc.)

I would be more than happy to hear that I am completely mistaken,
though!  I knew next to nothing about any of this before I started
and wouldn't pretend to claim to know much more about it now....

Regards,
Neil

> -----Original Message-----
> From: Rafael Espíndola [mailto:rafael.espindola@gmail.com]
> Sent: Thursday, November 05, 2015 12:14 PM
> To: Neil Schellenberger (neschell) <neschell@cisco.com>
> Cc: binutils@sourceware.org
> Subject: Re: [PATCH] MIPS support for --hash-style=gnu
> 
> Wouldn't it be possible to sort the got entries so that the symbols
> end up in the same order as the gnu hash table?
> 
> Cheers,
> Rafael
>

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

* Re: [PATCH] MIPS support for --hash-style=gnu
  2015-11-05 18:27   ` Neil Schellenberger (neschell)
@ 2015-11-05 18:45     ` Rafael Espíndola
  2015-11-05 23:33       ` Neil Schellenberger (neschell)
  0 siblings, 1 reply; 17+ messages in thread
From: Rafael Espíndola @ 2015-11-05 18:45 UTC (permalink / raw)
  To: Neil Schellenberger (neschell); +Cc: binutils

Now in plain text:

I am not sure I follow. In each .o file the text sections have
relocations to the got entries, not indexes, no?

Cheers, Rafael

On 5 November 2015 at 13:27, Neil Schellenberger (neschell)
<neschell@cisco.com> wrote:
> I did make a couple of abortive attempts to think of ways to do that,
> but in the end I failed.  The chief difficulty seems to me (perhaps
> entirely incorrectly) that any change in the order of the GOT entries
> would require a related change to all references to them from the
> .text and/or relocations.  (I was really trying to avoid fiddling with
> that since I was already in over my head trying to grok BFD,
> multi-got, mxgot, etc.... ) Is there some provably correct way to go
> about back-patching the new GOT entry numbers into the .text?
> (For obvious reasons, I /really/ want to avoid also needing any
> changes in all possible upstream code generators e.g. gcc, gas,
> clang etc.)
>
> I would be more than happy to hear that I am completely mistaken,
> though!  I knew next to nothing about any of this before I started
> and wouldn't pretend to claim to know much more about it now....
>
> Regards,
> Neil
>
>> -----Original Message-----
>> From: Rafael Espíndola [mailto:rafael.espindola@gmail.com]
>> Sent: Thursday, November 05, 2015 12:14 PM
>> To: Neil Schellenberger (neschell) <neschell@cisco.com>
>> Cc: binutils@sourceware.org
>> Subject: Re: [PATCH] MIPS support for --hash-style=gnu
>>
>> Wouldn't it be possible to sort the got entries so that the symbols
>> end up in the same order as the gnu hash table?
>>
>> Cheers,
>> Rafael
>>

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

* RE: [PATCH] MIPS support for --hash-style=gnu
  2015-11-05 18:45     ` Rafael Espíndola
@ 2015-11-05 23:33       ` Neil Schellenberger (neschell)
  0 siblings, 0 replies; 17+ messages in thread
From: Neil Schellenberger (neschell) @ 2015-11-05 23:33 UTC (permalink / raw)
  To: Rafael Espíndola; +Cc: binutils

[I apologize in advance for my ignorance of most of the details of MIPS relocations....]

/If/ I understand correctly, there are three relocations involved for MIPS64:
R_MIPS_GOT_DISP, R_MIPS_GOT_PAGE, and R_MIPS_GOT_OFST.  My concern
is that if I shuffle things around in the GOT, wouldn't there be a danger that
(given a big enough move from its original spot in the GOT) a compiler generated
code sequence originally of one instruction and R_MIPS_GOT_DISP might now need
multiple instructions using R_MIPS_GOT_PAGE and R_MIPS_GOT_OFST?

> -----Original Message-----
> From: Rafael Espíndola [mailto:rafael.espindola@gmail.com]
> Sent: Thursday, November 05, 2015 1:46 PM
> To: Neil Schellenberger (neschell) <neschell@cisco.com>
> Cc: binutils@sourceware.org
> Subject: Re: [PATCH] MIPS support for --hash-style=gnu
> 
> Now in plain text:
> 
> I am not sure I follow. In each .o file the text sections have
> relocations to the got entries, not indexes, no?
> 
> Cheers, Rafael
> 
> On 5 November 2015 at 13:27, Neil Schellenberger (neschell)
> <neschell@cisco.com> wrote:
> > I did make a couple of abortive attempts to think of ways to do that,
> > but in the end I failed.  The chief difficulty seems to me (perhaps
> > entirely incorrectly) that any change in the order of the GOT entries
> > would require a related change to all references to them from the
> > .text and/or relocations.  (I was really trying to avoid fiddling with
> > that since I was already in over my head trying to grok BFD,
> > multi-got, mxgot, etc.... ) Is there some provably correct way to go
> > about back-patching the new GOT entry numbers into the .text?
> > (For obvious reasons, I /really/ want to avoid also needing any
> > changes in all possible upstream code generators e.g. gcc, gas,
> > clang etc.)
> >
> > I would be more than happy to hear that I am completely mistaken,
> > though!  I knew next to nothing about any of this before I started
> > and wouldn't pretend to claim to know much more about it now....
> >
> > Regards,
> > Neil
> >
> >> -----Original Message-----
> >> From: Rafael Espíndola [mailto:rafael.espindola@gmail.com]
> >> Sent: Thursday, November 05, 2015 12:14 PM
> >> To: Neil Schellenberger (neschell) <neschell@cisco.com>
> >> Cc: binutils@sourceware.org
> >> Subject: Re: [PATCH] MIPS support for --hash-style=gnu
> >>
> >> Wouldn't it be possible to sort the got entries so that the symbols
> >> end up in the same order as the gnu hash table?
> >>
> >> Cheers,
> >> Rafael
> >>

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

* Re: [PATCH] MIPS support for --hash-style=gnu
  2015-11-05 17:14 ` Rafael Espíndola
  2015-11-05 18:27   ` Neil Schellenberger (neschell)
@ 2015-11-08 13:52   ` Richard Sandiford
  2015-11-09 19:05     ` Neil Schellenberger (neschell)
  1 sibling, 1 reply; 17+ messages in thread
From: Richard Sandiford @ 2015-11-08 13:52 UTC (permalink / raw)
  To: Rafael Espíndola; +Cc: Neil Schellenberger (neschell), binutils

Rafael Espíndola <rafael.espindola@gmail.com> writes:
> Wouldn't it be possible to sort the got entries so that the symbols
> end up in the same order as the gnu hash table?

That sounds like what Hiroki Kaminaga proposed originally.  The problem
is that we also need to sort the GOT so that symbols in the primary GOT
come before those that are only used in secondary GOTs.  (The latter still 
need to be in the ABI GOT.)  So if we did this we'd have to know the
hash sort order quite early in the link process and use it to decide
which input objects can use the primary GOT.  In extreme cases it could
be that none can, because of two commonly-accessed symbols ending up at
opposite ends of the ABI GOT.

( https://sourceware.org/ml/binutils/2007-03/msg00126.html )

Thanks,
Richard

>
> Cheers,
> Rafael
>
>
> On 5 October 2015 at 19:11, Neil Schellenberger (neschell)
> <neschell@cisco.com> wrote:
>> I am tasked with reducing the time spent in the interpreter/loader at
>> program start time for a very large, multi-platform,
>> multi-architecture, legacy system (25+Mloc, 2000+ DSOs, 1+M symbols).
>> On MIPS this loader overhead is several much larger than for other
>> supported architectures, which is not unexpected given the lack of
>> .gnu.hash support. [1] Measurement shows that a principal factor for
>> the programs most affected is the very large number of DSOs which are
>> directly or indirectly needed -- the chief expense being the cost of
>> successive table misses during symbol resolution.  A secondary factor
>> is that some of the executables and DSOs have a very large number of
>> symbols with relocations -- on MIPS these tend to run afoul of the
>> multi-GOT mechanism which places many into secondary GOTs, resulting
>> in unconditionally forcing their resolution at load time. [2][3][4] At
>> this stage in the lifecycle of this particular product, large scale
>> architectural changes tend not to be feasible (e.g. appreciably
>> decreasing either the number of DSOs or the number of relocations) so
>> a more transparent solution is preferable.  (c.f. [5][6])
>>
>> In order to tackle the main problem -- large numbers of needed DSOs --
>> some means of avoiding unprofitable (i.e. certain miss) hash table
>> probes would help significantly.  Since code to support Bloom
>> filtering already exists for GNU-style hash tables (DT_GNU_HASH), it
>> seemed attractive to enable that feature for MIPS (and then also
>> benefit from as much of the hash chain optimization as possible). [6]
>> As I understand it, the fundamental problem which has historically
>> prevented this support is, briefly, that the MIPS ABI requires that
>> the .dynsym table be sorted in a particular order, while .gnu.hash
>> mandates a different order; this appears to have stymied at least one
>> earlier attempt. [8] As I am expert neither with MIPS and its many ABI
>> oddities, nor with the implementation of the BFD linker, I have opted
>> to take a very, very simple -- if perhaps non-optimal -- approach.
>> Inspired by the comment made by Richard Sandison to Hiroki Kaminaga
>> concerning external symbol blocks [9], I chose to reuse GNU-style hash
>> tables, only with the simple addition of a translation table to map
>> the GNU symbol order to the MIPS symbol order.  The MIPS .dynsym table
>> proper continues to be completely identical: its sort order and
>> content are entirely unchanged.  The proposed changes also leave the
>> output of all other targets/backends entirely unchanged (including
>> MIPS using traditional SysV .hash).
>>
>> In an attempt to avoid forward compatibility issues, a new section and
>> related dynamic tag are proposed: .gnu.xhash and DT_GNU_XHASH.  (The
>> "X" standing either for "extended" or as a mnemonic for "translated",
>> as the reader prefers.)  This ensures that old binutils, glibc, and
>> other third party code do not attempt to behave as if a traditional
>> .gnu.hash/DT_GNU_HASH is present.  Likewise, the name of the section
>> was chosen so that it is not a textual prefix of .gnu.hash in an
>> attempt to preclude any insufficiently discriminating pattern from
>> matching inadvertently (e.g. in a script parsing readelf output).
>>
>> In practice, though, the .gnu.xhash section is virtually identical to
>> .gnu.hash. [9]
>>
>>         // Part 0: .gnu.xhash header
>>         Elf32_Word ngnusyms; // number of entries in chains (and
>> xlat); dynsymcount=symndx+ngnusyms
>>         // Part 1: .gnu.hash header
>>         Elf32_Word  nbuckets;  // number of hash table buckets
>>         Elf32_Word symndx; // number of initial .dynsym entires
>> skipped in chains[] (and xlat[])
>>         Elf32_Word  maskwords; // number of ElfW(Addr) words in bitmask
>>         Elf32_Word shift2; // bit shift of hashval for second Bloom
>> filter bit
>>         // Part 2: .gnu.hash Bloom filter
>>         ElfW(Addr)  bitmask[maskwords];  // 2 bit Bloom filter on hashval
>>         // Part 3: .gnu.hash buckets
>>         Elf32_Word  buckets[nbuckets];  // indices into chains[]
>>         // Part 4: .gnu.hash chains
>>         Elf32_Word chains[ngnusyms]; // consecutive hashvals in a
>> given bucket; last entry in chain has LSB set
>>         // Part 5: .gnu.xhash translation table
>>         Elf32_Word xlat[ngnusyms]; // parallel to chains[]; index into
>> .dynsym
>>
>> Parts 1 through 4 correspond exactly in layout to the traditional
>> .gnu.hash; part 4 has slightly different semantics since the correct
>> MIPS .dynsym index has to be first looked up in the parallel xlat
>> table i.e. the symbol corresponding to the hashval in chain[N] is
>> .dynsym[xlat[N]].  It is intentional that the .gnu.xhash layout
>> contains a .gnu.hash layout as a subcomponent: it represents an
>> attempt to reuse unchanged as much BFD and readelf code as possible.
>> The space cost of .gnu.xhash relative to .gnu.hash is ngnusyms+1 32
>> bit words.  (For time cost, the L2 cache hit from the xlat table
>> lookup is probably atrocious, but at that point we're already about to
>> perform a string compare which is probably going to be even more
>> atrocious....  In any case, it's a whole lot better than SysV .hash.)
>>
>> In order to reuse the maximum amount of existing code with the minimum
>> amount of copying while also maintaining 100% backward compatibility,
>> I chose to implement the support as a BFD ELF backend hook:
>> record_hash_symbol().  For all platforms other than MIPS, this hook is
>> NULL and the behaviour is to write .gnu.hash (or not) exactly as they
>> always have done.  For MIPS, this hook is non-NULL and (when
>> --hash-style=gnu) will output a .gnu.xhash section which the MIPS
>> specific ELF backend then updates with a translation table to allow
>> .gnu.xhash symbol indices to be translated to MIPS .dynsym indices at
>> the right time during linking.  The principal changes to the common
>> support (in bfd/elflink.c) are in bfd_elf_size_dynsym_hash_dynstr()
>> which computes the correct size for the .gnu.[x]hash section; and in
>> elf_renumber_gnu_hash_syms() which did the sorting for .gnu.hash.  On
>> the ELF MIPS specific side, the main changes are to
>> mips_elf_sort_hash_table_f(); and in the addition of the backend
>> function _bfd_mips_elf_record_hash_symbol() with an associated new
>> field in struct mips_elf_link_hash_entry.
>>
>> In bfd_elf_size_dynsym_hash_dynstr() the code was modified as little
>> as possible in order to keep the diff small and simple to review; the
>> unfortunate corollary to that is that the changes are a little ugly
>> and somewhat brittle (conditionally shifting the contents pointers
>> along etc.)  This also to an extent dictated the layout of the
>> .gnu.xhash section: it is essentially a .gnu.hash section with a
>> leading ngnusyms word and a following translation table.  (The basic
>> form of the .gnu.hash section was retained so as also to keep the
>> readelf changes to a minimum.)
>>
>> The elf_renumber_gnu_hash_syms() function no longer unconditionally
>> renumbers the symbols.  Instead, if the backend supplies
>> record_hash_symbol(), then that is called instead to allow it to
>> record the offset of the translation table entry for that symbol.  The
>> MIPS backend will then fill this in later once it has finished
>> fiddling with the GOT(s).  I chose to pass an offset here rather than
>> a pointer only because I wasn't entirely certain if it was
>> architecturally acceptable to assume that the content of the section
>> would never be replaced sometime in between (although this is not
>> currently the case).
>>
>> On the ELF MIPS side, mips_elf_sort_hash_table_f() now also outputs
>> the final index of each symbol into the .gnu.xhash translation table
>> as required.  This is also a bit brittle since it assumes that nothing
>> else is going to come along and change the order yet again afterwards,
>> but that is also true of all of the already existing MIPS backend
>> code.
>>
>> The readelf support essentially speaks for itself.  Again, it's a bit
>> ugly since I tried to stick only to insertions and not changing
>> existing lines, so as to make the code inspect simpler.
>>
>> No changes to gold are proposed here because, if I understand
>> correctly, there is as yet no general MIPS support in any case.
>>
>> I have included the glibc changes here only as a convenience to
>> reviewers; I will be posting to libc-alpha as well.  (It is perhaps
>> interesting to note in passing that although --hash-style=gnu was
>> disabled in the linker, DT_GNU_HASH support was never removed from the
>> glibc MIPS sysdep dl-lookup.c.  This means that on MIPS the new and
>> old hashvals are currently always both computed at runtime for every
>> symbol.  Fortunately in practice this cost appears to be entirely
>> negligible.  Laterally, I suspect that Jakub Jelinek had independently
>> confirmed this insignificance and is why .hashvals never made it into
>> .gnu.hash. [11] I experimented with adding .hashvals as well as
>> .gnu.xhash, but it made no appreciable difference.)
>>
>> The patch is relative to binutils-2_24 (which is where I'll ultimately
>> need it) but is equally applicable to trunk.  (The glibc patch is
>> relative to a lightly customized 2.16 but again is equally applicable
>> to trunk.)  As this is my first attempt at a patch for the linker,
>> I've omitted the ChangeLog and test cases for the moment, pending
>> feedback.  Technical and procedural criticism is gratefully welcomed.
>> I should very much like to thank the many who have taken the time to
>> post on these and related subjects over the years -- I would have
>> found even this modest attempt very difficult without the historical
>> context.  Particular thanks are due to Michael Meeks, Jakub Jelinek,
>> Richard Sandiford, and Hiroki Kaminaga.  Errors in comprehension and
>> judgement are entirely my own.
>>
>> Regards,
>> Neil
>>
>> P.S.  It is not entirely clear to me how xgot support does or should
>> interact with multi-GOT. [12] With xgot supporting about 2**32
>> entries, shouldn't it be the case that multiple GOTs are almost never
>> needed?
>>
>>
>> [1] Disable .gnu.hash on MIPS targets
>> https://www.sourceware.org/ml/binutils/2006-07/msg00341.html
>> [2]  MIPS Multi GOT  https://dmz-portal.mips.com/wiki/MIPS_Multi_GOT
>> [3] MIPS multi-got link support
>> https://www.sourceware.org/ml/binutils/2003-01/msg00165.html
>> [4] Description of MIPS/IRIX multigot
>> https://sourceware.org/ml/binutils/2014-03/msg00174.html
>> [5] Speeding up the dynamic linker with 100s of DSOs?
>> https://www.sourceware.org/ml/libc-alpha/2006-01/msg00018.html
>> [6] Re: OO.o / ld / -Bsymbolic patch
>> https://sourceware.org/ml/binutils/2005-07/msg00109.html
>> [7] [PATCH] DT_GNU_HASH: ~ 50% dynamic linking improvement
>> https://www.sourceware.org/ml/binutils/2006-06/msg00418.html
>> [8] [RFC][PATCH][EXPERIMENTAL] enable MIPS gnu hash (was: Re: use of
>> gnu hash for MIPS)
>> https://sourceware.org/ml/binutils/2007-08/msg00387.html
>> [9] Re: use of gnu hash for MIPS
>> https://sourceware.org/ml/binutils/2007-03/msg00126.html
>> [10] Re: GNU_HASH section format
>> https://sourceware.org/ml/binutils/2006-10/msg00377.html
>> [11] binutils/glibc .hashvals section ...
>> https://sourceware.org/ml/binutils/2006-01/msg00171.html
>> [12] Add -mxgot option to mips backend
>> https://gcc.gnu.org/ml/gcc-patches/2003-08/msg01856.html
>>

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

* RE: [PATCH] MIPS support for --hash-style=gnu
  2015-11-08 13:52   ` Richard Sandiford
@ 2015-11-09 19:05     ` Neil Schellenberger (neschell)
  0 siblings, 0 replies; 17+ messages in thread
From: Neil Schellenberger (neschell) @ 2015-11-09 19:05 UTC (permalink / raw)
  To: Richard Sandiford, Rafael Espíndola; +Cc: binutils

I did study the work done by Hiroki Kaminaga (and the original
work by Michael Meeks) and tried various experiments but in the
end I decided to follow the suggestion to use an external data
structure to translate between the GNU order and the MIPS order.
I did look at fiddling around with the primary GOT selection, but that
didn't seem to buy much in and of itself and I was also unable to 
convince myself that I was going to be capable of safely sort things
within or between the GOTs.  But if there's a better way to do this,
I would be glad to know!

> -----Original Message-----
> From: Richard Sandiford [mailto:rdsandiford@googlemail.com]
> Sent: Sunday, November 08, 2015 8:52 AM
> To: Rafael Espíndola <rafael.espindola@gmail.com>
> Cc: Neil Schellenberger (neschell) <neschell@cisco.com>;
> binutils@sourceware.org
> Subject: Re: [PATCH] MIPS support for --hash-style=gnu
> 
> Rafael Espíndola <rafael.espindola@gmail.com> writes:
> > Wouldn't it be possible to sort the got entries so that the symbols
> > end up in the same order as the gnu hash table?
> 
> That sounds like what Hiroki Kaminaga proposed originally.  The problem
> is that we also need to sort the GOT so that symbols in the primary GOT
> come before those that are only used in secondary GOTs.  (The latter still
> need to be in the ABI GOT.)  So if we did this we'd have to know the
> hash sort order quite early in the link process and use it to decide
> which input objects can use the primary GOT.  In extreme cases it could
> be that none can, because of two commonly-accessed symbols ending up at
> opposite ends of the ABI GOT.
> 
> ( https://sourceware.org/ml/binutils/2007-03/msg00126.html )
> 
> Thanks,
> Richard
> 
> >
> > Cheers,
> > Rafael
> >
> >
> > On 5 October 2015 at 19:11, Neil Schellenberger (neschell)
> > <neschell@cisco.com> wrote:
> >> I am tasked with reducing the time spent in the interpreter/loader at
> >> program start time for a very large, multi-platform,
> >> multi-architecture, legacy system (25+Mloc, 2000+ DSOs, 1+M symbols).
> >> On MIPS this loader overhead is several much larger than for other
> >> supported architectures, which is not unexpected given the lack of
> >> .gnu.hash support. [1] Measurement shows that a principal factor for
> >> the programs most affected is the very large number of DSOs which are
> >> directly or indirectly needed -- the chief expense being the cost of
> >> successive table misses during symbol resolution.  A secondary factor
> >> is that some of the executables and DSOs have a very large number of
> >> symbols with relocations -- on MIPS these tend to run afoul of the
> >> multi-GOT mechanism which places many into secondary GOTs, resulting
> >> in unconditionally forcing their resolution at load time. [2][3][4] At
> >> this stage in the lifecycle of this particular product, large scale
> >> architectural changes tend not to be feasible (e.g. appreciably
> >> decreasing either the number of DSOs or the number of relocations) so
> >> a more transparent solution is preferable.  (c.f. [5][6])
> >>
> >> In order to tackle the main problem -- large numbers of needed DSOs --
> >> some means of avoiding unprofitable (i.e. certain miss) hash table
> >> probes would help significantly.  Since code to support Bloom
> >> filtering already exists for GNU-style hash tables (DT_GNU_HASH), it
> >> seemed attractive to enable that feature for MIPS (and then also
> >> benefit from as much of the hash chain optimization as possible). [6]
> >> As I understand it, the fundamental problem which has historically
> >> prevented this support is, briefly, that the MIPS ABI requires that
> >> the .dynsym table be sorted in a particular order, while .gnu.hash
> >> mandates a different order; this appears to have stymied at least one
> >> earlier attempt. [8] As I am expert neither with MIPS and its many ABI
> >> oddities, nor with the implementation of the BFD linker, I have opted
> >> to take a very, very simple -- if perhaps non-optimal -- approach.
> >> Inspired by the comment made by Richard Sandison to Hiroki Kaminaga
> >> concerning external symbol blocks [9], I chose to reuse GNU-style hash
> >> tables, only with the simple addition of a translation table to map
> >> the GNU symbol order to the MIPS symbol order.  The MIPS .dynsym table
> >> proper continues to be completely identical: its sort order and
> >> content are entirely unchanged.  The proposed changes also leave the
> >> output of all other targets/backends entirely unchanged (including
> >> MIPS using traditional SysV .hash).
> >>
> >> In an attempt to avoid forward compatibility issues, a new section and
> >> related dynamic tag are proposed: .gnu.xhash and DT_GNU_XHASH.  (The
> >> "X" standing either for "extended" or as a mnemonic for "translated",
> >> as the reader prefers.)  This ensures that old binutils, glibc, and
> >> other third party code do not attempt to behave as if a traditional
> >> .gnu.hash/DT_GNU_HASH is present.  Likewise, the name of the section
> >> was chosen so that it is not a textual prefix of .gnu.hash in an
> >> attempt to preclude any insufficiently discriminating pattern from
> >> matching inadvertently (e.g. in a script parsing readelf output).
> >>
> >> In practice, though, the .gnu.xhash section is virtually identical to
> >> .gnu.hash. [9]
> >>
> >>         // Part 0: .gnu.xhash header
> >>         Elf32_Word ngnusyms; // number of entries in chains (and
> >> xlat); dynsymcount=symndx+ngnusyms
> >>         // Part 1: .gnu.hash header
> >>         Elf32_Word  nbuckets;  // number of hash table buckets
> >>         Elf32_Word symndx; // number of initial .dynsym entires
> >> skipped in chains[] (and xlat[])
> >>         Elf32_Word  maskwords; // number of ElfW(Addr) words in bitmask
> >>         Elf32_Word shift2; // bit shift of hashval for second Bloom
> >> filter bit
> >>         // Part 2: .gnu.hash Bloom filter
> >>         ElfW(Addr)  bitmask[maskwords];  // 2 bit Bloom filter on hashval
> >>         // Part 3: .gnu.hash buckets
> >>         Elf32_Word  buckets[nbuckets];  // indices into chains[]
> >>         // Part 4: .gnu.hash chains
> >>         Elf32_Word chains[ngnusyms]; // consecutive hashvals in a
> >> given bucket; last entry in chain has LSB set
> >>         // Part 5: .gnu.xhash translation table
> >>         Elf32_Word xlat[ngnusyms]; // parallel to chains[]; index into
> >> .dynsym
> >>
> >> Parts 1 through 4 correspond exactly in layout to the traditional
> >> .gnu.hash; part 4 has slightly different semantics since the correct
> >> MIPS .dynsym index has to be first looked up in the parallel xlat
> >> table i.e. the symbol corresponding to the hashval in chain[N] is
> >> .dynsym[xlat[N]].  It is intentional that the .gnu.xhash layout
> >> contains a .gnu.hash layout as a subcomponent: it represents an
> >> attempt to reuse unchanged as much BFD and readelf code as possible.
> >> The space cost of .gnu.xhash relative to .gnu.hash is ngnusyms+1 32
> >> bit words.  (For time cost, the L2 cache hit from the xlat table
> >> lookup is probably atrocious, but at that point we're already about to
> >> perform a string compare which is probably going to be even more
> >> atrocious....  In any case, it's a whole lot better than SysV .hash.)
> >>
> >> In order to reuse the maximum amount of existing code with the
> minimum
> >> amount of copying while also maintaining 100% backward compatibility,
> >> I chose to implement the support as a BFD ELF backend hook:
> >> record_hash_symbol().  For all platforms other than MIPS, this hook is
> >> NULL and the behaviour is to write .gnu.hash (or not) exactly as they
> >> always have done.  For MIPS, this hook is non-NULL and (when
> >> --hash-style=gnu) will output a .gnu.xhash section which the MIPS
> >> specific ELF backend then updates with a translation table to allow
> >> .gnu.xhash symbol indices to be translated to MIPS .dynsym indices at
> >> the right time during linking.  The principal changes to the common
> >> support (in bfd/elflink.c) are in bfd_elf_size_dynsym_hash_dynstr()
> >> which computes the correct size for the .gnu.[x]hash section; and in
> >> elf_renumber_gnu_hash_syms() which did the sorting for .gnu.hash.  On
> >> the ELF MIPS specific side, the main changes are to
> >> mips_elf_sort_hash_table_f(); and in the addition of the backend
> >> function _bfd_mips_elf_record_hash_symbol() with an associated new
> >> field in struct mips_elf_link_hash_entry.
> >>
> >> In bfd_elf_size_dynsym_hash_dynstr() the code was modified as little
> >> as possible in order to keep the diff small and simple to review; the
> >> unfortunate corollary to that is that the changes are a little ugly
> >> and somewhat brittle (conditionally shifting the contents pointers
> >> along etc.)  This also to an extent dictated the layout of the
> >> .gnu.xhash section: it is essentially a .gnu.hash section with a
> >> leading ngnusyms word and a following translation table.  (The basic
> >> form of the .gnu.hash section was retained so as also to keep the
> >> readelf changes to a minimum.)
> >>
> >> The elf_renumber_gnu_hash_syms() function no longer unconditionally
> >> renumbers the symbols.  Instead, if the backend supplies
> >> record_hash_symbol(), then that is called instead to allow it to
> >> record the offset of the translation table entry for that symbol.  The
> >> MIPS backend will then fill this in later once it has finished
> >> fiddling with the GOT(s).  I chose to pass an offset here rather than
> >> a pointer only because I wasn't entirely certain if it was
> >> architecturally acceptable to assume that the content of the section
> >> would never be replaced sometime in between (although this is not
> >> currently the case).
> >>
> >> On the ELF MIPS side, mips_elf_sort_hash_table_f() now also outputs
> >> the final index of each symbol into the .gnu.xhash translation table
> >> as required.  This is also a bit brittle since it assumes that nothing
> >> else is going to come along and change the order yet again afterwards,
> >> but that is also true of all of the already existing MIPS backend
> >> code.
> >>
> >> The readelf support essentially speaks for itself.  Again, it's a bit
> >> ugly since I tried to stick only to insertions and not changing
> >> existing lines, so as to make the code inspect simpler.
> >>
> >> No changes to gold are proposed here because, if I understand
> >> correctly, there is as yet no general MIPS support in any case.
> >>
> >> I have included the glibc changes here only as a convenience to
> >> reviewers; I will be posting to libc-alpha as well.  (It is perhaps
> >> interesting to note in passing that although --hash-style=gnu was
> >> disabled in the linker, DT_GNU_HASH support was never removed from
> the
> >> glibc MIPS sysdep dl-lookup.c.  This means that on MIPS the new and
> >> old hashvals are currently always both computed at runtime for every
> >> symbol.  Fortunately in practice this cost appears to be entirely
> >> negligible.  Laterally, I suspect that Jakub Jelinek had independently
> >> confirmed this insignificance and is why .hashvals never made it into
> >> .gnu.hash. [11] I experimented with adding .hashvals as well as
> >> .gnu.xhash, but it made no appreciable difference.)
> >>
> >> The patch is relative to binutils-2_24 (which is where I'll ultimately
> >> need it) but is equally applicable to trunk.  (The glibc patch is
> >> relative to a lightly customized 2.16 but again is equally applicable
> >> to trunk.)  As this is my first attempt at a patch for the linker,
> >> I've omitted the ChangeLog and test cases for the moment, pending
> >> feedback.  Technical and procedural criticism is gratefully welcomed.
> >> I should very much like to thank the many who have taken the time to
> >> post on these and related subjects over the years -- I would have
> >> found even this modest attempt very difficult without the historical
> >> context.  Particular thanks are due to Michael Meeks, Jakub Jelinek,
> >> Richard Sandiford, and Hiroki Kaminaga.  Errors in comprehension and
> >> judgement are entirely my own.
> >>
> >> Regards,
> >> Neil
> >>
> >> P.S.  It is not entirely clear to me how xgot support does or should
> >> interact with multi-GOT. [12] With xgot supporting about 2**32
> >> entries, shouldn't it be the case that multiple GOTs are almost never
> >> needed?
> >>
> >>
> >> [1] Disable .gnu.hash on MIPS targets
> >> https://www.sourceware.org/ml/binutils/2006-07/msg00341.html
> >> [2]  MIPS Multi GOT  https://dmz-portal.mips.com/wiki/MIPS_Multi_GOT
> >> [3] MIPS multi-got link support
> >> https://www.sourceware.org/ml/binutils/2003-01/msg00165.html
> >> [4] Description of MIPS/IRIX multigot
> >> https://sourceware.org/ml/binutils/2014-03/msg00174.html
> >> [5] Speeding up the dynamic linker with 100s of DSOs?
> >> https://www.sourceware.org/ml/libc-alpha/2006-01/msg00018.html
> >> [6] Re: OO.o / ld / -Bsymbolic patch
> >> https://sourceware.org/ml/binutils/2005-07/msg00109.html
> >> [7] [PATCH] DT_GNU_HASH: ~ 50% dynamic linking improvement
> >> https://www.sourceware.org/ml/binutils/2006-06/msg00418.html
> >> [8] [RFC][PATCH][EXPERIMENTAL] enable MIPS gnu hash (was: Re: use of
> >> gnu hash for MIPS)
> >> https://sourceware.org/ml/binutils/2007-08/msg00387.html
> >> [9] Re: use of gnu hash for MIPS
> >> https://sourceware.org/ml/binutils/2007-03/msg00126.html
> >> [10] Re: GNU_HASH section format
> >> https://sourceware.org/ml/binutils/2006-10/msg00377.html
> >> [11] binutils/glibc .hashvals section ...
> >> https://sourceware.org/ml/binutils/2006-01/msg00171.html
> >> [12] Add -mxgot option to mips backend
> >> https://gcc.gnu.org/ml/gcc-patches/2003-08/msg01856.html
> >>

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

* Re: [PATCH] MIPS support for --hash-style=gnu
  2015-10-05 23:11 [PATCH] MIPS support for --hash-style=gnu Neil Schellenberger (neschell)
  2015-10-30 11:09 ` Nick Clifton
  2015-11-05 17:14 ` Rafael Espíndola
@ 2016-02-05 22:38 ` Maciej W. Rozycki
  2016-04-19 19:15   ` Neil Schellenberger (neschell)
  2 siblings, 1 reply; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-02-05 22:38 UTC (permalink / raw)
  To: Neil Schellenberger (neschell)
  Cc: Nick Clifton, Huang Pei, ma.jiang, r, binutils

Hi Neil,

 First of all, thank you for your contribution.

 It's been a while since you posted the proposal, long enough that 
meanwhile I took the post of a binutils MIPS target maintainer.  I 
understand that Nick, who is the binutils head maintainer, has already 
approved your change and I am not going to object it, not at least in 
principle, however since the process is still in progress I'm taking the 
opportunity to chime in and comment on your proposal from the MIPS 
architecture's rather than general binutils' point of view.

 Have you been able to sort out your copyright assignment paperwork with 
FSF meanwhile?

On Tue, 6 Oct 2015, Neil Schellenberger (neschell) wrote:

> I am tasked with reducing the time spent in the interpreter/loader at 
> program start time for a very large, multi-platform, multi-architecture, 
> legacy system (25+Mloc, 2000+ DSOs, 1+M symbols).  On MIPS this loader 
> overhead is several much larger than for other supported architectures, 
> which is not unexpected given the lack of .gnu.hash support. [1] 
> Measurement shows that a principal factor for the programs most affected 
> is the very large number of DSOs which are directly or indirectly needed 
> -- the chief expense being the cost of successive table misses during 
> symbol resolution.  A secondary factor is that some of the executables 
> and DSOs have a very large number of symbols with relocations -- on MIPS 
> these tend to run afoul of the multi-GOT mechanism which places many 
> into secondary GOTs, resulting in unconditionally forcing their 
> resolution at load time. [2][3][4] At this stage in the lifecycle of 
> this particular product, large scale architectural changes tend not to 
> be feasible (e.g. appreciably decreasing either the number of DSOs or 
> the number of relocations) so a more transparent solution is preferable.  
> (c.f. [5][6])

 Thank you for the detailed background information and analysis, and the 
references.

> In order to tackle the main problem -- large numbers of needed DSOs -- 
> some means of avoiding unprofitable (i.e. certain miss) hash table 
> probes would help significantly.  Since code to support Bloom filtering 
> already exists for GNU-style hash tables (DT_GNU_HASH), it seemed 
> attractive to enable that feature for MIPS (and then also benefit from 
> as much of the hash chain optimization as possible). [6] As I understand 
> it, the fundamental problem which has historically prevented this 
> support is, briefly, that the MIPS ABI requires that the .dynsym table 
> be sorted in a particular order, while .gnu.hash mandates a different 
> order; this appears to have stymied at least one earlier attempt. [8] As 
> I am expert neither with MIPS and its many ABI oddities, nor with the 
> implementation of the BFD linker, I have opted to take a very, very 
> simple -- if perhaps non-optimal -- approach.  Inspired by the comment 
> made by Richard Sandison to Hiroki Kaminaga concerning external symbol 
> blocks [9], I chose to reuse GNU-style hash tables, only with the simple 
> addition of a translation table to map the GNU symbol order to the MIPS 
> symbol order.  The MIPS .dynsym table proper continues to be completely 
> identical: its sort order and content are entirely unchanged.  The 
> proposed changes also leave the output of all other targets/backends 
> entirely unchanged (including MIPS using traditional SysV .hash).

 One important point I need to make here is that for many environments it 
is going to be necessary to keep a standard ELF hash section in binaries 
along your newly introduced GNU hash section, because they will have to be 
cooperative with the existing tools out there.  This is indeed a standard 
arrangement supported by GNU LD (with the `--hash-style=both' option) in 
addition to producing binaries embedding a single kind of a hash section 
of your choice.  And this has already been used with other targets which 
support using a GNU hash section.  In fact I have previously experienced 
problems myself in a configuration which stopped producing ELF hash 
sections along GNU hash as that caused a tool, the prelinker (more on the 
tool below), to stop working as it didn't support the GNU hash back then.

 So your "very, very simple -- if perhaps non-optimal -- approach" is in 
my opinion actually the best (as most simple solutions usually are), as 
not only it reuses proved existing code we already have in the tools, also 
making long-term maintenance easier, but by keeping all the other ELF file 
structures unchanged it ensures backwards compatibility as well, with 
environments out there that for whatever reasons have or want to keep 
working with the standard ELF hash.

> In an attempt to avoid forward compatibility issues, a new section and 
> related dynamic tag are proposed: .gnu.xhash and DT_GNU_XHASH.  (The "X" 
> standing either for "extended" or as a mnemonic for "translated", as the 
> reader prefers.)  This ensures that old binutils, glibc, and other third 
> party code do not attempt to behave as if a traditional 
> .gnu.hash/DT_GNU_HASH is present.  Likewise, the name of the section was 
> chosen so that it is not a textual prefix of .gnu.hash in an attempt to 
> preclude any insufficiently discriminating pattern from matching 
> inadvertently (e.g. in a script parsing readelf output).

 I agree this is a good choice.

> In practice, though, the .gnu.xhash section is virtually identical to 
> .gnu.hash. [9]
> 
> 	// Part 0: .gnu.xhash header
> 	Elf32_Word  ngnusyms;  // number of entries in chains (and xlat); dynsymcount=symndx+ngnusyms
> 	// Part 1: .gnu.hash header
> 	Elf32_Word  nbuckets;  // number of hash table buckets
> 	Elf32_Word  symndx;  // number of initial .dynsym entires skipped in chains[] (and xlat[])
> 	Elf32_Word  maskwords; // number of ElfW(Addr) words in bitmask
> 	Elf32_Word  shift2;  // bit shift of hashval for second Bloom filter bit
> 	// Part 2: .gnu.hash Bloom filter
> 	ElfW(Addr)  bitmask[maskwords];  // 2 bit Bloom filter on hashval
> 	// Part 3: .gnu.hash buckets
> 	Elf32_Word  buckets[nbuckets];  // indices into chains[]
> 	// Part 4: .gnu.hash chains
> 	Elf32_Word  chains[ngnusyms];  // consecutive hashvals in a given bucket; last entry in chain has LSB set
> 	// Part 5: .gnu.xhash translation table
> 	Elf32_Word  xlat[ngnusyms];  // parallel to chains[]; index into .dynsym
> 
> Parts 1 through 4 correspond exactly in layout to the traditional 
> .gnu.hash; part 4 has slightly different semantics since the correct 
> MIPS .dynsym index has to be first looked up in the parallel xlat table 
> i.e. the symbol corresponding to the hashval in chain[N] is 
> .dynsym[xlat[N]].  It is intentional that the .gnu.xhash layout contains 
> a .gnu.hash layout as a subcomponent: it represents an attempt to reuse 
> unchanged as much BFD and readelf code as possible.  The space cost of 
> .gnu.xhash relative to .gnu.hash is ngnusyms+1 32 bit words.  (For time 
> cost, the L2 cache hit from the xlat table lookup is probably atrocious, 
> but at that point we're already about to perform a string compare which 
> is probably going to be even more atrocious....  In any case, it's a 
> whole lot better than SysV .hash.)

 Fair enough, however as a matter of interest -- have you actually 
benchmarked the difference between your choice and a `.gnu.xhash' layout 
where parts 4 and 5 are intermixed i.e.:

	struct {
		Elf32_Word  hashval;
		Elf32_Word  indxlat;
	} xchains[ngnusyms];

-- maybe in reality that doesn't matter that much, especially with a set 
associative cache?

> In order to reuse the maximum amount of existing code with the minimum 
> amount of copying while also maintaining 100% backward compatibility, I 
> chose to implement the support as a BFD ELF backend hook: 
> record_hash_symbol().  For all platforms other than MIPS, this hook is 
> NULL and the behaviour is to write .gnu.hash (or not) exactly as they 
> always have done.  For MIPS, this hook is non-NULL and (when 
> --hash-style=gnu) will output a .gnu.xhash section which the MIPS 
> specific ELF backend then updates with a translation table to allow 
> .gnu.xhash symbol indices to be translated to MIPS .dynsym indices at 
> the right time during linking.  The principal changes to the common 
> support (in bfd/elflink.c) are in bfd_elf_size_dynsym_hash_dynstr() 
> which computes the correct size for the .gnu.[x]hash section; and in 
> elf_renumber_gnu_hash_syms() which did the sorting for .gnu.hash.  On 
> the ELF MIPS specific side, the main changes are to 
> mips_elf_sort_hash_table_f(); and in the addition of the backend 
> function _bfd_mips_elf_record_hash_symbol() with an associated new field 
> in struct mips_elf_link_hash_entry.

 Please rename the hook to `record_xhash_symbol', to give the name at 
least some meaning.  Right now it does not really say anything offhand, 
you need to know from elsewhere that it's specific to the modified GNU 
hash.

> In bfd_elf_size_dynsym_hash_dynstr() the code was modified as little as 
> possible in order to keep the diff small and simple to review; the 
> unfortunate corollary to that is that the changes are a little ugly and 
> somewhat brittle (conditionally shifting the contents pointers along 
> etc.)  This also to an extent dictated the layout of the .gnu.xhash 
> section: it is essentially a .gnu.hash section with a leading ngnusyms 
> word and a following translation table.  (The basic form of the 
> .gnu.hash section was retained so as also to keep the readelf changes to 
> a minimum.)

 With the switch to DT_MIPS_SYMTABNO as discussed below you can get rid of 
the shift, with the only change remaining being extra contents added at 
the end, which will be very little disturbing.

> The elf_renumber_gnu_hash_syms() function no longer unconditionally 
> renumbers the symbols.  Instead, if the backend supplies 
> record_hash_symbol(), then that is called instead to allow it to record 
> the offset of the translation table entry for that symbol.  The MIPS 
> backend will then fill this in later once it has finished fiddling with 
> the GOT(s).  I chose to pass an offset here rather than a pointer only 
> because I wasn't entirely certain if it was architecturally acceptable 
> to assume that the content of the section would never be replaced 
> sometime in between (although this is not currently the case).

 I think this is a good choice regardless of any assumptions you may or 
may not make, this way you have a single section pointer and handle the 
rest with offsets (of course you can still locally use temporary pointers 
calculated with these offsets if this makes code more reasonable).

 Given the changed semantics I think you need to rename the function 
though, as the old name becomes confusing now.  Something like 
`elf_gnu_hash_process_symidx' might do (no idea why the current name is 
plural, only one symbol is handled per call).  You'll need to update the 
comment too, both at the head of the function and at its (only) call site.

> On the ELF MIPS side, mips_elf_sort_hash_table_f() now also outputs the 
> final index of each symbol into the .gnu.xhash translation table as 
> required.  This is also a bit brittle since it assumes that nothing else 
> is going to come along and change the order yet again afterwards, but 
> that is also true of all of the already existing MIPS backend code.

 By design we have a certain order of processing within BFD, so as long as 
you follow it you can rely on it rather than assuming.  Here the sorting 
is the final processing stage before sections are written out to output, 
so nothing is going to change the order.  And if this is to be changed in 
the future, then it'll be the problem of whoever is going to make that 
change to ensure nothing breaks.

> No changes to gold are proposed here because, if I understand correctly, 
> there is as yet no general MIPS support in any case.

 There is MIPS support in GOLD I believe, but the tool is maintained 
separately and you don't need to update it at the same time, or at all 
unless you want.

> I have included the glibc changes here only as a convenience to 
> reviewers; I will be posting to libc-alpha as well.  (It is perhaps 
> interesting to note in passing that although --hash-style=gnu was 
> disabled in the linker, DT_GNU_HASH support was never removed from the 
> glibc MIPS sysdep dl-lookup.c.  This means that on MIPS the new and old 
> hashvals are currently always both computed at runtime for every symbol.  
> Fortunately in practice this cost appears to be entirely negligible.  
> Laterally, I suspect that Jakub Jelinek had independently confirmed this 
> insignificance and is why .hashvals never made it into .gnu.hash. [11] I 
> experimented with adding .hashvals as well as .gnu.xhash, but it made no 
> appreciable difference.)

 This looks like good material to discuss on `libc-alpha' indeed.  I've 
skimmed over the patch and you'll have to update it to use 
DT_MIPS_SYMTABNO too.

> The patch is relative to binutils-2_24 (which is where I'll ultimately 
> need it) but is equally applicable to trunk.  (The glibc patch is 
> relative to a lightly customized 2.16 but again is equally applicable to 
> trunk.)  As this is my first attempt at a patch for the linker, I've 
> omitted the ChangeLog and test cases for the moment, pending feedback.  
> Technical and procedural criticism is gratefully welcomed.  I should 
> very much like to thank the many who have taken the time to post on 
> these and related subjects over the years -- I would have found even 
> this modest attempt very difficult without the historical context.  
> Particular thanks are due to Michael Meeks, Jakub Jelinek, Richard 
> Sandiford, and Hiroki Kaminaga.  Errors in comprehension and judgement 
> are entirely my own.

 Our head has diverged a little bit, making your patch conflict in 3 
places.  All were purely mechanical and trivial to resolve, so I made them 
so as to apply your proposal to my working tree and run through the usual 
regression testing I do for any serious changes.  Right now it includes 35 
MIPS target configurations (subject to change), in addition to other 
targets.  There have been no regressions, so from the quality's point of 
view your change is fine to go in, once the problems I've listed below 
have been addressed.

> P.S.  It is not entirely clear to me how xgot support does or should 
> interact with multi-GOT. [12] With xgot supporting about 2**32 entries, 
> shouldn't it be the case that multiple GOTs are almost never needed?

 The problem with XGOT is it requires recompiling all the sources involved 
in a static link, also causing code size growth.  All this for a mere 
handful of programs which overflow 16-bit GOT, however with the code size 
regression applying to all the programs which may fit in 16-bit GOT just 
fine.  The multi-GOT approach does not suffer from this problem as it does 
not require changes to object files generated.  It does have some other 
limitations, but they are marginal enough for multi-GOT to have virtually 
superseded XGOT.  NB XGOT dates back to much earlier than multi-GOT, it 
was already specified in the original MIPS SVR4 psABI[1].

On Thu, 14 Jan 2016, Neil Schellenberger (neschell) wrote:

> In a separate email chain, it was also noted that DT_MIPS_SYMTABNO might 
> be used to compute ngnusyms.
> I originally avoided DT_MIPS_SYMTABNO only because I wasn't absolutely 
> certain that it was guaranteed that it (the number of entries in the 
> .dynsym section) was always going to be the same as the the number of 
> entries in chains (plus symidx) so I decided to play it safe.  That may 
> well be needless paranoia on my part.

 Yes, you absolutely have to avoid alignment issues in 64-bit ELF, and 
using DT_MIPS_SYMTABNO is the right choice -- the presence of this tag's 
entry is mandatory in the dynamic structure, as per Figure 5-7: "Dynamic 
Array Tags d_tag" in the MIPS psABI[1].  This entry is needed for the 
dynamic linker, to process the global parts of the GOT and the dynamic 
symbol table which are mapped to each other and the very cause of this all 
hassle, so you can rely on it; DT_MIPS_SYMTABNO=symndx+ngnusyms.

 A handful of nits as to the patch itself:

> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> index add80b3..5418e1d 100644
> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -1216,6 +1216,13 @@ struct elf_backend_data
>    /* Return TRUE if symbol should be hashed in the `.gnu.hash' section.  */
>    bfd_boolean (*elf_hash_symbol) (struct elf_link_hash_entry *);
>  
> +  /* If non-NULL, called to register the location XLAT_LOC within
> +     .gnu.xhash at which real final dynindx for H should be written.
> +     If XLAT_LOC is zero, the symbol is not included in
> +     .gnu.xhash and no dynindx should be written.  */
> +  void (*record_hash_symbol)
> +    (struct elf_link_hash_entry *h, bfd_vma xlat_loc);
> +

 Please s/should/will/ as this is not a recommendation -- it describes how 
we will actually proceed.  Please also reformat the comment for more even 
alignment, i.e.:

  /* If non-NULL, called to register the location XLAT_LOC within
     .gnu.xhash at which real final dynindx for H will be written.
     If XLAT_LOC is zero, the symbol is not included in .gnu.xhash
     and no dynindx will be written.  */

> diff --git a/bfd/elflink.c b/bfd/elflink.c
> index 99b7ca1..1e3e2db 100644
> --- a/bfd/elflink.c
> +++ b/bfd/elflink.c
> @@ -271,8 +271,12 @@ _bfd_elf_link_create_dynamic_sections (bfd *abfd, struct bfd_link_info *info)
>  
>    if (info->emit_gnu_hash)
>      {
> -      s = bfd_make_section_anyway_with_flags (abfd, ".gnu.hash",
> -					      flags | SEC_READONLY);
> +      if (bed->record_hash_symbol == NULL)
> +	s = bfd_make_section_anyway_with_flags (abfd, ".gnu.hash",
> +						flags | SEC_READONLY);
> +      else
> +	s = bfd_make_section_anyway_with_flags (abfd, ".gnu.xhash",
> +						flags | SEC_READONLY);

 Sometimes you write the condition as `bed->record_hash_symbol == NULL' 
and sometimes as `bed->record_hash_symbol != NULL'.  I think it would make 
sense to keep it consistent, except where there is no `else' clause of 
course that is.  However in all cases where there actually is no `else' 
clause the condition is `bed->record_hash_symbol != NULL' and I find it 
more natural to read.  Can you therefore please adjust your code to use 
the `!=' variant throughout?

> @@ -5199,6 +5203,7 @@ struct collect_gnu_hash_codes
>    unsigned long int *counts;
>    bfd_vma *bitmask;
>    bfd_byte *contents;
> +  bfd_vma xlat;

 Please change the type to be `bfd_size_type' rather than `bfd_vma', as 
`bfd_vma' is used for target addresses (as the name implies) and we use 
`bfd_size_type' for offsets into structures processed internally (as these 
offsets will necessarily be in the same ranges as the respective sizes).

> @@ -5278,7 +5283,15 @@ elf_renumber_gnu_hash_syms (struct elf_link_hash_entry *h, void *data)
>    if (! (*s->bed->elf_hash_symbol) (h))
>      {
>        if (h->dynindx >= s->min_dynindx)
> -	h->dynindx = s->local_indx++;
> +	{
> +	  if (s->bed->record_hash_symbol != NULL)
> +	    {
> +	      (*s->bed->record_hash_symbol) (h, 0);
> +	      ++s->local_indx;
> +	    }
> +	  else
> +	    h->dynindx = s->local_indx++;
> +	}

 For consistency please use postincrementation in both legs.

> @@ -5295,7 +5308,14 @@ elf_renumber_gnu_hash_syms (struct elf_link_hash_entry *h, void *data)
>    bfd_put_32 (s->output_bfd, val,
>  	      s->contents + (s->indx[bucket] - s->symindx) * 4);
>    --s->counts[bucket];
> -  h->dynindx = s->indx[bucket]++;
> +  if (s->bed->record_hash_symbol != NULL)
> +    {
> +      bfd_vma xlat_loc = s->xlat + (s->indx[bucket]++ - s->symindx) * 4;
> +      BFD_ASSERT (xlat_loc != 0);
> +      (*s->bed->record_hash_symbol) (h, xlat_loc);

 Please add an empty line between variable declarations from following 
code.  With the observation below you can remove the assertion too.

> @@ -6645,12 +6685,15 @@ bfd_elf_size_dynsym_hash_dynstr (bfd *output_bfd, struct bfd_link_info *info)
>  		}
>  
>  	      cinfo.contents = contents;
> -
> +	      if (bed->record_hash_symbol != NULL)
> +		cinfo.xlat = contents + cinfo.nsyms * 4 - s->contents;

 I'd say just set `cinfo.xlat' unconditionally -- there's no benefit from 
the extra conditional and at worst the value won't be used (and then you 
can remove the assertion above).

> diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
> index d7498e1..d286a62 100644
> --- a/bfd/elfxx-mips.c
> +++ b/bfd/elfxx-mips.c
> @@ -3777,6 +3798,12 @@ mips_elf_sort_hash_table_f (struct mips_elf_link_hash_entry *h, void *data)
>        break;
>      }
>  
> +  if (h->gnuxhash_loc != 0 && hsd->gnuxhash != NULL)
> +    {
> +      bfd_put_32 (hsd->output_bfd, h->root.dynindx,
> +		  hsd->gnuxhash + h->gnuxhash_loc);
> +    }

 No need for curly brackets here.  Also please add a comment explaining 
what this piece does.

> @@ -15285,3 +15312,10 @@ _bfd_mips_post_process_headers (bfd *abfd, struct bfd_link_info *link_info)
>  	i_ehdrp->e_ident[EI_ABIVERSION] = 1;
>      }
>  }
> +
> +void
> +_bfd_mips_elf_record_hash_symbol (struct elf_link_hash_entry *h, bfd_vma xlat_loc)
> +{
> +  struct mips_elf_link_hash_entry *hmips = (struct mips_elf_link_hash_entry *) h;
> +  hmips->gnuxhash_loc = xlat_loc;
> +}

 Please add a comment to this function, explaining what it does.

 You need to wrap this piece as you went beyond 79 columns, see 
<https://www.gnu.org/prep/standards/html_node/Formatting.html>; I think 
separating the variable declaration from initialisation will help.  Also 
please add an empty line between the variable declaration and code that 
follows.

> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index 61ea0ad..e87b111 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -8454,6 +8457,16 @@ process_dynamic_section (FILE * file)
>  	    }
>  	  break;
>  
> +	case DT_GNU_XHASH:
> +	  dynamic_info_DT_GNU_XHASH = entry->d_un.d_val;
> +	  dynamic_info_DT_GNU_HASH = dynamic_info_DT_GNU_XHASH + 4;
> +	  if (do_dynamic)
> +	    {
> +	      print_vma (entry->d_un.d_val, PREFIX_HEX);
> +	      putchar ('\n');
> +	    }
> +	  break;

 With the removal of the `ngnusyms' entry you can make `DT_GNU_XHASH' fall 
through to `DT_GNU_HASH' here and avoid code duplication.

> @@ -9510,7 +9524,7 @@ process_symbol_table (FILE * file)
>        if (fseek (file,
>  		 (archive_file_offset
>  		  + offset_from_vma (file, buckets_vma
> -					   + 4 * (ngnubuckets + maxchain), 4)),
> +				     + 4 * (ngnubuckets + maxchain), 4)),

 Gratuitous change, please remove.  Formatting is wrong here (also after 
your change), but please don't mix coding style changes and code updates 
unless changing the ill-formatted line anyway.

> @@ -9543,7 +9581,45 @@ process_symbol_table (FILE * file)
>  
>        gnuchains = get_dynamic_data (file, maxchain, 4);
>  
> +      if (gnuchains == NULL)
> +	goto no_gnu_hash;
> +
> +      if (dynamic_info_DT_GNU_XHASH)
> +	{
> +	  if (fseek (file,
> +		     (archive_file_offset
> +		      + offset_from_vma (file, dynamic_info_DT_GNU_XHASH, 4)),
> +		     SEEK_SET))
> +	    {
> +	      error (_("Unable to seek to start of dynamic information\n"));
> +	      goto no_gnu_hash;
> +	    }
> +
> +	  if (fread (nb, 4, 1, file) != 1)
> +	    {
> +	      error (_("Failed to read in number of buckets\n"));
> +	      goto no_gnu_hash;
> +	    }
> +	  if (fseek (file,
> +		     (archive_file_offset
> +		      + offset_from_vma (file, buckets_vma
> +					       + 4 * (ngnubuckets
> +						      + maxchain), 4)),

 Bad formatting here, you need to enclose a wrapped expression in 
parentheses:

		      + offset_from_vma (file, (buckets_vma
						+ 4 * (ngnubuckets
						       + maxchain), 4))),

I wonder if one or more auxiliary variables can be used to reduce wrapping 
here and improve readability.  They might serve a documentation purpose 
even.

> @@ -9583,7 +9659,8 @@ process_symbol_table (FILE * file)
>  
>        if (dynamic_info_DT_GNU_HASH)
>  	{
> -	  printf (_("\nSymbol table of `.gnu.hash' for image:\n"));
> +	  printf (_("\nSymbol table of `%s' for image:\n"),
> +		  !dynamic_info_DT_GNU_XHASH ? ".gnu.hash" : ".gnu.xhash");

 Positive conditionals are easier to read, so please swap this expression.

> @@ -9597,7 +9674,10 @@ process_symbol_table (FILE * file)
>  
>  		do
>  		  {
> -		    print_dynamic_symbol (si, hn);
> +		    if (!dynamic_info_DT_GNU_XHASH)
> +		      print_dynamic_symbol (si, hn);
> +		    else
> +		      print_dynamic_symbol (gnuxlat[off], hn);

 And likewise this conditional.

> @@ -9931,7 +10011,8 @@ process_symbol_table (FILE * file)
>  	  return 0;
>  	}
>  
> -      printf (_("\nHistogram for `.gnu.hash' bucket list length (total of %lu buckets):\n"),
> +      printf (_("\nHistogram for `%s' bucket list length (total of %lu buckets):\n"),
> +	      !dynamic_info_DT_GNU_XHASH ? ".gnu.hash" : ".gnu.xhash",

 And likewise here.

> @@ -9977,6 +10058,8 @@ process_symbol_table (FILE * file)
>        free (lengths);
>        free (gnubuckets);
>        free (gnuchains);
> +      free (gnuxlat);
> +
>      }

 Extraneous new line, please remove.

> diff --git a/ld/testsuite/ld-mips-elf/hash1b.d b/ld/testsuite/ld-mips-elf/hash1b.d
> index 5af9037..6836ba9 100644
> --- a/ld/testsuite/ld-mips-elf/hash1b.d
> +++ b/ld/testsuite/ld-mips-elf/hash1b.d
> @@ -1,3 +1,4 @@
>  #source: hash1.s
>  #ld: -shared --hash-style=both
> -#error: .gnu.hash is incompatible with the MIPS ABI
> +#objdump: -dr
> +#pass
> diff --git a/ld/testsuite/ld-mips-elf/hash1c.d b/ld/testsuite/ld-mips-elf/hash1c.d
> index 09bff3c..72bdc18 100644
> --- a/ld/testsuite/ld-mips-elf/hash1c.d
> +++ b/ld/testsuite/ld-mips-elf/hash1c.d
> @@ -1,3 +1,4 @@
>  #source: hash1.s
>  #ld: -shared --hash-style=gnu
> -#error: .gnu.hash is incompatible with the MIPS ABI
> +#objdump: -dr
> +#pass

 Given that these tests (and `hash1a.d') were added along code to handle 
our non-support for GNU hash on the MIPS target in `mips_after_parse' with 
commit 73934d319dae it looks to me they were meant to verify that we bail 
out gracefully.  Now that this code is going away I think merely silencing 
the tests in this manner is not the way to go.

 With `mips_after_parse' removed they serve no purpose anymore, but I 
think at the very least we should have minimum coverage for the actual 
feature.  So instead please arrange for a dynamic symbol to be produced 
and then verify that the machinery works e.g. by passing the DSO built 
through `readelf -I'.  Especially as it seems we have little if any 
coverage here or at least I have troubles finding any relevant test cases.  
All I found is `ld/testsuite/ld-elf/hash.d' (which BTW needs to be updated 
accordingly; please include it with the next version of your patch) and 
that is really a bare minimum.

 I can help you with making such an update to these test cases if you find 
yourself having troubles with it.

 Then as the next step I think we should actually verify the contents of 
the section generated, so in addition to the minimal tests outlined above 
a `readelf -x .gnu.xhash' or `objdump -s -j .gnu.xhash' test would be good 
to have, with more than one dynamic symbol involved so as to make it less 
trivial.  This further test is not a prerequisite for the acceptance of 
your patch as far as I'm concerned, however if you'd like to experiment, 
learn a bit about our test suite and also to verify your work, then I 
encourage you and will greatly appreciate such an update.

 Please resubmit the change with the updates requested and a ChangeLog 
entry, written according to 
<https://www.gnu.org/prep/standards/html_node/Change-Logs.html> in case 
you haven't seen that, or let me know if you have any questions or 
comments.

 Regardless of this dynamic load performance improvement, which is greatly 
appreciated, what I think you might also look into to solve your problem 
is prelinking.  Have you considered that?  With the numerous mentions 
across the references you quoted you must have certainly been aware of the 
existence of the prelinker[2], which in principle is a tool to address the 
very problem you have, that is speeding up dynamic loading, especially 
where the number of dynamic symbols and/or DSOs is huge.  While not a part 
of the GNU project the tool is nevertheless free software available under 
the terms of the GNU GPL and currently maintained as a part of the Yocto 
Project[3].  Support for the MIPS target is included, which is what you 
may have not realised.

 The only drawback of prelinking I know of is that, by the nature of its 
operation, its incompatible with ASLR, so unless you need this feature the 
tool may be worth trying.

 And finally I apologise if the review process took maybe a little bit 
longer than you may have wished.  I'll do my best to drive it to a 
successful conclusion quickly now.  Again if you have any questions or 
comments, just let me know.

References:

[1] "SYSTEM V APPLICATION BINARY INTERFACE, MIPS RISC Processor
    Supplement, 3rd Edition", The Santa Cruz Operation, Inc., February 
    1996
    <http://www.linux-mips.org/pub/linux/mips/doc/ABI/mipsabi.pdf>
[2] Jakub Jelinek, "Prelink", Red Hat, Inc., December 10, 2003
    <https://people.redhat.com/jakub/prelink/prelink.pdf>
[3] "Cross-Prelink"
    <https://wiki.yoctoproject.org/wiki/Cross-Prelink>

  Maciej

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

* RE: [PATCH] MIPS support for --hash-style=gnu
  2016-02-05 22:38 ` Maciej W. Rozycki
@ 2016-04-19 19:15   ` Neil Schellenberger (neschell)
  2016-04-20 14:16     ` Joseph Myers
  2016-04-21  1:19     ` Maciej W. Rozycki
  0 siblings, 2 replies; 17+ messages in thread
From: Neil Schellenberger (neschell) @ 2016-04-19 19:15 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Nick Clifton, Huang Pei, ma.jiang, r, binutils

Hi Maciej,

First, my most sincere apologies for somehow missing this when it was posted!
My Outlook (blech) server side filters apparently decided to have a little lie down.

Thank you very much for your detailed and insightful analysis.  I concur, of course,
with all of your points regarding naming, formatting, and so forth.

>  Have you been able to sort out your copyright assignment paperwork with
> FSF meanwhile?

Sadly this remains stuck with Cisco's lawyers (despite repeated prodding).
I think my next move will be to try to get them to assign the code to me
so that I can assign it to the FSF.  (Sigh.)

>  One important point I need to make here is that for many environments it
> is going to be necessary to keep a standard ELF hash section in binaries
> along your newly introduced GNU hash section, because they will have to be
> cooperative with the existing tools out there.  This is indeed a standard
> arrangement supported by GNU LD (with the `--hash-style=both' option) in
> addition to producing binaries embedding a single kind of a hash section
> of your choice.  And this has already been used with other targets which
> support using a GNU hash section.  In fact I have previously experienced
> problems myself in a configuration which stopped producing ELF hash
> sections along GNU hash as that caused a tool, the prelinker (more on the
> tool below), to stop working as it didn't support the GNU hash back then.

I agree entirely.  We have been running successfully with both sections
present on builds with a loader that only understood the old hash as well
as builds which understood both.  So there is some reason to believe
this "should" work.  (I won't claim any extensive testing, though!)

>  Fair enough, however as a matter of interest -- have you actually
> benchmarked the difference between your choice and a `.gnu.xhash' layout
> where parts 4 and 5 are intermixed i.e.:
> 
> 	struct {
> 		Elf32_Word  hashval;
> 		Elf32_Word  indxlat;
> 	} xchains[ngnusyms];
> 
> -- maybe in reality that doesn't matter that much, especially with a set
> associative cache?

Unfortunately, I have not.  I stuck with a very simple extension to the existing
section because I feared that, in my extreme ignorance, I would break
something very subtle somewhere.

>  Yes, you absolutely have to avoid alignment issues in 64-bit ELF, and
> using DT_MIPS_SYMTABNO is the right choice -- the presence of this tag's
> entry is mandatory in the dynamic structure, as per Figure 5-7: "Dynamic
> Array Tags d_tag" in the MIPS psABI[1].  This entry is needed for the
> dynamic linker, to process the global parts of the GOT and the dynamic
> symbol table which are mapped to each other and the very cause of this all
> hassle, so you can rely on it; DT_MIPS_SYMTABNO=symndx+ngnusyms.

Now that I understand this all a bit better, I tend to agree.  The only, very
theoretical, counterargument is that this would tie .gnu.xhash to MIPS only,
which isn't technically necessary.  Pragmatically, though, it will almost
certainly never be used anywhere else....  (One can only hope.)

> > @@ -9510,7 +9524,7 @@ process_symbol_table (FILE * file)
> >        if (fseek (file,
> >  		 (archive_file_offset
> >  		  + offset_from_vma (file, buckets_vma
> > -					   + 4 * (ngnubuckets + maxchain), 4)),
> > +				     + 4 * (ngnubuckets + maxchain), 4)),
> 
>  Gratuitous change, please remove.  Formatting is wrong here (also after
> your change), but please don't mix coding style changes and code updates
> unless changing the ill-formatted line anyway.

Sorry, I didn't intend to include that in the patch.  It slipped through my
proofreading.

>  Given that these tests (and `hash1a.d') were added along code to handle
> our non-support for GNU hash on the MIPS target in `mips_after_parse' with
> commit 73934d319dae it looks to me they were meant to verify that we bail
> out gracefully.  Now that this code is going away I think merely silencing
> the tests in this manner is not the way to go.
> 
>  With `mips_after_parse' removed they serve no purpose anymore, but I
> think at the very least we should have minimum coverage for the actual
> feature.  So instead please arrange for a dynamic symbol to be produced
> and then verify that the machinery works e.g. by passing the DSO built
> through `readelf -I'.  Especially as it seems we have little if any
> coverage here or at least I have troubles finding any relevant test cases.
> All I found is `ld/testsuite/ld-elf/hash.d' (which BTW needs to be updated
> accordingly; please include it with the next version of your patch) and
> that is really a bare minimum.
> 
>  I can help you with making such an update to these test cases if you find
> yourself having troubles with it.
> 
>  Then as the next step I think we should actually verify the contents of
> the section generated, so in addition to the minimal tests outlined above
> a `readelf -x .gnu.xhash' or `objdump -s -j .gnu.xhash' test would be good
> to have, with more than one dynamic symbol involved so as to make it less
> trivial.  This further test is not a prerequisite for the acceptance of
> your patch as far as I'm concerned, however if you'd like to experiment,
> learn a bit about our test suite and also to verify your work, then I
> encourage you and will greatly appreciate such an update.
> 

I'm not too familiar with the dejagnu rig being used.  It isn't totally clear to
me exactly how to go about adding arbitrarily complex tests to it so I went
with something simple.  That and, well, deadlines....

>  The only drawback of prelinking I know of is that, by the nature of its
> operation, its incompatible with ASLR, so unless you need this feature the
> tool may be worth trying.

That was one of my first suggestions too.  Unfortunately, ASLR is a mandatory
part of the system in question.

Thank you again for your detailed review and again my apologies for my
tardy response.  I will once again prod our legal folks to see if we can't
find some sort of solution.  However, in the meantime, this will remain
in limbo I'm afraid.  (Sorry.)

Regards,
Neil
 

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

* RE: [PATCH] MIPS support for --hash-style=gnu
  2016-04-19 19:15   ` Neil Schellenberger (neschell)
@ 2016-04-20 14:16     ` Joseph Myers
  2016-04-20 16:44       ` Jim Wilson
  2016-04-21  1:19     ` Maciej W. Rozycki
  1 sibling, 1 reply; 17+ messages in thread
From: Joseph Myers @ 2016-04-20 14:16 UTC (permalink / raw)
  To: Neil Schellenberger (neschell)
  Cc: Maciej W. Rozycki, Nick Clifton, Huang Pei, ma.jiang, r, binutils

On Tue, 19 Apr 2016, Neil Schellenberger (neschell) wrote:

> >  Have you been able to sort out your copyright assignment paperwork with
> > FSF meanwhile?
> 
> Sadly this remains stuck with Cisco's lawyers (despite repeated prodding).
> I think my next move will be to try to get them to assign the code to me
> so that I can assign it to the FSF.  (Sigh.)

At one point Cisco had a process for doing this; copyright.list has

ANY     Cisco Systems, Inc.     2010-9-30
Agreement to transfer copyrights in future via "Software Letters".
Michael Enescu (CTO Open Source Initiatives)

and then a series of subsequent assignments for particular GNU packages 
naming particular authorized people (but I see Michael Enescu is no longer 
at Cisco).

-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH] MIPS support for --hash-style=gnu
  2016-04-20 14:16     ` Joseph Myers
@ 2016-04-20 16:44       ` Jim Wilson
  0 siblings, 0 replies; 17+ messages in thread
From: Jim Wilson @ 2016-04-20 16:44 UTC (permalink / raw)
  To: Joseph Myers, Neil Schellenberger (neschell)
  Cc: Maciej W. Rozycki, Nick Clifton, Huang Pei, ma.jiang, r, binutils

On 04/20/2016 07:16 AM, Joseph Myers wrote:
> On Tue, 19 Apr 2016, Neil Schellenberger (neschell) wrote:
>
>>>   Have you been able to sort out your copyright assignment paperwork with
>>> FSF meanwhile?
>>
>> Sadly this remains stuck with Cisco's lawyers (despite repeated prodding).
>> I think my next move will be to try to get them to assign the code to me
>> so that I can assign it to the FSF.  (Sigh.)
>
> At one point Cisco had a process for doing this; copyright.list has
>
> ANY     Cisco Systems, Inc.     2010-9-30
> Agreement to transfer copyrights in future via "Software Letters".
> Michael Enescu (CTO Open Source Initiatives)
>
> and then a series of subsequent assignments for particular GNU packages
> naming particular authorized people (but I see Michael Enescu is no longer
> at Cisco).

The Cisco process is complex.  The Cisco/FSF copyright assignment only 
allows the FSF to accept code from Cisco.  There is a separate Cisco 
internal process that has to be completed in order for Cisco folks to 
submit code to the FSF.  The Cisco/FSF copyright assignment requires 
software letters, that names specific people and specific projects.  If 
you don't have a software letter, the FSF can't accept code from you. 
The internal process requires about half a dozen signatures from 
different people in different groups at Cisco at different levels of the 
management chain.  The internal process also puts a number of 
restrictions on what can be contributed, and how it can be contributed. 
  You will likely have to complete the internal process before you can 
get a software letter.  With the internal process complete, it takes 
about 6 months to get a new software letter if you want to change the 
list of names.

When I was at Cisco, it took me 3 years to get permission to contribute 
gcc patches under reasonable terms.  And that was after the Cisco/FSF 
Copyright Assignment had already been signed.

I spent 4.5 year trying to get permission to contribute binutils 
patches, and then gave up and left.  The remnants of this attempt should 
still be open in the Cisco Open Source Initiative database.  You should 
try talking to the Cisco compiler group.

I tried the personal assignment route, and that went nowhere.  I 
couldn't get a usable disclaimer from Cisco.

I was working with Michael Enescu.  Now that he is gone, I don't know if 
that will make things easier or harder.

Part of the trouble with Cisco stems from the deal to buy Linksys, which 
eventually led to the FSF filing a GPL violation lawsuit against 
Linksys/Cisco, which required Cisco to accept a settlement.  This made 
the Cisco lawyers very reluctant to deal with anything FSF related.  I 
would expect that this issue will eventually die down, but not sure if 
enough time has passed yet.

Jim

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

* RE: [PATCH] MIPS support for --hash-style=gnu
  2016-04-19 19:15   ` Neil Schellenberger (neschell)
  2016-04-20 14:16     ` Joseph Myers
@ 2016-04-21  1:19     ` Maciej W. Rozycki
  1 sibling, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2016-04-21  1:19 UTC (permalink / raw)
  To: Neil Schellenberger (neschell), Nick Clifton
  Cc: Huang Pei, ma.jiang, r, binutils

Neil, Nick --

 Nick: I have included a request to you -- please see the second paragraph 
below and feel free to ignore the rest of this e-mail.

> First, my most sincere apologies for somehow missing this when it was 
> posted! My Outlook (blech) server side filters apparently decided to 
> have a little lie down.

 Well, the later you reply, the later you'll get a followup -- it's just 
that simple.  I'm glad that you've found the missing e-mail though.

> >  Have you been able to sort out your copyright assignment paperwork with
> > FSF meanwhile?
> 
> Sadly this remains stuck with Cisco's lawyers (despite repeated prodding).
> I think my next move will be to try to get them to assign the code to me
> so that I can assign it to the FSF.  (Sigh.)

 Jim's observations notwithstanding, if you find things going really bad, 
then would perhaps a one-off copyright disclaimer for this piece of code 
only be an option to consider?  I suppose it might be easier to arrange 
and my understanding is that, while not encouraged, this option is offered 
by FSF to make contributions easier in cases like this, where getting an 
assignment might be difficult.

 Nick, as the head maintainer would you please confirm or deny and if this 
is feasible provide Neil with the details?  I don't have access to the 
relevant FSF's resources myself.

> >  One important point I need to make here is that for many environments it
> > is going to be necessary to keep a standard ELF hash section in binaries
> > along your newly introduced GNU hash section, because they will have to be
> > cooperative with the existing tools out there.  This is indeed a standard
> > arrangement supported by GNU LD (with the `--hash-style=both' option) in
> > addition to producing binaries embedding a single kind of a hash section
> > of your choice.  And this has already been used with other targets which
> > support using a GNU hash section.  In fact I have previously experienced
> > problems myself in a configuration which stopped producing ELF hash
> > sections along GNU hash as that caused a tool, the prelinker (more on the
> > tool below), to stop working as it didn't support the GNU hash back then.
> 
> I agree entirely.  We have been running successfully with both sections
> present on builds with a loader that only understood the old hash as well
> as builds which understood both.  So there is some reason to believe
> this "should" work.  (I won't claim any extensive testing, though!)

 Having reviewed your code I believe it will be safe to install once the 
issues noted have been addressed, but I could put your change through 
glibc regression testing myself if you are not set up for doing it, to 
make sure that ELF hash support hasn't regressed and that GNU hash support 
gives the same results.  This is I believe the most comprehensive way of 
testing dynamic loading we have available at hand.

> >  Fair enough, however as a matter of interest -- have you actually
> > benchmarked the difference between your choice and a `.gnu.xhash' layout
> > where parts 4 and 5 are intermixed i.e.:
> > 
> > 	struct {
> > 		Elf32_Word  hashval;
> > 		Elf32_Word  indxlat;
> > 	} xchains[ngnusyms];
> > 
> > -- maybe in reality that doesn't matter that much, especially with a set
> > associative cache?
> 
> Unfortunately, I have not.  I stuck with a very simple extension to the existing
> section because I feared that, in my extreme ignorance, I would break
> something very subtle somewhere.

 Ack, fair enough.  Though obviously having performance figures for 
comparison would be desirable even if the alternative implementation was 
deemed too risky to deploy -- to know if we're losing or gaining anything 
by making a particular choice.

> >  Yes, you absolutely have to avoid alignment issues in 64-bit ELF, and
> > using DT_MIPS_SYMTABNO is the right choice -- the presence of this tag's
> > entry is mandatory in the dynamic structure, as per Figure 5-7: "Dynamic
> > Array Tags d_tag" in the MIPS psABI[1].  This entry is needed for the
> > dynamic linker, to process the global parts of the GOT and the dynamic
> > symbol table which are mapped to each other and the very cause of this all
> > hassle, so you can rely on it; DT_MIPS_SYMTABNO=symndx+ngnusyms.
> 
> Now that I understand this all a bit better, I tend to agree.  The only, very
> theoretical, counterargument is that this would tie .gnu.xhash to MIPS only,
> which isn't technically necessary.  Pragmatically, though, it will almost
> certainly never be used anywhere else....  (One can only hope.)

 I don't think this is a problem at all actually -- after all the very 
reason you needed to implement .gnu.xhash for the MIPS target is the same 
which makes DT_MIPS_SYMTABNO necessary.  So any other target requiring the 
mapping will likely have a similar dynamic entry (tag) already defined in 
its psABI, and if not, then we can require such a tag to be added as a GNU 
extension, just as such an extension .gnu.xhash already is.  Any tool 
which produces .gnu.xhash may as well produce such a dynamic entry.  So as 
I say, I don't think there is a technical limitation here, not even a 
potential one.

> >  Gratuitous change, please remove.  Formatting is wrong here (also after
> > your change), but please don't mix coding style changes and code updates
> > unless changing the ill-formatted line anyway.
> 
> Sorry, I didn't intend to include that in the patch.  It slipped through my
> proofreading.

 No worries, things happen sometimes (or rather all the time), and it's 
one of the purposes of the review process to get them shaken out.

> >  Given that these tests (and `hash1a.d') were added along code to handle
> > our non-support for GNU hash on the MIPS target in `mips_after_parse' with
> > commit 73934d319dae it looks to me they were meant to verify that we bail
> > out gracefully.  Now that this code is going away I think merely silencing
> > the tests in this manner is not the way to go.
> > 
> >  With `mips_after_parse' removed they serve no purpose anymore, but I
> > think at the very least we should have minimum coverage for the actual
> > feature.  So instead please arrange for a dynamic symbol to be produced
> > and then verify that the machinery works e.g. by passing the DSO built
> > through `readelf -I'.  Especially as it seems we have little if any
> > coverage here or at least I have troubles finding any relevant test cases.
> > All I found is `ld/testsuite/ld-elf/hash.d' (which BTW needs to be updated
> > accordingly; please include it with the next version of your patch) and
> > that is really a bare minimum.
> > 
> >  I can help you with making such an update to these test cases if you find
> > yourself having troubles with it.
> > 
> >  Then as the next step I think we should actually verify the contents of
> > the section generated, so in addition to the minimal tests outlined above
> > a `readelf -x .gnu.xhash' or `objdump -s -j .gnu.xhash' test would be good
> > to have, with more than one dynamic symbol involved so as to make it less
> > trivial.  This further test is not a prerequisite for the acceptance of
> > your patch as far as I'm concerned, however if you'd like to experiment,
> > learn a bit about our test suite and also to verify your work, then I
> > encourage you and will greatly appreciate such an update.
> > 
> 
> I'm not too familiar with the dejagnu rig being used.  It isn't totally clear to
> me exactly how to go about adding arbitrarily complex tests to it so I went
> with something simple.  That and, well, deadlines....

 It's quite easy actually.  First you need a source to build, preferably 
an assembly one, though in some cases a C one might be acceptable.

 Then you need a recipe to build it, as if entered at a shell's command 
line.  Since you need to verify dynamic load data here, this will have to 
include a way to assemble and link your binary to be verified.

 Finally you need to pick the tool you want to do verification with (i.e. 
`readelf -I' as I suggested), and produce a pattern to match the output 
from that tool against.  The pattern is a text file of TCL regular 
expressions -- in reality for many cases (and I believe in this one in 
particular) you just want to match output literally, so all you need to do 
is escaping characters that have a special meaning in TCL's regexp parser 
(documented here: <http://tmml.sourceforge.net/doc/tcl/re_syntax.html> 
BTW; it's a superset of the standard regular expression syntax).

 Since it's LD that you'll be verifying that it produces the expected hash 
contents the test case has to be a part of the LD testsuite (and the MIPS 
part thereof), located under ld/testsuite/ld-mips-elf/.  Take a look at 
`ld/testsuite/ld-mips-elf/got-dump-2.d' as an easy example (test cases of 
this kind are given a .d suffix; this is required by our test framework).

 At the top of the file you can see a series of lines starting with # 
followed by a keyword, colon and then some text.  These are commands for 
the test framework.  All the commands you'll need for your test case are 
present there:

- name    -- defines a description for the test, which will be printed and 
             logged as the test executes,

- source  -- names a file to assemble, it can be followed with options to 
             pass to GAS for the assembly of this file if needed; if you 
             need to assemble more than one file, then you can include 
             further `source' commands and corresponding GAS options on 
             separate lines each,

- as      -- gives options to pass to GAS globally for the assembly of all 
             sources, in addition to any options specified with `source',

- ld      -- gives options to pass to LD for linking; the names of all 
             objects assembled by `source' commands will be passed to LD 
             as well,

- readelf -- this gives the name of the tool to produce the result to 
             verify against the regexp included with the file (other 
             possibilities are `objdump' or `nm'), and the options to pass 
             to it to get its output.

There is a further explanation available in `ld/testsuite/lib/ld-lib.exp', 
near `run_dump_test', but I think the short reference above should be 
enough to get you set up.

 Then the rest of the file is the regexp pattern to match, line by line.  
Empty lines are ignored both in this file and in input matched against, so 
they can be used to improve readability.  As you can see parentheses are 
escaped so that they lose their special meaning and are matched literally 
instead.  You don't need to write this part by hand of course, you just 
produce it with the tool later used for matching.

 With a randomly picked up non-MIPS dynamic binary I have on the machine 
I'm writing this on and the output of `readelf -I' executed with that 
binary and passed through `sed 's/\([()\.]\)/\\\1/g'' I get this:

Histogram for bucket list length \(total of 1017 buckets\):
 Length  Number     % of total  Coverage
      0  108        \( 10\.6%\)
      1  266        \( 26\.2%\)     12\.3%
      2  273        \( 26\.8%\)     37\.4%
      3  207        \( 20\.4%\)     66\.1%
      4  103        \( 10\.1%\)     85\.1%
      5  41         \(  4\.0%\)     94\.5%
      6  15         \(  1\.5%\)     98\.7%
      7  3          \(  0\.3%\)     99\.6%
      8  1          \(  0\.1%\)    100\.0%

Histogram for `\.gnu\.hash' bucket list length \(total of 1011 buckets\):
 Length  Number     % of total  Coverage
      0  108        \( 10\.7%\)
      1  242        \( 23\.9%\)     11\.2%
      2  294        \( 29\.1%\)     38\.4%
      3  205        \( 20\.3%\)     66\.8%
      4  113        \( 11\.2%\)     87\.7%
      5  30         \(  3\.0%\)     94\.6%
      6  17         \(  1\.7%\)     99\.4%
      7  2          \(  0\.2%\)    100\.0%

-- so you'll have to include something similar as your pattern to match 
against.

 Now you're set up to wire your test case into the test framework, and you 
do it by adding:

run_dump_test "<name-of-your-test>"

to `ld/testsuite/ld-mips-elf/mips-elf.exp', with <name-of-your-test> 
substituted with the name you chose, sans the .d suffix.  I suggest that 
you just reuse `hash1a', `hash1b' and `hash1c' (which are already wired) 
and substitute their contents appropriately.

 There is a second, more complicated way to wire tests into the LD test 
suite, which requires some knowledge of the TCL programming language, but 
I don't think it's needed in your case.  It would be if there were more 
complex requirements, such as multiple shared libraries first built and 
then linked against.  It's clearly not the case here, so I won't be 
introducing you to that part.

 As to the change to `ld/testsuite/ld-elf/hash.d' I suggested -- first you 
need to remove:

#notarget: mips*-*-*

from that file (so that the test case is now run with the MIPS target as 
well) and see what happens.  Perhaps the test case will just pass, so no 
need to do anything else.  If it fails, then you need to see what happened 
and whether it's what's expected.  If in doubt, you can always ask here, 
or just me off the list.

> >  The only drawback of prelinking I know of is that, by the nature of its
> > operation, its incompatible with ASLR, so unless you need this feature the
> > tool may be worth trying.
> 
> That was one of my first suggestions too.  Unfortunately, ASLR is a mandatory
> part of the system in question.

 OK, I see.

> Thank you again for your detailed review and again my apologies for my
> tardy response.  I will once again prod our legal folks to see if we can't
> find some sort of solution.  However, in the meantime, this will remain
> in limbo I'm afraid.  (Sorry.)

 Understood.  I hope you can work out a solution from information provided 
in the course of this discussion.  Please let me know once you've made any 
progress here.

 Regardless, please let me know if you find anything above unclear, or if 
you have any other questions or comments.

  Maciej

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

* [PATCH] MIPS support for --hash-style=gnu
@ 2018-03-06 15:52 Jeremy Stenglein
  0 siblings, 0 replies; 17+ messages in thread
From: Jeremy Stenglein @ 2018-03-06 15:52 UTC (permalink / raw)
  To: binutils, petar.jovanovic, nickc

https://sourceware.org/ml/binutils/2015-10/msg00057.html

Neil wrote these patches 3 years ago for glibc and binutils.  He has
subsequently left Cisco but the patches have never been submitted.

There are major changes required in the patches for submission to latest
glibc and binutils.  Since I don't have time to work on the patches at
moment,
I release all ownership of these patches to the FSF community.  Petar,
please feel free to take up work on these two patches and submit them.

Thanks,
Jeremy

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

* RE: [PATCH] MIPS support for --hash-style=gnu
  2016-01-05  6:10 Koding
@ 2016-01-14 23:01 ` Neil Schellenberger (neschell)
  0 siblings, 0 replies; 17+ messages in thread
From: Neil Schellenberger (neschell) @ 2016-01-14 23:01 UTC (permalink / raw)
  To: Koding, binutils

[Apologies for the long delay: I have been out of the office....]

Hmm, good point.

In a separate email chain, it was also noted that DT_MIPS_SYMTABNO might be used to compute ngnusyms.
I originally avoided DT_MIPS_SYMTABNO only because I wasn't absolutely certain that it was guaranteed 
that it (the number of entries in the .dynsym section) was always going to be the same as the the number of 
entries in chains (plus symidx) so I decided to play it safe.  That may well be needless paranoia on my part.

Opinions?

Regards,
Neil

> -----Original Message-----
> From: Koding [mailto:majiang31312@majiang31312]
> Sent: Tuesday, January 05, 2016 1:11 AM
> To: binutils@sourceware.org
> Subject: Re: [PATCH] MIPS support for --hash-style=gnu
> 
> Hi all,
> 	I am using this patch for a loongson 3A cpu。After some tests , I
> found that there was a mistake that cause many many alignment-faults in
> this patch.
> 	The patch added a Elf32_Word ngnusyms in the head of .gnu.xhash
> section, then "ElfW(Addr)  bitmask[maskwords]"  became unaligned(8-
> bytes).
> 	IMO, we should  change "Elf32_Word ngnusyms" to "Elf32_Xword
> ngnusyms" , or add 4-bytes pad after ngnusyms.

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

* Re: [PATCH] MIPS support for --hash-style=gnu
@ 2016-01-06  2:34 Huang Pei
  0 siblings, 0 replies; 17+ messages in thread
From: Huang Pei @ 2016-01-06  2:34 UTC (permalink / raw)
  To: binutils, ma.jiang, r

Hi, everybody,

     On N64, the layout of .gnu.xhash cause unaligned access at bitmask 
field, so I suggest we remove ngnusyms at the header of the .gnu.xhash 
to avoid the unaligned access. Since MIPS ELF requires DT_MIPS_SYMTABNO 
Tag is present with
dynsymcount, so I think ngnusyms can be taken by dynsymcount - symndx

     // Part 0: .gnu.hash header
     Elf32_Word  nbuckets;  // number of hash table buckets
     Elf32_Word  symndx;  // number of initial .dynsym entires skipped 
in chains[] (and xlat[])
     Elf32_Word  maskwords; // number of ElfW(Addr) words in bitmask
     Elf32_Word  shift2;  // bit shift of hashval for second Bloom 
filter bit
     // Part 1: .gnu.hash Bloom filter
     ElfW(Addr)  bitmask[maskwords];  // 2 bit Bloom filter on hashval
     // Part 2: .gnu.hash buckets
     Elf32_Word  buckets[nbuckets];  // indices into chains[]
     // Part 3: .gnu.hash chains
     Elf32_Word  chains[ngnusyms];  // consecutive hashvals in a given 
bucket; last entry in chain has LSB set
     // Part 4: .gnu.xhash translation table
     Elf32_Word  xlat[ngnusyms];  // parallel to chains[]; index into 
.dynsym

below is the patch removing ngnusyms

https://github.com/heiher/binutils-mips/commit/ceefe8ac5f52285d2c98a43a66c2d7d31574b3f4

https://github.com/heiher/glibc-mips/commit/db33de9af675f9851c0941ca186baf87c79d8ab9

any suggestion?
........

In practice, though, the .gnu.xhash section is virtually identical to 
.gnu.hash. [9]

     // Part 0: .gnu.xhash header
     Elf32_Word  ngnusyms;  // number of entries in chains (and xlat); 
dynsymcount=symndx+ngnusyms
     // Part 1: .gnu.hash header
     Elf32_Word  nbuckets;  // number of hash table buckets
     Elf32_Word  symndx;  // number of initial .dynsym entires skipped 
in chains[] (and xlat[])
     Elf32_Word  maskwords; // number of ElfW(Addr) words in bitmask
     Elf32_Word  shift2;  // bit shift of hashval for second Bloom 
filter bit
     // Part 2: .gnu.hash Bloom filter
     ElfW(Addr)  bitmask[maskwords];  // 2 bit Bloom filter on hashval
     // Part 3: .gnu.hash buckets
     Elf32_Word  buckets[nbuckets];  // indices into chains[]
     // Part 4: .gnu.hash chains
     Elf32_Word  chains[ngnusyms];  // consecutive hashvals in a given 
bucket; last entry in chain has LSB set
     // Part 5: .gnu.xhash translation table
     Elf32_Word  xlat[ngnusyms];  // parallel to chains[]; index into 
.dynsym
..........

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

* Re: [PATCH] MIPS support for --hash-style=gnu
@ 2016-01-05  6:10 Koding
  2016-01-14 23:01 ` Neil Schellenberger (neschell)
  0 siblings, 1 reply; 17+ messages in thread
From: Koding @ 2016-01-05  6:10 UTC (permalink / raw)
  To: binutils

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

Hi all,
	I am using this patch for a loongson 3A cpu。After some tests , I found that there was a mistake that cause many many alignment-faults in this patch.
	The patch added a Elf32_Word ngnusyms in the head of .gnu.xhash section, then "ElfW(Addr)  bitmask[maskwords]"  became unaligned(8-bytes).
	IMO, we should  change "Elf32_Word ngnusyms" to "Elf32_Xword ngnusyms" , or add 4-bytes pad after ngnusyms.

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

end of thread, other threads:[~2018-03-06 15:52 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-05 23:11 [PATCH] MIPS support for --hash-style=gnu Neil Schellenberger (neschell)
2015-10-30 11:09 ` Nick Clifton
2015-11-05 17:14 ` Rafael Espíndola
2015-11-05 18:27   ` Neil Schellenberger (neschell)
2015-11-05 18:45     ` Rafael Espíndola
2015-11-05 23:33       ` Neil Schellenberger (neschell)
2015-11-08 13:52   ` Richard Sandiford
2015-11-09 19:05     ` Neil Schellenberger (neschell)
2016-02-05 22:38 ` Maciej W. Rozycki
2016-04-19 19:15   ` Neil Schellenberger (neschell)
2016-04-20 14:16     ` Joseph Myers
2016-04-20 16:44       ` Jim Wilson
2016-04-21  1:19     ` Maciej W. Rozycki
2016-01-05  6:10 Koding
2016-01-14 23:01 ` Neil Schellenberger (neschell)
2016-01-06  2:34 Huang Pei
2018-03-06 15:52 Jeremy Stenglein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).