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

Attachment: binutils-pr14156-1.patch
Description: Binary data