This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Introduce new .text.sorted.* sections.
- From: Martin Liška <mliska at suse dot cz>
- To: Cary Coutant <ccoutant at gmail dot com>
- Cc: Alan Modra <amodra at gmail dot com>, Binutils <binutils at sourceware dot org>, Ian Lance Taylor <iant at golang dot org>
- Date: Mon, 25 Nov 2019 13:09:38 +0100
- Subject: Re: [PATCH] Introduce new .text.sorted.* sections.
- References: <caa076c6-e356-f5e3-756d-eb1bbf498293@suse.cz> <20190925133427.GE19242@bubble.grove.modra.org> <501c6840-5c5f-2610-d0f9-aec269d07fda@suse.cz> <20190927074857.GC15102@bubble.grove.modra.org> <b19575b8-0524-6837-d081-1019e392b139@suse.cz> <e3625344-8771-fc66-6b4f-9cd1f94c211a@suse.cz> <CAJimCsHRYYagtDrkARsGGAOkk+sXWpDAJW=kmD0zff=at3Ui2A@mail.gmail.com>
On 11/22/19 8:39 PM, Cary Coutant wrote:
This looks fine to me, even though the gold implementation can produce
different results on input sections that match ".text.sorted.*". For
example, ".text.sorted.1" will be accepted by both ld.gold and ld.bfd,
but ld.bfd will sort it after ".text.sorted.02" while ld.gold would
sort it before.
Hi.
You are right, it's not precisely 1:1 among ld.bfd and ld.gold. I guess
we can live with that.
@@ -1132,12 +1132,15 @@ Layout::special_ordering_of_input_section(const
char* name)
".text.hot"
};
- for (size_t i = 0;
- i < sizeof(text_section_sort) / sizeof(text_section_sort[0]);
- i++)
+ unsigned scount = sizeof(text_section_sort) /
sizeof(text_section_sort[0]);
+ for (size_t i = 0; i < scount; i++)
if (is_prefix_of(text_section_sort[i], name))
return i;
+ int order;
+ if (sscanf (name, ".text.sorted.%d", &order) == 1)
+ return order + scount;
+
return -1;
}
Hello.
Thank you for the review.
Please declare scount as "const int" or "const size_t", and move the
declaration up to just under the declaration of text_section_sort.
Also, no space before "(" in C++ coding conventions.
Fixed.
Using sscanf this way will end up ignoring any non-digits that might follow
the index -- e.g., ".text.sorted.0001.foo". If that's the behavior you
want, fine (I guess), but if you want to sort based on whatever arbitrary
string follows ".text.sorted.", you'll need to add some extra logic
to Output_section::Input_section_sort_section_prefix_special_ordering_compare().
In that case, you can just add ".text.sorted" to the list in
text_section_sort.
I'm concerned about the different behavior between bfd ld and gold, as
sooner or later, someone's going to notice that they can use strings
instead of numbers to control the sort in bfd ld, and then complain that it
doesn't work in gold.
Ok, based on the review, I decided to come up with a special logic
in Input_section_sort_section_prefix_special_ordering_compare. The function
uses strcmp, so one can happily use any .text.sorted.FOO names.
Martin
-cary
>From 4e61b27dd90c8c0be590ac0bc649ed58f0bb7a75 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Wed, 18 Sep 2019 13:23:34 +0200
Subject: [PATCH] Introduce new .text.sorted.* sections.
gold/ChangeLog:
2019-09-24 Martin Liska <mliska@suse.cz>
* layout.cc (Layout::special_ordering_of_input_section):
Add ".text.sorted".
* output.cc: Special case ".text.sorted".
* testsuite/section_sorting_name.cc: Cover also .text.sorted
subsections.
* testsuite/section_sorting_name.sh: Likewise.
ld/ChangeLog:
2019-09-24 Martin Liska <mliska@suse.cz>
* scripttempl/arclinux.sc: Add .text.sorted.* which is sorted
by default.
* scripttempl/elf.sc: Likewise.
* scripttempl/elf64bpf.sc: Likewise.
* scripttempl/nds32elf.sc: Likewise.
* testsuite/ld-arm/arm-no-rel-plt.ld: Expect .text.sorted.*
in the default linker script.
* testsuite/ld-arm/fdpic-main.ld: Likewise.
* testsuite/ld-arm/fdpic-shared.ld: Likewise.
---
gold/layout.cc | 3 +-
gold/output.cc | 8 +++--
gold/testsuite/section_sorting_name.cc | 43 ++++++++++++++++++++++++++
gold/testsuite/section_sorting_name.sh | 6 ++++
ld/scripttempl/arclinux.sc | 1 +
ld/scripttempl/elf.sc | 1 +
ld/scripttempl/elf64bpf.sc | 1 +
ld/scripttempl/nds32elf.sc | 1 +
ld/testsuite/ld-arm/arm-no-rel-plt.ld | 1 +
ld/testsuite/ld-arm/fdpic-main.ld | 1 +
ld/testsuite/ld-arm/fdpic-shared.ld | 1 +
11 files changed, 64 insertions(+), 3 deletions(-)
diff --git a/gold/layout.cc b/gold/layout.cc
index 194d088c2a..4a6a7f69dd 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -1129,7 +1129,8 @@ Layout::special_ordering_of_input_section(const char* name)
".text.unlikely",
".text.exit",
".text.startup",
- ".text.hot"
+ ".text.hot",
+ ".text.sorted"
};
for (size_t i = 0;
diff --git a/gold/output.cc b/gold/output.cc
index df80587e66..48d2d2929e 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -3547,8 +3547,10 @@ Output_section::Input_section_sort_section_prefix_special_ordering_compare
const Output_section::Input_section_sort_entry& s2) const
{
// 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());
+ const char *s1_section_name = s1.section_name().c_str();
+ const char *s2_section_name = s2.section_name().c_str();
+ int o1 = Layout::special_ordering_of_input_section(s1_section_name);
+ int o2 = Layout::special_ordering_of_input_section(s2_section_name);
if (o1 != o2)
{
if (o1 < 0)
@@ -3558,6 +3560,8 @@ Output_section::Input_section_sort_section_prefix_special_ordering_compare
else
return o1 < o2;
}
+ else if (strstr(s1_section_name, ".text.sorted") == s1_section_name)
+ return strcmp(s1_section_name, s2_section_name) <= 0;
// Keep input order otherwise.
return s1.index() < s2.index();
diff --git a/gold/testsuite/section_sorting_name.cc b/gold/testsuite/section_sorting_name.cc
index 3a1d9baac3..8270b400aa 100644
--- a/gold/testsuite/section_sorting_name.cc
+++ b/gold/testsuite/section_sorting_name.cc
@@ -50,6 +50,49 @@ int hot_foo_0002()
return 1;
}
+extern "C"
+__attribute__ ((section(".text.sorted.0002")))
+int sorted_foo_0002()
+{
+ return 1;
+}
+
+extern "C"
+__attribute__ ((section(".text.sorted.0001.abc")))
+int sorted_foo_0001_abc()
+{
+ return 1;
+}
+
+
+extern "C"
+__attribute__ ((section(".text.sorted.0001")))
+int sorted_foo_0001()
+{
+ return 1;
+}
+
+extern "C"
+__attribute__ ((section(".text.sorted.0003")))
+int sorted_foo_0003()
+{
+ return 1;
+}
+
+extern "C"
+__attribute__ ((section(".text.sorted.z")))
+int sorted_foo_z()
+{
+ return 1;
+}
+
+extern "C"
+__attribute__ ((section(".text.sorted.y")))
+int sorted_foo_y()
+{
+ return 1;
+}
+
int vdata_0002 __attribute__((section(".data.0002"))) = 2;
int vbss_0002 __attribute__((section(".bss.0002"))) = 0;
diff --git a/gold/testsuite/section_sorting_name.sh b/gold/testsuite/section_sorting_name.sh
index ae44570f7f..00ad7cc20a 100755
--- a/gold/testsuite/section_sorting_name.sh
+++ b/gold/testsuite/section_sorting_name.sh
@@ -59,6 +59,12 @@ END {
check section_sorting_name.stdout "hot_foo_0001" "hot_foo_0002"
check section_sorting_name.stdout "hot_foo_0002" "hot_foo_0003"
+check section_sorting_name.stdout "sorted_foo_0001" "sorted_foo_0001_abc"
+check section_sorting_name.stdout "sorted_foo_0001_abc" "sorted_foo_0002"
+check section_sorting_name.stdout "sorted_foo_0002" "sorted_foo_0003"
+check section_sorting_name.stdout "sorted_foo_0003" "sorted_foo_y"
+check section_sorting_name.stdout "sorted_foo_y" "sorted_foo_z"
+
check section_sorting_name.stdout "vdata_0001" "vdata_0002"
check section_sorting_name.stdout "vdata_0002" "vdata_0003"
diff --git a/ld/scripttempl/arclinux.sc b/ld/scripttempl/arclinux.sc
index e13969ede0..41e8ccdf1b 100644
--- a/ld/scripttempl/arclinux.sc
+++ b/ld/scripttempl/arclinux.sc
@@ -491,6 +491,7 @@ cat <<EOF
${RELOCATING+*(.text.exit .text.exit.*)}
${RELOCATING+*(.text.startup .text.startup.*)}
${RELOCATING+*(.text.hot .text.hot.*)}
+ ${RELOCATING+*(SORT(.text.sorted.*))}
*(.text .stub${RELOCATING+ .text.* .gnu.linkonce.t.*})
/* .gnu.warning sections are handled specially by elf.em. */
*(.gnu.warning)
diff --git a/ld/scripttempl/elf.sc b/ld/scripttempl/elf.sc
index c3ad467bff..0d61881185 100644
--- a/ld/scripttempl/elf.sc
+++ b/ld/scripttempl/elf.sc
@@ -514,6 +514,7 @@ cat <<EOF
${RELOCATING+*(.text.exit .text.exit.*)}
${RELOCATING+*(.text.startup .text.startup.*)}
${RELOCATING+*(.text.hot .text.hot.*)}
+ ${RELOCATING+*(SORT(.text.sorted.*))}
*(.text .stub${RELOCATING+ .text.* .gnu.linkonce.t.*})
/* .gnu.warning sections are handled specially by elf.em. */
*(.gnu.warning)
diff --git a/ld/scripttempl/elf64bpf.sc b/ld/scripttempl/elf64bpf.sc
index de73775349..7937a41d2c 100644
--- a/ld/scripttempl/elf64bpf.sc
+++ b/ld/scripttempl/elf64bpf.sc
@@ -512,6 +512,7 @@ cat <<EOF
${RELOCATING+*(.text.exit .text.exit.*)}
${RELOCATING+*(.text.startup .text.startup.*)}
${RELOCATING+*(.text.hot .text.hot.*)}
+ ${RELOCATING+*(SORT(.text.sorted.*))}
*(.text .stub${RELOCATING+ .text.* .gnu.linkonce.t.*})
/* .gnu.warning sections are handled specially by elf.em. */
*(.gnu.warning)
diff --git a/ld/scripttempl/nds32elf.sc b/ld/scripttempl/nds32elf.sc
index 065c984f70..8d8d6e3f74 100644
--- a/ld/scripttempl/nds32elf.sc
+++ b/ld/scripttempl/nds32elf.sc
@@ -438,6 +438,7 @@ cat <<EOF
${RELOCATING+*(.text.exit .text.exit.*)}
${RELOCATING+*(.text.startup .text.startup.*)}
${RELOCATING+*(.text.hot .text.hot.*)}
+ ${RELOCATING+*(SORT(.text.sorted.*))}
*(.text .stub${RELOCATING+ .text.* .gnu.linkonce.t.*})
/* .gnu.warning sections are handled specially by elf.em. */
*(.gnu.warning)
diff --git a/ld/testsuite/ld-arm/arm-no-rel-plt.ld b/ld/testsuite/ld-arm/arm-no-rel-plt.ld
index d56e8203ad..14c1aeb439 100644
--- a/ld/testsuite/ld-arm/arm-no-rel-plt.ld
+++ b/ld/testsuite/ld-arm/arm-no-rel-plt.ld
@@ -65,6 +65,7 @@ SECTIONS
*(.text.exit .text.exit.*)
*(.text.startup .text.startup.*)
*(.text.hot .text.hot.*)
+ *(SORT(.text.sorted.*))
*(.text .stub .text.* .gnu.linkonce.t.*)
/* .gnu.warning sections are handled specially by elf.em. */
*(.gnu.warning)
diff --git a/ld/testsuite/ld-arm/fdpic-main.ld b/ld/testsuite/ld-arm/fdpic-main.ld
index d19a589d6c..b01b630fea 100644
--- a/ld/testsuite/ld-arm/fdpic-main.ld
+++ b/ld/testsuite/ld-arm/fdpic-main.ld
@@ -76,6 +76,7 @@ SECTIONS
*(.text.exit .text.exit.*)
*(.text.startup .text.startup.*)
*(.text.hot .text.hot.*)
+ *(SORT(.text.sorted.*))
*(.text .stub .text.* .gnu.linkonce.t.*)
/* .gnu.warning sections are handled specially by elf.em. */
*(.gnu.warning)
diff --git a/ld/testsuite/ld-arm/fdpic-shared.ld b/ld/testsuite/ld-arm/fdpic-shared.ld
index b1e262d829..b710ffa745 100644
--- a/ld/testsuite/ld-arm/fdpic-shared.ld
+++ b/ld/testsuite/ld-arm/fdpic-shared.ld
@@ -67,6 +67,7 @@ SECTIONS
*(.text.exit .text.exit.*)
*(.text.startup .text.startup.*)
*(.text.hot .text.hot.*)
+ *(SORT(.text.sorted.*))
*(.text .stub .text.* .gnu.linkonce.t.*)
/* .gnu.warning sections are handled specially by elf.em. */
*(.gnu.warning)
--
2.24.0