Fix symbol values and relocation addends for relocatable links.

The fix for PR 19291 broke some other cases where -r is used with scripts,
as reported in PR 22266. The original fix for PR 22266 ended up breaking
many cases for REL targets, where the addends are stored in the section data,
and are not being adjusted properly.

The problem was basically that in a relocatable output file (ET_REL),
symbol values are supposed to be relative to the start address of their
section. Usually in a relocatable file, all sections start at 0, so the
failure to get this right is often irrelevant, but with a linker script,
we occasionally see an output section whose starting address is not 0,
and gold would occasionally write a symbol with its relocated value instead
of its section-relative value.

This patch reverts the recent fix for PR 22266 as well as my original fix
for PR 19291. The original fix moved the symbol value adjustment to
write_local_symbols, but neglected to undo a few places where the adjustment
was also being applied, resulting in an occasional double adjustment. The
more recent fix removed those other adjustments, but then failed to
re-account for the adjustment when rewriting the relocations on REL targets.

With the old attempts reverted, we now apply the symbol value adjustment to
the one case that had been missed (non-section symbols in merge sections).
But now we also need to account for the adjustment when rewriting the addends
for RELA relocations.

gold/
	PR gold/19291
	PR gold/22266
	* object.cc (Sized_relobj_file::compute_final_local_value_internal):
	Revert changes from 2017-11-08 patch.  Adjust symbol value in
	relocatable links for non-section symbols.
	(Sized_relobj_file::compute_final_local_value): Revert changes from
	2017-11-08 patch.
	(Sized_relobj_file::do_finalize_local_symbols): Likewise.
	(Sized_relobj_file::write_local_symbols): Revert changes from
	2015-11-25 patch.
	* object.h (Sized_relobj_file::compute_final_local_value_internal):
	Revert changes from 2017-11-08 patch.
	* powerpc.cc (Target_powerpc::relocate_relocs): Adjust addend for
	relocatable links.
	* target-reloc.h (relocate_relocs): Adjust addend for relocatable links.
	* testsuite/pr22266_a.c (hello): New function.
	* testsuite/pr22266_main.c (main): Add test for merge sections.
	* testsuite/pr22266_script.t: Add rule for .rodata.

diff --git a/gold/ChangeLog b/gold/ChangeLog
index 4c631ca78a2c71f6cc03ebc9a1a90d5376a425b1..c3c7481ed787762644b9cb032229c3777113e8bb 100644 (file)
--- a/gold/ChangeLog
+++ b/gold/ChangeLog
@@ -1,3 +1,24 @@
+2017-11-27  Cary Coutant  <ccoutant@gmail.com>
+
+       PR gold/19291
+       PR gold/22266
+       * object.cc (Sized_relobj_file::compute_final_local_value_internal):
+       Revert changes from 2017-11-08 patch.  Adjust symbol value in
+       relocatable links for non-section symbols.
+       (Sized_relobj_file::compute_final_local_value): Revert changes from
+       2017-11-08 patch.
+       (Sized_relobj_file::do_finalize_local_symbols): Likewise.
+       (Sized_relobj_file::write_local_symbols): Revert changes from
+       2015-11-25 patch.
+       * object.h (Sized_relobj_file::compute_final_local_value_internal):
+       Revert changes from 2017-11-08 patch.
+       * powerpc.cc (Target_powerpc::relocate_relocs): Adjust addend for
+       relocatable links.
+       * target-reloc.h (relocate_relocs): Adjust addend for relocatable links.
+       * testsuite/pr22266_a.c (hello): New function.
+       * testsuite/pr22266_main.c (main): Add test for merge sections.
+       * testsuite/pr22266_script.t: Add rule for .rodata.
+
 2017-11-19  Ian Lance Taylor  <iant@google.com>
            Cary Coutant  <ccoutant@gmail.com>
 

diff --git a/gold/object.cc b/gold/object.cc
index 2e975bba52c4403c944e753c50f559fb4e9f587b..f7fe088d7ca83660c654f075dff3fe9a2f3c34e0 100644 (file)
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -2318,6 +2318,7 @@ Sized_relobj_file<size, big_endian>::compute_final_local_value_internal(
     unsigned int r_sym,
     const Symbol_value<size>* lv_in,
     Symbol_value<size>* lv_out,
+    bool relocatable,
     const Output_sections& out_sections,
     const std::vector<Address>& out_offsets,
     const Symbol_table* symtab)
@@ -2404,8 +2405,11 @@ Sized_relobj_file<size, big_endian>::compute_final_local_value_internal(
            {
              // This is not a section symbol.  We can determine
              // the final value now.
-             lv_out->set_output_value(
-                 os->output_address(this, shndx, lv_in->input_value()));
+             uint64_t value =
+               os->output_address(this, shndx, lv_in->input_value());
+             if (relocatable)
+               value -= os->address();
+             lv_out->set_output_value(value);
            }
          else if (!os->find_starting_output_address(this, shndx, &start))
            {
@@ -2419,7 +2423,10 @@ Sized_relobj_file<size, big_endian>::compute_final_local_value_internal(
                os->find_relaxed_input_section(this, shndx);
              if (posd != NULL)
                {
-                 lv_out->set_output_value(posd->address());
+                 uint64_t value = posd->address();
+                 if (relocatable)
+                   value -= os->address();
+                 lv_out->set_output_value(value);
                }
              else
                lv_out->set_output_value(os->address());
@@ -2428,10 +2435,14 @@ Sized_relobj_file<size, big_endian>::compute_final_local_value_internal(
            {
              // We have to consider the addend to determine the
              // value to use in a relocation.  START is the start
-             // of this input section.
+             // of this input section.  If we are doing a relocatable
+             // link, use offset from start output section instead of
+             // address.
+             Address adjusted_start =
+               relocatable ? start - os->address() : start;
              Merged_symbol_value<size>* msv =
                new Merged_symbol_value<size>(lv_in->input_value(),
-                                             start);
+                                             adjusted_start);
              lv_out->set_merged_symbol_value(msv);
            }
        }
@@ -2442,7 +2453,7 @@ Sized_relobj_file<size, big_endian>::compute_final_local_value_internal(
                                 + secoffset
                                 + lv_in->input_value());
       else
-       lv_out->set_output_value(os->address()
+       lv_out->set_output_value((relocatable ? 0 : os->address())
                                 + secoffset
                                 + lv_in->input_value());
     }
@@ -2468,11 +2479,12 @@ Sized_relobj_file<size, big_endian>::compute_final_local_value(
     const Symbol_table* symtab)
 {
   // This is just a wrapper of compute_final_local_value_internal.
+  const bool relocatable = parameters->options().relocatable();
   const Output_sections& out_sections(this->output_sections());
   const std::vector<Address>& out_offsets(this->section_offsets());
   return this->compute_final_local_value_internal(r_sym, lv_in, lv_out,
-                                                 out_sections, out_offsets,
-                                                 symtab);
+                                                 relocatable, out_sections,
+                                                 out_offsets, symtab);
 }
 
 // Finalize the local symbols.  Here we set the final value in
@@ -2492,6 +2504,7 @@ Sized_relobj_file<size, big_endian>::do_finalize_local_symbols(
   const unsigned int loccount = this->local_symbol_count_;
   this->local_symbol_offset_ = off;
 
+  const bool relocatable = parameters->options().relocatable();
   const Output_sections& out_sections(this->output_sections());
   const std::vector<Address>& out_offsets(this->section_offsets());
 
@@ -2500,8 +2513,9 @@ Sized_relobj_file<size, big_endian>::do_finalize_local_symbols(
       Symbol_value<size>* lv = &this->local_values_[i];
 
       Compute_final_local_value_status cflv_status =
-       this->compute_final_local_value_internal(i, lv, lv, out_sections,
-                                                out_offsets, symtab);
+       this->compute_final_local_value_internal(i, lv, lv, relocatable,
+                                                out_sections, out_offsets,
+                                                symtab);
       switch (cflv_status)
        {
        case CFLV_OK:
@@ -2666,7 +2680,6 @@ Sized_relobj_file<size, big_endian>::write_local_symbols(
       elfcpp::Sym<size, big_endian> isym(psyms);
 
       Symbol_value<size>& lv(this->local_values_[i]);
-      typename elfcpp::Elf_types<size>::Elf_Addr sym_value = lv.value(this, 0);
 
       bool is_ordinary;
       unsigned int st_shndx = this->adjust_sym_shndx(i, isym.get_st_shndx(),
@@ -2676,9 +2689,6 @@ Sized_relobj_file<size, big_endian>::write_local_symbols(
          gold_assert(st_shndx < out_sections.size());
          if (out_sections[st_shndx] == NULL)
            continue;
-         // In relocatable object files symbol values are section relative.
-         if (parameters->options().relocatable())
-           sym_value -= out_sections[st_shndx]->address();
          st_shndx = out_sections[st_shndx]->out_shndx();
          if (st_shndx >= elfcpp::SHN_LORESERVE)
            {
@@ -2698,7 +2708,7 @@ Sized_relobj_file<size, big_endian>::write_local_symbols(
          gold_assert(isym.get_st_name() < strtab_size);
          const char* name = pnames + isym.get_st_name();
          osym.put_st_name(sympool->get_offset(name));
-         osym.put_st_value(sym_value);
+         osym.put_st_value(lv.value(this, 0));
          osym.put_st_size(isym.get_st_size());
          osym.put_st_info(isym.get_st_info());
          osym.put_st_other(isym.get_st_other());
@@ -2716,7 +2726,7 @@ Sized_relobj_file<size, big_endian>::write_local_symbols(
          gold_assert(isym.get_st_name() < strtab_size);
          const char* name = pnames + isym.get_st_name();
          osym.put_st_name(dynpool->get_offset(name));
-         osym.put_st_value(sym_value);
+         osym.put_st_value(lv.value(this, 0));
          osym.put_st_size(isym.get_st_size());
          osym.put_st_info(isym.get_st_info());
          osym.put_st_other(isym.get_st_other());

diff --git a/gold/object.h b/gold/object.h
index c6c49277402c08a4b3cf4d31406483f2b7c14507..508e79cb3c07e6abc8cd28891c8f0bf80e32b0b1 100644 (file)
--- a/gold/object.h
+++ b/gold/object.h
@@ -2772,7 +2772,8 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   // LV_IN points to a local symbol value containing the input value.
   // LV_OUT points to a local symbol value storing the final output value,
   // which must not be a merged symbol value since before calling this
-  // method to avoid memory leak.  OUT_SECTIONS is an array of output
+  // method to avoid memory leak.  RELOCATABLE indicates whether we are
+  // linking a relocatable output.  OUT_SECTIONS is an array of output
   // sections.  OUT_OFFSETS is an array of offsets of the sections.  SYMTAB
   // points to a symbol table.
   //
@@ -2784,6 +2785,7 @@ class Sized_relobj_file : public Sized_relobj<size, big_endian>
   compute_final_local_value_internal(unsigned int r_sym,
                                     const Symbol_value<size>* lv_in,
                                     Symbol_value<size>* lv_out,
+                                    bool relocatable,
                                     const Output_sections& out_sections,
                                     const std::vector<Address>& out_offsets,
                                     const Symbol_table* symtab);

diff --git a/gold/powerpc.cc b/gold/powerpc.cc
index 244c221ef5e423a903b244627f893668392253b3..d52951984cdf6f68c289632057f58b897b7c7329 100644 (file)
--- a/gold/powerpc.cc
+++ b/gold/powerpc.cc
@@ -9734,6 +9734,8 @@ Target_powerpc<size, big_endian>::relocate_relocs(
       gold_assert(got2_addend != invalid_address);
     }
 
+  const bool relocatable = parameters->options().relocatable();
+
   unsigned char* pwrite = reloc_view;
   bool zap_next = false;
   for (size_t i = 0; i < reloc_count; ++i, prelocs += reloc_size)
@@ -9829,7 +9831,7 @@ Target_powerpc<size, big_endian>::relocate_relocs(
       // In an object file, r_offset is an offset within the section.
       // In an executable or dynamic object, generated by
       // --emit-relocs, r_offset is an absolute address.
-      if (!parameters->options().relocatable())
+      if (!relocatable)
        {
          offset += view_address;
          if (static_cast<Address>(offset_in_output_section) != invalid_address)
@@ -9842,8 +9844,15 @@ Target_powerpc<size, big_endian>::relocate_relocs(
       else if (strategy == Relocatable_relocs::RELOC_ADJUST_FOR_SECTION_RELA)
        {
          const Symbol_value<size>* psymval = object->local_symbol(orig_r_sym);
-         gold_assert(os != NULL);
-         addend = psymval->value(object, addend) - os->address();
+         addend = psymval->value(object, addend);
+         // In a relocatable link, the symbol value is relative to
+         // the start of the output section. For a non-relocatable
+         // link, we need to adjust the addend.
+         if (!relocatable)
+           {
+             gold_assert(os != NULL);
+             addend -= os->address();
+           }
        }
       else if (strategy == Relocatable_relocs::RELOC_SPECIAL)
        {
@@ -9866,7 +9875,7 @@ Target_powerpc<size, big_endian>::relocate_relocs(
       else
        gold_unreachable();
 
-      if (!parameters->options().relocatable())
+      if (!relocatable)
        {
          if (r_type == elfcpp::R_POWERPC_GOT_TLSGD16
              || r_type == elfcpp::R_POWERPC_GOT_TLSGD16_LO

diff --git a/gold/target-reloc.h b/gold/target-reloc.h
index c8b86c617a7e227c1b4c88fd6f54e99285fc59d0..79ed1d3e638e9712972a286ad470fe61bf743ac7 100644 (file)
--- a/gold/target-reloc.h
+++ b/gold/target-reloc.h
@@ -755,6 +755,8 @@ relocate_relocs(
 
   unsigned char* pwrite = reloc_view;
 
+  const bool relocatable = parameters->options().relocatable();
+
   for (size_t i = 0; i < reloc_count; ++i, prelocs += reloc_size)
     {
       Relocatable_relocs::Reloc_strategy strategy = relinfo->rr->strategy(i);
@@ -857,7 +859,7 @@ relocate_relocs(
       // In an object file, r_offset is an offset within the section.
       // In an executable or dynamic object, generated by
       // --emit-relocs, r_offset is an absolute address.
-      if (!parameters->options().relocatable())
+      if (!relocatable)
        {
          new_offset += view_address;
          if (offset_in_output_section != invalid_address)
@@ -892,10 +894,17 @@ relocate_relocs(
            {
            case Relocatable_relocs::RELOC_ADJUST_FOR_SECTION_RELA:
              {
-               typename elfcpp::Elf_types<size>::Elf_Swxword addend;
-               addend = Classify_reloc::get_r_addend(&reloc);
-               gold_assert(os != NULL);
-               addend = psymval->value(object, addend) - os->address();
+               typename elfcpp::Elf_types<size>::Elf_Swxword addend
+                   = Classify_reloc::get_r_addend(&reloc);
+               addend = psymval->value(object, addend);
+               // In a relocatable link, the symbol value is relative to
+               // the start of the output section. For a non-relocatable
+               // link, we need to adjust the addend.
+               if (!relocatable)
+                 {
+                   gold_assert(os != NULL);
+                   addend -= os->address();
+                 }
                Classify_reloc::put_r_addend(&reloc_write, addend);
              }
              break;

diff --git a/gold/testsuite/pr22266_a.c b/gold/testsuite/pr22266_a.c
index b58254c913bb09f354715e14fd95275066b34c24..bdc09d44160325ae58e88b1ec6233a53e1bc8bd9 100644 (file)
--- a/gold/testsuite/pr22266_a.c
+++ b/gold/testsuite/pr22266_a.c
@@ -3,3 +3,11 @@ static int int_from_a_1 = 0x11223344;
 
 __attribute__((section(".data.rel.ro.a")))
 int *p_int_from_a_2 = &int_from_a_1;
+
+const char *hello (void);
+
+const char *
+hello (void)
+{
+  return "XXXHello, world!" + 3;
+}
\ No newline at end of file

diff --git a/gold/testsuite/pr22266_main.c b/gold/testsuite/pr22266_main.c
index 1f3476efaa12ad63d09dc768681e7aae8fada1ec..b49b5e1214de78d5db353515a0780e1a343e8707 100644 (file)
--- a/gold/testsuite/pr22266_main.c
+++ b/gold/testsuite/pr22266_main.c
@@ -1,9 +1,13 @@
 #include <stdlib.h>
+#include <string.h>
 
 extern int *p_int_from_a_2;
+extern const char *hello (void);
 
 int main (void) {
   if (*p_int_from_a_2 != 0x11223344)
     abort ();
+  if (strcmp(hello(), "Hello, world!") != 0)
+    abort ();
   return 0;
 }

diff --git a/gold/testsuite/pr22266_script.t b/gold/testsuite/pr22266_script.t
index a9bc3640965052e441afd8e5eda2a858223e2c4e..7b49988093b1d8ddeb400882401e64b88c8e0e51 100644 (file)
--- a/gold/testsuite/pr22266_script.t
+++ b/gold/testsuite/pr22266_script.t
@@ -8,8 +8,9 @@ SECTIONS
     .text : {
         *(.text*)
     }
-    .rodata.cst16 : {
-        *(.rodata.cst16*)
+    .rodata :
+    {
+        *(.rodata .rodata.* .gnu.linkonce.r.*)
     }
     .data.rel.ro : {
         *(.data.rel.ro*)