[PATCH][GOLD] Use offsets within output sections during a relocatable link.

[PATCH][GOLD] Use offsets within output sections during a relocatable link.

[Doug Kwan (關振德)]

[-- Attachment #1: Type: text/plain, Size: 876 bytes --]

Hi,

    This patch fixes a problem that broke some loadable kernel modules
linked with the -r option.  We used output addresses, instead of
offsets relative to starts of output sections when we finalized values
of local symbols in merged sections.  The same problem potentially
affects relaxed sections and is also fixed by this patch.  I am not
sure if I should change the handling of section and TLS symbols so I
leave them alone.

This is tested by running the whole gold testsuite on x86_64 and
building the ARM linux kernel with a loadable module.  The kernel
module worked only with this patch.

-Doug


2010-05-27  Doug Kwan  <dougkwan@google.com>

        * object.cc (Sized_relobj::do_finalize_local_symbols): Use offset
        from start of output section instead of address for a local symbol
        in a merged or relaxed section when doing a relocatable link.

[-- Attachment #2: patch-merged-local-symbol.txt --]
[-- Type: text/plain, Size: 1426 bytes --]

Index: gold/object.cc
===================================================================
RCS file: /cvs/src/src/gold/object.cc,v
retrieving revision 1.123
diff -u -u -p -r1.123 object.cc
--- gold/object.cc	26 May 2010 15:47:39 -0000	1.123
+++ gold/object.cc	26 May 2010 15:57:55 -0000
@@ -1827,7 +1827,12 @@ Sized_relobj<size, big_endian>::do_final
 		  const Output_section_data* posd =
 		    os->find_relaxed_input_section(this, shndx);
 		  if (posd != NULL)
-		    lv.set_output_value(posd->address());
+		    {
+		      uint64_t relocatable_link_adjustment =
+			relocatable ? os->address() : 0;
+		      lv.set_output_value(posd->address()
+					  - relocatable_link_adjustment);
+		    }
 		  else
 		    lv.set_output_value(os->address());
 		}
@@ -1835,9 +1840,14 @@ Sized_relobj<size, big_endian>::do_final
 		{
 		  // 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.
+		  uint64_t adjusted_start =
+		    relocatable ? start - os->address() : start;
 		  Merged_symbol_value<size>* msv =
-		    new Merged_symbol_value<size>(lv.input_value(), start);
+		    new Merged_symbol_value<size>(lv.input_value(),
+						  adjusted_start);
 		  lv.set_merged_symbol_value(msv);
 		}
 	    }

Re: [PATCH][GOLD] Use offsets within output sections during a relocatable link.

[Ian Lance Taylor]

"Doug Kwan (關振德)" <dougkwan@google.com> writes:

> 2010-05-27  Doug Kwan  <dougkwan@google.com>
>
>         * object.cc (Sized_relobj::do_finalize_local_symbols): Use offset
>         from start of output section instead of address for a local symbol
>         in a merged or relaxed section when doing a relocatable link.

Thanks for finding this.


> --- gold/object.cc	26 May 2010 15:47:39 -0000	1.123
> +++ gold/object.cc	26 May 2010 15:57:55 -0000
> @@ -1827,7 +1827,12 @@ Sized_relobj<size, big_endian>::do_final
>  		  const Output_section_data* posd =
>  		    os->find_relaxed_input_section(this, shndx);
>  		  if (posd != NULL)
> -		    lv.set_output_value(posd->address());
> +		    {
> +		      uint64_t relocatable_link_adjustment =
> +			relocatable ? os->address() : 0;
> +		      lv.set_output_value(posd->address()
> +					  - relocatable_link_adjustment);
> +		    }

This is a template function.  The variable relocation_link_adjustment
should have type Address (a typedef in Sized_relobj), not uint64_t.


> @@ -1835,9 +1840,14 @@ Sized_relobj<size, big_endian>::do_final
>  		{
>  		  // 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.
> +		  uint64_t adjusted_start =
> +		    relocatable ? start - os->address() : start;
>  		  Merged_symbol_value<size>* msv =
> -		    new Merged_symbol_value<size>(lv.input_value(), start);
> +		    new Merged_symbol_value<size>(lv.input_value(),
> +						  adjusted_start);
>  		  lv.set_merged_symbol_value(msv);
>  		}
>  	    }

Here too, for the variable adjusted_start.

This is OK with those changes.

Thanks.

Ian

Re: [PATCH][GOLD] Use offsets within output sections during a relocatable link.

[Doug Kwan (關振德)]

I used uint64_t because START and Output_section::address() are of
type uint64_t.  I can certainly change that to Address.

-Doug

在 2010年5月27日上午1:32，Ian Lance Taylor <iant@google.com> 寫道：
> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>
>> 2010-05-27  Doug Kwan  <dougkwan@google.com>
>>
>>         * object.cc (Sized_relobj::do_finalize_local_symbols): Use offset
>>         from start of output section instead of address for a local symbol
>>         in a merged or relaxed section when doing a relocatable link.
>
> Thanks for finding this.
>
>
>> --- gold/object.cc    26 May 2010 15:47:39 -0000      1.123
>> +++ gold/object.cc    26 May 2010 15:57:55 -0000
>> @@ -1827,7 +1827,12 @@ Sized_relobj<size, big_endian>::do_final
>>                 const Output_section_data* posd =
>>                   os->find_relaxed_input_section(this, shndx);
>>                 if (posd != NULL)
>> -                 lv.set_output_value(posd->address());
>> +                 {
>> +                   uint64_t relocatable_link_adjustment =
>> +                     relocatable ? os->address() : 0;
>> +                   lv.set_output_value(posd->address()
>> +                                       - relocatable_link_adjustment);
>> +                 }
>
> This is a template function.  The variable relocation_link_adjustment
> should have type Address (a typedef in Sized_relobj), not uint64_t.
>
>
>> @@ -1835,9 +1840,14 @@ Sized_relobj<size, big_endian>::do_final
>>               {
>>                 // 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.
>> +               uint64_t adjusted_start =
>> +                 relocatable ? start - os->address() : start;
>>                 Merged_symbol_value<size>* msv =
>> -                 new Merged_symbol_value<size>(lv.input_value(), start);
>> +                 new Merged_symbol_value<size>(lv.input_value(),
>> +                                               adjusted_start);
>>                 lv.set_merged_symbol_value(msv);
>>               }
>>           }
>
> Here too, for the variable adjusted_start.
>
> This is OK with those changes.
>
> Thanks.
>
> Ian
>

Re: [PATCH][GOLD] Use offsets within output sections during a relocatable link.

[Ian Lance Taylor]

"Doug Kwan (關振德)" <dougkwan@google.com> writes:

> I used uint64_t because START and Output_section::address() are of
> type uint64_t.  I can certainly change that to Address.

Both of those are uint64_t because they are from calls to functions
which are not templated.  The values must still fit in
Sized_relobj<>::Address.

Ian

> 在 2010年5月27日上午1:32，Ian Lance Taylor <iant@google.com> 寫道：
>> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>>
>>> 2010-05-27  Doug Kwan  <dougkwan@google.com>
>>>
>>>         * object.cc (Sized_relobj::do_finalize_local_symbols): Use offset
>>>         from start of output section instead of address for a local symbol
>>>         in a merged or relaxed section when doing a relocatable link.
>>
>> Thanks for finding this.
>>
>>
>>> --- gold/object.cc    26 May 2010 15:47:39 -0000      1.123
>>> +++ gold/object.cc    26 May 2010 15:57:55 -0000
>>> @@ -1827,7 +1827,12 @@ Sized_relobj<size, big_endian>::do_final
>>>                 const Output_section_data* posd =
>>>                   os->find_relaxed_input_section(this, shndx);
>>>                 if (posd != NULL)
>>> -                 lv.set_output_value(posd->address());
>>> +                 {
>>> +                   uint64_t relocatable_link_adjustment =
>>> +                     relocatable ? os->address() : 0;
>>> +                   lv.set_output_value(posd->address()
>>> +                                       - relocatable_link_adjustment);
>>> +                 }
>>
>> This is a template function.  The variable relocation_link_adjustment
>> should have type Address (a typedef in Sized_relobj), not uint64_t.
>>
>>
>>> @@ -1835,9 +1840,14 @@ Sized_relobj<size, big_endian>::do_final
>>>               {
>>>                 // 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.
>>> +               uint64_t adjusted_start =
>>> +                 relocatable ? start - os->address() : start;
>>>                 Merged_symbol_value<size>* msv =
>>> -                 new Merged_symbol_value<size>(lv.input_value(), start);
>>> +                 new Merged_symbol_value<size>(lv.input_value(),
>>> +                                               adjusted_start);
>>>                 lv.set_merged_symbol_value(msv);
>>>               }
>>>           }
>>
>> Here too, for the variable adjusted_start.
>>
>> This is OK with those changes.
>>
>> Thanks.
>>
>> Ian
>>

Re: [PATCH][GOLD] Use offsets within output sections during a relocatable link.

[Doug Kwan (關振德)]

Thanks for the explanation.  I will change the types and check in the patch.

-Doug

在 2010年5月27日上午2:24，Ian Lance Taylor <iant@google.com> 寫道：
> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>
>> I used uint64_t because START and Output_section::address() are of
>> type uint64_t.  I can certainly change that to Address.
>
> Both of those are uint64_t because they are from calls to functions
> which are not templated.  The values must still fit in
> Sized_relobj<>::Address.
>
> Ian
>
>> 在 2010年5月27日上午1:32，Ian Lance Taylor <iant@google.com> 寫道：
>>> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>>>
>>>> 2010-05-27  Doug Kwan  <dougkwan@google.com>
>>>>
>>>>         * object.cc (Sized_relobj::do_finalize_local_symbols): Use offset
>>>>         from start of output section instead of address for a local symbol
>>>>         in a merged or relaxed section when doing a relocatable link.
>>>
>>> Thanks for finding this.
>>>
>>>
>>>> --- gold/object.cc    26 May 2010 15:47:39 -0000      1.123
>>>> +++ gold/object.cc    26 May 2010 15:57:55 -0000
>>>> @@ -1827,7 +1827,12 @@ Sized_relobj<size, big_endian>::do_final
>>>>                 const Output_section_data* posd =
>>>>                   os->find_relaxed_input_section(this, shndx);
>>>>                 if (posd != NULL)
>>>> -                 lv.set_output_value(posd->address());
>>>> +                 {
>>>> +                   uint64_t relocatable_link_adjustment =
>>>> +                     relocatable ? os->address() : 0;
>>>> +                   lv.set_output_value(posd->address()
>>>> +                                       - relocatable_link_adjustment);
>>>> +                 }
>>>
>>> This is a template function.  The variable relocation_link_adjustment
>>> should have type Address (a typedef in Sized_relobj), not uint64_t.
>>>
>>>
>>>> @@ -1835,9 +1840,14 @@ Sized_relobj<size, big_endian>::do_final
>>>>               {
>>>>                 // 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.
>>>> +               uint64_t adjusted_start =
>>>> +                 relocatable ? start - os->address() : start;
>>>>                 Merged_symbol_value<size>* msv =
>>>> -                 new Merged_symbol_value<size>(lv.input_value(), start);
>>>> +                 new Merged_symbol_value<size>(lv.input_value(),
>>>> +                                               adjusted_start);
>>>>                 lv.set_merged_symbol_value(msv);
>>>>               }
>>>>           }
>>>
>>> Here too, for the variable adjusted_start.
>>>
>>> This is OK with those changes.
>>>
>>> Thanks.
>>>
>>> Ian
>>>
>