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 25/01/2019 09:37, Sebastian Huber wrote:


On 23/01/2019 10:11, Sebastian Huber wrote:
On 18/01/2019 14:41, Alan Modra wrote:
On Fri, Jan 18, 2019 at 10:04:36AM +0100, Sebastian Huber wrote:
On 18/01/2019 01:24, Alan Modra wrote:
No, -ffunction-sections will make no difference.  Really, --wrap was
intended for wrapping system functions, which I guess is why the
feature was implemented only for undefined symbols.  I don't see a
fundamental reason why --wrap couldn't be made to work with defined
symbols, provided the compiler and assembler don't optimise references
to local functions.
In case it is acceptable to extend --wrap to work also for defined symbol references, then I would have a look at this and try to implement it. If you
already have an idea which functions needs to be touched for this new
feature, then this would be helpful for me.
It might just be a matter of calling bfd_wrapped_link_hash_lookup in
more places where we currently call bfd_link_hash_lookup and its
derivatives like elf_link_hash_lookup.  Knowing which places to change
is the difficult part.  Anything involved with relocation processing
for a start.


I think to wrap defined references you have to add things to other places than spots which call bfd_link_hash_lookup() or bfd_wrapped_link_hash_lookup(). I built ld with -O0 -g and used the following test program:

void f(void)
{
}

void g(void);

void _start(void)
{
    f();
    g();
}

I use this GDB script:

b bfd_wrapped_link_hash_lookup  if string[0] != '.' && (string[0] == 'f' || string[7] == 'f' || string[0] == 'g' || string[7] == 'g')
commands
c
end
b bfd_link_hash_lookup if string[0] != '.' && (string[0] == 'f' || string[7] == 'f' || string[0] == 'g' || string[7] == 'g')
commands
c
end
r

This yields:

gdb --command=main.gdb --args ld-new -wrap=f -wrap=g main.o -o main.exe
Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f2bd0 "f", create=1, copy=0, follow=0) at ../../bfd/linker.c:511
511       if (table == NULL || string == NULL)

Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f2bd2 "_start", create=1, copy=0, follow=0) at ../../bfd/linker.c:511
511       if (table == NULL || string == NULL)

Breakpoint 1, bfd_wrapped_link_hash_lookup (abfd=0x8eec40, info=0x8c2de0 <link_info>, string=0x8f2bd9 "g", create=1, copy=0, follow=0) at ../../bfd/linker.c:541
541       if (info->wrap_hash != NULL)

Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f35c0 "__wrap_g", create=1, copy=1, follow=0) at ../../bfd/linker.c:511
511       if (table == NULL || string == NULL)
/home/EB/sebastian_h/archive/binutils-git/build/ld/ld-new: main.o: in function `_start':
main.c:(.text+0x11): undefined reference to `__wrap_g'

The lookup is performed only once for "f". I think this is when ld finds the definition of f(). For the call in _start() we end up in other areas of ld.


Wrapping of undefined symbol references are dealt with during load_symbols()
which is performed early during the link process.

For defined symbols references we have to look at the relocations. During the final link performed by bfd_elf_final_link() which calls elf_link_input_bfd() which calls the architecture-specific elf_x86_64_relocate_section() we end up
in (elf-bfd.h):

/* This macro is to avoid lots of duplicated code in the body
   of xxx_relocate_section() in the various elfxx-xxxx.c files. */
#define RELOC_FOR_GLOBAL_SYMBOL(info, input_bfd, input_section, rel,    \
                r_symndx, symtab_hdr, sym_hashes,    \
                h, sec, relocation,            \
                unresolved_reloc, warned, ignored)    \
  do                                    \
    {                                    \
      /* It seems this can happen with erroneous or unsupported     \
     input (mixing a.out and elf in an archive, for example.)  */ \
      if (sym_hashes == NULL)                        \
    return FALSE;                            \
                                    \
      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));     \

Why is there a special case for input_section->flags & SEC_DEBUGGING) != 0 here?

If I extend this macro in case info->wrap_hash != NULL to do the symbol wrapping, then there is an issue with the __real_SYMBOL references which are no longer visible here. We only see SYMBOL and __wrap_SYMBOL. The SYMBOL could be a defined reference or a former __real_SYMBOL undefined reference.

Would it be feasible to the the wrapping at the relocation step and not during load_symbols()?


With this quick and dirty hack I can wrap at relocation level. There is a NULL pointer access in case of unresolved references.

--
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 35c495430dd37955ae345b64a3da2c15c3fa7e97 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] HACK: Do LD --wrap at relocation level

There is a NULL pointer access in case of unresolved references in
RELOC_FOR_GLOBAL_SYMBOL().
---
 bfd/elf-bfd.h     |   2 ++
 bfd/linker.c      | 104 ++++++++++++++++++++++++++++--------------------------
 include/bfdlink.h |   3 ++
 3 files changed, 59 insertions(+), 50 deletions(-)

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 5741c60264..5a1f29b819 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -2780,6 +2780,8 @@ extern asection _bfd_elf_large_com_section;
 	return FALSE;							\
 									\
       h = sym_hashes[r_symndx - symtab_hdr->sh_info];			\
+      h = ((struct elf_link_hash_entry *)				\
+	   wrap_hash_lookup (info, input_bfd, &h->root));		\
 									\
       if (info->wrap_hash != NULL					\
 	  && (input_section->flags & SEC_DEBUGGING) != 0)		\
diff --git a/bfd/linker.c b/bfd/linker.c
index cf7d673f59..9194fb3a0c 100644
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -536,28 +536,69 @@ bfd_wrapped_link_hash_lookup (bfd *abfd,
 			      bfd_boolean copy,
 			      bfd_boolean follow)
 {
-  bfd_size_type amt;
+  (void) abfd;
+  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.  */
 
+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;
+
+  if (*l == bfd_get_symbol_leading_char (input_bfd)
+      || *l == info->wrap_char)
+    ++l;
+
+#undef WRAP
+#define WRAP "__wrap_"
+
+  if (CONST_STRNEQ (l, WRAP))
+    {
+      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;
+	}
+    }
+  return h;
+}
+
+struct bfd_link_hash_entry *
+wrap_hash_lookup (struct bfd_link_info *info,
+		  bfd *input_bfd,
+		  struct bfd_link_hash_entry *h)
+{
   if (info->wrap_hash != NULL)
     {
       const char *l;
       char prefix = '\0';
+      bfd_size_type amt;
+      char *n;
 
-      l = string;
-      if (*l == bfd_get_symbol_leading_char (abfd) || *l == info->wrap_char)
+      l = h->root.string;
+      if (*l == bfd_get_symbol_leading_char (input_bfd) || *l == info->wrap_char)
 	{
 	  prefix = *l;
 	  ++l;
 	}
 
-#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.  */
 
@@ -570,11 +611,13 @@ bfd_wrapped_link_hash_lookup (bfd *abfd,
 	  n[1] = '\0';
 	  strcat (n, WRAP);
 	  strcat (n, l);
-	  h = bfd_link_hash_lookup (info->hash, n, create, TRUE, follow);
+	  h = bfd_link_hash_lookup (info->hash, n, FALSE, FALSE, FALSE);
 	  free (n);
 	  return h;
 	}
 
+#undef WRAP
+
 #undef  REAL
 #define REAL "__real_"
 
@@ -583,9 +626,6 @@ bfd_wrapped_link_hash_lookup (bfd *abfd,
 	  && bfd_hash_lookup (info->wrap_hash, l + sizeof REAL - 1,
 			      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.  */
@@ -598,7 +638,7 @@ bfd_wrapped_link_hash_lookup (bfd *abfd,
 	  n[0] = prefix;
 	  n[1] = '\0';
 	  strcat (n, l + sizeof REAL - 1);
-	  h = bfd_link_hash_lookup (info->hash, n, create, TRUE, follow);
+	  h = bfd_link_hash_lookup (info->hash, n, FALSE, FALSE, FALSE);
 	  free (n);
 	  return h;
 	}
@@ -606,44 +646,8 @@ bfd_wrapped_link_hash_lookup (bfd *abfd,
 #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.  */
-
-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;
-
-  if (*l == bfd_get_symbol_leading_char (input_bfd)
-      || *l == info->wrap_char)
-    ++l;
-
-  if (CONST_STRNEQ (l, WRAP))
-    {
-      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;
-	}
-    }
   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
diff --git a/include/bfdlink.h b/include/bfdlink.h
index bad52f9c50..478786c4fe 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -227,6 +227,9 @@ extern struct bfd_link_hash_entry *bfd_wrapped_link_hash_lookup
 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
+  (struct bfd_link_info *, bfd *, struct bfd_link_hash_entry *);
+
 /* Traverse a link hash table.  */
 extern void bfd_link_hash_traverse
   (struct bfd_link_hash_table *,
-- 
2.16.4


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