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]

Re: [PATCH] Fix references to __ehdr_start when it cannot be defined


On Mon, Nov 11, 2013 at 03:06:54PM -0800, Roland McGrath wrote:
> I'm not really sure that's possible.  It's only knowable when deciding
> the file layout, which is after the address layout has been done.  If
[snip]

It's quite possible to define a symbol then come back later and fix
the value, but this is all by the way.

[snip]
> The problem (vs the original __ehdr_start implementation) seems to arise this:
> 
>       /* Since we're defining the symbol, don't let it seem to have not
> 	 been defined.  record_dynamic_symbol and size_dynamic_sections
> 	 may depend on this.  */
>       h->root.type = bfd_link_hash_new;
>       if (h->root.u.undef.next != NULL || htab->root.undefs_tail == &h->root)
> 	bfd_link_repair_undef_list (&htab->root);
> 
> in bfd_elf_record_link_assignment.
> 
> In the case of __ehdr_start, we're not defining the symbol and it should

Right, and bfd_elf_record_link_assignment was designed to handle
defining symbols..  I owe you and Maciej an apology, since it was my
suggestion to use bfd_elf_record_link_assignment here.  Let's go back
to Maciej's original submission, modified somewhat.

Ideally, we would have some function to prevent symbols from being
made dynamic, which is what elf32.em:before_allocation is trying to do
for "__ehdr_start".  See also elf64-ppc.c:ppc64_elf_func_desc_adjust
where the same thing is done for ".TOC." without knowing the proper
value.

Maciej, this gets FAIL: MIPS magic __ehdr_start symbol test 2 (o32)
because the linker script defines __ehdr_start and so the symbol
is not made hidden and forced_local.  Should we update the testcase,
or do you think __ehdr_start should always be hidden?  The latter can
be done by simply removing the h->root.type tests.

	* emultempl/elf32.em (before_allocation): Don't use
	bfd_link_record_link_assignment to hide __ehdr_start.

diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 682f5e5..5f776a0 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -1487,10 +1487,26 @@ gld${EMULATION_NAME}_before_allocation (void)
 
       /* Make __ehdr_start hidden if it has been referenced, to
 	 prevent the symbol from being dynamic.  */
-      if (!bfd_elf_record_link_assignment (link_info.output_bfd, &link_info,
-					   "__ehdr_start", TRUE, TRUE))
-	einfo ("%P%F: failed to record assignment to %s: %E\n",
-	       "__ehdr_start");
+      if (!link_info.relocatable)
+	{
+	  struct elf_link_hash_entry *h;
+
+	  h = elf_link_hash_lookup (elf_hash_table (&link_info), "__ehdr_start",
+				    FALSE, FALSE, TRUE);
+
+	  /* Only adjust the export class if the symbol was referenced
+	     and not defined, otherwise leave it alone.  */
+	  if (h != NULL
+	      && (h->root.type == bfd_link_hash_new
+		  || h->root.type == bfd_link_hash_undefined
+		  || h->root.type == bfd_link_hash_undefweak
+		  || h->root.type == bfd_link_hash_common))
+	    {
+	      _bfd_elf_link_hash_hide_symbol (&link_info, h, TRUE);
+	      if (ELF_ST_VISIBILITY (h->other) != STV_INTERNAL)
+		h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_HIDDEN;
+	    }
+	}
 
       /* If we are going to make any variable assignments, we need to
 	 let the ELF backend know about them in case the variables are


-- 
Alan Modra
Australia Development Lab, IBM


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