Message ID | 20180325191943.8246-14-palves@redhat.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 109745 invoked by alias); 25 Mar 2018 19:29:23 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 109724 invoked by uid 89); 25 Mar 2018 19:29:22 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=sk:ppc64_e X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx3-rdu2.redhat.com (HELO mx1.redhat.com) (66.187.233.73) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 25 Mar 2018 19:29:21 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 295D5412A05D; Sun, 25 Mar 2018 19:19:53 +0000 (UTC) Received: from localhost.localdomain (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id AA51D202699A; Sun, 25 Mar 2018 19:19:52 +0000 (UTC) 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 20:19:41 +0100 Message-Id: <20180325191943.8246-14-palves@redhat.com> In-Reply-To: <20180325191943.8246-1-palves@redhat.com> References: <20180325191943.8246-1-palves@redhat.com> |
Commit Message
Pedro Alves
March 25, 2018, 7:19 p.m. UTC
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(-)
Comments
Dear binutils friends, This is a bfd patch that is part of a larger gdb patch series, whose cover letter is found here: https://sourceware.org/ml/gdb-patches/2018-03/msg00504.html Thanks, Pedro Alves On 03/25/2018 08:19 PM, Pedro Alves wrote: > 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; > } > >
On Sun, Mar 25, 2018 at 08:19:41PM +0100, Pedro Alves wrote: > * elf64-ppc.c (ppc64_elf_get_synthetic_symtab): Don't consider > ifunc and non-ifunc symbols duplicates. OK, thanks!
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; }