[gold] enable sorting of text sections with the same prefix
Sriraman Tallam
tmsriram@google.com
Tue Feb 12 19:12:00 GMT 2013
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
-------------- next part --------------
Index: layout.cc
===================================================================
RCS file: /cvs/src/src/gold/layout.cc,v
retrieving revision 1.245
diff -u -p -r1.245 layout.cc
--- layout.cc 24 Jan 2013 18:49:54 -0000 1.245
+++ layout.cc 12 Feb 2013 19:06:53 -0000
@@ -1654,6 +1654,10 @@ Layout::make_output_section(const char*
&& strcmp(name, ".text") == 0)
os->set_may_sort_attached_input_sections();
+ // GNU linker sorts section by name with --sort-section=name.
+ if (strcmp(parameters->options().sort_section(), "name") == 0)
+ os->set_must_sort_attached_input_sections();
+
// Check for .stab*str sections, as .stab* sections need to link to
// them.
if (type == elfcpp::SHT_STRTAB
Index: options.h
===================================================================
RCS file: /cvs/src/src/gold/options.h,v
retrieving revision 1.183
diff -u -p -r1.183 options.h
--- options.h 24 Jan 2013 18:49:54 -0000 1.183
+++ options.h 12 Feb 2013 19:06:53 -0000
@@ -1016,6 +1016,12 @@ class General_options
N_("Sort common symbols by alignment"),
N_("[={ascending,descending}]"));
+ DEFINE_enum(sort_section, options::TWO_DASHES, '\0', "none",
+ N_("Sort sections by name. \'--no-text-reorder\'"
+ " will override \'--sort-section=name\' for .text"),
+ N_("[none,name]"),
+ {"none", "name"});
+
DEFINE_uint(spare_dynamic_tags, options::TWO_DASHES, '\0', 5,
N_("Dynamic tag slots to reserve (default 5)"),
N_("COUNT"));
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.181
diff -u -p -r1.181 output.cc
--- output.cc 24 Jan 2013 18:49:54 -0000 1.181
+++ output.cc 12 Feb 2013 19:06:53 -0000
@@ -3498,8 +3498,9 @@ Output_section::Input_section_sort_secti
// Return true if S1 should come before S2. This is the sort comparison
// function for .text to sort sections with prefixes
// .text.{unlikely,exit,startup,hot} before other sections.
+
bool
-Output_section::Input_section_sort_section_name_special_ordering_compare
+Output_section::Input_section_sort_section_prefix_special_ordering_compare
::operator()(
const Output_section::Input_section_sort_entry& s1,
const Output_section::Input_section_sort_entry& s2) const
@@ -3513,7 +3514,7 @@ Output_section::Input_section_sort_secti
return false;
return s1.index() < s2.index();
}
-
+
// Some input section names have special ordering requirements.
int o1 = Layout::special_ordering_of_input_section(s1.section_name().c_str());
int o2 = Layout::special_ordering_of_input_section(s2.section_name().c_str());
@@ -3528,7 +3529,35 @@ Output_section::Input_section_sort_secti
}
// Keep input order otherwise.
- return s1.index() < s2.index();
+ return s1.index() < s2.index();
+}
+
+// Return true if S1 should come before S2. This is the sort comparison
+// function for sections to sort them by name.
+
+bool
+Output_section::Input_section_sort_section_name_compare
+ ::operator()(
+ const Output_section::Input_section_sort_entry& s1,
+ const Output_section::Input_section_sort_entry& s2) const
+{
+ // We sort all the sections with no names to the end.
+ if (!s1.section_has_name() || !s2.section_has_name())
+ {
+ if (s1.section_has_name())
+ return true;
+ if (s2.section_has_name())
+ return false;
+ return s1.index() < s2.index();
+ }
+
+ // We sort by name.
+ int compare = s1.section_name().compare(s2.section_name());
+ if (compare != 0)
+ return compare < 0;
+
+ // Keep input order otherwise.
+ return s1.index() < s2.index();
}
// This updates the section order index of input sections according to the
@@ -3599,9 +3628,12 @@ Output_section::sort_attached_input_sect
|| this->type() == elfcpp::SHT_FINI_ARRAY)
std::sort(sort_list.begin(), sort_list.end(),
Input_section_sort_init_fini_compare());
+ else if (strcmp(parameters->options().sort_section(), "name") == 0)
+ std::sort(sort_list.begin(), sort_list.end(),
+ Input_section_sort_section_name_compare());
else if (strcmp(this->name(), ".text") == 0)
- std::sort(sort_list.begin(), sort_list.end(),
- Input_section_sort_section_name_special_ordering_compare());
+ std::sort(sort_list.begin(), sort_list.end(),
+ Input_section_sort_section_prefix_special_ordering_compare());
else
std::sort(sort_list.begin(), sort_list.end(),
Input_section_sort_compare());
Index: output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.146
diff -u -p -r1.146 output.h
--- output.h 24 Jan 2013 18:49:54 -0000 1.146
+++ output.h 12 Feb 2013 19:06:53 -0000
@@ -4202,7 +4202,15 @@ class Output_section : public Output_dat
// This is the sort comparison function for .text to sort sections with
// prefixes .text.{unlikely,exit,startup,hot} before other sections.
- struct Input_section_sort_section_name_special_ordering_compare
+ struct Input_section_sort_section_prefix_special_ordering_compare
+ {
+ bool
+ operator()(const Input_section_sort_entry&,
+ const Input_section_sort_entry&) const;
+ };
+
+ // This is the sort comparison function for sorting sections by name.
+ struct Input_section_sort_section_name_compare
{
bool
operator()(const Input_section_sort_entry&,
cvs diff: Diffing po
cvs diff: Diffing testsuite
Index: testsuite/Makefile.am
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
retrieving revision 1.207
diff -u -p -r1.207 Makefile.am
--- testsuite/Makefile.am 24 Jan 2013 18:49:54 -0000 1.207
+++ testsuite/Makefile.am 12 Feb 2013 19:06:53 -0000
@@ -272,6 +272,16 @@ text_section_grouping.stdout: text_secti
text_section_no_grouping.stdout: text_section_no_grouping
$(TEST_NM) -n --synthetic text_section_no_grouping > text_section_no_grouping.stdout
+check_SCRIPTS += section_sorting_name.sh
+check_DATA += section_sorting_name.stdout
+MOSTLYCLEANFILES += section_sorting_name
+section_sorting_name.o: section_sorting_name.cc
+ $(CXXCOMPILE) -O0 -c -ffunction-sections -g -o $@ $<
+section_sorting_name: section_sorting_name.o gcctestdir/ld
+ $(CXXLINK) -Bgcctestdir/ -Wl,--sort-section=name section_sorting_name.o
+section_sorting_name.stdout: section_sorting_name
+ $(TEST_NM) -n --synthetic section_sorting_name > section_sorting_name.stdout
+
check_PROGRAMS += icf_virtual_function_folding_test
MOSTLYCLEANFILES += icf_virtual_function_folding_test icf_virtual_function_folding_test.map
icf_virtual_function_folding_test.o: icf_virtual_function_folding_test.cc
Index: testsuite/Makefile.in
===================================================================
RCS file: /cvs/src/src/gold/testsuite/Makefile.in,v
retrieving revision 1.219
diff -u -p -r1.219 Makefile.in
--- testsuite/Makefile.in 24 Jan 2013 18:49:55 -0000 1.219
+++ testsuite/Makefile.in 12 Feb 2013 19:06:53 -0000
@@ -82,6 +82,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__E
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_safe_so_test.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ final_layout.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ text_section_grouping.sh \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ section_sorting_name.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_preemptible_functions_test.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_string_merge_test.sh \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_sht_rel_addend_test.sh \
@@ -114,6 +115,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__E
@GCC_TRUE@@NATIVE_LINKER_TRUE@ final_layout.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ text_section_grouping.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ text_section_no_grouping.stdout \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ section_sorting_name.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_preemptible_functions_test.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_string_merge_test.stdout \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_sht_rel_addend_test.stdout \
@@ -130,6 +132,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__E
@GCC_TRUE@@NATIVE_LINKER_TRUE@ final_layout_script.lds \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ text_section_grouping \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ text_section_no_grouping \
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ section_sorting_name \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_virtual_function_folding_test \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_virtual_function_folding_test.map \
@GCC_TRUE@@NATIVE_LINKER_TRUE@ icf_preemptible_functions_test \
@@ -3725,6 +3728,8 @@ final_layout.sh.log: final_layout.sh
@p='final_layout.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
text_section_grouping.sh.log: text_section_grouping.sh
@p='text_section_grouping.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
+section_sorting_name.sh.log: section_sorting_name.sh
+ @p='section_sorting_name.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
icf_preemptible_functions_test.sh.log: icf_preemptible_functions_test.sh
@p='icf_preemptible_functions_test.sh'; $(am__check_pre) $(LOG_COMPILE) "$$tst" $(am__check_post)
icf_string_merge_test.sh.log: icf_string_merge_test.sh
@@ -4384,6 +4389,12 @@ uninstall-am:
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_NM) -n --synthetic text_section_grouping > text_section_grouping.stdout
@GCC_TRUE@@NATIVE_LINKER_TRUE@text_section_no_grouping.stdout: text_section_no_grouping
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_NM) -n --synthetic text_section_no_grouping > text_section_no_grouping.stdout
+@GCC_TRUE@@NATIVE_LINKER_TRUE@section_sorting_name.o: section_sorting_name.cc
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -c -ffunction-sections -g -o $@ $<
+@GCC_TRUE@@NATIVE_LINKER_TRUE@section_sorting_name: section_sorting_name.o gcctestdir/ld
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -Bgcctestdir/ -Wl,--sort-section=name section_sorting_name.o
+@GCC_TRUE@@NATIVE_LINKER_TRUE@section_sorting_name.stdout: section_sorting_name
+@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(TEST_NM) -n --synthetic section_sorting_name > section_sorting_name.stdout
@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_virtual_function_folding_test.o: icf_virtual_function_folding_test.cc
@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXCOMPILE) -O0 -c -ffunction-sections -fPIE -g -o $@ $<
@GCC_TRUE@@NATIVE_LINKER_TRUE@icf_virtual_function_folding_test: icf_virtual_function_folding_test.o gcctestdir/ld
More information about the Binutils
mailing list