[PATCH] ld: Add ehdr_start to bfd_link_hash_entry

[PATCH] ld: Add ehdr_start to bfd_link_hash_entry

[H.J. Lu]

__ehdr_start is a special symbol which is undefined during link.  It
becomes defined almost at the last minute if it is referenced.  Add
ehdr_start to bfd_link_hash_entry for __ehdr_start so that ELF linker
can properly handle it.

include/

	* bfdlink.h (bfd_link_hash_entry): Add ehdr_start.

ld/

	* ldelf.c (ldelf_before_allocation): Set ehdr_start for
	__ehdr_start.
---
 include/bfdlink.h | 3 +++
 ld/ldelf.c        | 1 +
 2 files changed, 4 insertions(+)

diff --git a/include/bfdlink.h b/include/bfdlink.h
index f3a52b04026..073baa64fb1 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -121,6 +121,9 @@ struct bfd_link_hash_entry
   /* Symbol defined in a linker script.  */
   unsigned int ldscript_def : 1;
 
+  /* This the special ELF symbol: __ehdr_start.  */
+  unsigned int ehdr_start : 1;
+
   /* Symbol will be converted from absolute to section-relative.  Set for
      symbols defined by a script from "dot" (also SEGMENT_START or ORIGIN)
      outside of an output section statement.  */
diff --git a/ld/ldelf.c b/ld/ldelf.c
index 202c43a58ef..dc317be290a 100644
--- a/ld/ldelf.c
+++ b/ld/ldelf.c
@@ -1617,6 +1617,7 @@ ldelf_before_allocation (char *audit, char *depaudit,
 	      ehdr_start->type = bfd_link_hash_defined;
 	      /* It will be converted to section-relative later.  */
 	      ehdr_start->rel_from_abs = 1;
+	      ehdr_start->ehdr_start = 1;
 	      ehdr_start->u.def.section = bfd_abs_section_ptr;
 	      ehdr_start->u.def.value = 0;
 	    }
-- 
2.33.1

Re: [PATCH] ld: Add ehdr_start to bfd_link_hash_entry

[Alan Modra]

On Sat, Jan 01, 2022 at 07:37:19PM -0800, H.J. Lu via Binutils wrote:
> __ehdr_start is a special symbol which is undefined during link.  It
> becomes defined almost at the last minute if it is referenced.  Add
> ehdr_start to bfd_link_hash_entry for __ehdr_start so that ELF linker
> can properly handle it.

We make some effort in ldelf_before_allocation to handle __ehdr_start
for bfd_elf_size_dynamic_sections by defining it early.  Where else do
we need special handling?

Hmm, I'm guessing for your DT_RELR support.  

I wonder if we can't revise the existing __ehdr_start hacks.  I think
I'd prefer that rather than adding another special case.  Let's see
if we can make this patch unnecessary.  Something along the lines of
the following, which currently hits an error on x86_64.

failed with: <./ld-new: tmpdir/ehdr_start.o(.rodata+0): reloc against `__ehdr_start': error 6>, no expected output
FAIL: ld-elf/ehdr_start-shared


diff --git a/bfd/elflink.c b/bfd/elflink.c
index 24acb37925c..49652724e4c 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -510,6 +510,10 @@ bfd_elf_link_record_dynamic_symbol (struct bfd_link_info *info,
       const char *name;
       size_t indx;
 
+      /* Linker defined symbols are not dynamic.  */
+      if (h->root.linker_def)
+	return true;
+
       if (h->root.type == bfd_link_hash_defined
 	  || h->root.type == bfd_link_hash_defweak)
 	{
diff --git a/ld/ldelf.c b/ld/ldelf.c
index 00caa6bef47..c31787fb30a 100644
--- a/ld/ldelf.c
+++ b/ld/ldelf.c
@@ -1563,52 +1563,19 @@ ldelf_before_allocation (char *audit, char *depaudit,
   const char *rpath;
   asection *sinterp;
   bfd *abfd;
-  struct bfd_link_hash_entry *ehdr_start = NULL;
-  unsigned char ehdr_start_save_type = 0;
-  char ehdr_start_save_u[sizeof ehdr_start->u
-			 - sizeof ehdr_start->u.def.next] = "";
 
   if (is_elf_hash_table (link_info.hash))
     {
       _bfd_elf_tls_setup (link_info.output_bfd, &link_info);
 
-      /* Make __ehdr_start hidden if it has been referenced, to
-	 prevent the symbol from being dynamic.  */
       if (!bfd_link_relocatable (&link_info))
 	{
 	  struct elf_link_hash_table *htab = elf_hash_table (&link_info);
 	  struct elf_link_hash_entry *h
 	    = elf_link_hash_lookup (htab, "__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))
-	    {
-	      const struct elf_backend_data *bed;
-	      bed = get_elf_backend_data (link_info.output_bfd);
-	      (*bed->elf_backend_hide_symbol) (&link_info, h, true);
-	      if (ELF_ST_VISIBILITY (h->other) != STV_INTERNAL)
-		h->other = (h->other & ~ELF_ST_VISIBILITY (-1)) | STV_HIDDEN;
-	      /* Don't leave the symbol undefined.  Undefined hidden
-		 symbols typically won't have dynamic relocations, but
-		 we most likely will need dynamic relocations for
-		 __ehdr_start if we are building a PIE or shared
-		 library.  */
-	      ehdr_start = &h->root;
-	      ehdr_start_save_type = ehdr_start->type;
-	      memcpy (ehdr_start_save_u,
-		      (char *) &ehdr_start->u + sizeof ehdr_start->u.def.next,
-		      sizeof ehdr_start_save_u);
-	      ehdr_start->type = bfd_link_hash_defined;
-	      /* It will be converted to section-relative later.  */
-	      ehdr_start->rel_from_abs = 1;
-	      ehdr_start->u.def.section = bfd_abs_section_ptr;
-	      ehdr_start->u.def.value = 0;
-	    }
+	  if (h != NULL)
+	    h->root.linker_def = 1;
 	}
 
       /* If we are going to make any variable assignments, we need to
@@ -1724,16 +1691,6 @@ ldelf_before_allocation (char *audit, char *depaudit,
 
   if (!bfd_elf_size_dynsym_hash_dynstr (link_info.output_bfd, &link_info))
     einfo (_("%F%P: failed to set dynamic section sizes: %E\n"));
-
-  if (ehdr_start != NULL)
-    {
-      /* If we twiddled __ehdr_start to defined earlier, put it back
-	 as it was.  */
-      ehdr_start->type = ehdr_start_save_type;
-      memcpy ((char *) &ehdr_start->u + sizeof ehdr_start->u.def.next,
-	      ehdr_start_save_u,
-	      sizeof ehdr_start_save_u);
-    }
 }
 /* Try to open a dynamic archive.  This is where we know that ELF
    dynamic libraries have an extension of .so (or .sl on oddball systems


-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH] ld: Add ehdr_start to bfd_link_hash_entry

[H.J. Lu]

On Tue, Jan 4, 2022 at 5:57 PM Alan Modra <amodra@gmail.com> wrote:
>
> On Sat, Jan 01, 2022 at 07:37:19PM -0800, H.J. Lu via Binutils wrote:
> > __ehdr_start is a special symbol which is undefined during link.  It
> > becomes defined almost at the last minute if it is referenced.  Add
> > ehdr_start to bfd_link_hash_entry for __ehdr_start so that ELF linker
> > can properly handle it.
>
> We make some effort in ldelf_before_allocation to handle __ehdr_start
> for bfd_elf_size_dynamic_sections by defining it early.  Where else do
> we need special handling?
>
> Hmm, I'm guessing for your DT_RELR support.

Correct.  I get DT_RELR almost working on x86.  When computing
relocation address for DT_RELR, I have

         h = relative_reloc->data[i].h;
          if (h != NULL)
            {
              if (h->root.type == bfd_link_hash_defined
                  || h->root.type == bfd_link_hash_defweak)
                {
                  sym_sec = h->root.u.def.section;
                  relocation = (h->root.u.def.value
                                + sym_sec->output_section->vma
                                + sym_sec->output_offset);
                }
              else if (bfd_link_pic (info) && h->root.ehdr_start)
                {
                  /* Assume that __ehdr_start in PIE and shared library
                     is 0. */
                  relocation = 0;
                  info->zero_ehdr_start = 1;
                }
              else
                abort ();

At the sizing phase, I got

Breakpoint 1, elf_x86_size_or_finish_relative_reloc (is_x86_64=true,
align_mask=1, info=0x6bfa20 <link_info>, htab=0x6dbd30,
unaligned=false, outrel=0x0) at
/export/gnu/import/git/gitlab/x86-binutils/bfd/elfxx-x86.c:1435
1435       if (h->root.type == bfd_link_hash_defined
(gdb) p *h
$3 = {root = {root = {next = 0x0, string = 0x6f29e6 "__ehdr_start",
      hash = 78192523}, type = bfd_link_hash_undefined,
    non_ir_ref_regular = 0, non_ir_ref_dynamic = 0, linker_def = 0,
    ldscript_def = 0, ehdr_start = 1, rel_from_abs = 1, u = {undef = {
        next = 0x0, abfd = 0x6eb4e0}, def = {next = 0x0, section = 0x6eb4e0,
        value = 0}, i = {next = 0x0, link = 0x6eb4e0, warning = 0x0}, c = {
        next = 0x0, p = 0x6eb4e0, size = 0}}}, indx = -1, dynindx = -1, got = {
    refcount = -1, offset = 18446744073709551615, glist = 0xffffffffffffffff,
    plist = 0xffffffffffffffff}, plt = {refcount = -1,
    offset = 18446744073709551615, glist = 0xffffffffffffffff,
    plist = 0xffffffffffffffff}, size = 0, dyn_relocs = 0x6f4180, type = 0,
  other = 2, target_internal = 0, ref_regular = 1, def_regular = 1,
  ref_dynamic = 0, def_dynamic = 0, ref_regular_nonweak = 1,
  ref_ir_nonweak = 0, dynamic_adjusted = 0, needs_copy = 0, needs_plt = 0,
  non_elf = 0, versioned = unversioned, forced_local = 1, dynamic = 0,
  mark = 0, non_got_ref = 0, dynamic_def = 0, ref_dynamic_nonweak = 0,
  pointer_equality_needed = 1, unique_global = 0, protected_def = 0,
  start_stop = 0, is_weakalias = 0, dynstr_index = 0, u = {alias = 0x0,
    elf_hash_value = 0}, verinfo = {verdef = 0x0, vertree = 0x0}, u2 = {
    start_stop_section = 0x0, vtable = 0x0}}
(gdb)

I can use 0 as a relocation value since it isn't final. But at the
finishing phase, I now got

Breakpoint 1, elf_x86_size_or_finish_relative_reloc (is_x86_64=true,
align_mask=0, info=0x6bfa20 <link_info>, htab=0x6dbd30,
unaligned=true, outrel=0x7fffffffd6c0) at
/export/gnu/import/git/gitlab/x86-binutils/bfd/elfxx-x86.c:1435
1435       if (h->root.type == bfd_link_hash_defined
(gdb) p *h
$4 = {root = {root = {next = 0x0, string = 0x6f29e6 "__ehdr_start",
      hash = 78192523}, type = bfd_link_hash_defined, non_ir_ref_regular = 0,
    non_ir_ref_dynamic = 0, linker_def = 0, ldscript_def = 0, ehdr_start = 1,
    rel_from_abs = 1, u = {undef = {next = 0x0, abfd = 0x6dadd0}, def = {
        next = 0x0, section = 0x6dadd0, value = 18446744073709551104}, i = {
        next = 0x0, link = 0x6dadd0,
        warning = 0xfffffffffffffe00 <error: Cannot access memory at
address 0xfffffffffffffe00>}, c = {next = 0x0, p = 0x6dadd0,
        size = 18446744073709551104}}}, indx = -1, dynindx = -1, got = {
    refcount = -1, offset = 18446744073709551615, glist = 0xffffffffffffffff,
    plist = 0xffffffffffffffff}, plt = {refcount = -1,
    offset = 18446744073709551615, glist = 0xffffffffffffffff,
    plist = 0xffffffffffffffff}, size = 0, dyn_relocs = 0x6f4180, type = 0,
  other = 2, target_internal = 0, ref_regular = 1, def_regular = 1,
  ref_dynamic = 0, def_dynamic = 0, ref_regular_nonweak = 1,
  ref_ir_nonweak = 0, dynamic_adjusted = 0, needs_copy = 0, needs_plt = 0,
  non_elf = 0, versioned = unversioned, forced_local = 1, dynamic = 0,
  mark = 0, non_got_ref = 0, dynamic_def = 0, ref_dynamic_nonweak = 0,
  pointer_equality_needed = 1, unique_global = 0, protected_def = 0,
  start_stop = 0, is_weakalias = 0, dynstr_index = 0, u = {alias = 0x0,
    elf_hash_value = 0}, verinfo = {verdef = 0x0, vertree = 0x0}, u2 = {
    start_stop_section = 0x0, vtable = 0x0}}
(gdb)

I think my patch can be dropped.

Thanks.

-- 
H.J.

Re: [PATCH] ld: Add ehdr_start to bfd_link_hash_entry

[Alan Modra]

On Tue, Jan 04, 2022 at 06:10:35PM -0800, H.J. Lu wrote:
> I think my patch can be dropped.

OK, I'm dropping any attempt at changing the __ehdr_start handling in
ldelf_before_allocation too.

-- 
Alan Modra
Australia Development Lab, IBM