[4/8] section-select: Completely rebuild matches
Checks
Commit Message
this resets all section matches before updating for newly created
sections (i.e. completely rebuilds the matches). This fixes powerpc
"TOC opt" tests that shows a difference in section order: originally
.got of "linker stubs" comes before .toc (both placed into the same
.got output section due to ".got {*(.got .toc)}". But .got of linker
stubs is created later, and in the second run of resolve_wilds is
appended to the list, hence is then coming after .toc (which was added
already in the earlier resolve_wilds run). So order swapped ->
test fails.
The result would still work, and it's unclear if the documented
meaning of multiple section selectors applies to lazily generated
sections like here as well. For now lets reinstate old behaviour and
simply always completely rebuild the matching sections.
(Note: the reset lists aren't freed or reused)
---
Alan: can you please take a look at the problem mentioned above? Without
this patch the "TOC opt" tests fails on powerpc because two sections are
swapped. But it's not quite clear if lazily added sections (.got of
"linker stubs") are also bound to the documented behaviour of multiple
globs in a single wild statement.
The result with the changed section order would continue to work, and if
we could decide that that's okay the section resolution wouldn't have to
rebuild stuff from scratch, roughly halving the time for it.
In that case I wouldn't patch 7/8 to remove the libbfd interface to get
the max section id and instead use it for early outs. Sections that are
generated late and lazy would then always be appended to their matching
wild statement.
Ciao,
Michael.
ld/ldlang.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
Comments
On Fri, Nov 25, 2022 at 04:55:12PM +0000, Michael Matz via Binutils wrote:
> this resets all section matches before updating for newly created
> sections (i.e. completely rebuilds the matches). This fixes powerpc
> "TOC opt" tests that shows a difference in section order: originally
> .got of "linker stubs" comes before .toc (both placed into the same
> .got output section due to ".got {*(.got .toc)}". But .got of linker
> stubs is created later, and in the second run of resolve_wilds is
> appended to the list, hence is then coming after .toc (which was added
> already in the earlier resolve_wilds run). So order swapped ->
> test fails.
That's a problem. The got header is created in the .got of "linker
stubs", and code setting the value of .TOC. assumes that the header
will be located first in the .got output section. This ties in with
where ld.so expects to find the header too.
Hello,
On Mon, 28 Nov 2022, Alan Modra wrote:
> On Fri, Nov 25, 2022 at 04:55:12PM +0000, Michael Matz via Binutils wrote:
> > this resets all section matches before updating for newly created
> > sections (i.e. completely rebuilds the matches). This fixes powerpc
> > "TOC opt" tests that shows a difference in section order: originally
> > .got of "linker stubs" comes before .toc (both placed into the same
> > .got output section due to ".got {*(.got .toc)}". But .got of linker
> > stubs is created later, and in the second run of resolve_wilds is
> > appended to the list, hence is then coming after .toc (which was added
> > already in the earlier resolve_wilds run). So order swapped ->
> > test fails.
>
> That's a problem. The got header is created in the .got of "linker
> stubs", and code setting the value of .TOC. assumes that the header
> will be located first in the .got output section. This ties in with
> where ld.so expects to find the header too.
I see. But then why is the testcase (and linker script?) not using
.got { *(.got) *(.toc) }
? The way it's written right now means "for each file, first its .got
then its .toc, intermixed", i.e. file1.got, file1.toc, file2.got,
file2.toc ... And it seems a bit fuzzy to speak about "files" for the
artificial generated pseudo files like "linker stubs".
(Just to be clear: rebuilding all matches also solves this problem)
Ciao,
Michael.
On Mon, Nov 28, 2022 at 02:24:30PM +0000, Michael Matz wrote:
> Hello,
>
> On Mon, 28 Nov 2022, Alan Modra wrote:
>
> > On Fri, Nov 25, 2022 at 04:55:12PM +0000, Michael Matz via Binutils wrote:
> > > this resets all section matches before updating for newly created
> > > sections (i.e. completely rebuilds the matches). This fixes powerpc
> > > "TOC opt" tests that shows a difference in section order: originally
> > > .got of "linker stubs" comes before .toc (both placed into the same
> > > .got output section due to ".got {*(.got .toc)}". But .got of linker
> > > stubs is created later, and in the second run of resolve_wilds is
> > > appended to the list, hence is then coming after .toc (which was added
> > > already in the earlier resolve_wilds run). So order swapped ->
> > > test fails.
> >
> > That's a problem. The got header is created in the .got of "linker
> > stubs", and code setting the value of .TOC. assumes that the header
> > will be located first in the .got output section. This ties in with
> > where ld.so expects to find the header too.
>
> I see. But then why is the testcase (and linker script?) not using
>
> .got { *(.got) *(.toc) }
>
> ? The way it's written right now means "for each file, first its .got
> then its .toc, intermixed", i.e. file1.got, file1.toc, file2.got,
> file2.toc ...
Yes. That's the way we want it. When linking small model code with a
total GOT/TOC of over 64k, the linker splits the TOC into multiple
pieces with r2 adjusting stubs inserted on calls between files that
use different pieces of the TOC. That scheme wouldn't work if a
file's .got entries were placed in a different piece of the TOC to the
file's .toc entries.
Hey,
On Tue, 29 Nov 2022, Alan Modra wrote:
> > > That's a problem. The got header is created in the .got of "linker
> > > stubs", and code setting the value of .TOC. assumes that the header
> > > will be located first in the .got output section. This ties in with
> > > where ld.so expects to find the header too.
> >
> > I see. But then why is the testcase (and linker script?) not using
> >
> > .got { *(.got) *(.toc) }
> >
> > ? The way it's written right now means "for each file, first its .got
> > then its .toc, intermixed", i.e. file1.got, file1.toc, file2.got,
> > file2.toc ...
>
> Yes. That's the way we want it. When linking small model code with a
> total GOT/TOC of over 64k, the linker splits the TOC into multiple
> pieces with r2 adjusting stubs inserted on calls between files that
> use different pieces of the TOC. That scheme wouldn't work if a
> file's .got entries were placed in a different piece of the TOC to the
> file's .toc entries.
Ah, nifty. Something like that occurred to me yesterday as well, and
either way, rewriting the linker script like above wouldn't have helped
this problem anyway, as long as "linker stubs".got would have been created
late it would always have been placed at the end of the list.
Although one could pedantically argue that those late-created sections
being placed at the end would indeed conform to "as they are found in the
linker input", I won't belabor that point. Complete rebuilding it is,
performance-wise it's a wash :)
Ciao,
Michael.
@@ -8400,6 +8400,26 @@ lang_propagate_lma_regions (void)
}
}
+static void
+reset_one_wild (lang_statement_union_type *statement)
+{
+ if (statement->header.type == lang_wild_statement_enum)
+ {
+ lang_wild_statement_type *stmt = &statement->wild_statement;
+ stmt->resolved = false;
+ stmt->max_section_id = 0;
+ /* XXX Leaks? */
+ lang_list_init (&stmt->matching_sections);
+ }
+}
+
+static void
+reset_resolved_wilds (void)
+{
+ lang_for_each_statement (reset_one_wild);
+ old_max_section_id = 0;
+}
+
void
lang_process (void)
{
@@ -8618,6 +8638,7 @@ lang_process (void)
ldemul_after_check_relocs ();
+ reset_resolved_wilds ();
resolve_wilds ();
/* Update wild statements in case the user gave --sort-section.