This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]