[RFC] [gold] Simplify relocation strategy logic

Cary Coutant ccoutant@google.com
Sat Mar 12 02:42:00 GMT 2011


Paul recently noticed that gold was generating PLT entries for lots of
symbols that didn't actually need PLT entries. I took a look at the
code and found an inconsistency in the logic for deciding when to
create a PLT entry (when scanning relocations) and the logic for
deciding whether or not to use the PLT entry as the target of the
relocation (when applying relocations). I suggested a small change
that would fix the problem, but Ian countered with a suggestion that
the logic should be cleaned up so that there's one new routine where
we used to use three -- needs_plt_entry(), needs_dynamic_reloc(), and
use_plt_offset().

Here's a start at doing that for x86_64 only, for your review and
comments. I've left in the old logic, so that it runs it side-by-side
with the new logic and prints any differences as warnings. (The
ChangeLog snippet below ignores those routines I've put in just for
the consistency checks.) Before this could be committed, I'll need to
update the other targets besides x86_64. I'll also need some help
testing the sparc, powerpc, and arm targets at that point.

The new routine, Symbol::relocation_strategy, returns two bit flags
that indicate whether the relocation should target a PLT entry (vs.
the symbol itself), and whether a dynamic relocation will be
necessary. I added two more Reference_flags to classify relocations as
PLT_REF (those that explicitly ask for a PLT offset) and GOT_REF
(those that explicitly target a GOT entry, which effectively makes it
a local reference). I also fixed a couple of mis-classified
relocations in the x86_64 version of Scan::get_reference_flags().

I get two sets of differences between the old logic and new logic
(using the gold testsuite). The first set are the cases I set out to
fix, where we used to generate unused PLT entries and now don't. These
all show up in the test log as messages of this form (always an
R_X86_64_64 relocation from the data segment to a function symbol):

gcctestdir/ld: warning: strategy conflict: needs_plt_entry() true,
strategy DYNAMIC (r_type 1, sym __gxx_personality_v0)
gcctestdir/ld: warning: strategy conflict: needs_plt_entry() true,
strategy DYNAMIC (r_type 1, sym f10())

The second set are from the TLS tests, and are all identical:

gcctestdir/ld: warning: strategy conflict: reloc implies PLT, strategy
NORMAL (r_type 4, sym __tls_get_addr)

These result from an R_X86_64_PLT32 referencing the undefined symbol
__tls_get_addr, which the new logic claims does not need either a PLT
entry or a dynamic relocation. In truth, it doesn't, because we always
optimize away the calls to __tls_get_addr when building an executable.
When building a shared library, the new logic does build the PLT
entry. I could check for gsym->is_defined_by_abi(), or I could fix it
so that the PLT32 relocation (now flagged as PLT_REF) always creates
the PLT entry, but it's not necessary in this case, and I can't think
of any other cases where it would be necessary.

-cary


        * symtab.cc (Symbol::relocation_strategy): New function.
        * symtab.h (Symbol::Reference_flags): Add PLT_REF, GOT_REF.
        (Symbol::Relocation_strategy): New enum type.
        * x86_64.cc (Scan::get_reference_flags): Return extra flags
        for certain relocations.
        (Scan::global): Use Symbol::relocation_strategy.
        (Relocate::relocate): Likewise.

Index: symtab.cc
===================================================================
RCS file: /cvs/src/src/gold/symtab.cc,v
retrieving revision 1.149
diff -u -p -r1.149 symtab.cc
--- symtab.cc	10 Mar 2011 01:31:32 -0000	1.149
+++ symtab.cc	12 Mar 2011 01:44:08 -0000
@@ -200,6 +200,88 @@ Symbol::allocate_base_common(Output_data
   this->u_.in_output_data.offset_is_from_end = false;
 }

+// Determine how a relocation should be applied to this symbol.
+// The bits in FLAGS indicates the type of reference implied by
+// the relocation (see Symbol::Reference_flags).  Returns flags
+// RELOCATE_PLT for a relocation that should relocate to the
+// address of a PLT entry, and RELOCATE_DYNAMIC if a dynamic
+// relocation is required.  Returns 0 if the relocation can be
+// applied normally.
+
+int
+Symbol::relocation_strategy(int flags) const
+{
+  int needs_dyn = 0;
+  int needs_plt = 0;
+
+  if ((flags & FUNCTION_CALL)
+      && this->is_weak_undefined()
+      && !parameters->doing_static_link())
+    {
+      // If this is a call to a weak undefined symbol, we need to use
+      // a PLT entry, because the symbol may be defined by a library
+      // loaded at runtime.
+      needs_plt = RELOCATE_PLT;
+    }
+  else if (this->is_undefined() && !parameters->options().shared())
+    {
+      // A reference to an undefined symbol from an executable (when not
+      // generating an error) will resolve to 0.
+      return 0;
+    }
+  else if (this->is_absolute())
+    {
+      // A reference to an absolute symbol can be relocated statically.
+      return 0;
+    }
+
+  if (this->type() == elfcpp::STT_GNU_IFUNC)
+    {
+      // An STT_GNU_IFUNC symbol always needs a PLT entry, even when
+      // doing a static link.
+      needs_plt = RELOCATE_PLT;
+    }
+
+  // A symbol may need a PLT entry or a dynamic relocation if it is
+  // defined in a dynamic object, undefined (when not building an
+  // executable -- references to undefs from an executable are handled
+  // above), or subject to pre-emption.
+  bool is_dynamic = (this->is_from_dynobj()
+		     || this->is_undefined()
+		     || this->is_preemptible());
+
+  if ((flags & ABSOLUTE_REF)
+      && parameters->options().output_is_position_independent())
+    {
+      // An absolute reference within a position-independent output file
+      // will need a dynamic relocation.
+      needs_dyn = RELOCATE_DYNAMIC;
+    }
+
+  if (is_dynamic
+      && (this->is_func() || (flags & PLT_REF))
+      && !(flags & GOT_REF)
+      && !parameters->doing_static_link())
+    {
+      // A function call to a dynamic symbol will use a PLT entry.
+      // Exception:  If we're already planning to create a dynamic
+      // relocation and the relocation doesn't specifically ask for
+      // the PLT offset, we can ignore the PLT entry.
+      if (!needs_dyn || (flags & PLT_REF))
+        needs_plt = RELOCATE_PLT;
+    }
+  else if (is_dynamic
+	   && !(flags & GOT_REF)
+	   && !parameters->doing_static_link())
+    {
+      // Any other reference to a dynamic, undefined, or pre-emptable
+      // symbol will need a dynamic relocation.
+      needs_dyn = RELOCATE_DYNAMIC;
+    }
+
+  return needs_plt | needs_dyn;
+}
+
 // Initialize the fields in Sized_symbol for SYM in OBJECT.

 template<int size>
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gold/symtab.h,v
retrieving revision 1.118
diff -u -p -r1.118 symtab.h
--- symtab.h	10 Mar 2011 01:31:32 -0000	1.118
+++ symtab.h	12 Mar 2011 01:44:08 -0000
@@ -627,9 +627,31 @@ class Symbol
     // A TLS-related reference.
     TLS_REF = 4,
     // A reference that can always be treated as a function call.
-    FUNCTION_CALL = 8
+    FUNCTION_CALL = 8,
+    // A specific reference to a PLT entry.
+    PLT_REF = 0x10,
+    // A reference to a GOT entry.
+    GOT_REF = 0x20
   };

+  // When scanning and applying relocations, we need to know whether
+  // the relocation should be applied normally, as a reference to a PLT
+  // entry, or by creating a dynamic relocation.  These bits may be
+  // or'ed together.  0 means the relocation should be applied normally.
+  enum Relocation_strategy
+  {
+    // Relocate to the address of a PLT entry.
+    RELOCATE_PLT = 1,
+    // Create a dynamic relocation.
+    RELOCATE_DYNAMIC = 2
+  };
+
+  // Determine how a relocation should be applied to this symbol.
+  // The bits in FLAGS indicates the type of reference implied by
+  // the relocation:  absolute, relative, TLS, function call.
+  int
+  relocation_strategy(int flags) const;
+
   // Given a direct absolute or pc-relative static relocation against
   // the global symbol, this function returns whether a dynamic relocation
   // is needed.
Index: x86_64.cc
===================================================================
RCS file: /cvs/src/src/gold/x86_64.cc,v
retrieving revision 1.121
diff -u -p -r1.121 x86_64.cc
--- x86_64.cc	15 Dec 2010 15:35:27 -0000	1.121
+++ x86_64.cc	12 Mar 2011 01:44:08 -0000
@@ -1270,15 +1270,19 @@ Target_x86_64::Scan::get_reference_flags

     case elfcpp::R_X86_64_PLT32:
     case elfcpp::R_X86_64_PLTOFF64:
-      return Symbol::FUNCTION_CALL | Symbol::RELATIVE_REF;
+      return Symbol::FUNCTION_CALL | Symbol::RELATIVE_REF | Symbol::PLT_REF;

     case elfcpp::R_X86_64_GOT64:
     case elfcpp::R_X86_64_GOT32:
+      // Absolute in GOT.
+      return Symbol::ABSOLUTE_REF | Symbol::GOT_REF;
     case elfcpp::R_X86_64_GOTPCREL64:
     case elfcpp::R_X86_64_GOTPCREL:
+      // PC-Relative in GOT.
+      return Symbol::RELATIVE_REF | Symbol::GOT_REF;
+
     case elfcpp::R_X86_64_GOTPLT64:
-      // Absolute in GOT.
-      return Symbol::ABSOLUTE_REF;
+      return Symbol::ABSOLUTE_REF | Symbol::GOT_REF;

     case elfcpp::R_X86_64_TLSGD:            // Global-dynamic
     case elfcpp::R_X86_64_GOTPC32_TLSDESC:  // Global-dynamic (from ~oliva url)
@@ -1764,6 +1768,85 @@ Target_x86_64::Scan::global_reloc_may_be
           || possible_function_pointer_reloc(r_type));
 }

+// TEMPORARY: Sanity check for relocation_strategy().
+// Compare the strategy with the older logic.
+
+const char*
+strategy_name(int strategy)
+{
+  switch (strategy)
+    {
+    case 0:
+      return "NORMAL";
+    case Symbol::RELOCATE_PLT:
+      return "PLT";
+    case Symbol::RELOCATE_DYNAMIC:
+      return "DYNAMIC";
+    case Symbol::RELOCATE_PLT | Symbol::RELOCATE_DYNAMIC:
+      return "PLT+DYN";
+    default:
+      gold_unreachable();
+    }
+}
+
+void
+check_plt_strategy(const Symbol* gsym, int r_type, int strategy)
+{
+  bool needs_plt = gsym->needs_plt_entry();
+  bool strat_plt = (strategy & Symbol::RELOCATE_PLT) != 0;
+  if (needs_plt != strat_plt)
+    gold_warning("strategy conflict: needs_plt_entry() %s, "
+		 "strategy %s "
+		 "(r_type %d, sym %s)",
+		 needs_plt ? "true" : "false",
+		 strategy_name(strategy),
+          	 r_type, gsym->name());
+}
+
+void
+check_forced_plt_strategy(const Symbol* gsym, int r_type, int strategy)
+{
+  bool needs_plt = true;
+  bool strat_plt = (strategy & Symbol::RELOCATE_PLT) != 0;
+  if (needs_plt != strat_plt)
+    gold_warning("strategy conflict: reloc implies PLT, "
+		 "strategy %s "
+		 "(r_type %d, sym %s)",
+		 strategy_name(strategy),
+          	 r_type, gsym->name());
+}
+
+void
+check_dynamic_strategy(const Symbol* gsym, int r_type, int flags, int strategy)
+{
+  bool needs_dyn_reloc = gsym->needs_dynamic_reloc(flags);
+  bool strat_dyn = (strategy & Symbol::RELOCATE_DYNAMIC) != 0;
+  if (needs_dyn_reloc != strat_dyn)
+    gold_warning("strategy conflict: needs_dynamic_reloc() %s, "
+		 "strategy %s "
+		 "(r_type %d, sym %s)",
+		 needs_dyn_reloc ? "true" : "false",
+		 strategy_name(strategy),
+          	 r_type, gsym->name());
+}
+
+void
+check_use_plt_strategy(const Symbol* gsym, int r_type, int flags, int strategy)
+{
+  bool use_plt = false;
+  if (gsym != NULL)
+    use_plt = gsym->use_plt_offset(flags);
+  bool strat_plt = (strategy & Symbol::RELOCATE_PLT) != 0;
+  if (use_plt != strat_plt)
+    gold_warning("strategy conflict: use_plt_offset() %s, "
+		 "strategy %s "
+		 "(r_type %d, sym %s)",
+		 use_plt ? "true" : "false",
+		 strategy_name(strategy),
+          	 r_type,
+          	 gsym == NULL ? "(local)" : gsym->name());
+}
+
 // Scan a relocation for a global symbol.

 inline void
@@ -1777,9 +1860,10 @@ Target_x86_64::Scan::global(Symbol_table
                             unsigned int r_type,
                             Symbol* gsym)
 {
-  // A STT_GNU_IFUNC symbol may require a PLT entry.
-  if (gsym->type() == elfcpp::STT_GNU_IFUNC
-      && this->reloc_needs_plt_for_ifunc(object, r_type))
+  int strategy = gsym->relocation_strategy(Scan::get_reference_flags(r_type));
+
+  // Make a PLT entry if necessary.
+  if (strategy & Symbol::RELOCATE_PLT)
     target->make_plt_entry(symtab, layout, gsym);

   switch (r_type)
@@ -1795,19 +1879,21 @@ Target_x86_64::Scan::global(Symbol_table
     case elfcpp::R_X86_64_16:
     case elfcpp::R_X86_64_8:
       {
-        // Make a PLT entry if necessary.
-        if (gsym->needs_plt_entry())
+        check_plt_strategy(gsym, r_type, strategy);
+        if (strategy & Symbol::RELOCATE_PLT
+            && gsym->is_from_dynobj()
+            && !parameters->options().shared())
           {
-            target->make_plt_entry(symtab, layout, gsym);
             // Since this is not a PC-relative relocation, we may be
             // taking the address of a function. In that case we need to
             // set the entry in the dynamic symbol table to the address of
             // the PLT entry.
-            if (gsym->is_from_dynobj() && !parameters->options().shared())
-              gsym->set_needs_dynsym_value();
+            gsym->set_needs_dynsym_value();
           }
         // Make a dynamic relocation if necessary.
-        if (gsym->needs_dynamic_reloc(Scan::get_reference_flags(r_type)))
+        check_dynamic_strategy(gsym, r_type, Scan::get_reference_flags(r_type),
+        		       strategy);
+        if (strategy & Symbol::RELOCATE_DYNAMIC)
           {
             if (gsym->may_need_copy_reloc())
               {
@@ -1860,11 +1946,11 @@ Target_x86_64::Scan::global(Symbol_table
     case elfcpp::R_X86_64_PC16:
     case elfcpp::R_X86_64_PC8:
       {
-        // Make a PLT entry if necessary.
-        if (gsym->needs_plt_entry())
-          target->make_plt_entry(symtab, layout, gsym);
+        check_plt_strategy(gsym, r_type, strategy);
+        check_dynamic_strategy(gsym, r_type, Scan::get_reference_flags(r_type),
+        		       strategy);
         // Make a dynamic relocation if necessary.
-        if (gsym->needs_dynamic_reloc(Scan::get_reference_flags(r_type)))
+        if (strategy & Symbol::RELOCATE_DYNAMIC)
           {
             if (gsym->may_need_copy_reloc())
               {
@@ -1948,16 +2034,14 @@ Target_x86_64::Scan::global(Symbol_table
     case elfcpp::R_X86_64_PLT32:
       // If the symbol is fully resolved, this is just a PC32 reloc.
       // Otherwise we need a PLT entry.
-      if (gsym->final_value_is_known())
-	break;
       // If building a shared library, we can also skip the PLT entry
       // if the symbol is defined in the output file and is protected
       // or hidden.
-      if (gsym->is_defined()
-          && !gsym->is_from_dynobj()
-          && !gsym->is_preemptible())
-	break;
-      target->make_plt_entry(symtab, layout, gsym);
+      if (!gsym->final_value_is_known()
+	   && !(gsym->is_defined()
+		&& !gsym->is_from_dynobj()
+		&& !gsym->is_preemptible()))
+	check_forced_plt_strategy(gsym, r_type, strategy);
       break;

     case elfcpp::R_X86_64_GOTPC32:
@@ -2264,10 +2348,13 @@ Target_x86_64::Relocate::relocate(const

   const Sized_relobj<64, false>* object = relinfo->object;

+  int strategy = 0;
+  if (gsym != NULL)
+    strategy = gsym->relocation_strategy(Scan::get_reference_flags(r_type));
+
   // Pick the value to use for symbols defined in the PLT.
   Symbol_value<64> symval;
-  if (gsym != NULL
-      && gsym->use_plt_offset(Scan::get_reference_flags(r_type)))
+  if (strategy & Symbol::RELOCATE_PLT)
     {
       symval.set_output_value(target->plt_section()->address()
 			      + gsym->plt_offset());
@@ -2315,6 +2402,8 @@ Target_x86_64::Relocate::relocate(const
       break;

     default:
+      check_use_plt_strategy(gsym, r_type, Scan::get_reference_flags(r_type),
+			     strategy);
       break;
     }



More information about the Binutils mailing list