It seems that although the documentation states that --sort-common will sort in
roughly ascending order by size, the implementation does so in descending order.
Created attachment 2707 [details]
Patch for correct --sort-common.
Either the documentation or the implementation is incorrect. In the latter
case, this patch fixes the implementation.
Can you add a testcase patch?
Sure, but I'll be hitting the road this week. Later on then.
Linker manual says
This option tells `ld' to sort the common symbols by size when it
places them in the appropriate output sections. First come all
the one byte symbols, then all the two byte, then all the four
byte, and then everything else. This is to prevent gaps between
symbols due to alignment constraints.
To prevent gaps between common symbols with different alignments,
shouldn't we sort common symbols by their sizes in descending order,
assuming small sizes need small alignments?
You are correct in that this way a gap would be avoided when the size changes.
But given that all like-sized symbols are laid out back to back, this only
happens when the size changes.
So, either the manual must be changed or the code. I, for one, would rather
have the code changed...
Subject: Re: --sort-common Not Implemented Per Documentation
> You are correct in that this way a gap would be avoided when the size changes.
> But given that all like-sized symbols are laid out back to back, this only
> happens when the size changes.
> So, either the manual must be changed or the code. I, for one, would rather
> have the code changed...
But this might break existing applications which rely upon the current
behaviour. Better I feel would be to fix the documentation and then
provide a extended version of the --sort-common switch which takes an
argument which specifies the direction of the sorting. eg:
--sort-common=biggest-first [this would be the default]
To prevent gaps between symbols due to alignment constraints, shouldn't
we sort common symbols by their alignments?
As a matter of fact, the current implementation actually sorts by alignment.
I like Nick's suggestion, only I prefer terser wording, such as "ascending" or
"descending". But that's me.
(In reply to comment #8)
> As a matter of fact, the current implementation actually sorts by alignment.
> I like Nick's suggestion, only I prefer terser wording, such as "ascending" or
> "descending". But that's me.
In that case, we need to fix the typo in the linker manual first: `--sort-common'
should be sorted by alignment, not by size.
Created attachment 2720 [details]
Change the documentation according to the code.
Right - I have checked in your documentation patch. (Sorry about the delay in
getting to this).
Do you have a patch to implement the new user-choice-of-sorting algorithm, or
is there really no need for this now that the documentation is correct ?
I think that the choice should be available. I'll craft it up in a moment.
Created attachment 2750 [details]
Patch providing the user with a choice.
Please, review this patch that allows the user to choose between ascending or
descending order, assuming descending order as currently if nothing is
Created attachment 2769 [details]
Add test of --sort-common command line option
Thanks for the patch. I have applied it together with the changelog entry
below. Plus I added a line about it to the ld/NEWS file, and I created a linker
testsuite test for it so that we can be sure that the option continues to work.
Interestingly the new test fails for the cr16-elf and crx-elf ports, but I
could not determine why this was happening.
2008-05-31 Evandro Menezes <firstname.lastname@example.org>
* ld.h (enum sort_order): New.
* ldlang.c (lang_check: Fix comment.
(lang_common): Sort commons in ascending or descending order.
* lexsup.c (ld_options): Have --sort-common take an option
(parse_args): Handle argument to --sort-common.
* ld.texinfo (--sort-common): Document new optional argument.
* NEWS: Mention new feature.
2008-05-31 Nick Clifton <email@example.com>
* ld-elfcomm/elfcomm.exp (test_sort_common): Test the
ascending/descending argument to the --sort-common command line
* ld-elfcomm/sort-common.s: New file.