[PATCH] gold: Don't adjust local symbol values in relocatable link twice

[James Clarke <jrtc27 at jrtc27 dot com>]

The fix committed for PR gold/19291 ended up breaking other cases. The
commit added adjustment code to write_local_symbols, but in many cases
compute_final_local_value_internal had already subtracted the output
section's address. To fix this, all other adjustments are now removed, so
only the one in write_local_symbols is left.

gold/
	PR gold/22266
	* object.cc (Sized_relobj_file::compute_final_local_value_internal):
	Drop relocatable parameter and stop adjusting output value based on
	it.
	(Sized_relobj_file::compute_final_local_value): Stop passing
	relocatable to compute_final_local_value_internal.
	(Sized_relobj_file::do_finalize_local_symbols): Ditto.
	* object.h (Sized_relobj_file::compute_final_local_value_internal):
	Drop relocatable parameter.
---

A simple test is available at http://paste.debian.net/990796/, which is
much simpler than the real-world example in PR gold/22266. You can see
that int_from_a_1 ends up having a negative (well, large positive since
it's unsigned) symbol value when gold is used before this patch, so the
value printed by the program is not the expected 11223344 but whatever
happens to be before it in memory.

 gold/object.cc | 27 ++++++++-------------------
 gold/object.h  |  4 +---
 2 files changed, 9 insertions(+), 22 deletions(-)

diff --git a/gold/object.cc b/gold/object.cc
index 4110686ff3..22a41a2db6 100644
--- a/gold/object.cc
+++ b/gold/object.cc
@@ -2318,7 +2318,6 @@ 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)
@@ -2420,10 +2419,7 @@ Sized_relobj_file<size, big_endian>::compute_final_local_value_internal(
 		os->find_relaxed_input_section(this, shndx);
 	      if (posd != NULL)
 		{
-		  Address relocatable_link_adjustment =
-		    relocatable ? os->address() : 0;
-		  lv_out->set_output_value(posd->address()
-					   - relocatable_link_adjustment);
+		  lv_out->set_output_value(posd->address());
 		}
 	      else
 		lv_out->set_output_value(os->address());
@@ -2432,14 +2428,10 @@ 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.  If we are doing a relocatable
-	      // link, use offset from start output section instead of
-	      // address.
-	      Address adjusted_start =
-		relocatable ? start - os->address() : start;
+	      // of this input section.
 	      Merged_symbol_value<size>* msv =
 		new Merged_symbol_value<size>(lv_in->input_value(),
-					      adjusted_start);
+					      start);
 	      lv_out->set_merged_symbol_value(msv);
 	    }
 	}
@@ -2450,7 +2442,7 @@ Sized_relobj_file<size, big_endian>::compute_final_local_value_internal(
 				 + secoffset
 				 + lv_in->input_value());
       else
-	lv_out->set_output_value((relocatable ? 0 : os->address())
+	lv_out->set_output_value(os->address()
 				 + secoffset
 				 + lv_in->input_value());
     }
@@ -2476,12 +2468,11 @@ 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,
-						  relocatable, out_sections,
-						  out_offsets, symtab);
+						  out_sections, out_offsets,
+						  symtab);
 }
 
 // Finalize the local symbols.  Here we set the final value in
@@ -2501,7 +2492,6 @@ 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());
 
@@ -2510,9 +2500,8 @@ 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, relocatable,
-						 out_sections, out_offsets,
-						 symtab);
+	this->compute_final_local_value_internal(i, lv, lv, out_sections,
+						 out_offsets, symtab);
       switch (cflv_status)
 	{
 	case CFLV_OK:
diff --git a/gold/object.h b/gold/object.h
index 508e79cb3c..c6c4927740 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -2772,8 +2772,7 @@ 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.  RELOCATABLE indicates whether we are
-  // linking a relocatable output.  OUT_SECTIONS is an array of output
+  // method to avoid memory leak.  OUT_SECTIONS is an array of output
   // sections.  OUT_OFFSETS is an array of offsets of the sections.  SYMTAB
   // points to a symbol table.
   //
@@ -2785,7 +2784,6 @@ 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);
-- 
2.14.2