RE: Mips target in gold - revision 2 - part 1

[Sasa Stankovic <Sasa dot Stankovic at imgtec dot com>]

Hi Cary,

The updated target-independent patches are attached.

Regards,
Sasa
________________________________________
From: Cary Coutant [ccoutant@google.com]
Sent: Friday, January 31, 2014 7:25 PM
To: Sasa Stankovic
Cc: binutils@sourceware.org; iant@google.com; Petar Jovanovic
Subject: Re: Mips target in gold - revision 2 - part 1

>> You shouldn't need to include "stringpool.h". The set_dynsym_indexes
>> function takes a Stringpool*, so you only need a forward declaration
>> for class Stringpool.
>
> Stringpool is defined as a template in file stringpool.h. If I remove "stringpool.h" then following needs to be added to file target.h:
>
> template<typename Stringpool_char>
> class Stringpool_template;
>
> typedef Stringpool_template<char> Stringpool;

Right. Putting the include in target.h is better.

dynsym.patch:

> 2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * symtab.cc (Symbol_table::set_dynsym_indexes): Allow a target to set
> dynsym indexes.
> * target.h (Target::has_custom_set_dynsym_indexes): New function.
> (Target::set_dynsym_indexes): New function.

+  // Whether the target has a custom set_dynsym_indexes method.
+  virtual bool
+  has_custom_set_dynsym_indexes() const
+  { return false; }
+
+  // Custom set_dynsym_indexes method for a target.
+  virtual unsigned int
+  set_dynsym_indexes(std::vector<Symbol*>*, unsigned int,
std::vector<Symbol*>*,
+                     Stringpool*, Versions*, Symbol_table*) const
+  { gold_unreachable(); }

These public interfaces should be changed to non-virtual functions
that call protected virtual functions (avoid public virtual
interfaces).  The virtual functions typically are named "do_...".
Check most of the other public interfaces in this class for examples.
(For rationale, see the discussion of the NVI idiom in Item 35 in
Scott Meyers' _Effective C++_.)


dynamic-tag.patch:

>  2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * output.cc (Output_data_dynamic::Dynamic_entry::write):
> Get the value of DYNAMIC_CUSTOM dynamic entry.
> * output.h (Output_data_dynamic::add_custom): New function.
> (Dynamic_entry::Dynamic_entry): New constructor for DYNAMIC_CUSTOM
> dynamic entry.
> (enum Dynamic_entry::Classification): Add DYNAMIC_CUSTOM.
> * target.h (Target::dynamic_tag_custom_value): New function.

This is OK.


adjust_dynsym.patch:

>  2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * symtab.cc (Symbol_table::sized_write_globals): Allow a target to
> adjust dynamic symbol value.
> * target.h (Target::adjust_dyn_symbol): New function.

+  // Adjust the value written to the dynamic symbol table.
+  virtual void
+  adjust_dyn_symbol(const Symbol*, unsigned char*) const
+  { }

Change to a non-virtual public interface, as above.


init-output-data.patch:

>  2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * symtab.cc (Sized_symbol<32>::init_output_data):
> Instantiate the template.
> (Sized_symbol<64>::init_output_data): Likewise.

This is OK.


nonvis.patch:

>  2014-01-30  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
>
> * symtab.h (Symbol::set_nonvis): New function.

This is OK.

Thanks!

-cary

diff --git a/src/gold/ChangeLog b/src/gold/ChangeLog
index 5996f3e..c063614 100644
--- a/src/gold/ChangeLog
+++ b/src/gold/ChangeLog
@@ -1,3 +1,12 @@
+2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
+
+	* symtab.cc (Symbol_table::set_dynsym_indexes): Allow a target to set
+	dynsym indexes.
+	* target.h (Target::has_custom_set_dynsym_indexes): New function.
+	(Target::do_has_custom_set_dynsym_indexes): New function.
+	(Target::set_dynsym_indexes): New function.
+	(Target::do_set_dynsym_indexes): New function.
+
 2014-02-05  Cary Coutant  <ccoutant@google.com>
 
 	Fix issues with gold undefined symbol diagnostics.
diff --git a/src/gold/symtab.cc b/src/gold/symtab.cc
index 225856a..d7a20fd 100644
--- a/src/gold/symtab.cc
+++ b/src/gold/symtab.cc
@@ -2371,6 +2371,25 @@ Symbol_table::set_dynsym_indexes(unsigned int index,
 {
   std::vector<Symbol*> as_needed_sym;
 
+  // Allow a target to set dynsym indexes.
+  if (parameters->target().has_custom_set_dynsym_indexes())
+    {
+      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);
+        }
+
+      return parameters->target().set_dynsym_indexes(&dyn_symbols, index, syms,
+                                                     dynpool, versions, this);
+    }
+
   for (Symbol_table_type::iterator p = this->table_.begin();
        p != this->table_.end();
        ++p)
diff --git a/src/gold/target.h b/src/gold/target.h
index 415b7ed..1f9dda0 100644
--- a/src/gold/target.h
+++ b/src/gold/target.h
@@ -37,6 +37,7 @@
 #include "elfcpp.h"
 #include "options.h"
 #include "parameters.h"
+#include "stringpool.h"
 #include "debug.h"
 
 namespace gold
@@ -62,6 +63,7 @@ class Output_section;
 class Input_objects;
 class Task;
 struct Symbol_location;
+class Versions;
 
 // The abstract class for target specific handling.
 
@@ -454,6 +456,21 @@ class Target
   entry_symbol_name() const
   { return this->pti_->entry_symbol_name; }
 
+  // Whether the target has a custom set_dynsym_indexes method.
+  bool
+  has_custom_set_dynsym_indexes() const
+  { return this->do_has_custom_set_dynsym_indexes(); }
+
+  // Custom set_dynsym_indexes method for a target.
+  unsigned int
+  set_dynsym_indexes(std::vector<Symbol*>* dyn_symbols, unsigned int index,
+                     std::vector<Symbol*>* syms, Stringpool* dynpool,
+                     Versions* versions, Symbol_table* symtab) const
+  {
+    return this->do_set_dynsym_indexes(dyn_symbols, index, syms, dynpool,
+                                       versions, symtab);
+  }
+
  protected:
   // This struct holds the constant information for a child class.  We
   // use a struct to avoid the overhead of virtual function calls for
@@ -725,6 +742,18 @@ class Target
   do_gc_mark_symbol(Symbol_table*, Symbol*) const
   { }
 
+  // This may be overridden by the child class.
+  virtual bool
+  do_has_custom_set_dynsym_indexes() const
+  { return false; }
+
+  // This may be overridden by the child class.
+  virtual unsigned int
+  do_set_dynsym_indexes(std::vector<Symbol*>*, unsigned int,
+                        std::vector<Symbol*>*, Stringpool*, Versions*,
+                        Symbol_table*) const
+  { gold_unreachable(); }
+
  private:
   // The implementations of the four do_make_elf_object virtual functions are
   // almost identical except for their sizes and endianness.  We use a template.

diff --git a/src/gold/ChangeLog b/src/gold/ChangeLog
index c063614..fd8de2d 100644
--- a/src/gold/ChangeLog
+++ b/src/gold/ChangeLog
@@ -1,5 +1,9 @@
 2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
 
+	* symtab.h (Symbol::set_nonvis): New function.
+
+2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
+
 	* symtab.cc (Symbol_table::set_dynsym_indexes): Allow a target to set
 	dynsym indexes.
 	* target.h (Target::has_custom_set_dynsym_indexes): New function.
diff --git a/src/gold/symtab.h b/src/gold/symtab.h
index 9aff274..d17f9eb 100644
--- a/src/gold/symtab.h
+++ b/src/gold/symtab.h
@@ -259,6 +259,11 @@ class Symbol
   nonvis() const
   { return this->nonvis_; }
 
+  // Set the non-visibility part of the st_other field.
+  void
+  set_nonvis(unsigned int nonvis)
+  { this->nonvis_ = nonvis; }
+
   // Return whether this symbol is a forwarder.  This will never be
   // true of a symbol found in the hash table, but may be true of
   // symbol pointers attached to object files.

diff --git a/src/gold/ChangeLog b/src/gold/ChangeLog
index fd8de2d..bae8d2c 100644
--- a/src/gold/ChangeLog
+++ b/src/gold/ChangeLog
@@ -1,5 +1,16 @@
 2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
 
+	* output.cc (Output_data_dynamic::Dynamic_entry::write):
+	Get the value of DYNAMIC_CUSTOM dynamic entry.
+	* output.h (Output_data_dynamic::add_custom): New function.
+	(Dynamic_entry::Dynamic_entry): New constructor for DYNAMIC_CUSTOM
+	dynamic entry.
+	(enum Dynamic_entry::Classification): Add DYNAMIC_CUSTOM.
+	* target.h (Target::dynamic_tag_custom_value): New function.
+	(Target::do_dynamic_tag_custom_value): New function.
+
+2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
+
 	* symtab.h (Symbol::set_nonvis): New function.
 
 2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
diff --git a/src/gold/output.cc b/src/gold/output.cc
index 348ad64..abe0629 100644
--- a/src/gold/output.cc
+++ b/src/gold/output.cc
@@ -1797,6 +1797,10 @@ Output_data_dynamic::Dynamic_entry::write(
       val = pool->get_offset(this->u_.str);
       break;
 
+    case DYNAMIC_CUSTOM:
+      val = parameters->target().dynamic_tag_custom_value(this->tag_);
+      break;
+
     default:
       val = this->u_.od->address() + this->offset_;
       break;
diff --git a/src/gold/output.h b/src/gold/output.h
index 574d270..1cba0b7 100644
--- a/src/gold/output.h
+++ b/src/gold/output.h
@@ -2578,6 +2578,11 @@ class Output_data_dynamic : public Output_section_data
   add_string(elfcpp::DT tag, const std::string& str)
   { this->add_string(tag, str.c_str()); }
 
+  // Add a new dynamic entry with custom value.
+  void
+  add_custom(elfcpp::DT tag)
+  { this->add_entry(Dynamic_entry(tag)); }
+
  protected:
   // Adjust the output section to set the entry size.
   void
@@ -2642,6 +2647,11 @@ class Output_data_dynamic : public Output_section_data
       : tag_(tag), offset_(DYNAMIC_STRING)
     { this->u_.str = str; }
 
+    // Create an entry with a custom value.
+    Dynamic_entry(elfcpp::DT tag)
+      : tag_(tag), offset_(DYNAMIC_CUSTOM)
+    { }
+
     // Return the tag of this entry.
     elfcpp::DT
     tag() const
@@ -2665,7 +2675,9 @@ class Output_data_dynamic : public Output_section_data
       // Symbol adress.
       DYNAMIC_SYMBOL = -3U,
       // String.
-      DYNAMIC_STRING = -4U
+      DYNAMIC_STRING = -4U,
+      // Custom value.
+      DYNAMIC_CUSTOM = -5U
       // Any other value indicates a section address plus OFFSET.
     };
 
diff --git a/src/gold/target.h b/src/gold/target.h
index 1f9dda0..dc09058 100644
--- a/src/gold/target.h
+++ b/src/gold/target.h
@@ -471,6 +471,11 @@ class Target
                                        versions, symtab);
   }
 
+  // Get the custom dynamic tag value.
+  unsigned int
+  dynamic_tag_custom_value(elfcpp::DT tag) const
+  { return this->do_dynamic_tag_custom_value(tag); }
+
  protected:
   // This struct holds the constant information for a child class.  We
   // use a struct to avoid the overhead of virtual function calls for
@@ -754,6 +759,11 @@ class Target
                         Symbol_table*) const
   { gold_unreachable(); }
 
+  // This may be overridden by the child class.
+  virtual unsigned int
+  do_dynamic_tag_custom_value(elfcpp::DT) const
+  { gold_unreachable(); }
+
  private:
   // The implementations of the four do_make_elf_object virtual functions are
   // almost identical except for their sizes and endianness.  We use a template.

diff --git a/src/gold/ChangeLog b/src/gold/ChangeLog
index bae8d2c..af86fcc 100644
--- a/src/gold/ChangeLog
+++ b/src/gold/ChangeLog
@@ -1,5 +1,11 @@
 2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
 
+	* symtab.cc (Sized_symbol<32>::init_output_data):
+	Instantiate the template.
+	(Sized_symbol<64>::init_output_data): Likewise.
+
+2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
+
 	* output.cc (Output_data_dynamic::Dynamic_entry::write):
 	Get the value of DYNAMIC_CUSTOM dynamic entry.
 	* output.h (Output_data_dynamic::add_custom): New function.
diff --git a/src/gold/symtab.cc b/src/gold/symtab.cc
index d7a20fd..23aea0c 100644
--- a/src/gold/symtab.cc
+++ b/src/gold/symtab.cc
@@ -3642,6 +3642,32 @@ Symbol_table::define_with_copy_reloc<64>(
     elfcpp::Elf_types<64>::Elf_Addr value);
 #endif
 
+#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_32_BIG)
+template
+void
+Sized_symbol<32>::init_output_data(const char* name, const char* version,
+				   Output_data* od, Value_type value,
+				   Size_type symsize, elfcpp::STT type,
+				   elfcpp::STB binding,
+				   elfcpp::STV visibility,
+				   unsigned char nonvis,
+				   bool offset_is_from_end,
+				   bool is_predefined);
+#endif
+
+#if defined(HAVE_TARGET_64_LITTLE) || defined(HAVE_TARGET_64_BIG)
+template
+void
+Sized_symbol<64>::init_output_data(const char* name, const char* version,
+				   Output_data* od, Value_type value,
+				   Size_type symsize, elfcpp::STT type,
+				   elfcpp::STB binding,
+				   elfcpp::STV visibility,
+				   unsigned char nonvis,
+				   bool offset_is_from_end,
+				   bool is_predefined);
+#endif
+
 #ifdef HAVE_TARGET_32_LITTLE
 template
 void

diff --git a/src/gold/ChangeLog b/src/gold/ChangeLog
index af86fcc..35a3464 100644
--- a/src/gold/ChangeLog
+++ b/src/gold/ChangeLog
@@ -1,5 +1,12 @@
 2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
 
+	* symtab.cc (Symbol_table::sized_write_globals): Allow a target to
+	adjust dynamic symbol value.
+	* target.h (Target::adjust_dyn_symbol): New function.
+	(Target::do_adjust_dyn_symbol): New function.
+
+2014-02-07  Sasa Stankovic  <Sasa.Stankovic@imgtec.com>
+
 	* symtab.cc (Sized_symbol<32>::init_output_data):
 	Instantiate the template.
 	(Sized_symbol<64>::init_output_data): Likewise.
diff --git a/src/gold/symtab.cc b/src/gold/symtab.cc
index 23aea0c..e921544 100644
--- a/src/gold/symtab.cc
+++ b/src/gold/symtab.cc
@@ -3012,6 +3012,8 @@ Symbol_table::sized_write_globals(const Stringpool* sympool,
 	  unsigned char* pd = dynamic_view + (dynsym_index * sym_size);
 	  this->sized_write_symbol<size, big_endian>(sym, dynsym_value, shndx,
 						     binding, dynpool, pd);
+          // Allow a target to adjust dynamic symbol value.
+          parameters->target().adjust_dyn_symbol(sym, pd);
 	}
     }
 
diff --git a/src/gold/target.h b/src/gold/target.h
index dc09058..aa249ca 100644
--- a/src/gold/target.h
+++ b/src/gold/target.h
@@ -476,6 +476,11 @@ class Target
   dynamic_tag_custom_value(elfcpp::DT tag) const
   { return this->do_dynamic_tag_custom_value(tag); }
 
+  // Adjust the value written to the dynamic symbol table.
+  void
+  adjust_dyn_symbol(const Symbol* sym, unsigned char* view) const
+  { this->do_adjust_dyn_symbol(sym, view); }
+
  protected:
   // This struct holds the constant information for a child class.  We
   // use a struct to avoid the overhead of virtual function calls for
@@ -764,6 +769,11 @@ class Target
   do_dynamic_tag_custom_value(elfcpp::DT) const
   { gold_unreachable(); }
 
+  // This may be overridden by the child class.
+  virtual void
+  do_adjust_dyn_symbol(const Symbol*, unsigned char*) const
+  { }
+
  private:
   // The implementations of the four do_make_elf_object virtual functions are
   // almost identical except for their sizes and endianness.  We use a template.

diff --git a/src/gold/mips.cc b/src/gold/mips.cc
index 6b81a38..50bac77 100644
--- a/src/gold/mips.cc
+++ b/src/gold/mips.cc
@@ -3160,7 +3160,7 @@ class Target_mips : public Sized_target<size, big_endian>
   }
 
   bool
-  has_custom_set_dynsym_indexes() const
+  do_has_custom_set_dynsym_indexes() const
   { return true; }
 
   void
@@ -3192,9 +3192,9 @@ class Target_mips : public Sized_target<size, big_endian>
   // vector SYMS.  The names are added to DYNPOOL.  This returns an
   // updated dynamic symbol index.
   unsigned int
-  set_dynsym_indexes(std::vector<Symbol*>* dyn_symbols, unsigned int index,
-                     std::vector<Symbol*>* syms, Stringpool* dynpool,
-                     Versions* versions, Symbol_table* symtab) const
+  do_set_dynsym_indexes(std::vector<Symbol*>* dyn_symbols, unsigned int index,
+                        std::vector<Symbol*>* syms, Stringpool* dynpool,
+                        Versions* versions, Symbol_table* symtab) const
   {
     std::vector<Symbol*> non_got_symbols;
     std::vector<Symbol*> got_symbols;
@@ -3372,11 +3372,11 @@ class Target_mips : public Sized_target<size, big_endian>
 
   // Get the custom dynamic tag value.
   unsigned int
-  dynamic_tag_custom_value(elfcpp::DT) const;
+  do_dynamic_tag_custom_value(elfcpp::DT) const;
 
   // Adjust the value written to the dynamic symbol table.
   virtual void
-  adjust_dyn_symbol(const Symbol* sym, unsigned char* view) const
+  do_adjust_dyn_symbol(const Symbol* sym, unsigned char* view) const
   {
     elfcpp::Sym<size, big_endian> isym(view);
     elfcpp::Sym_write<size, big_endian> osym(view);
@@ -8354,7 +8354,7 @@ Target_mips<size, big_endian>::do_finalize_sections(Layout* layout,
 // Get the custom dynamic tag value.
 template<int size, bool big_endian>
 unsigned int
-Target_mips<size, big_endian>::dynamic_tag_custom_value(elfcpp::DT tag) const
+Target_mips<size, big_endian>::do_dynamic_tag_custom_value(elfcpp::DT tag) const
 {
   Output_section* dynsym = this->layout_->find_output_section(".dynsym");
   gold_assert(dynsym != NULL);