PR26150, Assertion when asm() defines global symbols, -flto and --start-group

Message ID 20200623151755.GQ21885@bubble.grove.modra.org
State New
Headers show
Series
  • PR26150, Assertion when asm() defines global symbols, -flto and --start-group
Related show

Commit Message

Cui, Lili via Binutils June 23, 2020, 3:17 p.m.
If an archive map contains symbols that aren't actually defined by the
indexed element for any reason, then loading that element will leave
the symbol undefined (or common).  This leads to the possibility of
the element being loaded again should the archive be searched again
due to the action of --start-group/--end-group.  The testcase
triggering this problem was an archive containing fat lto objects,
with the archive map incorrectly created by ar rather than gcc-ar, but
there are of course other ways to break an armap.

This patch isn't exactly elegant, but it's close to impossible to do
anything earlier, before the lto plugin replaces the bfd.

	PR 26150
	* ldlang.c (ldlang_add_file): Assert that we aren't adding the
	current end of link.next list again too.
	* ldmain.c (add_archive_element): Don't load archive elements
	again that have already been loaded.


-- 
Alan Modra
Australia Development Lab, IBM

Patch

diff --git a/ld/ldlang.c b/ld/ldlang.c
index 8ab6a0b0c3..38ed83a274 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -7266,7 +7266,8 @@  ldlang_add_file (lang_input_statement_type *entry)
 
   /* The BFD linker needs to have a list of all input BFDs involved in
      a link.  */
-  ASSERT (entry->the_bfd->link.next == NULL);
+  ASSERT (link_info.input_bfds_tail != &entry->the_bfd->link.next
+	  && entry->the_bfd->link.next == NULL);
   ASSERT (entry->the_bfd != link_info.output_bfd);
 
   *link_info.input_bfds_tail = entry->the_bfd;
diff --git a/ld/ldmain.c b/ld/ldmain.c
index d34d30ae33..94a745ec31 100644
--- a/ld/ldmain.c
+++ b/ld/ldmain.c
@@ -822,10 +822,6 @@  add_archive_element (struct bfd_link_info *info,
   input->local_sym_name = bfd_get_filename (abfd);
   input->the_bfd = abfd;
 
-  parent = bfd_usrdata (abfd->my_archive);
-  if (parent != NULL && !parent->flags.reload)
-    parent->next = input;
-
   /* Save the original data for trace files/tries below, as plugins
      (if enabled) may possibly alter it to point to a replacement
      BFD, but we still want to output the original BFD filename.  */
@@ -853,6 +849,23 @@  add_archive_element (struct bfd_link_info *info,
     }
 #endif /* BFD_SUPPORTS_PLUGINS */
 
+  if (link_info.input_bfds_tail == &input->the_bfd->link.next
+      || input->the_bfd->link.next != NULL)
+    {
+      /* We have already loaded this element, and are attempting to
+	 load it again.  This can happen when the archive map doesn't
+	 match actual symbols defined by the element.  */
+      free (input);
+      bfd_set_error (bfd_error_malformed_archive);
+      return FALSE;
+    }
+
+  /* Set the file_chain pointer of archives to the last element loaded
+     from the archive.  See ldlang.c:find_rescan_insertion.  */
+  parent = bfd_usrdata (abfd->my_archive);
+  if (parent != NULL && !parent->flags.reload)
+    parent->next = input;
+
   ldlang_add_file (input);
 
   if (config.map_file != NULL)