Message ID | 20180325191943.8246-3-palves@redhat.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 21972 invoked by alias); 25 Mar 2018 19:19:49 -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 21808 invoked by uid 89); 25 Mar 2018 19:19:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.3 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=Hx-languages-length:1984, noticing, that'll, thatll 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:19:47 +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 34832401CC41 for <gdb-patches@sourceware.org>; Sun, 25 Mar 2018 19:19:46 +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 DA8AE202699A for <gdb-patches@sourceware.org>; Sun, 25 Mar 2018 19:19:45 +0000 (UTC) From: Pedro Alves <palves@redhat.com> To: gdb-patches@sourceware.org Subject: [PATCH v2 02/15] Fix calling ifunc functions when resolver has debug info and different name Date: Sun, 25 Mar 2018 20:19:30 +0100 Message-Id: <20180325191943.8246-3-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
Currently, on Fedora 27 (glibc 2.26), if you try to call strlen in the inferior you get: (gdb) p strlen ("hello") $1 = (size_t (*)(const char *)) 0x7ffff554aac0 <__strlen_avx2> strlen is an ifunc function, and what we see above is the result of calling the ifunc resolver in the inferior. That returns a pointer to the actual target function that implements strlen on my machine. GDB should have turned around and called the resolver automatically without the user noticing. This is was caused by commit: commit bf223d3e808e6fec9ee165d3d48beb74837796de Date: Mon Aug 21 11:34:32 2017 +0100 Handle function aliases better (PR gdb/19487, errno printing) which added the find_function_alias_target call to c-exp.y, to try to find an alias with debug info for a minsym. For ifunc symbols, that finds the ifunc's resolver if it has debug info (in the example it's called "strlen_ifunc"), with the result that GDB calls that as a regular function. After this commit, we get now get: (top-gdb) p strlen ("hello") '__strlen_avx2' has unknown return type; cast the call to its declared return type Which is correct, because __strlen_avx2 is written in assembly. That'll be improved in a following patch, though. gdb/ChangeLog: yyyy-mm-dd Pedro Alves <palves@redhat.com> * c-exp.y (variable production): Skip finding an alias for ifunc symbols. --- gdb/c-exp.y | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Comments
On 2018-03-25 03:19 PM, Pedro Alves wrote: > Currently, on Fedora 27 (glibc 2.26), if you try to call strlen in the > inferior you get: > > (gdb) p strlen ("hello") > $1 = (size_t (*)(const char *)) 0x7ffff554aac0 <__strlen_avx2> > > strlen is an ifunc function, and what we see above is the result of > calling the ifunc resolver in the inferior. That returns a pointer to > the actual target function that implements strlen on my machine. GDB > should have turned around and called the resolver automatically > without the user noticing. > > This is was caused by commit: > > commit bf223d3e808e6fec9ee165d3d48beb74837796de > Date: Mon Aug 21 11:34:32 2017 +0100 > > Handle function aliases better (PR gdb/19487, errno printing) > > which added the find_function_alias_target call to c-exp.y, to try to > find an alias with debug info for a minsym. For ifunc symbols, that > finds the ifunc's resolver if it has debug info (in the example it's > called "strlen_ifunc"), with the result that GDB calls that as a > regular function. > > After this commit, we get now get: > > (top-gdb) p strlen ("hello") > '__strlen_avx2' has unknown return type; cast the call to its declared return type > > Which is correct, because __strlen_avx2 is written in assembly. > That'll be improved in a following patch, though. > > gdb/ChangeLog: > yyyy-mm-dd Pedro Alves <palves@redhat.com> > > * c-exp.y (variable production): Skip finding an alias for ifunc > symbols. > --- > gdb/c-exp.y | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/gdb/c-exp.y b/gdb/c-exp.y > index 8dc3c068a5e..e2ea07cd792 100644 > --- a/gdb/c-exp.y > +++ b/gdb/c-exp.y > @@ -1081,7 +1081,9 @@ variable: name_not_typename > is important for example for "p > *__errno_location()". */ > symbol *alias_target > - = find_function_alias_target (msymbol); > + = (msymbol.minsym->type != mst_text_gnu_ifunc > + ? find_function_alias_target (msymbol) > + : NULL); > if (alias_target != NULL) > { > write_exp_elt_opcode (pstate, OP_VAR_VALUE); > Just one question, to which I really don't have a preconceived answer: Would it make sense to add that filtering to find_function_alias_target or another function deeper in the call chain instead? In other words, would it ever make sense for find_function_alias_target to return an non-NULL result for an mst_text_gnu_ifunc minimal symbol? Simon Simon
Hi Simon, On 04/01/2018 04:44 AM, Simon Marchi wrote: >> diff --git a/gdb/c-exp.y b/gdb/c-exp.y >> index 8dc3c068a5e..e2ea07cd792 100644 >> --- a/gdb/c-exp.y >> +++ b/gdb/c-exp.y >> @@ -1081,7 +1081,9 @@ variable: name_not_typename >> is important for example for "p >> *__errno_location()". */ >> symbol *alias_target >> - = find_function_alias_target (msymbol); >> + = (msymbol.minsym->type != mst_text_gnu_ifunc >> + ? find_function_alias_target (msymbol) >> + : NULL); >> if (alias_target != NULL) >> { >> write_exp_elt_opcode (pstate, OP_VAR_VALUE); >> > > Just one question, to which I really don't have a preconceived answer: > > Would it make sense to add that filtering to find_function_alias_target or > another function deeper in the call chain instead? In other words, would > it ever make sense for find_function_alias_target to return an non-NULL > result for an mst_text_gnu_ifunc minimal symbol? For ifuncs, find_function_alias_target will finds the debug symbol for the resolver. If that has a different name from the ifunc (it will if you use gcc's attribute ifunc, then it'll work the same as any other minsym, in the sense that it'll find an alias. So from that angle, the function works as intended, and it wasn't clear that we'll always want to treat ifuncs differently. Note this is the only caller of find_function_alias_target currently. Another reason for leaving the check here is that patch #4 adds another case of "do this for ifuncs" just above this code, but for the "sym.symbol" case. It felt better to keep both of the "for ifunc, do this" cases close together. Thanks, Pedro Alves
diff --git a/gdb/c-exp.y b/gdb/c-exp.y index 8dc3c068a5e..e2ea07cd792 100644 --- a/gdb/c-exp.y +++ b/gdb/c-exp.y @@ -1081,7 +1081,9 @@ variable: name_not_typename is important for example for "p *__errno_location()". */ symbol *alias_target - = find_function_alias_target (msymbol); + = (msymbol.minsym->type != mst_text_gnu_ifunc + ? find_function_alias_target (msymbol) + : NULL); if (alias_target != NULL) { write_exp_elt_opcode (pstate, OP_VAR_VALUE);