This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH,trunk+2.21.1] Fix link order problem with LD plugin API.
- From: Dave Korn <dave dot korn dot cygwin at gmail dot com>
- To: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Sat, 29 Jan 2011 01:48:55 +0000
- Subject: [PATCH,trunk+2.21.1] Fix link order problem with LD plugin API.
Hi list,
This patch updates the existing plugin API in LD to correct the effective
link order of the files added by the plugin. (It does not deal with the need
for pass-throughs; that is a separate and orthogonal issue that I will tackle
in a subsequent patch. I hope that the combination of these two fixes should
address all the same issues HJ's two-stage linking approach was designed to fix.)
At the moment added files and libraries are simply concatenated onto the end
of the statement list. This is more-or-less the same as adding them onto the
end of the command-line; this means that their contributions get merged into
sections after all the non-IR files, including in particular crtend.o. That's
pretty serious when it results in something like this:
> .eh_frame 0x00404000 0x200
> *(.eh_frame)
> .eh_frame 0x00404000 0x58 /gnu/gcc/obj-clean-r169274/gcc/testsuite/g++/../../crtbegin.o
> .eh_frame 0x00404058 0x5c /gnu/gcc/obj-clean-r169274/gcc/testsuite/g++/../../crtend.o
> .eh_frame 0x004040b4 0x68 /win/c/DOCUME~1/ADMINI~1/LOCALS~1/Temp/ccF5bqWv.lto.o
because it breaks EH!
The attached patch doesn't implement two-stage linking, but takes the
approach Cary described(*) in GOLD:
> On 03/12/2010 18:46, Cary Coutant wrote:
>
>> I should have remembered that I already dealt with this problem long
>> ago -- gold defers the layout of all real objects that follow the
>> first claimed IR file until after the replacement files have been laid
>> out. With respect to physical layout of the sections, this effectively
>> makes the link order equivalent to putting the replacement files in
>> the place of the first IR file. No "endcap" option is necessary.
In LD, there's nothing to "defer", as layout hasn't begun yet. All we need
to do is add the replacement files into the middle of the existing statement
list, immediately after the first claimed IR file. Complicated only slightly
by the fact that there are in fact three separate singly-linked chains through
the list of statements, that's what this patch does, and it results in the
correct result:
> .eh_frame 0x00404000 0x200
> *(.eh_frame)
> .eh_frame 0x00404000 0x58 /gnu/gcc/obj-clean-r169274/gcc/testsuite/g++/../../crtbegin.o
> .eh_frame 0x00404058 0x68 /win/c/DOCUME~1/ADMINI~1/LOCALS~1/Temp/ccfuGGNR.lto.o
> .eh_frame 0x004040c0 0x5c /gnu/gcc/obj-clean-r169274/gcc/testsuite/g++/../../crtend.o
ld/ChangeLog:
2011-01-29 Dave Korn <dave.korn.cygwin@gmail.com>
* ldlang.c (lang_process): After adding and opening new input files
passed from plugin, splice them into correct place in statement list
chains to preserve critical link order.
(lang_list_insert_after): New helper function.
(lang_list_remove_tail): Likewise.
Built and tested on i686-pc-cygwin. I'm running a subset of the G++/GCC
testsuites to sanity-check it as well, and I'll put it through a cycle or two
on one of the cfarm machines while I'm at it.
Assuming all that shows it working, OK to commit this to trunk, and after a
few days to settle in (and submit another plugin-related patch or two) to the
branch?
cheers,
DaveK
Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.360
diff -p -u -r1.360 ldlang.c
--- ld/ldlang.c 14 Jan 2011 02:18:22 -0000 1.360
+++ ld/ldlang.c 29 Jan 2011 00:42:09 -0000
@@ -85,6 +85,11 @@ static void print_statement_list (lang_s
static void print_statements (void);
static void print_input_section (asection *, bfd_boolean);
static bfd_boolean lang_one_common (struct bfd_link_hash_entry *, void *);
+static void lang_list_insert_after (lang_statement_list_type *destlist,
+ lang_statement_list_type *srclist,
+ lang_statement_union_type **field);
+static void lang_list_remove_tail (lang_statement_list_type *destlist,
+ lang_statement_list_type *origlist);
static void lang_record_phdrs (void);
static void lang_do_version_exports_section (void);
static void lang_finalize_version_expr_head
@@ -6420,21 +6425,54 @@ lang_process (void)
open_input_bfds (statement_list.head, FALSE);
#ifdef ENABLE_PLUGINS
+ if (plugin_active_plugins_p ())
{
- union lang_statement_union **listend;
+ lang_statement_list_type added;
+ lang_statement_list_type files, inputfiles;
/* Now all files are read, let the plugin(s) decide if there
are any more to be added to the link before we call the
- emulation's after_open hook. */
- listend = statement_list.tail;
- ASSERT (!*listend);
+ emulation's after_open hook. We create a private list of
+ input statements for this purpose, which we will eventually
+ insert into the global statment list after the first claimed
+ file. */
+ added = *stat_ptr;
+ /* We need to manipulate all three chains in synchrony. */
+ files = file_chain;
+ inputfiles = input_file_chain;
if (plugin_call_all_symbols_read ())
einfo (_("%P%F: %s: plugin reported error after all symbols read\n"),
plugin_error_plugin ());
- /* If any new files were added, they will be on the end of the
- statement list, and we can open them now by getting open_input_bfds
- to carry on from where it ended last time. */
- if (*listend)
- open_input_bfds (*listend, FALSE);
+ /* Open any newly added files, updating the file chains. */
+ open_input_bfds (added.head, FALSE);
+ /* Restore the global list pointer now they have all been added. */
+ lang_list_remove_tail (stat_ptr, &added);
+ /* And detach the fresh ends of the file lists. */
+ lang_list_remove_tail (&file_chain, &files);
+ lang_list_remove_tail (&input_file_chain, &inputfiles);
+ /* Were any new files added? */
+ if (added.head != NULL)
+ {
+ /* If so, we will insert them into the statement list immediately
+ after the first input file that was claimed by the plugin. */
+ lang_input_statement_type *claim1;
+ /* Locate first claimed input file. */
+ for (claim1 = (lang_input_statement_type *) input_file_chain.head;
+ claim1 != NULL;
+ claim1 = (lang_input_statement_type *) claim1->next_real_file)
+ if (claim1->claimed)
+ break;
+ /* If a plugin adds input files without having claimed any, we
+ don't really have a good idea where to place them. Just putting
+ them at the start or end of the list is liable to leave them
+ outside the crtbegin...crtend range. */
+ ASSERT (claim1 != NULL);
+ /* Splice the new statement list into the old one after claim1. */
+ lang_list_insert_after (stat_ptr, &added, &claim1->header.next);
+ /* Likewise for the file chains. */
+ lang_list_insert_after (&file_chain, &files, &claim1->next);
+ lang_list_insert_after (&input_file_chain, &inputfiles,
+ &claim1->next_real_file);
+ }
}
#endif /* ENABLE_PLUGINS */
@@ -6857,6 +6895,50 @@ lang_statement_append (lang_statement_li
list->tail = field;
}
+/* Insert SRCLIST into DESTLIST after given element by chaining
+ on FIELD as the next-pointer. (Counterintuitively does not need
+ a pointer to the actual after-node itself, just its chain field.) */
+
+static void
+lang_list_insert_after (lang_statement_list_type *destlist,
+ lang_statement_list_type *srclist,
+ lang_statement_union_type **field)
+{
+ /* Are we adding at the very end of the list? */
+ if (*field == NULL)
+ {
+ /* (*field == NULL) should be same as (destlist->tail == field),
+ if not then the element isn't really in the DESTLIST. */
+ ASSERT (destlist->tail == field);
+ /* Yes, append to and update tail pointer. */
+ *(destlist->tail) = srclist->head;
+ destlist->tail = srclist->tail;
+ }
+ else
+ {
+ /* We're inserting in the middle somewhere. */
+ *(srclist->tail) = *field;
+ *field = srclist->head;
+ }
+}
+
+/* Detach new nodes added to DESTLIST since the time ORIGLIST
+ was taken as a copy of it and leave them in ORIGLIST. */
+
+static void
+lang_list_remove_tail (lang_statement_list_type *destlist,
+ lang_statement_list_type *origlist)
+{
+ union lang_statement_union **savetail;
+ /* Check that ORIGLIST really is an earlier state of DESTLIST. */
+ ASSERT (origlist->head == destlist->head);
+ savetail = origlist->tail;
+ origlist->head = *(savetail);
+ origlist->tail = destlist->tail;
+ destlist->tail = savetail;
+ *savetail = NULL;
+}
+
/* Set the output format type. -oformat overrides scripts. */
void