public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: gdb-patches@sourceware.org
Cc: Binutils <binutils@sourceware.org>
Subject: [PATCH v2 13/15] PPC64: always make synthetic .text symbols for GNU ifunc symbols
Date: Sun, 25 Mar 2018 19:29:00 -0000	[thread overview]
Message-ID: <20180325191943.8246-14-palves@redhat.com> (raw)
In-Reply-To: <20180325191943.8246-1-palves@redhat.com>

If you create an ifunc using GCC's __attribute__ ifunc, like:

 extern int gnu_ifunc (int arg);
 static int gnu_ifunc_target (int arg) { return 0; }
 __typeof (gnu_ifunc) *gnu_ifunc_resolver (unsigned long hwcap) { return gnu_ifunc_target; }
 __typeof (gnu_ifunc) gnu_ifunc __attribute__ ((ifunc ("gnu_ifunc_resolver")));

then you end up with two (function descriptor) symbols, one for the
ifunc itself, and another for the resolver:

   (...)
   12: 0000000000020060    104 FUNC    GLOBAL DEFAULT       18 gnu_ifunc_resolver
   (...)
   16: 0000000000020060    104 GNU_IFUNC GLOBAL DEFAULT       18 gnu_ifunc
   (...)

Both ifunc and resolver symbols have the same address/value, so
ppc64_elf_get_synthetic_symtab only creates a synthetic text symbol
for one of them.  In the case above, it ends up being created for the
resolver, only:

  (gdb) maint print msymbols
  (...)
  [ 7] t 0x980 .frame_dummy section .text
  [ 8] T 0x9e4 .gnu_ifunc_resolver section .text
  [ 9] T 0xa58 __glink_PLTresolve section .text
  (...)

GDB needs to know when a program stepped into an ifunc resolver, so
that it can know whether to step past the resolver into the target
function without the user noticing.  The way GDB does it is my
checking whether the current PC points to an ifunc symbol (since
resolver and ifunc have the same address by design).

The problem is then that ppc64_elf_get_synthetic_symtab never creates
the synchetic symbol for the ifunc, so GDB stops stepping at the
resolver (in a test added by the following patch):

  (gdb) step
  gnu_ifunc_resolver (hwcap=21) at gdb/testsuite/gdb.base/gnu-ifunc-lib.c:33
  33      {
  (gdb) FAIL: gdb.base/gnu-ifunc.exp: resolver_attr=1: resolver_debug=1: final_debug=0: step

After this commit, we get:

  [ 8] i 0x9e4 .gnu_ifunc section .text
  [ 9] T 0x9e4 .gnu_ifunc_resolver section .text

And stepping an ifunc call takes to the final function:
  (gdb) step
  0x00000000100009e8 in .final ()
  (gdb) PASS: gdb.base/gnu-ifunc.exp: resolver_attr=1: resolver_debug=1: final_debug=0: step

An alternative to touching bfd I considered was for GDB to check
whether there's an ifunc data symbol / function descriptor that points
to the current PC, whenever the program stops, but discarded it
because we'd have to do a linear scan over .opd over an over to find a
matching function descriptor for the current PC.  At that point I
considered caching that info, but quickly dismissed it as then that
has no advantage (memory or performance) over just creating the
synthetic ifunc text symbol in the first place.

I ran the binutils and ld testsuites on PPC64 ELFv1 (machine gcc110 on
the GCC compile farm), and saw no regressions.  This commit is part of
a GDB patch series that includes GDB tests that fail without this fix.

OK to apply?

bfd/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* elf64-ppc.c (ppc64_elf_get_synthetic_symtab): Don't consider
	ifunc and non-ifunc symbols duplicates.
---
 bfd/elf64-ppc.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index 751ad778b26..791ec49b73d 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -3330,13 +3330,23 @@ ppc64_elf_get_synthetic_symtab (bfd *abfd,
 
       if (!relocatable && symcount > 1)
 	{
-	  /* Trim duplicate syms, since we may have merged the normal and
-	     dynamic symbols.  Actually, we only care about syms that have
-	     different values, so trim any with the same value.  */
+	  /* Trim duplicate syms, since we may have merged the normal
+	     and dynamic symbols.  Actually, we only care about syms
+	     that have different values, so trim any with the same
+	     value.  Don't consider ifunc and ifunc resolver symbols
+	     duplicates however, because GDB wants to know whether a
+	     text symbol is an ifunc resolver.  */
 	  for (i = 1, j = 1; i < symcount; ++i)
-	    if (syms[i - 1]->value + syms[i - 1]->section->vma
-		!= syms[i]->value + syms[i]->section->vma)
-	      syms[j++] = syms[i];
+	    {
+	      const asymbol *s0 = syms[i - 1];
+	      const asymbol *s1 = syms[i];
+
+	      if ((s0->value + s0->section->vma
+		   != s1->value + s1->section->vma)
+		  || ((s0->flags & BSF_GNU_INDIRECT_FUNCTION)
+		      != (s1->flags & BSF_GNU_INDIRECT_FUNCTION)))
+		syms[j++] = syms[i];
+	    }
 	  symcount = j;
 	}
 
-- 
2.14.3

  parent reply	other threads:[~2018-03-25 19:29 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-25 19:19 [PATCH v2 00/15] Fixing GNU ifunc support Pedro Alves
2018-03-25 19:19 ` [PATCH v2 03/15] Calling ifunc functions when target has no debug info but resolver has Pedro Alves
2018-04-01  4:22   ` Simon Marchi
2018-04-10 21:48     ` Pedro Alves
2018-04-10 21:54       ` Pedro Alves
2018-03-25 19:19 ` [PATCH v2 15/15] Fix resolving GNU ifunc bp locations when inferior runs resolver Pedro Alves
2018-03-25 19:19 ` [PATCH v2 08/15] Eliminate find_pc_partial_function_gnu_ifunc Pedro Alves
2018-03-25 19:19 ` [PATCH v2 05/15] Fix elf_gnu_ifunc_resolve_by_got buglet Pedro Alves
2018-04-01  4:32   ` Simon Marchi
2018-04-10 21:52     ` Pedro Alves
2018-03-25 19:19 ` [PATCH v2 01/15] Fix breakpoints in ifunc after inferior resolved it (@got.plt symbol creation) Pedro Alves
2018-04-01  3:35   ` Simon Marchi
2018-04-10 21:20     ` Pedro Alves
2018-04-14 16:36       ` Simon Marchi
2018-03-25 19:19 ` [PATCH v2 02/15] Fix calling ifunc functions when resolver has debug info and different name Pedro Alves
2018-04-01  3:44   ` Simon Marchi
2018-04-10 21:20     ` Pedro Alves
2018-03-25 19:19 ` [PATCH v2 07/15] Breakpoints, don't skip prologue of ifunc resolvers with debug info Pedro Alves
2018-03-25 19:19 ` [PATCH v2 11/15] Fix stepping past GNU ifunc resolvers (introduce lookup_msym_prefer) Pedro Alves
2018-06-18 20:26   ` [PATCH] Silence GCC "uninitialized" warning on minsyms.c:lookup_minimal_symbol_by_pc_section Sergio Durigan Junior
2018-06-19 15:22     ` Pedro Alves
2018-06-19 16:55       ` Sergio Durigan Junior
2018-06-19 18:47       ` Tom Tromey
2018-03-25 19:19 ` [PATCH v2 12/15] For PPC64/ELFv1: Introduce mst_data_gnu_ifunc Pedro Alves
2018-03-25 19:25 ` [PATCH v2 04/15] Calling ifunc functions when resolver has debug info, user symbol same name Pedro Alves
2018-03-25 19:25 ` [PATCH v2 09/15] Factor out minsym_found/find_function_start_sal overload Pedro Alves
2018-03-25 19:28 ` [PATCH v2 14/15] Extend GNU ifunc testcases Pedro Alves
2018-03-25 19:29 ` [PATCH v2 06/15] Fix setting breakpoints on ifunc functions after they're already resolved Pedro Alves
2018-03-25 19:29 ` [PATCH v2 10/15] For PPC64: elf_gnu_ifunc_record_cache: handle plt symbols in .text section Pedro Alves
2018-03-25 19:29 ` Pedro Alves [this message]
2018-03-25 19:33   ` [PATCH v2 13/15] PPC64: always make synthetic .text symbols for GNU ifunc symbols Pedro Alves
2018-03-26  7:54   ` Alan Modra
2018-04-26 12:23 ` [PATCH v2 00/15] Fixing GNU ifunc support Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180325191943.8246-14-palves@redhat.com \
    --to=palves@redhat.com \
    --cc=binutils@sourceware.org \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).