This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]