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: Does the LD --wrap feature work for library internal references?


On 26/01/2019 00:52, Alan Modra wrote:
On Fri, Jan 25, 2019 at 02:55:35PM +0100, Sebastian Huber wrote:
@@ -2780,6 +2780,8 @@ extern asection _bfd_elf_large_com_section;
  	return FALSE;							\
  									\
        h = sym_hashes[r_symndx - symtab_hdr->sh_info];			\
Is it possible to arrange for the symbol hashes to be set up to point
to the wrapped symbols?  I'm thinking a change to always use
bfd_wrapped_link_hash_lookup in _bfd_generic_link_add_one_symbol might
work.

Is this hash table only used for the relocations? We need mappings for

__real_S -> S
__wrap_S -> __wrap_S
S -> __wrap_S

I don't know why there is a special case for debug sections in RELOC_FOR_GLOBAL_SYMBOL():

      if (info->wrap_hash != NULL
          && (input_section->flags & SEC_DEBUGGING) != 0)
        eh = ((struct elf_link_hash_entry *)
          unwrap_hash_lookup (info, input_bfd, &eh->root));

Replacing bfd_link_hash_lookup() with bfd_wrapped_link_hash_lookup() in _bfd_generic_link_add_one_symbol() is not enough. The wrapping needs to be added at least to this area in the function as well:

  do
    {
      enum link_action action;
      int prev;

      prev = h->type;
      /* Treat symbols defined by early linker script pass as undefined.  */
      if (h->ldscript_def)
    prev = bfd_link_hash_undefined;
      cycle = FALSE;
      action = link_action[(int) row][prev];
      switch (action)
    {
    case FAIL:
      abort ();

1473          switch (action)
(gdb) p action
$1 = DEF
(gdb) bt
#0  _bfd_generic_link_add_one_symbol (info=0x8c2de0 <link_info>, abfd=0x8eec40, name=0x8f2c00 "f", flags=2, section=0x8efe20, value=0, string=0x0, copy=0, collect=0, hashp=0x8f2bb0) at ../../bfd/linker.c:1473 #1  0x00000000004a3172 in elf_link_add_object_symbols (abfd=0x8eec40, info=0x8c2de0 <link_info>) at ../../bfd/elflink.c:4701 #2  0x00000000004a57dd in bfd_elf_link_add_symbols (abfd=0x8eec40, info=0x8c2de0 <link_info>) at ../../bfd/elflink.c:5740 #3  0x0000000000411cb5 in load_symbols (entry=0x8c6390, place=0x7fffffffd5e0) at ../../ld/ldlang.c:3080 #4  0x000000000041298c in open_input_bfds (s=0x8c6390, mode=OPEN_BFD_NORMAL) at ../../ld/ldlang.c:3529
#5  0x00000000004197c5 in lang_process () at ../../ld/ldlang.c:7383
#6  0x000000000041def9 in main (argc=6, argv=0x7fffffffd838) at ../../ld/ldmain.c:440

I change it like this to see what happens:

diff --git a/bfd/linker.c b/bfd/linker.c
index cf7d673f59..508ccac4c1 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -1431,20 +1431,12 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
   else
     row = DEF_ROW;

-  if (hashp != NULL && *hashp != NULL)
-    h = *hashp;
-  else
+  h = bfd_wrapped_link_hash_lookup (abfd, info, name, TRUE, copy, FALSE);
+  if (h == NULL)
     {
-      if (row == UNDEF_ROW || row == UNDEFW_ROW)
-       h = bfd_wrapped_link_hash_lookup (abfd, info, name, TRUE, copy, FALSE);
-      else
-       h = bfd_link_hash_lookup (info->hash, name, TRUE, copy, FALSE);
-      if (h == NULL)
-       {
-         if (hashp != NULL)
-           *hashp = NULL;
-         return FALSE;
-       }
+      if (hashp != NULL)
+       *hashp = NULL;
+      return FALSE;
     }

   if (info->notice_all

This leads to multiple definition errors and unresolved references:

build/ld/ld-new: main2.o: in function `__wrap_f':
main2.c:(.text+0x11): multiple definition of `__wrap_f'; main2.o:main2.c:(.text+0x0): first defined here
build/ld/ld-new: g.o: in function `g':
g.c:(.text+0x0): multiple definition of `__wrap_g'; main2.o:main2.c:(.text+0x2c): first defined here
build/ld/ld-new: main2.o: in function `__wrap_f':
main2.c:(.text+0x25): undefined reference to `f'
build/ld/ld-new: main2.o: in function `__wrap_g':
main2.c:(.text+0x40): undefined reference to `g'

I am not sure if the wrapping can be done using only the symbol hash table. We have the definition of the symbol and the references to it.

Attached is a second version which does the wrapping in RELOC_FOR_GLOBAL_SYMBOL(). It removes bfd_wrapped_link_hash_lookup(). I am not sure if this is a good idea. This function is used in various places. Maybe the wrapping needs to be done on other spots and not only in RELOC_FOR_GLOBAL_SYMBOL().

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax     : +49 89 189 47 41-09
E-Mail  : sebastian.huber@embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

>From 0b0c3ab7d52bfcb3f8dc5f54194f9e20550a61c8 Mon Sep 17 00:00:00 2001
From: Sebastian Huber <sebastian.huber@embedded-brains.de>
Date: Fri, 25 Jan 2019 14:53:04 +0100
Subject: [PATCH v2] HACK: Do LD --wrap at relocation level

This patch removes bfd_wrapped_link_hash_lookup().
---
 bfd/cofflink.c    |   6 +-
 bfd/elf-bfd.h     |  12 ++--
 bfd/elflink.c     |  11 +---
 bfd/linker.c      | 190 +++++++++++++++++++++---------------------------------
 include/bfdlink.h |   8 +--
 ld/ldexp.c        |  18 ++----
 ld/plugin.c       |   6 +-
 7 files changed, 98 insertions(+), 153 deletions(-)

diff --git a/bfd/cofflink.c b/bfd/cofflink.c
index e4031b9a31..3124d2cd7f 100644
--- a/bfd/cofflink.c
+++ b/bfd/cofflink.c
@@ -2884,9 +2884,9 @@ _bfd_coff_reloc_link_order (bfd *output_bfd,
       struct coff_link_hash_entry *h;
 
       h = ((struct coff_link_hash_entry *)
-	   bfd_wrapped_link_hash_lookup (output_bfd, flaginfo->info,
-					 link_order->u.reloc.p->u.name,
-					 FALSE, FALSE, TRUE));
+	   bfd_link_hash_lookup (flaginfo->info->hash,
+				 link_order->u.reloc.p->u.name,
+				 FALSE, FALSE, TRUE));
       if (h != NULL)
 	{
 	  if (h->indx >= 0)
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 5741c60264..b4985b442a 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2781,10 +2781,14 @@ extern asection _bfd_elf_large_com_section;
 									\
       h = sym_hashes[r_symndx - symtab_hdr->sh_info];			\
 									\
-      if (info->wrap_hash != NULL					\
-	  && (input_section->flags & SEC_DEBUGGING) != 0)		\
-	h = ((struct elf_link_hash_entry *)				\
-	     unwrap_hash_lookup (info, input_bfd, &h->root));		\
+      if (info->wrap_hash != NULL)					\
+	{								\
+	  h = (struct elf_link_hash_entry *)				\
+	      wrap_hash_lookup_for_reloc (info, input_bfd,		\
+					  input_section, &h->root);	\
+	  if (h == NULL)						\
+	    return FALSE;						\
+	}								\
 									\
       while (h->root.type == bfd_link_hash_indirect			\
 	     || h->root.type == bfd_link_hash_warning)			\
diff --git a/bfd/elflink.c b/bfd/elflink.c
index e50c0e4b38..cb722cf643 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -1069,11 +1069,7 @@ _bfd_elf_merge_symbol (bfd *abfd,
   sec = *psec;
   bind = ELF_ST_BIND (sym->st_info);
 
-  if (! bfd_is_und_section (sec))
-    h = elf_link_hash_lookup (elf_hash_table (info), name, TRUE, FALSE, FALSE);
-  else
-    h = ((struct elf_link_hash_entry *)
-	 bfd_wrapped_link_hash_lookup (abfd, info, name, TRUE, FALSE, FALSE));
+  h = elf_link_hash_lookup (elf_hash_table (info), name, TRUE, FALSE, FALSE);
   if (h == NULL)
     return FALSE;
   *sym_hash = h;
@@ -11259,9 +11255,8 @@ elf_reloc_link_order (bfd *output_bfd,
       /* Treat a reloc against a defined symbol as though it were
 	 actually against the section.  */
       h = ((struct elf_link_hash_entry *)
-	   bfd_wrapped_link_hash_lookup (output_bfd, info,
-					 link_order->u.reloc.p->u.name,
-					 FALSE, FALSE, TRUE));
+	   bfd_link_hash_lookup (info->hash, link_order->u.reloc.p->u.name,
+				 FALSE, FALSE, TRUE));
       if (h != NULL
 	  && (h->root.type == bfd_link_hash_defined
 	      || h->root.type == bfd_link_hash_defweak))
diff --git a/bfd/linker.c b/bfd/linker.c
index cf7d673f59..c72719397e 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -524,126 +524,98 @@ bfd_link_hash_lookup (struct bfd_link_hash_table *table,
   return ret;
 }
 
-/* Look up a symbol in the main linker hash table if the symbol might
-   be wrapped.  This should only be used for references to an
-   undefined symbol, not for definitions of a symbol.  */
-
 struct bfd_link_hash_entry *
-bfd_wrapped_link_hash_lookup (bfd *abfd,
-			      struct bfd_link_info *info,
-			      const char *string,
-			      bfd_boolean create,
-			      bfd_boolean copy,
-			      bfd_boolean follow)
+wrap_hash_lookup_for_reloc (struct bfd_link_info *info,
+			    bfd *input_bfd,
+			    struct bfd_section *input_section,
+			    struct bfd_link_hash_entry *h)
 {
+  const char *l;
+  char prefix = '\0';
   bfd_size_type amt;
-
-  if (info->wrap_hash != NULL)
-    {
-      const char *l;
-      char prefix = '\0';
-
-      l = string;
-      if (*l == bfd_get_symbol_leading_char (abfd) || *l == info->wrap_char)
-	{
-	  prefix = *l;
-	  ++l;
-	}
+  char *n;
 
 #undef WRAP
 #define WRAP "__wrap_"
 
-      if (bfd_hash_lookup (info->wrap_hash, l, FALSE, FALSE) != NULL)
-	{
-	  char *n;
-	  struct bfd_link_hash_entry *h;
-
-	  /* This symbol is being wrapped.  We want to replace all
-	     references to SYM with references to __wrap_SYM.  */
+  BFD_ASSERT (info->wrap_hash != NULL);
 
-	  amt = strlen (l) + sizeof WRAP + 1;
-	  n = (char *) bfd_malloc (amt);
-	  if (n == NULL)
-	    return NULL;
-
-	  n[0] = prefix;
-	  n[1] = '\0';
-	  strcat (n, WRAP);
-	  strcat (n, l);
-	  h = bfd_link_hash_lookup (info->hash, n, create, TRUE, follow);
-	  free (n);
-	  return h;
-	}
+  l = h->root.string;
+  if (*l == bfd_get_symbol_leading_char (input_bfd) || *l == info->wrap_char)
+    {
+      prefix = *l;
+      ++l;
+    }
 
-#undef  REAL
-#define REAL "__real_"
+  if ((input_section->flags & SEC_DEBUGGING) != 0 && CONST_STRNEQ (l, WRAP))
+    {
+      const char *s = l + sizeof WRAP - 1;
 
-      if (*l == '_'
-	  && CONST_STRNEQ (l, REAL)
-	  && bfd_hash_lookup (info->wrap_hash, l + sizeof REAL - 1,
-			      FALSE, FALSE) != NULL)
+      if (bfd_hash_lookup (info->wrap_hash, s, FALSE, FALSE) != NULL)
 	{
-	  char *n;
-	  struct bfd_link_hash_entry *h;
-
-	  /* This is a reference to __real_SYM, where SYM is being
-	     wrapped.  We want to replace all references to __real_SYM
-	     with references to SYM.  */
-
-	  amt = strlen (l + sizeof REAL - 1) + 2;
-	  n = (char *) bfd_malloc (amt);
-	  if (n == NULL)
-	    return NULL;
-
-	  n[0] = prefix;
-	  n[1] = '\0';
-	  strcat (n, l + sizeof REAL - 1);
-	  h = bfd_link_hash_lookup (info->hash, n, create, TRUE, follow);
-	  free (n);
+	  char save = 0;
+	  if (s - (sizeof WRAP - 1) != h->root.string)
+	    {
+	      --s;
+	      save = *s;
+	      *(char *) s = *h->root.string;
+	    }
+	  h = bfd_link_hash_lookup (info->hash, s, FALSE, FALSE, FALSE);
+	  if (save)
+	    *(char *) s = save;
 	  return h;
 	}
-
-#undef REAL
     }
 
-  return bfd_link_hash_lookup (info->hash, string, create, copy, follow);
-}
-
-/* If H is a wrapped symbol, ie. the symbol name starts with "__wrap_"
-   and the remainder is found in wrap_hash, return the real symbol.  */
+  if (bfd_hash_lookup (info->wrap_hash, l, FALSE, FALSE) != NULL)
+    {
+      /* This symbol is being wrapped.  We want to replace all
+	 references to SYM with references to __wrap_SYM.  */
+
+      amt = strlen (l) + sizeof WRAP + 1;
+      n = (char *) bfd_malloc (amt);
+      if (n == NULL)
+	return NULL;
+
+      n[0] = prefix;
+      n[1] = '\0';
+      strcat (n, WRAP);
+      strcat (n, l);
+      h = bfd_link_hash_lookup (info->hash, n, TRUE, TRUE, FALSE);
+      free (n);
+      return h;
+    }
 
-struct bfd_link_hash_entry *
-unwrap_hash_lookup (struct bfd_link_info *info,
-		    bfd *input_bfd,
-		    struct bfd_link_hash_entry *h)
-{
-  const char *l = h->root.string;
+#undef WRAP
 
-  if (*l == bfd_get_symbol_leading_char (input_bfd)
-      || *l == info->wrap_char)
-    ++l;
+#undef  REAL
+#define REAL "__real_"
 
-  if (CONST_STRNEQ (l, WRAP))
+  if (*l == '_'
+      && CONST_STRNEQ (l, REAL)
+      && bfd_hash_lookup (info->wrap_hash, l + sizeof REAL - 1,
+			  FALSE, FALSE) != NULL)
     {
-      l += sizeof WRAP - 1;
-
-      if (bfd_hash_lookup (info->wrap_hash, l, FALSE, FALSE) != NULL)
-	{
-	  char save = 0;
-	  if (l - (sizeof WRAP - 1) != h->root.string)
-	    {
-	      --l;
-	      save = *l;
-	      *(char *) l = *h->root.string;
-	    }
-	  h = bfd_link_hash_lookup (info->hash, l, FALSE, FALSE, FALSE);
-	  if (save)
-	    *(char *) l = save;
-	}
+      /* This is a reference to __real_SYM, where SYM is being
+	 wrapped.  We want to replace all references to __real_SYM
+	 with references to SYM.  */
+
+      amt = strlen (l + sizeof REAL - 1) + 2;
+      n = (char *) bfd_malloc (amt);
+      if (n == NULL)
+	return NULL;
+
+      n[0] = prefix;
+      n[1] = '\0';
+      strcat (n, l + sizeof REAL - 1);
+      h = bfd_link_hash_lookup (info->hash, n, TRUE, TRUE, FALSE);
+      free (n);
+      return h;
     }
+
+#undef REAL
   return h;
 }
-#undef WRAP
 
 /* Traverse a generic link hash table.  Differs from bfd_hash_traverse
    in the treatment of warning symbols.  When warning symbols are
@@ -1400,8 +1372,7 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
       /* Create the indirect symbol here.  This is for the benefit of
 	 the plugin "notice" function.
 	 STRING is the name of the symbol we want to indirect to.  */
-      inh = bfd_wrapped_link_hash_lookup (abfd, info, string, TRUE,
-					  copy, FALSE);
+      inh = bfd_link_hash_lookup (info->hash, string, TRUE, copy, FALSE);
       if (inh == NULL)
 	return FALSE;
     }
@@ -1435,10 +1406,7 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info,
     h = *hashp;
   else
     {
-      if (row == UNDEF_ROW || row == UNDEFW_ROW)
-	h = bfd_wrapped_link_hash_lookup (abfd, info, name, TRUE, copy, FALSE);
-      else
-	h = bfd_link_hash_lookup (info->hash, name, TRUE, copy, FALSE);
+      h = bfd_link_hash_lookup (info->hash, name, TRUE, copy, FALSE);
       if (h == NULL)
 	{
 	  if (hashp != NULL)
@@ -2045,11 +2013,6 @@ _bfd_generic_link_output_symbols (bfd *output_bfd,
 		 the relocs in the output format being used.  */
 	      h = NULL;
 	    }
-	  else if (bfd_is_und_section (bfd_get_section (sym)))
-	    h = ((struct generic_link_hash_entry *)
-		 bfd_wrapped_link_hash_lookup (output_bfd, info,
-					       bfd_asymbol_name (sym),
-					       FALSE, FALSE, TRUE));
 	  else
 	    h = _bfd_generic_link_hash_lookup (_bfd_generic_hash_table (info),
 					       bfd_asymbol_name (sym),
@@ -2352,9 +2315,8 @@ _bfd_generic_reloc_link_order (bfd *abfd,
       struct generic_link_hash_entry *h;
 
       h = ((struct generic_link_hash_entry *)
-	   bfd_wrapped_link_hash_lookup (abfd, info,
-					 link_order->u.reloc.p->u.name,
-					 FALSE, FALSE, TRUE));
+	   bfd_link_hash_lookup (info->hash, link_order->u.reloc.p->u.name,
+				 FALSE, FALSE, TRUE));
       if (h == NULL
 	  || ! h->written)
 	{
@@ -2609,10 +2571,6 @@ default_indirect_link_order (bfd *output_bfd,
 		 generic_link_add_symbol_list.  */
 	      if (sym->udata.p != NULL)
 		h = (struct bfd_link_hash_entry *) sym->udata.p;
-	      else if (bfd_is_und_section (bfd_get_section (sym)))
-		h = bfd_wrapped_link_hash_lookup (output_bfd, info,
-						  bfd_asymbol_name (sym),
-						  FALSE, FALSE, TRUE);
 	      else
 		h = bfd_link_hash_lookup (info->hash,
 					  bfd_asymbol_name (sym),
diff --git a/include/bfdlink.h b/include/bfdlink.h
index bad52f9c50..2f3586cf11 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -221,11 +221,9 @@ extern struct bfd_link_hash_entry *bfd_wrapped_link_hash_lookup
   (bfd *, struct bfd_link_info *, const char *, bfd_boolean,
    bfd_boolean, bfd_boolean);
 
-/* If H is a wrapped symbol, ie. the symbol name starts with "__wrap_"
-   and the remainder is found in wrap_hash, return the real symbol.  */
-
-extern struct bfd_link_hash_entry *unwrap_hash_lookup
-  (struct bfd_link_info *, bfd *, struct bfd_link_hash_entry *);
+extern struct bfd_link_hash_entry *wrap_hash_lookup_for_reloc
+  (struct bfd_link_info *, bfd *, struct bfd_section *,
+   struct bfd_link_hash_entry *);
 
 /* Traverse a link hash table.  */
 extern void bfd_link_hash_traverse
diff --git a/ld/ldexp.c b/ld/ldexp.c
index 60b17ef576..f0bd5157f9 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -705,10 +705,8 @@ fold_name (etree_type *tree)
       break;
 
     case DEFINED:
-      h = bfd_wrapped_link_hash_lookup (link_info.output_bfd,
-					&link_info,
-					tree->name.name,
-					FALSE, FALSE, TRUE);
+      h = bfd_link_hash_lookup (link_info.hash, tree->name.name, FALSE, FALSE,
+				TRUE);
       new_number (h != NULL
 		  && (h->type == bfd_link_hash_defined
 		      || h->type == bfd_link_hash_defweak
@@ -724,10 +722,8 @@ fold_name (etree_type *tree)
 	new_rel_from_abs (expld.dot);
       else
 	{
-	  h = bfd_wrapped_link_hash_lookup (link_info.output_bfd,
-					    &link_info,
-					    tree->name.name,
-					    TRUE, FALSE, TRUE);
+	  h = bfd_link_hash_lookup (link_info.hash, tree->name.name, TRUE,
+				    FALSE, TRUE);
 	  if (!h)
 	    einfo (_("%F%P: bfd_link_hash_lookup failed: %E\n"));
 	  else if (h->type == bfd_link_hash_defined
@@ -943,10 +939,8 @@ is_sym_value (const etree_type *tree, bfd_vma val)
 	  && tree->type.node_code == NAME
 	  && (def = symbol_defined (tree->name.name)) != NULL
 	  && def->iteration == (lang_statement_iteration & 255)
-	  && (h = bfd_wrapped_link_hash_lookup (link_info.output_bfd,
-						&link_info,
-						tree->name.name,
-						FALSE, FALSE, TRUE)) != NULL
+	  && (h = bfd_link_hash_lookup (link_info.hash, tree->name.name,
+					FALSE, FALSE, TRUE)) != NULL
 	  && h->ldscript_def
 	  && h->type == bfd_link_hash_defined
 	  && h->u.def.section == bfd_abs_section_ptr
diff --git a/ld/plugin.c b/ld/plugin.c
index ea1a7f7064..a5566b449b 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -675,12 +675,8 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms,
       asection *owner_sec;
       int res;
 
-      if (syms[n].def != LDPK_UNDEF)
-	blhe = bfd_link_hash_lookup (link_info.hash, syms[n].name,
+      blhe = bfd_link_hash_lookup (link_info.hash, syms[n].name,
 				     FALSE, FALSE, TRUE);
-      else
-	blhe = bfd_wrapped_link_hash_lookup (link_info.output_bfd, &link_info,
-					     syms[n].name, FALSE, FALSE, TRUE);
       if (!blhe)
 	{
 	  /* The plugin is called to claim symbols in an archive element
-- 
2.16.4


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