Re: [gold] enable sorting of text sections with the same prefix

[Alexander Ivchenko <aivchenk at gmail dot com>]

Hi,

The attached patch enables sorting output sections by name (except for
special output sections like ctors, dtors etc.) when
--sort-section=name option is specified (like in bfd).

make check-gold passes for me on x86_64 linux. Is it OK for trunk?

thanks,
Alexander



2013/4/19 Alexander Ivchenko <aivchenk@gmail.com>:
> Ian, could you please take a look? I rebased the patch..
>
> thank you,
> Alexander
>
> 2013/3/12 Alexander Ivchenko <aivchenk@gmail.com>:
>> *ping*
>>
>> thanks,
>> Alexander
>>
>> 2013/2/13 Alexander Ivchenko <aivchenk@gmail.com>:
>>>>The documentation of --sort-section=name for GNU ld is unfortunately
>>>>tied to the notion of a default linker script, which gold does not
>>>>share.  Still, we ought to be able to come up with some plausible
>>>>meaning for gold.  And restricting the behaviour to .data and .bss
>>>>does not make sense to me
>>>
>>> that's true, e.g for a testcase with .data and .sdata here what ld -M gave me:
>>>
>>> .data           0x0000000000600188        0xc
>>>  *(SORT(.data) SORT(.data.*) SORT(.gnu.linkonce.d.*))
>>>  .data          0x0000000000600188        0x0 section_sorting_name_3.o
>>>  .data          0x0000000000600188        0x0 section_sorting_name_1.o
>>>  .data          0x0000000000600188        0x0 section_sorting_name_2.o
>>>  .data.0001     0x0000000000600188        0x4 section_sorting_name_1.o
>>>                 0x0000000000600188                vdata_0001
>>>  .data.0002     0x000000000060018c        0x4 section_sorting_name_2.o
>>>                 0x000000000060018c                vdata_0002
>>>  .data.0003     0x0000000000600190        0x4 section_sorting_name_3.o
>>>                 0x0000000000600190                vdata_0003
>>>
>>> .sdata.0003     0x0000000000600194        0x4
>>>  .sdata.0003    0x0000000000600194        0x4 section_sorting_name_3.o
>>>                 0x0000000000600194                vsdata_0003
>>>
>>> .sdata.0001     0x0000000000600198        0x4
>>>  .sdata.0001    0x0000000000600198        0x4 section_sorting_name_1.o
>>>                 0x0000000000600198                vsdata_0001
>>>
>>> .sdata.0002     0x000000000060019c        0x4
>>>  .sdata.0002    0x000000000060019c        0x4 section_sorting_name_2.o
>>>                 0x000000000060019c                vsdata_0002
>>>
>>> It seems like there is no particular reason for not sorting .sdata
>>> sections: just BFD script doesn't do it. Therefore I agree with Sri
>>> when he said:
>>>
>>>>Why not sort all output sections when --sort-section=name is
>>>>specified? However, for special output sections like ctors, dtors,
>>>>init_array, fini_array, etc. the original sort compare function will
>>>>be used. For all other sections, use the new sort compare can be used.
>>>>No need to hard code any names.
>>>
>>> that approach looks reasonable. Also thank you very much, Sri, for
>>> your fix. I attached the patch with your
>>> changes and also with updated ChangLog.
>>>
>>> thanks,
>>> Alexander
>>>
>>> 2013/2/12 Sriraman Tallam <tmsriram@google.com>:
>>>> On Tue, Feb 12, 2013 at 10:22 AM, Sriraman Tallam <tmsriram@google.com> wrote:
>>>>> On Tue, Feb 12, 2013 at 9:21 AM, Ian Lance Taylor <iant@google.com> wrote:
>>>>>> On Tue, Feb 12, 2013 at 7:01 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
>>>>>>> 2013/2/9 Ian Lance Taylor <iant@google.com>:
>>>>>>>> On Fri, Feb 8, 2013 at 5:11 PM, Ian Lance Taylor <iant@google.com> wrote:
>>>>>>>>> On Thu, Feb 7, 2013 at 3:56 AM, Alexander Ivchenko <aivchenk@gmail.com> wrote:
>>>>>>>>>> thank you for your help, Sri. I fixed help string and deleted warning.
>>>>>>>>>>
>>>>>>>>>> Ian, could you please take a look at the attached patch?
>>>>>>>>>
>>>>>>>>> As far as I can see, with this patch, when you use
>>>>>>>>> --sort-section=name, gold will only sort sections that start with
>>>>>>>>> .data. and .bss. by name.  Other sections remain unsorted.  This does
>>>>>>>>> not appear to be what the GNU linker does.  The GNU linker appears to
>>>>>>>>> sort all input sections by name when using --sort-section=name.
>>>>>>>>
>>>>>>>> Sorry, I guess that's not quite right.  You call the sorting code one
>>>>>>>> every section.  But you only set must_sort_attached_input_sections on
>>>>>>>> the .data and .bss sections.  How can you get away with that?
>>>>>>>>
>>>>>>>> Ian
>>>>>>>
>>>>>>> You mean from hardcoding those names (.bss and .data) there? I'm not
>>>>>>> sure so far,
>>>>>>> but I know that BFD sorts them by name when we have -sort-section=name
>>>>>>> and at the
>>>>>>> same time, BFD doesn't sort, say,.sdata and .sbss.
>>>>>>> Do we need to fully mimic the behavior of BFD for this option?
>>>>>>
>>>>>> We do not need to fully mimic GNU ld.  However, we need to understand
>>>>>> how and why GNU ld behaves the way it does.  When I look at the GNU ld
>>>>>> code, I don't see anything that restricts the effect of
>>>>>> --sort-section=name to the .data and .bss sections.  Nor is it
>>>>>> documented to behave that way.
>>>>>>
>>>>>> The documentation of --sort-section=name for GNU ld is unfortunately
>>>>>> tied to the notion of a default linker script, which gold does not
>>>>>> share.  Still, we ought to be able to come up with some plausible
>>>>>> meaning for gold.  And restricting the behaviour to .data and .bss
>>>>>> does not make sense to me.
>>>>>
>>>>> Why not sort all output sections when --sort-section=name is
>>>>> specified? However, for special output sections like ctors, dtors,
>>>>> init_array, fini_array, etc. the original sort compare function will
>>>>> be used. For all other sections, use the new sort compare can be used.
>>>>> No need to hard code any names.
>>>>>
>>>>>>
>>>>>> Also, in order for this to work correctly, you must call
>>>>>> set_may_sort_attached_input_sections when you create the output
>>>>>> section.  You aren't doing that, and I'm surprised that your code is
>>>>>> working reliably.
>>>>>
>>>>> I missed this part completely when I was reviewing his code, sorry!. I
>>>>> am not surprised his patch works for ".text" because the input
>>>>> sections are retained as he can piggy back on default text sorting.
>>>>> But, how does his test pass for bss and data? I will apply his patch
>>>>> and find out.
>>>>
>>>> I figured out how input sections are kept for some ".data" sections.
>>>> When the first object is seen, the isecn entries for its .data and
>>>> .bss are not kept. But after the first object, Layout::layout is
>>>> called which sets must_sort for .data and .bss. From then on, .data
>>>> and .bss are saved. This is definitely wrong. Infact, this patch does
>>>> not work correctly on the test case included and produces an assert in
>>>> reloc.cc:830 when I tried it. This is because some input section
>>>> entries have isecn and some do not. We encountered an instance of this
>>>> problem earlier with the text reordering patch.
>>>>
>>>> This can be fixed by removing the lines which set_must_sort in
>>>> Layout::layout and simply set_must_sort to all output sections in
>>>> Layout::make_output_section. Please note that setting may_sort and
>>>> then must_sort later is not necessary here since we know for sure that
>>>> we are going to sort this.
>>>>
>>>> I have modified this patch accordingly and attached a new patch that
>>>> sorts all output sections by name when --sort-section=name is passed.
>>>> For special output sections like .ctors, it will still use the
>>>> original sort compare function. I have not special cased if for
>>>> ".data" and ".bss" but I am not sure if ".sdata" and ".sbss" must be
>>>> ignored.
>>>>
>>>> Thanks
>>>> Sri
>>>>
>>>>
>>>>
>>>>>
>>>>> Sri
>>>>>
>>>>>>
>>>>>> Ian

Attachment: enable_sorting_sections_by_name_9.patch
Description: Binary data