Re: Mips target in gold - part 3

[Cary Coutant <ccoutant at google dot com>]

> 2. dynsym.patch
> This patch allows a target to set dynamic symbol indexes. Mips ABI requires that the order of symbols in .got matches the order of dynamic symbols. The patch adds a call to a target hook at the beginning of the method Symbol_table::set_dynsym_indexes. Mips implementation of the method set_dynsym_indexes is almost identical to Symbol_table::set_dynsym_indexes, except that at the start of the method dynamic symbols are ordered based on their position in the .got.

Needs a ChangeLog entry.

--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -2369,12 +2369,29 @@ Symbol_table::set_dynsym_indexes(unsigned int index,
  Versions* versions)
 {
   std::vector<Symbol*> as_needed_sym;
+  std::vector<Symbol*> dyn_symbols;

   for (Symbol_table_type::iterator p = this->table_.begin();
        p != this->table_.end();
        ++p)
     {
       Symbol* sym = p->second;
+      if (!sym->should_add_dynsym_entry(this))
+        sym->set_dynsym_index(-1U);
+      else
+        dyn_symbols.push_back(sym);
+    }
+
+  // Allow a target to set dynsym indexes.
+  if (parameters->target().custom_set_dynsym_indexes())
+    return parameters->target().set_dynsym_indexes(&dyn_symbols, index, syms,
+                                                   dynpool, versions, this);
+
+  for (std::vector<Symbol *>::iterator p = dyn_symbols.begin();
+       p != dyn_symbols.end();
+       ++p)
+    {
+      Symbol* sym = *p;

       // Note that SYM may already have a dynamic symbol index, since
       // some symbols appear more than once in the symbol table, with

At this point, the existing code is going to check for
!sym->should_add_dynsym_entry, but now that you're preprocessing the
symbol table and making a second loop over just dynsyms, you already
know the answer to the question.

Since you need the dyn_symbols vector only when the target has a
custom set_dynsym_indexes method, I'd suggest putting the whole first
loop inside the check for custom_set_dynsym_indexes, and leaving the
original loop unchanged. The other targets shouldn't have to spend the
time building a new list of dynsyms.


--- a/gold/target.h
+++ b/gold/target.h
@@ -453,6 +455,15 @@ class Target
   entry_symbol_name() const
   { return "_start"; }

+  virtual bool
+  custom_set_dynsym_indexes() const
+  { return false; }

Predicates like this should begin with a word like "is", "has", or
"can", that makes is clear the function is testing for some condition
rather than actually setting custom dynsym indexes. In this case, I
think it should be "has", as in "this target HAS a custom
set_dynsym_indexes method."

-cary