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, 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.

-- 
H.J.