Re: PATCH: PR ld/14156: -sort-section=alignment trashes init/fini sections, and anything similar

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Jul 5, 2012 at 8:48 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jun 28, 2012 at 6:56 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Jun 27, 2012 at 11:47 PM, Alan Modra <amodra@gmail.com> wrote:
>>> On Wed, Jun 27, 2012 at 07:12:47PM -0700, H.J. Lu wrote:
>>>> On Wed, Jun 27, 2012 at 6:26 PM, Alan Modra <amodra@gmail.com> wrote:
>>>> > On Wed, Jun 27, 2012 at 10:30:11AM -0700, H.J. Lu wrote:
>>>> >>           case lang_wild_statement_enum:
>>>> >>             sec = s->wild_statement.section_list;
>>>> >> -           for (sec = s->wild_statement.section_list; sec != NULL;
>>>> >> -                sec = sec->next)
>>>> >> +           /* Don't sort .init/.fini sections.  */
>>>> >> +           if (sec == NULL
>>>> >> +               || strcmp (sec->spec.name, ".init") == 0
>>>> >> +               || strcmp (sec->spec.name, ".fini") == 0)
>>>> >> +             break;
>>>> >> +           for (; sec != NULL; sec = sec->next)
>>>> >>               {
>>>> >>                 switch (sec->spec.sorted)
>>>> >>                   {
>>>> >
>>>> > If we are going to hard code .init and .fini names (and we do that
>>>> > elsewhere) I think it would be better to disable the sorting when
>>>> > looking at the output_section_statement rather than the
>>>> > wild_statement.
>>>>
>>>> I thought about to do it at
>>>>
>>>>             case lang_output_section_statement_enum:
>>>>               update_wild_statements
>>>>                 (s->output_section_statement.children.head);
>>>>               break;
>>>>
>>>> Then I had to handle all those different enums.  It is
>>>
>>> Just don't descend the tree at .init and .fini output section
>>> statements.
>>>
>>>            case lang_output_section_statement_enum:
>>>              if (strcmp (s->output_section_statement.name, ".init") != 0
>>>                  && strcmp (s->output_section_statement.name, ".fini") != 0)
>>>                update_wild_statements
>>>                  (s->output_section_statement.children.head);
>>>              break;
>>>
>>>> > Hmm, even better, inplement a SORT_NONE script keyword to extend our
>>>> > current list of sorting keywords, SORT_BY_NAME, SORT_BY_ALIGNMENT and
>>>> > SORT_BY_INIT_PRIORITY.  SORT_NONE does the obvious, overriding
>>>> > --sort-section=name and --sort-section=alignment.
>>>> >
>>>>
>>>> It won't work correctly with existing customer linker scripts.
>>>> I think we should just hardcode ,init/.fini section names.
>>>
>>> So who has linker scripts and uses --sort-section, has just discovered
>>> that it doesn't work with .init/.fini function pasting, and isn't
>>> willing or able to fix their scripts?
>>>
>>> In fact if we want to be sure that existing scripts continue to work
>>> as they always have, then we can't special case .init/.fini in
>>> update_wild_statements!  A new script keyword is by far the safest
>>> option, except in the rare case like glibc where a build system takes
>>> a default linker script as an input.
>>>
>>
>> Here is the patch to add SORT_NONE and not to sort .init/.fini
>> section.  I hardcode .init/.fini sections to support existing linker
>> scripts.   OK to install?
>>
>> Thanks.
>>
>> --
>> H.J.
>> ---
>> ld/
>>
>> 2012-06-28  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>         PR ld/14156
>>         * NEWS: Mention SORT_NONE.
>>
>>         * ld.h (sort_type): Add by_none.
>>
>>         * ld.texinfo: Document SORT_NONE.
>>
>>         * ldgram.y: Handle SORT_NONE.
>>
>>         * ldlang.c (update_wild_statements): Handle by_none.  Don't
>>         sort .init/.fini sections.
>>
>>         * ldlex.l: Add SORT_NONE.
>>
>> ld/testsuite/
>>
>> 2012-06-28  H.J. Lu  <hongjiu.lu@intel.com>
>>
>>         PR ld/14156
>>         * ld-elf/fini0.s: New file.
>>         * ld-elf/fini1.s: Likewise.
>>         * ld-elf/fini2.s: Likewise.
>>         * ld-elf/fini3.s: Likewise.
>>         * ld-elf/finin.s: Likewise.
>>         * ld-elf/foo0.s: Likewise.
>>         * ld-elf/foo1.s: Likewise.
>>         * ld-elf/foo2.s: Likewise.
>>         * ld-elf/foo3.s: Likewise.
>>         * ld-elf/foon.s: Likewise.
>>         * ld-elf/init0.s: Likewise.
>>         * ld-elf/init1.s: Likewise.
>>         * ld-elf/init2.s: Likewise.
>>         * ld-elf/init3.s: Likewise.
>>         * ld-elf/initn.s: Likewise.
>>         * ld-elf/pr14156a.d: Likewise.
>>         * ld-elf/pr14156b.d: Likewise.
>>         * ld-elf/pr14156c.d: Likewise.
>>         * ld-elf/pr14156c.t: Likewise.
>
> Ping.
>

Ping.

-- 
H.J.