[PATCH] gold/arm: define $a/$d markers in .plt

[PATCH] gold/arm: define $a/$d markers in .plt

[Roland McGrath]

Ian: Cary seems to like this version of the patch but it needs your sign-off.

When gold generates an ARM .plt section, it fails to define any magic
marker symbols (local symbols named $a or $d).  These are necessary to
make objdump -d disassemble the .plt contents rather than just showing
them as data words.

This patch makes it generate the necessary marker symbols at the start
of the .plt section.

The Symbol_table changes are all based on Cary's advice (on the binutils
list).  I really can't explain or vouch for this being the best way to
enable the missing functionality: defining synthetic local symbols
without regard to preexisting symbols of the same name.  Cary thinks
that do_define_in_output_segment and do_define_as_constant should get
the same treatment (though there are so far no uses of those that need
it); he also agrees with me that the boilerplate chunk (21 lines that
becomes 32 lines) that calls define_special_symbol (or doesn't) should
be factored out rather than repeated three times, for maintainability;
we both agree that those improvements can come after this change goes in
(and I think Cary has implicitly volunteered to do that cleanup).

It was Cary's suggestion that I change define_in_output_data this way
rather than adding a separate method.  But one thing I'm not entirely
sure about is that I think this means that _GLOBAL_OFFSET_TABLE_ and
__rel{,a}_iplt_{start,end} cannot be referenced by name any more.  It
might be the case that things expect to use _GLOBAL_OFFSET_TABLE_ by
name, and it's certainly true that __rel{,a}_iplt_{start,end} are used
by name (that's why they exist).  Cary's proposal to consolidate this
behavior in do_define_in_output_segment as well would make it affect
_TLS_MODULE_BASE_ too.  I don't know how that symbol is used.

No regressions in 'make check' (x86_64-linux-gnu host).  I verified the new
functionality with a trivial ARM link that generates some PLT entries.

Ok for trunk?


Thanks,
Roland


gold/
2012-04-19  Roland McGrath  <mcgrathr@google.com>

	* arm.cc (Output_data_plt_arm::add_marker_symbols): New method.
	(Output_data_plt_arm::add_marker_symbol): New method.
	(Target_arm::make_plt_entry): Call it.

	* symtab.h (Symbol_table::Forced_locals): Rename to ...
	(Symbol_table::Symbol_vector): ... this.
	(Symbol_table::forced_locals_): Update type.
	(Symbol_table::generated_locals_): New member.
	* symtab.cc (Symbol_table::Symbol_table): Initialize it.
	(Symbol_table::do_define_in_output_data): Don't use
	define_special_symbol for STB_LOCAL.  Instead, just create a symbol
	and add it to generated_locals_.
	(Symbol_table::sized_finalize): Update use of type name.
	Add generated_locals_ to symtab similar to forced_locals_.
	(Symbol_table::sized_write_globals): Write generated_locals_ out.

diff --git a/gold/arm.cc b/gold/arm.cc
index dc6e64a..08d0728 100644
--- a/gold/arm.cc
+++ b/gold/arm.cc
@@ -7223,8 +7223,31 @@ class Output_data_plt_arm : public Output_section_data
   get_plt_entry_size()
   { return sizeof(plt_entry); }
 
+  // Define marker symbols at the start of the PLT so that
+  // objdump will know to disassemble it as ARM instructions.
+  void
+  add_marker_symbols(Output_section* os, Symbol_table* symtab)
+  {
+    // The last word of first_plt_entry is data.
+    // The leading words, and all of subsequent entries, are ARM instructions.
+    this->add_marker_symbol(os, symtab, 0, false);
+    this->add_marker_symbol(os, symtab, 16, true);
+    this->add_marker_symbol(os, symtab, 20, false);
+  }
+
  protected:
   void
+  add_marker_symbol(Output_section* os, Symbol_table* symtab,
+		    Arm_address offset, bool is_data)
+  {
+    symtab->define_in_output_data(is_data ? "$d" : "$a", NULL,
+				  Symbol_table::PREDEFINED, os,
+				  offset, 0, elfcpp::STT_NOTYPE,
+				  elfcpp::STB_LOCAL, elfcpp::STV_DEFAULT, 0,
+				  false, false);
+  }
+
+  void
   do_adjust_output_section(Output_section* os);
 
   // Write to a map file.
@@ -7430,10 +7453,14 @@ Target_arm<big_endian>::make_plt_entry(Symbol_table* symtab, Layout* layout,
       this->got_section(symtab, layout);
 
       this->plt_ = new Output_data_plt_arm<big_endian>(layout, this->got_plt_);
-      layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
+
+      Output_section* os =
+          layout->add_output_section_data (".plt", elfcpp::SHT_PROGBITS,
 				      (elfcpp::SHF_ALLOC
 				       | elfcpp::SHF_EXECINSTR),
 				      this->plt_, ORDER_PLT, false);
+
+      this->plt_->add_marker_symbols(os, symtab);
     }
   this->plt_->add_entry(gsym);
 }
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 1edb88d..804ea09 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -1,6 +1,6 @@
 // symtab.cc -- the gold symbol table
 
-// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
 // Written by Ian Lance Taylor <iant@google.com>.
 
 // This file is part of gold.
@@ -532,7 +532,7 @@ Symbol_table::Symbol_table(unsigned int count,
                            const Version_script_info& version_script)
   : saw_undefined_(0), offset_(0), table_(count), namepool_(),
     forwarders_(), commons_(), tls_commons_(), small_commons_(),
-    large_commons_(), forced_locals_(), warnings_(),
+    large_commons_(), forced_locals_(), generated_locals_(), warnings_(),
     version_script_(version_script), gc_(NULL), icf_(NULL)
 {
   namepool_.reserve(count);
@@ -1884,7 +1884,41 @@ Symbol_table::do_define_in_output_data(
   Sized_symbol<size>* oldsym;
   bool resolve_oldsym;
 
-  if (parameters->target().is_big_endian())
+  if (binding == elfcpp::STB_LOCAL)
+    {
+      // This is a special local symbol, so it doesn't need to be
+      // in the symbol table by name.  Just make a naked symbol.
+      // We'll add it to the generated_locals_ list below.
+      const Target& target = parameters->target();
+      if (!target.has_make_symbol())
+	sym = new Sized_symbol<size>();
+      else
+	{
+	  if (parameters->target().is_big_endian())
+	    {
+#if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
+	      Sized_target<size, true>* sized_target =
+		parameters->sized_target<size, true>();
+	      sym = sized_target->make_symbol();
+#else
+	      gold_unreachable();
+#endif
+	    }
+	  else
+	    {
+#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_64_LITTLE)
+	      Sized_target<size, false>* sized_target =
+		parameters->sized_target<size, false>();
+	      sym = sized_target->make_symbol();
+#else
+	      gold_unreachable();
+#endif
+	    }
+	}
+
+      oldsym = NULL;
+    }
+  else if (parameters->target().is_big_endian())
     {
 #if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
       sym = this->define_special_symbol<size, true>(&name, &version,
@@ -1914,8 +1948,9 @@ Symbol_table::do_define_in_output_data(
 
   if (oldsym == NULL)
     {
-      if (binding == elfcpp::STB_LOCAL
-	  || this->version_script_.symbol_is_local(name))
+      if (binding == elfcpp::STB_LOCAL)
+	this->generated_locals_.push_back(sym);
+      else if (this->version_script_.symbol_is_local(name))
 	this->force_local(sym);
       else if (version != NULL)
 	sym->set_is_default();
@@ -2500,9 +2535,23 @@ Symbol_table::sized_finalize(off_t off, Stringpool* pool,
   unsigned int index = *plocal_symcount;
   const unsigned int orig_index = index;
 
-  // First do all the symbols which have been forced to be local, as
+  // First do all the symbols which have been generated as local, as
   // they must appear before all global symbols.
-  for (Forced_locals::iterator p = this->forced_locals_.begin();
+  for (Symbol_vector::iterator p = this->generated_locals_.begin();
+       p != this->generated_locals_.end();
+       ++p)
+    {
+      Symbol* sym = *p;
+      if (this->sized_finalize_symbol<size>(sym))
+	{
+	  this->add_to_final_symtab<size>(sym, pool, &index, &off);
+	  ++*plocal_symcount;
+	}
+    }
+
+  // Next do all the symbols which have been forced to be local.
+  // They too must appear before all global symbols.
+  for (Symbol_vector::iterator p = this->forced_locals_.begin();
        p != this->forced_locals_.end();
        ++p)
     {
@@ -2976,6 +3025,36 @@ Symbol_table::sized_write_globals(const Stringpool* sympool,
 	}
     }
 
+  // Next do all the symbols which have been generated as local.
+  for (Symbol_vector::const_iterator p = this->generated_locals_.begin();
+       p != this->generated_locals_.end();
+       ++p)
+    {
+      Sized_symbol<size>* sym = static_cast<Sized_symbol<size>*>(*p);
+      unsigned int sym_index = sym->symtab_index();
+      unsigned int shndx = sym->output_data()->out_shndx();
+      typename elfcpp::Elf_types<size>::Elf_Addr sym_value = sym->value();
+
+      gold_assert(sym->binding() == elfcpp::STB_LOCAL);
+
+      if (shndx >= elfcpp::SHN_LORESERVE)
+	{
+	  if (sym_index != -1U)
+	    symtab_xindex->add(sym_index, shndx);
+	  shndx = elfcpp::SHN_XINDEX;
+	}
+
+      if (sym_index != -1U)
+	{
+	  sym_index -= first_global_index;
+	  gold_assert(sym_index < output_count);
+	  unsigned char* ps = psyms + (sym_index * sym_size);
+	  this->sized_write_symbol<size, big_endian>(sym, sym_value, shndx,
+						     elfcpp::STB_LOCAL,
+						     sympool, ps);
+	}
+    }
+
   of->write_output_view(this->offset_, oview_size, psyms);
   if (dynamic_view != NULL)
     of->write_output_view(this->dynamic_offset_, dynamic_size, dynamic_view);
diff --git a/gold/symtab.h b/gold/symtab.h
index feed245..94942b2 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -1,6 +1,6 @@
 // symtab.h -- the gold symbol table   -*- C++ -*-
 
-// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
 // Written by Ian Lance Taylor <iant@google.com>.
 
 // This file is part of gold.
@@ -1822,8 +1822,8 @@ class Symbol_table
   sized_write_section_symbol(const Output_section*, Output_symtab_xindex*,
 			     Output_file*, off_t) const;
 
-  // The type of the list of symbols which have been forced local.
-  typedef std::vector<Symbol*> Forced_locals;
+  // The type of a simple vector of symbols.
+  typedef std::vector<Symbol*> Symbol_vector;
 
   // A map from symbols with COPY relocs to the dynamic objects where
   // they are defined.
@@ -1871,7 +1871,9 @@ class Symbol_table
   // A list of symbols which have been forced to be local.  We don't
   // expect there to be very many of them, so we keep a list of them
   // rather than walking the whole table to find them.
-  Forced_locals forced_locals_;
+  Symbol_vector forced_locals_;
+  // A list of symbols that were generated as local.
+  Symbol_vector generated_locals_;
   // Manage symbol warnings.
   Warnings warnings_;
   // Manage potential One Definition Rule (ODR) violations.

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Cary Coutant]

> It was Cary's suggestion that I change define_in_output_data this way
> rather than adding a separate method.  But one thing I'm not entirely
> sure about is that I think this means that _GLOBAL_OFFSET_TABLE_ and
> __rel{,a}_iplt_{start,end} cannot be referenced by name any more.  It
> might be the case that things expect to use _GLOBAL_OFFSET_TABLE_ by
> name, and it's certainly true that __rel{,a}_iplt_{start,end} are used
> by name (that's why they exist).  Cary's proposal to consolidate this
> behavior in do_define_in_output_segment as well would make it affect
> _TLS_MODULE_BASE_ too.  I don't know how that symbol is used.

I'd argue that if those symbols need to be referenced by name, they
should be defined as STB_GLOBAL instead of STB_LOCAL. I could be
wrong, though. If I am, sorry, I guess you'll need to revert to the
separate method you had before.

-cary

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Roland McGrath]

On Thu, Apr 19, 2012 at 5:39 PM, Cary Coutant <ccoutant@google.com> wrote:
> I'd argue that if those symbols need to be referenced by name, they
> should be defined as STB_GLOBAL instead of STB_LOCAL. I could be
> wrong, though. If I am, sorry, I guess you'll need to revert to the
> separate method you had before.

These are forced-local cases.  They are considered global at link time,
but must not be global in the dynamic symbol table.  I verified that my
change is indeed a regression for a direct reference to _GLOBAL_OFFSET_TABLE_.

I'm not going to bother reworking the patch yet again until we hear from
Ian that one flavor or another would actually be accepted.


Thanks,
Roland

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Ian Lance Taylor]

Roland McGrath <mcgrathr@google.com> writes:

> The Symbol_table changes are all based on Cary's advice (on the binutils
> list).  I really can't explain or vouch for this being the best way to
> enable the missing functionality: defining synthetic local symbols
> without regard to preexisting symbols of the same name.  Cary thinks
> that do_define_in_output_segment and do_define_as_constant should get
> the same treatment (though there are so far no uses of those that need
> it); he also agrees with me that the boilerplate chunk (21 lines that
> becomes 32 lines) that calls define_special_symbol (or doesn't) should
> be factored out rather than repeated three times, for maintainability;
> we both agree that those improvements can come after this change goes in
> (and I think Cary has implicitly volunteered to do that cleanup).

The three functions (do_define_in_output_data,
do_define_in_output_segment, and do_define_as_constant) all need to act
the same way.  You are keying off of binding == STB_LOCAL to omit adding
the name to the global symbol table, but as you've discovered that mixes
together two unrelated cases.  Your case is a symbol that is always
local and is never referenced by name.  The other case, the one
currently supported, is a symbol that is global but is forced local by a
version script.  A symbol like _GLOBAL_OFFSET_TABLE_ is the latter.

So I think your code needs to add a new value to the Defined enum,
PREDEFINED_UNREFERENCED or something, to add a new local symbol with no
globally visible name.

Then you can handle that in define_special_symbol if you like, or via
some other helper function that calls define_special_symbol.


> -      layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
> +
> +      Output_section* os =
> +          layout->add_output_section_data (".plt", elfcpp::SHT_PROGBITS,
>  				      (elfcpp::SHF_ALLOC
>  				       | elfcpp::SHF_EXECINSTR),
>  				      this->plt_, ORDER_PLT, false);

No space before '('.


> diff --git a/gold/symtab.cc b/gold/symtab.cc
> index 1edb88d..804ea09 100644
> --- a/gold/symtab.cc
> +++ b/gold/symtab.cc
> @@ -1,6 +1,6 @@
>  // symtab.cc -- the gold symbol table
>  
> -// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
> +// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.

In general GCC and BFD keep the spaces between years and instead break
the line before "Free Software Foundation," and gold should do the
same.  Same in symtab.h.



> +  // Next do all the symbols which have been generated as local.

Seems to me that this loop should be before the loop over the entire
symbol table, not after.


> -  // The type of the list of symbols which have been forced local.
> -  typedef std::vector<Symbol*> Forced_locals;
> +  // The type of a simple vector of symbols.
> +  typedef std::vector<Symbol*> Symbol_vector;

I don't like using words like "vector" as part of a type, because they
imply a specific container implementation when the point should be what
the container holds.  Just call this type Symbols, which happens to
match the type Object::Symbols.

Ian

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Roland McGrath]

I'm going to leave this to Doug Kwan, who told me it's already on his
to-do list.


Thanks,
Roland

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Cary Coutant]

> The three functions (do_define_in_output_data,
> do_define_in_output_segment, and do_define_as_constant) all need to act
> the same way.  You are keying off of binding == STB_LOCAL to omit adding
> the name to the global symbol table, but as you've discovered that mixes
> together two unrelated cases.  Your case is a symbol that is always
> local and is never referenced by name.  The other case, the one
> currently supported, is a symbol that is global but is forced local by a
> version script.  A symbol like _GLOBAL_OFFSET_TABLE_ is the latter.
>
> So I think your code needs to add a new value to the Defined enum,
> PREDEFINED_UNREFERENCED or something, to add a new local symbol with no
> globally visible name.
>
> Then you can handle that in define_special_symbol if you like, or via
> some other helper function that calls define_special_symbol.

How about instead adding a "force_local" flag to these three
functions, then using STB_GLOBAL with force_local == true for
_GLOBAL_OFFSET_TABLE_ and similar symbols? I think it's misleading to
use STB_LOCAL for a symbol that actually does have global scope
(within the link).

-cary

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[David Miller]

From: Cary Coutant <ccoutant@google.com>
Date: Wed, 25 Apr 2012 11:14:16 -0700

> How about instead adding a "force_local" flag to these three
> functions, then using STB_GLOBAL with force_local == true for
> _GLOBAL_OFFSET_TABLE_ and similar symbols? I think it's misleading to
> use STB_LOCAL for a symbol that actually does have global scope
> (within the link).

BTW, this reminds me of something I noticed while working on Sparc
IFUNC support.

GOLD downgrades a protected symbol from STB_GLOBAL to STB_LOCAL in the
final link, which I've not seen the BFD linker do.

This has potential implications on sparc, because STB_LOCAL symbols
have their relocations calculated differently than STB_GLOBAL ones
in the dynamic-linker.

The difference is that for STB_LOCAL symbols, the dynamic-linker
assumes that the symbol value is present in the relocation's addend.
Therefore it uses the map address as the relocation value for this
case.

This is why I have that test on gsym->forced_local() in the
Target_sparc::Scan::global() code which is trying to figure out
whether we should use R_SPARC_GLOB_DAT or not.

I even added an assertion and detailed comment to that code explaining
this situation:

--------------------
		|| (gsym->type() == elfcpp::STT_GNU_IFUNC
		    && parameters->options().output_is_position_independent()
		    && !gsym->is_forced_local()))
	      {
		unsigned int r_type = elfcpp::R_SPARC_GLOB_DAT;

		// If this symbol is forced local, this relocation will
		// not work properly.  That's because ld.so on sparc
		// (and 32-bit powerpc) expects st_value in the r_addend
		// of relocations for STB_LOCAL symbols.  Curiously the
		// BFD linker does not promote global hidden symbols to be
		// STB_LOCAL in the dynamic symbol table like Gold does.
		gold_assert(!gsym->is_forced_local());
		got->add_global_with_rel(gsym, GOT_TYPE_STANDARD, rela_dyn,
					 r_type);
	      }
--------------------

Is it really such a good idea to perform this change from STB_GLOBAL
to STB_LOCAL for symbols with protected visibility?  What does it buy
us?

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Ian Lance Taylor]

Cary Coutant <ccoutant@google.com> writes:

> How about instead adding a "force_local" flag to these three
> functions, then using STB_GLOBAL with force_local == true for
> _GLOBAL_OFFSET_TABLE_ and similar symbols? I think it's misleading to
> use STB_LOCAL for a symbol that actually does have global scope
> (within the link).

Works for me.

Ian

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Ian Lance Taylor]

David Miller <davem@davemloft.net> writes:

> GOLD downgrades a protected symbol from STB_GLOBAL to STB_LOCAL in the
> final link, which I've not seen the BFD linker do.

It does?  That doesn't sound right.  It would make protected symbols
useless in shared libraries.  I don't see it happening in, e.g.,
protected_1.so in the testsuite.  Where do you see that?

Symbols with hidden and internal visibility are forced to STB_LOCAL in a
final link, but not protected symbols.  Gold does that because that is
what the ELF ABI specifies should happen.

Ian

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[David Miller]

From: Ian Lance Taylor <iant@google.com>
Date: Wed, 25 Apr 2012 12:07:49 -0700

> Symbols with hidden and internal visibility are forced to STB_LOCAL in a
> final link, but not protected symbols.  Gold does that because that is
> what the ELF ABI specifies should happen.

Sorry, this is indeed the case I was talking about.

While working on IFUNC I compared various test cases side by side
and the BFD linker didn't make this transformation.

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Ian Lance Taylor]

David Miller <davem@davemloft.net> writes:

> From: Ian Lance Taylor <iant@google.com>
> Date: Wed, 25 Apr 2012 12:07:49 -0700
>
>> Symbols with hidden and internal visibility are forced to STB_LOCAL in a
>> final link, but not protected symbols.  Gold does that because that is
>> what the ELF ABI specifies should happen.
>
> Sorry, this is indeed the case I was talking about.
>
> While working on IFUNC I compared various test cases side by side
> and the BFD linker didn't make this transformation.

If you go to http://sco.com/developers/gabi/latest/ch4.symtab.html and
search for STV_HIDDEN you will find this sentence:

    A hidden symbol contained in a relocatable object must be either
    removed or converted to STB_LOCAL binding by the link-editor when
    the relocatable object is included in an executable file or shared
    object.

There is a similar sentence for STV_INTERNAL.

This seems to make sense in general, in that at runtime one shared
library can not refer to a hidden or internal symbol defined in a
different shared library.  I can see how it matters for SPARC, but it
still seems more or less like the right thing to do.

Ian

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[David Miller]

From: Ian Lance Taylor <iant@google.com>
Date: Wed, 25 Apr 2012 12:44:12 -0700

> This seems to make sense in general, in that at runtime one shared
> library can not refer to a hidden or internal symbol defined in a
> different shared library.  I can see how it matters for SPARC, but it
> still seems more or less like the right thing to do.

Ok, thanks for all the info.

I will say in passing that this made things confusing for me from a
gold target's perspective.  It seemd to me that the whole point of
having seperate global and local Scan methods is that relocations are
processed differently for STB_LOCAL vs. STB_GLOBAL symbols.

So, from this perspective, this forced local case adds at least a
small wrinkle.  Imagine my surprise while debugging that a gsym seen
in global relocation scanning ended up STB_LOCAL in the final symbol
table :-)

Anyways, thanks again Ian.

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Cary Coutant]

> I will say in passing that this made things confusing for me from a
> gold target's perspective.  It seemd to me that the whole point of
> having seperate global and local Scan methods is that relocations are
> processed differently for STB_LOCAL vs. STB_GLOBAL symbols.
>
> So, from this perspective, this forced local case adds at least a
> small wrinkle.  Imagine my surprise while debugging that a gsym seen
> in global relocation scanning ended up STB_LOCAL in the final symbol
> table :-)

I think you're conflating local to a relocatable object with local to
a load module. The downgrading of internal and hidden STB_GLOBAL
symbols to STB_LOCAL happens at the very end, so the symbols are
treated as local only by the dynamic loader. In every other respect,
the linker treats such symbols as global.

A forced-local symbol is handled by the global Scan method, as it's
still a global symbol across the set of relocatable objects being
linked.

-cary

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[David Miller]

From: Cary Coutant <ccoutant@google.com>
Date: Wed, 25 Apr 2012 13:50:37 -0700

> I think you're conflating local to a relocatable object with local to
> a load module. The downgrading of internal and hidden STB_GLOBAL
> symbols to STB_LOCAL happens at the very end, so the symbols are
> treated as local only by the dynamic loader. In every other respect,
> the linker treats such symbols as global.

In every other respect, except the very one that matters for
relocation handling :-)

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Cary Coutant]

>> I think you're conflating local to a relocatable object with local to
>> a load module. The downgrading of internal and hidden STB_GLOBAL
>> symbols to STB_LOCAL happens at the very end, so the symbols are
>> treated as local only by the dynamic loader. In every other respect,
>> the linker treats such symbols as global.
>
> In every other respect, except the very one that matters for
> relocation handling :-)

Sorry, I don't really understand the issue. You've explained that
dynamic relocations for STB_LOCAL symbols need to have st_value in
r_addend, and that you have a test for forced_local() in the
Scan::global() code. But that test is an assertion, which suggests
that you don't expect that to happen. If it does happen, why can't you
just arrange for the linker to write the relocation with the proper
value of r_addend? That may require some changes outside the sparc
target code, but it shouldn't be too difficult. Could it be as simple
as setting the is_symbolless attribute on the dynamic relocation
(which tells Output_reloc::write to use the symbol value for the
addend)?

It doesn't seem to me that this is really a problem with downgrading
STB_GLOBAL symbols to STB_LOCAL, but an issue with writing the
relocations.

-cary

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[David Miller]

From: Cary Coutant <ccoutant@google.com>
Date: Wed, 25 Apr 2012 14:59:00 -0700

> Sorry, I don't really understand the issue. You've explained that
> dynamic relocations for STB_LOCAL symbols need to have st_value in
> r_addend, and that you have a test for forced_local() in the
> Scan::global() code. But that test is an assertion,

Look 3 lines above the assertion, I test for forced_local in the
actual if() statement as well.  Which in effect does already what
you're suggesting, which is use a relative relocation in these
situations and thus end up triggering all of the symbolless logic.

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Cary Coutant]

>> We usually break long lines like this after the '=', and indent the
>> next line 4 spaces:
>
> That's a marked departure from the usual GNU indentation style, which would
> be either what I wrote or breaking the line before the '=' and indenting
> the new line two spaces.  But OK.

Yeah, I'm not sure exactly what the rule is for this particular case
-- there's a mixture of styles in gold, but I think what I described
predominates. I'll defer to Ian to rule on the preferred style.

>> For completeness, we should apply the same treatment to
>> do_define_in_output_segment and do_define_as_constant. It'd be OK with
>> me to defer that to a follow-up patch.
>
> Ok, I'll leave it to you then.  IMHO it would be worthwhile to break out
> the boilerplate of calling the appropriate template flavor of
> define_special_symbol (and now sometimes not) into a common subroutine.

Agreed.

-cary

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Roland McGrath]

On Wed, Apr 18, 2012 at 3:22 PM, Cary Coutant <ccoutant@google.com> wrote:
> We usually break long lines like this after the '=', and indent the
> next line 4 spaces:

That's a marked departure from the usual GNU indentation style, which would
be either what I wrote or breaking the line before the '=' and indenting
the new line two spaces.  But OK.

> Given the change above this one, we will not have added any STB_LOCAL
> symbols to the symbol table, so the force_local call is unnecessary in
> that case. If the symbol is not STB_LOCAL, but
> this->version_script_.symbol_is_local(name) is true, then we do still
> need to call force_local. So perhaps this would work:

That makes sense.

> For completeness, we should apply the same treatment to
> do_define_in_output_segment and do_define_as_constant. It'd be OK with
> me to defer that to a follow-up patch.

Ok, I'll leave it to you then.  IMHO it would be worthwhile to break out
the boilerplate of calling the appropriate template flavor of
define_special_symbol (and now sometimes not) into a common subroutine.

Hmm.  I notice that my addition doesn't #ifdef the use, so it's calling
both endian flavors even if a configuration doesn't have both.  I'll fix
that.

> It looks good to me with those changes, but you still need Ian's
> approval to commit.

I've made the two changes above in my local copy, and won't bother
reposting since you know exactly what it looks like.


Thanks,
Roland

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Cary Coutant]

> Target_arm<big_endian>::make_plt_entry(Symbol_table* symtab, Layout*
> layout,
>       this->got_section(symtab, layout);
>
>       this->plt_ = new Output_data_plt_arm<big_endian>(layout, this->got_plt_);
> -      layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
> -                                     (elfcpp::SHF_ALLOC
> -                                      | elfcpp::SHF_EXECINSTR),
> +
> +      Output_section* os = layout->add_output_section_data
> +       (".plt", elfcpp::SHT_PROGBITS,
> +        (elfcpp::SHF_ALLOC | elfcpp::SHF_EXECINSTR),
>                                      this->plt_, ORDER_PLT, false);

We usually break long lines like this after the '=', and indent the
next line 4 spaces:

      Output_section* os =
          layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
                                          (elfcpp::SHF_ALLOC
                                           | elfcpp::SHF_EXECINSTR),
                                          this->plt_, ORDER_PLT, false);

> @@ -1919,6 +1945,8 @@ Symbol_table::do_define_in_output_data(
>        this->force_local(sym);
>       else if (version != NULL)
>        sym->set_is_default();
> +      if (binding == elfcpp::STB_LOCAL)
> +       this->generated_locals_.push_back(sym);
>       return sym;
>     }

Given the change above this one, we will not have added any STB_LOCAL
symbols to the symbol table, so the force_local call is unnecessary in
that case. If the symbol is not STB_LOCAL, but
this->version_script_.symbol_is_local(name) is true, then we do still
need to call force_local. So perhaps this would work:

      if (binding == elfcpp::STB_LOCAL)
        this->generated_locals_.push_back(sym);
      else if (this->version_script_.symbol_is_local(name))
        this->force_local(sym);
      else if (version != NULL)
        sym->set_is_default();
      return sym;

For completeness, we should apply the same treatment to
do_define_in_output_segment and do_define_as_constant. It'd be OK with
me to defer that to a follow-up patch.

It looks good to me with those changes, but you still need Ian's
approval to commit.

-cary

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Roland McGrath]

On Wed, Apr 18, 2012 at 2:18 PM, Cary Coutant <ccoutant@google.com> wrote:
> This is purely pedantic/nitpicky, but I think it would be better to
> place the generated locals before the forced locals, so that the
> output symbol table is: (a) locals from relocatables, (b) locals
> generated by the linker, (c) static globals turned into dynamic
> locals, and (d) globals. It just makes a nicer transition.

Ok.

> Again, nitpicky, but maybe we want to change the name of the typedef
> Forced_locals?

Sure.

Here's a new patch (same effect, same testing) incorporating these
and your earlier suggestion.

Ok for trunk?


Thanks,
Roland


gold/
2012-04-18  Roland McGrath  <mcgrathr@google.com>

	* arm.cc (Output_data_plt_arm::add_marker_symbols): New method.
	(Output_data_plt_arm::add_marker_symbol): New method.
	(Target_arm::make_plt_entry): Call it.

	* symtab.h (Symbol_table::Forced_locals): Rename to ...
	(Symbol_table::Symbol_vector): ... this.
	(Symbol_table::forced_locals_): Update type.
	(Symbol_table::generated_locals_): New member.
	* symtab.cc (Symbol_table::Symbol_table): Initialize it.
	(Symbol_table::do_define_in_output_data): Don't use
	define_special_symbol for STB_LOCAL.  Instead, just create a symbol
	and add it to generated_locals_.
	(Symbol_table::sized_finalize): Update use of type name.
	Add generated_locals_ to symtab similar to forced_locals_.
	(Symbol_table::sized_write_globals): Write generated_locals_ out.

diff --git a/gold/arm.cc b/gold/arm.cc
index dc6e64a..0bed822 100644
--- a/gold/arm.cc
+++ b/gold/arm.cc
@@ -7223,8 +7223,31 @@ class Output_data_plt_arm : public Output_section_data
   get_plt_entry_size()
   { return sizeof(plt_entry); }

+  // Define marker symbols at the start of the PLT so that
+  // objdump will know to disassemble it as ARM instructions.
+  void
+  add_marker_symbols(Output_section* os, Symbol_table* symtab)
+  {
+    // The last word of first_plt_entry is data.
+    // The leading words, and all of subsequent entries, are ARM instructions.
+    this->add_marker_symbol(os, symtab, 0, false);
+    this->add_marker_symbol(os, symtab, 16, true);
+    this->add_marker_symbol(os, symtab, 20, false);
+  }
+
  protected:
   void
+  add_marker_symbol(Output_section* os, Symbol_table* symtab,
+		    Arm_address offset, bool is_data)
+  {
+    symtab->define_in_output_data(is_data ? "$d" : "$a", NULL,
+				  Symbol_table::PREDEFINED, os,
+				  offset, 0, elfcpp::STT_NOTYPE,
+				  elfcpp::STB_LOCAL, elfcpp::STV_DEFAULT, 0,
+				  false, false);
+  }
+
+  void
   do_adjust_output_section(Output_section* os);

   // Write to a map file.
@@ -7430,10 +7453,13 @@
Target_arm<big_endian>::make_plt_entry(Symbol_table* symtab, Layout*
layout,
       this->got_section(symtab, layout);

       this->plt_ = new Output_data_plt_arm<big_endian>(layout, this->got_plt_);
-      layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
-				      (elfcpp::SHF_ALLOC
-				       | elfcpp::SHF_EXECINSTR),
+
+      Output_section* os = layout->add_output_section_data
+	(".plt", elfcpp::SHT_PROGBITS,
+	 (elfcpp::SHF_ALLOC | elfcpp::SHF_EXECINSTR),
 				      this->plt_, ORDER_PLT, false);
+
+      this->plt_->add_marker_symbols(os, symtab);
     }
   this->plt_->add_entry(gsym);
 }
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 1edb88d..5e2d6fc 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -1,6 +1,6 @@
 // symtab.cc -- the gold symbol table

-// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
 // Written by Ian Lance Taylor <iant@google.com>.

 // This file is part of gold.
@@ -532,7 +532,7 @@ Symbol_table::Symbol_table(unsigned int count,
                            const Version_script_info& version_script)
   : saw_undefined_(0), offset_(0), table_(count), namepool_(),
     forwarders_(), commons_(), tls_commons_(), small_commons_(),
-    large_commons_(), forced_locals_(), warnings_(),
+    large_commons_(), forced_locals_(), generated_locals_(), warnings_(),
     version_script_(version_script), gc_(NULL), icf_(NULL)
 {
   namepool_.reserve(count);
@@ -1884,7 +1884,33 @@ Symbol_table::do_define_in_output_data(
   Sized_symbol<size>* oldsym;
   bool resolve_oldsym;

-  if (parameters->target().is_big_endian())
+  if (binding == elfcpp::STB_LOCAL)
+    {
+      // This is a special local symbol, so it doesn't need to be
+      // in the symbol table by name.  Just make a naked symbol.
+      // We'll add it to the generated_locals_ list below.
+      const Target& target = parameters->target();
+      if (!target.has_make_symbol())
+	sym = new Sized_symbol<size>();
+      else
+	{
+	  if (parameters->target().is_big_endian())
+	    {
+	      Sized_target<size, true>* sized_target =
+		parameters->sized_target<size, true>();
+	      sym = sized_target->make_symbol();
+	    }
+	  else
+	    {
+	      Sized_target<size, false>* sized_target =
+		parameters->sized_target<size, false>();
+	      sym = sized_target->make_symbol();
+	    }
+	}
+
+      oldsym = NULL;
+    }
+  else if (parameters->target().is_big_endian())
     {
 #if defined(HAVE_TARGET_32_BIG) || defined(HAVE_TARGET_64_BIG)
       sym = this->define_special_symbol<size, true>(&name, &version,
@@ -1919,6 +1945,8 @@ Symbol_table::do_define_in_output_data(
 	this->force_local(sym);
       else if (version != NULL)
 	sym->set_is_default();
+      if (binding == elfcpp::STB_LOCAL)
+	this->generated_locals_.push_back(sym);
       return sym;
     }

@@ -2500,9 +2528,24 @@ Symbol_table::sized_finalize(off_t off, Stringpool* pool,
   unsigned int index = *plocal_symcount;
   const unsigned int orig_index = index;

-  // First do all the symbols which have been forced to be local, as
+  // First do all the symbols which have been generated as local, as
   // they must appear before all global symbols.
-  for (Forced_locals::iterator p = this->forced_locals_.begin();
+  for (Symbol_vector::iterator p = this->generated_locals_.begin();
+       p != this->generated_locals_.end();
+       ++p)
+    {
+      Symbol* sym = *p;
+      gold_assert(sym->is_forced_local());
+      if (this->sized_finalize_symbol<size>(sym))
+	{
+	  this->add_to_final_symtab<size>(sym, pool, &index, &off);
+	  ++*plocal_symcount;
+	}
+    }
+
+  // Next do all the symbols which have been forced to be local.
+  // They too must appear before all global symbols.
+  for (Symbol_vector::iterator p = this->forced_locals_.begin();
        p != this->forced_locals_.end();
        ++p)
     {
@@ -2976,6 +3019,36 @@ Symbol_table::sized_write_globals(const
Stringpool* sympool,
 	}
     }

+  // Next do all the symbols which have been generated as local.
+  for (Symbol_vector::const_iterator p = this->generated_locals_.begin();
+       p != this->generated_locals_.end();
+       ++p)
+    {
+      Sized_symbol<size>* sym = static_cast<Sized_symbol<size>*>(*p);
+      unsigned int sym_index = sym->symtab_index();
+      unsigned int shndx = sym->output_data()->out_shndx();
+      typename elfcpp::Elf_types<size>::Elf_Addr sym_value = sym->value();
+
+      gold_assert(sym->binding() == elfcpp::STB_LOCAL);
+
+      if (shndx >= elfcpp::SHN_LORESERVE)
+	{
+	  if (sym_index != -1U)
+	    symtab_xindex->add(sym_index, shndx);
+	  shndx = elfcpp::SHN_XINDEX;
+	}
+
+      if (sym_index != -1U)
+	{
+	  sym_index -= first_global_index;
+	  gold_assert(sym_index < output_count);
+	  unsigned char* ps = psyms + (sym_index * sym_size);
+	  this->sized_write_symbol<size, big_endian>(sym, sym_value, shndx,
+						     elfcpp::STB_LOCAL,
+						     sympool, ps);
+	}
+    }
+
   of->write_output_view(this->offset_, oview_size, psyms);
   if (dynamic_view != NULL)
     of->write_output_view(this->dynamic_offset_, dynamic_size, dynamic_view);
diff --git a/gold/symtab.h b/gold/symtab.h
index feed245..94942b2 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -1,6 +1,6 @@
 // symtab.h -- the gold symbol table   -*- C++ -*-

-// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
 // Written by Ian Lance Taylor <iant@google.com>.

 // This file is part of gold.
@@ -1822,8 +1822,8 @@ class Symbol_table
   sized_write_section_symbol(const Output_section*, Output_symtab_xindex*,
 			     Output_file*, off_t) const;

-  // The type of the list of symbols which have been forced local.
-  typedef std::vector<Symbol*> Forced_locals;
+  // The type of a simple vector of symbols.
+  typedef std::vector<Symbol*> Symbol_vector;

   // A map from symbols with COPY relocs to the dynamic objects where
   // they are defined.
@@ -1871,7 +1871,9 @@ class Symbol_table
   // A list of symbols which have been forced to be local.  We don't
   // expect there to be very many of them, so we keep a list of them
   // rather than walking the whole table to find them.
-  Forced_locals forced_locals_;
+  Symbol_vector forced_locals_;
+  // A list of symbols that were generated as local.
+  Symbol_vector generated_locals_;
   // Manage symbol warnings.
   Warnings warnings_;
   // Manage potential One Definition Rule (ODR) violations.

Re: [PATCH] gold/arm: define $a/$d markers in .plt

[Cary Coutant]

> +  // Next do all the symbols which have been generated as local.
> +  // They too must appear before all global symbols.
> +  for (Forced_locals::iterator p = this->generated_locals_.begin();
> +       p != this->generated_locals_.end();
> +       ++p)
> +    {
> +      Symbol* sym = *p;
> +      gold_assert(sym->is_forced_local());
> +      if (this->sized_finalize_symbol<size>(sym))
> +       {
> +         this->add_to_final_symtab<size>(sym, pool, &index, &off);
> +         ++*plocal_symcount;
> +       }
> +    }

This is purely pedantic/nitpicky, but I think it would be better to
place the generated locals before the forced locals, so that the
output symbol table is: (a) locals from relocatables, (b) locals
generated by the linker, (c) static globals turned into dynamic
locals, and (d) globals. It just makes a nicer transition.

> @@ -1872,6 +1888,8 @@ class Symbol_table
>   // expect there to be very many of them, so we keep a list of them
>   // rather than walking the whole table to find them.
>   Forced_locals forced_locals_;
> +  // A list of symbols that were generated as local.
> +  Forced_locals generated_locals_;

Again, nitpicky, but maybe we want to change the name of the typedef
Forced_locals?

-cary

[PATCH] gold/arm: define $a/$d markers in .plt

[Roland McGrath]

When gold generates an ARM .plt section, it fails to define any magic
marker symbols (local symbols named $a or $d).  These are necessary to
make objdump -d disassemble the .plt contents rather than just showing
them as data words.

This patch makes it generate the necessary marker symbols at the start of
the .plt section.

The Symbol_table changes are all based on Cary's advice earlier in this
thread.  I really can't explain or vouch for this being the best way to
enable the missing functionality: defining local synthetic symbols without
regard to preexisting symbols of the same name.

No regressions in 'make check' (x86_64-linux-gnu host).  I verified the new
functionality with a trivial ARM link that generates some PLT entries.

Ok for trunk?


Thanks,
Roland


gold/
2012-04-18  Roland McGrath  <mcgrathr@google.com>

	* arm.cc (Output_data_plt_arm::add_marker_symbols): New method.
	(Output_data_plt_arm::add_marker_symbol): New method.
	(Target_arm::make_plt_entry): Call it.

	* symtab.h (Symbol_table::define_local_in_output_data): New method.
	(Symbol_table::do_define_local_in_output_data): New template method.
	(Symbol_table::generated_locals_): New member.
	* symtab.cc (Symbol_table::Symbol_table): Initialize it.
	(Symbol_table::define_local_in_output_data): New method.
	(Symbol_table::do_define_local_in_output_data): New method.
	(Symbol_table::sized_finalize): Add generated_locals_ to symtab
	similar to forced_locals_.
	(Symbol_table::sized_write_globals): Write generated_locals_ out.

diff --git a/gold/arm.cc b/gold/arm.cc
index dc6e64a..6232192 100644
--- a/gold/arm.cc
+++ b/gold/arm.cc
@@ -7223,8 +7223,29 @@ class Output_data_plt_arm : public Output_section_data
   get_plt_entry_size()
   { return sizeof(plt_entry); }

+  // Define marker symbols at the start of the PLT so that
+  // objdump will know to disassemble it as ARM instructions.
+  void
+  add_marker_symbols(Output_section* os, Symbol_table* symtab)
+  {
+    // The last word of first_plt_entry is data.
+    // The leading words, and all of subsequent entries, are ARM instructions.
+    this->add_marker_symbol(os, symtab, 0, false);
+    this->add_marker_symbol(os, symtab, 16, true);
+    this->add_marker_symbol(os, symtab, 20, false);
+  }
+
  protected:
   void
+  add_marker_symbol(Output_section* os, Symbol_table* symtab,
+		    Arm_address offset, bool is_data)
+  {
+    symtab->define_local_in_output_data(is_data ? "$d" : "$a",
+					Symbol_table::PREDEFINED, os,
+					offset, 0, elfcpp::STT_NOTYPE, false);
+  }
+
+  void
   do_adjust_output_section(Output_section* os);

   // Write to a map file.
@@ -7430,10 +7451,13 @@
Target_arm<big_endian>::make_plt_entry(Symbol_table* symtab, Layout*
layout,
       this->got_section(symtab, layout);

       this->plt_ = new Output_data_plt_arm<big_endian>(layout, this->got_plt_);
-      layout->add_output_section_data(".plt", elfcpp::SHT_PROGBITS,
-				      (elfcpp::SHF_ALLOC
-				       | elfcpp::SHF_EXECINSTR),
+
+      Output_section* os = layout->add_output_section_data
+	(".plt", elfcpp::SHT_PROGBITS,
+	 (elfcpp::SHF_ALLOC | elfcpp::SHF_EXECINSTR),
 				      this->plt_, ORDER_PLT, false);
+
+      this->plt_->add_marker_symbols(os, symtab);
     }
   this->plt_->add_entry(gsym);
 }
diff --git a/gold/symtab.cc b/gold/symtab.cc
index 1edb88d..cf09853 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -1,6 +1,6 @@
 // symtab.cc -- the gold symbol table

-// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
 // Written by Ian Lance Taylor <iant@google.com>.

 // This file is part of gold.
@@ -532,7 +532,7 @@ Symbol_table::Symbol_table(unsigned int count,
                            const Version_script_info& version_script)
   : saw_undefined_(0), offset_(0), table_(count), namepool_(),
     forwarders_(), commons_(), tls_commons_(), small_commons_(),
-    large_commons_(), forced_locals_(), warnings_(),
+    large_commons_(), forced_locals_(), generated_locals_(), warnings_(),
     version_script_(version_script), gc_(NULL), icf_(NULL)
 {
   namepool_.reserve(count);
@@ -1934,6 +1934,81 @@ Symbol_table::do_define_in_output_data(
     }
 }

+// Define a special local symbol based on an Output_data.
+
+Symbol*
+Symbol_table::define_local_in_output_data(const char* name,
+					  Defined defined, Output_data* od,
+					  uint64_t value, uint64_t symsize,
+					  elfcpp::STT type,
+					  bool offset_is_from_end)
+{
+  if (parameters->target().get_size() == 32)
+    {
+#if defined(HAVE_TARGET_32_LITTLE) || defined(HAVE_TARGET_32_BIG)
+      return this->do_define_local_in_output_data<32>(name, defined, od,
+						      value, symsize, type,
+						      offset_is_from_end);
+#else
+      gold_unreachable();
+#endif
+    }
+  else if (parameters->target().get_size() == 64)
+    {
+#if defined(HAVE_TARGET_64_LITTLE) || defined(HAVE_TARGET_64_BIG)
+      return this->do_define_local_in_output_data<64>(name, defined, od,
+						      value, symsize, type,
+						      offset_is_from_end);
+#else
+      gold_unreachable();
+#endif
+    }
+  else
+    gold_unreachable();
+}
+
+// Define a special local symbol based on an Output_data, sized version.
+
+template<int size>
+Sized_symbol<size>*
+Symbol_table::do_define_local_in_output_data(
+    const char* name, Defined defined, Output_data* od,
+    typename elfcpp::Elf_types<size>::Elf_Addr value,
+    typename elfcpp::Elf_types<size>::Elf_WXword symsize,
+    elfcpp::STT type, bool offset_is_from_end)
+{
+  Sized_symbol<size>* sym;
+
+  const Target& target = parameters->target();
+  if (!target.has_make_symbol())
+    sym = new Sized_symbol<size>();
+  else
+    {
+      if (parameters->target().is_big_endian())
+	{
+	  Sized_target<size, true>* sized_target =
+	    parameters->sized_target<size, true>();
+	  sym = sized_target->make_symbol();
+	}
+      else
+	{
+	  Sized_target<size, false>* sized_target =
+	    parameters->sized_target<size, false>();
+	  sym = sized_target->make_symbol();
+	}
+
+      if (sym == NULL)
+	return NULL;
+    }
+
+  sym->init_output_data(name, NULL, od, value, symsize, type,
+			elfcpp::STB_LOCAL, elfcpp::STV_DEFAULT, 0,
+			offset_is_from_end, defined == PREDEFINED);
+  sym->set_is_forced_local();
+  this->generated_locals_.push_back(sym);
+  return sym;
+}
+
 // Define a symbol based on an Output_segment.

 Symbol*
@@ -2515,6 +2590,21 @@ Symbol_table::sized_finalize(off_t off, Stringpool* pool,
 	}
     }

+  // Next do all the symbols which have been generated as local.
+  // They too must appear before all global symbols.
+  for (Forced_locals::iterator p = this->generated_locals_.begin();
+       p != this->generated_locals_.end();
+       ++p)
+    {
+      Symbol* sym = *p;
+      gold_assert(sym->is_forced_local());
+      if (this->sized_finalize_symbol<size>(sym))
+	{
+	  this->add_to_final_symtab<size>(sym, pool, &index, &off);
+	  ++*plocal_symcount;
+	}
+    }
+
   // Now do all the remaining symbols.
   for (Symbol_table_type::iterator p = this->table_.begin();
        p != this->table_.end();
@@ -2976,6 +3066,36 @@ Symbol_table::sized_write_globals(const
Stringpool* sympool,
 	}
     }

+  // Next do all the symbols which have been generated as local.
+  for (Forced_locals::const_iterator p = this->generated_locals_.begin();
+       p != this->generated_locals_.end();
+       ++p)
+    {
+      Sized_symbol<size>* sym = static_cast<Sized_symbol<size>*>(*p);
+      unsigned int sym_index = sym->symtab_index();
+      unsigned int shndx = sym->output_data()->out_shndx();
+      typename elfcpp::Elf_types<size>::Elf_Addr sym_value = sym->value();
+
+      gold_assert(sym->binding() == elfcpp::STB_LOCAL);
+
+      if (shndx >= elfcpp::SHN_LORESERVE)
+	{
+	  if (sym_index != -1U)
+	    symtab_xindex->add(sym_index, shndx);
+	  shndx = elfcpp::SHN_XINDEX;
+	}
+
+      if (sym_index != -1U)
+	{
+	  sym_index -= first_global_index;
+	  gold_assert(sym_index < output_count);
+	  unsigned char* ps = psyms + (sym_index * sym_size);
+	  this->sized_write_symbol<size, big_endian>(sym, sym_value, shndx,
+						     elfcpp::STB_LOCAL,
+						     sympool, ps);
+	}
+    }
+
   of->write_output_view(this->offset_, oview_size, psyms);
   if (dynamic_view != NULL)
     of->write_output_view(this->dynamic_offset_, dynamic_size, dynamic_view);
diff --git a/gold/symtab.h b/gold/symtab.h
index feed245..243de8b 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -1,6 +1,6 @@
 // symtab.h -- the gold symbol table   -*- C++ -*-

-// Copyright 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, Inc.
+// Copyright 2006,2007,2008,2009,2010,2011,2012 Free Software Foundation, Inc.
 // Written by Ian Lance Taylor <iant@google.com>.

 // This file is part of gold.
@@ -1393,6 +1393,13 @@ class Symbol_table
 		     unsigned char nonvis, bool only_if_ref,
                      bool force_override);

+  // Define a special local symbol based on an Output_data.
+  // This can define multiple symbols of the same name.
+  Symbol*
+  define_local_in_output_data(const char* name, Defined,
+			      Output_data*, uint64_t value, uint64_t symsize,
+			      elfcpp::STT type, bool offset_is_from_end);
+
   // Define a set of symbols in output sections.  If ONLY_IF_REF is
   // true, only define them if they are referenced.
   void
@@ -1716,6 +1723,15 @@ class Symbol_table
 			   elfcpp::STV visibility, unsigned char nonvis,
 			   bool offset_is_from_end, bool only_if_ref);

+  // Define a local symbol in an Output_data, sized version.
+  template<int size>
+  Sized_symbol<size>*
+  do_define_local_in_output_data(
+    const char* name, Defined, Output_data*,
+    typename elfcpp::Elf_types<size>::Elf_Addr value,
+    typename elfcpp::Elf_types<size>::Elf_WXword symsize,
+    elfcpp::STT type, bool offset_is_from_end);
+
   // Define a symbol in an Output_segment, sized version.
   template<int size>
   Sized_symbol<size>*
@@ -1872,6 +1888,8 @@ class Symbol_table
   // expect there to be very many of them, so we keep a list of them
   // rather than walking the whole table to find them.
   Forced_locals forced_locals_;
+  // A list of symbols that were generated as local.
+  Forced_locals generated_locals_;
   // Manage symbol warnings.
   Warnings warnings_;
   // Manage potential One Definition Rule (ODR) violations.