public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
@ 2010-10-19  8:17 Doug Kwan (關振德)
  2010-10-19 14:08 ` Ian Lance Taylor
  0 siblings, 1 reply; 12+ messages in thread
From: Doug Kwan (關振德) @ 2010-10-19  8:17 UTC (permalink / raw)
  To: Ian Lance Taylor, binutils

Hi

    This patch changes code writing output section headers so that the
SHF_LINK_ORDER flag of a section of type SHT_ARM_EXIDX is always set.
The flag is required to be set for such a section by the ARM EHABI.
The existing code drops the SHF_LINK_ORDER flag and that confuses
other tools.  Gold does not handle SHF_LINK_ORDER in general but the
ARM back-end can handle the EXIDX sections.

-Doug


2010-10-19  Doug Kwan  <dougkwan@google.com>

        * output.cc(Output_section::write_header): Set SHF_LINK_ORDER flags of
        ARM EXIDX sections.

Index: gold/output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.136
diff -u -u -p -r1.136 output.cc
--- gold/output.cc	18 Oct 2010 05:39:23 -0000	1.136
+++ gold/output.cc	19 Oct 2010 08:09:15 -0000
@@ -3091,6 +3091,10 @@ Output_section::write_header(const Layou
   elfcpp::Elf_Xword flags = this->flags_;
   if (this->info_section_ != NULL && this->info_uses_section_index_)
     flags |= elfcpp::SHF_INFO_LINK;
+  // ARM EHABI requires a SHT_ARM_EXIDX section to have the SHF_LINK_ORDER
+  // flag set.
+  if (this->type_ == elfcpp::SHT_ARM_EXIDX)
+    flags |= elfcpp::SHF_LINK_ORDER;
   oshdr->put_sh_flags(flags);

   oshdr->put_sh_addr(this->address());

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
  2010-10-19  8:17 [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections Doug Kwan (關振德)
@ 2010-10-19 14:08 ` Ian Lance Taylor
  2010-10-19 16:12   ` Doug Kwan (關振德)
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Lance Taylor @ 2010-10-19 14:08 UTC (permalink / raw)
  To: Doug Kwan (關振德); +Cc: binutils

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

>     This patch changes code writing output section headers so that the
> SHF_LINK_ORDER flag of a section of type SHT_ARM_EXIDX is always set.
> The flag is required to be set for such a section by the ARM EHABI.
> The existing code drops the SHF_LINK_ORDER flag and that confuses
> other tools.  Gold does not handle SHF_LINK_ORDER in general but the
> ARM back-end can handle the EXIDX sections.
>
> -Doug
>
>
> 2010-10-19  Doug Kwan  <dougkwan@google.com>
>
>         * output.cc(Output_section::write_header): Set SHF_LINK_ORDER flags of
>         ARM EXIDX sections.

I don't see the ARM backend actually creating any SHT_ARM_EXIDX
sections.  So it seems to me that they are input sections.  If the ABI
requires the input sections to have the SHF_LINK_ORDER flag set, then
that setting should carry through to the output section.  Does that not
happen?

Ian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
  2010-10-19 14:08 ` Ian Lance Taylor
@ 2010-10-19 16:12   ` Doug Kwan (關振德)
  2010-10-19 16:33     ` Cary Coutant
  2010-10-19 16:54     ` Ian Lance Taylor
  0 siblings, 2 replies; 12+ messages in thread
From: Doug Kwan (關振德) @ 2010-10-19 16:12 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: binutils

Gold does not handle SHF_LINK_ORDER flag in general, we drop the flag
when searching for an output section.

Output_section*
Layout::choose_output_section(const Relobj* relobj, const char* name,
                              elfcpp::Elf_Word type, elfcpp::Elf_Xword flags,
                              bool is_input_section, Output_section_order order,
                              bool is_relro)
...

  // Some flags in the input section should not be automatically
  // copied to the output section.
  flags &= ~ (elfcpp::SHF_INFO_LINK
              | elfcpp::SHF_LINK_ORDER
              | elfcpp::SHF_GROUP
              | elfcpp::SHF_MERGE


在 2010年10月19日下午10:07,Ian Lance Taylor <iant@google.com> 寫道:
> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>
>>     This patch changes code writing output section headers so that the
>> SHF_LINK_ORDER flag of a section of type SHT_ARM_EXIDX is always set.
>> The flag is required to be set for such a section by the ARM EHABI.
>> The existing code drops the SHF_LINK_ORDER flag and that confuses
>> other tools.  Gold does not handle SHF_LINK_ORDER in general but the
>> ARM back-end can handle the EXIDX sections.
>>
>> -Doug
>>
>>
>> 2010-10-19  Doug Kwan  <dougkwan@google.com>
>>
>>         * output.cc(Output_section::write_header): Set SHF_LINK_ORDER flags of
>>         ARM EXIDX sections.
>
> I don't see the ARM backend actually creating any SHT_ARM_EXIDX
> sections.  So it seems to me that they are input sections.  If the ABI
> requires the input sections to have the SHF_LINK_ORDER flag set, then
> that setting should carry through to the output section.  Does that not
> happen?
>
> Ian
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
  2010-10-19 16:12   ` Doug Kwan (關振德)
@ 2010-10-19 16:33     ` Cary Coutant
  2010-10-19 16:52       ` Doug Kwan (關振德)
  2010-10-19 16:54     ` Ian Lance Taylor
  1 sibling, 1 reply; 12+ messages in thread
From: Cary Coutant @ 2010-10-19 16:33 UTC (permalink / raw)
  To: Doug Kwan (關振德); +Cc: Ian Lance Taylor, binutils

> Gold does not handle SHF_LINK_ORDER flag in general, we drop the flag
> when searching for an output section.

The code you quoted is only dropping it for the purposes of looking
for an already-existing matching output section, which is the right
thing to do. As far as I can tell, if the first input section has
SHF_LINK_ORDER set, then the output section should also have it set.

All that said, the LINK_ORDER flag has no meaning at all for anything
but ET_REL files. It's a signal to the linker that the input sections
must be sorted in the same relative order as the corresponding
sections indicated by the sh_link field. Any tool that's getting
confused if LINK_ORDER isn't set in an ET_EXEC or ET_DYN file is doing
something wrong -- probably just overzealously insisting on seeing a
flag simply because it was there in their first sample output. If the
tool is trying to verify that that section is actually sorted
correctly, looking for the flag isn't a guarantee of that -- gold
doesn't actually implement the ordering implied by the flag (but it
also doesn't do any reordering that would require it to do so). It
would be better for the tool to check the actual ordering of the
section for consistency with the ordering of the corresponding
section.

Is the problem you've encountered with -r links?

-cary

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
  2010-10-19 16:33     ` Cary Coutant
@ 2010-10-19 16:52       ` Doug Kwan (關振德)
  0 siblings, 0 replies; 12+ messages in thread
From: Doug Kwan (關振德) @ 2010-10-19 16:52 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Ian Lance Taylor, binutils

The input EXIDX sections are sorted by the ARM backend whether
SHF_LINK_ORDER flag is set or not.  In the output, gold sets the
sh_link field of section headers of EXIDX sections correctly to their
respective text section but the SHF_LINK_ORDER flag is dropped for
some reason.  The output is then passed to strip, which becomes
confused because of lack of SHF_LINK_ORDER flag and clears all the
sh_link field of EXIDX section headers.

Yes, the problem happens during linking of a relocatable kernel loadable module.

-Doug

在 2010年10月20日上午12:33,Cary Coutant <ccoutant@google.com> 寫道:
>> Gold does not handle SHF_LINK_ORDER flag in general, we drop the flag
>> when searching for an output section.
>
> The code you quoted is only dropping it for the purposes of looking
> for an already-existing matching output section, which is the right
> thing to do. As far as I can tell, if the first input section has
> SHF_LINK_ORDER set, then the output section should also have it set.
>
> All that said, the LINK_ORDER flag has no meaning at all for anything
> but ET_REL files. It's a signal to the linker that the input sections
> must be sorted in the same relative order as the corresponding
> sections indicated by the sh_link field. Any tool that's getting
> confused if LINK_ORDER isn't set in an ET_EXEC or ET_DYN file is doing
> something wrong -- probably just overzealously insisting on seeing a
> flag simply because it was there in their first sample output. If the
> tool is trying to verify that that section is actually sorted
> correctly, looking for the flag isn't a guarantee of that -- gold
> doesn't actually implement the ordering implied by the flag (but it
> also doesn't do any reordering that would require it to do so). It
> would be better for the tool to check the actual ordering of the
> section for consistency with the ordering of the corresponding
> section.
>
> Is the problem you've encountered with -r links?
>
> -cary
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
  2010-10-19 16:12   ` Doug Kwan (關振德)
  2010-10-19 16:33     ` Cary Coutant
@ 2010-10-19 16:54     ` Ian Lance Taylor
  2010-10-19 18:54       ` Doug Kwan (關振德)
  1 sibling, 1 reply; 12+ messages in thread
From: Ian Lance Taylor @ 2010-10-19 16:54 UTC (permalink / raw)
  To: Doug Kwan (關振德); +Cc: binutils

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

> Gold does not handle SHF_LINK_ORDER flag in general, we drop the flag
> when searching for an output section.
>
> Output_section*
> Layout::choose_output_section(const Relobj* relobj, const char* name,
>                               elfcpp::Elf_Word type, elfcpp::Elf_Xword flags,
>                               bool is_input_section, Output_section_order order,
>                               bool is_relro)
> ...
>
>   // Some flags in the input section should not be automatically
>   // copied to the output section.
>   flags &= ~ (elfcpp::SHF_INFO_LINK
>               | elfcpp::SHF_LINK_ORDER
>               | elfcpp::SHF_GROUP
>               | elfcpp::SHF_MERGE

Oh yeah, sorry about that.

That is incorrect when generating relocatable output.  Rather than
testing for SHT_ARM_EXIDX in the target-independent code, suppose we add
another bit flag to Output_section.  Then we can set it if
SHF_LINK_ORDER is set in the input section, which will be correct once
we finally support SHF_LINK_ORDER.  And the ARM backend can set it at
will.

Ian

> 在 2010年10月19日下午10:07,Ian Lance Taylor <iant@google.com> 寫道:
>> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>>
>>>     This patch changes code writing output section headers so that the
>>> SHF_LINK_ORDER flag of a section of type SHT_ARM_EXIDX is always set.
>>> The flag is required to be set for such a section by the ARM EHABI.
>>> The existing code drops the SHF_LINK_ORDER flag and that confuses
>>> other tools.  Gold does not handle SHF_LINK_ORDER in general but the
>>> ARM back-end can handle the EXIDX sections.
>>>
>>> -Doug
>>>
>>>
>>> 2010-10-19  Doug Kwan  <dougkwan@google.com>
>>>
>>>         * output.cc(Output_section::write_header): Set SHF_LINK_ORDER flags of
>>>         ARM EXIDX sections.
>>
>> I don't see the ARM backend actually creating any SHT_ARM_EXIDX
>> sections.  So it seems to me that they are input sections.  If the ABI
>> requires the input sections to have the SHF_LINK_ORDER flag set, then
>> that setting should carry through to the output section.  Does that not
>> happen?
>>
>> Ian
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
  2010-10-19 16:54     ` Ian Lance Taylor
@ 2010-10-19 18:54       ` Doug Kwan (關振德)
  2010-10-19 19:29         ` Ian Lance Taylor
       [not found]         ` <AANLkTimBgvCfeY3gTPV8UkUPH2igcmhzd=LJDVF5gRgd@mail.gmail.com>
  0 siblings, 2 replies; 12+ messages in thread
From: Doug Kwan (關振德) @ 2010-10-19 18:54 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: binutils

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

Here is a revised patch.

-Doug

2010-10-19  Doug Kwan  <dougkwan@google.com>

	* arm.cc (Target_arm::do_finalize_sections): Force SHF_LINK_ORDER
	flag in section headers of EXIDX sections in a relocatable link.
	* output.cc (Output_section::Output_section): Initialize member
	force_link_order_.
	* output.h (Output_section::force_link_order): New method.
	(Output_section::set_force_link_order): Ditto.
	(Output_section::force_link_order_): New data member.


在 2010年10月20日上午12:53,Ian Lance Taylor <iant@google.com> 寫道:
> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>
>> Gold does not handle SHF_LINK_ORDER flag in general, we drop the flag
>> when searching for an output section.
>>
>> Output_section*
>> Layout::choose_output_section(const Relobj* relobj, const char* name,
>>                               elfcpp::Elf_Word type, elfcpp::Elf_Xword flags,
>>                               bool is_input_section, Output_section_order order,
>>                               bool is_relro)
>> ...
>>
>>   // Some flags in the input section should not be automatically
>>   // copied to the output section.
>>   flags &= ~ (elfcpp::SHF_INFO_LINK
>>               | elfcpp::SHF_LINK_ORDER
>>               | elfcpp::SHF_GROUP
>>               | elfcpp::SHF_MERGE
>
> Oh yeah, sorry about that.
>
> That is incorrect when generating relocatable output.  Rather than
> testing for SHT_ARM_EXIDX in the target-independent code, suppose we add
> another bit flag to Output_section.  Then we can set it if
> SHF_LINK_ORDER is set in the input section, which will be correct once
> we finally support SHF_LINK_ORDER.  And the ARM backend can set it at
> will.
>
> Ian
>
>> 在 2010年10月19日下午10:07,Ian Lance Taylor <iant@google.com> 寫道:
>>> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>>>
>>>>     This patch changes code writing output section headers so that the
>>>> SHF_LINK_ORDER flag of a section of type SHT_ARM_EXIDX is always set.
>>>> The flag is required to be set for such a section by the ARM EHABI.
>>>> The existing code drops the SHF_LINK_ORDER flag and that confuses
>>>> other tools.  Gold does not handle SHF_LINK_ORDER in general but the
>>>> ARM back-end can handle the EXIDX sections.
>>>>
>>>> -Doug
>>>>
>>>>
>>>> 2010-10-19  Doug Kwan  <dougkwan@google.com>
>>>>
>>>>         * output.cc(Output_section::write_header): Set SHF_LINK_ORDER flags of
>>>>         ARM EXIDX sections.
>>>
>>> I don't see the ARM backend actually creating any SHT_ARM_EXIDX
>>> sections.  So it seems to me that they are input sections.  If the ABI
>>> requires the input sections to have the SHF_LINK_ORDER flag set, then
>>> that setting should carry through to the output section.  Does that not
>>> happen?
>>>
>>> Ian
>>>
>

[-- Attachment #2: patch-link-order.txt --]
[-- Type: text/plain, Size: 2786 bytes --]

Index: gold/arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.124
diff -u -u -p -r1.124 arm.cc
--- gold/arm.cc	17 Oct 2010 15:12:50 -0000	1.124
+++ gold/arm.cc	19 Oct 2010 18:37:01 -0000
@@ -8497,6 +8497,9 @@ Target_arm<big_endian>::do_finalize_sect
 	Arm_output_section<big_endian>* os =
 	  Arm_output_section<big_endian>::as_arm_output_section(*p);
 	os->set_exidx_section_link();
+	// Make sure SHF_LINK_ORDER flag is set.
+	if (parameters->options().relocatable())
+	  os->set_force_link_order(true);
       }
 }
 
Index: gold/output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.136
diff -u -u -p -r1.136 output.cc
--- gold/output.cc	18 Oct 2010 05:39:23 -0000	1.136
+++ gold/output.cc	19 Oct 2010 18:37:01 -0000
@@ -2005,7 +2005,8 @@ Output_section::Output_section(const cha
     always_keeps_input_sections_(false),
     tls_offset_(0),
     checkpoint_(NULL),
-    lookup_maps_(new Output_section_lookup_maps)
+    lookup_maps_(new Output_section_lookup_maps),
+    force_link_order_(false)
 {
   // An unallocated section has no address.  Forcing this means that
   // we don't need special treatment for symbols defined in debug
@@ -3091,6 +3092,8 @@ Output_section::write_header(const Layou
   elfcpp::Elf_Xword flags = this->flags_;
   if (this->info_section_ != NULL && this->info_uses_section_index_)
     flags |= elfcpp::SHF_INFO_LINK;
+  if (this->force_link_order_)
+    flags |= elfcpp::SHF_LINK_ORDER;
   oshdr->put_sh_flags(flags);
 
   oshdr->put_sh_addr(this->address());
Index: gold/output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.115
diff -u -u -p -r1.115 output.h
--- gold/output.h	18 Oct 2010 05:39:23 -0000	1.115
+++ gold/output.h	19 Oct 2010 18:37:01 -0000
@@ -3314,6 +3314,16 @@ class Output_section : public Output_dat
   void
   print_merge_stats();
 
+  // Whether we always set SHF_LINK_ORDER in section header.
+  bool
+  force_link_order() const
+  { return this->force_link_order_; }
+
+  // Force setting SHF_LINK_ORDER in output section header. 
+  void
+  set_force_link_order(bool value)
+  { this->force_link_order_ = value; }
+
  protected:
   // Return the output section--i.e., the object itself.
   Output_section*
@@ -3765,6 +3775,8 @@ class Output_section : public Output_dat
   Checkpoint_output_section* checkpoint_;
   // Fast lookup maps for merged and relaxed input sections.
   Output_section_lookup_maps* lookup_maps_;
+  // Force SHF_LINK_ORDER in section header.
+  bool force_link_order_;
 };
 
 // An output segment.  PT_LOAD segments are built from collections of

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
  2010-10-19 18:54       ` Doug Kwan (關振德)
@ 2010-10-19 19:29         ` Ian Lance Taylor
       [not found]         ` <AANLkTimBgvCfeY3gTPV8UkUPH2igcmhzd=LJDVF5gRgd@mail.gmail.com>
  1 sibling, 0 replies; 12+ messages in thread
From: Ian Lance Taylor @ 2010-10-19 19:29 UTC (permalink / raw)
  To: Doug Kwan (關振德); +Cc: binutils

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

> 2010-10-19  Doug Kwan  <dougkwan@google.com>
>
> 	* arm.cc (Target_arm::do_finalize_sections): Force SHF_LINK_ORDER
> 	flag in section headers of EXIDX sections in a relocatable link.
> 	* output.cc (Output_section::Output_section): Initialize member
> 	force_link_order_.
> 	* output.h (Output_section::force_link_order): New method.
> 	(Output_section::set_force_link_order): Ditto.
> 	(Output_section::force_link_order_): New data member.


> +  // Whether we always set SHF_LINK_ORDER in section header.
> +  bool
> +  force_link_order() const
> +  { return this->force_link_order_; }
> +
> +  // Force setting SHF_LINK_ORDER in output section header. 
> +  void
> +  set_force_link_order(bool value)
> +  { this->force_link_order_ = value; }
> +
>   protected:
>    // Return the output section--i.e., the object itself.
>    Output_section*
> @@ -3765,6 +3775,8 @@ class Output_section : public Output_dat
>    Checkpoint_output_section* checkpoint_;
>    // Fast lookup maps for merged and relaxed input sections.
>    Output_section_lookup_maps* lookup_maps_;
> +  // Force SHF_LINK_ORDER in section header.
> +  bool force_link_order_;
>  };

Name the field simply is_link_order_, make it a 1 bit bitfield, and put
it just after is_noload_.  Name the accessors is_link_order and
set_is_link_order.

This is OK with those changes.

Thanks.

Ian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
       [not found]         ` <AANLkTimBgvCfeY3gTPV8UkUPH2igcmhzd=LJDVF5gRgd@mail.gmail.com>
@ 2010-10-19 20:29           ` Ian Lance Taylor
       [not found]           ` <AANLkTi=4DTSjUz5HCbU8uARWHEd8Deh3jhFYgrxqpaU2@mail.gmail.com>
  1 sibling, 0 replies; 12+ messages in thread
From: Ian Lance Taylor @ 2010-10-19 20:29 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Doug Kwan (關振德), binutils

Cary Coutant <ccoutant@google.com> writes:

> I'm not wild about this patch. I think it would be better to just refuse to
> combine sections with unlike flags in a -r link -- ie, don't filter out so
> many of the flags. I'm not near the code at the moment, but I don't think
> any target-specific magic is necessary here.

That approach seems reasonable to me too--taking a different approach
when doing a relocatable link.

Ian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
       [not found]           ` <AANLkTi=4DTSjUz5HCbU8uARWHEd8Deh3jhFYgrxqpaU2@mail.gmail.com>
@ 2010-10-20 10:39             ` Doug Kwan (關振德)
  2010-10-20 14:54               ` Ian Lance Taylor
  2010-10-20 17:41               ` Cary Coutant
  0 siblings, 2 replies; 12+ messages in thread
From: Doug Kwan (關振德) @ 2010-10-20 10:39 UTC (permalink / raw)
  To: Cary Coutant, Ian Lance Taylor; +Cc: binutils

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

Here is a new patch.  I changed the code in layout.cc so that
SHF_LINK_ORDER is not filter in a relocatable link.  I also added a
check in the ARM back-end to warn about EXIDX input sections without
SHF_LINK_ORDER flags.

-Doug

2010-10-20  Doug Kwan  <dougkwan@google.com>

        * arm.cc (Arm_relobj::do_read_symbols): Warn about ARM EXIDX sections
        without SHF_LINK_ORDER flags.
        * layout.cc (Layout::choose_output_section): Do not filter
        SHF_LINK_ORDER flag in a relocatable link.


在 2010年10月20日上午8:41,Doug Kwan (關振德) <dougkwan@google.com> 寫道:
> I will try your approach.
>
> On Oct 20, 2010 3:39 AM, "Cary Coutant" <ccoutant@google.com> wrote:
>> I'm not wild about this patch. I think it would be better to just refuse
>> to
>> combine sections with unlike flags in a -r link -- ie, don't filter out so
>> many of the flags. I'm not near the code at the moment, but I don't think
>> any target-specific magic is necessary here.
>>
>> -cary
>> On Oct 19, 2010 11:54 AM, "Doug Kwan (關振德)" <dougkwan@google.com> wrote:
>>> Here is a revised patch.
>>>
>>> -Doug
>>>
>>> 2010-10-19 Doug Kwan <dougkwan@google.com>
>>>
>>> * arm.cc (Target_arm::do_finalize_sections): Force SHF_LINK_ORDER
>>> flag in section headers of EXIDX sections in a relocatable link.
>>> * output.cc (Output_section::Output_section): Initialize member
>>> force_link_order_.
>>> * output.h (Output_section::force_link_order): New method.
>>> (Output_section::set_force_link_order): Ditto.
>>> (Output_section::force_link_order_): New data member.
>>>
>>>
>>> 在 2010年10月20日上午12:53,Ian Lance Taylor <iant@google.com> 寫道:
>>>> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>>>>
>>>>> Gold does not handle SHF_LINK_ORDER flag in general, we drop the flag
>>>>> when searching for an output section.
>>>>>
>>>>> Output_section*
>>>>> Layout::choose_output_section(const Relobj* relobj, const char* name,
>>>>> elfcpp::Elf_Word type, elfcpp::Elf_Xword flags,
>>>>> bool is_input_section, Output_section_order order,
>>>>> bool is_relro)
>>>>> ...
>>>>>
>>>>> // Some flags in the input section should not be automatically
>>>>> // copied to the output section.
>>>>> flags &= ~ (elfcpp::SHF_INFO_LINK
>>>>> | elfcpp::SHF_LINK_ORDER
>>>>> | elfcpp::SHF_GROUP
>>>>> | elfcpp::SHF_MERGE
>>>>
>>>> Oh yeah, sorry about that.
>>>>
>>>> That is incorrect when generating relocatable output. Rather than
>>>> testing for SHT_ARM_EXIDX in the target-independent code, suppose we add
>>>> another bit flag to Output_section. Then we can set it if
>>>> SHF_LINK_ORDER is set in the input section, which will be correct once
>>>> we finally support SHF_LINK_ORDER. And the ARM backend can set it at
>>>> will.
>>>>
>>>> Ian
>>>>
>>>>> 在 2010年10月19日下午10:07,Ian Lance Taylor <iant@google.com> 寫道:
>>>>>> "Doug Kwan (關振德)" <dougkwan@google.com> writes:
>>>>>>
>>>>>>> This patch changes code writing output section headers so that the
>>>>>>> SHF_LINK_ORDER flag of a section of type SHT_ARM_EXIDX is always set.
>>>>>>> The flag is required to be set for such a section by the ARM EHABI.
>>>>>>> The existing code drops the SHF_LINK_ORDER flag and that confuses
>>>>>>> other tools. Gold does not handle SHF_LINK_ORDER in general but the
>>>>>>> ARM back-end can handle the EXIDX sections.
>>>>>>>
>>>>>>> -Doug
>>>>>>>
>>>>>>>
>>>>>>> 2010-10-19 Doug Kwan <dougkwan@google.com>
>>>>>>>
>>>>>>> * output.cc(Output_section::write_header): Set SHF_LINK_ORDER flags
>>>>>>> of
>>>>>>> ARM EXIDX sections.
>>>>>>
>>>>>> I don't see the ARM backend actually creating any SHT_ARM_EXIDX
>>>>>> sections. So it seems to me that they are input sections. If the ABI
>>>>>> requires the input sections to have the SHF_LINK_ORDER flag set, then
>>>>>> that setting should carry through to the output section. Does that not
>>>>>> happen?
>>>>>>
>>>>>> Ian
>>>>>>
>>>>
>

[-- Attachment #2: patch-no-filter.txt --]
[-- Type: text/plain, Size: 1659 bytes --]

? patch-force-link-order.txt
Index: gold/arm.cc
===================================================================
RCS file: /cvs/src/src/gold/arm.cc,v
retrieving revision 1.124
diff -u -u -p -r1.124 arm.cc
--- gold/arm.cc	17 Oct 2010 15:12:50 -0000	1.124
+++ gold/arm.cc	20 Oct 2010 10:31:22 -0000
@@ -6662,6 +6662,10 @@ Arm_relobj<big_endian>::do_read_symbols(
 						     + text_shndx * shdr_size);
 	      this->make_exidx_input_section(i, shdr, text_shndx, text_shdr);
 	    }
+	  // EHABI 4.4.1 requires that SHF_LINK_ORDER flag to be set.
+	  if ((shdr.get_sh_flags() & elfcpp::SHF_LINK_ORDER) == 0)
+	    gold_warning(_("SHF_LINK_ORDER not set in EXIDX section %s of %s"),
+			 this->section_name(i).c_str(), this->name().c_str());
 	}
     }
 
Index: gold/layout.cc
===================================================================
RCS file: /cvs/src/src/gold/layout.cc,v
retrieving revision 1.184
diff -u -u -p -r1.184 layout.cc
--- gold/layout.cc	18 Oct 2010 05:39:22 -0000	1.184
+++ gold/layout.cc	20 Oct 2010 10:31:22 -0000
@@ -489,11 +489,15 @@ Layout::choose_output_section(const Relo
   // Some flags in the input section should not be automatically
   // copied to the output section.
   flags &= ~ (elfcpp::SHF_INFO_LINK
-	      | elfcpp::SHF_LINK_ORDER
 	      | elfcpp::SHF_GROUP
 	      | elfcpp::SHF_MERGE
 	      | elfcpp::SHF_STRINGS);
 
+  // We only clear the SHF_LINK_ORDER flag in for
+  // a non-relocatable link.
+  if (!parameters->options().relocatable())
+    flags &= ~elfcpp::SHF_LINK_ORDER;
+
   if (this->script_options_->saw_sections_clause())
     {
       // We are using a SECTIONS clause, so the output section is

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
  2010-10-20 10:39             ` Doug Kwan (關振德)
@ 2010-10-20 14:54               ` Ian Lance Taylor
  2010-10-20 17:41               ` Cary Coutant
  1 sibling, 0 replies; 12+ messages in thread
From: Ian Lance Taylor @ 2010-10-20 14:54 UTC (permalink / raw)
  To: Doug Kwan (關振德); +Cc: Cary Coutant, binutils

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

> 2010-10-20  Doug Kwan  <dougkwan@google.com>
>
>         * arm.cc (Arm_relobj::do_read_symbols): Warn about ARM EXIDX sections
>         without SHF_LINK_ORDER flags.
>         * layout.cc (Layout::choose_output_section): Do not filter
>         SHF_LINK_ORDER flag in a relocatable link.

This is OK.

Thanks.

Ian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections.
  2010-10-20 10:39             ` Doug Kwan (關振德)
  2010-10-20 14:54               ` Ian Lance Taylor
@ 2010-10-20 17:41               ` Cary Coutant
  1 sibling, 0 replies; 12+ messages in thread
From: Cary Coutant @ 2010-10-20 17:41 UTC (permalink / raw)
  To: Doug Kwan (關振德); +Cc: Ian Lance Taylor, binutils

> Here is a new patch.  I changed the code in layout.cc so that
> SHF_LINK_ORDER is not filter in a relocatable link.  I also added a
> check in the ARM back-end to warn about EXIDX input sections without
> SHF_LINK_ORDER flags.

Thanks, that looks better!

As a separate issue, there may be a couple of other flags we don't
want to clear for -r links as well. I'll take a closer look at that.

-cary

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2010-10-20 17:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-19  8:17 [GOLD][PATCH] Set SHF_LINK_ORDER flags of ARM EXIDX sections Doug Kwan (關振德)
2010-10-19 14:08 ` Ian Lance Taylor
2010-10-19 16:12   ` Doug Kwan (關振德)
2010-10-19 16:33     ` Cary Coutant
2010-10-19 16:52       ` Doug Kwan (關振德)
2010-10-19 16:54     ` Ian Lance Taylor
2010-10-19 18:54       ` Doug Kwan (關振德)
2010-10-19 19:29         ` Ian Lance Taylor
     [not found]         ` <AANLkTimBgvCfeY3gTPV8UkUPH2igcmhzd=LJDVF5gRgd@mail.gmail.com>
2010-10-19 20:29           ` Ian Lance Taylor
     [not found]           ` <AANLkTi=4DTSjUz5HCbU8uARWHEd8Deh3jhFYgrxqpaU2@mail.gmail.com>
2010-10-20 10:39             ` Doug Kwan (關振德)
2010-10-20 14:54               ` Ian Lance Taylor
2010-10-20 17:41               ` Cary Coutant

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).