Re: [GOLD] PowerPC tls_get_addr_optimize

[Alan Modra <amodra at gmail dot com>]

On Thu, Aug 03, 2017 at 10:33:16PM -0700, Cary Coutant wrote:
> > I decided to not
> > template clone because treating its arguments as Sized_symbol isn't
> > correct given that mips derives its own symbol class.  Passing in a
> > size at least gives a nod towards supporting clone on derived
> > classes.
> 
> Cloning with memcpy would preclude us from ever putting a member in
> the symbol class that has a custom assignment operator, right? (Not
> that I ever would want to do that -- Symbol is supposed to be
> lightweight -- but I still don't want to build in gotchas like that.)
> 
> I'd prefer to template it and use a real assignment rather than
> passing in the size and using memcpy. Since it's only called from the
> target code, the caller will know what symbol type to pass in as the
> template parameter, and if a target with its own symbol class needs to
> use it, it can provide a clone() override in its derived class.
> 
> I'm prepared to reconsider if you still disagree.

Delta from the last patch, to see if you still like it this way.  I
don't pretend to be a C++ programmer, so don't really have an opinion
worth that much.  :)  Note removal of private operator=


diff --git a/gold/powerpc.cc b/gold/powerpc.cc
index b9f0012..79d35f8 100644
--- a/gold/powerpc.cc
+++ b/gold/powerpc.cc
@@ -2523,8 +2523,13 @@ Target_powerpc<size, big_endian>::do_define_standard_symbols(
       // a non-dynamic definition.
       else if (this->tls_get_addr_opt_->is_undefined()
 	       || this->tls_get_addr_opt_->is_from_dynobj())
-	symtab->clone(this->tls_get_addr_opt_, this->tls_get_addr_,
-		      sizeof (Sized_symbol<size>));
+	{
+	  Sized_symbol<size>* from
+	    = static_cast<Sized_symbol<size>*>(this->tls_get_addr_);
+	  Sized_symbol<size>* to
+	    = static_cast<Sized_symbol<size>*>(this->tls_get_addr_opt_);
+	  symtab->clone<Sized_symbol<size> >(to, from);
+	}
     }
 }
 
diff --git a/gold/resolve.cc b/gold/resolve.cc
index 5a35f34..b31f0b3 100644
--- a/gold/resolve.cc
+++ b/gold/resolve.cc
@@ -921,8 +921,9 @@ Symbol_table::report_resolve_problem(bool is_error, const char* msg,
 // version_, and is_forced_local_ flag are copied.  version_ is
 // cleared if from->version_ is clear.  Returns true if this symbol
 // should be forced local.
+template<typename Sym_type>
 bool
-Symbol::clone(const Symbol* from, size_t symbol_size)
+Symbol::clone(const Sym_type* from)
 {
   // Don't allow cloning after dynamic linking info is attached to symbols.
   // We aren't prepared to merge such.
@@ -934,7 +935,8 @@ Symbol::clone(const Symbol* from, size_t symbol_size)
   const char* name = this->name_;
   const char* version = this->version_;
   bool was_forced_local = this->is_forced_local_;
-  memcpy (this, from, symbol_size);
+  Sym_type* to = static_cast<Sym_type*>(this);
+  *to = *from;
   this->name_ = name;
   if (from->version_)
     this->version_ = version;
@@ -1144,4 +1146,11 @@ Symbol_table::override_with_special<64>(Sized_symbol<64>*,
 					const Sized_symbol<64>*);
 #endif
 
+template
+bool
+Symbol::clone<Sized_symbol<32> >(const Sized_symbol<32>*);
+
+template
+bool
+Symbol::clone<Sized_symbol<64> >(const Sized_symbol<64>*);
 } // End namespace gold.
diff --git a/gold/symtab.h b/gold/symtab.h
index 635405a..f34e394 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -902,8 +902,9 @@ class Symbol
   // version_, and is_forced_local_ flag are copied.  version_ is
   // cleared if from->version_ is clear.  Returns true if this symbol
   // should be forced local.
+  template<typename Sym_type>
   bool
-  clone(const Symbol* from, size_t symbol_size);
+  clone(const Sym_type* from);
 
  protected:
   // Instances of this class should always be created at a specific
@@ -975,7 +976,6 @@ class Symbol
 
  private:
   Symbol(const Symbol&);
-  Symbol& operator=(const Symbol&);
 
   // Symbol name (expected to point into a Stringpool).
   const char* name_;
@@ -1199,7 +1199,6 @@ class Sized_symbol : public Symbol
 
  private:
   Sized_symbol(const Sized_symbol&);
-  Sized_symbol& operator=(const Sized_symbol&);
 
   // Symbol value.  Before Layout::finalize this is the offset in the
   // input section.  This is set to the final value during
@@ -1703,10 +1702,11 @@ class Symbol_table
   { return version_script_; }
 
   // Completely override existing symbol.
+  template<typename Sym_type>
   void
-  clone(Symbol* to, const Symbol* from, size_t symbol_size)
+  clone(Sym_type* to, const Sym_type* from)
   {
-    if (to->clone(from, symbol_size))
+    if (to->clone<Sym_type>(from))
       this->force_local(to);
   }
 

-- 
Alan Modra
Australia Development Lab, IBM